refactor: replace dependency on Block with BlockContext and Chunks on apply chunk
#12746
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12746 +/- ##
===========================================
+ Coverage 1.35% 70.65% +69.29%
===========================================
Files 670 849 +179
Lines 120927 174555 +53628
Branches 120927 174555 +53628
===========================================
+ Hits 1637 123328 +121691
+ Misses 119201 46062 -73139
- Partials 89 5165 +5076
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
chain/chain/src/chain.rs
Outdated
| @@ -3722,30 +3742,38 @@ impl Chain { | |||
| fn get_update_shard_job( | |||
| &self, | |||
| me: &Option<AccountId>, | |||
| block: &Block, | |||
| // TODO(#10584): introduce separate structure which can be derived from | |||
There was a problem hiding this comment.
This can be an enum that either has Block or OptimisticBlock.
There was a problem hiding this comment.
When looking into that, I realised I can just use ApplyChunkBlockContext because that's all what matters. It led to one more change - InvalidChunkProofs must be generated outside the method. But it's not an important error to support precisely, and we won't be able to return such errors while having optimistic block anyway.
chain/chain/src/chain.rs
Outdated
| let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(prev_hash)?; | ||
| let shard_layout = self.epoch_manager.get_shard_layout(&epoch_id)?; | ||
| let shard_id = shard_layout.get_shard_id(shard_index)?; | ||
| let chunk_header = &chunk_headers[shard_index]; |
There was a problem hiding this comment.
In Chunks struct, they say that Index trait is deprecated and recommends using .get() instead. Just to align with the new resharding implementattion.
| let chunk_header = &chunk_headers[shard_index]; | |
| let chunk_header = &chunk_headers.get(shard_index); |
Co-authored-by: Razvan Barbascu <r.barbascu@gmail.com>
|
Please note I did more refactoring as response to #12746 (comment). |
Block with BlockHeader and Chunks on apply chunkBlock with BlockContext and Chunks on apply chunk
Pull Request is not mergeable
Context
We want to improve chunk processing efficiency by applying chunks optimistically (#10584), when all partial chunks for the next height and block metadata (from an
OptimisticBlock) are already available. This allows the results of chunk application to be reused when the actual block is received, enabling the next chunk to be produced immediately.Currently, this work is a step towards supporting
OptimisticBlock. WhileOptimisticBlockis not introduced yet, this refactor prepares the codebase for its implementation by reducing data dependency on the current block.Change
I replaced the dependency on
BlockwithApplyChunkBlockContextandchunk_headers: &Chunks. We'll just need to add conversion fromOptimisticBlocktoApplyChunkBlockContextlater. Chunk headers are just taken from block, and for optimistic block, they must be supplied by ShardsManager.Some APIs are refactored to reflect that change.
Next steps
OptimisticBlocktoApplyChunkBlockContext.get_update_shard_jobforOptimisticBlockand reuse result if it is called for the actualBlock.