feat: builder selection, broadcast validation, stateless flow for gloas block production#9595
Open
nflaig wants to merge 38 commits into
Open
feat: builder selection, broadcast validation, stateless flow for gloas block production#9595nflaig wants to merge 38 commits into
nflaig wants to merge 38 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for both stateless and stateful block production flows in the post-Gloas fork, allowing validators to request full block contents (including the execution payload envelope, KZG proofs, and blobs) when self-building. It updates the produceBlockV4 and publishExecutionPayloadEnvelope endpoints, implements broadcast validation logic, and updates the CLI, types, and tests accordingly. The review feedback highlights two important improvements: validating the submitted blob and KZG proof counts against expected values in the stateless flow to prevent downstream errors, and using optional chaining when accessing blobsBundle properties to avoid potential runtime crashes during block production.
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.
nflaig
commented
Jul 5, 2026
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
nflaig
commented
Jul 14, 2026
| if (builderBid === null && builderSelection === routes.validator.BuilderSelection.BuilderOnly) { | ||
| throw new ApiError(400, `No builder bid available for slot=${slot} with builderSelection=builderonly`); | ||
| } | ||
| const buildLocalBlock = builderSelection !== routes.validator.BuilderSelection.BuilderOnly; |
Member
Author
There was a problem hiding this comment.
I feel like this builderonly option shouldn't exist at all, but removing it would be a breaking change, at the same time, I don't know why anyone would use this, was added by Gajinder, if I remember correctly this was relevant for dvts at some point, but there is no known users that I am aware of now
Member
There was a problem hiding this comment.
technically every hard fork is a breaking change. I don't think we have to support this thing forever. Happy to retire it, think about a deprecation strategy.
Member
Author
There was a problem hiding this comment.
technically every hard fork is a breaking change.
not really, I mean yes, users have to upgrade their node but with 0 other changes otherwise, but regarding builder settings, if we go with the proposal from terence/potuz, this needs to be reworked anyways
nflaig
commented
Jul 14, 2026
| * TODO GLOAS: re-evaluate cutoff timing | ||
| * TODO GLOAS: re-evaluate cutoff timing due to attestation deadline changes in gloas | ||
| */ | ||
| const BLOCK_PRODUCTION_RACE_CUTOFF_MS = 2_000; |
Member
Author
There was a problem hiding this comment.
this may be fine even for gloas if the deadline is at 3 seconds since we only need to publish the tiny beacon block, but if the attestation deadline is further reduced, this needs to be revisited for sure
also worth considering using a _BPS value instead of _MS to scale with slot duration
nflaig
commented
Jul 14, 2026
| // handle shouldOverrideBuilder separately | ||
| if (engine.status === "fulfilled" && engine.value.shouldOverrideBuilder) { | ||
| logger.info("Selected engine block: censorship suspected in builder blocks", { | ||
| logger.warn("Selected engine block: censorship suspected in builder blocks", { |
Member
Author
There was a problem hiding this comment.
unrelated, but I thought we always logged this as a warn so users can notice it more easily in the logs
nflaig
marked this pull request as ready for review
July 14, 2026 16:51
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9595 +/- ##
============================================
- Coverage 52.49% 52.49% -0.01%
============================================
Files 848 848
Lines 60485 60483 -2
Branches 4466 4466
============================================
- Hits 31754 31752 -2
Misses 28670 28670
Partials 61 61 🚀 New features to boost your workflow:
|
nflaig
marked this pull request as draft
July 17, 2026 18:00
Member
Author
|
need to rework this a bit, after bringing in the circuit breaker changes dealing with |
nflaig
commented
Jul 17, 2026
| }) | ||
| : Promise.reject(new Error("Builder disabled")); | ||
|
|
||
| const enginePromise = isEngineEnabled |
Member
Author
There was a problem hiding this comment.
the diff here is kinda annoying but it's due to getting rid of builderonly behavior, so isEngineEnabled can only be true now, the bool is no longer required
nflaig
marked this pull request as ready for review
July 17, 2026 21:28
This comment was marked as off-topic.
This comment was marked as off-topic.
![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: b2d14ade48
ℹ️ 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".
nflaig
changed the title
nflaig
changed the title
wemeetagain
approved these changes
Jul 17, 2026
twoeths
reviewed
Jul 20, 2026
| publishBlindedBlockV2, | ||
|
|
||
| async publishExecutionPayloadEnvelope({signedExecutionPayloadEnvelope}) { | ||
| async publishExecutionPayloadEnvelope({signedEnvelope, broadcastValidation}) { |
Member
There was a problem hiding this comment.
this signedEnvelope could be SignedExecutionPayloadEnvelopeContents so I'd like to reflect on the name
Suggested change
| async publishExecutionPayloadEnvelope({signedEnvelope, broadcastValidation}) { | |
| async publishExecutionPayloadEnvelope({signedEnvelopeOrContents, broadcastValidation}) { |
twoeths
reviewed
Jul 20, 2026
| if (cachedGloasResult?.cells) { | ||
| cells = cachedGloasResult.cells; | ||
| } else { | ||
| dataColumnTimer = metrics?.peerDas.dataColumnSidecarComputationTime.startTimer(); |
Member
There was a problem hiding this comment.
this dataColumTimer is hard to monitor as is
consider adding source: computed | cache or something like that to differentiate having to compute cells or not
getGloasDataColumnSidecars() takes minority of the total time I guess
twoeths
reviewed
Jul 20, 2026
| !isForkPostGloas(produceResult.fork) || | ||
| produceResult.type !== BlockType.Full | ||
| ) { | ||
| throw Error(`Missing cached block production result for produced block root=${blockRoot}`); |
Member
There was a problem hiding this comment.
add more information to the error message, at least the slot
twoeths
reviewed
Jul 20, 2026
| @@ -987,9 +1051,41 @@ export function getValidatorApi( | |||
| void chain.persistBlock(block, "produced_engine_block"); | |||
Member
There was a problem hiding this comment.
post-gloas we need to also at least persist payload envelope
(not part of this PR through)
twoeths
reviewed
Jul 20, 2026
| if (blockState === null || !isStatePostGloas(blockState)) { | ||
| throw new ApiError( | ||
| 400, | ||
| `Unable to regenerate block state for consensus checks blockRoot=${blockRootHex}` |
Member
There was a problem hiding this comment.
add block slot to the error message
also this means an issue of beacon-node itself (5xx), not validator client (4xx)
twoeths
reviewed
Jul 20, 2026
| // Publish all data column sidecars | ||
| ...dataColumnSidecars.map((dataColumnSidecar) => () => network.publishDataColumnSidecar(dataColumnSidecar)), | ||
| // Import execution payload. Signature already verified above | ||
| // Import execution payload. Signature is verified during broadcast validation, an already |
Member
There was a problem hiding this comment.
signature was not verified if "none" BroadcastValidation
twoeths
reviewed
Jul 20, 2026
| } | ||
|
|
||
| const payloadInput = chain.seenPayloadEnvelopeInputCache.get(blockRootHex); | ||
| if (!payloadInput) { |
Member
There was a problem hiding this comment.
this check should be moved to before line 852
we only need to get payloadInput from seenPayloadEnvelopeInputCache once
then reuse the returned payloadInput found
twoeths
reviewed
Jul 20, 2026
| builderIndex: envelope.builderIndex, | ||
| isSelfBuild, | ||
| blobDataIncluded, | ||
| broadcastValidation, |
Member
There was a problem hiding this comment.
would be great to also include number of blobs if we have it
...(submittedContents !== null ? {submittedBlobs: submittedContents.blobs.length} : {}),
twoeths
reviewed
Jul 20, 2026
| broadcastValidation, | ||
| }) | ||
| .catch((e: Error) => { | ||
| this.metrics?.blockProposingErrors.inc({error: "publish"}); |
Member
There was a problem hiding this comment.
create (if we don't have any) and use new metrics for payload
also at least include slot/block root to the error messages
twoeths
reviewed
Jul 20, 2026
| ).assertOk(); | ||
| } | ||
|
|
||
| this.logger.info("Published block and execution payload envelope", { |
Member
There was a problem hiding this comment.
need to include stateful/stateless information by using executionPayloadIncluded variable
twoeths
reviewed
Jul 20, 2026
| }) | ||
| .catch((e: Error) => { | ||
| this.metrics?.blockProposingErrors.inc({error: "publish"}); | ||
| throw extendError(e, "Failed to publish execution payload envelope"); |
Member
There was a problem hiding this comment.
same to above, use payload metrics + enhance error message to include slot
also include stateful/stateless information
twoeths
reviewed
Jul 20, 2026
twoeths
left a comment
twoeths
left a comment
Member
There was a problem hiding this comment.
looks good to me, I dropped minor comments
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
5 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.
--payloadLocalflag to configure behavior stateless vs. statefulproduceBlockV4now respectsbuilder_selectionandbuilder_boost_factordefaultbuilder selection strategy (uses in-protocol p2p bids and local block, selecting by value), explicit--builder.selectionconfig is still honoredbroadcast_validationfor publishing execution payload envelopes (exceptequivocationchecks){schema: type: boolean}in query params and request headersbuilderonlybuilder selection option, it's a safety concern and adds unnecessary complexity