Skip to content

fix: payload status on fork chocie init #8987

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ensi321 wants to merge 2 commits into from
Closed

fix: payload status on fork chocie init #8987

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b670f88
fix payload status on fork chocie init
ensi321 Mar 5, 2026
c1387ff
lint
ensi321 Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions packages/beacon-node/src/chain/forkChoice/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
ForkChoice,
ForkChoiceStore,
JustifiedBalancesGetter,
PayloadStatus,
ProtoArray,
ProtoBlock,
ForkChoiceOpts as RawForkChoiceOpts,
Expand Down Expand Up @@ -157,7 +156,7 @@ export function initializeForkChoiceFromFinalizedState(
: {executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}),

dataAvailabilityStatus: DataAvailabilityStatus.PreData,
payloadStatus: isForkPostGloas ? PayloadStatus.PENDING : PayloadStatus.FULL, // TODO GLOAS: Post-gloas how do we know if the checkpoint payload is FULL or EMPTY?
payloadStatus: getCheckpointPayloadStatus(state, checkpoint.epoch),
builderIndex: isForkPostGloas ? (state as CachedBeaconStateGloas).latestExecutionPayloadBid.builderIndex : null,

@gemini-code-assist gemini-code-assist Bot Mar 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

The pull request modifies the initialization of payloadStatus in initializeForkChoiceFromFinalizedState and initializeForkChoiceFromUnfinalizedState to use getCheckpointPayloadStatus(state, checkpoint.epoch). For Gloas (ePBS) forks, getCheckpointPayloadStatus can return PayloadStatus.FULL if the execution payload is marked as available in the state.

However, ProtoArray.onBlock, which is called during fork choice initialization (lines 134 and 305-308), only creates PENDING and EMPTY variants for Gloas blocks. It does not create the FULL variant. The FULL variant is only created later via onExecutionPayload when the payload actually arrives.

By initializing the ForkChoiceStore with payloadStatus = FULL for the finalized or justified checkpoints while ProtoArray lacks the corresponding FULL variant, several critical operations will fail:

  • getJustifiedBlock() and getFinalizedBlock() will throw a MISSING_PROTO_ARRAY_BLOCK error.
  • isFinalizedRootOrDescendant() will fail to walk back to the finalized block if it attempts to match the FULL variant (e.g., when a child block's parentBlockHash refers to the execution payload hash).
  • findHead() will throw INVALID_BEST_NODE if it cannot verify the finalized root relationship.

This mismatch results in a complete denial of service for the node's fork choice processing and breaks related API endpoints.

@lodekeeper lodekeeper Mar 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 from my side as reviewer. This should not initialize anchor/head payload status from checkpoint-epoch availability for the reasons above (can incorrectly choose FULL and mismatch proto-array variants).\n\nAuthor guidance is captured here: https://github.com/ChainSafe/lodestar/pull/8987#issuecomment-4003698056\n\nOnce this is wired to the correct head-slot semantics, the concern should be resolved.

blockHashFromBid: isForkPostGloas
? toRootHex((state as CachedBeaconStateGloas).latestExecutionPayloadBid.blockHash)
Expand Down Expand Up @@ -254,7 +253,7 @@ export function initializeForkChoiceFromUnfinalizedState(
: {executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}),

dataAvailabilityStatus: DataAvailabilityStatus.PreData,
payloadStatus: isForkPostGloas ? PayloadStatus.PENDING : PayloadStatus.FULL, // TODO GLOAS: Post-gloas how do we know if the checkpoint payload is FULL or EMPTY?
payloadStatus: getCheckpointPayloadStatus(unfinalizedState, computeEpochAtSlot(blockHeader.slot)),
builderIndex: isForkPostGloas

@gemini-code-assist gemini-code-assist Bot Mar 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

Initializing payloadStatus with the result of getCheckpointPayloadStatus for an unfinalized state can lead to a MISSING_PROTO_ARRAY_BLOCK error if the status is FULL. For Gloas blocks, the ProtoArray is initialized with PENDING and EMPTY variants only. Using FULL here will cause a mismatch that prevents the fork choice from correctly identifying the head or walking the block tree. Additionally, the function getCheckpointPayloadStatus is designed for checkpoint slots but is being used here for the headBlock, which is not necessarily at a checkpoint slot. This leads to incorrectly using the payload status of the epoch's checkpoint block for the headBlock. To fix this, you should determine the payload status for blockHeader.slot directly, by checking unfinalizedState.executionPayloadAvailability for the blockHeader.slot.

@lodekeeper lodekeeper Mar 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on this risk. I reviewed it and confirmed this is actionable: using getCheckpointPayloadStatus(...) for the anchor/head init path can produce a FULL variant mismatch and is semantically wrong for non-checkpoint head slots.\n\nI left the same guidance to the author here: https://github.com/ChainSafe/lodestar/pull/8987#issuecomment-4003698056\n\nRecommended fix remains to keep anchor/head init at PENDING for post-Gloas and limit getCheckpointPayloadStatus usage to checkpoint-context status computation only.

? (unfinalizedState as CachedBeaconStateGloas).latestExecutionPayloadBid.builderIndex
: null,
Expand Down
Loading