Eagerly sign sync committee messages on head events

[shane-moore]

Proposed Changes

SyncCommitteeService currently waits for the sync message due point every slot, even when the head block arrived seconds earlier. Per the Altair honest validator spec, a sync committee member should produce the message "either when (a) the validator has received a valid block from the expected block proposer for the current slot or (b) get_sync_message_due_ms() milliseconds has transpired since the start of the slot", whichever comes first. This applies the head-event trigger that #7892 added for attestations to sync committee messages:

• sync_committee_service races a head event against its due timer. Head-triggered slots sign the event's block root (the head monitor only forwards non-optimistic heads) and publish shortly after block arrival instead of at the due point.
• Wires SYNC_MESSAGE_DUE_BPS_GLOAS = 2500 (Gloas time parameters) into ChainSpec with a fork-aware get_sync_message_due_at_slot, so the timer fallback tightens to 25% of the slot at Gloas.
• Converts head events from mpsc to tokio::broadcast with BeaconNodeFallback::subscribe_to_head_events() so both services can consume them. The shared head_event_or_deadline helper also fixes a busy-spin in the attestation service when the head event channel closes.
• Adds sync committee endpoints to MockBeaconNode and service tests on the shared VC harness.

Additional Info

N/A

AI Assistance Disclosure

Tools used: Claude Code Fable 5

[dapplion]

What drives the need for this PR? Any stakeholder needs more eager sync messages?

[dapplion]

🤖 Agent-assisted review (Claude Code). Five notes, ordered by how much they matter.

validator_client/validator_services/src/sync_committee_service.rs:266
The attestation path compares the head event root against the beacon node that sent it. This path does not. Is this deliberate?

validator_client/validator_services/src/sync_committee_service.rs:145
next_slot is always slot_of(now) + 1, so a slot that the loop is already inside gets no deadline. Only a head event covers it.

validator_client/validator_services/src/sync_committee_service.rs:193
spawn_contribution_tasks now returns Ok(()) on every path, so this crit! is unreachable and the latch advances for slots that signed nothing.

validator_client/validator_services/src/sync_committee_service.rs:794
100ms of real time is not enough for BLS signing and two HTTP requests, so this test can fail on a loaded runner.

consensus/types/src/core/chain_spec.rs:4068
CONTRIBUTION_DUE_BPS_GLOAS (5000) is still not connected, so at Gloas the client aggregates at 66.67% of the slot, not 50%.

[shane-moore]

Anchor motivated this PR. It combines attestation and sync message signing into one gossip batch. Because of that, an eager attestation waits for lh to start the sync message duty

This PR starts that duty from the same head event. anchor can then finish the batch and return the attestation sooner. This gives it more time to reach aggregators before 6 seconds

Altair spec compliance is a nice bonus too

[mergify]

Some required checks have failed. Could you please take a look @shane-moore? 🙏

[shane-moore]

Thanks, I went through each point:

1. This is deliberate. The attestation path makes a second request for AttestationData, so it verifies that response still matches the event root. The sync path needs only the root already supplied by the non-optimistic head event, so there is no second value to compare. Re-fetching the head would add a round trip without independently validating the event. No change.

2. Correct for the slot already in progress when the service starts. Every subsequent slot is armed as the next slot and receives its deadline, and the previous implementation had the same startup behavior. Covering the startup slot with a timer would change restart behavior because Lighthouse has no persistent record of whether it already sent a sync message for that slot. No change.

3. Good catch on the dead Result and crit!. Fixed in e1b9bb4. spawn_contribution_tasks now returns (), the unreachable critical branch is removed, the latch is named last_processed_slot, and the trace is emitted only after task submission. The latch intentionally records the scheduled head-or-deadline attempt, not confirmed signing. Reopening the slot would introduce a later same-slot retry policy, and retrying after task submission could duplicate signing.

4. Agreed, fixed in 257a738. The helper now polls for up to five seconds at 10 ms intervals and still returns as soon as the expected messages arrive.

5. Agreed that the Gloas contribution deadline is still unwired. I am keeping it out of this PR because it is a pre-existing, separate inner-slot timing path that requires ChainSpec/Config plumbing, validator-monitor updates, and its own tests. This PR remains scoped to eagerly triggering unaggregated sync messages. The broader timing work is tracked by Inner slot timing refactor for Gloas/EIP-7732 #8615.

[dapplion]

🤖 automated (dapplion's agent): second pass on 257a738. CI is fully green and cargo nextest run -p validator_services --lib is 37/37 locally — the new sync tests take ~23s each, but so does every other test in that binary, so that is ValidatorClientHarness construction cost rather than anything this PR added. Four notes.

1. get_sync_message_due_at_slot is wired on the VC side only. Three beacon node callers still use the fork-blind get_sync_message_due():

• beacon_node/beacon_chain/src/validator_monitor.rs:1539 — grades sync message delay against 33.33%, so post-Gloas it silently under-reports lateness.
• beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs:74 and light_client_finality_update_verification.rs:78 — these reject with Error::TooEarly when an update arrives before the due point. Post-Gloas the VC publishes at 25% while the receiving BN gate still expects 33.33%.

GLOAS_FORK_EPOCH is u64::MAX in every config so nothing breaks today, but this PR is what creates the split between the two values. Worth a tracking issue next to #8615 rather than leaving it implicit.

2. validator_client/validator_services/src/sync_committee_service.rs:142 — last_processed_slot starts at None, so the first head event processes the in-progress slot no matter how late it arrives. At ~10s into a 12s slot, delay_until_slot_offset(contribution_due) saturates to ZERO and the contribution task fires immediately, publishing a contribution well past its due point. Not harmful, but it produces useless traffic and error logs. It also means the startup slot is covered by head events, just badly, which cuts against the earlier reply on this point. The guard at :236 only catches a slot that has fully rolled over, not one whose contribution deadline has already passed.

3. sync_committee_service.rs:172 — a backwards clock step (NTP correction) makes current_slot <= last_processed_slot hold until real time catches back up, and the service signs nothing for that window. The previous loop fired unconditionally every slot, so this failure mode is new. Bounded and rare.

4. Nits. get_sync_message_due_at_slot departs from the existing ChainSpec::get_attestation_due::<E>(slot) naming for an identically shaped helper. sync_committee_service.rs:225 uses positional {} formatting where the rest of the file uses structured %slot. .unwrap_or(Duration::ZERO) at :211 conflates "slot expired" with "due now" — downstream handles it, but an explicit early return would read better.

Two things I checked and consider settled. The BN does not require beacon_block_root to name a known block (sync_committee_verification.rs:470-540), so publishing the event root to a BN that has not yet imported it is accepted — the earlier reply about there being no second value to cross-check holds, and the missing BN affinity relative to the attestation path does not matter here. And the mpsc→broadcast switch correctly removes head-of-line blocking in the head monitor; recv() is cancel safe, so the select! in head_event_or_deadline loses no events, and the Lagged branch is handled.

None of the above is blocking.

[mergify]

Some required checks have failed. Could you please take a look @shane-moore? 🙏

[shane-moore]

Thanks, went through each point:

1. Agreed, filed Use fork-aware sync message due point in beacon node callers #9784. The light client TooEarly gates are the behavior-relevant callers: the spec defines them via get_sync_message_due_ms(), which Gloas modifies, so post-Gloas we'd drop and downscore spec-compliant peers publishing between 25% and 33.33% of the slot. Left out of this PR since they're BN gossip validation with their own fork-boundary question, noted in the issue.

2. Fair correction to my earlier point 2: the head-event path does cover the startup slot, including arbitrarily late. Fixed in df5a158. spawn_contribution_tasks now skips a slot whose contribution deadline has already passed, so a late first head event no longer publishes un-aggregatable messages or fires the contribution task into crit! logs. Only the startup slot could reach this in practice, since in steady state the deadline timer processes each slot before a late head event can. Added late_head_event_past_contribution_deadline_is_skipped to cover it.

3. Agreed the failure mode is new, but I think the window it matters in is nearly empty: NTP daemons slew steady-state offsets and only step at boot (e.g. chrony makestep), and at boot the latch is fresh anyway, so there's rarely a latched slot to stall on. Re-signing across a step-back (the old behavior) also only pays off when the pre-step messages were invalid-early; in the other direction it just re-publishes messages BNs reject as non-current, trading a quiet gap for error noise. Given that, keeping the latch semantics matching latest_attested_slot in the attestation service seems right. No change.

4. Fixed the positional formatting and replaced the unwrap_or(Duration::ZERO) with an explicit expired-slot return in 09296ab. On naming: the short name is occupied by the fork-blind get_sync_message_due() the BN callers still use, so the rename folds into Use fork-aware sync message due point in beacon node callers #9784, where that getter gets removed.

[dapplion]

Weird because clock only fails on system failures, so permanent failure is better

[shane-moore]

I feel you on that, done in b095400

[michaelsproul]

it also fails pre-genesis, but I think we already wait for genesis in the attestation service/sync committee service. that consideration might just affect the comment

[shane-moore]

good point, fixed the wording in 367bb6f

[mergify]

This pull request has merge conflicts. Could you please resolve them @shane-moore? 🙏

[NikhilSharmaWe]

@shane-moore, the PR has some merge conflicts, just flagging it.