fix: make addPayloadEnvelope idempotent on API path

[markolazic01]

Motivation

addPayloadEnvelope throws if called when an envelope is already set. This is reachable from the API handler when the envelope has already been set by another source before the call reaches addPayloadEnvelope, causing a 500 to the VC.

Description

All call sites of addPayloadEnvelope were audited before applying the fix:

gossipHandlers.ts — unguarded, but all handler throws are caught by gossipValidatorFn and converted to TopicValidatorResult.Ignore, so safe
sync/unknownBlock.ts — guarded with !payloadInput.hasPayloadEnvelope() before calling
sync/utils/downloadByRange.ts — same guard, safe
api/impl/beacon/blocks/index.ts — unguarded, the only path where the throw caused a user-visible error

Adds the same hasPayloadEnvelope() guard to the API handler, matching the existing sync path pattern. A debug log is emitted when the envelope is already set to aid debugging.

Closes #9071

[gemini-code-assist]

Code Review

This pull request adds a check to prevent overwriting an existing execution payload envelope, logging a debug message when skipping a duplicate. The reviewer recommends verifying whether the incoming envelope matches the existing one by comparing their block hashes, and logging a warning instead of a debug message if they differ, as this could indicate a potential equivocation or software bug.

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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7ba49abf0

ℹ️ 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".

[chatgpt-codex-connector]

Handle known envelopes before validation

When the duplicate envelope is already in seenPayloadEnvelopeInputCache before this API call starts validation, the earlier validateApiExecutionPayloadEnvelope() call still throws ENVELOPE_ALREADY_KNOWN, so execution never reaches this new idempotency guard. This only fixes the narrow race where another path sets the envelope after validation but before addPayloadEnvelope(); VC retries or gossip-first arrivals still get an error instead of an idempotent success. Consider checking/returning success for the already-known case before validation, or mapping that validation result to the intended duplicate behavior.

Useful? React with 👍 / 👎.

[lodekeeper]

This one is still relevant. The current branch catches ENVELOPE_ALREADY_KNOWN before reaching the add, but that catch returns immediately, so it avoids the validation throw while also bypassing later publish/process side effects. After #9595 this should be reworked on top of the new signedEnvelopeOrContents / broadcastValidation flow rather than patched in place.

[markolazic01]

needs more work

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d40231371e

ℹ️ 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".

[markolazic01]

fixed the typing in the test, the failing type check should now pass

[nflaig]

let's revisit this after #9595, converting to draft for now

[nflaig]

• this will conflict with feat: builder selection, broadcast validation, stateless flow for gloas block production #9595

[markolazic01]

will follow up after it lands 👍

[lodekeeper]

Confirmed now that #9595 is merged: GitHub marks #9504 as DIRTY, and merge-tree conflicts in blocks/index.ts around this exact validation/add-envelope area. I posted the situation analysis and recommended rebasing/reworking on current unstable rather than stacking more changes onto this draft branch.

[markolazic01]

@lodekeeper now that #9595 is merged, can you write down a situation analysis?

[lodekeeper]

Situation after #9595:

• feat: builder selection, broadcast validation, stateless flow for gloas block production #9595 is merged, and fix: make addPayloadEnvelope idempotent on API path #9504 is now a draft with mergeStateStatus=DIRTY against unstable. A side-effect-free merge-tree check confirms the conflict is in packages/beacon-node/src/api/impl/beacon/blocks/index.ts around the same validation/add-envelope area this PR changes.
• The original fix: make addPayloadEnvelope idempotent on API path #9504 goal is still valid: make duplicate publishExecutionPayloadEnvelope API submissions idempotent instead of failing the VC retry path.
• The Gemini hash-mismatch thread appears addressed in the current branch by aad8ee5: the branch compares the cached envelope payload block hash and warns if the same beacon block root is seen with a different payload hash.
• The two Codex idempotency threads are not fully resolved in the current branch. It now catches ENVELOPE_ALREADY_KNOWN from validation, but that path returns immediately, so a retry after a partial API failure can still skip the publish/process side effects. The later hasPayloadEnvelope() guard does fall through to publish/process, but only for the race where validation passed and the envelope appeared before add.
• feat: builder selection, broadcast validation, stateless flow for gloas block production #9595 changed this endpoint substantially: it now accepts signedEnvelopeOrContents, has broadcastValidation modes, handles stateless blob contents, and has special handling for already-known envelopes with submitted contents. So I would not stack more changes onto the current conflicted draft branch.

Recommended next shape: rebase/rework this on current unstable and centralize the duplicate decision after validation has established the request context. Desired behavior should be:

1. No cached envelope: add the API envelope, publish envelope/data columns, process execution payload.
2. Same cached envelope: skip only the add, but still run the publish/process side effects that are valid for this request, especially when submitted blob contents are present.
3. Different cached envelope for the same beacon block root: warn and do not silently treat it as a normal duplicate.

No code pushed from me in this pass because the branch is conflicted with #9595 and needs a deliberate rework on top of the new endpoint shape.

[markolazic01]

@nflaig do we want to close this? Your changes in #9595 seem to fix the issue.
Only thing this PR has to offer is an envelope block hash divergence check/warning (which needs to be adapted) + tests, and these changes can be made into a new PR.

[lodekeeper]

@markolazic01 @nflaig I agree with closing #9504 rather than reworking this draft.

I rechecked current unstable after #9595. The original duplicate-publish idempotency issue is covered in the new endpoint shape: ENVELOPE_ALREADY_KNOWN is treated as a successful stateful retry, while the submitted-contents path still continues so data columns can be published and the payload can be processed. That is the important behavior this PR was trying to get to, but the old branch is now dirty/conflicting against the post-#9595 API shape.

The only remaining value I see is the narrower observability/test follow-up Marko called out: when an envelope is already known, validation short-circuits before comparing an incoming duplicate payload block hash against the cached/bid hash, so an adapted warning plus focused tests could still be useful. I would do that as a fresh small PR on unstable rather than salvage #9504.

Concrete recommendation: close #9504, and track a clean follow-up only for the divergent-hash warning/test coverage if we still want that signal.

[nflaig]

@nflaig do we want to close this? Your changes in #9595 seem to fix the issue. Only thing this PR has to offer is an envelope block hash divergence check/warning (which needs to be adapted) + tests, and these changes can be made into a new PR.

ah yes I think so, I didn't wanna close your PR yet because I didn't double check your PR, but since you did that we can probably close it for now, can keep the issue open as a reminder

[markolazic01]

sounds good @nflaig