Skip to content

fix(sync): judge the sync gate against peers' Status heads #791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MegaRedHand wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(sync): judge the sync gate against peers' Status heads #791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db61388
fix(sync): judge the sync gate against peers' Status heads
MegaRedHand Nov 11, 2026
5e0a9d1
fix(sync): judge the gate on every block arrival, not only on the tick
MegaRedHand Nov 12, 2026
d907b60
docs(sync): the half-epoch Status refresh is Prysm's cadence, not Lig…
MegaRedHand Nov 12, 2026
6f08c98
perf(p2p): spread Status refreshes over the interval instead of one b…
MegaRedHand Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 98 additions & 36 deletions crates/blockchain/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)),
Expand All @@ -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.
///
Expand Down Expand Up @@ -356,7 +356,7 @@ impl BlockChain {

Self::start_actor(
store,
SyncStatusTracker::new(false),
SyncStatusTracker::new(false, SyncThresholds::BEACON),
sync_status_controller,
events,
ChainDuties::Beacon,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2088,8 +2088,15 @@ impl BlockChainServer {
// the store clock, which only `on_tick` and an early arrival
// advance.
metrics::update_current_slot(self.wall_clock_slot());
// Judges the sync gate too.
let head = self.recompute_beacon_head().await;
cascade.absorb_head(head);
} else {
// Lean duties read the gate in the tick that judges it, but the
// tick waits behind a catch-up's blocks the same way beacon's
// does, and the Lean API, the status gauge and the next arrival's
// reaggregation check read it in between.
self.update_sync_status(self.wall_clock_slot());
}

cascade.observe(blocks);
Expand Down Expand Up @@ -2306,8 +2313,16 @@ impl BlockChainServer {
/// `forkchoiceUpdated` verdict's) happens inside it. After the
/// `forkchoiceUpdated` round trip, so `execution_optimistic` reflects the
/// verdict it carried.
///
/// And where the sync gate is judged between ticks, for the same reason
/// the head is: on a catch-up the tick waits behind every block range sync
/// forwarded, so a gate judged only there kept answering `Synced` to the
/// validator client for the whole catch-up, however far behind. Judged
/// before the `forkchoiceUpdated` round trip, so a slow execution client
/// does not delay it by one more engine call.
async fn recompute_beacon_head(&mut self) -> HeadTimings {
let mut timings = self.update_head_from_fork_choice();
self.update_sync_status(self.wall_clock_slot());
if let Some((start, end)) = self.notify_forkchoice_updated().await {
timings.fcu_start = Some(start);
timings.fcu_end = Some(end);
Expand Down Expand Up @@ -3584,32 +3599,30 @@ 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(),
current_slot,
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);
}
Expand All @@ -3620,16 +3633,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()
Expand Down Expand Up @@ -4490,7 +4504,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,
}
Expand Down Expand Up @@ -5277,15 +5291,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
Expand All @@ -5297,13 +5314,58 @@ 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
);
}

#[tokio::test]
async fn a_block_arrival_judges_the_gate_without_waiting_for_the_tick() {
// A node reconnecting to peers far ahead of it: range sync forwards
// the whole catch-up at once, and the tick waits in the mailbox behind
// every block of it. The gate has to move on the arrivals themselves,
// or it answers `Synced` for the whole catch-up. A block whose parent
// is unknown is enough: whatever becomes of a block, its arrival ends
// in the head recompute.
let mut server = syncing_server(None);
let orphan = || bare_block(1, H256::repeat_byte(0xee));

// Peers as stale as the node: a stall, synced.
server.store.set_network_head(Some(0));
server.on_block(orphan(), ImportTimings::default()).await;
assert_eq!(
server.sync_status_controller.get(),
crate::metrics::SyncStatus::Synced
);

// A peer reports a head at the clock; the next arrival closes the gate.
let wall_clock_slot = server.wall_clock_slot();
server.store.set_network_head(Some(wall_clock_slot));
server.on_block(orphan(), ImportTimings::default()).await;
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)
Expand Down
Loading
Loading