[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
Syncing refusal	produceBlockV3 and produceBlockV4 answer 503 while the node is syncing, before the head is read, as the duties endpoints do. It does not bound the advance: a node more than 8 slots behind reads as synced (#559). produceBlockV3 advances the head on a blocking task, as produceBlockV4 already did
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. The head state's advance to the slot runs on a blocking task, as in produceBlockV4; moving assembly there too 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.

Deliberately not bounded

• 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, and we decided to leave it that way. It is not an equivocation path, only relay traffic from this node's address, and 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.

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.
• A syncing node refused on both production endpoints, without asking the builder for a bid.
• 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.

[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.

[pablodeymo]

Re-read at 699cb16. The five response commits do what the replies say: the forged-registration test, the six tests I asked for, the multiply-first boost factor, the escaped error text and the finality checks on the three publish paths are all there, and I found nothing wrong in them. Three things left, plus two notes.

1. Block production still advances from a stale head (proposal.rs:693, gloas_proposal.rs:686)

refuse_future_slot caps the target slot, and the finality checks cap how far back a published block's parent can be. produceBlockV3 and produceBlockV4 advance from the head instead, and nothing bounds the distance from the head to the wall clock or refuses while syncing. On a node catching up with the network (a restart after downtime, say), a local request for the current slot runs every epoch transition between the head and now: inline on the async runtime for V3, on a blocking thread for V4. That is the produce half of my point 1.

get_proposer_duties_v2, post_sync_duties and post_liveness already answer 503 on SyncStatus::Syncing (validator.rs:515, :111, :225). The same gate on the two production handlers, before the head is read, closes this for a node that is behind a live network, and a validator client that checks sync status before proposing loses nothing. I would not cap the distance instead: a stalled network reads as synced on purpose, and there the long advance is the work the proposal needs. That case is an argument for moving V3's advance onto a blocking task as V4's is, which is already your follow-up for assembly.

2. Point 3: follow-up, or dropped?

Your comment left the bound on in-flight blinded_blocks submissions as a post-merge follow-up. The description now says "we decided to leave it that way". Which one stands? If it is still a follow-up, an issue would keep it from getting lost.

3. engine_getPayload at one 1 s attempt reaches every proposal

This came in after my review and is wider than the builder flow: a fulu proposal with no builder and gloas's produceBlockV4 go through it too. execution-apis #958 is a good reason for it. But what #958 itself reports is that ethrex and reth held 20-30% of getPayload calls for 2-5 s under slow-block load on heze-devnet-2/3. Against a version of either that has not picked up its side of #958, each of those calls was a late payload under the 8 s ladder and is no payload now, so a missed slot unless a bid stands in. Is that the intended outcome for those versions? If it is, I'd say so in docs/beacon_engine.md, naming the two. The kurtosis battery will not show it either way: it runs geth, which #958 lists as staying under 0.5 s.

Notes, no change asked

• Recording a registration only once the builder accepts it closes my point 2 for a builder that answers an error. MEV-Boost answers 200 when at least one relay does (handleRegisterValidator in server/service.go), so behind it the forged registration is kept out only if every relay refuses it.
• During a long stretch without finality the publish paths' advance is bounded only by the distance to the finalized slot, which is the bound gossip has too.

[MegaRedHand]

Thanks for the third round. The change is in f4c72d2.

1. Production from a stale head: both done.

• produceBlockV3 and produceBlockV4 answer 503 while the node is syncing. The check comes after the 400 checks and before the head is read, as in the duties endpoints. Tests:
  • V3: a syncing node answers 503 without asking the builder for a bid;
  • V4: the refusal comes ahead of the no-execution-client 503, with its own message.
• V3 now reads the head and advances it on a blocking task, as V4's prepare does. That covers the stalled network you describe.
• One caveat: the gate doesn't catch a restart after downtime. The tracker takes the freshest block from LiveChain, which only imported blocks reach (insert_pending_block skips it). A node more than 8 slots behind therefore reads as a stalled network and reports Synced until it is back within 8 slots. That is Duty sync gate reports Synced while the node is far behind, so it attests and proposes on a stale head #559, and why Bound how far Beacon API requests can advance a state before validating them #701 says the 503 guards don't cover beacon catch-up. Until Duty sync gate reports Synced while the node is far behind, so it attests and proposes on a stale head #559 is fixed, the blocking task is what keeps that advance off the runtime. Once it is, both gates cover catch-up with no change here.
• The circuit breaker still reads the same sync status. With the gate in front, that only matters if the status flips mid-request. It stays because Lighthouse's is_healthy has it.

2. Dropped. We won't bound in-flight blinded_blocks submissions, since none of Lighthouse, Prysm, Teku, Nimbus or Lodestar does (table above). The description's "Deliberately not bounded" section is the current position. It replaces the "follow-up" in my earlier comment.

3. Intended. getPayload stays at one 1 s attempt, and the docs stay as they are. ethrex and reth have their side of ethereum/execution-apis#958 in flight (lambdaclass/ethrex#7460, paradigmxyz/reth#28165). The 2–5 s answers that PR reports came from slow-block load on a devnet, an edge case that doesn't happen in production.

Notes: both are right. MEV-Boost's handleRegisterValidator answers 200 as soon as any relay does. Neither note needs a change.

[pablodeymo]

/review

[github-actions]

🤖 Gemini Code Review

The implementation of the Fulu builder flow (MEV-Boost) looks solid and well-tested.

Key highlights:

• Registration & bid safety (registrations.rs, builder_flow/bid.rs, builder_flow/cache.rs): Recording validator registrations in BuilderRegistrations only after the builder accepts them prevents unverified/forged registration requests from poisoning the cached target gas limit used during bid validation.
• Equivocation & DoS guards (blinded_blocks.rs, proposal.rs, gloas_proposal.rs): Sharing PublishedProposals across /eth/v2/beacon/blocks and /eth/v2/beacon/blinded_blocks prevents publishing two different blocks for the same slot, and checking refuse_future_slot, refuse_finalized_slot, and refuse_parent_behind_finality before advance_to_slot bounds unauthenticated state-advance work.
• Execution engine & builder coordination (engine/src/client.rs, proposal.rs, builder_flow/choice.rs): Bounding engine_getPayloadV5/V6 to a single 1s attempt and racing the local build against LOCAL_BUILD_DEADLINE (2s) once a valid bid is available ensures a stalled EL cannot cause a missed slot, while choice::boosted avoids pre-scaling truncation and u128 overflow.
• Error sanitization (builder_client/mod.rs): error_text properly escapes control characters and Unicode line/paragraph separators without splitting escapes at the length cap.

No issues found — LGTM.

---

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

[github-actions]

🤖 Codex Code Review

• [P2] Expiration forgets the registration timestamp needed to reject stale replays — cache.rs:39–52. Once an entry expires, an older signed registration can be cached again after a successful forwarding response, even though the relay retains its newer registration and ignores the stale update. If their gas targets differ, bid::validate then rejects legitimate relay bids against the stale target, disabling builder proposals. Preserve the newest registration’s timestamp and contents independently of its eligibility expiry, and add a regression test covering expiration followed by an older registration.

Tests were not run.

---

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

[github-actions]

🤖 Claude Code Review

Review: fulu builder flow (MEV-Boost)

This is a careful PR. The bid checks, the boost-factor arithmetic, the shared equivocation guard and the submission running on its own task are well reasoned and well tested. Most of the findings below are behavioral edge cases rather than correctness bugs.

High

1. builder_boost_factor=0 still queries the builder and can serve a blinded block (proposal.rs, produce / choice::choose)
   • choose(None, Some(_), 0) returns Builder. The test "a bid paying nothing stands in for no local" uses factor 0 and asserts exactly that outcome.
   • The order loop also falls back to the bid when the local block fails to assemble, whatever the factor.
   • Several validator clients send 0 to mean "never use the builder for this validator". Lighthouse's VC does this when builder_proposals is off, and Lighthouse's BN does not call getHeader at all for Some(0).
   • A VC that opted out may then receive Eth-Execution-Payload-Blinded: true and have no blinded publish path for it.
   • Suggested fix: in produce, set request = None when boost_factor == 0. That skips the bid fetch and keeps the response unblinded.

Medium

2. CPU-bound work on the async runtime (proposal.rs, produce)

   • The head advance now runs in spawn_blocking, which is good.
   • Several other steps still run inline on a runtime worker:
     • accepted_bid: a BLS pairing.
     • pack: verified_sync_aggregate, another BLS verification.
     • Up to four state transitions: assemble_blinded_block and assemble_block, each with a retry without the packed contents.
   • This happened before for the local path. The builder path roughly doubles it, during the latency-critical window at the start of the slot.
   • Suggested fix: move bid acceptance plus the assemble loop into spawn_blocking, or at least the assembly.

3. PublishedProposals is created inside proposal::routes() (proposal.rs ~L132)

   • The guard is only as wide as one call to routes(). If the router is ever built twice (another merge, the test driver), two independent guards would exist and the protection silently disappears.
   • Suggested fix: construct it in start_beacon_rpc_server and layer it there, like FeeRecipients.
   • It is also memory-only, so a restart mid-slot forgets the claim. That is acceptable for best effort, but worth one line in docs/rpc.md so operators don't treat it as slashing protection.

4. The guard is keyed by slot only (blinded_blocks.rs, claim)

   • A node serving many validators can see two different proposers for the same slot on competing forks across an epoch boundary, since shuffling depends on the fork's RANDAO.
   • The second would be refused even though it is not an equivocation.
   • Keying on (slot, proposer_index) is closer to what is actually slashable. The failure mode is rare and conservative, so this is low urgency.

Low

5. A refused batch drops every registration's refresh (registrations.rs)

   • Registrations are recorded only when the builder accepts the whole call. One bad entry in a VC's batch (the builder 400s the call) means none of the valid ones are refreshed.
   • After REGISTRATION_LIFETIME_EPOCHS, those validators silently fall back to local builds.
   • That is a safe failure, but warn! should make it visible, perhaps naming that cached registrations will lapse.

6. Removed Extension(handles.builder) layer (lib.rs)

   • Any remaining handler that still extracts Extension<Option<BuilderClient>> will now fail at runtime with a missing-extension 500.
   • The builders.rs routes and the status probe are the places to check. Please confirm with a grep, or with the the_new_routes_are_wired_with_the_production_layers test covering the fulu routes too.

7. refuse_future_slot reads SystemTime directly (proposal.rs ~L466)

   • The rest of the module derives the wall slot from the store (node::wall_slot(&store)).
   • Two clock sources can drift in tests and future refactors. Consider deriving now_ms the same way wall_slot does.

8. Equal-timestamp registrations replace the held one (cache.rs, record)

   • registration.timestamp < entry.timestamp lets an equal timestamp replace the held entry. Since only builder-accepted registrations reach this point, this is harmless.
   • If the intent is "builders ignore older", the comment should say "older or equal is ignored", or the behavior should be left as is and documented.

Nits

• client.rs: "execution-apis #958 (merged 2026-10-26)". Please double-check that date.
• blinded_blocks.rs: the NOT_202 500 is good. Consider also surfacing status in the API message so the VC log carries it, not only the node log.
• breaker.rs: NoFinality reads the advanced state's finalized checkpoint, not fork choice's. That is documented and fine, but it diverges from Lighthouse (fork choice). One line in the module doc would help.
• produce: when the deadline drops the local build, the error is a 503 even if the bid then fails to assemble. Consider logging both causes together, so an operator sees "EL slow and bid failed STF" as a single event.

Looks good

• bid::validate ordering (cheap checks first, pairing last). The overflow-safe checked_add for the block number. The blob-gas check bounded by max_blobs before the multiply.
• boosted(): an exact multiply-first scaling without a wider type, and the deviation is documented.
• local_and_bid: the deadline applies only when a validated bid exists, so a builder never makes the local path worse.
• getPayload as a single attempt with a 1 s timeout, plus the wire-level test confirming one connection.
• error_text sanitization of control characters and U+2028/U+2029 before builder text reaches logs or VC responses, with escape-safe truncation.
• Registrations are remembered only after the builder accepts them, which closes the forged-gas-limit vector, and there is a test for it.
• The finality bounds (refuse_finalized_slot, refuse_parent_behind_finality) are applied before any state advance on all three publish paths, and the empty-epoch-start case is handled by comparing roots.

---

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

[github-actions]

🤖 DeepSeek Code Review

Overall this is a substantial and mostly well-tested PR. The builder-flow logic is carefully documented and the security review of builder trust vs relay is good. A few points worth addressing:

1. PublishedProposals::claim can accept a block for a slot that became finalized during validation.
   In crates/net/rpc/src/beacon/blinded_blocks.rs, claim rejects only slot < finalized_slot, while the publish-path checks in proposal.rs (refuse_finalized_slot) reject slot <= finalized_slot. The early refuse_finalized_slot runs before signature/state validation, but claim later recomputes finalized_slot(&store). If finality advances while the request is being processed, a slot that passed the early check may now equal finalized_slot, and claim will accept it.
   Suggest re-checking refuse_finalized_slot immediately before claim, or change claim to reject slot <= finalized_slot.

2. post_block performs expensive sidecar construction before the equivocation guard.
   In proposal.rs, fulu post_block validates the proposer signature and builds/verify sidecars, then calls published.claim. A proposer that has already signed one block can send a second valid signed block for the same slot and cause the node to repeat the expensive sidecar work before the second block is rejected as a duplicate.
   Consider claiming immediately after signature verification but before sidecar building, since the claim only forbids a different root and already-verified signatures imply the proposer cannot later send a legitimate different block for that slot without equivocating.

3. pack runs BLS verification on the async runtime.
   In proposal.rs, produce calls pack, which invokes pack_attestations, pack_operations, and verified_sync_aggregate synchronously. Especially verified_sync_aggregate performs BLS verification and can block a Tokio worker.
   Move pack into the existing spawn_blocking block or wrap it in tokio::task::spawn_blocking before the builder/local selection.

4. get_block still refuses to use the builder when no execution client is configured.
   get_block returns 503 no execution client configured to build a payload with before entering produce. But produce itself is designed to fall back to a valid builder bid when the local payload fails (without_a_local_payload_the_bid_is_served test uses a dead engine, not None).
   If the intended behavior is that --builder without --execution-endpoint may still serve blinded builder blocks, the early engine check should be relaxed or made conditional. If it is intentional, it is worth documenting because it looks inconsistent with the local-failure fallback path.

5. The rest of the changes look good:

   • error_text escaping control characters before logging builder error bodies is a solid hardening.
   • The builder bid validation, circuit breaker, registration cache, and shouldOverrideBuilder parsing are clear and well-tested.
   • The verify_proposer_signature refactor is correct for the blinded-block root property.
   • engine_getPayload single-attempt behavior matches the updated engine spec and is well justified.

---

Automated review by DeepSeek · deepseek-v4-pro · custom prompt

[pablodeymo]

Agreed on all three.

1. f4c72d2 does it: both production endpoints refuse while syncing, and V3's advance is off the runtime. You are right about the caveat: with #559 open the gate does not cover a node far behind, as #701 already says, so what is left of the unbounded advance stays with those two.

2. Accepted as dropped. The description's "Deliberately not bounded" section is the record.

3. Accepted as it is, with the ethrex and reth fixes you linked still open on their side.

[github-actions]

🤖 Kimi Code Review

The builder flow looks well thought out overall: bid validation, registration caching, equivocation claims, and the special handling of non-202 2xx responses are covered by strong tests. Two concerns remain:

• Medium — unauthenticated requests can still trigger arbitrarily expensive state advances. In crates/net/rpc/src/beacon/proposal.rs around refuse_parent_behind_finality (lines 502–518) and produce (lines 699–720), and similarly in blinded_blocks.rs around lines 158–190, the destination slot is clock-bounded but the distance from the parent/head is not. During prolonged non-finality or a stalled network, requests can repeatedly advance an old held state to the current slot before any signature authentication. Moving production advances to spawn_blocking also allows concurrent requests to consume the blocking pool. The comment near get_block explicitly notes that the syncing check does not cover this stalled-network case. Consider bounding concurrent advances and caching/sharing work per (parent_root, slot); a distance limit could also help, provided its liveness implications during non-finality are considered.

• Low — the new timeout test has a fragile wall-clock upper bound. crates/net/engine/tests/wire_smoke.rs, around lines 277–280, requires elapsed time to remain below 2.5 seconds. A loaded CI runner can exceed that despite correctly making one attempt. The exact timeout error and single-connection assertion already demonstrate the intended behavior; consider removing the upper elapsed-time assertion or using controlled time.

No blocking fork-choice, signature-verification, or bid-selection correctness issues stood out in the diff.

---

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

[github-actions]

🤖 Grok Code Review

The builder flow is carefully structured, and the registration-only-after-accept path, same-root publish retry, and multiply-first boost math all look right. A few consensus-safety and liveness issues are worth fixing before this ships.

1. Slot claim is per slot, not per proposer (PublishedProposals::claim in crates/net/rpc/src/beacon/blinded_blocks.rs, used from both publish paths in proposal.rs). Equivocation is per validator. This map rejects any second root for the slot, including a block from a different proposer_index on a side fork whose parent this node still holds. Publish is unauthenticated and does not require the fork-choice head, so a valid side-fork block posted first occupies the slot and the canonical proposer's publishBlock / blinded_blocks gets 400. Key the claim by (slot, proposer_index), and do not let a non-head parent suppress the head proposal.

2. The claim is memory-only and is taken before the block is actually broadcast. A restart (or a second process behind the same VC) drops it, which is the case this guard exists for. It is also taken before the builder ack and before import, and it is not rolled back on a definite failure. Same-root retry is correct; losing the entry on restart is not. Persist it, or treat it as best-effort and say so next to the type.

3. Publish still does unbounded work before the signature check when finality lags. refuse_parent_behind_finality only decides parents at or below the finalized epoch start. A parent just above that slot skips the ancestry check, then advance_to_slot runs up to the wall clock before verify_proposer_signature. With finality stuck, an unauthenticated body can pin a blocking thread for many epochs. Cap slot - parent_slot before the advance.

4. Bid signature verification runs on the async worker. accepted_bid → bid::validate does the pairing inside the local_and_bid future (proposal.rs), so it stalls that task, including the in-flight local build. Publish already uses spawn_blocking for the same work; this path should too.

5. LOCAL_BUILD_DEADLINE drops the local payload before the blinded block is known to assemble. In local_and_bid, a validated bid past 2s turns the local future into ServiceUnavailable and cancels the engine call. If assemble_blinded_block then fails, there is no local block left and the slot is missed. Keep the local build running until the blinded block is actually produced.

6. excess_blob_gas and base_fee_per_gas are not in bid::validate. Parent hash, randao, timestamp, block number, withdrawals, blob gas, and gas limit are. Those two still fail closed in assemble_blinded_block, but only after the bid has already won the comparison and the local payload may already have been abandoned (Point 5). Check them in validate, next to the other header fields.

7. Log sanitizer misses bidi overrides. error_text escapes is_control plus U+2028/U+2029, which is the right idea, but U+202E and the other Cf bidi controls are not controls and can still reorder a log line or a VC error string. Escape the bidi set as well.

Smaller notes:

• get_block still 503s when no execution client is configured, before the builder is asked, so a builder-only process cannot propose even with a valid bid. That matches the docs; it is a sharp edge next to the "failed local build falls back to the bid" path.
• The circuit-breaker Syncing trip is effectively dead on produceBlockV3: the handler already returns 503, so lean_beacon_builder_fallback_total{reason="syncing"} will not move.
• claim treats slot == finalized_slot as live, while refuse_finalized_slot rejects slot <= finalized_slot. Safe today only because every caller checks first. Use the same bound in claim.
• get_payload_gives_up_after_one_attempt_at_its_own_timeout can flake on a slow CI box: the connection count is the real assertion, the elapsed < retry_floor check is not.

The circuit-breaker window, zero-bid handling, shouldOverrideBuilder default, and "remember a registration only after the builder accepts it" tests are in good shape.

---

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

[MegaRedHand]

/review

[github-actions]

🤖 DeepSeek Code Review

⚠️ Review failed: DeepSeek returned an empty content field (finish_reason=stop, reasoning_content present=yes). Raise max_tokens or lower reasoning_effort ('none' disables thinking).

---

Automated review by DeepSeek

[github-actions]

🤖 Codex Code Review

• [P2] Scope publication claims to (slot, proposer_index) — blinded_blocks.rs:45. PublishedProposals keys claims only by slot, so publishing a valid block from one fork prevents a different validator from publishing for that slot on another fork. Proposer assignments can differ between branches, and these blocks are not a slashable double proposal because their proposer indices differ. Key claims by both slot and proposer index, and test that different proposers can publish while conflicting roots from the same proposer remain rejected.

---

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

[github-actions]

🤖 Claude Code Review

Review: fulu builder flow (MEV-Boost), getPayload timeout, publish guards

This is a well-structured PR. The decision logic is split into small, testable modules (bid, breaker, cache, choice) with thorough tests. Recording a registration only after the builder accepts it is a good defense, and so is the shared equivocation guard between blocks and blinded_blocks.

Correctness and risk

1. The getPayload change affects every local proposal, not just the builder path (engine/src/client.rs, ENGINE_GET_PAYLOAD_TIMEOUT).

   • Before: up to 3 attempts at 8 s each. Now: a single attempt at 1 s, for V5 and V6.
   • This applies to every node, including those without --builder.
   • A loaded EL that answers in 1.2 s now turns a late block into a missed one, and with no bid there is no fallback.
   • The doc notes that Lighthouse and Teku use 2 s. Consider 2 s, or mention this behaviour change in the PR description and changelog.
   • The "merged 2026-10-26" date for execution-apis #958 also looks wrong; please verify it.

2. Removing .layer(Extension(handles.builder)) can break other handlers at runtime (rpc/src/lib.rs).

   • Any remaining handler that extracts Extension<Option<BuilderClient>> (for example a status or probe endpoint, or the test driver) will now fail at request time with a 500 instead of failing to compile.
   • Please grep for Extension<Option<BuilderClient>> and confirm nothing else uses it.

3. get_block and gloas post_produce_block now require Extension<SyncStatusController>.

   • It is a required (non-Option) extractor.
   • Every router that mounts these routes must layer it, including the hive/test-driver routers if they do. Otherwise production of blocks returns 500.

4. refuse_future_slot reads SystemTime::now() directly (proposal.rs).

   • Elsewhere the code uses the store's clock (super::node::wall_slot(&store) in registrations.rs).
   • Mixing the two clock sources makes the checks inconsistent, and makes the tests depend on fixtures happening to align the pinned clock with real time.
   • Prefer deriving now_ms from the store's time source.

5. CPU-bound work still runs on the async runtime (proposal.rs::produce).

   • The head advance was moved to spawn_blocking, but these were not:
     • pack (verifies the sync aggregate with BLS),
     • accepted_bid (a BLS pairing inside the bid future, polled within tokio::join!),
     • local_block,
     • blinded_block (a full process_block each, and up to twice per path on retry).
   • With the builder path this can be up to four state transitions inline. Consider moving pack and the assembly loop into a single spawn_blocking.

6. The finalized-slot boundary differs between two checks (blinded_blocks.rs, PublishedProposals::claim).

   • claim refuses slot < finalized_slot, while refuse_finalized_slot refuses <=.
   • This is harmless, because the stricter check runs first, but the Finalized arm is only reachable through a finality race.
   • A one-line comment, or aligning the two to <=, would avoid confusion.

7. The publish guard relies on the stored finalized root matching the parent root a VC names (refuse_parent_behind_finality).

   • The test fixtures needed finalize_at because the anchor's stored root differed from the header root.
   • Please confirm that after a real checkpoint sync, beacon_finalized_checkpoint().root is the block root VCs will name as parent_root. Otherwise every publish is refused until the next finalization.

Security

8. error_text does not escape bidi/format characters (builder_client/mod.rs).

   • It escapes Cc characters plus U+2028/U+2029, but not Cf characters such as U+202A–U+202E or U+2066–U+2069.
   • Those can visually reorder log text ("Trojan Source"-style spoofing).
   • Since the stated goal is that builder text cannot forge or restyle log lines, consider escaping them too.

9. PublishedProposals is in-memory only. That is acceptable, since VC slashing protection is the real guard. It is worth one line in the doc saying it does not survive a restart.

Minor

10. Some tests depend on wall-clock timing and may flake on loaded CI runners:

    • get_payload_gives_up_after_one_attempt_at_its_own_timeout asserts elapsed < 2.5 s.
    • The deadline tests (a_silent_execution_client_gives_way…, without_a_bid_a_slow_local_build…) each take 2–2.5 s of real time.

    Consider tokio::time::pause where the engine client permits it.

11. In produce, when the deadline drops the local build and the blinded block then also fails, the client receives the 503 "did not return a payload in time". It would be clearer to surface the builder failure, or both failures.

12. choice::boosted and the the_bid_is_scaled_before_it_is_rounded test are correct, and the deviation is well documented.

Other parts look correct

• The breaker logic, including the window, the wrap handling and the head-skips computation.
• Bid validation ordering, with the signature check last.
• Registration lifetime handling.
• Moving the blinded_blocks submission onto its own spawned task.

---

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

[github-actions]

🤖 Gemini Code Review

This PR looks great — the Fulu builder flow (produceBlockV3, POST /eth/v2/beacon/blinded_blocks, bid validation, circuit breaker, boost-factor selection, and registration caching) is well-structured and thoroughly tested.

A few highlights that stood out positively:

• Equivocation protection (crates/net/rpc/src/beacon/blinded_blocks.rs, proposal.rs): Sharing PublishedProposals across beacon/blocks (Fulu and Gloas) and beacon/blinded_blocks, allowing idempotent retries of the same block root while refusing a second distinct root for the same slot, and running submit_blinded_block in a detached task so a client disconnect cannot drop the reveal or its outcome log.
• Pre-advance DoS bounds (crates/net/rpc/src/beacon/proposal.rs, gloas_proposal.rs): Checking refuse_future_slot, refuse_finalized_slot, and refuse_parent_behind_finality before loading and advancing parent/head states prevents unauthenticated callers from triggering unbounded slot/epoch transitions.
• Registration cache poisoning defense (crates/net/rpc/src/beacon/registrations.rs:183-194): Recording validator registrations in BuilderRegistrations only after the builder accepts them ensures an unverified registration with a forged gas_limit cannot overwrite a validator's cached target and cause subsequent valid bids to be rejected.
• Log/terminal injection sanitization (crates/net/rpc/src/beacon/builder_client/mod.rs:484-501): error_text escaping control characters and Unicode line/paragraph separators while respecting MAX_ERROR_TEXT without splitting escape sequences.

No issues found; good to merge.

---

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

[github-actions]

🤖 Grok Code Review

The builder flow is carefully structured: bid checks before any state transition, registrations stored only after the builder accepts them, the boost-factor math is exact, and the shared publish claim correctly treats a same-root retry as safe. A few validator-safety gaps are still worth fixing.

1. Equivocation guard is memory-only. PublishedProposals in blinded_blocks.rs lives in a process-local Mutex and is pruned only by finality. A restart after submitBlindedBlockV2 has been sent (or after a full block was gossiped) but before the validator client sees 200 lets a retry produce and publish a different block for that slot. Persist (slot, root) before the submit/gossip, and reload it on startup.

2. produceBlockV3 is not idempotent per slot. produce in proposal.rs does not cache the unsigned block. A validator-client timeout retry can get a local block the first time and a blinded block the second (bid arrives, boost factor, shouldOverrideBuilder, breaker). The client can sign both; the claim in post_block / post_blinded_block only helps if both publishes hit this process after the first claim. Cache the Produced result for the slot and return that exact block on retry until the slot is claimed or finalized.

3. Local builds ignore the registered gas limit. builder_request uses registration.gas_limit only to validate the bid. build_payload is still called with fee_recipients only, so a breaker trip or a local win does not apply the gas limit the validator registered. Feed that gas limit (and the registered fee recipient, if FeeRecipients was not updated from this registration) into the local payload attributes.

4. getPayload now fails the whole proposal at 1s. ENGINE_GET_PAYLOAD_TIMEOUT replaces an 8s × 3 ladder for every local build, not just the builder path. A payload that needs ~1.5s (blobs, cell proofs) becomes a missed slot. Lighthouse and Teku use 2s, which also matches LOCAL_BUILD_DEADLINE. Prefer 2s unless you have EL timings that justify 1s.

5. Finality check stops at the parent. refuse_parent_behind_finality only rejects a parent at or below the finalized slot. A parent above that slot whose chain does not include the finalized block is still advanced, signed against, and gossiped. The advance is bounded, but this is short of gossip’s finalized-ancestor REJECT. Walk parents back to the finalized slot (the walk is the same bound) before spawn_blocking.

Smaller notes:

• local_and_bid always waits out the header timeout when the local payload finishes first. That is right for a bid that might win, but a dead builder then adds a full timeout to every registered proposal. Fine if --builder-header-timeout stays at 1s; worth a metric if it is raised.
• accepted_bid runs the pairing on the async worker. Move that bls::verify into spawn_blocking with the state advance.
• claim allows slot == finalized_slot while refuse_finalized_slot uses <=. Endpoints never hit the mismatch; align claim with <= so a direct caller cannot record a slot the API will not publish.
• blobs * GAS_PER_BLOB in bid.rs should be checked_mul so a hostile list cannot wrap in release builds.
• Add a test that hash_tree_root(BlindedBeaconBlock) == hash_tree_root(full block) for the same body. Publish verifies the blinded root; gossip will verify the revealed block. Those roots must be identical or every builder block dies on import.

---

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

[github-actions]

🤖 Kimi Code Review

Overall, the builder flow is carefully implemented: bid validation, proposer-signature verification, boost-factor arithmetic, and the shared equivocation guard all look sound. I found three concerns:

1. Medium — The new finality checks do not fully bound unauthenticated state-advance work.
   refuse_parent_behind_finality in crates/net/rpc/src/beacon/proposal.rs (around lines 493–512) only rejects an old parent when parent_slot <= finalized_slot. A known parent at finalized_slot + 1 therefore passes even when the requested block is thousands of epochs later. Both publish paths then advance that parent state before checking the proposer or signature (proposal.rs post_block; blinded_blocks.rs:151–190). Repeated requests with invalid signatures can consequently consume substantial CPU and, now that production also uses spawn_blocking, accumulate blocking-pool tasks. Consider an explicit parent-to-block distance/resource bound and a concurrency limit for these advances; the current comments suggesting the work is bounded are stronger than the implementation.

2. Medium/behavioral — A configured builder cannot serve a block when no execution client is configured.
   In proposal.rs get_block (approximately lines 558–565), engine == None returns 503 before produce or builder_request runs. Thus an unreachable engine can fall back to a bid, but an unconfigured engine cannot, even though all bid checks and blinded-block assembly are otherwise independent of a local payload. If builder-only proposals are intended, make the local build optional and still fetch the bid; otherwise document that --builder also requires --execution-endpoint.

3. Low — Registration acceptance and visibility to block production are racy.
   registrations.rs (around lines 177–198) spawns forwarding and returns success without waiting for the builder’s response or the subsequent registrations.record. A validator client can receive 200 and immediately request a proposal while the cache is still empty, silently causing a local proposal. This is most relevant for a proposer registering shortly before its slot. Consider awaiting acceptance/recording before responding, or explicitly tracking in-flight registrations for production.

The extensive tests, sanitized builder errors, no-retry getPayload handling, and no-local-fallback behavior after blinded submission are particularly good.

---

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