Convert produceBlockV4 to POST and round-trip Eth-Builder-Url (Gloas builder API 4/5) - #9806
Conversation
|
This pull request is part of a Mergify stack:
|
c6cb1f0 to
0ad508e
Compare
Revision history
|
0ad508e to
1541fc9
Compare
1541fc9 to
b75cd14
Compare
b75cd14 to
d763878
Compare
d763878 to
510daa9
Compare
510daa9 to
9ffd62f
Compare
d6bc23f to
f979c15
Compare
f979c15 to
f9c9f6b
Compare
f9c9f6b to
275486f
Compare
chong-he
left a comment
There was a problem hiding this comment.
Looks good, just some nits. I also notice the comment (beacon-APIs #630) is mentioned too many times, would be good to clean them up
| // Accepts the `BuilderPreferenceEntry` list as either JSON or SSZ, selected by the request's | ||
| // `Content-Type` (`application/octet-stream` => SSZ, otherwise JSON). A required |
There was a problem hiding this comment.
Nit: comment like this maybe is not necessary?
There was a problem hiding this comment.
Yeah agree, this is a fairly common pattern.
| .and_then(|content_type: Option<String>, body: Bytes| async move { | ||
| let builder_config: BuilderConfig = if content_type.as_deref() | ||
| == Some(SSZ_CONTENT_TYPE_HEADER) | ||
| { | ||
| BuilderConfig::from_ssz_bytes(&body).map_err(|e| { | ||
| warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}")) | ||
| })? | ||
| } else { | ||
| serde_json::from_slice(&body).map_err(|e| { | ||
| warp_utils::reject::custom_deserialize_error(format!("{e:?}")) | ||
| })? | ||
| }; |
There was a problem hiding this comment.
Both endpoints in this PR combines the request body of JSON and SSZ into one funciton, which is fine. The default is still JSON because if the header is not specified as application/octet-stream, then it goes to the else branch.
Just want to point out that I see in Lighthouse code base it is usually separate the JSON and SSZ request body into 2 functions, for example:
pub fn post_validator_proposer_preferencesaccepts the JSON request bodypub fn post_validator_proposer_preferences_sszaccepts the SSZ request body
There was a problem hiding this comment.
Good point, but this is just one parameter in the post body.
I'm fine either way, I'll leave this to mark.
6359b33 to
add5a11
Compare
add5a11 to
30c6c29
Compare
30c6c29 to
19d0ad7
Compare
|
Queued — the merge queue status continues in this comment ↓. |
19d0ad7 to
c9970db
Compare
|
|
||
| chain.task_executor.spawn( | ||
| async move { | ||
| match builders.forward_signed_block(&url, &block).await { |
There was a problem hiding this comment.
The client used for forward_signed_block follows redirects by default, so the builder can redirect the request to a different URL. The Beacon API spec explicitly prohibits this. Should we disable redirects on the client used here?
| } | ||
|
|
||
| // Consensus-consistency checks shared with the gossip verifier. | ||
| verify_bid_consistency(bid, proposal_slot, proposer_preferences, state, spec)?; |
There was a problem hiding this comment.
verify_direct_bid can accept a bid with block_hash == parent_block_hash. If that bid wins selection, per_block_processing rejects it and block production fails. Lighthouse does not then select another bid, even if the local build succeeded. This is in code merged by #9805, which the new POST endpoint exposes to configured builders. Should we add the hash check from #9970 to verify_direct_bid in a follow-up, so the invalid bid is rejected before selection?
There was a problem hiding this comment.
Nice catch. This was a disturbing issue with claude's rebase..
How the gap happened
Bid verification started out gossip-only: the checks lived inline in
GossipVerifiedPayloadBid::new, with private helpers — among them
verify_bid_payment_and_blobs (the gossip-only execution_payment == 0 rule plus the
blob-commitment limit) and verify_builder (active / version / collateral).
When #9805 added the direct (builder-API) intake, I extracted the checks common to both
channels into a shared layer: verify_bid_consistency (slot, fee recipient, blobs, state
conditions — what verify_direct_bid calls) and verify_bid_state_conditions (absorbing
verify_builder). As part of that, verify_bid_payment_and_blobs was split: its blobs half
became the shared verify_bid_blobs, while the payment check stayed behind as gossip-only.
The gossip path kept its own inline composition and does not call verify_bid_consistency.
While #9805 was still open, #9970 (implementing ethereum/consensus-specs#5594) added the
block_hash != parent_block_hash check — and placed it in verify_bid_payment_and_blobs,
which was the natural home in unstable at the time, since gossip was the only bid intake
that existed there.
Rebasing #9805 over #9970 then hit a conflict in that helper (the branch had changed its
signature). This change was placed right next to the gossip-only execution_payment == 0
check, so it got lost in the merge and applied only to the gossip side, rather than placed in
verify_bid_consistency, the shared validation function.
The fix extracts the check as verify_bid_block_hash_not_parent, called from both
verify_bid_payment_and_blobs (gossip, behavior unchanged) and verify_bid_consistency
(covering direct), with a direct-path regression test. I also audited every
process_execution_payload_bid assert against verify_direct_bid — this was the only one
not front-run by the direct path.
There was a problem hiding this comment.
Thanks for the analysis. This is actually a bit concerning and maybe we should be careful about single commit rebases for stacks. Its hard to keep track of without agents and you can't always trust agents.
I'm not really sure how we can be safe from these weird rebases for stacks from now on. We should chat about it on the next LH call.
c9970db to
33e8cdc
Compare
33e8cdc to
a03f186
Compare
…builder API 4/5) Fourth PR of the Gloas builder API stack (beacon-APIs #630): - convert `/eth/v4/validator/blocks/{slot}` to POST with an optional `BuilderConfig` body (min_bid, builder_boost_factor, direct builders) - add `POST /eth/v1/validator/builder_preferences` for forwarding signed builder preferences - set `Eth-Builder-Url` on produceBlockV4 responses when a direct-builder bid wins, accept it on `POST /eth/v2/beacon/blocks`, and forward the signed block to that builder The validator client still uses the legacy GET methods at this point; it migrates in the final PR of this stack. Change-Id: I0ad30b8f36ad9b588ea1a0398220f92c9597bb95
a03f186 to
81335ac
Compare
Merge Queue Status
This pull request spent 34 minutes 19 seconds in the queue, including 28 minutes 10 seconds running CI. Required conditions to merge
|
* Avoid walking already indexed validators in the monitor (sigp#9995) ## Issue Addressed The [validator monitor](https://github.com/sigp/lighthouse/blob/115bd16fb565c3df2b169a740fa5754adde00762/beacon_node/beacon_chain/src/validator_monitor.rs#L484-L502) walks the already indexed registry during recent block imports, including with default monitoring and zero monitored validators. Milhouse's `iter().skip(index)` traverses the skipped prefix even when there are no new validators. ## Proposed Changes Start the iterator at the first unknown index with `iter_from`, keeping absolute validator indices. A regression test covers empty registries, growth, shorter forks, late registration and pending updates for both fixed and progressive lists. ## Additional Info A local mainnet A/B on 7 Sep 2026 used two Lighthouse followers with separate mock ELs, zero monitored validators and a registry of about 2.36 million validators. Each follower had a 3-CPU quota on the same AMD EPYC-Milan VM. The builds used baseline `1256bd99`, with only this loop changed in the treatment. Each window followed one epoch of settling, with binaries swapped between nodes for the second window. | Median `import_block` duration | Baseline | Treatment | Matched blocks | | --- | ---: | ---: | ---: | | 19:29-19:43 UTC | 199.87 ms | 38.47 ms | 66 | | 19:53-20:07 UTC, after swap | 191.60 ms | 40.56 ms | 66 | One first-window trace was unmatched and excluded; baseline logs confirm that block was received and became head. All slow outliers are included. All 132 matched imports improved. Mean paired import savings were 159.70 ms and 152.68 ms; the monitor child span accounted for 158.24 ms and 150.67 ms respectively. This measures local import completion with simulated execution validity. The monitor runs under the fork-choice write lock, normally after early attestability. These results do not establish earlier attestations, whole-client CPU savings or the same absolute saving in production. * Remove buggy parent_root calculations from Gloas block production (sigp#9997) ## Issue Addressed Gloas block production was calculating the parent_root twice, once before advancing the parent state and once after. The calculation prior to the state advance **would yield the wrong block root** in the case where the parent state was not already advanced. In practice, this didn't occur very often because the state advance timer would make the advanced state available in the state cache. https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L185-L191 This potentially incorrect block root was used to calculate `should_build_on_full` on the next line: https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L193-L199 As a result, we would sometimes end up reading `should_build_on_full` for the **grandparent** block rather than the parent. Often, this wouldn't make any difference. The impact of this bug was further mitigated by the fact that we re-calculated the `parent_root` a 2nd time _after_ the state advance: https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L378-L398 This `parent_root` would always be correct, so the block would always be valid, just possibly building on the wrong empty/full variant. ## Proposed Changes Calculating the `parent_root` at all is conceptually unnecessary. It was already decided and known when we called `load_state_for_block_production`. This PR threads that value through and removes both re-calculations. A regression test is added in `gloas_block_production_parent_root_with_unadvanced_state`. I've verified that it fails on unstable and passes with this patch. ## Additional Info Codex was used, but as you can see from the commit history, there was a lot of tweaking to arrive at the simplest fix. Some more refactors and cleanups are possible (e.g. removing ReOrgInputs), but I'll do that in a separate PR to avoid muddying this bugfix. This bug is similar to another one recently found in block prod. I'm going to try to weed them all out: - sigp#9983 * Ignore exits for all withdrawing validators in Gloas block prod (sigp#9983) ## Issue Addressed Closes: - sigp#9981 ## Proposed Changes Filter out exits for validators which _could_ be impacted by any withdrawal triggered in the parent execution payload. See linked issue for details of the bug. This fix prevents production of invalid blocks (!!) in some scenarios post-Gloas. ## Additional Info Fix & comments written manually, tests fixed by Codex. Regression test by Codex with manual review. * Add attestations test with `payload_present` in op pool (sigp#9531) Add a test in operation pool about the index for the cases with and without payload_present. Written with Claude Code and did a self review Currently blocked waiting for ethereum/consensus-specs#5399 fix Update: the fix ethereum/consensus-specs#5473 is merged and included in [v1.7.0-alpha.13](https://github.com/ethereum/consensus-specs/releases/tag/v1.7.0-alpha.13), currently pending for Lighthouse update to alpha 13 * Don't return Pending node when fork choice reverts to justified block (sigp#9962) Closes sigp#9544. ## Description - `find_head_walk` was filtering virtual EMPTY/FULL children of a PENDING node against `viable_nodes`. Under deep non-finality the justified seed can be non-viable, so those children were dropped and `get_head` returned Pending, which breaks block production. - Skip the filter for PENDING heads. Viability filtering still applies to real block children from EMPTY/FULL nodes. - Adds `pending_head_resolves_when_justified_subtree_non_viable` in `gloas_payload.rs`. * Fix flaky unknown block test (sigp#10001) ## Issue Addressed Fix flaky CI test: - sigp#9999 ## Proposed Changes The test was failing due to a race between importing the block on gossip vs importing the block as a result of its `getBlobs` call completing. The `getBlobs` codepath was missing a notification to the reprocess queue, which this PR adds. There's also a new test added which prevents regression on the `getBlobs` codepath. ## Additional Info Heavily Codex driven, manually reviewed. Change makes sense to me. * Gloas spec beta 0 (sigp#10014) ## Issue Addressed Each commit has a link to the relevant consensus spec PR * Convert produceBlockV4 to POST and round-trip Eth-Builder-Url (Gloas builder API 4/5) (sigp#9806) Fourth PR of the Gloas builder API stack (beacon-APIs sigp#630): - convert `/eth/v4/validator/blocks/{slot}` to POST with an optional `BuilderConfig` body (min_bid, builder_boost_factor, direct builders) - add `POST /eth/v1/validator/builder_preferences` for forwarding signed builder preferences - set `Eth-Builder-Url` on produceBlockV4 responses when a direct-builder bid wins, accept it on `POST /eth/v2/beacon/blocks`, and forward the signed block to that builder The validator client still uses the legacy GET methods at this point; it migrates in the final PR of this stack. * `engine_getBlobsV4` (sigp#9438) ## Issue Addressed Implement EIP-8070's engine API calls: `engine_getBlobsV4`. ## Additional Info Claude was heavily used for prototyping. Gloas support will be added after sigp#9325 is merged. * Add 0x02 support to validator creation (sigp#9702) ## Issue Addressed NA ## Proposed Changes Adds the `--compounding` flag to `lighthouse validator_manager create` to allow creation of validators with 0x02-prefixed withdrawal credentials. ## Additional Info I had to move some test vectors from Holesky to Hoodi as `ethstaker-deposit-cli` has stopped supporting Holesky. Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io> * Standardise FCR metrics (sigp#9987) ## Issue Addressed Closes sigp#9663 ## Proposed Changes Adopt [beacon-metrics](https://github.com/ethereum/beacon-metrics/blob/master/metrics.md#fast-confirmation) | Standard metric | Was | Fires | |---|---|---| | `beacon_fast_confirmation_slot` | `beacon_fcr_confirmed_root_slot` | after each FCR run | | `beacon_fast_confirmation_reorgs_total` | new | head reorgs off a confirmed block | | `beacon_fast_confirmation_fallbacks_total` | `beacon_fcr_revert_to_finalized_total` | confirmed root reverts to finalized | | `beacon_fast_confirmation_restarts_total` | `beacon_fcr_restart_from_justified_total` | restart from the observed justified checkpoint | Added a metric to track head reorgs out of the previous confirmed root: - `_confirmed_root_reorgs_total` * Enable FCR test cases (sigp#9964) ## Issue Addressed Enable remaining FCR test cases ## Proposed Changes Fix fake_crypto to produce correct states if the input pub keys are valid ## Additional Info * broadcast PTC votes early on payload availability (sigp#9434) - Updated PayloadAttestationService to broadcast PTC votes as soon as the execution payload and data columns are available (via SSE execution_payload_available event). - Retained the existing deadline as fallback. - Follows the same early-trigger pattern used by AttestationService. Addresses sigp#9422 and sigp#9584 Co-authored-by: Eitan Seri- Levi <eserilev@gmail.com> Co-authored-by: Eitan Seri-Levi <eserilev@ucsc.edu> Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io> * Avoid copying unchanged justified balances (sigp#10023) ## Issue Addressed - `find_head` clones `JustifiedBalances` on every call, even when it hasn't changed. - During block import, this copy happens while holding the fork choice write lock. ## Proposed Changes - Use the existing `PartialEq` implementation to only clone `JustifiedBalances` when it changes. ## Additional Info - An instrumented mainnet run with a mock EL found unchanged balances in **130 of 131 calls**, including all 33 block imports. - Tested against the parent commit with two mainnet beacon nodes in Kurtosis, using mock ELs and equal CPU and memory limits. Both stayed synced with matching block roots and finalized checkpoints. - Across **96 block imports per node**, average fork choice time during block import dropped from **31.58 ms to 15.00 ms**, about **53% lower**. This includes `get_head` and the early attester cache update. Both nodes ran on the same dev box. * Bump `rustls` to fix cargo audit failure (sigp#10052) ## Issue Addressed Cargo audit failure from `rustls`: https://rustsec.org/advisories/RUSTSEC-2026-0285.html ## Proposed Changes Bump locked `rustls` version to `0.23.45` * Revert "Move `SlotAssignment` cache to `CanonicalHead` (sigp#9661)" (sigp#10047) ## Issue Addressed Per this comment: sigp#9831 (comment), it turns out we don't need the `SlotAssignments` cache for regular fork choice. Attempting to use it in the general case is difficult because we need to deal with the fact that cache may be stale, and that building it while holding a lock could be very slow. This PR reverts dbd2824 so that the cache is used ONLY for FCR. ## Proposed Changes Reverted the commit dbd2824 and resolved conflicts in `beacon_node/beacon_chain/src/canonical_head.rs` mostly relating to the fork choice poisoning (unrelated changes on nearby lines). ## Additional Info Codex used for conflict res, results checked manually. * fork choice compliance tests (sigp#9710) ## Issue Addressed Add consensus spec fork choice compliance tests Some tweaks were made to get tests passing: 1. For attestation related tests: If attestations are for a future-slot and queued we treat them as import failed. 2. Updated test runners to recompute head before every block import 3. Blocks rejected by block verification instead of fork choice skip fork-choice assertions. 4. We don't re-import block attestations after `on_block`. Lighthouse block import already does this, but via the from-block path, which skips the `validate_target_epoch_against_current_time` validation step 5. We only run complaince tests for fake_crypto + fulu/gloas minimal * Add base support for Heze block production (sigp#9714) ## Proposed Changes - Enable Heze block production by constructing `BeaconBlockHeze` in `complete_partial_beacon_block_gloas` (the body is identical to the Gloas beacon block variant) - This work is a pre-requisite for testing the upcoming FOCIL PRs - Route Heze block building through the existing Gloas `getPayload` path, because the [engine API spec for Bogota](https://github.com/ethereum/execution-apis/blob/main/src/engine/bogota.md) defines no new `getPayload` version for this fork. Heze responses are deliberately deserialized into the Gloas containers (see [TODO added](https://github.com/sigp/lighthouse/pull/9714/changes#diff-8b79597c7e1a33167a01f676e50f09dbc40e761dad5b294086ebda40828e1641R1064-R1065)), so the Heze-specific engine types stay unused until the payloads diverge. Likewise, envelope submission still uses the Gloas `newPayloadV5` path - Add test producing and importing blocks across the Heze boundary epoch This PR unblocks Heze testing through the beacon chain harness. Previously, with block production stubbed, the harness could not build or advance a Heze chain. This is a blocker for any Heze integration tests, a prerequisite for the upcoming FOCIL PRs' tests. --------- Co-authored-by: Jimmy Chen <jchen.tc@gmail.com> Co-authored-by: Michael Sproul <michaelsproul@users.noreply.github.com> Co-authored-by: chonghe <44791194+chong-he@users.noreply.github.com> Co-authored-by: Nikhil Sharma <nikhilsharma230303@gmail.com> Co-authored-by: Eitan Seri-Levi <eserilev@gmail.com> Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Co-authored-by: Daniel Knopik <107140945+dknopik@users.noreply.github.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io> Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Co-authored-by: Roheemah <60899500+AbolareRoheemah@users.noreply.github.com> Co-authored-by: Eitan Seri-Levi <eserilev@ucsc.edu> Co-authored-by: Mac L <mjladson@pm.me> Co-authored-by: Cristian Conache <C.conache@gmail.com>
Fourth PR of the Gloas builder API stack (beacon-APIs #630):
/eth/v4/validator/blocks/{slot}to POST with an optionalBuilderConfigbody (min_bid, builder_boost_factor, direct builders)POST /eth/v1/validator/builder_preferencesfor forwarding signedbuilder preferences
Eth-Builder-Urlon produceBlockV4 responses when a direct-builder bidwins, accept it on
POST /eth/v2/beacon/blocks, and forward the signedblock to that builder
The validator client still uses the legacy GET methods at this point; it
migrates in the final PR of this stack.