feat: trigger processExecutionPayload() once#9223
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the payload and data column synchronization logic to decouple envelope arrival from data availability. It introduces a waitForAllData mechanism in PayloadEnvelopeInput and a verifyPayloadsDataAvailability helper, allowing the execution payload import process to wait for necessary data columns—either via sampling or reconstruction—before proceeding. Review feedback highlights the need to properly guard the resolution of the new allDataPromise to ensure it only fires once and suggests a safer way to calculate completion time to avoid potential crashes with incomplete inputs.
Performance Report✔️ no performance regression detected Full benchmark results
|
There was a problem hiding this comment.
💡 Codex Review
lodestar/packages/beacon-node/src/chain/blocks/importExecutionPayload.ts
Lines 101 to 103 in b37bc6c
importExecutionPayload() now runs on envelope arrival before columns are complete, but this block still emits execution_payload_available immediately. That means subscribers can be told a payload is “available” while verifyPayloadsDataAvailability() is still waiting (up to 12s), which breaks the event’s stated semantics and can trigger premature payload-attestation logic in downstream consumers.
ℹ️ 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".
| // NOTE: we do NOT call chain.processExecutionPayload here. That is triggered only by | ||
| // envelope arrival (gossip or API). An in-flight importExecutionPayload is awaiting | ||
| // payloadInput.waitForAllData(); addColumn above will resolve it once hasAllData flips. |
There was a problem hiding this comment.
Re-trigger payload import when columns arrive after wait timeout
With the data-column trigger removed here, payload import is now only kicked off on envelope arrival; if that single import attempt times out waiting for columns in verifyPayloadsDataAvailability(), later column arrivals have no path to restart processing. In practice this can leave a valid envelope permanently unimported when columns are delayed beyond the timeout window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
In this case, unknown block sync should be triggered to gather payloads. This can be triggered via timeout or via new block arrival which builds on the payload.
There was a problem hiding this comment.
There should be something added here like the fulu path, which awaits the payload and data, and triggers a chain event in the incomplete case.
twoeths
left a comment
There was a problem hiding this comment.
the new function verifyPayloadsDataAvailability(payloadInputs: PayloadEnvelopeInput[]) is a preparation for gloas range sync
Motivation
Post-gloas,
chain.processExecutionPayload()was being triggered from 4 sites, including one that fires on every data-column gossip sidecar — up to 128 times per slot. Each call reached the queue only to be silently dropped by a!isComplete()guard while waiting for the envelope or missing columns. A standing TODO inimportBlock.tstracked this same issue.Description
Mirror the existing
BlockInput/verifyBlocksDataAvailabilitypattern for payload envelopes.chain.processExecutionPayload()only on envelope arrival (gossip + API publish). Removes the per-column gossip trigger and thegetBlobsTracker.triggerGetBlobsonComplete callback from block import.PayloadEnvelopeInput.waitForAllData(timeout, signal)— mirrorsBlockInput.waitForAllData; resolves once on the firststate.hasAllDatatransition.verifyPayloadsDataAvailability(payloadInputs, signal)— exact twin ofverifyBlocksDataAvailability(array input,PAYLOAD_DATA_AVAILABILITY_TIMEOUT = 12_000). Reusable by future gloas sync services.importExecutionPayloadawaits the helper after the ProtoBlock lookup and before claiming a write-queue slot.!isComplete()early return inPayloadEnvelopeProcessor— theimportStatusWeakMap still dedupes duplicate triggers for the samepayloadInput.onCompleteparameter fromGetBlobsTracker.triggerGetBlobs.waitForAllDatavswaitForComputedAllDatadistinction.Closes #9150
AI Assistance Disclosure
Used Claude Code to assist with codebase exploration, planning, implementation, and tests. The design, code, and review were gone through by me before submission.