engineapi: validate payloadAttributes before SYNCING short-circuit#20728
Merged
engineapi: validate payloadAttributes before SYNCING short-circuit#20728
Conversation
Per the engine API spec, payloadAttributes must be validated against the
fork schedule regardless of the fork-choice update's sync state. The
previous ordering returned {status: SYNCING} without running the
withdrawals/beacon-root/fork-version checks, so a CL that sent
fcuV2 with nil withdrawals at a Shanghai timestamp got no error while
the head was still syncing.
Extract the checks into validatePayloadAttributes and run them before
the SYNCING short-circuit. Add a regression test covering the SYNCING
path. Fixes hive engine-withdrawals / Empty Withdrawals (Paris).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates engine_forkchoiceUpdated handling to validate payloadAttributes against the fork schedule even when forkchoice processing returns SYNCING, aligning behavior with the Engine API spec and preventing invalid attributes from being masked by a syncing response.
Changes:
- Extracted fork/version-specific
payloadAttributeschecks into a newvalidatePayloadAttributeshelper. - Moved
payloadAttributesvalidation to run before theSYNCING/non-VALIDshort-circuit inforkchoiceUpdated. - Added a regression test ensuring invalid attributes return
-38003even when the EL would otherwise respondSYNCING.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
execution/engineapi/engine_server.go |
Adds validatePayloadAttributes and invokes it before the SYNCING short-circuit in forkchoiceUpdated. |
execution/engineapi/testing_api_test.go |
Adds a regression test covering invalid payloadAttributes on the SYNCING path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
taratorio
approved these changes
Apr 22, 2026
sudeepdino008
pushed a commit
to benderrobert/erigon
that referenced
this pull request
Apr 29, 2026
…gontech#20891) ## Summary - Hive `engine-cancun / Invalid PayloadAttributes, Missing BeaconRoot, Syncing=True (Cancun)` (test 376 of [this run](https://hive.ethpandaops.io/#/test/generic/1777279481-85063528cea3c966b05ec4fef21a48ad?testnumber=376)) was failing because Erigon validated `parentBeaconBlockRoot` before the SYNCING short-circuit and returned `-38003` when the head was unknown. - Split `validatePayloadAttributes` into request-level (pre-FCU) and point-(8)-extension (post-FCU) halves so the spec-mandated ordering is preserved on both the SYNCING and VALID paths. - The fix that motivated erigontech#20728 (hive `engine-withdrawals / Empty Withdrawals (Paris)`) still passes — its check (V2 wrong withdrawals presence) is in the pre-FCU group. ## Why [`cancun.md`](https://github.com/ethereum/execution-apis/blob/main/src/engine/cancun.md#engine_forkchoiceupdatedv3) says fcuV3 _extends_ point (8) of `engine_forkchoiceUpdatedV1`: > 2. **Extend point (8)** of the `engine_forkchoiceUpdatedV1` specification by defining the following sequence of checks that **MUST** be run over `payloadAttributes`: > 1. `payloadAttributes` matches the `PayloadAttributesV3` structure, return `-38003: Invalid payload attributes` on failure. > 2. `payloadAttributes.timestamp` does not fall within the time frame of the Cancun fork, return `-38005: Unsupported fork` on failure. > 3. `payloadAttributes.timestamp` is greater than `timestamp` of a block referenced by `forkchoiceState.headBlockHash`, return `-38003: Invalid payload attributes` on failure. …and point (8) of V1 (paris.md) gates the entire processing flow on the head being `VALID`: > 8. Client software **MUST** process provided `payloadAttributes` **after successfully applying the `forkchoiceState`** and **only if the payload referenced by `forkchoiceState.headBlockHash` is `VALID`**. So with an unknown head, point (8) doesn't fire and the response is the SYNCING one in point (9). The `Syncing=True` family of hive cancun tests pin exactly this. The pre-FCU "wrong version of structure" rule is different — it lives in the V2 [Request section](https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#engine_forkchoiceupdatedv2) and has no precondition on head state, which is why erigontech#20728 correctly moved it before the short-circuit. ## What changed - `validatePayloadAttributesPreFCU` keeps the request-level checks: `parentBeaconBlockRoot` on V1/V2, V1/V2 fcu at a Cancun timestamp, withdrawals presence vs Shanghai. Runs before the SYNCING short-circuit. - `validatePayloadAttributesPostFCU` runs the V3 point-(8) extensions: `parentBeaconBlockRoot != null` for V3+, Cancun-window for V3+, `SlotNumber != null` for V4+. Runs only when `status.Status == ValidStatus`. - Two regression tests in `testing_api_test.go`: - `TestForkchoiceUpdatedV3DefersAttributesValidationWhenSyncing` — pins the hive `Syncing=True` scenario (no error, status=SYNCING). - `TestForkchoiceUpdatedV3RejectsMissingBeaconRootWhenValid` — pins the spec-mandated `-38003` when the head is VALID so the deferral can't regress into a permanent swallow. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: info@weblogix.biz <admin@10gbps.weblogix.it>
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
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.
Summary
forkchoiceUpdated, so a CL that sends invalid attributes gets-38003/-38005even when the EL is still syncing.validatePayloadAttributeshelper (no behavior change for VALID-status FCUs).Why
The engine API spec requires
payloadAttributesto be validated against the fork schedule independently of the fork-choice sync state. The previous code returned{status: SYNCING}up front wheneverHandleForkChoicecouldn't produceVALID(e.g. unknown head, busy executor), so the CL never saw the spec-mandated error.Noticed while looking at hive run
1776846836-81a466ccd994e017e6ff298c773d9ab6— the single failure inengine-withdrawalswasEmpty Withdrawals (Paris), where CLMocker sent fcuV2 withwithdrawals: nullat a Shanghai timestamp:Geth validates attributes at the top of its fcuV2 handler (before the shared forkchoice update path) for the same reason. This PR brings erigon in line.
Test plan
go test ./execution/engineapi/... -count=1(new + existing tests pass)make lint(clean)make erigon(builds)engine-withdrawalsto confirmEmpty Withdrawals (Paris)now passes