Skip to content

Add Eth-Builder-Url header and pass it during block production - #11248

Merged
StefanBratanov merged 6 commits into
Consensys-Incorporated:masterfrom
StefanBratanov:eth_builder_url
Sep 15, 2026
Merged

StefanBratanov merged 6 commits into
Consensys-Incorporated:masterfrom
StefanBratanov:eth_builder_url

Conversation

@StefanBratanov

@StefanBratanov StefanBratanov commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR Description

When we create block with https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/ValidatorRequiredApi/produceBlockV4 we send back a header with the builder URL if builder was used, then VC uses that data to send it back to BN via the same header https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/ValidatorRequiredApi/publishBlockV2 . This PR basically achieves that apart from the usage of it which would be covered in a future PR. I did some refactor in BlockFactory classes to make it nicer and more extensible and less confusing. One of the main changes is the change to BlockContainerAndMetaData which now stores more information (payloadIncluded and builderUrl)

Fixed Issue(s)

related to #11194

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Note

Medium Risk
Touches block production and publish API contracts and refactors block factory metadata assembly on validator-critical paths; changes are mostly additive but affect how builder bids are surfaced end-to-end.

Overview
Adds Eth-Builder-Url to the beacon APIs so produce-block V4 can return the winning builder URL (when the bid came from the builder API) and publish-block V2 can accept the same header on the way back. BlockContainerAndMetaData now carries builderUrl and payloadIncluded, and Gloas block production fills those from slot caches and include_payload / self-built vs external-bid logic.

BlockFactory classes are refactored: unsigned block creation is split into createNewUnsignedBlock plus a shared createBlockContainerAndMetaDataBuilder, with Deneb/Gloas building metadata via the builder instead of mutating containers in place. ValidatorApiChannel.sendSignedBlock gains an Optional<String> builderUrl argument (call sites updated; full downstream use is noted as follow-up work).

Adds PostNewBlockV4IntegrationTest and OpenAPI fixture updates; drops the old V3 GLOAS JSON golden file in favor of V4-focused fixtures.

Reviewed by Cursor Bugbot for commit fd7c4bb. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@StefanBratanov
StefanBratanov force-pushed the eth_builder_url branch 2 times, most recently from d063703 to 00ed6a6 Compare September 8, 2026 13:43
@rolfyone

rolfyone commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Finding 1 — CONFIRMED (correctness): NPE in SSZ V4 block response path
ProduceBlockRequest.java:258 — The 4-arg ProduceBlockResponse SSZ constructor never sets executionPayloadIncluded (it's Boolean, boxed). When toMetaDataV4() calls .payloadIncluded(response.executionPayloadIncluded), it auto-unboxes null →
NullPointerException. Any validator with preferSszBlockEncoding=true hitting a V4 produce-block endpoint will crash block production for that slot.

Fix: Either set executionPayloadIncluded in the SSZ constructor (infer it from whether builderUrl is present/absent), or change the field type to boolean (primitive) and explicitly initialize it.


Finding 2 — CONFIRMED (correctness): Builder URL silently dropped in remote-VC path
RemoteValidatorApiHandler.java:273 — sendSignedBlock() now accepts builderUrl but calls typeDefClient.sendSignedBlock(blockContainer, broadcastValidationLevel) — the 2-arg overload, unchanged. OkHttpValidatorTypeDefClient and
SendSignedBlockRequest have no builderUrl parameter, so the Eth-Builder-Url header is never added to the HTTP POST /eth/v2/beacon/blocks request. The BN never sees the builder URL and the builder doesn't receive the signed block.

Fix: Thread builderUrl through OkHttpValidatorTypeDefClient.sendSignedBlock() and SendSignedBlockRequest.submit(), adding it as a request header when present.


Finding 3 — CONFIRMED (test-coverage): payloadIncluded=false path untested
PostNewBlockV4Test.java — shouldHandleWhenBeaconBlockIsProduced was deleted with no replacement. That test was the only one asserting HEADER_INCLUDE_PAYLOAD=false. After refactoring from instanceof BlockContentsGloas to
blockContainerAndMetaData.payloadIncluded(), a regression in the false-payload path would go undetected.


Finding 4 — PLAUSIBLE (correctness): Builder.build() missing null-checks
BlockContainerAndMetaData.java — Required fields (blockContainer, specMilestone, executionPayloadValue, consensusBlockValue) are not validated in build(). Silent nulls surface as NPEs at unrelated call sites, obscuring the root cause.

@StefanBratanov
StefanBratanov force-pushed the eth_builder_url branch 3 times, most recently from aa9c593 to 75414d7 Compare September 10, 2026 14:12
@StefanBratanov

Copy link
Copy Markdown
Contributor Author

Finding 1 — CONFIRMED (correctness): NPE in SSZ V4 block response path ProduceBlockRequest.java:258 — The 4-arg ProduceBlockResponse SSZ constructor never sets executionPayloadIncluded (it's Boolean, boxed). When toMetaDataV4() calls .payloadIncluded(response.executionPayloadIncluded), it auto-unboxes null → NullPointerException. Any validator with preferSszBlockEncoding=true hitting a V4 produce-block endpoint will crash block production for that slot.

Fix: Either set executionPayloadIncluded in the SSZ constructor (infer it from whether builderUrl is present/absent), or change the field type to boolean (primitive) and explicitly initialize it.

Finding 2 — CONFIRMED (correctness): Builder URL silently dropped in remote-VC path RemoteValidatorApiHandler.java:273 — sendSignedBlock() now accepts builderUrl but calls typeDefClient.sendSignedBlock(blockContainer, broadcastValidationLevel) — the 2-arg overload, unchanged. OkHttpValidatorTypeDefClient and SendSignedBlockRequest have no builderUrl parameter, so the Eth-Builder-Url header is never added to the HTTP POST /eth/v2/beacon/blocks request. The BN never sees the builder URL and the builder doesn't receive the signed block.

Fix: Thread builderUrl through OkHttpValidatorTypeDefClient.sendSignedBlock() and SendSignedBlockRequest.submit(), adding it as a request header when present.

Finding 3 — CONFIRMED (test-coverage): payloadIncluded=false path untested PostNewBlockV4Test.java — shouldHandleWhenBeaconBlockIsProduced was deleted with no replacement. That test was the only one asserting HEADER_INCLUDE_PAYLOAD=false. After refactoring from instanceof BlockContentsGloas to blockContainerAndMetaData.payloadIncluded(), a regression in the false-payload path would go undetected.

Finding 4 — PLAUSIBLE (correctness): Builder.build() missing null-checks BlockContainerAndMetaData.java — Required fields (blockContainer, specMilestone, executionPayloadValue, consensusBlockValue) are not validated in build(). Silent nulls surface as NPEs at unrelated call sites, obscuring the root cause.

1 is legit and I fixed it in this PR.
2 will be fixed in the next PR to avoid too many changes
4 is nice and I fixed it
For 3, we need to introduce PostBlockV4IntegrationTest and add these test cases. The other test is just a unit test pretty much and PostBlockV4 is fairly simple it just translates whatever we return, integration tests would cover other cases.

@StefanBratanov

Copy link
Copy Markdown
Contributor Author

@rolfyone added integration tests for block v4 as well to cover include_payload cases

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9db4229. Configure here.

@rolfyone rolfyone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StefanBratanov
StefanBratanov enabled auto-merge (squash) September 15, 2026 20:20
@StefanBratanov
StefanBratanov merged commit 2317886 into Consensys-Incorporated:master Sep 15, 2026
174 of 178 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 15, 2026
@StefanBratanov
StefanBratanov deleted the eth_builder_url branch September 15, 2026 20:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants