cl/phase1/forkchoice: use post-pull-up values for prior-epoch updateCheckpoints#21339
Merged
Merged
Conversation
…heckpoints
Per spec `compute_pulled_up_tip`, the prior-epoch `updateCheckpoints`
call should use the checkpoints from the post-pull-up state (after
`process_justification_and_finalization`), not the realized values
restored back onto the state for replay/simulation purposes.
Previously OnBlock did:
ProcessJustificationBitsAndFinality(lastProcessedState, nil) // pull-up
unrealizedJustifications.Store(.., lastProcessedState.CurrentJustifiedCheckpoint())
...
// restore pre-pull-up values onto lastProcessedState
lastProcessedState.SetCurrentJustifiedCheckpoint(currentJustifiedCheckpoint)
lastProcessedState.SetFinalizedCheckpoint(stateFinalized)
...
if blockEpoch < currentEpoch {
// BUG: reads the just-restored pre-pull-up values
updateCheckpoints(lastProcessedState.CurrentJustifiedCheckpoint(), ...)
}
So `store.justified_checkpoint` / `store.finalized_checkpoint` were
updated with the *realized* values, not the post-pull-up ones. For
late-arriving prior-epoch blocks this leaves the store behind the
spec's view by one epoch boundary.
Capture the post-pull-up values immediately after
`ProcessJustificationBitsAndFinality` and use them for both the
per-block unrealized stores and the prior-epoch `updateCheckpoints`
call. Builds on #21310.
Refs: #21301
AskAlexSharov
approved these changes
May 22, 2026
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
2 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.
Summary
Follow-up to #21310. Fixes a remaining spec deviation in
OnBlock's prior-epochupdateCheckpointscall: percompute_pulled_up_tip, the prior-epoch update should use the post-pull-up checkpoints (after `process_justification_and_finalization` runs), not the realized values that were restored onto the state for replay purposes.The bug
In `cl/phase1/forkchoice/on_block.go`:
```go
ProcessJustificationBitsAndFinality(lastProcessedState, nil) // pull-up runs here
unrealizedJustifications.Store(.., lastProcessedState.CurrentJustifiedCheckpoint())
...
// restore pre-pull-up values onto lastProcessedState (for replay/simulation)
lastProcessedState.SetCurrentJustifiedCheckpoint(currentJustifiedCheckpoint)
lastProcessedState.SetFinalizedCheckpoint(stateFinalized)
...
if blockEpoch < currentEpoch {
// BUG: now reads the just-restored pre-pull-up values, not the pulled-up ones
updateCheckpoints(lastProcessedState.CurrentJustifiedCheckpoint(), lastProcessedState.FinalizedCheckpoint())
}
```
Effect: `store.justified_checkpoint` / `store.finalized_checkpoint` get updated with realized values rather than the post-pull-up values for late-arriving prior-epoch blocks. The store ends up one epoch boundary behind what the spec would have.
Fix
Capture the post-pull-up values into local variables right after `ProcessJustificationBitsAndFinality` and use them consistently in:
Test plan
Refs: #21301, #21310
Already validated on main
This same change (commit
3cfb67d6ef) is included in #21327 (cp/21310-to-main→main), where all CI checks are passing: lint, unit tests, race tests, integration tests, kurtosis, mainnet-rpc-integ, hive, reproducible build across linux/mac/windows. See #21327.So this PR is the same patch already exercised by CI on the main-branch flavor of #21310 — applying it to
performancefor parity.