feat(validator_store): add configurable proposer delay for MEV bids

[shane-moore]

Problem, Evidence, and Context

Anchor requests a block as soon as RANDAO pre-consensus finishes. Relays that hold out for a better bid have not answered yet, so operators leave MEV on the table

Closes #1207.

Change Overview

--proposer-delay-ms is an absolute floor from the start of the slot, not added latency. The request lands at max(randao_completion, slot_start + delay), so a duty whose pre-consensus already ran past the target is never delayed further. This matches go-ssv exactly, so an operator's existing value carries across clients.

The wait sits at the success return point of randao_reveal. That is the last point Anchor owns before block production: the reveal is a required parameter of the block request, so Lighthouse structurally cannot ask earlier.

Reading order: cli.rs for the two flags, client/src/config.rs for the startup gate, then validator_store/src/lib.rs for the decision and the wait. Everything else is plumbing, metrics, and docs.

Unchanged: default is 0, so behaviour is identical for anyone who does not set the flag. No change to the signing path, slashing protection, QBFT, or publication. Not fork-gated.

Risks, Trade-offs, and Mitigations

The feature deliberately spends proposal headroom, so a large value can cost a proposal. That is the trade the operator opts into, and the docs say so rather than presenting it as free.

• Default 0; above 1000 needs --allow-dangerous-proposer-delay (matching the SSV node's threshold); above 4000 is refused as a typo guard, not as a safe ceiling.
• Fails open: if the slot clock cannot say how far into the slot we are, the delay is skipped.
• Two histograms (anchor_randao_reveal_completion_offset_seconds, anchor_proposer_delay_applied_seconds) let an operator see whether the delay bites at all. The applied wait is measured after the sleep, not before, so it reports the real wait.

Known and deliberate: the floor is approximate under a backward clock adjustment, since elapsed time is sampled once and the wait runs on a monotonic timer. A re-check loop could turn a clock correction into extra proposal-critical delay, so failing open is preferred.

Post-Gloas this keeps working but buys something different, and go-ssv splits it into a second fork-gated knob. Tracked separately in #1212; out of scope here.

Validation

• 13 new tests (8 policy, 5 config), all pure, no runtime or store needed.
• make test full suite green; make lint, make cargo-fmt-check, make mdlint, make cli-reference-check all pass.
• Adversarial review by Codex (gpt-5.6-sol, two rounds) against the diff, plus four cleanup passes. Findings applied include the applied-wait metric recording the planned rather than actual wait, and a docs claim that only the consensus leader requests a block (every operator does).

Rollback

Revert the commit. The default is 0, so no operator is affected unless they opted in, and there is no config, data, or on-disk migration.

[shane-moore]

@claude, please take a look

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 4m 44s —— View job

---

PR Review: configurable proposer delay for MEV bids

• Read the PR diff against origin/unstable
• Review CLI flags (cli.rs)
• Review config/startup gate (client/src/config.rs)
• Review core decision + wait logic (validator_store/src/lib.rs)
• Review metrics/instrumentation
• Review docs
• Post review feedback

---

Overall this is a high-quality, well-scoped PR. The design is sound and I did not find any correctness, safety, or security issues I'm confident about. A few things I specifically checked and confirmed are correct:

Verified correct

• Non-wrapping elapsed measurement. elapsed_in_slot (lib.rs:200) measures against start_of(clock_slot) rather than the current slot, so an overrunning duty reads as a growing offset instead of re-arming the floor. The contrast against determine_slot_elapsed_ms is proven by elapsed_in_slot_does_not_wrap_when_a_duty_overruns. Good, subtle catch to design around.
• Floor vs. added latency. proposer_delay_decision_at (lib.rs:216) waits only delay − elapsed, and the exact-boundary case (elapsed == delay) correctly resolves to TargetPassed via checked_sub → Some(ZERO) → the _ arm. Tested.
• Fails open. None elapsed → ClockUnavailable → no wait; Duration::MAX → TargetPassed. Both tested.
• Startup warn! is visible. enable_logging runs at anchor/src/main.rs:40, before config::from_cli at :67, so the "flag has no effect" and error messages in proposer_delay_from_millis reach the operator.
• Metric measures the real sleep. await_proposer_delay (lib.rs:325) samples Instant::now() before/after the sleep and records the observed wait, matching the PR description and the metric doc-comment. Span records land correctly since the method runs inside the .instrument(span) future.
• Label stability is guarded by proposer_delay_outcome_labels_are_stable, and the labels are cross-referenced from the docs — a nice contract to lock down since they're a dashboard/query surface.

Minor observation (non-blocking)

The policy (proposer_delay_decision_at) has excellent pure-function coverage, but the async wrapper await_proposer_delay (the side-effect-producing part: sleep, metric emission, span records) has no direct test. The wrapper is thin and the risk is low, so this is optional — but a tokio::time paused-clock test asserting that a Waited(_) decision actually advances the timer by the planned amount and emits the waited label would close the gap between "the decision is right" and "the wait is applied correctly." Worth a follow-up at most, not a blocker.

Everything else — CLI plumbing, config threading, testing harness default (Duration::ZERO), metrics buckets, and docs — is consistent and correct. Nice work on the docs and the fail-open reasoning in particular.
• feat/proposer-delay

[codecov-commenter]

Codecov Report

❌ Patch coverage is 82.60870% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.00%. Comparing base (fd3c7f9) to head (a7b6342).

Files with missing lines	Patch %	Lines
anchor/validator_store/src/lib.rs	85.35%	23 Missing ⚠️
anchor/client/src/config.rs	85.45%	8 Missing ⚠️
anchor/validator_store/src/metrics.rs	52.94%	8 Missing ⚠️
anchor/client/src/lib.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1213      +/-   ##
============================================
+ Coverage     64.84%   65.00%   +0.16%     
============================================
  Files           163      163              
  Lines         27785    28005     +220     
============================================
+ Hits          18017    18205     +188     
- Misses         9768     9800      +32     

Flag	Coverage Δ
rust	65.00% <82.60%> (+0.16%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shane-moore]

@claude Added the paused-clock coverage in 34faefd.

await_proposer_delay only ever read self.proposer_delay, so it is now a free function like its two neighbours. That drops the store dependency and lets the tests run on a virtual clock, where tokio::time::Instant observes the same clock sleep advances. One test asserts a 300ms floor with 100ms already elapsed advances the timer by exactly 200ms; the other that all three no-wait cases (overdue, no clock, disabled) return without advancing it at all.

Also dropped an intra-doc link from a public item to a private one, which rustdoc warns about and CI does not check.

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 2m 4s —— View job

---

Review: paused-clock coverage (34faefd)

• Read the new commit's diff against origin/unstable
• Review the await_proposer_delay refactor to a free function
• Review the two new paused-clock tests
• Review the rustdoc link fix
• Post review feedback

---

This directly closes the gap I flagged in the earlier review — the async wrapper's side effects (sleep, metric emission) are now exercised, not just the pure decision. The changes are sound and I found no correctness issues.

Refactor is safe. await_proposer_delay only ever read self.proposer_delay, so lifting it to a free function (proposer_delay: Duration, elapsed: Option<Duration>) loses nothing. The single production caller at lib.rs:2433 now passes self.proposer_delay explicitly, and it sits inside the same .instrument(span) future, so the Span::current().record(...) calls still land on the right span. It also now matches its two pure neighbours (proposer_delay_decision_at, elapsed_in_slot), which is a nice consistency win.

The paused-clock tests are the right shape and the arithmetic is correct. Traced against proposer_delay_decision_at (lib.rs:226):

• (300ms, Some(100ms)) → checked_sub = 200ms (non-zero) → Waited(200ms) → sleeps 200ms. Asserting elapsed() == 200ms is exactly the "floor, not added latency" contract, and it's the one assertion the decision tests structurally cannot make.
• (300ms, Some(400ms)) → checked_sub = None → TargetPassed; (300ms, None) → ClockUnavailable; (0, ZERO) → Disabled. All three → wait() == None → no sleep → elapsed() == ZERO. The table covers the three distinct no-wait branches (overdue / no clock / disabled), so a regression that turned any one of them into a sleep would fail — this is the "delay a proposal with no time to spare" case, worth locking down.

Using start_paused = true so tokio::time::Instant and sleep share the virtual clock is the correct way to assert exact wait amounts deterministically.

Doc link fix is right and consistent. The public trait method randao_reveal (lib.rs:2362) can't intra-doc-link to the private await_proposer_delay without a rustdoc warning, so demoting it to a plain code span is correct. The remaining [await_proposer_delay] intra-doc link at lib.rs:189 is on the private proposer_delay field — private→private, so that one stays valid.

One note, non-blocking: the no-wait test still calls metrics::observe_timer_vec(&PROPOSER_DELAY_APPLIED, ["disabled"|"target_passed"|"clock_unavailable"], ZERO) against the global metric registry. That's harmless (and arguably good — it proves the metric path runs on the no-wait branches too), just be aware it mutates process-global metric state if a future test ever asserts on those histograms.

Verification: I could not run cargo test here — test execution is blocked by sandbox permissions in this job, so I verified the two tests by tracing their inputs through proposer_delay_decision_at and ProposerDelayDecision::wait() rather than by running them. The PR description reports the full suite green locally; worth confirming CI is green on this commit before merge.

Nice, tightly-scoped follow-up.
• feat/proposer-delay

[shane-moore]

@claude Leaving the global metric mutation as-is: isolating it would cost more machinery than the risk warrants, and as you note it proves the metric path runs on the no-wait branches too. Noted for anyone who later asserts on those histograms. CI green on 34faefd.

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 20s —— View job

---

Acknowledged — global metric mutation stays as-is

Agreed. Isolating it (per-test registry or a mockable metrics handle) would add more machinery than the risk warrants for what is a harmless observation on the no-wait branches. Recording ZERO against PROPOSER_DELAY_APPLIED on the disabled / target_passed / clock_unavailable paths actually earns its keep: it proves those branches run the metric path without sleeping, which is exactly the "no time to spare" case worth locking down.

The only thing to keep in mind is the note you already flagged — if someone later adds a test that asserts on those histograms, they'll need to account for the process-global state these tests write. That's a future-test concern, not a change to make now.

Nothing outstanding from my side. With CI green on 34faefdc and the earlier coverage gap closed, I have no further review feedback — this looks ready.
• feat/proposer-delay

[jnhsigmap]

I found 1 that might deserve a look. Feel free to discard this is its not needed. There is currently no test that calls randao_reveal

• If a refactor moves await_proposer_delay above collect_signature or onto the error path, this will go undetected by the test suite,
• This moves the wait above collect_signature and creates added latency by spending proposal headroom twice.

Things that protect us a little bit -

• This proposer delay feature defaults to off.
• Accidentally introducing this regression will hit operators who have opted in.

You also note that early randao reveal in #1179 will restructure the internals of randao_reveal. Maybe for piece of mind we address this now?

Pre-setup, anchor/validator_store/src/testing/common.rs:

• This requires a bit of a refactor to ValidatorStoreTestHarness and CapturedSignatureCall
• Wraps existing new_with_fork body in new_with_options and creates second call for this case (proposer delay) that adds a Duration.
• Timestamp on captured mock signature calls.
• Shared clock on ValidatorStoreTestHarness.

-use tokio::sync::watch;
+use tokio::{sync::watch, time::Instant};

 pub(super) const TEST_SLOT: u64 = 1;
-const SLOT_DURATION_SECS: u64 = 12;
+pub(super) const SLOT_DURATION_SECS: u64 = 12;

 pub(super) struct CapturedSignatureCall {
     pub(super) requester: SignatureRequester,
     pub(super) validator_pubkey: PublicKeyBytes,
     pub(super) signing_root: Hash256,
+    /// When the call was made.
+    pub(super) captured_at: Instant,
 }

         self.captured.lock().push(CapturedSignatureCall {
             requester,
             validator_pubkey: signing_data.validator_pubkey,
             signing_root: signing_data.root,
+            captured_at: Instant::now(),
         });

 pub(super) struct ValidatorStoreTestHarness {
     pub(super) validator_store:
         Arc<AnchorValidatorStore<ManualSlotClock, MainnetEthSpec, MockConsensusDecider>>,
     committee_setups: Vec<CommitteeSetup>,
     pub(super) captured_calls: CapturedCalls,
+    /// Shares `current_time` with the store's clone, so tests can reposition the clock.
+    pub(super) slot_clock: ManualSlotClock,
     pub(super) is_synced_tx: watch::Sender<bool>,
     _slashing_db_dir: TempDir,
     _exit_signal: async_channel::Sender<()>,
 }

     pub(super) fn new_with_fork(
         committee_setups: Vec<CommitteeSetup>,
         our_operator_id: OperatorId,
         active_fork: Fork,
+    ) -> Self {
+        // No proposer delay by default: most tests assert timing-free behaviour.
+        Self::new_with_options(
+            committee_setups,
+            our_operator_id,
+            active_fork,
+            Duration::ZERO,
+        )
+    }
+
+    pub(super) fn new_with_proposer_delay(
+        committee_setups: Vec<CommitteeSetup>,
+        our_operator_id: OperatorId,
+        proposer_delay: Duration,
+    ) -> Self {
+        Self::new_with_options(
+            committee_setups,
+            our_operator_id,
+            Fork::Boole,
+            proposer_delay,
+        )
+    }
+
+    fn new_with_options(
+        committee_setups: Vec<CommitteeSetup>,
+        our_operator_id: OperatorId,
+        active_fork: Fork,
+        proposer_delay: Duration,
     ) -> Self {

             30_000_000,
             None,
             false,
-            Duration::ZERO, // no proposer delay: tests assert timing-free behaviour
+            proposer_delay,
             false,
             is_synced_rx,
             executor,

         Self {
             validator_store,
             committee_setups,
             captured_calls,
+            slot_clock,
             is_synced_tx,
             _slashing_db_dir: slashing_db_dir,
             _exit_signal: exit_signal,
         }

Then comes the actual test implementation:

2. anchor/validator_store/src/testing/mod.rs

 mod committee_aggregate;
 mod committee_attestation;
+mod proposer_delay;
 mod sync_selection_proof;

3. New file: anchor/validator_store/src/testing/proposer_delay.rs

//! Integration tests for the proposer delay applied by `randao_reveal()`.
//!
//! These pin where the wait sits in the method, not just the delay arithmetic. The
//! hazard defended against is hoisting the wait above `collect_signature`, which would turn
//! the floor into added latency on top of pre-consensus and make the error path block for
//! the full delay.
//!
//! The harness clock is positioned to 100ms into `TEST_SLOT` so the documented recommended
//! delay of 300ms (safely under both config bounds: the 1000ms acknowledgement gate and the
//! 4000ms hard cap) leaves a 200ms remainder.

use std::time::Duration;

use bls::PublicKeyBytes;
use slot_clock::ManualSlotClock;
use ssv_types::OperatorId;
use tokio::time::Instant;
use types::Epoch;
use validator_store::ValidatorStore;

use super::common::*;
use crate::Error;

const OUR_OPERATOR_ID: OperatorId = OperatorId(1);
const OPERATOR_IDS: [OperatorId; 4] = [OperatorId(1), OperatorId(2), OperatorId(3), OperatorId(4)];
const COMMITTEE_INDEX: usize = 0;
const VALIDATOR_INDEX: usize = 0;

/// The documented recommended `--proposer-delay-ms` value; a configurable, realistic setting.
const PROPOSER_DELAY: Duration = Duration::from_millis(300);
/// How far into `TEST_SLOT` the clock is repositioned before each test.
const ELAPSED_IN_SLOT_AT_CALL: Duration = Duration::from_millis(100);
/// The floor minus the elapsed time: 300ms - 100ms.
const EXPECTED_REMAINING_WAIT: Duration = Duration::from_millis(200);

/// Epoch of `TEST_SLOT` (slot 1) on the mainnet spec.
const SIGNING_EPOCH: Epoch = Epoch::new(0);

/// Places the shared clock `ELAPSED_IN_SLOT_AT_CALL` into `TEST_SLOT`, replacing the harness
/// default of 5s in, which the 300ms floor could never reach.
fn reposition_clock_early_in_test_slot(slot_clock: &ManualSlotClock) {
    slot_clock.set_current_time(
        Duration::from_secs(TEST_SLOT * SLOT_DURATION_SECS) + ELAPSED_IN_SLOT_AT_CALL,
    );
}

fn harness_with_proposer_delay() -> ValidatorStoreTestHarness {
    let committee = create_committee_setup(&OPERATOR_IDS, 1, 0);
    let harness = ValidatorStoreTestHarness::new_with_proposer_delay(
        vec![committee],
        OUR_OPERATOR_ID,
        PROPOSER_DELAY,
    );
    reposition_clock_early_in_test_slot(&harness.slot_clock);
    harness
}

/// The delay is a floor from slot start, applied *after* signature collection: with the clock
/// 100ms into the slot, the 300ms floor sleeps only the 200ms remainder. Time is paused, so
/// `Instant` observes virtual time and the mock collector's `captured_at` proves the signature
/// was collected before any sleep ran.
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn randao_reveal_success_waits_only_the_floor_remainder_after_collection() {
    let harness = harness_with_proposer_delay();
    let validator = harness.validator_metadata(COMMITTEE_INDEX, VALIDATOR_INDEX);
    let started = Instant::now();

    let result = harness
        .validator_store
        .randao_reveal(validator.public_key, SIGNING_EPOCH)
        .await;

    result.expect("randao reveal should succeed");
    assert_eq!(
        started.elapsed(),
        EXPECTED_REMAINING_WAIT,
        "delay must be a floor from slot start, sleeping only the remainder"
    );
    let captured = harness.captured_calls.lock();
    assert_eq!(captured.len(), 1, "expected one sign_and_collect call");
    assert_eq!(
        captured[0].captured_at, started,
        "signature must be collected before the proposer delay sleeps"
    );
}

/// A failing duty must not be held to the floor: the wait sits on the success path only, so an
/// unknown pubkey errors immediately with no virtual time spent.
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn randao_reveal_unknown_pubkey_fails_without_waiting() {
    let harness = harness_with_proposer_delay();
    let unknown_pubkey = PublicKeyBytes::deserialize(&[0xFF; 48]).expect("valid length");
    let started = Instant::now();

    let result = harness
        .validator_store
        .randao_reveal(unknown_pubkey, SIGNING_EPOCH)
        .await;

    assert!(
        matches!(result, Err(Error::UnknownPubkey(pk)) if pk == unknown_pubkey),
        "unknown pubkey should fail with UnknownPubkey"
    );
    assert_eq!(
        started.elapsed(),
        Duration::ZERO,
        "error path must not apply the proposer delay"
    );
}

[jnhsigmap]

at anchor/validator_store/src/lib.rs:294-298 -

• "Slot clock unavailable" is one of the two causes that reach this outcome/branch.
• The comment inside elapsed_in_slot (lib.rs:206-207) provides the alternative: "a readable clock whose current time is before the slot started."
• Worth adding more detail to guide debugging.

Something like this?

      if let ProposerDelayDecision::ClockUnavailable = decision {
          warn!(
              checkpoint = instrumentation::checkpoints::PROPOSER_DELAY_APPLIED,
              outcome,
              "Slot timing unreadable (clock unavailable or reported time before slot start), skipping configured \
               proposer delay"
          );

[mergify]

Queued — the merge queue status continues in this comment ↓.

[shane-moore]

Both applied in 591bc23: your randao_reveal tests and harness refactor essentially verbatim (all 53 crate tests pass), and the expanded clock warn wording. Thanks for the ready-made drafts!

[mergify]

Merge Queue Status

• 🟠 Waiting for queue conditions
• ⏳ Enter queue
• ⏳ Run checks
• ⏳ Merge

Waiting for

• -closed [📌 queue requirement]
• check-success=target-branch-check

All conditions

• -closed [📌 queue requirement]
• any of [🔀 queue conditions]:
  • all of [📌 queue conditions of queue rule default]:
    • check-success=target-branch-check
    • #approved-reviews-by >= 1
    • check-success=license/cla
    • github-review-approved [🛡 GitHub repository ruleset rule unstable and release-v*]
    • label!=do-not-merge
    • any of [🛡 GitHub repository ruleset rule unstable and release-v*]:
      • check-success = license/cla
      • check-neutral = license/cla
      • check-skipped = license/cla
• -conflict [📌 queue requirement]
• -draft [📌 queue requirement]
• any of [📌 queue -> configuration change requirements]:
  • -mergify-configuration-changed
  • check-success = Configuration changed