Use the block import notification fired after the completion of block import on the domain side #1217
Conversation
Unlike the block import notification from the subspace consensus fired before the completion of block import, `every_block_import_stream()` in client will notify the every imported primary block after the completion of block import pipeline. Although now domain actually consumes the block import notification from the client, we still keep the notification from sc-consensus-subspace in order to pause the block import when needed. The sematic of block import throttling size is changed but should be fine, previously the buffer was 100 blocks, which means the primary chain can be ahead of the domain up to 100 blocks, now the buffer size of 100 blocks means the primary chain may can only ahead of the domain up to 50 blocks.
fork_choice is no longer needed in the sc-consensus-subspace block import notification and domain block info.
|
Just to confirm, this still supports starting executor later after primary chain synced to some height, correct? This was a question from SDK team and generally important feature to keep. |
No, and it's never true actually. This PR remains the same executor behavior as before, only the implementation detail is changed. If you want to run executor, it has to be specified when the primary node starts from genesis at present, but we can support this feature if the primary node keeps all the historical blocks. @nazar-pc |
| // from the corresponding primary block, it's fine to forcibly always use | ||
| // the longest chain for simplicity as we manually build all the domain | ||
| // branches by literally following the primary chain branches anyway. | ||
| let fork_choice = ForkChoiceStrategy::LongestChain; |
There was a problem hiding this comment.
Can you remind me: do we finalize on execution chain the same blocks that are finalized on the primary chain?
There was a problem hiding this comment.
No, we don't follow the finalize event from primary chain right now. We briefly talked about this before, but never made it on the list :P, tracked in #1246 now.
sc-consensus-subspaceis kept as we still need to pause the primary block import if needed, but the semantic of block import throttling size is slight changed, now when the block import throttling buffer size is 100 blocks, the primary chain may be ahead of the domain chain up to 50 (100/2) blocks.Close #1143
Code contributor checklist: