-
Notifications
You must be signed in to change notification settings - Fork 252
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
Finalize domain blocks and default domain block and state pruning #3372
Conversation
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.
Look good in general, left some questions.
Also, this PR set proper block/state pruning arg for the domain chain, we also need to ensure the corresponding consensus block/state are not pruned because they are required by FP generation.
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.
Just some questions about how the code works, overall it makes sense to me.
Edit: none of these are blocking, feel free to resolve or ask me to open a PR instead.
crates/subspace-node/Cargo.toml
Outdated
@@ -51,6 +51,7 @@ sc-network = { git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a814 | |||
sc-proof-of-time = { version = "0.1.0", path = "../sc-proof-of-time" } | |||
sc-subspace-chain-specs = { version = "0.1.0", path = "../sc-subspace-chain-specs" } | |||
sc-service = { git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305", default-features = false } | |||
sc-state-db = { git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305", default-features = false } |
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: There are no explicit features or optional dependencies in this crate, and it uses std collections unconditionally:
https://github.com/autonomys/polkadot-sdk/blob/94a1a8143a89bbe9f938c1939ff68abc1519a305/substrate/client/state-db/Cargo.toml
It doesn’t matter much either way, but it’s slightly confusing to suggest there are features we’re not using:
sc-state-db = { git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305", default-features = false } | |
sc-state-db = { git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305" } |
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.
Good enough to merge, I don't mind if you want to tweak that comment (or not).
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.
Thanks! just one nit, non-blocker for this PR.
Forgot to add TODO :( |
This PR brings following changes
challenge_period * 2
blocks for block and state pruning beyond the finalized domain blocksCloses: #2353
Closes: #3005
Code contributor checklist: