wip: pruned gap sync with ConsensusBroadcast origin#9678
wip: pruned gap sync with ConsensusBroadcast origin#9678
ConsensusBroadcast origin#9678Conversation
| // Calculate the weight of the block in case it is missing. | ||
| let stored_weight = aux_schema::load_block_weight(&*self.client, hash) | ||
| .map_err(|e| ConsensusError::ClientImport(e.to_string()))?; | ||
| // If the stored weight is missing, it means it was skipped when the block was first | ||
| // imported. It needs to happen again, along with epoch change tracking. | ||
| if stored_weight.is_some() { | ||
| // When re-importing existing block strip away intermediates. | ||
| // In case of initial sync intermediates should not be present... | ||
| let _ = block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY); | ||
| block.fork_choice = Some(ForkChoiceStrategy::Custom(false)); | ||
| return self.inner.import_block(block).await.map_err(Into::into) | ||
| } |
There was a problem hiding this comment.
This is the really ugly part - first warp sync imports the block, but skips a bunch of stuff, including the weight calculation. Then when re-importing we check if the weight is missing, and if it is, we do everything in the re-import that normally happens during regular block import (weight calculation and epoch changes tracking).
| /// Re-validate existing block. | ||
| pub import_existing: bool, | ||
| /// Re-validate existing block. | ||
| pub allow_missing_parent: bool, |
There was a problem hiding this comment.
I know you want me to rename this - I'll get to it eventually 😄
| let create_gap = if origin == BlockOrigin::ConsensusBroadcast { | ||
| // Never create gaps for consensus imported blocks, because this import origin is used | ||
| // during warp sync, and the following gap sync needs to import all blocks. | ||
| false | ||
| } else { | ||
| create_gap | ||
| }; |
There was a problem hiding this comment.
This one we could get rid of. In the end you have control over the import parameters here:
You could just set create_gap to false there.
|
Ah btw, if this importing of justifications currently fails on master, please make sure that this does not get published in the 2509 release. Meaning that you should revert the old changes that don't work, and backport the revert. |
This PR fixes block import during Warp sync, which was silently failing due to "Unknown parent" errors - a typical case during Warp sync and the `full_node_warp_sync` test was not detecting such failure. Changes - Relaxed verification for Warp synced blocks: The fix relaxes verification requirements for Warp synced blocks by not performing full verification, with the assumption that these blocks are part of the finalized chain and have already been verified using the provided warp sync proof. - New `BlockOrigin` variants: For improved clarity, two additional `BlockOrigin` items have been introduced: - `WarpSync` - `GapSync` - Gap sync improvements: Warp synced blocks are now skipped during the gap sync block import phase, which required improvements to gap handling when committing the block import operation in the database. - Enhanced testing: The Warp sync zombienet test has been modified to more thoroughly assert both warp and gap sync phases. This PR builds on changes by @sistemd in #9678 --------- Co-authored-by: sistemd <enntheprogrammer@gmail.com> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR fixes block import during Warp sync, which was silently failing due to "Unknown parent" errors - a typical case during Warp sync and the `full_node_warp_sync` test was not detecting such failure. Changes - Relaxed verification for Warp synced blocks: The fix relaxes verification requirements for Warp synced blocks by not performing full verification, with the assumption that these blocks are part of the finalized chain and have already been verified using the provided warp sync proof. - New `BlockOrigin` variants: For improved clarity, two additional `BlockOrigin` items have been introduced: - `WarpSync` - `GapSync` - Gap sync improvements: Warp synced blocks are now skipped during the gap sync block import phase, which required improvements to gap handling when committing the block import operation in the database. - Enhanced testing: The Warp sync zombienet test has been modified to more thoroughly assert both warp and gap sync phases. This PR builds on changes by @sistemd in #9678 --------- Co-authored-by: sistemd <enntheprogrammer@gmail.com> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR fixes block import during Warp sync, which was silently failing due to "Unknown parent" errors - a typical case during Warp sync and the `full_node_warp_sync` test was not detecting such failure. Changes - Relaxed verification for Warp synced blocks: The fix relaxes verification requirements for Warp synced blocks by not performing full verification, with the assumption that these blocks are part of the finalized chain and have already been verified using the provided warp sync proof. - New `BlockOrigin` variants: For improved clarity, two additional `BlockOrigin` items have been introduced: - `WarpSync` - `GapSync` - Gap sync improvements: Warp synced blocks are now skipped during the gap sync block import phase, which required improvements to gap handling when committing the block import operation in the database. - Enhanced testing: The Warp sync zombienet test has been modified to more thoroughly assert both warp and gap sync phases. This PR builds on changes by @sistemd in #9678 --------- Co-authored-by: sistemd <enntheprogrammer@gmail.com> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit b2a4296)
This is only the changes for BABE right now, AURA side also needs to be updated. IMO this is not looking very good.