fix(validator_store): publish VotingAssignments 50ms past slot boundary

[shane-moore]

Problem, Evidence, and Context

• MetadataService Phase 1 sleeps to the slot boundary and then reads the wall clock. A timer wake a few milliseconds early reads the previous slot, republishes it, and the next iteration sleeps past the new slot. Waiters fail with MetadataSlotPassed, and the Phase 2/3 cascade costs the operator three slots of committee participation.
• Observed on a 512-slot scale-100 devnet soak: 6 skipped slots, 3 of them quorum-losing across a 4-operator committee, with wakes measured 1 to 5 ms before the boundary.
• Addresses the Anchor side of Lighthouse proposer scheduler can skip an assigned slot after an early timer wake #1223. The proposer-scheduler half of that issue lives in Lighthouse and is tracked separately.

Change Overview

• Phase 1 now sleeps duration_to_next_slot() + 50ms, the same idiom Phase 3 already uses (its 2/3-slot offset makes it immune to this race) and the same shape Lighthouse uses for slot-keyed work. 50 ms mirrors the message validator's CLOCK_ERROR_TOLERANCE, the inter-node clock error the network already budgets for, and is 10-50x the measured drift.
• The loop is extracted into a small run_slot_start_publisher helper so the race is unit-testable.
• Reading order: the VOTING_ASSIGNMENTS_PUBLISH_DELAY doc comment, then the helper, then the two tests.
• Intentionally unchanged: update_voting_assignments itself, the waiter contract, and Phase 2/3 scheduling.

Risks, Trade-offs, and Mitigations

• Assignments publish 50 ms later. Every consumer was traced: the selection-proof flows hold an absolute 2/3-slot deadline and the voting-context build triggers no earlier than a head event. Nothing reads the data inside the first 50 ms; block proposals never read it at all.
• A backwards clock step larger than 50 ms during one sleep produces a duplicate publication of the previous slot. Harmless under the waiter contract, which only fails a waiter when a newer slot arrives, and it self-corrects at the next boundary. All other clock-fault behavior is unchanged from the previous code.

Validation

• New deterministic regression test drives the early wake under paused time by holding the wall clock short of the boundary when the timer fires; it fails with the offset removed (mutation-checked) and passes with it.
• Steady-state test asserts one publication per slot, strictly increasing.
• Full make test (including ssv-spec tests): 622 passed, 0 failed. fmt and clippy clean.
• Validated on a 4-operator, 100-validator ssv-mini devnet (same harness class that reproduced the race): in a 128-slot scored window under Boole, every node published exactly once per slot with strictly increasing slots, zero MetadataSlotPassed, and 120 of 123 produced blocks carried sync participation of 500+/512 on a finalizing chain. The two slots without publications coincided with a measured 34-second host-VM stall (all containers silent), not publisher behavior.

Rollback

• Revert the single commit. No config, database, or wire-format impact.

[shane-moore]

@claude, please take a look

[claude-code-actions-sigp]

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

---

Review: PR #1235 — publish VotingAssignments 50ms past slot boundary

• Read the changed file metadata_service.rs
• Compare against origin/unstable base
• Analyze the early-wake race fix and 50ms offset
• Review the run_slot_start_publisher helper and tests
• Trace consumers / waiter contract

---

Verdict: this is a clean, correctly-scoped fix. The diagnosis matches the code, the offset is the right mechanism, and the tests genuinely reproduce the race. No blocking issues found.

Correctness — the fix holds

• The core change (metadata_service.rs:219) sleeps duration_to_next_slot() + 50ms. Since duration_to_next_slot() is recomputed fresh each loop iteration, the offset does not accumulate: after a wake at boundary + 50ms, the next duration_to_next_slot() returns slot_duration - 50ms, so the following sleep is a full slot_duration and lands at next_boundary + 50ms. Steady state publishes each slot exactly once. ✅
• An early timer wake of up to 50ms still lands past the wall-clock boundary, so update_voting_assignments (metadata_service.rs:383) reads the new slot rather than republishing the previous one. ✅
• Waiter contract verified (lib.rs:884-904): get_voting_assignments blocks on wait_for(slot >= requested) and only returns MetadataSlotPassed when a strictly newer slot has been published. The 50ms simply becomes extra wait time for any consumer that raced ahead — it cannot turn into a passed-slot error. The duplicate-publication-on-large-backwards-clock-step case described in the PR is genuinely harmless under this contract. ✅

Consumer impact — 50ms is invisible

Traced every get_voting_assignments caller (lib.rs:2875, 2987, 3054): all sit behind the selection-proof path whose absolute deadline is 2/3 * slot_duration (lib.rs:2871, 2981). Phase 2 (VotingContext) only reads assignments after a head event or the ~1/3-slot fallback fires (metadata_service.rs:310-320), both far past 50ms. Nothing reads the data inside the first 50ms. ✅

Tests — reproduce the race well

The two start_paused tests (metadata_service.rs:2529, 2556) cleanly separate the virtual tokio timer (the monotonic sleep) from the ManualSlotClock (the wall clock read), which is exactly what makes the drift reproducible. early_wake_still_publishes_the_new_slot asserts the empty-publish state at the early wake and then the single TEST_SLOT + 1 publish after the offset elapses — a real mutation check on the + 50ms. AAA structure and naming are clear.

Minor, non-blocking observations

1. Error-branch clock source changed. The extracted helper's fallback (metadata_service.rs:223) now sleeps slot_clock.slot_duration(), whereas the old inline loop used the spec-derived slot_duration. These should be equal, so this is fine — just flagging the source-of-truth swap in case a future divergence between spec and slot-clock durations is ever possible.
2. Phase 2/3 still sleep exactly to the boundary (metadata_service.rs:308, and Phase 3 already carries a 2/3 offset at :364). Correctly out of scope here — Phase 2 re-reads slot_clock.now() while waiting for a head event, so a few ms of early drift is immaterial to it, and the PR body already scopes the proposer half to Lighthouse. No action needed; noting for completeness.

Note on verification

I reviewed by reading source; I could not execute cargo test / clippy in this environment (sandbox restrictions). The PR reports make test = 622 passed with fmt/clippy clean, which is consistent with what I read.
· branch fix/1223-metadata-early-wake

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.47%. Comparing base (82bac14) to head (42bf00b).

Files with missing lines	Patch %	Lines
anchor/validator_store/src/metadata_service.rs	85.71%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1235      +/-   ##
============================================
+ Coverage     65.42%   65.47%   +0.05%     
============================================
  Files           164      164              
  Lines         28075    28125      +50     
============================================
+ Hits          18367    18415      +48     
- Misses         9708     9710       +2     

Flag	Coverage Δ
rust	65.47% <85.71%> (+0.05%)	⬆️

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 On the error-branch clock source: the two durations are equal by construction. The production clock is built at client/src/lib.rs:338 as SystemTimeSlotClock::new(spec.genesis_slot, genesis_time, spec.get_slot_duration()), its only construction site, so slot_clock.slot_duration() and the spec value cannot diverge without that line changing. The helper reads the clock so it stays self-contained.

[claude-code-actions-sigp]

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

---

Re: error-branch clock source

Agreed — verified and correct. Grepping the whole workspace, SystemTimeSlotClock::new has exactly one call site (anchor/client/src/lib.rs:338), and it's constructed with spec.get_slot_duration():

let slot_clock = SystemTimeSlotClock::new(
    spec.genesis_slot,
    Duration::from_secs(genesis_time),
    spec.get_slot_duration(),
);

Every other SlotClock::new in the tree is a ManualSlotClock inside tests. So in production slot_clock.slot_duration() returns exactly the spec-derived value — the two cannot diverge without that line changing, and reading through the clock keeps run_slot_start_publisher self-contained rather than threading a separate slot_duration argument through it.

That was a non-blocking "source-of-truth swap" note in the first place, and your reasoning resolves it. No change needed. My review stands as approving.
· branch fix/1223-metadata-early-wake

[shane-moore]

ssv-mini validation results

Setup: 4 Anchor operators, 100 managed validators on a full 4-operator committee, Boole active, 12s slots, image built from 42bf00b.

Scored window of 128 slots (736-863):

• Every node published VotingAssignments exactly once per slot, slots strictly increasing, zero MetadataSlotPassed in the window.
• The Lighthouse proposer scheduler can skip an assigned slot after an early timer wake #1223 failure signature (previous-slot republish followed by a skipped slot) did not occur anywhere in the run.
• Downstream health on the path this issue was breaking: 123/128 slots had blocks and 120 of those carried sync participation of 500+/512, with finality advancing throughout.
• The two slots without publications coincided with a measured 34-second host-VM stall (all containers silent), an environment artifact rather than publisher behavior. On resume the publisher picked up at the current slot as designed.