feat(optimistic_block): shuffle incoming receipts based on prev hash #12777
Conversation
Could you elaborate? In fact, this shuffling is only about order of source shards from which receipts are taken, receipts within a shard don't change their order. Yeah CPs will know order of source shards when they produce chunk, but I don't see how this can be used maliciously. |
There was a problem hiding this comment.
I took a better look at this, and it seems that the receipts that we are working with here are generated in the previous block (or older), and we are using the prev block hash.
This prev block hash is unknown to the chunk producers for those heights.
Given this, it looks safe.
Yeah that's also true :) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12777 +/- ##
==========================================
- Coverage 70.51% 70.51% -0.01%
==========================================
Files 846 846
Lines 174633 174649 +16
Branches 174633 174649 +16
==========================================
+ Hits 123149 123157 +8
- Misses 46254 46259 +5
- Partials 5230 5233 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
#10584
There is another unexpected dependency on block hash during chunk application - it is used in
shuffle_receipt_proofsto shuffle new receipts targeting our shard. As block hash is unknown in optimistic block, I replace it with prev block hash with a protocol upgrade.Additionally, use
Chunksinstead ofBlockincollect_incoming_receipts_from_chunks- it will be useful for optimistic block execution flow later.Security
Some block producer can brute force hashes to get salt which gives more desirable order. But block hash is prone to that as well, prev hash has equivalent safety.
Upgrade
I use
BlockHeightForReceiptIdfeature because it has similar goal and it is going to be released soon. Adding separate feature makes code harder to read I think.Testing
IMO it makes sense only to check consistency of the shuffling, I don't see much value in checking that specific salt is used. So I claim that running existing tests is enough to test that change.