[MegaRedHand]

Fourth step of MEV-Boost builder support. With this PR, a fulu proposer registered through register_validator can receive a builder's blinded block from produceBlockV3 and publish it back through blinded_blocks. Stacked on #696.

Scope: the beacon node only. The validator client does not yet register, ask for blinded blocks or publish them; that is the next PR. Gloas (the builder API alongside gossip bids) comes after.

What it adds

Piece	Behavior
shouldOverrideBuilder	Read from engine_getPayloadV5, carried on the built payload
Registration cache	Each validator keeps the latest registration the builder accepted: this node does not verify their signatures, the builder does, so one it refuses is never recorded. Fee recipients are set either way. An entry lasts 2 epochs after it was last sent, and an older timestamp never replaces a newer one
Circuit breaker	Keeps the builder out while the node is syncing or the head is optimistic. Also trips on more than --builder-fallback-skips (3) consecutive empty slots (counted from the head block, as Lighthouse does), on more than --builder-fallback-skips-per-epoch (8) empty slots in the last 32, or on more than --builder-fallback-epochs-since-finalization (3) epochs since the state's finalized checkpoint
Bid validation	Checks, in order: parent hash, prev_randao, timestamp, block number, withdrawals root, blob count and blob gas, gas limit against the registered target, then the BLS signature (DOMAIN_APPLICATION_BUILDER). Every refusal is counted by reason
Selection	Uses builder_boost_factor. The local payload wins iff local >= factor * bid / 100, multiplied first and rounded down once, so the default of 100 compares exactly (the spec's text rounds the bid first; see docs/spec_deviations.md). A tie goes local; shouldOverrideBuilder beats everything; u64::MAX means builder; the default factor is 100. The winner is chosen before assembly, so only one state transition runs. If the chosen block fails to assemble, the other one is used. This includes a local choice falling back to the bid, which Lighthouse would fail instead, following beacon-APIs' "prefer local unless it is not viable". Under shouldOverrideBuilder that fallback logs a warning, along with why the local block failed. A bid that pays nothing is accepted but never beats a local payload, whatever the factor: it is served only when the local payload or its block fails
produceBlockV3	A builder win answers the blinded block with Eth-Execution-Payload-Blinded: true and the bid's value. A local win answers as before. Once a valid bid is in hand, the local build gets 2 s from the request (Lodestar's race cutoff; Lighthouse's getPayload timeout) before the bid is served instead; with no valid bid the local build is awaited as before
engine_getPayload	One attempt with a 1 s timeout, instead of the generic 8 s × 3 retries. execution-apis #958 removed the method's timeout and requires an immediate answer; Prysm and Nimbus use 1 s, Lighthouse and Teku 2 s
POST /eth/v2/beacon/blinded_blocks	Fulu, SSZ or JSON. Checks the parent, the proposer and its signature, claims the slot, then submits to the builder on a spawned task. The builder's 202 becomes 200 and its 400 is passed through; anything else is a 500. Another 2xx is a 500 with its own error log and outcome label (unexpected_success), since that builder may still publish the block. There is no local fallback, since that would be a second block. broadcast_validation is not read
Slot guard	post_block, the gloas post_block and blinded_blocks all claim the slot after the signature check and before anything leaves the node. A different root for a claimed slot gets a 400. Before any state is advanced, a block not after the finalized slot, or whose parent cannot descend from the finalized checkpoint, gets a 400 too: gossip's own rules, which bound how far an unauthenticated publish can make the node advance
Future-slot bound	produceBlockV3, produceBlockV4, both publishBlockV2 paths and blinded_blocks refuse a slot that has not started (more than MAXIMUM_GOSSIP_CLOCK_DISPARITY, 500 ms, ahead of the node's clock) before advancing any state to it, as Nimbus does. A block further ahead would be ignored on gossip anyway
Builder error text	Every control character in a builder's error message (line breaks, escape sequences, U+2028/U+2029) is written out before the 1 KiB cap, so it cannot forge a log line or drive a terminal
Metrics	lean_beacon_builder_bid_rejections_total{reason}, lean_beacon_builder_payload_source_total{source}, lean_beacon_builder_fallback_total{reason}

Known limitations

• The slot guard is in memory and per node. A restart forgets it, and two beacon nodes behind one validator client do not share it. It stops this node from handing out two different blocks for a slot, which is the case the builder flow adds. It is not slashing protection.
• Registered proposers wait for the bid. The header request runs alongside the local build and is bounded by --builder-header-timeout (1 s), so it adds at most the difference.
• Block assembly still runs on the async runtime, as the local path already did. Moving it onto a blocking task is a follow-up.
• Not checked, as in every client reviewed:
  • the bid's fee recipient, and the payment to the proposer: builders name themselves and pay with the payload's last transaction, which the header does not carry, and the relay checks that payment (see docs/spec_deviations.md);
  • its execution requests;
  • the header fields only an execution client could verify.

Follow-ups (to review after merge)

• Bound what blinded_blocks forwards. It sends on any blinded block whose proposer signature verifies, so a local caller can blind a recent gossip block, have it pass, and make this node submit it to the relay. Nothing bounds the submissions in flight. This is not an equivocation path, but it is relay spam from this node's address. No other client limits it either. Lighthouse, Prysm, Teku, Nimbus and Lodestar all forward blinded blocks whose headers they did not serve, and none caps concurrent submissions. Two options: a semaphore on in-flight submissions, or forwarding only headers this node served.

Tests

• Bid validation: one case per check, and a bid paying nothing accepted.
• Selection: a table of local and bid outcomes, zero bids and the one-wei edge at factor 100 included; the scaled bid checked against the exact product, and its saturation.
• Circuit breaker:
  • every trip;
  • edges near genesis and across the block_roots wrap;
  • the consecutive count on a state advanced through slot processing.
• Registration cache, and register_validator recording only what the builder accepts: a forged registration under a later timestamp leaves the accepted one in place.
• produceBlockV3 against the mock relay:
  • builder win;
  • local win;
  • override;
  • boost factor;
  • relay down;
  • invalid bid;
  • both sides failing;
  • a zero bid standing in for a failed local payload, and losing to a working one.
• blinded_blocks:
  • accepted;
  • builder 400;
  • builder 500;
  • a builder answering 200;
  • the claim surviving a failed submit;
  • the cross-endpoint guard, both ways (full then blinded, blinded then full);
  • a refused block leaving the slot unclaimed;
  • two different blocks posted at once;
  • the body limit.
• A gloas block claiming its slot.
• A slot that has not started, refused on every block endpoint.
• Builder client:
  • a 2xx other than 202 as its own error;
  • line breaks written out in error text.
• CLI flags.

make lint passes, as do the rpc lib and integration tests and the binary's tests.

[github-actions]

🤖 Kimi Code Review

I'll review this PR which implements the Fulu builder flow (MEV-Boost integration) for ethlambda. Let me analyze the key components systematically.

Overall Assessment

This is a substantial, well-structured PR adding builder/MEV-Boost support. The code is generally high quality with good test coverage, but I found several correctness and safety issues that need attention.

---

Critical Issues

1. Integer Overflow in choice.rs Boost Calculation (Line 58-62)

let boosted = u128::from(boost_factor).saturating_mul(bid / 100);

File: crates/net/rpc/src/beacon/builder_flow/choice.rs

Problem: The bid / 100 division before multiplication loses precision for small bids. A bid of 99 wei becomes 0, making any boost_factor > 0 result in boosted = 0, causing incorrect local payload selection.

Fix: Use proper fixed-point arithmetic:

let boosted = u128::from(boost_factor)
    .saturating_mul(bid)
    .saturating_div(100);

This preserves precision and matches the documented semantics: "local payload wins iff local >= factor * (bid / 100)" — actually the docs say factor * bid / 100, not factor * (bid / 100).

---

2. Missing Pruning in PublishedProposals — Memory Growth

File: crates/net/rpc/src/beacon/blinded_blocks.rs, lines 88-98

pub(crate) fn claim(
    &self,
    slot: Slot,
    root: Root,
    finalized_slot: Slot,
) -> Result<(), ClaimError> {
    if slot < finalized_slot {
        return Err(ClaimError::Finalized);
    }
    let mut published = self.0.lock().expect("published proposals lock poisoned");
    *published = published.split_off(&finalized_slot);
    // ...
}

Problem: split_off(&finalized_slot) keeps entries >= finalized_slot, but the check slot < finalized_slot only prevents new claims below finalized. However, if finalized_slot advances past existing entries, those entries are pruned. But: the split_off uses &finalized_slot as key — this is correct for BTreeMap (keeps key >= given).

Actually, re-reading: this looks correct. The issue is more subtle — there's no background pruning. If the node never produces blocks, old entries accumulate. But given the bounded nature (only when this node proposes), this is acceptable.

Status: Acceptable for now, but consider documenting this is bounded by proposal frequency.

---

3. Race Condition in Block Production — Double Proposal Risk

File: crates/net/rpc/src/beacon/proposal.rs, lines 608-700

The produce function fetches bid and builds payload concurrently:

let (built, el_version, bid) = tokio::join!(
    build_payload(store, engine, fee_recipients, &state, head_root, proposer),
    graffiti::execution_client_version(engine, own_version),
    fetch_bid(request.as_ref(), slot),
);

Then later claims the slot after building:

// In post_block / post_blinded_block, not in produce!
if let Err(err) = published.claim(slot, root, finalized_slot(&store)) {

Problem: The slot claim happens in the publish endpoint, not in produce. This creates a window where:

1. produce returns a blinded block to validator client
2. Validator client signs and submits to post_blinded_block
3. Meanwhile, another request (or retry) could produce a different block for same slot

Actually, looking more carefully: produce doesn't claim. The claim is in post_blinded_block and post_block. But produce can be called multiple times before publication. This is by design — the validator client might request, then not publish. But it means the node could produce multiple different blocks for the same slot if called repeatedly.

The actual risk: The PublishedProposals claim in post_blinded_block checks signed.message.hash_tree_root(). But what if two different produce calls return different blinded blocks (e.g., different bids arrived), the validator signs both, and submits both? The second would be caught by PublishedProposals if roots differ.

This appears correct — the claim prevents equivocation at publish time. The window between produce and publish is the validator client's responsibility.

---

4. BLS Signature Verification Missing Domain Check in verify_proposer_signature

File: crates/blockchain/state_transition/src/beacon/stf/mod.rs, lines 224-235

pub fn verify_proposer_signature(
    state: &BeaconState,
    proposer_index: ValidatorIndex,
    block_root: Root,
    signature: &BlsSignature,
) -> bool {
    let Ok(proposer) = state.validator(proposer_index) else {
        return false;
    };
    let domain = get_domain(state, constants::DOMAIN_BEACON_PROPOSER, None);
    let signing_root = compute_signing_root(block_root, domain);
    bls::verify(&proposer.pubkey, signing_root, signature)
}

Problem: The fork parameter to get_domain is None, which uses the state's current fork. For a blinded block, this is correct if the state is at the block's slot. But this function is public and documented as working for "a blinded block has the root of its full block, so one check serves both."

Verification needed: Confirm that advance_to_slot in the caller advances through any epoch boundaries, which would update the state's fork. Looking at post_blinded_block:

let state = advance_to_slot(&parent_state, slot, &config)
    .map_err(|_| "advancing the parent state failed")?;

This should handle fork transitions. Status: Likely correct, but worth explicit verification in tests.

---

5. Builder Bid Validation — Missing Execution Payload Hash Check

File: crates/net/rpc/src/beacon/builder_flow/bid.rs, lines 89-118

The validate function checks many header fields but not block_hash. The comment says:

What is left unchecked (the payload's execution validity, its block hash and the fields only an execution client could verify) is trusted to the relay

Problem: The block_hash is commit-revealed later — the proposer signs the blinded block, then the builder reveals the full payload. If the builder lies about block_hash, the revealed payload won't match, but the proposer is already committed.

Status: This is the fundamental trust model of MEV-Boost. Documented and accepted. But consider adding a comment that this is intentional and matches other clients.

---

6. Critical: should_override_builder Not Used in Local Block Path

File: crates/net/rpc/src/beacon/proposal.rs, lines 650-670

let local_offer = local.as_ref().ok().map(|parts| LocalOffer {
    value: wei_u128(&parts.built.block_value),
    should_override_builder: parts.built.should_override_builder,
});

This is used in choice::choose. But looking at local_block function, the should_override_builder field from BuiltPayload is only read for the choice, not passed through to the actual block.

Wait — re-reading: should_override_builder is an input to the choice, not a field in the produced block. The execution client is saying "prefer local regardless of bid value." This is correct usage.

But: Is should_override_builder persisted anywhere? If the execution client sets this flag, does the block production honor it in the block itself? Looking at BuiltPayload:

pub should_override_builder: bool,

This comes from the engine API. It's only used in the choice, not in the block. This is correct — it's a hint for payload selection, not a consensus field.

---

7. Potential Division by Zero in choice.rs

File: crates/net/rpc/src/beacon/builder_flow/choice.rs, line 58

let boosted = u128::from(boost_factor).saturating_mul(bid / 100);

If bid < 100, bid / 100 == 0. With boost_factor > 0, boosted == 0. Any positive local.value wins, which may be unintended.

Actually this matches the documented "factor * (bid / 100)" but the docs also say "100 compares the two as they are." With bid=50, boosted = 0, so local wins with any positive value. This seems like a precision loss bug.

Fix: Use the corrected formula from Item 1:

let boosted = u128::from(boost_factor)
    .saturating_mul(bid)
    .saturating_div(100);

---

8. Missing Check: builder_boost_factor Upper Bound

File: crates/net/rpc/src/beacon/proposal.rs, line 375

#[serde(default)]
builder_boost_factor: Option<u64>,

No validation that this is a reasonable percentage. u64::MAX is explicitly handled as "always take builder" (except override), but what about values like 10_000 (10000%)?

Status: The math uses u128 for intermediate, so no overflow. Very large values just heavily weight toward local. This is acceptable — it's a client configuration.

---

9. Test Gap: No Test for should_override_builder in SSZ Serialization

File: crates/net/engine/src/building.rs, lines 264-280

The GetPayloadV5Response deserializes should_override_builder with #[serde(default)]. Tests verify JSON deserialization, but no test verifies SSZ encoding/decoding if BuiltPayload is ever SSZ-encoded.

Looking at BuiltPayload — it's not derived SszEncode/SszDecode. It's only used internally. Status: Acceptable.

---

10. Lock Poisoning Handling — Inconsistent Panic Strategy

File: Multiple files

The code uses .expect("... lock poisoned") throughout, which panics on poisoned locks. Examples:

• crates/net/rpc/src/beacon/blinded_blocks.rs:93
• crates/net/rpc/src/beacon/builder_flow/cache.rs:52
• crates/net/rpc/src/beacon/proposal.rs:564

Problem: In a consensus client, a panic is a DoS vector. A poisoned lock from a panicked thread brings down the whole node.

Recommendation: Consider using parking_lot locks which don't poison, or handle poisoning gracefully. This is a broader codebase issue, not specific to this PR.

---

Medium Issues

11. Builder Request Doesn't Include builder_boost_factor in Cache Key

File: crates/net/rpc/src/beacon/proposal.rs, fetch_bid function

The builder bid is fetched without considering builder_boost_factor. If the same slot is requested with different factors, the same bid is returned. But produce is called fresh each time, so this is fine — no caching at this layer.

Status: Correct, no issue.

---

12. advance_to_slot Error Handling in post_blinded_block

File: crates/net/rpc/src/beacon/blinded_blocks.rs, lines 183-186

let state = advance_to_slot(&parent_state, slot, &config)
    .map_err(|_| "advancing the parent state failed")?;

The error is discarded. advance_to_slot can fail for various reasons (invalid state transition, etc.). Logging the actual error would help debugging.

Fix:

.map_err(|err| {
    error!(%err, "advancing the parent state failed");
    "advancing the parent state failed"
})?;

---

13. Missing Timeout on submit_blinded_block

File: crates/net/rpc/src/beacon/blinded_blocks.rs, lines 220-235

let submitted = tokio::spawn(async move {
    let result = client.submit_blinded_block(&signed, ForkName::Fulu).await;
    // ...
})
.await;

The client.submit_blinded_block likely has its own timeout (from BuilderClient config), but this isn't obvious. If not, this could hang indefinitely.

Check: Verify BuilderClient has a timeout configured. Looking at initialization in main.rs, the builder_header_timeout is used. But is there a separate timeout for submit_blinded_block?

Looking at BuilderClient — not shown in diff, but the builder_header_timeout is specifically for getHeader. The submission timeout might be different.

Status: Needs verification. Consider adding explicit timeout documentation.

---

14. Registration Timestamp Comparison — Potential Replay

File: crates/net/rpc/src/beacon/builder_flow/cache.rs, lines 45-48

Some(entry) if registration.timestamp < entry.registration.timestamp => {}
Some(entry) => {
    entry.registration = registration.clone();
    entry.seen = epoch;
}

An older timestamp is ignored, but equal timestamps replace. If a validator client resends the same registration with the same timestamp, it refreshes the lifetime. This is correct.

But: what if a malicious relay sends old registrations with forged timestamps? The timestamp field is not cryptographically verified — it's just a u64 in the ValidatorRegistrationV1.

Status: This matches the trust model — the builder API client trusts the relay. The validator client is expected to use current timestamps.

---

Minor Issues / Suggestions

15. Typo in Comment

File: crates/net/rpc/src/beacon/builder_flow/bid.rs, line 274

/// The gloas gossip predicate this reuses agrees with Lighthouse's

Should be "gas" not "gloas" (which is a fork name).

---

16. Inconsistent Error Type for post_blinded_block

File: crates/net/rpc/src/beacon/blinded_blocks.rs, line 153

return ApiError::Internal("no builder is configured (--builder)").into_response();

This returns 500 for a configuration issue. A 400 or 503 might be more appropriate since the endpoint requires a builder to be configured.

Actually, looking at beacon-APIs spec — this is a server configuration issue, so 500 is arguably correct. But 503 "Service Unavailable" with a clear message might be better.

Status: Stylistic choice, not a bug.

---

17. Test Naming Inconsistency

File: crates/net/rpc/src/beacon/proposal.rs, line 1420

async fn a_bid_worth_more_than_the_local_payload_is_served_blinded() {

Tests use a_... prefix inconsistently. Some use the_.... Not a functional issue.

---

18. Documentation Clarification Needed

File: docs/rpc.md, lines 452-453

A bid is checked before it competes: a non-zero value, the head's execution block as its parent, the proposal's prev_randao and timestamp, the parent's block number plus one, the withdrawals root, the blob commitment and blob gas bounds, a gas limit within one step of the parent's toward the registered target, and the builder's signature.

Missing: the fee_recipient is explicitly not checked, as noted in spec_deviations.md. The RPC docs should mention this or link to the deviation.

---

Correctness Verification: Fork Choice and Consensus

19. Circuit Breaker Empty Slot Detection

File: crates/net/rpc/src/beacon/builder_flow/breaker.rs, lines 108-118

fn is_empty(state: &BeaconState, i: Slot) -> bool {
    match (
        get_block_root_at_slot(state, i),
        get_block_root_at_slot(state, i - 1),
    ) {
        (Ok(root), Ok(previous)) => root == previous,
        _ => false,
    }
}

Verification: This checks if block_roots[i] == block_roots[i-1]. In process_slots, empty slots carry forward the previous block root. This is correct per the comment.

Edge case: At slot 0, i - 1 would underflow. But is_empty is only called with i >= 1 (see check: (1..slot).rev() and window_start = slot.saturating_sub(SLOTS_PER_EPOCH).max(1)). Verified correct.

---

20. State Transition for Blinded Block

File: crates/net/rpc/src/beacon/proposal.rs, blinded_block function

The assemble_blinded_block is called with BlindedBlockInputs. This must:

1. Process the block through the state transition
2. Compute the state root
3. Return a BlindedBeaconBlock (with ExecutionPayloadHeader, not full payload)

Not shown in diff, but assuming this exists and is correct. The test a_bid_whose_block_fails_to_build_falls_back_to_a_local_block_that_fails_too verifies fallback behavior.

---

Summary

Severity	Count	Items
Critical	1	Item 1: Integer overflow/precision loss in boost calculation
High	1	Item 7: Division by zero-like behavior in choice (same root cause)
Medium	3	Items 12, 13, 14
Low/Style	5	Items 15, 16, 17, 18, 10

Required Changes

1. Fix choice.rs line 58 — use saturating_mul(bid).saturating_div(100) instead of saturating_mul(bid / 100)
2. Add error logging in post_blinded_block for advance_to_slot failure
3. Fix typo "gloas" → "gas" in bid.rs comment
4. Verify/document submit_blinded_block timeout behavior
5. Add cross-reference in rpc.md to spec_deviations.md for fee recipient

Good Practices Observed

• Comprehensive test coverage with property-based style tests
• Clear separation of concerns (bid validation, breaker, choice, cache)
• Proper use of tokio::spawn for builder submission to prevent client hangup cancellation
• PublishedProposals prevents equivocation across full and blinded block paths
• Metrics for

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

[github-actions]

🤖 Codex Code Review

Overall this looks solid. The new builder flow is thoughtfully guarded: bid validation is fairly strict, the anti-equivocation slot claim is shared across both publish paths, and the circuit-breaker logic is explicit and tested.

• No correctness or security bug jumped out in fork choice / justification / attestation / STF / SSZ areas from this PR’s touched code; the risky surface here is mostly builder integration, and the checks are generally conservative.

• One thing I’d flag is a potential availability footgun in crates/net/rpc/src/beacon/blinded_blocks.rs:177 and crates/net/rpc/src/beacon/blinded_blocks.rs:199. The slot is claimed before the builder submission result is known, and the claim is never released if submit_blinded_block returns 400/500 or the task fails. That means a transient builder failure can permanently block a retry of the same slot through this node, even though nothing was published. If that behavior is intentional, it should be documented as “signing commits the slot locally”; otherwise consider claiming only after a successful handoff, or rolling the claim back on failure.

• Related to that, crates/net/rpc/src/beacon/proposal.rs:258 claims the slot before publish_beacon_block, and crates/net/rpc/src/beacon/proposal.rs:267 returns 500 if P2P is down without releasing the claim. Same operational consequence: a local transport failure can make retried publication impossible for that slot. For slashing protection this may be acceptable, but it is stricter than “published once” and closer to “attempted once”.

• In crates/net/rpc/src/beacon/builder_flow/choice.rs:52, the boost comparison computes factor * (bid / 100) rather than (factor * bid) / 100. That floors early and slightly favors the builder/local boundary for bids not divisible by 100 wei. Probably negligible in practice, but if you want exact beacon-APIs semantics, reorder to multiply first with saturating arithmetic and divide afterward.

• In crates/net/rpc/src/beacon/builder_flow/breaker.rs:90 and crates/net/rpc/src/beacon/builder_flow/breaker.rs:95, both skip counters scan historical slots linearly on every proposal. That’s fine at epoch scale, but the consecutive-skip scan can walk up to slot-1 until the first filled slot. It’s not dangerous, just worth noting if proposal hot paths become tighter; bounding it by the threshold would avoid needless reads once tripping is inevitable.

• Minor maintainability point: crates/net/rpc/src/beacon/registrations.rs:100 records cached registrations before the async forward completes. That matches the local-fallback design, but it means block production may consult registrations the builder never actually accepted. If that is the intended trust model, a short comment near builder.registrations.record(...) is enough; otherwise cache-on-success would be safer.

If you want, I can turn these into ready-to-post GitHub review comments.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Claude Code Review

Review of PR 704: builder flow for fulu block production

This covers the builder bid fetch, bid validation, the circuit breaker, the local-vs-builder choice, POST /eth/v2/beacon/blinded_blocks and the equivocation guard. Overall it is carefully built and well tested. I found no correctness or security blockers. I read the diff but did not build it or run the tests. The notes below are minor.

What looks good

• Fallback ordering in produce: the chosen source is built first, and the other is tried only if that build fails. local_error surfaces only if the builder does not step in, so a bad bid or a failed local build never costs the proposal.
• Bid checks: they run cheapest first, with the BLS pairing last. The state transition is also a backstop through assemble_blinded_block, and a bid that fails it is counted as state_transition.
• Equivocation guard:
  • PublishedProposals is shared by the full-block, gloas and blinded publish paths.
  • Retrying the same root is allowed.
  • Claims are made only after proposer, parent and signature checks, so an unauthenticated caller can't poison a slot.
  • The slot == finalized_slot boundary is tested.
• Blinded publish: the submission runs in a detached tokio::spawn, so a client disconnect can't cancel the reveal after the proposer has signed. There is deliberately no local fallback after signing.
• Circuit breaker: the "empty slot iff block_roots[i] == block_roots[i-1]" logic is correct. Slot 0 is never judged, and slots older than the state's history count as filled. The wrap-around and genesis-edge tests cover this.
• Arithmetic: checked_add on block number, saturating_mul in choose, and the u64::MAX and should_override_builder special cases are handled explicitly.

Minor points

1. choose truncates the bid (choice.rs, bid / 100 before multiplying). With factor 100, a bid of 1 ETH + 99 wei loses to a local payload of exactly 1 ETH, because the bid is rounded down to a multiple of 100 wei. The impact is negligible. If you want it exact, use local * 100 >= factor * bid with saturating_mul or a u128 overflow guard. The current form is defensible since it matches the beacon-APIs text.

2. The bid fetch could be skipped (proposal.rs, produce). fetch_bid runs in the tokio::join! even when builder_boost_factor == 0 (the local payload always wins). The builder request is wasted and its latency, up to the header timeout, still gates the response. Skipping the request in builder_request when the factor is 0 would help. should_override_builder is only known after the build, so that case can't be skipped up front.

3. PublishedProposals is in-memory only. After a restart, a validator client that retries with a different block for a slot already signed gets no protection. This is consistent with the repo's note that there is no slashing-protection record, but the spec_deviations doc could say it explicitly.

4. Blocking work in post_blinded_block. store.get_state(&parent_root) is a storage read that runs on the async task. The state advance and BLS check are correctly in spawn_blocking. Moving the state read inside it would match the pattern in registrations.rs. The endpoint also advances a full state per request with no concurrency limit. That is acceptable on a validator-client-facing API, but worth knowing.

5. Fee-recipient mismatch is only logged at debug! (accepted_bid). That matches the documented relay behavior. A counter would help operators spot a relay that stops paying through a payload transaction.

6. Small simplification: Packed::droppable() rebuilds empty_sync_aggregate() for every comparison. It only runs on error paths, so this is cosmetic.

Tests
Coverage is good: the selection table, one rejection per bid field, forged and wrong-domain signatures, the breaker boundaries, registration expiry, and the CLI default and requires behavior. A test for the boost_factor == 0 fetch skip would go with point 2 if you adopt it.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[pablodeymo]

Security read of this PR, with the bid validation and the blinded-block path reviewed separately. No critical or high issues: the bid checks match Lighthouse's validate_bid against the state advanced to the proposal slot, the slot guard is atomic, shared by all three publish paths, set after the signature check and before anything leaves the node, and there is no local fallback once a blinded block is signed. One thing I'd fix before merging, plus smaller ones.

1. Bound the slot before advance_to_slot (blinded_blocks.rs, and the pre-existing post_block, post_gloas_block and produce)

blinded_blocks checks only require_fulu_slot and slot > parent.slot() before advance_to_slot, and the proposer and signature checks come after. A local caller posting a blinded block with slot = head + 32 * 10^6 and a garbage signature makes the node run a million epoch transitions on a blocking thread the client cannot cancel, one thread per repeat. post_block and post_gloas_block have the same shape, and produce runs its unbounded advance on the async runtime, which stalls the whole API. Refusing slot > wall_slot + 1 before advancing closes all four; this PR adds the third publish-side instance, so it seems the place to do it.

2. The registration cache trusts unsigned registrations, before the relay answers

register records the kept registrations into builder.registrations before the forward and keeps them whatever the relay says; the registration signature is not checked (#696). So a local caller can register the operator's proposer with gas_limit = 1 and a fresh timestamp: the relay rejects it, but this node now expects every real bid to step toward 1, every bid fails Rejection::GasLimit, and the proposer builds locally until the VC's next resend wins on timestamp, which the caller can keep beating. The same call opts any validator into the builder path. Recording only after the relay answers Ok, or verifying the registration signature once per validator per epoch, closes it. Low, since the same surface already sets the local fee recipient, but this PR is where the cache starts steering bids.

3. Any gossip block can be replayed through blinded_blocks

A blinded block's signing root equals the full block's, so a local caller can blind any recent network block and it passes parent, proposer and signature. The node claims the slot and spawns an uncancellable submit_blinded_block (up to two 5 s attempts) with no bound on how many are in flight, plus an error! per refusal. Not an equivocation path, but relay spam from this node's IP. A semaphore on in-flight submissions would do, or forwarding only headers this node served.

Smaller

• The fallback order serves the builder block when the local block fails to assemble, even when the EL set shouldOverrideBuilder. One block per request, so not an equivocation, but the override should probably hold, and a lean_beacon_builder_fallback_total reason for "local assembly failed" would keep a masked EL problem visible.
• A zero-value bid is refused even when the local build failed, so that slot is missed where a 0-value builder block would have been valid. Fine as a choice; worth a doc line.
• choose computes factor * (bid / 100); Lighthouse multiplies first. Up to 99 wei of difference, no overflow, just noting it.
• Only a builder 202 is success; a builder answering 200 is a 500 and a missed slot with no fallback, and broadcast_validation is not parsed. Both worth a line in docs/rpc.md.
• The builder's 400 text is reflected to the VC and logged with newlines intact, as in feat(beacon): --builder and register_validator #696.

Tests I'd add: blinded-then-full on the same slot (only full-then-blinded is covered); a claim on the gloas post_block path (the description says it claims, nothing exercises it); a rejected blinded block not claiming the slot; two different valid blocks posted concurrently; a far-future slot once bounded; a builder answering 200.

Checked and clean: parent hash, prev_randao, timestamp and withdrawals root from the advanced state; block number with checked_add; blob cap from the proposal epoch's schedule; the gas-limit step rule with no underflow; the signature over the BuilderBid root with the genesis-version builder domain, checked last; override beats everything, u64::MAX means builder, tie goes local, wei_u128 saturates symmetrically; the breaker's thresholds and block_roots wrap; the claim's check-and-insert under one mutex, pruned at the finalized epoch start; the 8 MiB streamed response cap and per-call timeouts on the builder client; the builder URL scrubbed from every error; static metric labels.

[MegaRedHand]

@pablodeymo on point 3 (a gossip block replayed through blinded_blocks, with nothing bounding the submissions in flight): I checked the other clients' blinded publish paths, and none of them limits this.

Client	A blinded block whose header it did not serve	Bound on submissions in flight
Lighthouse	reconstruct_block misses its payload cache and calls propose_blinded_beacon_block directly, before any signature, proposer or slot check (http_api/src/publish_blocks.rs)	none
Prysm	handleBlindedBlock forwards it. An equivocation check runs only with broadcast_validation=consensus_and_equivocation (rpc/prysm/v1alpha1/validator/proposer.go, rpc/eth/beacon/handlers.go)	none
Teku	Logs "Blinded block seems to not be built via either builder or local EL. Trying to unblind it via builder endpoint anyway." and forwards it (ExecutionBuilderModule.java)	none
Nimbus	Decodes it and goes straight to unblindAndRouteBlockMEV, which assumes slashing protection already ran (rpc/rest_beacon_api.nim, validators/message_router_mev.nim)	none
Lodestar	Past fulu, submitBlindedBlockToBuilder whenever the block is not in its production cache (api/impl/beacon/blocks/index.ts)	none

ethlambda already checks more here than any of them: the proposer and its signature are verified against the parent's state before the slot is claimed or anything is sent. I've left the bound as a follow-up after merge, in the PR description: either a semaphore on in-flight submissions, or forwarding only headers this node served.

[MegaRedHand]

Thanks for the reviews. The changes are in 074b459, 086f50c, d0b6752 and dc7e26f.

@pablodeymo's security review

1. Bound the slot before advance_to_slot: done. Every endpoint that advances a state now refuses a slot that has not started before advancing anything. That covers produceBlockV3, both publishBlockV2 paths, blinded_blocks, and also produceBlockV4, which had the same unbounded advance. A slot counts as not started when it begins more than MAXIMUM_GOSSIP_CLOCK_DISPARITY (500 ms) after the node's clock: gossip's is_future_slot, and the rule Nimbus applies to block production. A validator client asks for its block once its slot has started, and a block further ahead would be ignored on gossip anyway.
2. Registrations recorded before the relay answers: fixed. They are recorded for block production only once the builder accepts them, as Prysm, Teku and Lodestar do. Your scenario is a test: a forged gas_limit = 1 under a later timestamp is refused, and the accepted registration stays. Signatures are still left to the relay, since no client verifies them itself. Fee recipients are still set either way, as prepare_beacon_proposer would set them.
3. Gossip blocks replayed through blinded_blocks: see my comment above. No client bounds this, so it is a post-merge follow-up in the description.

Smaller points:

• shouldOverrideBuilder: I kept the fallback to the builder's block when the local block fails, because a block beats a missed slot, and Lodestar does the same. It now logs a warning when that happens under the override. The local assembly error, which was dropped before, is logged too.
• Zero-value bids: no longer refused. One never beats a local payload, whatever the factor, so it is served only when the local payload or its block fails.
• The boost factor: Lighthouse divides first too, following the spec's text (execution_layer/src/lib.rs:1241, still the case on unstable). It only skips the arithmetic at factor 100 (Skip calculation of boosted_relay_value when builder_boost_factor pro… sigp/lighthouse#5352). We now multiply first: factor * bid / 100, rounded down once and saturating. That matches Teku, Lodestar, Nimbus and our gloas produceBlockV4, and spec_deviations.md records the departure from the spec's order.
• Only 202 counts: kept. A builder answering any other 2xx gets its own error (outcome label unexpected_success) and a log line saying it may still publish the block, so it can be told apart from a refusal. Both this and broadcast_validation not being read are in docs/rpc.md.
• The builder's 400 text: line breaks are written out as \n and \r before the 1 KiB cap.

Tests: all six you listed are added:

• blinded-then-full on one slot;
• a claim on the gloas publish path;
• a refused blinded block leaving the slot unclaimed;
• two different blocks posted at once;
• a slot that has not started, on every block endpoint;
• a builder answering 200.

Automated reviews

• Boost-factor rounding (Kimi, Codex, Claude): changed as above.
• Registrations cached before the forward (Codex): fixed as above.
• The slot claim kept after a failed submit (Codex): intended. The same block again still passes as a retry (a_claim_survives_a_failed_submission); only a different block is refused.
• The breaker's unbounded walk (Codex): the consecutive-skips count is now one step from the head block, as Lighthouse computes head_skips.
• Skipping getHeader at factor 0 (Claude): not adopted. beacon-APIs defines factor 0 as "prefer local unless an error makes it unviable", so the bid is the fallback, and no other client skips the request.
• A counter for fee-recipient mismatches (Claude): not added. A mismatch is the normal case: builders name themselves as the fee recipient and pay the proposer with the payload's last transaction, which the relay checks. bid::validate and spec_deviations.md now say why.
• The in-memory slot guard (Claude): already under Known limitations.
• get_state on the async runtime (Claude): a follow-up, with block assembly, under Known limitations.
• The advance_to_slot error dropped (Kimi): now logged at every advance site.
• Already in place:
  • the submission timeout is SUBMIT_TIMEOUT, 5 s (Kimi);
  • rpc.md already points at the fee-recipient deviation (Kimi);
  • the "gloas" in the gas-limit test comment names the gloas gossip predicate it reuses, so it is not a typo (Kimi).

[MegaRedHand]

/review

[github-actions]

🤖 Codex Code Review

• [P1] Bound local building by the proposal deadline — proposal.rs:653. tokio::join! waits for the execution client even when a valid builder bid has already arrived. Engine calls retry three times with an eight-second timeout per attempt, so a stalled client delays the builder fallback beyond the 12-second slot and causes a missed proposal. Apply a slot-appropriate deadline to the local build, then use the validated bid when that deadline expires. Add a test with a nonresponding execution client and a healthy relay.

Tests could not run because rustup attempted to write to a read-only directory.

---

Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt

[github-actions]

🤖 Gemini Code Review

This PR is well-designed and thorough. The Fulu builder flow (produceBlockV3 bid fetching/validation/selection, circuit breaker, registration caching, and publishBlindedBlockV2) is cleanly structured and covers the critical edge cases:

• Equivocation prevention (beacon/blinded_blocks.rs, beacon/proposal.rs): PublishedProposals is shared across /eth/v2/beacon/blocks (Fulu & Gloas) and /eth/v2/beacon/blinded_blocks, claiming slots atomically before broadcast/submission while permitting idempotent retries of the same block root and pruning below the finalized slot.
• Disconnect-safe reveal (beacon/blinded_blocks.rs:199): Spawning the submitBlindedBlockV2 call ensures a validator client timeout/disconnect after signing does not abort the reveal to the relay, and distinguishing non-202 2xx responses (BuilderError::UnexpectedSuccess) avoids masking potential publications.
• Registration cache integrity (beacon/registrations.rs:137-148, beacon/builder_flow/cache.rs): Recording registrations only after the builder accepts them prevents unverified registrations from poisoning a validator's cached target gas limit.
• Future-slot DoS guard (beacon/proposal.rs:456-464): Checking is_future_slot before advancing state on both block production and publication endpoints prevents unbounded process_slots work and bounds PublishedProposals entries.
• Bid validation & boost arithmetic (beacon/builder_flow/bid.rs, choice.rs, breaker.rs): Pre-STF bid checks order cheap field checks before the BLS pairing, boosted avoids precision loss without widening past u128, and assemble_blinded_block verifies the full state transition before serving a blinded block.

No issues found — looks good to merge.

---

Automated review by Gemini · gemini-4-argon · custom prompt

[github-actions]

🤖 Claude Code Review

Review: fulu builder flow (MEV-Boost), PR 704

Overall this is well structured. The bid checks, breaker, choice and publish guard are separate, testable units with good coverage. The decisions that diverge from the spec (fee recipient, boost-factor rounding) are documented. Comments below are in priority order.

Correctness / security

1. blinded_blocks.rs (the spawn_blocking check closure): advancing from an arbitrary held parent is still unbounded.

   • refuse_future_slot caps the target slot, but not the distance from the parent.
   • A caller can name any parent root the store still holds (an old finalized or pruned-late state) with a current slot. advance_to_slot then runs every epoch transition in between.
   • That cost is paid before the proposer signature is checked, so the request is unauthenticated.
   • post_block has the same shape (pre-existing), but this PR adds a new endpoint with it.
   • Fix: refuse a parent below the finalized slot, or cap slot - parent_state.slot() before advancing.

2. bid.rs (validate, signature check): the bid is verified against the pubkey it names itself.

   • Anyone controlling the endpoint, or a MITM on a plain-HTTP --builder, can re-sign a tampered bid with their own key and pass.
   • This matches Lighthouse (MEV-Boost owns the relay keys), so it is not a regression. However, the module doc and docs/rpc.md present "the builder's signature" as a meaningful check.
   • Either say it only guards integrity against a non-adversarial endpoint, or add an optional --builder-pubkey allowlist.

3. builder_client/mod.rs (error_text): only \n and \r are escaped.

   • Other control characters pass through unchanged: ESC/ANSI sequences, \x0b, \x0c, and U+2028/U+2029. These still allow terminal and log spoofing.
   • Suggest escaping every char::is_control (plus U+2028/2029). Alternatively, use text.escape_debug().collect::<String>() before truncating.

4. proposal.rs (refuse_future_slot): the 500 ms tolerance is tight for an API boundary.

   • Gossip's disparity bound is meant for peers. A VC/BN pair with more than 500 ms of skew, or a VC that requests slightly early, now misses proposals on every endpoint, gloas included.
   • The DoS concern only needs a coarse bound, e.g. "at most one slot ahead".
   • Also, this reads SystemTime::now() directly, while the rest of the crate and the tests use wall_slot(&store). Using the store's clock keeps one notion of "now".

5. proposal.rs (produce / choice::choose): builder_boost_factor=0 still calls getHeader and can serve a blinded block.

   • It does so when the local build fails.
   • Some VCs send 0 specifically to mean "builder disabled for this validator" (e.g. Lodestar's executiononly).
   • Consider skipping the builder request entirely at factor 0. That also saves the relay round-trip. If keeping the fallback is intentional, call it out in spec_deviations.md.

6. lib.rs (router layering): Extension(handles.builder) (Option<BuilderClient>) is no longer layered.

   • Any remaining handler that extracts Extension<Option<BuilderClient>> will now 500 at runtime with "missing extension"; the compiler will not catch it.
   • Please grep for other consumers, such as the status probe and gloas paths.

Robustness

7. blinded_blocks.rs (PublishedProposals): the guard is in-memory and per router instance.

   • A restart forgets every claim. That is acceptable, since the VC's slashing protection is authoritative, but worth a line in the doc comment.
   • Also make sure proposal::routes() is only ever instantiated once in the beacon router. A second instance would hold a separate map.

8. proposal.rs (blinded_block): the retry without packed contents also fires when the failure comes from the bid itself.

   • Examples are a bad header or bad requests. In that case it runs a second full state transition that is guaranteed to fail.
   • Minor, but it doubles the worst case.

9. proposal.rs (produce): up to four assemble_* state transitions can now run on a tokio worker.

   • Those are blinded, blinded retry, local and local retry.
   • The local path already did this before the PR, but the worst case has grown. Consider wrapping the selection and assembly loop in spawn_blocking.

Minor / nits

• breaker.rs: the threshold comparison is > (strictly more than). The docs say this consistently. Double-check it matches Lighthouse's head_skips semantics if "Lighthouse's defaults" is meant to imply identical behaviour.
• cache.rs (record): retaining over the whole map on every call is O(n) per registration request. Fine at current scale; just noting it.
• blinded_blocks.rs: the claim is taken before the spawned submission. A transport error that never reached the builder leaves the slot claimed, but a same-root retry passes. Good, and covered by a_claim_survives_a_failed_submission.
• choice.rs: the boosted split arithmetic is correct, and the exhaustive check against the exact product is a nice test.

---

Automated review by Claude · claude-opus-5-5 · custom prompt

[github-actions]

🤖 Grok Code Review

I'll inspect the builder-flow control paths and a few helpers the diff depends on before calling out concrete issues.The equivocation claim and the slot-advance guard look like the highest-risk spots. I'll confirm how those helpers behave before writing the review.

---

Automated review by Grok (xAI) · grok-4.7 · custom prompt

[github-actions]

🤖 Kimi Code Review

Review

No blocking correctness or security issues found. The bid validation, proposer-signature refactor, payload-selection fallback ordering, circuit-breaker arithmetic, and 202-only submission handling look sound. The same-slot claim is atomic, and the concurrent-publication and failed-submission tests cover the important races and equivocation cases well.

One non-blocking caveat:

• Anti-equivocation state is lost on restart — crates/net/rpc/src/beacon/blinded_blocks.rs:43-112. PublishedProposals is entirely in memory. A restart after submitBlindedBlockV2 succeeds but before the revealed block returns over gossip forgets the claim, allowing a different block for the same slot to be published. This is particularly relevant for blinded blocks because nothing is imported locally at submission time. If this is intended only as best-effort protection alongside validator-client slashing protection, documenting that limitation would be useful; otherwise, the claim needs persistence before contacting the builder.

Otherwise, the PR is unusually well documented and tested, including malformed bids, zero-value bids, shouldOverrideBuilder, builder failures, oversized bodies, unexpected 2xx responses, and future-slot refusal.

---

Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt

[github-actions]

🤖 DeepSeek Code Review

⚠️ Review failed: DeepSeek API request failed: curl exit code 92

---

Automated review by DeepSeek

[MegaRedHand]

Thanks for the second round. The changes are in 60c697a, with #696's latest merged in at 0d4dadf. I checked each point against Lighthouse, Prysm, Teku, Lodestar and Nimbus.

Changed

• The local build held up a valid bid (Codex). Once a bid has passed its checks, the local build gets 2 s from the request; past that, the bid is served through the existing fallback. Without a valid bid, the local build is still awaited, so a slow execution client and a relay answering 204 still produce a block. How the others handle it:

  • Lodestar races builder and engine with a cutoff at 1667 bps of the slot (2 s of 12, api/impl/validator/index.ts:129).
  • Lighthouse waits for both, but its getPayload times out at 2 s and isn't retried (engine_api/http.rs:52, engines.rs:352).
  • Tests: a silent execution client with a valid bid serves the blinded block; with no bid, a slow build is still waited for; a build inside the deadline still competes.

• engine_getPayload's timeout. Separately, it is now one attempt at 1 s, instead of the generic 8 s × 3 retries. execution-apis #958 dropped the method's timeout: an execution client must answer at once. A retry can't produce the slot's payload in time anyway.

  • The other clients: Prysm 1 s (300 ms from gloas), Nimbus 1 s, Lighthouse and Teku 2 s.
  • Test: a server that never answers fails V5 and V6 after one connection each.

• An unauthenticated publish could advance from an old parent (Claude). All three publish paths (post_block, the gloas one and blinded_blocks) now refuse two blocks before advancing any state:

  • a block not after the finalized slot;
  • a block whose parent cannot descend from the finalized checkpoint (parent at or below the finalized slot, and not the finalized block itself).

  These are gossip's own rules, which every client applies to blocks. A child of a finalized block that sits below its epoch's first slot still passes; a test pins that case. Each path has its own tests.

• Control characters in a builder's error text (Claude). Every control character, not just \n and \r, is written out before the 1 KiB cap: escape sequences, U+2028/U+2029, and tabs as \t. The cap never cuts an escape in two. The validator client's error text in feat(validator): propose builder blocks (fulu) #705 does the same.

• What the bid signature proves (Claude). The docs now say it: the signature is checked against the key the bid names, so it catches a corrupted bid. It doesn't stop the endpoint, or a man in the middle on a plain-HTTP --builder, from re-signing a changed bid. No client checks more:

  • Lighthouse (builder_bid.rs:196), Teku, Prysm and Nimbus verify against the bid's own pubkey;
  • Lodestar doesn't verify the bid signature at all.

Checked, not changed

• Extension<Option<BuilderClient>> no longer layered (Claude): nothing extracts it. Every handler takes Option<Extension<Arc<Builder>>>, which is what is layered.
• Retrying without packed contents when the bid is at fault (Claude): that case doesn't reach the retry. Every header field the state transition checks on the stand-in payload is checked earlier by bid::validate, against the same state. What can fail both attempts is the RANDAO reveal or the proposer, which fail a local block the same way. Telling them apart would mean matching assertion strings.
• The breaker's > (Claude): it matches Lighthouse's is_healthy_pre_gloas (beacon_chain.rs:7869-7891), which is healthy iff <= the threshold in all three checks, with the same quantities, the same window and the 3/8/3 defaults.
• The 500 ms future-slot bound (Claude): kept. It is the rule Nimbus and Lodestar apply to block production. A validator client asks once its slot has started, and the bound reads the same wall clock the gossip rules do.
• Asking for a bid at factor 0 (Claude): kept, as in the first round. beacon-APIs defines 0 as "prefer local unless an error makes it unviable". A client that wants no builder block (Lodestar's executiononly) refuses a blinded answer itself.
• The in-memory slot guard (Claude, Kimi): under Known limitations. It is not slashing protection.
• Assembly on the async runtime (Claude): a follow-up, under Known limitations.