[r3.6] cl/beacon: resolve payload withdrawals in one place - #23339
Merged
Conversation
Split out of #23105, which grew too large to review in one piece. `produceBeaconBody` chose between three withdrawal sources inline — a Gloas head whose payload was revealed, a Gloas head whose payload was not, and everything before Gloas — each with its own hand-written conversion loop, and then assembled the payload attributes around whichever it picked. The choice is now a method that names what it selects between, and the attributes come from a single version-aware constructor. What is left inline is the genuinely fork-specific part: the two fields only Gloas sends. The three conversion loops go through the shared converter from hand-written when reviewing that PR. The refactor itself is behaviour-neutral, but the constructor also fixes two cases where the old inline construction built a request the chosen wire version cannot express: - **the parent beacon block root is omitted below Deneb** — Capella and Bellatrix dispatch to `forkchoiceUpdatedV2`/`V1`, and `validatePayloadAttributesPreFCU` rejects a non-nil parent root there with `InvalidPayloadAttributesErr`; - **withdrawals are omitted below Capella** — Bellatrix dispatches to `forkchoiceUpdatedV1`, which has no withdrawals field, and the expectation is a non-nil empty slice, so one was being sent. Both only bite on the engine transport, and both made the request fail rather than be ignored. The routing is otherwise unchanged, including the details that are easy to lose in a refactor: - a FULL Gloas head reads from the state copy with the parent payload applied, not from the head state; - an EMPTY Gloas head reads the expectation the state already cached, and does not compute a fresh one; - before Gloas the expectation is computed from the head state; - the resulting slice keeps its nil-ness in every case. `TestExpectedWithdrawalsReadsTheRightSourcePerFork` gives the two Gloas states different withdrawal outcomes, so it fails if the source selection is removed rather than passing either way. `TestPayloadAttributesOmitFieldsTheChosenVersionCannotCarry` pins the two gates across every fork. Part of a series splitting #23105. (cherry picked from commit af897d9)
yperbasis
approved these changes
Aug 17, 2026
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
2 participants
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.
Cherry-pick of #23280 to release/3.6. Depends on #23320, which is already on this branch.
r3.6-specific adaptations
One conflict, of the same shape as the earlier backports in this series:
produceBeaconBodyselects the withdrawal source with anif/else if/elsechain here, wheremainuses aswitch. The change replaces that block with the new resolver either way, so the resolution was to take it.I verified the result by diffing the change this branch introduces against the change #23280 introduces: the only differences are in the removed lines, which are the two shapes of the same inline block. Every added line is identical.
Behaviour change carried over
The refactor is behaviour-neutral, but the version-aware attributes constructor also fixes two cases where the old inline construction built a request the chosen wire version cannot express:
forkchoiceUpdatedV1/V2reject it;forkchoiceUpdatedV1has no such field.Both only bite on the engine transport, and both made the request fail rather than be ignored.
Part of backporting the series that split #23105.