Skip to content

fix: reject out-of-range builder_index in execution payload bid gossip validation - #9624

Merged
nflaig merged 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/gloas-payload-bid-builder-index-bounds
Jul 9, 2026
Merged

fix: reject out-of-range builder_index in execution payload bid gossip validation#9624
nflaig merged 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/gloas-payload-bid-builder-index-bounds

Conversation

@lodekeeper

Copy link
Copy Markdown
Contributor

Problem

validateExecutionPayloadBid (Gloas execution payload bid gossip validation) does not implement the spec's [REJECT] bid.builder_index < len(state.builders) bounds check. It looks up the builder inside a try/catch meant to turn an out-of-range index into a GossipReject:

let builder: gloas.Builder;
try {
  builder = state.getBuilder(bid.builderIndex);
} catch {
  throw new ExecutionPayloadBidError(GossipAction.REJECT, {code: BUILDER_NOT_ELIGIBLE, ...});
}
if (!isActiveBuilder(builder, state.finalizedCheckpoint.epoch)) { ... }

But state.getBuilder(i) returns a lazy SSZ view (builders.getReadonly(i)) that is not bounds-checked eagerly. An out-of-range builder_index therefore does not throw at getBuilder — it throws LeafNode has no right node later, on deferred field access inside isActiveBuilder (builder.depositEpoch), which is outside the try/catch. The net effect is an uncaught error on the gossip validation path instead of a clean REJECT.

Fix

Add an explicit bid.builder_index < len(state.builders) bounds check up front using the existing state.getBuildersLength(), and drop the now-ineffective try/catch.

Testing

Found by running the consensus-specs #5294 Gloas networking reference tests (spec v1.7.0-alpha.12). The gossip_execution_payload_bid__reject_builder_index_out_of_range case now returns REJECT (previously an uncaught throw), with no regression across the rest of the gossip_execution_payload_bid suite (minimal + mainnet presets).

Note: these Gloas gossip validators currently have no unit-test coverage on unstable; the reftest suite (wired up in #9372) is their canonical coverage.

🤖 Generated with AI assistance

…p validation

`validateExecutionPayloadBid` looked up the builder via `state.getBuilder(bid.builderIndex)`
inside a try/catch meant to convert an out-of-range index into a `GossipReject`. But
`getBuilder` returns a lazy SSZ `getReadonly` view that is not bounds-checked eagerly, so an
out-of-range `builder_index` does not throw there -- it throws `LeafNode has no right node`
later, on deferred field access inside `isActiveBuilder`, escaping the try/catch. The result
is an uncaught error instead of a clean `GossipReject`.

Add an explicit `bid.builder_index < len(state.builders)` bounds check up front (spec step
`[REJECT] bid.builder_index is a valid/active builder index`) and drop the now-ineffective
try/catch.

🤖 Generated with AI assistance
@lodekeeper
lodekeeper requested a review from a team as a code owner July 9, 2026 08:03

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds an explicit bounds check for bid.builderIndex against state.getBuildersLength() during execution payload bid validation. This prevents deferred errors from escaping when accessing lazy SSZ views returned by state.getBuilder. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@nflaig
nflaig enabled auto-merge (squash) July 9, 2026 08:59
@nflaig
nflaig merged commit 33dbc18 into ChainSafe:unstable Jul 9, 2026
18 of 19 checks passed
@wemeetagain

Copy link
Copy Markdown
Member

🎉 This PR is included in v1.45.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants