feat: ignore index-1 attestation when payload is missing#9213
Merged
feat: ignore index-1 attestation when payload is missing#9213
Conversation
Spec PRs: - #5008: correct field name `block_root` to `beacon_block_root` in ExecutionPayloadEnvelopesByRoot validation comment - #5022: add block-known check in on_payload_attestation_message (already handled by gossip validation in payloadAttestationMessage.ts) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When attestation.data.index == 1 (payload present for a past block), the execution payload for the referenced block must have been seen. This adds the IGNORE check to both beacon_attestation and beacon_aggregate_and_proof gossip validation per consensus-specs #4939. The REJECT check (payload passes validation) is inherently satisfied since FULL variants in fork choice can only have Valid or Syncing execution status — invalid payloads never create FULL variants. The network processor already handles requesting missing payload envelopes via ExecutionPayloadEnvelopesByRoot and queuing messages for later processing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ensi321
changed the title
![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 a new attestation error code, EXECUTION_PAYLOAD_NOT_SEEN, and implements validation logic in aggregateAndProof.ts and attestation.ts to ensure that for attestations with index == 1, the corresponding execution payload has been seen. It also includes a minor documentation fix in executionPayloadEnvelope.ts. The review feedback suggests using the more comprehensive chain.seenPayloadEnvelope method instead of chain.forkChoice.hasPayloadUnsafe to check for the existence of the execution payload, as it better aligns with the specification by checking both gossip caches and fork choice state.
ensi321
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4652529094
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ensi321
changed the title
ensi321
changed the title
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
- Only check payload-seen when block is known; unknown blocks are handled later by verifyHeadBlockAndTargetRoot which enables the API retry path for unknown roots - Use chain.seenPayloadEnvelope which checks both the gossip cache (seenPayloadEnvelopeInputCache) and fork choice FULL variant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@lodekeeper can you do a review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da26e3413d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // 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))) { |
There was a problem hiding this comment.
Throwing EXECUTION_PAYLOAD_NOT_SEEN here causes submitPoolAttestationsV2 to fail index-1 submissions whenever the block is known but the payload envelope is still in flight, because its retry helper only catches UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT (validateGossipFnRetryUnknownRoot in network/processor/gossipHandlers.ts) and only waits for block availability. In fallback/DVT setups (multiple beacon nodes), this creates a regression where valid attestations are dropped instead of being retried once the envelope is imported.
Useful? React with 👍 / 👎.
| // 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))) { |
There was a problem hiding this comment.
This new IGNORE branch introduces the same failure mode for publishAggregateAndProofsV2: index-1 aggregates now error out when the block is present but payload envelope import has not completed, yet the API retry path only handles unknown block roots and does not retry on EXECUTION_PAYLOAD_NOT_SEEN. As a result, valid aggregates from external/fallback beacon nodes can be rejected instead of being processed after envelope retrieval.
Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment.
Sounds like both of these codex messages are saying that our queuing needs to handle these failed attestations / aggregates
Contributor
Author
There was a problem hiding this comment.
I think it is out of the scope of this PR. Tracking this issue at #9224
twoeths
reviewed
Apr 14, 2026
ensi321
added a commit
that referenced
this pull request
Apr 16, 2026
twoeths
approved these changes
Apr 16, 2026
Contributor
twoeths
left a comment
twoeths
left a comment
There was a problem hiding this comment.
looks good to me
note that the error can only happens on api path with a fallback node
we already handle the gossip path in network processor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9213 +/- ##
=========================================
Coverage 52.53% 52.53%
=========================================
Files 848 848
Lines 61424 61424
Branches 4528 4528
=========================================
Hits 32269 32269
Misses 29090 29090
Partials 65 65 🚀 New features to boost your workflow:
|
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
3 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.
beacon_attestationandbeacon_aggregate_and_proof) when the execution payload has not been seen. The REJECT check (payload passes validation) is inherently satisfied since FULL variants in fork choice can only have Valid or Syncing execution status. The network-layer plumbing (requesting missing envelopes viaExecutionPayloadEnvelopesByRoot, queuing messages) was already in place.