feat: defer payload processing to next block#9257
Conversation
Move slot from ExecutionPayloadEnvelope to ExecutionPayload as slotNumber (uint64) and add slotNumber to PayloadAttributes per consensus-specs#4840. Updates all verification, gossip validation, signing, serialization, and SSZ byte extraction accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eturn New function implements deferred parent execution payload processing (consensus-specs#5094). Moves execution requests, builder payment, and availability updates from envelope processing to block processing. Removes the isParentBlockFull early return in processWithdrawals since processParentExecutionPayload now runs before withdrawals. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lock New ordering for Gloas blocks: 1. processParentExecutionPayload (NEW - before header) 2. processBlockHeader 3. processWithdrawals (no longer has empty-parent early return) 4. processExecutionPayloadBid 5. ... rest unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Envelope verification no longer mutates state. All state effects (execution requests, builder payment, availability, latestBlockHash) have moved to processParentExecutionPayload in the next block. Changes: - Remove state cloning, execution request processing, builder payment, availability/latestBlockHash updates, and state root verification - Add executionRequestsRoot check against bid commitment - Return void instead of post-state - Remove verifyStateRoot and dontTransferCache options - Rename stateView method to verifyExecutionPayloadEnvelope Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…onPayload With deferred payload processing, the envelope no longer produces a separate post-state. The FULL variant node shares the same stateRoot as the PENDING node. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- importExecutionPayload now does pure verification only — no state cloning, no post-payload state computation, no state root check, no regen.processState(), no checkpoint caching - Remove stateRoot from executionPayload and executionPayloadGossip events - Remove stateRoot from envelope construction in validator API - Remove stateRoot from fork choice onExecutionPayload call - Envelope verification runs via verifyExecutionPayloadEnvelope (void) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rocessing Block production: - Add executionRequestsRoot to ExecutionPayloadBid construction - Add parentExecutionRequests to BeaconBlockBody via getParentExecutionRequests() - Add getParentExecutionRequests() to BeaconChain (returns parent's execution requests from cached envelope, or empty if EMPTY parent) Gossip validation: - Add executionRequestsRoot check in envelope validation - Add EXECUTION_REQUESTS_ROOT_MISMATCH error code Cleanup: - Remove stateRoot from validator envelope logging - Fix spec test for void-returning processExecutionPayloadEnvelope - Update stale TODO comment in writePayloadEnvelopeInputToDb Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use electra.ExecutionRequests type instead of unknown[] - Fix parentBlockRootHex -> parentBlock.blockRoot variable name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements deferred payload processing for the Gloas fork, aligning the codebase with consensus-specs#5094. The implementation replaces the immediate state mutation in processExecutionPayloadEnvelope with a deferred approach in processParentExecutionPayload, where state effects are applied in the following block. Significant updates were made to block production and fork choice to handle the distinction between FULL and EMPTY parent variants. Review feedback identifies a critical consensus-breaking deviation in how builder payments are settled for older slots, a missing validation check for the execution head when the parent block is empty, and a logic error in fork choice initialization that hardcodes the payload status of checkpoints.
…processing # Conflicts: # packages/beacon-node/src/chain/blocks/importExecutionPayload.ts # packages/beacon-node/test/spec/presets/fork_choice.test.ts # packages/beacon-node/test/spec/utils/specTestIterator.ts # packages/state-transition/src/block/processExecutionPayloadEnvelope.ts
Performance Report✔️ no performance regression detected Full benchmark results
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56f686ce52
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
I don't know what happened here but it seems needed for spec tests, revisit later
|
|
||
| // 4. Apply backpressure from the write queue, before doing verification work. | ||
| // The actual DB write is deferred until after verification succeeds. | ||
| await this.unfinalizedPayloadEnvelopeWrites.waitForSpace(); |
There was a problem hiding this comment.
I don't understand why we did this, this doesn't make sense to me, I moved that check down after verification
| // 6. Persist payload envelope to hot DB (performed asynchronously to avoid blocking) | ||
| // 6. Persist payload envelope to hot DB. Wait for write-queue space here to apply backpressure | ||
| // on the import pipeline during sync, then perform the write asynchronously to avoid blocking. | ||
| await this.unfinalizedPayloadEnvelopeWrites.waitForSpace(); |
There was a problem hiding this comment.
do we even wanna update fork choice before applying backpressure? I keep this for now but worth to reconsider
There was a problem hiding this comment.
I think we should update fork choice before persisting it as fork choice is quite time sensitive. Writing to DB is not.
There was a problem hiding this comment.
we do that already, the db write is async, this is more about when to apply backpressure which in normal case shouldn't even happen, mostly during sync this is relevant
There was a problem hiding this comment.
it would be good to have more eyes on importExecutionPayload later but it's good enough to merge this cc @wemeetagain @ensi321 @twoeths
**Motivation** - We should not prune `PayloadEnvelopeInput` from `seenPayloadEnvelopeInputCache` right after we persist to DB, because block production needs it (next-slot `getParentExecutionRequests` and `prepareExecutionPayload` read the cached envelope synchronously). **Description** - Keep the last 2 `PayloadEnvelope`s in memory (head + head.parent) - In the worst case, we fall back to loading from DB. - This PR gets the most part of #9249 targeting #9257 to make it ready for unstable - block production modification is removed due to #9257 **AI Assistance Disclosure** Used Claude Code to assist with implementation and review. --------- Co-authored-by: Nico Flaig <nflaig@protonmail.com> Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com>
Will target
unstableafter #9254 is mergedimplement ethereum/consensus-specs#5094