[MegaRedHand]

First gloas step of the builder work: the validator client publishes signed proposer preferences (fee recipient, target gas limit) for its gloas proposals. Stacked on #705.

Why: from gloas, beacon-APIs makes SignedProposerPreferences the source of a proposer's fee recipient and gas limit, superseding prepare_beacon_proposer. A proposer without preferences also gets no trustless bids: p2p bids for its slot are ignored.

What it adds

Piece	Behavior
Publishing	Every slot, for the current and next epoch that are gloas epochs (so warm-up starts one epoch before the fork). Fetches v2 proposer duties, whose dependent_root is the shuffling dependent root the preferences must carry; the duty loop keeps v1. Keeps only duties inside the target epoch, one per slot, so a node's answer cannot make it sign more than an epoch's worth. Signs each own duty whose slot has not started and which not every node holds yet for that (slot, dependent_root). Submits one preference per request to every node at once, so one refusal does not hold back the others. A preference only some nodes accepted is re-sent, as first signed, until every node holds it. A reorg that moves the dependent root re-publishes
Unconditional	Not gated on --builder-proposals, as in Lighthouse: from gloas the preferences carry the fee recipient for every proposal. Validators with no fee recipient are skipped, with one warning per epoch
Signing domain	DOMAIN_PROPOSER_PREFERENCES at the fork version of the proposal epoch, as consensus-specs v1.7.0-beta.4 signs and verifies it, and as Lighthouse, Prysm, Teku, Lodestar and Nimbus do. ethlambda's beacon node also still accepts beta.2's domain (the fork of the epoch before the proposal's), since our fixtures are pinned to beta.2
Eth-Consensus-Version	The target epoch's fork (gloas)
Gloas boost factor	With --builder-proposals, produceBlockV4's body carries the configured builder_boost_factor, so pooled p2p bids can win on value. Without it the factor stays 0
Metrics	ethlambda_validator_proposer_preferences_published_total, ..._failures_total (preferences a node refused or did not take), ..._duty_fetch_failures_total (failed v2 duty fetches). A refusal is warned once per preference, then logged at debug

Known limitations

• A preference cannot be replaced for the same slot and dependent root, so a fee recipient changed after publishing applies from the next dependent root on.

Tests

• Unit: the v2 path; per-entry submission; refusal isolation; reorg re-publish; the pre-gloas gate; started slots; missing fee recipients; single-flight polling; the fork-version choice on both sides of the boundary; the boost factor in the gloas body.
• End to end through ethlambda's own beacon node: preferences signed by the validator client from its own v2 duties pass the node's gossip rules and are gossiped.

make lint passes, as do the validator lib, rpc lib and binary tests.

Devnet results

Kurtosis (ethereum-package) with this branch's head (18a907cf), Lighthouse and Geth glamsterdam-devnet-8, and buildoor as the builder. Fulu from genesis, gloas at epoch 6. Three nodes: Lighthouse alone; ethlambda's validator client on a Lighthouse beacon node (--builder-proposals --prefer-builder-proposals, 40% of stake); ethlambda for both (--builder-proposals). All three stayed on one head through the fork.

Works

• v2 duties: 0 fetch failures, against both ethlambda's and Lighthouse's beacon node.
• Publishing: from the warm-up epoch on, once per (slot, dependent_root), 0 failures through ethlambda's beacon node, which validated, pooled and gossiped every preference.
• Lighthouse accepts our preferences from the second gloas epoch on: buildoor, which follows a Lighthouse node, cached all 32 slots of epochs 7 and 8, ours included.
• Fee recipient timing is exactly as documented: changed through the keymanager in epoch 9, epoch 10's preferences and blocks kept the old address, and epoch 11's carried and paid the new one. The pre-sign check logs an ERROR for every block in between (20 here), and its hint ("reaches the node only at the next one") is wrong at gloas, where it can take two epochs.
• Metrics move as documented.
• The builder stack underneath (feat(validator): propose builder blocks (fulu) #705 and below) works in fulu: registrations, filtering, blinded blocks through both ethlambda's and Lighthouse's beacon node, the bid gas-limit check, boost factor 0, and the circuit breaker.

Fails at the fork boundary

1. Signing domain (fixed since: the client now signs at the proposal epoch's fork version, as beta.4 and every client do). Lighthouse refused all 16 epoch-6 preferences from the validator client with BadSignature, then accepted the same client's epoch-7 and epoch-8 ones. The client retried each refused preference every slot until its slot started (749 failures).
2. Warm-up preferences are gossiped to nobody. ethlambda's beacon node joins the gloas topics an epoch before the fork and gossips the warm-up preferences at once, but Lighthouse subscribes only 2 slots before the fork. They are never re-sent, so no Lighthouse node, and no builder, saw them. The node still logs Published proposer preferences to gossipsub at info, because the publish error is logged only at debug (swarm_adapter.rs). Possible fixes: re-send the pooled preferences at the fork, or publish the warm-up epoch's preferences closer to it.

Together, on a network with Lighthouse, none of our validators had a preference for the first gloas epoch.

Not tested

• Gloas bid weighing (builder_boost_factor in produceBlockV4): buildoor's builder was never onboarded (the builder registry was empty on both clients), so there were no bids and every gloas block was self-built. Both beacon nodes accepted the request body.
• Preferences without --builder-proposals, the reorg re-publish, and validators with no fee recipient.
• Whether ethlambda's beacon node accepts the other signing version: Lighthouse's epoch-6 preferences never reached it.

Found along the way, not from this PR (same on main)

• Finality stopped at the fork. At gloas the validator client omits committee_index from attestation_data and sends payload_attestation_data?slot=, as beacon-APIs master says. Lighthouse glamsterdam-devnet-8 requires committee_index and serves /payload_attestation_data/{slot}, answering 400 to both. The client on Lighthouse missed every attestation from the fork on: target participation fell from 97% to 58%, and justification stopped.
• The validator client has no --gas-limit: it registers (and from gloas prefers) 60M unless set per validator through the keymanager. On a 200M chain every builder bid failed the gas-limit check.
• ethlambda's local payloads are empty (getPayload right after forkchoiceUpdated), so any bid beats a local build, and boost factors above 0 cannot be told apart.
• Lighthouse glamsterdam-devnet-8's gloas blocks pay the zero address, which our client flags as a fee recipient mismatch.

https://claude.ai/code/session_011vghNjfTQmZNRCjPkRs1wP

[github-actions]

🤖 Claude Code Review

Review of PR 707: validator client publishes signed proposer preferences

I read the diff and the checked-out preferences.rs, signing.rs and the fork-choice helper. I did not build the PR or run its tests. The change looks correct and well tested, with only minor points.

Correctness

• The DependentRoot::V2 refactor in rpc/src/beacon/validator.rs is behavior-preserving. compute_shuffling_dependent_slot is start_slot(epoch.saturating_sub(MIN_SEED_LOOKAHEAD)).saturating_sub(1), which matches the old inline code when MIN_SEED_LOOKAHEAD is 1. Sharing it with the state transition also removes a drift risk.
• sign_proposer_preferences signs at the fork version of proposal_epoch - MIN_SEED_LOOKAHEAD, matching the gossip rule. Both sides of the fork boundary are tested. The PR documents the one case where this differs from the validator guide.
• publish_epoch skips started slots, sends one preference per request, and remembers each success by (slot, dependent_root). Pruning is keyed correctly. Poisoned mutexes are recovered, and InFlightGuard clears the flag on panic.
• The mock now bumps proposer_duties_v2_calls only after guard(), so failed calls are not counted. The mock's doc comments say the same.

Minor points

1. preferences.rs:269-310: each preference is sent in turn, and each send goes to every node. A client with many keys and a slow node could take a while per poll. The in_flight guard keeps polls from stacking, but a poll that overruns skips the next slot's poll. That could cost preferences for the slots after it, which gossip IGNOREs once they have started. A handful per epoch is fine, so this only matters at high key counts. Sending with bounded concurrency (join_all or buffer_unordered) would remove the risk cheaply.
2. preferences.rs:183-198: a failed duties fetch is counted and logged at debug!, and also warns once per epoch. That is intentional, but fetch_warned is pruned with retain(>= epoch), so a node that stays down warns once per target epoch. That is acceptable.
3. published is keyed on (slot, dependent_root) only. A fee recipient or gas limit changed later is not re-sent for the same root, which the PR documents under "No replacement". Fine.
4. http.rs: builder_config formats the JSON by hand. That is safe because the factor is a u64, but serde_json::json! would be more idiomatic. This is a nit.
5. lib.rs: the spawn of preferences.poll(...) is a detached task, so panics are swallowed apart from the guard reset. The guard makes this safe.

Tests
Coverage is thorough: v2 path, per-entry submission, refusal isolation, reorg re-publish, the pre-gloas gate, started slots, missing fee recipients, single-flight, fork-version choice, the boost factor on the wire, and an end-to-end run against ethlambda's own beacon node. The Lighthouse fork-boundary interop is flagged as unverified, which is honest.

No blocking issues.

---

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

[github-actions]

🤖 Codex Code Review

I reviewed the PR diff and the touched Rust paths in context. Overall this looks careful and the added tests materially improve confidence around proposer-preferences signing and v2 duties. I did not find a clear consensus-breaking bug in the new logic.

• No blocking correctness/security issue stands out in the added proposer-preferences flow.
• The main behavior change — signing DOMAIN_PROPOSER_PREFERENCES using the epoch before the proposal and sourcing dependent_root from v2 proposer duties — is well-motivated and covered by tests in crates/validator/src/signing.rs:335, crates/validator/src/preferences.rs:183, and crates/net/rpc/src/beacon/builder_market_tests.rs:1065.

Findings

• Minor robustness concern: PreferencesService keeps published, refusal_warned, no_fee_warned, and duty_fetch_warned behind std::sync::Mutex in async code (crates/validator/src/preferences.rs:99). If any of those locks are ever held across future .await refactors, this becomes a latent footgun. It looks safe today, but I’d consider tokio::sync primitives or extra care/comments around lock scope for maintainability.
• Potential operator-confusion/API concern: the CLI help now says --builder-proposals “boost factor weighs p2p bids in produceBlockV4” even though the option still sounds boolean and gloas publishing is now unconditional (bin/ethlambda/src/validator.rs:80). The implementation seems intentional, but the UX semantics have shifted enough that this may surprise operators. Not a correctness bug, but worth ensuring docs/help text make it crystal clear that on gloas this flag only influences boost factor, not whether proposer preferences are published.
• Test gap, low severity: the end-to-end test explicitly notes it does not pin the fork-boundary version choice (crates/net/rpc/src/beacon/builder_market_tests.rs:1072). Since fork-boundary domain mismatches are exactly the kind of subtle consensus-adjacent bug that can reappear later, I’d keep an eye on whether the unit coverage in crates/validator/src/signing.rs:335 is enough, or add an integration-style boundary case if practical.

What looks good

• Correct use of v2 proposer duties for dependent_root, which matches the gossip validation contract and avoids a likely source of silent rejection (crates/validator/src/beacon_node/http.rs:642, crates/validator/src/preferences.rs:183).
• Good defensive behavior around blinded responses when builder proposals are off; refusing a blinded block in that mode avoids signing something this client cannot publish (crates/validator/src/beacon_node/http.rs:668).
• Sensible deduplication semantics keyed by (proposal_slot, dependent_root), which handles reorgs without suppressing valid resubmission (crates/validator/src/preferences.rs:99).
• Strong test coverage for HTTP encoding/decoding, gossip acceptance, duplicate handling, and signature-domain behavior.

Looks good to me aside from the minor maintainability/UX notes above.

---

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

[github-actions]

🤖 Kimi Code Review

I'll review this PR which implements proposer preferences for the Gloas fork. Let me analyze the changes systematically.

Overall Assessment

This is a well-structured PR adding Gloas fork proposer preferences support. The core logic is sound, but I've identified several issues ranging from minor to potentially serious.

---

Critical Issues

1. Potential Division by Zero in builder_boost_factor Documentation vs. Implementation

File: crates/validator/src/beacon_node/http.rs, lines 88-96

The documentation states 0 prefers local payload, but the JSON sends "0" as a string. The spec's produceBlockV4 expects this to be a number, not a string. However, looking more carefully, the format string produces "0" (quoted string), not 0 (number).

fn builder_config(boost_factor: Option<u64>) -> String {
    let factor = boost_factor.unwrap_or(0);
    format!(r#"{{"min_bid":"0","builder_boost_factor":"{factor}","builders":[]}}"#)
}

Issue: builder_boost_factor is serialized as a string ("0", "250"), but the Beacon API spec defines it as a number (uint64). This will cause spec-compliant beacon nodes to reject the request or parse it incorrectly.

Fix: Remove quotes around {factor}:

format!(r#"{{"min_bid":"0","builder_boost_factor":{factor},"builders":[]}}"#)

Similarly, verify whether min_bid should be a string or number per spec.

---

Security Issues

2. Missing Input Validation on builder_boost_factor

File: crates/validator/src/beacon_node/http.rs, line 93

boost_factor is passed directly from CLI/config into JSON without bounds checking. While u64 is inherently bounded, consider whether extremely large values (e.g., u64::MAX) could cause issues in downstream parsing.

Mitigation: The current code is probably safe, but consider capping or documenting maximum sensible values.

---

Correctness Issues

3. Fork Version for Signing Domain at Fork Boundary

File: crates/validator/src/signing.rs, lines 335-342

let proposal_epoch = compute_epoch_at_slot(preferences.proposal_slot);
let domain = self.domain(
    DOMAIN_PROPOSER_PREFERENCES,
    proposal_epoch.saturating_sub(preset::MIN_SEED_LOOKAHEAD),
);

Issue: MIN_SEED_LOOKAHEAD is 4 epochs (mainnet), not 1. This subtracts 4 epochs, not 1.

Wait—let me re-check. The comment says "the epoch before the proposal's" but the code subtracts MIN_SEED_LOOKAHEAD (4).

Looking at the test at line 689:

let preferences = preferences_for(gloas_epoch * preset::SLOTS_PER_EPOCH + 2);

And the test verifies it signs with fulu_fork_version when proposal_epoch == GLOAS_EPOCH. With MIN_SEED_LOOKAHEAD = 4, GLOAS_EPOCH - 4 is still Fulu (assuming Fulu ends at GLOAS_EPOCH - 1).

But what if proposal_epoch == GLOAS_EPOCH + 3? Then proposal_epoch - 4 = GLOAS_EPOCH - 1, still Fulu. Only at GLOAS_EPOCH + 4 does it become Gloas.

This contradicts the comment which says "the epoch before the proposal's." The spec's validate_proposer_preferences_gossip uses the fork at the dependent block's epoch, which for v2 duties is compute_shuffling_dependent_slot(epoch) — that's start_slot(epoch) - 1, i.e., the last slot of the previous epoch, which falls in epoch - 1.

So the correct fork epoch should be proposal_epoch - 1, not proposal_epoch - MIN_SEED_LOOKAHEAD.

File: crates/validator/src/signing.rs, line 339

proposal_epoch.saturating_sub(preset::MIN_SEED_LOOKAHEAD),

Fix: This should be proposal_epoch.saturating_sub(1) to match the actual gossip rule domain.

However, looking at the test a_preference_for_the_first_gloas_epoch_is_signed_under_the_fulu_version — it passes with GLOAS_EPOCH because GLOAS_EPOCH - 4 is still in Fulu territory. But the test a_preference_for_the_second_gloas_epoch_is_signed_under_the_gloas_version uses GLOAS_EPOCH + 1, and GLOAS_EPOCH + 1 - 4 = GLOAS_EPOCH - 3, which is still Fulu!

Wait, let me re-read the test... The test at line 691 only checks that it verifies under gloas_fork_version, but doesn't check that it fails under fulu_fork_version. Let me check...

Actually, looking more carefully at line 720-724:

assert!(verify(
    &pubkey,
    &signature,
    under(context.config.gloas_fork_version)
));

This only asserts it verifies under Gloas, not that it fails under Fulu. So the test doesn't catch the MIN_SEED_LOOKAHEAD issue.

This is a bug: The signing domain uses MIN_SEED_LOOKAHEAD (4) instead of 1, meaning for epochs GLOAS_EPOCH through GLOAS_EPOCH+2, the signature would verify under Fulu in ethlambda but potentially fail against other clients' gossip validation that uses the correct epoch-1 semantics.

Wait—I need to re-read the spec deviation document more carefully. It says:

validate_proposer_preferences_gossip verifies against the dependent block's state advanced to the lookahead start

The "lookahead start" might indeed be MIN_SEED_LOOKAHEAD epochs before. Let me re-check the consensus spec...

Actually, looking at the CLAUDE.md change:

the gossip rule's signing domain

And the spec deviation says:

the gossip rule's state (at the start of the previous epoch)

Hmm, "previous epoch" suggests 1 epoch, not 4. But "lookahead start" in the spec might mean MIN_SEED_LOOKAHEAD.

Given the explicit test and the detailed comment, this might be intentional. However, the comment says "the epoch before the proposal's" which is 1, not 4. This inconsistency between comment and code is confusing and potentially wrong.

Recommendation: Verify against the actual consensus spec for validate_proposer_preferences_gossip. If the spec uses epoch - 1, fix the code. If it uses epoch - MIN_SEED_LOOKAHEAD, fix the comment.

4. compute_shuffling_dependent_slot vs. V1 Definition Consistency

File: crates/net/rpc/src/beacon/validator.rs, lines 540-544

Self::V2 => compute_shuffling_dependent_slot(epoch),

Good: This delegates to the state transition function, preventing drift. But verify that compute_shuffling_dependent_slot is actually exported and correct.

---

Performance Issues

5. Lock Contention in PreferencesService::poll

File: crates/validator/src/preferences.rs, lines 181-186

for target in [epoch, epoch + 1] {
    if self.context.config.fork_at_epoch(target) < ForkName::Gloas {
        continue;
    }
    self.publish_epoch(current_slot, target, &pubkeys, &store).await;
}

The loop processes epochs sequentially. Each publish_epoch awaits v2 duty fetches and submissions. Since the two epochs are independent, they could be parallelized with try_join! or join!.

Mitigation: Low impact (only 2 epochs), but worth considering for high-latency nodes.

6. Per-Preference Sequential Submissions

File: crates/validator/src/preferences.rs, lines 252-280

Preferences are sent one-by-one in a loop with await each iteration. For validators with many proposal slots, this serializes network round-trips.

Mitigation: Consider limited concurrency (e.g., futures::stream::iter(...).buffer_unordered(4)) to parallelize submissions while maintaining per-preference error handling.

---

Rust Best Practices

7. Mutex Poison Recovery Hides Bugs

File: crates/validator/src/preferences.rs, lines 121-125

fn lock<T>(mutex: &Mutex<T>) -> MutexGuard<'_, T> {
    mutex
        .lock()
        .unwrap_or_else(std::sync::PoisonError::into_inner)
}

While documented, silently recovering from poisoned mutexes can hide panics in other threads. In a consensus client, this could mask critical failures.

Recommendation: Consider whether parking_lot::Mutex (which doesn't poison) would be cleaner, or at least log when recovering from poison.

8. AtomicBool Ordering Could Be Relaxed

File: crates/validator/src/preferences.rs, line 178

if self.in_flight.swap(true, Ordering::AcqRel) {

AcqRel is stronger than needed for a simple flag. Acquire on load, Release on store would suffice, or even Relaxed since there's no data dependency being synchronized (the actual data goes through Arc clones).

Fix: Use Ordering::Acquire for the load part of swap... actually swap doesn't let you split. Consider compare_exchange_weak(false, true, Ordering::Acquire, Ordering::Relaxed) for better performance on some architectures.

9. std::sync::Mutex in Async Code

File: crates/validator/src/preferences.rs, multiple locations

Using std::sync::Mutex and immediately locking/unlocking in async code is acceptable for very short critical sections, but parking_lot::Mutex or tokio::sync::Mutex would be more idiomatic. The current pattern of lock(&self.published).insert(...) is fine since it doesn't hold across await points.

---

Error Handling & Edge Cases

10. Empty preferences List Early Return

File: crates/validator/src/beacon_node/http.rs, lines 837-839

if preferences.is_empty() {
    return Ok(());
}

Good defensive coding. But verify this matches the spec—does the Beacon API allow empty POSTs? The mock test confirms this behavior, but ensure it's consistent with other similar endpoints.

11. Missing Handling of dependent_root Mismatch

File: crates/validator/src/preferences.rs

The service remembers published preferences by (slot, dependent_root), but what if a node accepts a preference with a different dependent_root than what the service expects? The v2 duties fetch gives one root, but the node's head might have moved on.

Current behavior: The preference would be signed with root A, sent to node with head at root B, and the node's gossip rule would reject it. The service would retry next slot.

Potential issue: If there's a consistent mismatch (e.g., node sees different fork choice), the service would warn once and then debug-log retries, potentially missing proposals silently.

Mitigation: Consider adding metrics or more visible logging for persistent mismatches.

---

Testing Issues

12. Test verifies Function Duplicates Production Logic

File: crates/validator/src/preferences.rs, lines 437-463

The verifies helper reconstructs domain computation manually instead of using SigningContext. This creates a maintenance burden and risk of test-production drift.

Recommendation: Use SigningContext::domain or a test helper that shares code.

13. Missing Test for Fork Boundary Signing

File: crates/validator/src/signing.rs

The tests cover first and second Gloas epochs, but miss:

• Epoch exactly at GLOAS_EPOCH + 3 (where MIN_SEED_LOOKAHEAD subtraction would still yield Fulu if using 4, but Gloas if using 1)
• The transition from Fulu to Gloas during the epoch before Gloas (when signing preferences for GLOAS_EPOCH during GLOAS_EPOCH - 1)

---

Documentation & Comments

14. Inconsistent Terminology: "gloas" vs "Gloas"

Minor: Some comments use lowercase "gloas" for the fork name. Standardize to "Gloas" when referring to the fork.

15. CLAUDE.md Update Is Good but Could Be Clearer

The CLAUDE.md change correctly documents the behavior, but the sentence:

From the epoch before gloas it signs proposer preferences every slot for its proposals in the current and next epoch

Could be misread as "from the epoch before Gloas starts" vs "starting from the epoch before each target epoch." The actual behavior is: starting from GLOAS_EPOCH - 1, sign preferences for slots in GLOAS_EPOCH and GLOAS_EPOCH + 1.

---

Minor Issues

16. builder_config Function Name

File: crates/validator/src/beacon_node/http.rs, line 88

The name builder_config is slightly misleading—it's the self-build configuration, not general builder configuration. produce_block_v4_body or self_build_config might be clearer.

17. Redundant std::slice::from_ref Import Style

File: crates/validator/src/preferences.rs, line 269

std::slice::from_ref(&entry)

Consider importing std::slice::from_ref at the top for consistency, or use &[entry] directly if the trait accepts it.

Actually, looking at the trait:

preferences: &[gloas::SignedProposerPreferences],

std::slice::from_ref(&entry) produces &[SignedProposerPreferences], which is correct. &[&entry] would be wrong. This is fine.

---

Summary Table

Priority	Issue	Location	Fix
Critical	builder_boost_factor serialized as string not number	http.rs:94	Remove quotes: "{factor}" → {factor}
High	Signing domain uses MIN_SEED_LOOKAHEAD (4) vs epoch-1; comment/code mismatch	signing.rs:339	Verify spec, fix code or comment
Medium	Mutex poison recovery hides bugs	preferences.rs:121	Use parking_lot or log recovery
Medium	Sequential epoch processing	preferences.rs:181	Consider join!
Medium	Sequential preference submission	preferences.rs:252	Consider limited parallelism
Low	AtomicBool ordering could be relaxed	preferences.rs:178	compare_exchange_weak
Low	Test logic duplicates production	preferences.rs:437	Share domain computation

The PR is well-tested and the architecture is sound. The string-vs-number JSON serialization is the most critical fix needed before merge.

---

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

[pablodeymo]

Security read of this PR, with the signing side and the network/node side reviewed separately. No critical or high issues: the message is signed under DOMAIN_PROPOSER_PREFERENCES alone, so nothing here is slashable or replayable under another domain; only keys in the store are ever signed for, and a key removed between the duties fetch and signing is skipped; no secret is logged or cloned; no lock is held across an await; the single-flight guard clears on panic and drop; and the signing domain (fork version at proposal_epoch - 1), the v2 dependent root (compute_shuffling_dependent_slot, the same function the gossip rule uses) and the Eth-Consensus-Version all line up with what the node verifies, pinned by the signing.rs tests and the end-to-end gossip test. Two things I'd fix before merging, plus smaller ones.

1. Bound and check the duties before signing (preferences.rs::publish_epoch, http.rs::proposer_duties_v2)

The v2 duties are taken as they come: no cap on the entry count and no check that duty.slot lies in the target epoch. The fallback's try_each hands the list from the first node that answers, so one buggy or hostile node can return N entries naming our keys at far-future slots with a rotating dependent_root. Each slot the client then signs N messages while holding the store read guard (tokio's RwLock is write-preferring, so a keymanager write queued behind it blocks the slot loop's next read), sends N sequential POSTs to every node at up to 8 s each inside the single-flight poll, and inserts N keys into published or refusal_warned. The prune keeps every slot at or above the current epoch's start, so far-future keys are never dropped. Nothing dangerous is signed (own domain, own fee recipient), so this is amplification and memory growth, not key exposure. Dropping duties whose slot is outside [start(target), start(target + 1)) bounds everything to 32 entries per target and makes the prune effective.

2. A node that refused once never receives the preference again (fallback.rs::try_all_inner, preferences.rs::publish_epoch)

try_all answers Ok as soon as one node accepts, and the caller then marks (slot, dependent_root) published and skips it on every later poll. The retry-every-slot the module doc promises only happens when every node refused. A node that was syncing, restarting, or lacked the dependent block at that moment ends up with no entry in its builder market, and since gossip never queues or replays, nothing fills it in later. If the client fails over to that node for produceBlockV4 at the slot, it ignores every p2p bid and self-builds, and the gas target falls back to the parent bid's. Registrations (#705) re-send the whole set every epoch and so self-heal; preferences have no periodic resend. Keying published by (slot, root, position), or marking it published only when every configured node accepted, would close it.

Smaller

• validator_index and dependent_root go into the signed message verbatim from the node. The duty loop already resolved the indices, so a cross-check is cheap; a wrong index is rejected by gossip today, so the cost is only a refused send.
• The zero address passes --suggested-fee-recipient (hex and length only), and from gloas this PR signs and publishes it unconditionally, so a mistyped zero address burns rewards. Same point as on feat(validator): propose builder blocks (fulu) #705, with more reach here.
• The settings are read twice per message (fee recipient, then gas limit), so a keymanager write in between pairs an old recipient with a new gas limit for that (slot, root), which cannot then be replaced. One snapshot per message would do.
• Response bodies are still reflected uncapped into BeaconNodeStatus and from there into the once-per-key warn line; the client keeps reqwest's default redirect policy. Both pre-existing (feat(validator): propose builder blocks (fulu) #705).
• The route comment about a validator client naming fulu during the epoch before gloas describes other clients; this one always sends the target epoch's fork.

Tests I'd add: duties at slots outside the target epoch are dropped, and a list of hundreds of entries is bounded; a node that refused while another accepted receives the preference on a later poll; a key removed between the duties fetch and signing is skipped without losing the others; a preference signed during the first gloas epoch itself passing the node's rules end to end (the gossip test has gloas at epoch 0, so the boundary is unit-pinned only).

Checked and clean: pre-gloas target epochs skipped before any fetch; started slots skipped, matching the node's IGNORE; one request per preference, so one refusal cannot hold back the others; an empty list never reaches the wire; builder_boost_factor is a u64 in a format!, None renders 0, and the node multiplies in u128, so no clamp is needed; poisoned mutexes recovered; a bounced poll exits at once; no_fee_warned and fetch_warned hold at most two live epochs; the node route accepts a fulu or gloas header and caps entries at the lookahead length; nothing new derives Debug on a secret, and logs carry pubkeys, slots and indices only.

[MegaRedHand]

/review

[github-actions]

🤖 Gemini Code Review

This PR looks great. The implementation of PreferencesService, the v2 proposer duties client method, the produceBlockV4 builder boost factor handling, and the gossip signing domain handling at the fork boundary are all clean, well-documented, and thoroughly covered by unit and integration tests.

A few highlights:

• Using compute_shuffling_dependent_slot directly in DependentRoot::slot (crates/net/rpc/src/beacon/validator.rs:543) ensures the RPC route and state transition logic cannot drift.
• Sending preferences individually and tracking state per (slot, dependent_root) in PreferencesService (crates/validator/src/preferences.rs:270-310) cleanly avoids a single stale/conflicting preference blocking the rest of the epoch's preferences, while handling reorgs and mid-epoch configuration updates naturally.
• Signing at proposal_epoch.saturating_sub(MIN_SEED_LOOKAHEAD) (crates/validator/src/signing.rs:342-345) accurately matches the fork version in the lookahead state used by validate_proposer_preferences_gossip during the first epoch of Gloas.

No issues found — ready to merge.

---

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

[github-actions]

🤖 Codex Code Review

• [P2] Retry preferences on nodes that failed to accept them — preferences.rs:281. FallbackBeaconNode::try_all_inner returns success when any node accepts, so this permanently marks the preference published even if another node was unavailable. When that node recovers, subsequent polls skip delivery; gossip does not guarantee replay of preferences published during its downtime. If block production fails over to it, builder bids may be ignored and the requested gas target unavailable. Track acceptance per node and retry failed deliveries using the same signed preference. Add a regression test covering partial failure followed by recovery.

Tests could not run: Rustup attempted to write to its read-only temporary directory.

---

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

[github-actions]

🤖 Claude Code Review

Review: proposer preferences in the validator client (PR 707)

Overall this is well-structured. Single-flight polling, per-entry submits, per-(slot, root) dedup and once-per-epoch warnings are all sensible, and the tests are thorough. The main gap is in the gloas proposal flow (Item 1); the rest are smaller.

Correctness / consensus

1. A non-zero gloas boost factor lets a p2p bid win, but the proposal path is unchanged.

• Where: http.rs builder_config, BlockRequest docs in mod.rs.
• Problem: With --builder-proposals, produceBlockV4 can now return a block whose bid has builder_index != BUILDER_INDEX_SELF_BUILD. The trait doc for execution_payload_envelope still describes it as "the envelope the node cached for the self-built payload". The diff does not touch proposal.rs.
• Consequence: If the proposal service unconditionally fetches, signs and publishes the envelope after a gloas block, a won builder bid will produce a 404. That gets logged as a missed envelope, or retried, on every builder-won proposal. Nothing should ever sign an envelope for a builder's payload.
• Fix: Confirm the proposal path branches on the bid's builder index. Add a test where the mock returns a non-self-build gloas block.

2. Unconditional publishing combined with factor 0 sends builders a mixed signal.

• Without --builder-proposals, preferences are published, so builders are invited to bid. The body then carries builder_boost_factor: "0", so no bid can ever win.
• Publishing is defensible (fee recipient), and the docs say this. Still, consider stating explicitly in docs/cli.md that bids are solicited but never taken without the flag. Operators reading the metrics may otherwise be confused.

3. The signing domain deviates from validator.md, which is an interop risk.

• Where: signing.rs sign_proposer_preferences.
• The reasoning about get_domain(state.fork) vs. the schedule is sound. In the common case, first-gloas-epoch preferences are signed during the fulu epoch, so both definitions agree.
• The divergence only hits re-signs inside the first gloas epoch, e.g. after a reorg moves the root or after a restart.
• Risk: If Lighthouse, Prysm or others verify against the head state rather than the advanced dependent state, those re-signed preferences will be rejected on their mesh. Only ethlambda's endpoint is known to accept both.
• Ask: Worth confirming against another client's gossip implementation before relying on it. The deviation doc should mention which side other clients took.

4. Verify that the DependentRoot::V2 refactor matches the old formula at the edges.

• Where: rpc/beacon/validator.rs, around line 540.
• The old code was compute_start_slot_at_epoch(epoch.saturating_sub(1)).saturating_sub(1), which gives slot 0 for both epoch 0 and epoch 1.
• Please confirm compute_shuffling_dependent_slot saturates identically for epochs 0 and 1. If it does not, the served root changes there.
• A unit test pinning DependentRoot::V2.slot(0), slot(1) and slot(2) would lock this down.

Behaviour

5. Only the first node's duties are signed against.

• Where: fallback.rs proposer_duties_v2 / submit_proposer_preferences.
• Duties come from the first node that answers (try_each_quiet), but the submit goes to all nodes. A secondary node on a different head refuses the entry, and that refusal is only logged at debug!.
• published is then set as soon as one node accepts, so the refusing node is never retried even after it converges.
• This is acceptable given the cost, but worth a sentence in the module doc ("Why every node gets them") so nobody assumes full fan-out is guaranteed.

6. Pre-fork submissions carry a gloas Eth-Consensus-Version.

• Where: preferences.rs publish_epoch.
• During the epoch before gloas, fork_at_epoch(target) yields Gloas while the node's head is still fulu.
• That is correct per the endpoint semantics ("the fork the preferences belong to"). However, the only test with a fulu header is the mock/HTTP one; the node-side integration test runs with gloas at epoch 0.
• An ethlambda-node test at a gloas boundary with this header would close the gap.

Minor

• preferences.rs, fee-recipient check: lock(&self.published) is taken once per duty inside the loop. The lists are tiny so this is fine, but a single guard held across the loop would read cleaner.
• metrics.rs, failures counter: the doc comment says "no beacon node accepted", while docs/metrics.md says "refused or failed to take". Align the wording.
• Docs layout: CLAUDE.md and README.md reflow is fine. The README line now exceeds the surrounding wrap width.
• builder_market_tests.rs, integration test: the spawned axum::serve task is never shut down. That is fine in tests, but holding the JoinHandle and calling abort() at the end is tidier.

Nice work extracting test_support and the quiet fallback variants; the try_*_inner split keeps the logging policy in one place.

---

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

[github-actions]

🤖 Kimi Code Review

Finding

• Partial fallback submission is incorrectly remembered as fully published — crates/validator/src/preferences.rs, in publish_epoch’s successful submit_proposer_preferences arm, together with crates/validator/src/beacon_node/fallback.rs’s try_all_inner. try_all_inner returns Ok(()) when at least one node accepts, as the new three-node fallback test demonstrates. The service then inserts (slot, dependent_root) into the global published set and never submits that preference again. If node A accepts while node B is down or refuses, B is never retried once it recovers. This contradicts the module’s “a node that refused a send must be tried again” behavior and can reduce gossip/builder coverage or leave a later block-producing node without the preference’s fee recipient. Track publication per node, or return incomplete coverage from the fallback submission and retry until every configured node has accepted. Add a test where one node accepts initially and a failing node recovers on a later poll.

Otherwise, the change looks well considered: the v2 dependent-root calculation now shares the state-transition helper, the fork-boundary signing domain is explicitly tested, single-flight cleanup is panic-safe, and one-preference-per-request avoids batch-wide rejection poisoning unrelated entries.

---

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

[github-actions]

🤖 Grok Code Review

The signing-domain choice and the v2 dependent-root alignment look right, and the tests pin both. One fan-out bug will drop preferences on a multi-node setup.

1. Partial fan-out is treated as fully published. FallbackBeaconNode::submit_proposer_preferences (crates/validator/src/beacon_node/fallback.rs, around line 420) uses try_all_quiet, which returns Ok when any node accepts — the new test expects success with the middle node down. PreferencesService then inserts (slot, dependent_root) into published on that Ok (crates/validator/src/preferences.rs, around lines 275–282) and never sends again. A node that was syncing or briefly down never sees the preference, and builders on its mesh ignore the slot. That breaks the invariant in the module docs (“every node gets them”). Registrations heal this by resending every epoch; preferences do not. Identical resubmits are already short-circuited by the node, so only mark published when every configured node accepted (or track acceptance per node). Do not use the “no beacon node accepted” warning for a partial success. A test where one node fails the first call and accepts the retry would lock this in.

2. A hung poll stalls preferences for good. poll (preferences.rs, around lines 149–153) sets in_flight and lib.rs (around line 375) drops the task if the previous one is still running. InFlightGuard only clears the flag when that task finishes. If proposer_duties_v2 or the submit has no deadline, one stuck HTTP call stops every later poll until restart. Confirm the HTTP client times out; otherwise add a timeout so the guard always runs.

3. fetch_warned is not cleared on success (preferences.rs, around lines 171–172 and 188). A failed fetch in the epoch before gloas inserts the gloas epoch, a later success leaves the entry, and a failure after the fork boundary is only a debug!. Remove the epoch from fetch_warned when the fetch succeeds.

The gossip-rule domain in SigningContext::sign_proposer_preferences (previous epoch / MIN_SEED_LOOKAHEAD, not the head state’s fork) matches get_domain on the lookahead state, and the first-gloas-epoch test is the right pin. Routing DependentRoot::V2 through compute_shuffling_dependent_slot is the right way to keep the duties route from drifting, assuming that helper is still start_slot(epoch - 1) - 1. One preference per request, quiet fallback logs, and not gating preferences on --builder-proposals are all sound.

---

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

[github-actions]

🤖 DeepSeek Code Review

Overall this is solid, well-documented work. The fork-boundary domain logic and tests are especially good, and I didn’t find a consensus-critical or memory-safety bug in the diff.

A few non-blocking concerns:

• Partial success in fallback preferences — FallbackBeaconNode::submit_proposer_preferences uses try_all_quiet (crates/validator/src/beacon_node/fallback.rs around the new method at lines ~414-435). That returns Ok if any node accepts. PreferencesService::publish_epoch then marks the preference as published (crates/validator/src/preferences.rs around lines ~294-302) and will never retry nodes that failed on that attempt. The module docs specifically justify sending to every node because each node gossips what it accepts, so a transiently failed node can permanently miss that (slot, dependent_root). Either record/retry per-node acceptance or explicitly document that one node’s gossip is sufficient.

• Pre-Gloas overhead in poll — PreferencesService::poll (crates/validator/src/preferences.rs around lines ~115-140) always reads the validator store and takes four mutexes to prune even when both target epochs are before Gloas. This runs every slot before the fork. An early return after the fork_at_epoch checks would avoid pointless store reads and lock traffic.

• Quiet fallback method is now a shared-API behavior — FallbackBeaconNode::proposer_duties_v2 (crates/validator/src/beacon_node/fallback.rs around lines ~321-327) suppresses fallback-level failure warnings because the preferences service logs itself. That’s fine for the current caller, but the behavior is now attached to a general BeaconNodeApi method; any future caller will also get no warning on total failure. Consider documenting that on the trait method or moving the quiet handling higher up.

• Slow poll with stale current_slot — poll captures current_slot at start, and a slow poll can span a slot boundary. It may then attempt to submit preferences for slots that have already started, causing spurious refusals/warnings. Re-checking the current slot before each submission would reduce that edge case.

---

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

[MegaRedHand]

Thanks for the reviews. The changes are in 68cb7e0, 7edb397 and 3d3220b, on top of #705's (merged in at 382fa78). I checked each point against Lighthouse, Prysm, Teku, Lodestar and Nimbus. That check turned up one thing nobody had flagged, so it comes first.

The signing domain: we were the odd one out

consensus-specs v1.7.0-beta.4 signs and verifies proposer preferences with compute_fork_version(proposal_epoch):

• the signer is get_signed_proposer_preferences;
• the gossip rule is validate_proposer_preferences_gossip.

All five clients do the same: Lighthouse spec.fork_at_epoch(proposal_epoch), Prysm params.Fork(epoch), Teku spec.fork(proposalEpoch), Lodestar config.getDomain(proposalSlot), Nimbus forkAtEpoch(proposal_slot.epoch).

This client signed by beta.2's rule instead: get_domain on the lookahead state, which is the fork of the epoch before the proposal's. That is why Lighthouse refused every first-gloas-epoch preference on the devnet ("Known limitations" in the description).

• The client now signs at the proposal epoch's fork version. The tests assert gloas's version, and not fulu's, for both the first and the second gloas epoch.
• The node still accepts all three candidate domains: beta.4's (now tried first), plus beta.2's two. Our fixtures are pinned to beta.2, and a client still on that rule loses nothing. This can narrow once the fixtures move.
• A new end-to-end test with fulu before gloas covers this: a preference the client signs for the first gloas epoch passes our node's gossip rules, and verifies under gloas's version only.

@pablodeymo's security review

1. Bound and check the duties: done. Duties whose slot is outside the target epoch are dropped, and only the first one per slot is kept. Each target epoch therefore costs at most SLOTS_PER_EPOCH signatures, requests and remembered keys, whatever the node answers, and the prune now covers everything kept. No other client checks this: Lodestar has a window of 8 slots ahead, and the rest take the duties as they come. So this is extra, not a different direction.

2. A node that refused never got the preference again: fixed. A preference now counts as done only once every node accepted it. Until then it is re-sent to all of them each slot, as first signed rather than re-signed, so a fee recipient changed in between cannot give the late node a second, different preference. Re-sending to a node that already holds it is safe: ethlambda's node answers 200 for an identical entry, and Lighthouse's treats it as already seen. The published count takes each preference once. No client retries a single refusing node:

   • Lighthouse and Lodestar stop at the first success;
   • Nimbus counts any 200;
   • Teku re-posts on every duty reload;
   • Prysm re-pushes on a host switch.

   Sending to every node is what Teku and Nimbus do.

Smaller points:

• Validating validator_index: not cross-checked. A wrong index can only fail the node's gossip check (the proposer lookahead REJECTs it), so it costs one refused request. No client cross-checks it either; Lighthouse, Teku and Lodestar all take the index from the duty.
• Zero address: reported at error at startup, not refused (see feat(validator): propose builder blocks (fulu) #705: no client refuses it on the command line).
• One settings snapshot per message: done. The fee recipient and gas limit are read together, under one lock.
• Error bodies and redirects: see feat(validator): propose builder blocks (fulu) #705. Bodies are capped and every control character is written out. Redirects stay followed, as in every client.
• The route comment: corrected. No client names fulu: Prysm hardcodes gloas, and the others, ours included, take the fork of the preference's slot. The node still accepts fulu as leniency.

Tests: added:

• duties outside the target epoch are dropped;
• 400 duties give one preference per slot;
• with two nodes, the refusing node gets the preference once it accepts, and the acceptance is counted once;
• a key removed between the duties fetch and signing is skipped without losing the others;
• the first-gloas-epoch end-to-end test above.

Automated reviews

• builder_boost_factor as a string (Kimi): correct as it is. beacon-APIs quotes every Uint64.
• MIN_SEED_LOOKAHEAD is 4 (Kimi): it is 1 on both presets. The signing domain changed anyway, as described above.
• Concurrency across nodes and entries (Claude, Kimi): calls to every node now run concurrently (feat(validator): propose builder blocks (fulu) #705). The work per poll is bounded at one entry per slot.
• std::sync::Mutex in async code (Codex, Kimi): no lock is held across an await, and every write is a single set operation.
• --builder-proposals help text (Codex): it now says preferences are published with or without the flag, and that from gloas the flag only sets the boost factor.
• A boundary test end to end (Codex): added, as above.
• builder_config by hand (Claude): kept. The only input is a u64.