cl/phase1/forkchoice: align getFilterBlockTree with consensus spec#21317
Closed
sudeepdino008 wants to merge 1 commit into
Closed
cl/phase1/forkchoice: align getFilterBlockTree with consensus spec#21317sudeepdino008 wants to merge 1 commit into
sudeepdino008 wants to merge 1 commit into
Conversation
8663209 to
7f1b3d0
Compare
The post-GLOAS rewrite (#18956, f3ad70f) restored ancestor-descent for the finalized check but left two justified-side deviations from the spec's filter_block_tree / get_voting_source: 1. Voting source selection used unrealized unconditionally with a realized fallback. Spec get_voting_source branches strictly on current_epoch > block_epoch — unrealized only for prior-epoch blocks; current-epoch blocks must use the block's realized current_justified_checkpoint. 2. correct_justified used voting_source.epoch >= justified.epoch with an is_previous_epoch_justified-gated +2 fallback. Spec is the three flat disjuncts: epoch == GENESIS, voting_source.epoch == justified.epoch, or voting_source.epoch + 2 >= current_epoch. The finalized ancestor-descent check from f3ad70f is preserved unchanged. Observed effect on bloatnet: epoch-boundary head regression that caused ~30-block execution unwinds every ~6 minutes is eliminated. Validated by 1 hour at chain tip on bloatnet covering 5+ epoch boundaries — zero unwinds, zero filterTree rejects, Caplin lagSlots=0 on all 172 forkchoice updates in the window. Refs: #21301, #21310
7f1b3d0 to
9402e29
Compare
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
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.
Summary
Cherry-pick / port of #21310 to
main, combined with the finalized ancestor-descent check that was already onmain(from #18956, commitf3ad70f1f7).mainalready has a partial spec-faithful rewrite ofgetFilterBlockTree— the finalized side now does ancestor descent (correct per spec) — but the justified side still deviates fromfilter_block_tree/get_voting_sourcein two ways. This PR fixes those.Spec reference
filter_block_tree:
get_voting_source:
Changes
Voting source selection: was unrealized-with-fallback unconditional. Now branched on
current_epoch > block_epochas the spec requires — unrealized only for prior-epoch blocks; current-epoch (or future) blocks use the block's realizedcurrent_justified_checkpoint.correct_justified: wasvoting_source.Epoch >= justifiedCheckpoint.Epochwith anis_previous_epoch_justified-gated+2fallback. Now the three flat disjuncts straight from the spec:epoch == GENESISorvoting_source.epoch == justified.epochorvoting_source.epoch + 2 >= current_epoch. (The previous>=accepts cases spec rejects; the previous gated+2rejects cases spec accepts.)correct_finalized: unchanged — keeps the ancestor-descent check fromf3ad70f1f7(finalizedCheckpoint.Root == f.Ancestor(blockRoot, finalizedSlot).Root).Background
Original observed symptom on bloatnet was ~30-block execution unwinds every ~6 minutes (every epoch boundary). Root cause:
getFilterBlockTreerejected the leaf at each new epoch's first slot, Caplin's head fell back to an older block, and execution unwound to follow. Both pre-existing deviations contributed:Equal(or>=with the gated fallback) rejected leaves whose voting source's epoch was within 2 ofcurrent_epochbut not equal tojustified.epoch.Issue: #21301
Original performance-branch PR: #21310
Test plan
go build ./cl/phase1/forkchoice/cleanperformance(cl/phase1/forkchoice: align getFilterBlockTree with consensus spec #21310): 1 hour at chain tip, 5+ epoch boundaries, zero unwinds, zero filterTree rejects,Caplin lagSlots=0on all 172 FCU eventsmain