-
Notifications
You must be signed in to change notification settings - Fork 712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hotfix: blockchain/backend: Skip genesis leaf to unblock syncing #5103
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion the correct abort condition here would be to to insert a check here:
https://github.com/paritytech/polkadot-sdk/pull/5103/files#diff-37542b5b93e687ac878ece2ca83625c87aa26cf014cff6dff76459bbf3ed751eR424
The check should be:
if current_header_metadata.hash == finalized_chain_block_hash {
result.displaced_leaves.push((leaf_number, leaf_hash));
debug!(
target: crate::LOG_TARGET,
?leaf_hash,
elapsed = ?now.elapsed(),
"Leaf is ancestor of finalized block" );
break;
}
Basically these situations should be covered by this:
(l) = marked as leaf
(f) = to-be-finalized
G(l) -> A -> B -> C(f) -> D
Genesis block G should be marked as displaced leaf. An ancestor of the finalized block is not a leaf.
G -> A -> B(l) -> C(f) -> D
Here block B should be marked as displaced leafs. However in both cases B and G should not be marked as displaced blocks.
G -> A -> B(l) -> ... GAP ... C(f) -> D
If there is a gap in between we leave B alone for now.
@@ -324,6 +365,11 @@ pub trait Backend<Block: BlockT>: | |||
// If points back to the finalized header then nothing left to do, this leaf will be | |||
// checked again later | |||
if current_header_metadata.hash == finalized_block_hash { | |||
debug!( | |||
target: crate::LOG_TARGET, | |||
"Leaf points to the finalized header {leaf_hash:?}, skipping for now (elapsed {:?})", now.elapsed(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: IMO since we are using tracing lib now we should not use the inline formatting but the structured logging only. This plus some consistend naming gives us nicely formatted logs.
Signed-off-by: Alexandru Vasile <[email protected]>
You are basically repeating the check some lines below for I think we should simplify this even more and put here the following: if leaf_number < finalized_block_number {
result.displaced_leaves.push((leaf_number, leaf_hash));
// go from leaf to finalized chain
// by getting the hash at block `n` (starting with `leaf_number - 1`)
// Compare if the hash at `n` is the same hash of the block of the fork
// If not, go up and compare the same and push the block to `displaced_blocks`.
continue
} Generally this should be faster, as we don't need to go down from the tip of the chain back to genesis block in the case of warp syncing. Also for most of the other cases it should probably be faster. |
Yes that is true, however we don't want to iterate to the parent, as its not available for the genesis block.
Wdym by this? We will go the "unknown block" route while the gap exists. This is intended however, see the discussion here Your proposed solution works in the short term, but currently we are accepting leaves that could be below the gap, and we should not break that. |
Let's use @skunert proposal and call it a day. We need to really refactor this beast of pruning logic to make it easier to reason about. |
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Can you please add a prdoc? |
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
After some testing, we have decided to add an extra check for the genesis hash. In the current state, syncing is unblocked and this case is entirely skipped:
|
// The genesis block is never displaced, as it is part of the canonical chain. | ||
let genesis_hash = self.info().genesis_hash; | ||
let leaves = self.leaves()?.into_iter().filter(|h| *h != genesis_hash).collect::<Vec<_>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// The genesis block is never displaced, as it is part of the canonical chain. | |
let genesis_hash = self.info().genesis_hash; | |
let leaves = self.leaves()?.into_iter().filter(|h| *h != genesis_hash).collect::<Vec<_>>(); | |
let leaves = self.leaves()?; |
We want that the genesis
leaf is removed.
This is what I said above
I'm not sure that just skipping the TLDR: We need to prune it. Maybe to move this forward, bring back your initial hack and with checking the |
Yes, instead of just skipping, should be added to displaced leafs. AFAIK this is what were were doing before the touching of this code started. |
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
The CI pipeline was cancelled due to failure one of the required jobs. |
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
…nv/fix-leaf-genesis
Signed-off-by: Alexandru Vasile <[email protected]>
This PR effectively skips over cases where the blockchain reports the genesis block as leaf. The issue manifests as the blockchain getting stuck and not importing blocks after a while. Although the root-cause of why the blockchain reports the genesis as leaf is not scoped, this hot-fix is unblocking the new release. While at it, added some extra debug logs to identify issues more easily in the future. ### Issue ``` 2024-07-22 10:06:08.708 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0x8f8e…7f34 finalized_block_number=24148459 2024-07-22 10:06:08.708 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14 (elapsed 25.74µs) leaf_number=24148577 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14, skipping for now (elapsed 70.72µs) // This is Kusama genesis 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe (elapsed 127.271µs) leaf_number=0 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Skip more blocks until we get all blocks on finalized chain until the height of the parent block current_hash=0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe current_num=0 finalized_num=24148458 ``` ### Before ``` 2024-07-20 00:45:00.234 INFO tokio-runtime-worker substrate: ⚙️ Preparing 0.0 bps, target=#24116589 (50 peers), best: #24116498 (0xb846…8720), finalized #24116493 (0x50b6…2445), ⬇ 2.3MiB/s ⬆ 2.6kiB/s ... 2024-07-20 14:05:18.572 INFO tokio-runtime-worker substrate: ⚙️ Syncing 0.0 bps, target=#24124495 (51 peers), best: #24119976 (0x6970…aeb3), finalized #24119808 (0xd900…abe4), ⬇ 2.2MiB/s ⬆ 3.1kiB/s 2024-07-20 14:05:23.573 INFO tokio-runtime-worker substrate: ⚙️ Syncing 0.0 bps, target=#24124495 (51 peers), best: #24119976 (0x6970…aeb3), finalized #24119808 (0xd900…abe4), ⬇ 2.2MiB/s ⬆ 5.8kiB/s ``` ### After ``` 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x4e8cf3ff18e7d13ff7fec28f9fc8ce6eff5492ed8dc046e961b76dec5c0cfddf (elapsed 39.26µs) leaf_number=24150969 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x4e8cf3ff18e7d13ff7fec28f9fc8ce6eff5492ed8dc046e961b76dec5c0cfddf, skipping for now (elapsed 49.69µs) 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 54.57µs) 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 58.78µs) finalized_block_hash=0x02b3…5338 finalized_block_number=24150967 2024-07-22 10:41:12.357 INFO tokio-runtime-worker substrate: 🏆 Imported #24150970 (0x4e8c…fddf → 0x3637…56bb) 2024-07-22 10:41:12.862 INFO tokio-runtime-worker substrate: 💤 Idle (50 peers), best: #24150970 (0x3637…56bb), finalized #24150967 (0x02b3…5338), ⬇ 2.0MiB/s ⬆ 804.7kiB/s 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0xa1534a105b90e7036a18ac1c646cd2bd6c41c66cc055817f4f51209ab9070e5c finalized_block_number=24150968 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb (elapsed 62.48µs) leaf_number=24150970 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, skipping for now (elapsed 71.76µs) 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 75.96µs) 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 80.27µs) finalized_block_hash=0xa153…0e5c finalized_block_number=24150968 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0xa1534a105b90e7036a18ac1c646cd2bd6c41c66cc055817f4f51209ab9070e5c finalized_block_number=24150968 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb (elapsed 39.67µs) leaf_number=24150970 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, skipping for now (elapsed 50.3µs) 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 54.52µs) 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 58.66µs) finalized_block_hash=0xa153…0e5c finalized_block_number=24150968 2024-07-22 10:41:17.863 INFO tokio-runtime-worker substrate: 💤 Idle (50 peers), best: #24150970 (0x3637…56bb), finalized #24150968 (0xa153…0e5c), ⬇ 1.2MiB/s ⬆ 815.0kiB/s 2024-07-22 10:41:18.399 INFO tokio-runtime-worker substrate: 🏆 Imported #24150971 (0x3637…56bb → 0x4ee3…5f7c) ``` Closes: #5088 --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
This PR backports hotfix from #5103 --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Alexandru Vasile <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
…itytech#5103) This PR effectively skips over cases where the blockchain reports the genesis block as leaf. The issue manifests as the blockchain getting stuck and not importing blocks after a while. Although the root-cause of why the blockchain reports the genesis as leaf is not scoped, this hot-fix is unblocking the new release. While at it, added some extra debug logs to identify issues more easily in the future. ### Issue ``` 2024-07-22 10:06:08.708 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0x8f8e…7f34 finalized_block_number=24148459 2024-07-22 10:06:08.708 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14 (elapsed 25.74µs) leaf_number=24148577 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0xd62aea69664b74c55b7e79ab5855b117d213156a5e9ab05ad0737772aaf42c14, skipping for now (elapsed 70.72µs) // This is Kusama genesis 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe (elapsed 127.271µs) leaf_number=0 2024-07-22 10:06:08.709 DEBUG tokio-runtime-worker db::blockchain: Skip more blocks until we get all blocks on finalized chain until the height of the parent block current_hash=0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe current_num=0 finalized_num=24148458 ``` ### Before ``` 2024-07-20 00:45:00.234 INFO tokio-runtime-worker substrate: ⚙️ Preparing 0.0 bps, target=#24116589 (50 peers), best: #24116498 (0xb846…8720), finalized #24116493 (0x50b6…2445), ⬇ 2.3MiB/s ⬆ 2.6kiB/s ... 2024-07-20 14:05:18.572 INFO tokio-runtime-worker substrate: ⚙️ Syncing 0.0 bps, target=#24124495 (51 peers), best: #24119976 (0x6970…aeb3), finalized #24119808 (0xd900…abe4), ⬇ 2.2MiB/s ⬆ 3.1kiB/s 2024-07-20 14:05:23.573 INFO tokio-runtime-worker substrate: ⚙️ Syncing 0.0 bps, target=#24124495 (51 peers), best: #24119976 (0x6970…aeb3), finalized #24119808 (0xd900…abe4), ⬇ 2.2MiB/s ⬆ 5.8kiB/s ``` ### After ``` 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x4e8cf3ff18e7d13ff7fec28f9fc8ce6eff5492ed8dc046e961b76dec5c0cfddf (elapsed 39.26µs) leaf_number=24150969 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x4e8cf3ff18e7d13ff7fec28f9fc8ce6eff5492ed8dc046e961b76dec5c0cfddf, skipping for now (elapsed 49.69µs) 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 54.57µs) 2024-07-22 10:41:10.897 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 58.78µs) finalized_block_hash=0x02b3…5338 finalized_block_number=24150967 2024-07-22 10:41:12.357 INFO tokio-runtime-worker substrate: 🏆 Imported #24150970 (0x4e8c…fddf → 0x3637…56bb) 2024-07-22 10:41:12.862 INFO tokio-runtime-worker substrate: 💤 Idle (50 peers), best: #24150970 (0x3637…56bb), finalized #24150967 (0x02b3…5338), ⬇ 2.0MiB/s ⬆ 804.7kiB/s 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0xa1534a105b90e7036a18ac1c646cd2bd6c41c66cc055817f4f51209ab9070e5c finalized_block_number=24150968 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb (elapsed 62.48µs) leaf_number=24150970 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, skipping for now (elapsed 71.76µs) 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 75.96µs) 2024-07-22 10:41:14.772 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 80.27µs) finalized_block_hash=0xa153…0e5c finalized_block_number=24150968 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Checking for displaced leaves after finalization. leaves=[0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe] finalized_block_hash=0xa1534a105b90e7036a18ac1c646cd2bd6c41c66cc055817f4f51209ab9070e5c finalized_block_number=24150968 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Handle displaced leaf 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb (elapsed 39.67µs) leaf_number=24150970 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Leaf points to the finalized header 0x363763b16c23fc20a84f38f67014fa7ae6ba9c708fc074890016699e5ca756bb, skipping for now (elapsed 50.3µs) 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Skip genesis block 0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe reporterd as leaf (elapsed 54.52µs) 2024-07-22 10:41:14.795 DEBUG tokio-runtime-worker db::blockchain: Finished with result DisplacedLeavesAfterFinalization { displaced_leaves: [], displaced_blocks: [] } (elapsed 58.66µs) finalized_block_hash=0xa153…0e5c finalized_block_number=24150968 2024-07-22 10:41:17.863 INFO tokio-runtime-worker substrate: 💤 Idle (50 peers), best: #24150970 (0x3637…56bb), finalized #24150968 (0xa153…0e5c), ⬇ 1.2MiB/s ⬆ 815.0kiB/s 2024-07-22 10:41:18.399 INFO tokio-runtime-worker substrate: 🏆 Imported #24150971 (0x3637…56bb → 0x4ee3…5f7c) ``` Closes: paritytech#5088 --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
* master: (27 commits) Bridges improved tests and nits (#5128) Fix misleading comment about RewardHandler in epm config (#3095) Introduce a workflow updating the wishlist leaderboards (#5085) membership: Restructure pallet into separate files (#4536) Fix after ring-proof api change (#5126) Bump paritytech/review-bot from 2.4.0 to 2.5.0 (#5057) Bump docker/login-action from 3.0.0 to 3.3.0 (#5109) Bump docker/build-push-action from 5.1.0 to 6.5.0 (#5108) Bump peter-evans/create-pull-request from 5.0.0 to 6.1.0 (#5093) Tx Payment: drop ED requirements for tx payments with exchangeable asset (#4488) Remove `pallet-getter` usage from pallet-transaction-payment (#4970) pallet macro: do not generate try-runtime related code when frame-support doesn't have try-runtime. (#5099) fix(chain-spec): ChainSpecBuilder with object as default genesis (#4345) Migrate BEEFY BLS crypto to bls12-381 curve (#4931) Bump clap from 4.5.9 to 4.5.10 in the known_good_semver group (#5120) Use jobserver in wasm-builder to limit concurrency of spawned cargo processes (#4946) include events for voting (#4613) [subsystem-bench] Add mocks for own assignments triggering (#5042) Remove not-audited warning (#5114) hotfix: blockchain/backend: Skip genesis leaf to unblock syncing (#5103) ...
This PR effectively skips over cases where the blockchain reports the genesis block as leaf.
The issue manifests as the blockchain getting stuck and not importing blocks after a while.
Although the root-cause of why the blockchain reports the genesis as leaf is not scoped, this hot-fix is unblocking the new release.
While at it, added some extra debug logs to identify issues more easily in the future.
Issue
Before
After
Closes: #5088