Skip to content

fix: require imported payload for index==1 attestation gossip #9611

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nflaig merged 1 commit into from
Jul 7, 2026
+12 −5
Merged

fix: require imported payload for index==1 attestation gossip #9611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/beacon-node/src/chain/opPools/proposerPreferencesPool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {toRootHex} from "@lodestar/utils";
export class ProposerPreferencesPool {
private readonly bySlot = new Map<Slot, Map<RootHex, gloas.SignedProposerPreferences>>();

/** Lookup for bid validation: matches `(bid.slot, get_proposer_dependent_root(parent_state, ...))`. */
/** Lookup for bid validation: matches `(bid.slot, get_shuffling_dependent_root(store, bid.parent_block_root, epoch))`. */
get(slot: Slot, dependentRootHex: RootHex): gloas.SignedProposerPreferences | null {
return this.bySlot.get(slot)?.get(dependentRootHex) ?? null;
}
Expand Down
15 changes: 11 additions & 4 deletions packages/beacon-node/src/chain/validation/attestation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,17 @@ async function validateAttestationNoSignatureCheck(
// [REJECT] If `attestation.data.index == 1` (payload present for a past
// block), the execution payload for `block` passes validation.
// [IGNORE] When `attestation.data.index == 1` (payload present for a past block),
// the corresponding execution payload for `block` has been seen (a client MAY queue
// attestations for processing once the payload is retrieved and SHOULD request the
// payload envelope via `ExecutionPayloadEnvelopesByRoot`).
if (block !== null && attData.index === 1 && !chain.seenPayloadEnvelope(toRootHex(attData.beaconBlockRoot))) {
// the corresponding execution payload for `block` has been fully imported, including its
// data -- i.e. `is_payload_verified(store, beacon_block_root)` returns True (consensus-specs
// #5355). `forkChoice.hasPayloadHexUnsafe` is the equivalent of `root in store.payloads`:

@nflaig nflaig Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we need to add a AGENTS.md rule or something to stop these spec reference like consensus-specs #5355, I would rather not have this or have a full URL to the spec PR

// it is true only once the SignedExecutionPayloadEnvelope has been imported and verified,
// not merely seen on gossip. A client MAY queue attestations until the payload is imported
// and SHOULD request the payload envelope via `ExecutionPayloadEnvelopesByRoot`.
if (
block !== null &&
attData.index === 1 &&
!chain.forkChoice.hasPayloadHexUnsafe(toRootHex(attData.beaconBlockRoot))

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce imported-payload rule for aggregates too

Please apply the same imported-payload gate to aggregate validation as well. validateAggregateAndProof still uses chain.seenPayloadEnvelope(...), and seenPayloadEnvelope includes seenPayloadEnvelopeInputCache.hasPayload before importExecutionPayload adds the FULL fork-choice variant, so an index==1 aggregate submitted through publishAggregateAndProofsV2 can still be accepted and re-published after merely seeing a gossip envelope but before DA/EL import has completed. That leaves the #5355 rule enforced for single attestations here but not for aggregated attestations carrying the same payload-present vote.

Useful? React with 👍 / 👎.

@nflaig nflaig Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this code was unreachable anyways because we would queue those attestations and not process them cc @twoeths correct me if that's wrong

@twoeths twoeths Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes we do queue index=1 gossip attestation awaiting for payload
for api attestation, we did not handle that. We only call await validateGossipFnRetryUnknownRoot(validateFn, network, chain, slot, beaconBlockRoot), which is not enough for gloas.

) {
throw new AttestationError(GossipAction.IGNORE, {
code: AttestationErrorCode.EXECUTION_PAYLOAD_NOT_SEEN,
beaconBlockRoot: toRootHex(attData.beaconBlockRoot),
Expand Down
Loading