This repository was archived by the owner on Nov 15, 2023. It is now read-only.
beefy: initialize voter from genesis and fix initial sync#11959
Merged
acatangiu merged 14 commits intoparitytech:masterfrom Sep 5, 2022
Merged
beefy: initialize voter from genesis and fix initial sync#11959acatangiu merged 14 commits intoparitytech:masterfrom
acatangiu merged 14 commits intoparitytech:masterfrom
Conversation
Now that we have justifications import, we can drop the "lean beefy" behaviour and start building justifications chain from Genesis with containing all past sessions' mandatory blocks justifications.
Signed-off-by: acatangiu <adrian@parity.io>
During initial block import blocks are not finalized, so trying to validate and append justifications within block import fails (for initial network sync imported blocks). Changes: - Move justification validation to _after_ `inner.block_import()`, so block is imported in backend and runtime api can be called to get the BEEFY authorities for said block. - Move append-to-backend for imported BEEFY justification to voter, because it already has the required logic to BEEFY-finalize blocks only after GRANDPA finalized them. - Mark voting rounds as concluded when finalizing through imported justifications as well as when finalizing through voting. Signed-off-by: acatangiu <adrian@parity.io>
The only way we'd get _different_ _validated_ justifications for same block number is if authorities are double voting, which will be handled later.
Contributor
Author
|
I will follow-up with a zombienet regression test for this. |
svyatonik
reviewed
Aug 3, 2022
| backend | ||
| .blockchain() | ||
| .expect_header(BlockId::hash(*hash)) | ||
| .expect("just finalized block should be available; qed.") |
Contributor
There was a problem hiding this comment.
I'd also avoid calling expect here - there are db issues. concurrency, caches, mocked backends, ... they may cause their own issues. Otoh BEEFY worker may actually stop working because of that, so I can't say that logging + ignoring an error is any better :)
Contributor
Author
There was a problem hiding this comment.
yes, that is my thinking as well, if this errors out on a session boundary the voter is dead in the water anyway.
This way at least you can see the beefy-gadget task crashing and restart it..
BEEFY voter should resume voting from either: - last BEEFY finalized block, - session start, whichever is closest to head. Signed-off-by: acatangiu <adrian@parity.io>
Signed-off-by: acatangiu <adrian@parity.io>
davxy
approved these changes
Sep 1, 2022
Member
davxy
left a comment
There was a problem hiding this comment.
Overall LGTM. I've just added a nitpick (not super relevant) and a question
ark0f
pushed a commit
to gear-tech/substrate
that referenced
this pull request
Feb 27, 2023
…#11959) * client/beefy: use backend instead of client where possible * client/beefy: initialize voter from genesis Now that we have justifications import, we can drop the "lean beefy" behaviour and start building justifications chain from Genesis with containing all past sessions' mandatory blocks justifications. * client/beefy: walk finality tree_route to catch session changes * client/beefy: fix block import During initial block import blocks are not finalized, so trying to validate and append justifications within block import fails (for initial network sync imported blocks). Changes: - Move justification validation to _after_ `inner.block_import()`, so block is imported in backend and runtime api can be called to get the BEEFY authorities for said block. - Move append-to-backend for imported BEEFY justification to voter, because it already has the required logic to BEEFY-finalize blocks only after GRANDPA finalized them. - Mark voting rounds as concluded when finalizing through imported justifications as well as when finalizing through voting. * client/beefy: valid justifications are one per block number The only way we'd get _different_ _validated_ justifications for same block number is if authorities are double voting, which will be handled later. * client/beefy: process incoming justifs during major sync * client/beefy: correct voter initialization BEEFY voter should resume voting from either: - last BEEFY finalized block, - session start, whichever is closest to head. * client/beefy: test voter initialization * client/beefy: impl review suggestions Signed-off-by: acatangiu <adrian@parity.io>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
Now that we have justifications import (added in #11821), we can drop the "lean beefy" behaviour and start building justifications chain from Genesis with containing all past sessions' mandatory blocks justifications.
For each finality notification also walk finality
tree_routeto catch session changes.During initial block import blocks are not finalized, so trying to validate and append justifications within block import fails (for initial network sync imported blocks). Fix this by:
inner.block_import(), so block is imported in backend and runtime api can be called to get the BEEFY authorities for said block.Fixes #11954
Regression test in zombienet: paritytech/polkadot#5855