From db6138890799877f9182551743640baf7388fedc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:24:35 -0300 Subject: [PATCH] fix(sync): judge the sync gate against peers' Status heads A node reported itself synced while far behind (#559): the tracker took the freshest block the network knows from the node's own imported chain, which trails the head during a catch-up, so its network-stall override fired and the node attested and proposed on a stale head. The beacon follower had the same bug and reported synced through whole catch-ups. Follow Lighthouse and Prysm instead: a node is syncing only when its head trails the wall clock and the head its peers report in Status is ahead of its own. P2P keeps each connected peer's latest Status head (re-requested every few slots, dropped on disconnect, a head past the clock ignored) and publishes the second-highest of three or more on the Store. Thresholds are per chain: leanSpec's 4/2 on lean, Lighthouse's two epochs of clock lag and one epoch of peer lead on beacon. --- crates/blockchain/src/lib.rs | 90 +++++--- crates/blockchain/src/sync_status.rs | 286 ++++++++++++++++++------ crates/net/p2p/src/lib.rs | 73 ++++++ crates/net/p2p/src/peer_heads.rs | 216 ++++++++++++++++++ crates/net/p2p/src/req_resp/handlers.rs | 27 +++ crates/storage/src/store.rs | 42 ++++ docs/architecture.md | 16 +- docs/beacon_wire.md | 10 +- docs/rpc.md | 2 +- docs/spec_deviations.md | 39 ++++ 10 files changed, 689 insertions(+), 112 deletions(-) create mode 100644 crates/net/p2p/src/peer_heads.rs diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index 251048b7a..f35555eab 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -34,7 +34,7 @@ use crate::aggregation::{ PRIOR_WORKER_JOIN_TIMEOUT, aggregation_deadline, run_aggregation_worker, }; use crate::key_manager::ValidatorKeyPair; -use crate::sync_status::SyncStatusTracker; +use crate::sync_status::{SyncStatusTracker, SyncThresholds}; use spawned_concurrency::actor; use spawned_concurrency::error::ActorError; use spawned_concurrency::protocol; @@ -304,7 +304,7 @@ impl BlockChain { Self::start_actor( store, - SyncStatusTracker::new(gate_duties), + SyncStatusTracker::new(gate_duties, SyncThresholds::LEAN), sync_status_controller, events, ChainDuties::Lean(Box::new(lean)), @@ -322,7 +322,7 @@ impl BlockChain { /// [`ChainDuties::Beacon`]). Feeding it gossip and calling this from /// `run_node` are later slices; this constructor only builds the actor. /// - /// Uses [`SyncStatusTracker::new`]`(false)`: the tracker still drives the + /// Uses [`SyncStatusTracker::new`]`(false, ..)`: the tracker still drives the /// `lean_node_sync_status` metric (the name predates beacon support), but /// there are no duties on this arm for it to gate. /// @@ -356,7 +356,7 @@ impl BlockChain { Self::start_actor( store, - SyncStatusTracker::new(false), + SyncStatusTracker::new(false, SyncThresholds::BEACON), sync_status_controller, events, ChainDuties::Beacon, @@ -2018,7 +2018,7 @@ impl BlockChainServer { safe_slots_to_import_optimistically, last_tick_instant: None, epoch_precompute_in_flight: None, - sync_status: SyncStatusTracker::new(false), + sync_status: SyncStatusTracker::new(false, SyncThresholds::BEACON), sync_status_controller: SyncStatusController::default(), events: ChainEvents::new(EventBus::default(), &store), duties: ChainDuties::Beacon, @@ -3584,18 +3584,16 @@ impl BlockChainServer { /// /// Reads the head through [`Self::head_slot`], which is where the /// per-chain part of that lives (lean's `Store::head_slot` and beacon's - /// `Store::beacon_head` decode different tables). The tracker itself is - /// chain-agnostic, but the freshest block seen is not: on a beacon - /// follower, once the clock is past a fork the node does not follow (see - /// [`Self::clock_is_past_followed_forks`]) the network is treated as fresh. + /// `Store::beacon_head` decode different tables), and the network's head + /// from [`Store::network_head`], the head the node's peers report in + /// `Status`. The tracker itself is chain-agnostic, with per-chain + /// thresholds; see [`SyncThresholds`]. On a beacon follower, once the clock + /// is past a fork the node does not follow (see + /// [`Self::clock_is_past_followed_forks`]) the network is treated as being + /// at the clock, since no peer on that fork can tell it so. fn update_sync_status(&mut self, current_slot: u64) { let head_slot = self.head_slot(); - let live_max_slot = self - .store - .max_live_chain_slot() - .expect("max live chain slot exists") - .unwrap_or(head_slot); - let max_seen_slot = match self.store.chain() { + let network_head = match self.store.chain() { Chain::Beacon if Self::clock_is_past_followed_forks( &self.store.config(), @@ -3603,13 +3601,13 @@ impl BlockChainServer { head_slot, ) => { - current_slot + Some(current_slot) } - Chain::Beacon | Chain::Lean => live_max_slot, + Chain::Beacon | Chain::Lean => self.store.network_head(), }; let status = self .sync_status - .update(current_slot, head_slot, max_seen_slot); + .update(current_slot, head_slot, network_head); metrics::set_node_sync_status(status); self.sync_status_controller.set(status); } @@ -3620,16 +3618,17 @@ impl BlockChainServer { /// Every fork is followed today, so this is false; it stays as the guard /// for the next fork the node cannot follow. /// - /// Blocks past the fork do not reach the live chain from the network, so the freshest block - /// the store knows stays at the head and the sync tracker would read the - /// network as stalled `NETWORK_STALL_THRESHOLD` slots after the fork and - /// report `Synced` for good on a node that follows nothing. Nothing else - /// tells the node the network is ahead: gossip blocks past the fork are - /// ignored in p2p. The fork digest rolls over at runtime, so a node that - /// runs across a fork it can follow switches topics and Status digests on - /// its own; for a fork it cannot follow that switch brings nothing in. The - /// clock is the one signal that is always there, and treating the network - /// as fresh lets head lag decide. + /// Past such a fork the peers that moved on answer `Status` from a digest + /// this node cannot sync from, so none of their heads is recorded (see the + /// Status handlers in p2p), and the peers left behind report heads as + /// stale as ours. The sync tracker would read that silence as a stalled + /// network and report `Synced` for good on a node that follows nothing. + /// Nothing else tells the node the network is ahead: gossip blocks past the + /// fork are ignored in p2p. The fork digest rolls over at runtime, so a node + /// that runs across a fork it can follow switches topics and Status + /// digests on its own; for a fork it cannot follow that switch brings + /// nothing in. The clock is the one signal that is always there, and + /// treating the network as being at the clock lets head lag decide. fn clock_is_past_followed_forks(config: &Config, current_slot: u64, head_slot: u64) -> bool { let fork_at = |slot: u64| config.fork_at_epoch(compute_epoch_at_slot(slot)); !fork_at(current_slot).is_followed() && fork_at(head_slot).is_followed() @@ -4490,7 +4489,7 @@ mod tests { safe_slots_to_import_optimistically: constants::SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY, last_tick_instant: None, epoch_precompute_in_flight: None, - sync_status: SyncStatusTracker::new(false), + sync_status: SyncStatusTracker::new(false, SyncThresholds::BEACON), sync_status_controller: SyncStatusController::default(), duties: ChainDuties::Beacon, } @@ -5277,15 +5276,18 @@ mod tests { beacon_server(beacon_store_with_config(GENESIS_TIME, 0, config)) } + /// Three epochs in: past the beacon tracker's two-epoch clock tolerance + /// with the head still at the anchor (slot 0). + const THREE_EPOCHS: u64 = 3 * preset::SLOTS_PER_EPOCH; + #[test] fn the_clock_reaching_a_followed_fork_does_not_force_syncing() { - // Gloas is followed, so blocks past the fork do reach the live chain - // and the tracker's own stall detection applies as before: with no - // block at all it reads a stalled network as synced, the same as when - // no fork is scheduled. + // Gloas is followed, so the fork rule stays out of it and the peers + // decide: every peer as stale as we are is a stalled network, read as + // synced, the same as when no fork is scheduled. let mut server = syncing_server(Some(1)); - let past_the_fork = preset::SLOTS_PER_EPOCH + 8; - server.update_sync_status(past_the_fork); + server.store.set_network_head(Some(0)); + server.update_sync_status(THREE_EPOCHS); assert_eq!( server.sync_status_controller.get(), crate::metrics::SyncStatus::Synced @@ -5297,13 +5299,29 @@ mod tests { // The same silence with no gloas scheduled is a stalled network, which // the tracker deliberately reports as synced. let mut server = syncing_server(None); - server.update_sync_status(preset::SLOTS_PER_EPOCH + 8); + server.store.set_network_head(Some(0)); + server.update_sync_status(THREE_EPOCHS); assert_eq!( server.sync_status_controller.get(), crate::metrics::SyncStatus::Synced ); } + #[test] + fn a_follower_behind_its_peers_reads_as_syncing() { + // The mainnet follower `docs/beacon_wire.md` describes: catching up, + // two epochs and more behind the clock, with peers at the clock. It + // used to read synced, since the freshest block it knew of was its own + // head. + let mut server = syncing_server(None); + server.store.set_network_head(Some(THREE_EPOCHS)); + server.update_sync_status(THREE_EPOCHS); + assert_eq!( + server.sync_status_controller.get(), + crate::metrics::SyncStatus::Syncing + ); + } + /// Gloas at epoch 1, so slot `SLOTS_PER_EPOCH` is the first gloas slot. fn gloas_at_epoch_one() -> Config { Config::mainnet().with_fork_epoch(ForkName::Gloas, 1) diff --git a/crates/blockchain/src/sync_status.rs b/crates/blockchain/src/sync_status.rs index 27dff999b..52a04d5fd 100644 --- a/crates/blockchain/src/sync_status.rs +++ b/crates/blockchain/src/sync_status.rs @@ -1,7 +1,8 @@ use std::sync::Arc; use std::sync::atomic::{AtomicU8, Ordering}; -use tracing::debug; +use ethlambda_types::beacon::preset::SLOTS_PER_EPOCH; +use tracing::info; use crate::metrics::SyncStatus; @@ -44,17 +45,70 @@ impl Default for SyncStatusController { } } -/// Local head lag beyond which the node is considered to be syncing. +/// How far behind a node may fall before it reports itself syncing, per chain. /// -/// See: leanSpec PR #708. -const SYNC_LAG_THRESHOLD: u64 = 4; -/// Freshest-known block lag beyond which the network is considered stalled. +/// The node is syncing only when **both** of these hold: /// -/// During a network-wide stall the node remains synced so validators can help -/// the chain recover. -const NETWORK_STALL_THRESHOLD: u64 = 8; -/// Recovery band that prevents the sync status from flapping near the threshold. -const SYNC_HYSTERESIS_BAND: u64 = 2; +/// - its head trails the wall clock by more than [`Self::clock_lag`], which is +/// local evidence no peer can forge, and +/// - the head its peers report (see `Store::network_head`) is more than +/// [`Self::peer_lead`] slots ahead of its own. +/// +/// This is Lighthouse's and Prysm's shape. Lighthouse serves duties unless a +/// range sync is running *and* the head is over `sync_tolerance_epochs` (2) +/// behind the clock, and only a peer more than 32 slots ahead starts a range +/// sync. Prysm resyncs only when the head is two epochs behind the clock *and* +/// peers report a head two epochs ahead. Peers decide between "the network is +/// ahead of me" and "the network is stalled"; the clock keeps a peer's claim +/// from idling a node whose own head is current. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct SyncThresholds { + /// Slots the head may trail the wall clock before the node can be syncing. + pub(crate) clock_lag: u64, + /// Slots the peers' head must lead ours before the node can be syncing. + pub(crate) peer_lead: u64, + /// Recovery band: once syncing, both lags must drop this many slots below + /// their thresholds before the node reports synced again, so the status + /// does not flap at the boundary. + pub(crate) hysteresis: u64, + /// Slots after the first tick during which, with no peer having reported a + /// head yet, the node assumes the network is at the wall clock. + /// + /// The first tick runs before any peer has answered a `Status`, so without + /// this a node resuming a stale database would read "no peer is ahead" as a + /// stalled network and do its duties on that stale head until the first + /// handshake lands. Prysm holds a starting node in initial sync until it + /// has peers; this bounds that hold, so a node that never finds a peer (a + /// single-node devnet, or one cut off from the start) still falls back to + /// treating the silence as a stall. + pub(crate) startup_grace: u64, +} + +impl SyncThresholds { + /// Lean consensus: leanSpec PR #708's lag threshold and hysteresis band, + /// applied to both conditions, with the stall override replaced by the peer + /// lead (see `docs/spec_deviations.md`). Lean has no epochs, so neither + /// client's epoch-sized tolerance carries over. + pub(crate) const LEAN: Self = Self { + clock_lag: 4, + peer_lead: 4, + hysteresis: 2, + startup_grace: 8, + }; + + /// The Beacon Chain: Lighthouse's numbers. Two epochs of clock lag + /// (`DEFAULT_SYNC_TOLERANCE_EPOCHS`, Prysm's `shouldReSync` too) and a peer + /// lead of one epoch (`SLOT_IMPORT_TOLERANCE`, the lead that makes a + /// Lighthouse peer `Advanced`). Neither client has a hysteresis band. The + /// startup hold is an epoch, since finding beacon peers through discovery + /// can take a while; Prysm holds until it has peers, however long. + pub(crate) const BEACON: Self = Self { + clock_lag: 2 * SLOTS_PER_EPOCH, + peer_lead: SLOTS_PER_EPOCH, + hysteresis: 0, + startup_grace: SLOTS_PER_EPOCH, + }; +} pub(crate) struct SyncStatusTracker { syncing: bool, @@ -65,51 +119,68 @@ pub(crate) struct SyncStatusTracker { /// returns `true`: the gate is observe-only. Seeded from the CLI /// `--disable-duty-sync-gate` flag (gating stays on by default). gate_duties: bool, -} - -impl Default for SyncStatusTracker { - fn default() -> Self { - Self { - syncing: false, - gate_duties: true, - } - } + thresholds: SyncThresholds, + /// The slot at which the startup hold ends; set by the first update. + startup_ends_at: Option, + /// Whether any peer has reported a head yet. Once one has, losing every + /// peer reads as a stall, not as startup. + heard_from_peers: bool, } impl SyncStatusTracker { /// Build a tracker, choosing whether the syncing state gates duties. - pub(crate) fn new(gate_duties: bool) -> Self { + pub(crate) fn new(gate_duties: bool, thresholds: SyncThresholds) -> Self { Self { + syncing: false, gate_duties, - ..Self::default() + thresholds, + startup_ends_at: None, + heard_from_peers: false, } } + /// Re-judge the status at `current_slot` (the wall clock's slot). + /// + /// `network_head` is the head slot the node's peers report, or `None` when + /// no connected peer has reported one. With no report the node cannot be + /// behind anyone, so it reads as synced (a stalled or empty network, where + /// its validators should keep working), except during the startup hold. pub(crate) fn update( &mut self, current_slot: u64, head_slot: u64, - max_seen_slot: u64, + network_head: Option, ) -> SyncStatus { - let head_lag = current_slot.saturating_sub(head_slot); - let network_lag = current_slot.saturating_sub(max_seen_slot); + let SyncThresholds { + clock_lag: clock_threshold, + peer_lead: lead_threshold, + hysteresis, + startup_grace, + } = self.thresholds; + let startup_ends_at = *self + .startup_ends_at + .get_or_insert(current_slot.saturating_add(startup_grace)); + self.heard_from_peers |= network_head.is_some(); + let starting_up = !self.heard_from_peers && current_slot < startup_ends_at; + + // During the startup hold, assume the network is at the clock. + let network_head = network_head.or(starting_up.then_some(current_slot)); + let clock_lag = current_slot.saturating_sub(head_slot); + let peer_lead = network_head.map_or(0, |slot| slot.saturating_sub(head_slot)); + let band = if self.syncing { hysteresis } else { 0 }; let was_syncing = self.syncing; - if network_lag > NETWORK_STALL_THRESHOLD { - self.syncing = false; - } else if self.syncing { - self.syncing = head_lag > SYNC_LAG_THRESHOLD.saturating_sub(SYNC_HYSTERESIS_BAND); - } else { - self.syncing = head_lag > SYNC_LAG_THRESHOLD; - } + self.syncing = clock_lag > clock_threshold.saturating_sub(band) + && peer_lead > lead_threshold.saturating_sub(band); if self.syncing != was_syncing { - debug!( + info!( current_slot, head_slot, - max_seen_slot, - head_lag, - network_lag, + network_head, + clock_lag, + peer_lead, + starting_up, syncing = self.syncing, "Sync status changed" ); @@ -138,61 +209,146 @@ impl SyncStatusTracker { mod tests { use super::*; + const LEAN: SyncThresholds = SyncThresholds::LEAN; + + /// A lean tracker past its startup hold, with peers heard from, so each + /// test exercises the steady-state rule unless it says otherwise. + fn steady_lean() -> SyncStatusTracker { + let mut tracker = SyncStatusTracker::new(true, LEAN); + tracker.update(0, 0, Some(0)); + tracker + } + #[test] - fn sync_status_allows_lag_through_threshold() { - let mut tracker = SyncStatusTracker::default(); + fn behind_both_the_clock_and_the_peers_is_syncing() { + let mut tracker = steady_lean(); + // Issue #559: the head is 550 slots back and peers are at the clock. + assert_eq!(tracker.update(600, 50, Some(600)), SyncStatus::Syncing); + assert!(!tracker.duties_allowed()); + } - for lag in 0..=SYNC_LAG_THRESHOLD { - assert_eq!(tracker.update(10 + lag, 10, 10 + lag), SyncStatus::Synced); + #[test] + fn lag_up_to_the_threshold_is_synced() { + let mut tracker = steady_lean(); + for lag in 0..=LEAN.clock_lag { + let current = 10 + lag; + assert_eq!( + tracker.update(current, 10, Some(current)), + SyncStatus::Synced + ); } - - let first_syncing_slot = 10 + SYNC_LAG_THRESHOLD + 1; + let current = 10 + LEAN.clock_lag + 1; assert_eq!( - tracker.update(first_syncing_slot, 10, first_syncing_slot), + tracker.update(current, 10, Some(current)), SyncStatus::Syncing ); } #[test] - fn sync_status_detects_local_lag_when_fresh_blocks_are_known() { - let mut tracker = SyncStatusTracker::default(); - let current_slot = 10 + SYNC_LAG_THRESHOLD + 1; + fn a_stalled_network_keeps_duties_on_however_long_it_lasts() { + // Nobody is ahead of us: the network is not producing, so validators + // keep working to restart it, at any clock lag. leanSpec's + // `test_network_wide_stall_keeps_duties_live`. + let mut tracker = steady_lean(); + for current in [10, 50, 1_000] { + assert_eq!(tracker.update(current, 0, Some(0)), SyncStatus::Synced); + } + } - assert_eq!( - tracker.update(current_slot, 10, current_slot), - SyncStatus::Syncing - ); + #[test] + fn missed_proposals_do_not_close_the_gate() { + // leanSpec #1194: several empty slots in a row push the clock lag past + // the threshold on every honest node, but no peer is ahead of any of + // them, so none stops working. + let mut tracker = steady_lean(); + assert_eq!(tracker.update(16, 10, Some(10)), SyncStatus::Synced); + assert_eq!(tracker.update(16, 10, Some(11)), SyncStatus::Synced); + } + + #[test] + fn a_peer_ahead_cannot_idle_a_node_at_the_clock() { + // The clock condition is what bounds a lying peer: our own head is + // current, so no claimed lead turns duties off. + let mut tracker = steady_lean(); + assert_eq!(tracker.update(100, 98, Some(500)), SyncStatus::Synced); } #[test] - fn sync_status_treats_stale_known_blocks_as_network_stall() { - let mut tracker = SyncStatusTracker::default(); + fn losing_every_peer_reads_as_a_stall() { + let mut tracker = steady_lean(); + assert_eq!(tracker.update(100, 50, Some(100)), SyncStatus::Syncing); + // Cut off: no peer reports anything any more. + assert_eq!(tracker.update(120, 50, None), SyncStatus::Synced); + // Back in touch, still behind. + assert_eq!(tracker.update(130, 50, Some(130)), SyncStatus::Syncing); + } - assert_eq!(tracker.update(100, 0, 0), SyncStatus::Synced); + #[test] + fn startup_holds_a_stale_node_until_a_peer_reports() { + let mut tracker = SyncStatusTracker::new(true, LEAN); + // Resumed 300 slots behind, nobody heard from yet. + assert_eq!(tracker.update(300, 0, None), SyncStatus::Syncing); + // The first handshake says the network is with us after all. + assert_eq!(tracker.update(301, 0, Some(0)), SyncStatus::Synced); } #[test] - fn sync_status_hysteresis_prevents_flapping() { - let mut tracker = SyncStatusTracker::default(); + fn the_startup_hold_ends_without_peers() { + // A node that never finds a peer stops holding after the grace period + // and treats the silence as a stall, so a single node still works. + let mut tracker = SyncStatusTracker::new(true, LEAN); + assert_eq!(tracker.update(300, 0, None), SyncStatus::Syncing); + let after_grace = 300 + LEAN.startup_grace; + assert_eq!(tracker.update(after_grace, 0, None), SyncStatus::Synced); + } - assert_eq!(tracker.update(15, 10, 15), SyncStatus::Syncing); - assert_eq!(tracker.update(15, 11, 15), SyncStatus::Syncing); - assert_eq!(tracker.update(15, 10, 15), SyncStatus::Syncing); - assert_eq!(tracker.update(15, 13, 15), SyncStatus::Synced); + #[test] + fn a_current_node_is_synced_from_the_first_tick() { + let mut tracker = SyncStatusTracker::new(true, LEAN); + assert_eq!(tracker.update(300, 299, None), SyncStatus::Synced); } #[test] - fn network_stall_reopens_sync_status() { - let mut tracker = SyncStatusTracker::default(); + fn hysteresis_prevents_flapping() { + let mut tracker = steady_lean(); + assert_eq!(tracker.update(15, 10, Some(15)), SyncStatus::Syncing); + // Inside the band on both counts: still syncing. + assert_eq!(tracker.update(15, 12, Some(15)), SyncStatus::Syncing); + // Below the band on the clock: synced. + assert_eq!(tracker.update(15, 13, Some(15)), SyncStatus::Synced); + } - assert_eq!(tracker.update(20, 0, 20), SyncStatus::Syncing); - assert_eq!(tracker.update(30, 0, 20), SyncStatus::Synced); + #[test] + fn the_beacon_thresholds_are_lighthouses() { + let mut tracker = SyncStatusTracker::new(true, SyncThresholds::BEACON); + tracker.update(0, 0, Some(0)); + let epoch = SLOTS_PER_EPOCH; + // A checkpoint-synced follower 100 slots back with peers at the clock. + assert_eq!(tracker.update(1_000, 900, Some(1_000)), SyncStatus::Syncing); + // Within two epochs of the clock, Lighthouse serves duties. + assert_eq!( + tracker.update(1_000, 1_000 - 2 * epoch, Some(1_000)), + SyncStatus::Synced + ); + // Peers within an epoch of us are not `Advanced`. + assert_eq!( + tracker.update(1_000, 900, Some(900 + epoch)), + SyncStatus::Synced + ); } #[test] - fn future_head_saturates_lag_at_zero() { - let mut tracker = SyncStatusTracker::default(); + fn a_disabled_gate_still_tracks_but_never_blocks() { + let mut tracker = SyncStatusTracker::new(false, LEAN); + tracker.update(0, 0, Some(0)); + assert_eq!(tracker.update(600, 50, Some(600)), SyncStatus::Syncing); + assert!(tracker.is_syncing()); + assert!(tracker.duties_allowed()); + } - assert_eq!(tracker.update(15, 20, 20), SyncStatus::Synced); + #[test] + fn a_future_head_saturates_at_zero() { + let mut tracker = steady_lean(); + assert_eq!(tracker.update(15, 20, Some(20)), SyncStatus::Synced); } } diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index cc9ca2bd1..7febf4b54 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -115,6 +115,7 @@ pub mod discovery; mod gossipsub; pub mod lean; pub mod metrics; +mod peer_heads; mod req_resp; pub(crate) mod swarm_adapter; @@ -1212,6 +1213,7 @@ impl P2P { .beacon() .map(|beacon| beacon::scoring::PeerScoring::new(&beacon.config, beacon.fork_digest)); let local_metadata = built.wire.beacon().map(|beacon| beacon.metadata.clone()); + let first_status_refresh = status_refresh_interval(&store); let server = P2PServer { swarm_handle, @@ -1226,6 +1228,7 @@ impl P2P { outbound_requests: HashMap::new(), range_sync_state: None, beacon_fetched_through, + peer_heads: Default::default(), bootnode_addrs: built.bootnode_addrs, node_names, discovery: discovery.map(|handle| DiscoveryState::new(handle, built.local_peer_id)), @@ -1283,6 +1286,13 @@ impl P2P { handle.context(), p2p_protocol::LeaveExpiredAggregatorSubnets, ); + // The handshake covers a new peer, so the first refresh waits a full + // interval. + send_after( + first_status_refresh, + handle.context(), + p2p_protocol::RefreshPeerStatus, + ); // The dial loop's first tick. Nothing else schedules one, so without // discovery the loop never runs. if discovery_enabled { @@ -1384,6 +1394,9 @@ pub struct P2PServer { /// whether or not it has been imported yet. Gossip never advances it, so /// range sync reads it beside the store's head; see `beacon_sync_target`. pub(crate) beacon_fetched_through: u64, + /// Every connected peer's latest `Status` head, aggregated onto + /// `Store::network_head` for the sync tracker. See [`peer_heads`]. + pub(crate) peer_heads: peer_heads::PeerHeads, bootnode_addrs: HashMap>, node_names: HashMap, @@ -1561,6 +1574,9 @@ pub(crate) trait P2PProtocol: Send + Sync { fn advance_fork_schedule(&self) -> Result<(), ActorError>; #[allow(dead_code)] // invoked via send_after, not called directly fn refresh_peer_scoring(&self) -> Result<(), ActorError>; + /// Re-ask every connected peer for its `Status`; see [`peer_heads`]. + #[allow(dead_code)] // invoked via send_after, not called directly + fn refresh_peer_status(&self) -> Result<(), ActorError>; } #[actor(protocol = P2PProtocol)] @@ -1734,6 +1750,27 @@ impl P2PServer { beacon::scoring::refresh(self); } + /// Re-ask every connected peer for its `Status`, so the head each one + /// reported at the handshake does not go stale while it stays connected. + /// Both chains; see [`peer_heads`] for the pace. + #[send_handler] + async fn handle_refresh_peer_status( + &mut self, + _msg: p2p_protocol::RefreshPeerStatus, + ctx: &Context, + ) { + send_after( + status_refresh_interval(&self.store), + ctx.clone(), + p2p_protocol::RefreshPeerStatus, + ); + let peers: Vec = self.connected_peers.keys().copied().collect(); + for peer in peers { + self.peer_heads.begin_refresh(peer); + request_status(self, peer).await; + } + } + /// The deadline of a range batch held back for custody. Scheduled once, /// when the batch starts waiting; see [`RANGE_BATCH_CUSTODY_WAIT`]. #[send_handler] @@ -1940,6 +1977,40 @@ async fn fetch_missing_columns(server: &mut P2PServer, block_root: H256, columns // --- Manual Handler for swarm events --- +/// How often to re-ask every connected peer for its `Status`; see +/// [`peer_heads`] for the pace on each chain. +fn status_refresh_interval(store: &Store) -> Duration { + let slots = match store.chain() { + Chain::Lean => peer_heads::STATUS_REFRESH_SLOTS_LEAN, + Chain::Beacon => peer_heads::STATUS_REFRESH_SLOTS_BEACON, + }; + Duration::from_millis(store.config().slot_duration_ms.saturating_mul(slots)) +} + +/// Send `peer` our `Status`, asking for its own: the periodic refresh. The +/// handshake in `ConnectionEstablished` sends the same request, with its own +/// logging around it. +async fn request_status(server: &P2PServer, peer: PeerId) { + // Built before the send, so no borrow of `server.wire` is alive across it. + let beacon_status = server.wire.beacon().map(|wire| { + beacon::handler::build_status(&server.store, wire, beacon::handler::StatusVersion::V1) + }); + match beacon_status { + Some(status) => beacon::handler::send_status(server, peer, status).await, + None => { + let our_status = build_status(&server.store); + server + .swarm_handle + .send_request( + peer, + Request::LeanStatus(our_status), + ReqRespProtocol::LeanStatus, + ) + .await; + } + } +} + impl Handler for P2PServer { async fn handle(&mut self, msg: WrappedSwarmEvent, ctx: &Context) { handle_swarm_event(self, msg.0, ctx).await; @@ -2072,6 +2143,7 @@ async fn handle_swarm_event( .remove(&peer_id) .unwrap_or(closed_direction); forget_discovered_peer(server, &peer_id); + peer_heads::forget(server, &peer_id); server.refresh_peer_metrics(); let peer_count = server.connected_peers.len(); metrics::notify_peer_disconnected( @@ -3042,6 +3114,7 @@ pub(crate) mod test_support { outbound_requests: HashMap::new(), range_sync_state: None, beacon_fetched_through: 0, + peer_heads: Default::default(), bootnode_addrs: HashMap::new(), node_names: HashMap::new(), discovery: Some(crate::discovery::dial::DiscoveryState::new( diff --git a/crates/net/p2p/src/peer_heads.rs b/crates/net/p2p/src/peer_heads.rs new file mode 100644 index 000000000..4700d6588 --- /dev/null +++ b/crates/net/p2p/src/peer_heads.rs @@ -0,0 +1,216 @@ +//! The head slot this node's peers report in `Status`, published on the +//! [`Store`](ethlambda_storage::Store) as the sync tracker's evidence that the +//! network is ahead of this node. +//! +//! Lighthouse and Prysm both decide "I am behind" against what their peers +//! report rather than against blocks they have imported: a node whose import +//! has stalled, or that is still catching up, has imported nothing recent, so +//! its own blocks cannot tell it the network moved on. A peer's head is a +//! claim, not a verified block, so it is bounded three ways: a head past the +//! wall clock is dropped (both clients refuse such a `Status` at the +//! handshake), several peers must agree before the value moves, and the sync +//! tracker also requires the node's own head to trail the clock, so no claim +//! can stop a node that is current. +//! +//! Peers send `Status` once per connection, so a claim would go stale while +//! the peer stays connected. [`STATUS_REFRESH_SLOTS_LEAN`] and +//! [`STATUS_REFRESH_SLOTS_BEACON`] pace a re-request, the way Lighthouse +//! re-statuses every 300 s and Prysm every half epoch. + +use std::collections::{HashMap, HashSet}; + +use ethlambda_types::beacon::preset::SLOTS_PER_EPOCH; +use ethlambda_types::time::unix_now_ms; +use libp2p::PeerId; +use tracing::debug; + +use crate::P2PServer; + +/// From this many reporting peers on, the network head is the second-highest +/// report rather than the highest, so one peer alone cannot move it. Below +/// it the highest report counts: with one or two peers, a stale or still +/// syncing peer must not veto a fresher one, and the tracker's clock condition +/// already keeps a lone liar from idling a node that is current. +pub(crate) const PEER_HEAD_QUORUM: usize = 3; + +/// Slots a `Status` head may sit past the wall clock and still count: Lighthouse's +/// `FUTURE_SLOT_TOLERANCE` and Prysm's `maxFutureStatusHeadSlot`. +pub(crate) const FUTURE_HEAD_TOLERANCE: u64 = 1; + +/// How often a lean node re-asks every connected peer for its `Status`: every +/// lead the lean sync tracker tolerates (`SyncThresholds::LEAN.peer_lead`), so +/// a claim is never staler than the gap it is meant to detect. +pub(crate) const STATUS_REFRESH_SLOTS_LEAN: u64 = 4; + +/// How often a beacon node re-asks every connected peer for its `Status`: +/// Prysm's half epoch (`maintainPeerStatuses`). +pub(crate) const STATUS_REFRESH_SLOTS_BEACON: u64 = SLOTS_PER_EPOCH / 2; + +/// How far ahead a peer answering a *refresh* must be before its answer starts +/// a range sync, per chain. A handshake answer starts one at any lead, as it +/// always has; a refresh lands every few slots on a node that is keeping up +/// with gossip, where a peer one slot ahead is ordinary and a one-block range +/// request would only race gossip. Lighthouse makes a peer `Advanced`, the one +/// kind its range sync follows, only beyond `SLOT_IMPORT_TOLERANCE` (32). +pub(crate) const REFRESH_RANGE_SYNC_LEAD_LEAN: u64 = 4; +/// The beacon counterpart of [`REFRESH_RANGE_SYNC_LEAD_LEAN`]. +pub(crate) const REFRESH_RANGE_SYNC_LEAD_BEACON: u64 = SLOTS_PER_EPOCH; + +/// Every connected peer's latest `Status` head. +#[derive(Debug, Default)] +pub(crate) struct PeerHeads { + /// The head slot each peer last reported, while it stays connected. + heads: HashMap, + /// Peers whose outstanding `Status` request is a refresh rather than the + /// handshake; see [`REFRESH_RANGE_SYNC_LEAD_LEAN`]. + refreshing: HashSet, +} + +impl PeerHeads { + /// Record `peer`'s head, unless it sits past `wall_slot` by more than + /// [`FUTURE_HEAD_TOLERANCE`]; a peer whose head is dropped loses any + /// earlier report too, since a peer that claims the future is not one to + /// believe about the present. Returns whether the head was recorded. + pub(crate) fn record(&mut self, peer: PeerId, head_slot: u64, wall_slot: u64) -> bool { + if head_slot > wall_slot.saturating_add(FUTURE_HEAD_TOLERANCE) { + self.heads.remove(&peer); + return false; + } + self.heads.insert(peer, head_slot); + true + } + + /// Forget a disconnected peer. + pub(crate) fn forget(&mut self, peer: &PeerId) { + self.heads.remove(peer); + self.refreshing.remove(peer); + } + + /// The network's head as the peers report it: the second-highest report + /// once [`PEER_HEAD_QUORUM`] peers have reported, the highest before that, + /// `None` with no report at all. + pub(crate) fn network_head(&self) -> Option { + let mut heads: Vec = self.heads.values().copied().collect(); + heads.sort_unstable_by(|a, b| b.cmp(a)); + let rank = if heads.len() >= PEER_HEAD_QUORUM { + 1 + } else { + 0 + }; + heads.get(rank).copied() + } + + /// Note that the `Status` request about to go to `peer` is a refresh. + pub(crate) fn begin_refresh(&mut self, peer: PeerId) { + self.refreshing.insert(peer); + } + + /// Whether `peer`'s `Status` answer is to a refresh, clearing the mark. + pub(crate) fn take_refresh(&mut self, peer: &PeerId) -> bool { + self.refreshing.remove(peer) + } +} + +/// The slot the wall clock is in, on the store's time grid. +fn wall_clock_slot(server: &P2PServer) -> u64 { + let grid = server.store.config().time_grid(); + unix_now_ms().saturating_sub(grid.genesis_time_ms()) / grid.milliseconds_per_slot +} + +/// Record the head `peer` reported in a `Status` (request or answer) and +/// republish the network head. +pub(crate) fn observe(server: &mut P2PServer, peer: PeerId, head_slot: u64) { + let wall_slot = wall_clock_slot(server); + if !server.peer_heads.record(peer, head_slot, wall_slot) { + debug!( + %peer, + peer_head_slot = head_slot, + wall_slot, + "Ignoring a Status head past the wall clock" + ); + } + publish(server); +} + +/// Forget a disconnected peer's head and republish the network head. +pub(crate) fn forget(server: &mut P2PServer, peer: &PeerId) { + server.peer_heads.forget(peer); + publish(server); +} + +fn publish(server: &P2PServer) { + server + .store + .set_network_head(server.peer_heads.network_head()); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn no_report_is_no_network_head() { + assert_eq!(PeerHeads::default().network_head(), None); + } + + #[test] + fn below_quorum_the_highest_report_counts() { + // Two peers, one of them stale: the fresher one is not vetoed. + let mut heads = PeerHeads::default(); + heads.record(PeerId::random(), 100, 100); + heads.record(PeerId::random(), 40, 100); + assert_eq!(heads.network_head(), Some(100)); + } + + #[test] + fn at_quorum_one_peer_cannot_move_the_head() { + let mut heads = PeerHeads::default(); + heads.record(PeerId::random(), 100, 100); + heads.record(PeerId::random(), 50, 100); + heads.record(PeerId::random(), 50, 100); + assert_eq!(heads.network_head(), Some(50)); + // A second peer at the clock is enough. + heads.record(PeerId::random(), 100, 100); + assert_eq!(heads.network_head(), Some(100)); + } + + #[test] + fn a_head_past_the_clock_is_dropped_with_the_peers_earlier_report() { + let mut heads = PeerHeads::default(); + let liar = PeerId::random(); + assert!(heads.record(liar, 90, 100)); + assert!(heads.record(liar, 100 + FUTURE_HEAD_TOLERANCE, 100)); + assert!(!heads.record(liar, 100 + FUTURE_HEAD_TOLERANCE + 1, 100)); + assert_eq!(heads.network_head(), None); + } + + #[test] + fn a_later_report_replaces_an_earlier_one() { + // Latest, not highest: a peer that reorged to a lower head is believed. + let mut heads = PeerHeads::default(); + let peer = PeerId::random(); + heads.record(peer, 100, 100); + heads.record(peer, 95, 100); + assert_eq!(heads.network_head(), Some(95)); + } + + #[test] + fn a_disconnected_peer_is_forgotten() { + let mut heads = PeerHeads::default(); + let peer = PeerId::random(); + heads.record(peer, 100, 100); + heads.begin_refresh(peer); + heads.forget(&peer); + assert_eq!(heads.network_head(), None); + assert!(!heads.take_refresh(&peer)); + } + + #[test] + fn a_refresh_mark_is_taken_once() { + let mut heads = PeerHeads::default(); + let peer = PeerId::random(); + heads.begin_refresh(peer); + assert!(heads.take_refresh(&peer)); + assert!(!heads.take_refresh(&peer)); + } +} diff --git a/crates/net/p2p/src/req_resp/handlers.rs b/crates/net/p2p/src/req_resp/handlers.rs index 5801a84f2..57556dd67 100644 --- a/crates/net/p2p/src/req_resp/handlers.rs +++ b/crates/net/p2p/src/req_resp/handlers.rs @@ -53,6 +53,7 @@ use crate::beacon::transition; use crate::discovery::enr::node_id_from_peer_id; use crate::lean::messages::{BlocksByRootRequest, RequestedBlockRoots, Status}; use crate::lean::protocols::MAX_REQUEST_BLOCKS; +use crate::peer_heads; use crate::req_resp::messages::BlocksByRangeRequest; use crate::{ BACKOFF_MULTIPLIER, CustodyWait, INITIAL_BACKOFF_MS, MAX_FETCH_RETRIES, MAX_SYNC_RANGE, @@ -598,17 +599,25 @@ async fn handle_lean_status_request( peer: PeerId, ) { trace!(finalized_slot=%request.finalized.slot, head_slot=%request.head.slot, "Received status request from peer {peer}"); + peer_heads::observe(server, peer, request.head.slot); let our_status = build_status(&server.store); respond(server, channel, ResponsePayload::LeanStatus(our_status)); } async fn handle_lean_status_response(server: &mut P2PServer, status: Status, peer: PeerId) { trace!(finalized_slot=%status.finalized.slot, head_slot=%status.head.slot, "Received status response from peer {peer}"); + let refresh = server.peer_heads.take_refresh(&peer); + peer_heads::observe(server, peer, status.head.slot); let our_head_slot = server.store.head_slot(); if status.head.slot <= our_head_slot { return; } + // A refresh answer only updates the peer's head unless it shows this node + // well behind; see `REFRESH_RANGE_SYNC_LEAD_LEAN`. + if refresh && status.head.slot - our_head_slot <= peer_heads::REFRESH_RANGE_SYNC_LEAD_LEAN { + return; + } let gap = status.head.slot - our_head_slot; debug!( %peer, @@ -1730,7 +1739,14 @@ async fn handle_status_request( peer_finalized_epoch = peer_status.finalized_epoch(), "Beacon status received" ); + // Recorded only from a digest this node syncs from, as an answer is + // below: a peer still on a superseded digest is not on the chain being + // followed. + if transition::is_syncable_digest(wire, peer_status.fork_digest(), unix_now_ms()) { + peer_heads::observe(server, peer, peer_status.head_slot()); + } } + let wire = server.wire.beacon().expect("checked above"); let our_status = beacon_handler::build_status(&server.store, wire, StatusVersion::of(&peer_status)); respond(server, channel, ResponsePayload::Status(our_status)); @@ -1836,6 +1852,7 @@ async fn handle_status_response( status: BeaconStatus, ctx: &Context, ) { + let refresh = server.peer_heads.take_refresh(&peer); let Some(wire) = server.wire.beacon() else { return; }; @@ -1866,8 +1883,10 @@ async fn handle_status_response( %peer, peer_head_slot, peer_finalized_epoch = status.finalized_epoch(), + refresh, "Beacon handshake complete" ); + peer_heads::observe(server, peer, peer_head_slot); // A whole-block decode per handshake, the same one `build_status` already // pays to send ours. @@ -1877,6 +1896,13 @@ async fn handle_status_response( else { return; }; + // A refresh answer only updates the peer's head unless it shows this node + // an epoch behind; see `REFRESH_RANGE_SYNC_LEAD_BEACON`. + if refresh + && target_range.end - target_range.start <= peer_heads::REFRESH_RANGE_SYNC_LEAD_BEACON + { + return; + } debug!( %peer, @@ -2780,6 +2806,7 @@ pub(crate) mod tests { outbound_requests: HashMap::new(), range_sync_state: None, beacon_fetched_through: 0, + peer_heads: Default::default(), bootnode_addrs: HashMap::new(), node_names: HashMap::new(), discovery: Some(crate::discovery::dial::DiscoveryState::new( diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 14ac9116f..9cdb313c0 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::num::NonZeroUsize; use std::ops::Range; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, LazyLock, Mutex, MutexGuard}; use std::time::Instant; @@ -253,6 +254,9 @@ impl Chain { /// snapshot read or a diff-chain reconstruction. const STATE_CACHE_CAPACITY: usize = 32; +/// [`Store::network_head`]'s "no peer has reported" marker. No slot reaches it. +const NO_NETWORK_HEAD: u64 = u64::MAX; + /// Keep block proofs for at least this many slots below the tip, even once /// finalized. Proofs older than this window are pruned only when the window /// lies entirely within finalized history; see [`Store::prune_old_block_proofs`]. @@ -1196,6 +1200,11 @@ pub struct Store { /// node's own API), which is why it lives on the `Store` all three share. /// See [`ObservedLiveness`]. Always empty on lean. observed_liveness: Arc, + /// The head slot this node's peers report, as P2P last aggregated it from + /// their `Status` messages, or [`NO_NETWORK_HEAD`] while no connected peer + /// has reported one. Written by P2P, read by the chain actor's sync + /// tracker; see [`Store::network_head`]. + network_head: Arc, /// The total active balance per epoch, shared the way /// [`Self::committee_cache`] is: by the chain actor's state transition /// and anything else holding a `Store` clone. See @@ -1846,6 +1855,7 @@ impl Store { pending_states, committee_cache: Arc::new(CommitteeCache::default()), observed_liveness: Arc::new(ObservedLiveness::default()), + network_head: Arc::new(AtomicU64::new(NO_NETWORK_HEAD)), active_balance_cache: Arc::new(ActiveBalanceCache::default()), beacon: Default::default(), state_writer, @@ -3294,6 +3304,27 @@ impl Store { &self.observed_liveness } + /// The head slot this node's peers report, or `None` while no connected + /// peer has reported one. + /// + /// The sync tracker's evidence that the network is ahead of this node. It + /// is a peer *claim*, not a verified block: P2P drops a head past the wall + /// clock and asks several peers to agree before the value moves, and the + /// tracker additionally requires the node's own head to trail the clock, + /// so no claim alone can stop a node that is current. Runtime-only and + /// shared by every clone of this `Store`. + pub fn network_head(&self) -> Option { + let slot = self.network_head.load(Ordering::Relaxed); + (slot != NO_NETWORK_HEAD).then_some(slot) + } + + /// Publish the peers' head; see [`Self::network_head`]. P2P is the only + /// writer. + pub fn set_network_head(&self, slot: Option) { + let slot = slot.unwrap_or(NO_NETWORK_HEAD); + self.network_head.store(slot, Ordering::Relaxed); + } + /// The total-active-balance cache shared by every clone of this store. /// /// An owned `Arc` for the same reason as [`Self::committee_cache`]. @@ -7192,6 +7223,17 @@ mod tests { assert!(store.cached_state(key).is_some()); } + #[test] + fn the_network_head_is_shared_across_store_clones() { + let store = Store::test_store(); + let clone = store.clone(); + assert_eq!(store.network_head(), None); + clone.set_network_head(Some(42)); + assert_eq!(store.network_head(), Some(42)); + clone.set_network_head(None); + assert_eq!(store.network_head(), None); + } + #[test] fn observed_liveness_is_shared_across_store_clones() { let store = beacon_test_store(Arc::new(InMemoryBackend::new())); diff --git a/docs/architecture.md b/docs/architecture.md index 9a4df60e0..14d0c27f8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -166,12 +166,16 @@ flag unset on a placement that does not cover every subnet. ### Sync gate -`sync_status.rs` tracks how far the local head lags the slot clock. Past the threshold the -node reports itself syncing and stops attesting and proposing, since a head derived from a -partial view is not worth voting for. A hysteresis band stops the state from flapping at the -boundary, and a network-wide stall (nobody else is ahead either) leaves the node synced so -its validators can help the chain recover. The same status feeds the `lean_node_sync_status` -metric and, through a shared controller, the `/lean/v0/node/syncing` endpoint. +`sync_status.rs` decides whether the node is behind. It is syncing, and stops attesting +and proposing, when its head trails the slot clock **and** the head its peers report is +ahead of its own, since a head derived from a partial view is not worth voting for. The +peers' head is each connected peer's latest `Status`, re-requested every few slots, which +p2p aggregates onto `Store::network_head` (`crates/net/p2p/src/peer_heads.rs`). A +hysteresis band stops the state from flapping at the boundary, and a network-wide stall +(nobody else is ahead either) leaves the node synced so its validators can help the chain +recover. This departs from leanSpec's rule; see `docs/spec_deviations.md`. The same status +feeds the `lean_node_sync_status` metric and, through a shared controller, the +`/lean/v0/node/syncing` endpoint. `--disable-duty-sync-gate` reduces the gate to observe-only. ### Missing parents diff --git a/docs/beacon_wire.md b/docs/beacon_wire.md index c2138785b..14b44ca72 100644 --- a/docs/beacon_wire.md +++ b/docs/beacon_wire.md @@ -295,10 +295,12 @@ Three differences from lighthouse: `MESH_DELIVERY_MAX_HEAD_LAG` slots, and back on only after an epoch within that lag, so the counters have refilled. While P3 is off its threshold and weight are zero but its cap is not, so the counters keep counting to their - real ceiling. `SyncStatus` cannot serve as this gate: on the mainnet - follower it reported `synced` through a 10-minute catch-up up to 98 slots - behind, since its network-stall rule reads a lagging freshest-known block - as a stalled network. + real ceiling. `SyncStatus` is not this gate: it reports syncing only past + two epochs of lag, and only once peers report a head ahead, where P3 must + be off from `MESH_DELIVERY_MAX_HEAD_LAG` slots. (It also used to read a + lagging freshest-known block as a stalled network, and reported `synced` + through a 10-minute catch-up up to 98 slots behind on the mainnet + follower.) - **Leaving a topic waits out the peers' backoff.** Gossipsub's default backoff after unsubscribing (10 s) is shorter than its prune backoff (60 s), and a rust-libp2p peer that sees a mesh peer unsubscribe backs off diff --git a/docs/rpc.md b/docs/rpc.md index bdcc260ec..74b8cb83f 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -190,7 +190,7 @@ The fork-choice tree from the finalized root, with LMD-GHOST weights computed ov { "is_syncing": false, "head_slot": 1024, "sync_distance": 1, "finalized_slot": 986 } ``` -`is_syncing` is the node's own stateful sync decision: head-vs-wall-clock lag with hysteresis and a network-stall override, updated each tick. It is the same signal that gates validator duties and drives the `lean_node_sync_status` metric, so the endpoint, the gate, and the metric always agree. +`is_syncing` is the node's own stateful sync decision, updated each tick: true while the head trails the wall clock and the head the node's peers report in `Status` is ahead of it, with hysteresis. With no peer ahead (a stalled network, or no peers) it is false. It is the same signal that gates validator duties and drives the `lean_node_sync_status` metric, so the endpoint, the gate, and the metric always agree. `sync_distance` is the raw number of slots between the node's current head and the current wall-clock slot, computed per request. Because `is_syncing` carries hysteresis and stall handling and is not recomputed from `sync_distance`, the two can point different ways near the threshold or during a network-wide stall. diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 49ee15c03..6bce1a744 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -28,6 +28,45 @@ interval, and stops early once it runs out of time. - **leanSpec:** `aggregate()` is called inline and synchronously from `tick_interval`, at interval 2 only. It walks every attestation data with fresh evidence, with no job cap, no time budget, no worker, and no cancellation. - **Equivalence:** on cancellation the worker emits only the groups that finished, so a slot may pack fewer aggregates than the synchronous path would; any such subset still yields a valid block, affecting how many votes are included rather than signature validity. The job cap has the same character: it bounds prover work per slot, not what a block may carry. +## The duty sync gate compares the head with peers' heads, not with imported blocks + +A node reports itself syncing, and stops proposing and attesting, only when its +head trails the wall clock **and** its peers report a head ahead of it. + +- **leanSpec:** `ValidatorService._is_synced_for_duties` (PR #708) takes the + freshest block it knows from imported blocks only. When that block is more + than 8 slots behind the wall clock it reads the network as stalled and allows + duties; otherwise the node is syncing once its head is more than 4 slots + behind the clock, with a hysteresis band of 2. + `test_no_extra_blocks_treats_isolation_as_network_stall` pins the case of a + node 100 slots behind with nothing newer imported, which does its duties. +- **ethlambda:** the node is syncing when its head is more than 4 slots behind + the wall clock and the head its peers report is more than 4 slots ahead of + its own, with the same band of 2 on both (`SyncThresholds::LEAN`, + `crates/blockchain/src/sync_status.rs`). The peers' head comes from `Status` + (`crates/net/p2p/src/peer_heads.rs`): each connected peer's latest report, + re-requested every 4 slots, a head past the wall clock by more than a slot + dropped, and the second-highest report counted once 3 or more peers have + reported (the highest below that). With no report at all the node cannot be + behind anyone and reads as synced, except during an 8-slot hold at startup + for a node whose head already trails the clock. +- **Why:** imported blocks cannot tell "the network is stalled" from "this node + is behind". A node catching up has imported nothing recent either, so the + specification's rule reports it synced for the whole catch-up and it proposes + and attests on a stale head (lambdaclass/ethlambda#559). Lighthouse and Prysm + decide against what their peers report, and require the head to trail the + clock too, so no peer's claim can idle a node that is current. Comparing with + peers also keeps a run of missed proposals (leanSpec #1194) from closing the + gate on every honest node, since none of them is behind another. +- **Equivalence:** the two agree whenever no peer is further ahead of the node + than its newest imported block. They differ for a node behind its peers with + nothing newer imported, which the specification lets do its duties and this + node does not. + +The beacon node runs the same rule with Lighthouse's numbers (two epochs of +clock lag, one epoch of peer lead, a refresh every half epoch). consensus-specs +has no sync gate, and the Beacon API leaves `is_syncing` to the client. + ## Attestation scoring on block building Attestations are scored and selected when packing a block, rather than taken in