fix: make addPayloadEnvelope idempotent on API path#9504
Conversation
markolazic01
changed the title
addPayloadEnvelope idempotent on API path
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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[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: 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".
| seenTimestampSec, | ||
| peerIdStr: undefined, | ||
| }); | ||
| if (!payloadInput.hasPayloadEnvelope()) { |
There was a problem hiding this comment.
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 👍 / 👎.
|
needs more work |
There was a problem hiding this comment.
💡 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".
|
fixed the typing in the test, the failing type check should now pass |
|
|
||
| await validateApiExecutionPayloadEnvelope(chain, signedExecutionPayloadEnvelope); | ||
| try { | ||
| await validateApiExecutionPayloadEnvelope(chain, signedExecutionPayloadEnvelope); |
There was a problem hiding this comment.
- this will conflict with feat: builder selection, broadcast validation, stateless flow for gloas block production #9595
There was a problem hiding this comment.
will follow up after it lands 👍
4 participants
Motivation
addPayloadEnvelopethrows 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 toTopicValidatorResult.Ignore, so safesync/unknownBlock.ts— guarded with!payloadInput.hasPayloadEnvelope()before callingsync/utils/downloadByRange.ts— same guard, safeapi/impl/beacon/blocks/index.ts— unguarded, the only path where the throw caused a user-visible errorAdds 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