Remove the need to wait for target block header in warp sync implementation#5431
Merged
dmitry-markin merged 2 commits intoparitytech:masterfrom Aug 23, 2024
Merged
Conversation
bkchr
approved these changes
Aug 21, 2024
lexnv
approved these changes
Aug 22, 2024
Contributor
lexnv
left a comment
lexnv
left a comment
There was a problem hiding this comment.
LGTM! Nicely done here!
From the testing perspective, did we encounter any issues while trying to sync a full node to kusama for example? We've used that reliably in the past to filter out extra edge-cases.
The code logic looks good to me, I would also wait for Dmitry for a review 🙏
dmitry-markin
approved these changes
Aug 22, 2024
Contributor
dmitry-markin
left a comment
dmitry-markin
left a comment
There was a problem hiding this comment.
This is exactly what #3537 meant. Thank you!
Contributor
Author
|
I see no reason why it wouldn't work, but I didn't run it on Kusama myself. Can someone give it a try, please? |
2 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 10, 2024
This is a step towards #5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With #5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 10, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
mordamax
pushed a commit
to paritytech-stg/polkadot-sdk
that referenced
this pull request
Sep 11, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump.
nazar-pc
added a commit
to autonomys/polkadot-sdk
that referenced
this pull request
Sep 27, 2024
This is a step towards paritytech#5333 The commits with code moving (but no other changes) and actual changes are separated for easier review. Essentially this results in `SyncingStrategy` trait replacing struct (which is renamed to `PolkadotSyncingStrategy`, open for better name suggestions). Technically it is already possible to replace `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>` in syncing engine, but I decided to postpone such change until we actually have an ability to customize it. It should also be possible to swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports regular full sync from genesis (it also implements `SyncingStrategy` trait). While extracted trait still has a lot of non-generic stuff in it like exposed knowledge of warp sync and `StrategyKey` with hardcoded set of options, I believe this is a step in the right direction and will address those in upcoming PRs. With paritytech#5431 that landed earlier warp sync configuration is more straightforward, but there are still numerous things interleaved that will take some time to abstract away nicely and expose in network config for developers. For now this is an internal change even though data structures are technically public and require major version bump. (cherry picked from commit 1f1f20a)
aurexav
added a commit
to darwinia-network/darwinia
that referenced
this pull request
May 29, 2025
aurexav
added a commit
to darwinia-network/darwinia
that referenced
this pull request
May 30, 2025
* Initialize * Companion for paritytech/polkadot-sdk#4977 * Companion for paritytech/polkadot-sdk#5078 * Companion for paritytech/polkadot-sdk#5214 * Companion for paritytech/polkadot-sdk#5431 * Companion for paritytech/polkadot-sdk#4792 * Companion for paritytech/polkadot-sdk#5613 * Companion for paritytech/polkadot-sdk#7688 * Fixes * Companion for paritytech/polkadot-sdk#5364 * Fixes * Disable storage limit & fix format issue * Benchmark only * Patch evm * Update patch info * Fixes --------- Co-authored-by: bear <boundless.forest@outlook.com>
Patrick8rz
added a commit
to Patrick8rz/darwinia
that referenced
this pull request
Sep 6, 2025
* Initialize * Companion for paritytech/polkadot-sdk#4977 * Companion for paritytech/polkadot-sdk#5078 * Companion for paritytech/polkadot-sdk#5214 * Companion for paritytech/polkadot-sdk#5431 * Companion for paritytech/polkadot-sdk#4792 * Companion for paritytech/polkadot-sdk#5613 * Companion for paritytech/polkadot-sdk#7688 * Fixes * Companion for paritytech/polkadot-sdk#5364 * Fixes * Disable storage limit & fix format issue * Benchmark only * Patch evm * Update patch info * Fixes --------- Co-authored-by: bear <boundless.forest@outlook.com>
jacksonLewis88
added a commit
to jacksonLewis88/darwinia
that referenced
this pull request
Sep 25, 2025
* Initialize * Companion for paritytech/polkadot-sdk#4977 * Companion for paritytech/polkadot-sdk#5078 * Companion for paritytech/polkadot-sdk#5214 * Companion for paritytech/polkadot-sdk#5431 * Companion for paritytech/polkadot-sdk#4792 * Companion for paritytech/polkadot-sdk#5613 * Companion for paritytech/polkadot-sdk#7688 * Fixes * Companion for paritytech/polkadot-sdk#5364 * Fixes * Disable storage limit & fix format issue * Benchmark only * Patch evm * Update patch info * Fixes --------- Co-authored-by: bear <boundless.forest@outlook.com>
kinetic-smith803tr
added a commit
to kinetic-smith803tr/darwinia
that referenced
this pull request
Sep 26, 2025
* Initialize * Companion for paritytech/polkadot-sdk#4977 * Companion for paritytech/polkadot-sdk#5078 * Companion for paritytech/polkadot-sdk#5214 * Companion for paritytech/polkadot-sdk#5431 * Companion for paritytech/polkadot-sdk#4792 * Companion for paritytech/polkadot-sdk#5613 * Companion for paritytech/polkadot-sdk#7688 * Fixes * Companion for paritytech/polkadot-sdk#5364 * Fixes * Disable storage limit & fix format issue * Benchmark only * Patch evm * Update patch info * Fixes --------- Co-authored-by: bear <boundless.forest@outlook.com>
github-project-automation
bot
moved this from Todo
to Done
in [preview] release tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm not sure if this is exactly what #3537 meant, but I think it should be fine to wait for relay chain before initializing parachain node fully, which removed the need for background task and extra hacks throughout the stack just to know where warp sync should start.
Previously there were both
WarpSyncParamsandWarpSyncConfig, but there was no longer any point in having two data structures, so I simplified it to justWarpSyncConfig.Fixes #3537