fix: checkpoint sync for skipped slot#9329
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for skipped-slot checkpoint synchronization, ensuring that the first batch of a sync chain can correctly fetch the parent payload envelope when necessary. Key changes include updates to the Batch and SyncChain logic, the addition of a createFromBid method for cache seeding, and improved validation for range sync responses. The reviewer suggested a minor cleanup in packages/beacon-node/src/chain/blocks/index.ts to remove a redundant variable, which is a valid improvement.
twoeths
force-pushed
the
te/skipped_cp_state_sync_e2e
branch
from
28465ae to
b1ed8b8
Compare
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
Contributor
Author
|
@lodekeeper please review |
lodekeeper
reviewed
May 6, 2026
Contributor
lodekeeper
left a comment
lodekeeper
left a comment
There was a problem hiding this comment.
Main logic looks sound overall. I left one sync-robustness note on the new parent-by-root column path. I also agree with the Gemini cleanup on the redundant blockSlots temporary in processBlocks.
|
|
||
| private downloadByRange: SyncChainFns["downloadByRange"] = async (peer, batch) => { | ||
| const batchBlocks = batch.getBlocks(); | ||
| const requests = batch.getRequestsForPeer(peer); |
Contributor
There was a problem hiding this comment.
🟡 Parent by-root column requests still bypass peer custody filtering
getRequestsForPeer() only filters columnsRequest, but this new skipped-checkpoint path can also carry sampled columns in parentPayloadRequest.columns. That means we can still select a peer that did not advertise some of those columns, then immediately ask it for them by-root in the first batch. In the best case that just churns retries; in the worst case it can stall the very path this PR is trying to unblock.
Could we run parentPayloadRequest.columns through the same peer.custodyColumns filter before spreading requests into downloadByRange()?
Contributor
|
Reviewed — left one note on the new parent-by-root column request path, and I agree with the Gemini cleanup on the redundant |
wemeetagain
reviewed
May 6, 2026
Comment on lines
-126
to
-130
| const blockState = await this.regen.getBlockSlotState( | ||
| protoBlock, | ||
| protoBlock.slot, | ||
| {dontTransferCache: true}, | ||
| RegenCaller.processBlock |
Member
There was a problem hiding this comment.
can do
const blockState = await this.regen.getBlockSlotState(
protoBlock,
protoBlock.slot,
{dontTransferCache: true},
RegenCaller.processBlock
).catch(() =>
// only happen at the 1st batch of skipped slot checkpoint sync
this.regen.getClosestHeadState(protoBlock)
);
Contributor
There was a problem hiding this comment.
Behavior-wise this is equivalent to the current wrapError(...) branch. I slightly prefer the expanded form here because this is a pretty unusual path: we are intentionally swallowing a regen failure and falling back to getClosestHeadState() only for the skipped-checkpoint case, so keeping the err branch explicit makes that special-case easier to spot (and easier to tighten later if we ever want to narrow which regen failures are allowed to fall through). So I think the current form is fine as-is.
nflaig
reviewed
May 7, 2026
nflaig
reviewed
May 7, 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
4 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.
Motivation
Lodestar is not able to do checkpoint sync from skipped slot checkpoint because it does not download the parent payload envelope to feed forkchoice
Description
Batch.getRequest()decides which requests to downloadBatch.downloadSuccess()handler: need to make sure parent envelope is fully downloadeddownloadByRangeheuristic, it needs to download whatever specified inBatch.getRequest()checkpointSync.test.tse2eCloses #9319
AI Assistance Disclosure
Create with the help of Claude