From 77926d4cbb95c5fcca1693ecb6c135a5ec2076db Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 15:06:36 +0900 Subject: [PATCH 1/8] feat(bootstrap): persist and restore the whole routing table across a restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A restart currently keeps the k peers nearest to self and nothing else, then rebuilds the rest of the routing table at two k-buckets per 7.5 to 12.5 minutes — a cadence this crate's own comment describes as approximately once-per-day full-table maintenance. Until that completes the node cannot name anyone closer to a distant key than itself, so every consumer that asks "am I among the w closest to this key" gets yes for most of the keyspace. A close group cannot stand in for a routing table, and the reason is combinatorial rather than a matter of degree. For a key K, write c for the number of leading bits K shares with self. Any peer p sharing exactly c leading bits with self agrees with self on bits [0,c) and differs at bit c; K does the same; so p agrees with K at bit c and is strictly closer to K than self is. Every peer in bucket c therefore answers the question, and a node holding w of them can always answer it correctly — while a node whose bucket c is empty cannot, however many neighbours it has. Preserving peers across every bucket is what makes a restored table answer as the original did, at every width. Simulated on an 891-node network at the two widths a storage consumer uses, against a correct share of 1.01% and 2.24%: a converged table claims 1.05% and 2.01%; today's nearest-20 cache claims 41.4% and 95.6%; a full-table snapshot claims 1.05% and 2.01%. Keeping only nine peers per bucket fixes the narrow width but leaves the wide one at 12.3%, which is why the snapshot carries the whole capped table — about 129 entries and 25 KB for that network. The snapshot is written to its own file so an older binary cannot mistake it for a close group, and the close-group cache continues to be written alongside so a downgrade keeps working. It is accepted only when its schema version, owning node, network fingerprint, integrity checksum and age all check out; the owner binding is load-bearing rather than hygiene, because bucket indices are relative to the owner, so another node's snapshot describes a different partition of the id space. It records no trust scores. The close-group cache imports trust before dialling, which is defensible for k vetted neighbours and is not defensible for a whole table: a file on disk must not decide that hundreds of unverified peers start above neutral. Restored peers are dial candidates only, verified through the ordinary identity-checked path before they can enter the routing table, because routing-table membership carries authority for callers above this crate. Restoration is on by default with a kill switch, dials at bounded concurrency, and abandons the remainder once a wall-clock budget expires, so a snapshot full of departed peers costs a bounded startup delay and then behaves exactly like having had no snapshot at all. --- src/bootstrap/mod.rs | 14 +- src/bootstrap/routing_snapshot.rs | 562 ++++++++++++++++++++++++++++++ src/network.rs | 293 +++++++++++++++- 3 files changed, 867 insertions(+), 2 deletions(-) create mode 100644 src/bootstrap/routing_snapshot.rs diff --git a/src/bootstrap/mod.rs b/src/bootstrap/mod.rs index 2c18ca5..78af79e 100644 --- a/src/bootstrap/mod.rs +++ b/src/bootstrap/mod.rs @@ -10,8 +10,20 @@ // distributed under these licenses is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -//! Close-group cache for warm-loading trusted peers across restarts. +//! Persisted peer knowledge for warm-starting across restarts. +//! +//! Two files, written side by side: +//! +//! - [`cache`] holds the `k` peers nearest to self — the close group. Kept so a +//! downgrade to an older binary still finds what it expects. +//! - [`routing_snapshot`] holds the whole routing table. A close group cannot +//! reconstruct a routing table, because the peers that answer "is anyone +//! closer to this key than me?" for a distant key are exactly the ones a +//! close group leaves out. See that module for why this is combinatorial +//! rather than a matter of degree. pub mod cache; +pub mod routing_snapshot; pub use cache::{CachedCloseGroupPeer, CloseGroupCache}; +pub use routing_snapshot::{RoutingSnapshot, SnapshotPeer, network_fingerprint}; diff --git a/src/bootstrap/routing_snapshot.rs b/src/bootstrap/routing_snapshot.rs new file mode 100644 index 0000000..6f766df --- /dev/null +++ b/src/bootstrap/routing_snapshot.rs @@ -0,0 +1,562 @@ +// Copyright 2024 Saorsa Labs Limited +// +// This software is licensed under the MIT license or the Apache License, Version 2.0 +// , at your +// option. This file may not be copied, modified, or distributed except +// according to those terms. +// +// Unless required by applicable law or agreed to in writing, software +// distributed under these licenses is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + +//! Routing snapshot: the whole routing table, persisted across a restart. +//! +//! # Why the close-group cache is not enough +//! +//! [`CloseGroupCache`](super::cache::CloseGroupCache) persists the `k` peers +//! nearest to self. That is the right shape for reconnecting to a close group, +//! and the wrong shape for reconstructing a routing table, because it drops +//! every peer that is not a neighbour — which is precisely the population a +//! node consults to answer "is anyone closer to this key than me?". +//! +//! The consequence is combinatorial, not statistical. For a key `K`, write +//! `c = CPL(self, K)` for the number of leading bits they share. Any peer `p` +//! with `CPL(p, self) == c` agrees with self on bits `[0, c)` and differs at +//! bit `c`; `K` agrees with self on `[0, c)` and also differs at bit `c`; +//! therefore `p` agrees with `K` at bit `c`, giving `CPL(p, K) >= c + 1`. +//! **Every peer in bucket `c` is strictly closer to `K` than self is.** +//! +//! So a node holding `w` peers in bucket `c` can always answer "no, I am not +//! among the `w` closest to `K`" — and a node whose bucket `c` is empty cannot, +//! however many neighbours it has. A snapshot must therefore preserve peers +//! across *every* bucket, not the nearest `k` overall. Preserving up to the +//! bucket capacity is what makes the restored table answer as the original did, +//! for every key, at every width. +//! +//! # What a snapshot is, and is not +//! +//! Restored peers are **dial candidates**. They are not inserted into the +//! routing table, are not trusted, and confer no authority until they have been +//! dialled and identity-verified through the ordinary path — routing-table +//! membership is an authorization fact for callers above this crate, and a file +//! on disk must never be able to grant it. +//! +//! For the same reason a snapshot records **no trust scores**. The close-group +//! cache does carry them, and imports them into the `TrustEngine` before its +//! peers are dialled, which is defensible for a set of `k` neighbours a node has +//! already vetted. It is not defensible for a whole-table file: pre-dial trust +//! restoration would let a file on disk decide that hundreds of unverified peers +//! start above neutral. A snapshot answers "where were my peers", never "how +//! much did I trust them" — trust is re-earned from live behaviour. +//! +//! # Bindings +//! +//! A snapshot is accepted only when every binding holds: +//! +//! - **Schema version.** An unknown version is discarded rather than guessed at. +//! - **Owner.** Bucket indices are relative to the owning node's id, so another +//! node's snapshot is not merely stale, it is *meaningless* — it describes a +//! different partition of the id space. This is the binding that matters most. +//! - **Network fingerprint.** Derived from the configured bootstrap set, so a +//! snapshot does not follow a node between networks. Bootstrap lists do change +//! legitimately; the cost of a mismatch is one cold start, which is exactly +//! today's behaviour, so failing closed here is cheap. +//! - **Integrity.** A checksum over the payload, so a truncated or bit-flipped +//! file is rejected instead of partially believed. +//! - **Age.** Reuses the close-group cache's rules, including rejecting +//! timestamps far in the future so a broken clock cannot make a snapshot look +//! fresh forever. +//! +//! None of these is a defence against an attacker who can write to the node's +//! data directory: such an attacker owns the node. They defend against the +//! accidents that actually happen — copied directories, cloned images, rolled +//! back filesystems, half-written files, and a snapshot outliving the network it +//! was taken on. + +use std::io::Write as _; +use std::path::Path; +use std::time::Duration; + +use serde::{Deserialize, Serialize}; + +use crate::PeerId; +use crate::address::MultiAddr; + +/// A peer recorded in a routing snapshot. +/// +/// Identity and addresses only. See the module docs for why no trust score is +/// carried: this file must not be able to promote unverified peers above +/// neutral before they have been dialled. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SnapshotPeer { + /// Peer identity, re-verified on dial before it can enter the routing table. + pub peer_id: PeerId, + /// Addresses last known to reach this peer. + pub addresses: Vec, +} + +/// Filename for the routing snapshot. +/// +/// Deliberately distinct from `close_group_cache.json`: an older binary must +/// never read this file and mistake a whole-table snapshot for a close group. +/// The two are written side by side so a downgrade keeps working. +pub const ROUTING_SNAPSHOT_FILENAME: &str = "routing_snapshot.json"; + +/// Schema version for [`RoutingSnapshot`]. +/// +/// Bump on any change to the payload's meaning. An unrecognised version is +/// discarded, never coerced. +pub const ROUTING_SNAPSHOT_SCHEMA_VERSION: u32 = 1; + +/// Maximum tolerated wall-clock skew for a snapshot timestamp in the future. +const MAX_FUTURE_TIMESTAMP_SKEW: Duration = Duration::from_secs(5 * 60); + +/// Why a snapshot on disk was not used. +/// +/// Every variant is a reason an operator may need to see: a node that silently +/// cold-starts every time looks identical to one with no snapshot at all. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SnapshotRejection { + /// The schema version is not one this build understands. + UnknownSchemaVersion { + /// Version found in the file. + found: u32, + /// Version this build writes. + expected: u32, + }, + /// The snapshot was written by a different node. + ForeignOwner, + /// The snapshot was taken on a different network. + ForeignNetwork, + /// The checksum does not match the payload. + CorruptChecksum, + /// The snapshot is older than the configured maximum age, or its timestamp + /// is implausibly far in the future. + Stale, + /// The file could not be parsed at all. + Unreadable, +} + +impl std::fmt::Display for SnapshotRejection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::UnknownSchemaVersion { found, expected } => { + write!( + f, + "unknown schema version {found} (this build writes {expected})" + ) + } + Self::ForeignOwner => write!(f, "written by a different node"), + Self::ForeignNetwork => write!(f, "taken on a different network"), + Self::CorruptChecksum => write!(f, "checksum mismatch"), + Self::Stale => write!(f, "stale or implausibly future-dated"), + Self::Unreadable => write!(f, "unparseable"), + } + } +} + +/// The checksummed body of a snapshot. +/// +/// Split from the envelope so the checksum covers exactly the bytes whose +/// integrity is being asserted, and so adding an envelope field later cannot +/// silently change what was signed for. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RoutingSnapshotPayload { + /// Schema version of this payload. + pub schema_version: u32, + /// Node that wrote the snapshot. Bucket indices are relative to this id. + pub owner: PeerId, + /// Fingerprint of the network the snapshot was taken on. + pub network_fingerprint: String, + /// When the snapshot was written (seconds since UNIX epoch). + pub saved_at_epoch_secs: u64, + /// Every routing-table peer at the time of writing, across all buckets. + pub peers: Vec, +} + +/// A persisted routing table, with the bindings needed to use it safely. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RoutingSnapshot { + /// The checksummed body. + pub payload: RoutingSnapshotPayload, + /// Hex-encoded BLAKE3 of the canonical payload encoding. + pub checksum: String, +} + +/// Derive a network fingerprint from the configured bootstrap addresses. +/// +/// Order-independent, so reordering the configured list is not a change of +/// network. An empty list yields a well-known fingerprint so that a node with no +/// configured bootstrap peers still round-trips its own snapshot. +#[must_use] +pub fn network_fingerprint(bootstrap_peers: &[MultiAddr]) -> String { + let mut rendered: Vec = bootstrap_peers.iter().map(ToString::to_string).collect(); + rendered.sort_unstable(); + let mut hasher = blake3::Hasher::new(); + hasher.update(b"saorsa-routing-snapshot-network-v1"); + for entry in &rendered { + hasher.update(entry.as_bytes()); + hasher.update(b"\n"); + } + hex::encode(hasher.finalize().as_bytes()) +} + +impl RoutingSnapshotPayload { + /// Canonical checksum over this payload. + /// + /// Computed from the serialized form so it covers every field without a + /// hand-maintained list that could drift as fields are added. + fn checksum(&self) -> anyhow::Result { + let encoded = serde_json::to_vec(self) + .map_err(|e| anyhow::anyhow!("failed to encode routing snapshot payload: {e}"))?; + let mut hasher = blake3::Hasher::new(); + hasher.update(b"saorsa-routing-snapshot-payload-v1"); + hasher.update(&encoded); + Ok(hex::encode(hasher.finalize().as_bytes())) + } +} + +impl RoutingSnapshot { + /// Build a snapshot from the current routing table. + /// + /// # Errors + /// + /// Returns an error if the payload cannot be encoded for checksumming. + pub fn new( + owner: PeerId, + network_fingerprint: String, + saved_at_epoch_secs: u64, + peers: Vec, + ) -> anyhow::Result { + let payload = RoutingSnapshotPayload { + schema_version: ROUTING_SNAPSHOT_SCHEMA_VERSION, + owner, + network_fingerprint, + saved_at_epoch_secs, + peers, + }; + let checksum = payload.checksum()?; + Ok(Self { payload, checksum }) + } + + /// Number of peers carried. + #[must_use] + pub fn peer_count(&self) -> usize { + self.payload.peers.len() + } + + /// Check every binding, returning the peers only if all of them hold. + /// + /// Checked cheapest-first, and integrity before meaning: there is no point + /// interpreting fields from a file that failed its checksum. + /// + /// # Errors + /// + /// Returns the first binding that failed, so the caller can log why a + /// snapshot was discarded rather than reporting a bare absence. + pub fn validate( + &self, + expected_owner: &PeerId, + expected_network: &str, + now_epoch_secs: u64, + max_age: Option, + ) -> Result<&[SnapshotPeer], SnapshotRejection> { + if self.payload.schema_version != ROUTING_SNAPSHOT_SCHEMA_VERSION { + return Err(SnapshotRejection::UnknownSchemaVersion { + found: self.payload.schema_version, + expected: ROUTING_SNAPSHOT_SCHEMA_VERSION, + }); + } + let Ok(expected_checksum) = self.payload.checksum() else { + return Err(SnapshotRejection::CorruptChecksum); + }; + if expected_checksum != self.checksum { + return Err(SnapshotRejection::CorruptChecksum); + } + if self.payload.owner != *expected_owner { + return Err(SnapshotRejection::ForeignOwner); + } + if self.payload.network_fingerprint != expected_network { + return Err(SnapshotRejection::ForeignNetwork); + } + if self.is_stale(now_epoch_secs, max_age) { + return Err(SnapshotRejection::Stale); + } + Ok(&self.payload.peers) + } + + /// Whether the snapshot is older than `max_age`, or dated implausibly far + /// in the future. + /// + /// `None` disables the maximum-age check; a future timestamp beyond the + /// tolerated skew is always rejected, so a broken clock cannot make a + /// snapshot look fresh indefinitely. + #[must_use] + pub fn is_stale(&self, now_epoch_secs: u64, max_age: Option) -> bool { + let future_skew = self + .payload + .saved_at_epoch_secs + .saturating_sub(now_epoch_secs); + if future_skew > MAX_FUTURE_TIMESTAMP_SKEW.as_secs() { + return true; + } + max_age.is_some_and(|max_age| { + now_epoch_secs.saturating_sub(self.payload.saved_at_epoch_secs) > max_age.as_secs() + }) + } + + /// Write the snapshot to `{dir}/routing_snapshot.json`. + /// + /// Atomic: written to a uniquely-named temporary file in the same directory + /// and persisted by rename, so a crash mid-write leaves either the previous + /// snapshot or none — never a half-written one. The checksum makes a + /// half-written file detectable even if the platform's rename is not atomic. + /// + /// # Errors + /// + /// Returns an error if the directory cannot be created, or the file cannot + /// be serialized, written, or persisted. + pub async fn save_to_dir(&self, dir: &Path) -> anyhow::Result<()> { + tokio::fs::create_dir_all(dir).await.map_err(|e| { + anyhow::anyhow!( + "failed to create routing snapshot directory {}: {e}", + dir.display() + ) + })?; + + let path = dir.join(ROUTING_SNAPSHOT_FILENAME); + let json = serde_json::to_string_pretty(self) + .map_err(|e| anyhow::anyhow!("failed to serialize routing snapshot: {e}"))?; + + let dir_owned = dir.to_path_buf(); + tokio::task::spawn_blocking(move || { + let mut tmp = tempfile::NamedTempFile::new_in(&dir_owned).map_err(|e| { + anyhow::anyhow!("failed to create temp file in {}: {e}", dir_owned.display()) + })?; + tmp.write_all(json.as_bytes()) + .map_err(|e| anyhow::anyhow!("failed to write routing snapshot: {e}"))?; + tmp.persist(&path).map_err(|e| { + anyhow::anyhow!( + "failed to persist routing snapshot to {}: {e}", + path.display() + ) + })?; + Ok(()) + }) + .await + .map_err(|e| anyhow::anyhow!("routing snapshot save task panicked: {e}"))? + } + + /// Read the snapshot from `{dir}/routing_snapshot.json`. + /// + /// Returns `Ok(None)` when there is no snapshot, and + /// `Err(SnapshotRejection::Unreadable)` when there is one that cannot be + /// parsed — a corrupt file is a fact worth logging, not an absence. + /// + /// # Errors + /// + /// Returns [`SnapshotRejection::Unreadable`] for an unreadable or + /// unparseable file. + pub async fn load_from_dir(dir: &Path) -> Result, SnapshotRejection> { + let path = dir.join(ROUTING_SNAPSHOT_FILENAME); + match tokio::fs::read_to_string(&path).await { + Ok(json) => match serde_json::from_str(&json) { + Ok(snapshot) => Ok(Some(snapshot)), + Err(_) => Err(SnapshotRejection::Unreadable), + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(_) => Err(SnapshotRejection::Unreadable), + } + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + const NOW: u64 = 1_700_000_000; + + fn peer() -> SnapshotPeer { + SnapshotPeer { + peer_id: PeerId::random(), + addresses: vec!["/ip4/10.0.1.1/udp/9000/quic".parse().unwrap()], + } + } + + fn snapshot(owner: PeerId, network: &str, saved_at: u64, count: usize) -> RoutingSnapshot { + let peers = (0..count).map(|_| peer()).collect(); + RoutingSnapshot::new(owner, network.to_string(), saved_at, peers).unwrap() + } + + #[tokio::test] + async fn round_trips_through_disk() { + let owner = PeerId::random(); + let snap = snapshot(owner, "net-a", NOW, 40); + let dir = tempfile::tempdir().unwrap(); + + snap.save_to_dir(dir.path()).await.unwrap(); + let loaded = RoutingSnapshot::load_from_dir(dir.path()) + .await + .unwrap() + .unwrap(); + + assert_eq!(loaded.peer_count(), 40); + let peers = loaded.validate(&owner, "net-a", NOW, None).unwrap(); + assert_eq!(peers.len(), 40); + } + + #[tokio::test] + async fn a_missing_snapshot_is_absence_not_an_error() { + let dir = tempfile::tempdir().unwrap(); + assert!( + RoutingSnapshot::load_from_dir(dir.path()) + .await + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn a_truncated_file_is_reported_not_silently_ignored() { + // Truncation must be distinguishable from "no snapshot": a node that + // cold-starts every time because its file is corrupt looks exactly like + // one that never had a snapshot. + let dir = tempfile::tempdir().unwrap(); + let snap = snapshot(PeerId::random(), "net-a", NOW, 8); + snap.save_to_dir(dir.path()).await.unwrap(); + + let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); + let json = tokio::fs::read_to_string(&path).await.unwrap(); + let truncated = &json[..json.len() / 2]; + tokio::fs::write(&path, truncated).await.unwrap(); + + assert_eq!( + RoutingSnapshot::load_from_dir(dir.path()) + .await + .unwrap_err(), + SnapshotRejection::Unreadable + ); + } + + #[test] + fn a_bit_flip_that_keeps_the_json_valid_fails_the_checksum() { + // The case a parser cannot catch: still-valid JSON, wrong contents. + let owner = PeerId::random(); + let mut snap = snapshot(owner, "net-a", NOW, 4); + snap.payload.saved_at_epoch_secs = NOW - 1; + + assert_eq!( + snap.validate(&owner, "net-a", NOW, None).unwrap_err(), + SnapshotRejection::CorruptChecksum + ); + } + + #[test] + fn another_nodes_snapshot_is_refused() { + // Bucket indices are relative to the owner, so a foreign snapshot is + // not stale data — it describes a different partition of the id space. + let snap = snapshot(PeerId::random(), "net-a", NOW, 4); + assert_eq!( + snap.validate(&PeerId::random(), "net-a", NOW, None) + .unwrap_err(), + SnapshotRejection::ForeignOwner + ); + } + + #[test] + fn a_snapshot_from_another_network_is_refused() { + let owner = PeerId::random(); + let snap = snapshot(owner, "net-a", NOW, 4); + assert_eq!( + snap.validate(&owner, "net-b", NOW, None).unwrap_err(), + SnapshotRejection::ForeignNetwork + ); + } + + #[test] + fn an_unknown_schema_version_is_refused_rather_than_guessed_at() { + let owner = PeerId::random(); + let mut snap = snapshot(owner, "net-a", NOW, 4); + snap.payload.schema_version = ROUTING_SNAPSHOT_SCHEMA_VERSION + 1; + snap.checksum = snap.payload.checksum().unwrap(); + + assert_eq!( + snap.validate(&owner, "net-a", NOW, None).unwrap_err(), + SnapshotRejection::UnknownSchemaVersion { + found: ROUTING_SNAPSHOT_SCHEMA_VERSION + 1, + expected: ROUTING_SNAPSHOT_SCHEMA_VERSION, + } + ); + } + + #[test] + fn staleness_is_bounded_on_both_sides_of_now() { + let owner = PeerId::random(); + + let old = snapshot(owner, "net-a", NOW - 7200, 4); + assert_eq!( + old.validate(&owner, "net-a", NOW, Some(Duration::from_secs(3600))) + .unwrap_err(), + SnapshotRejection::Stale + ); + assert!(old.validate(&owner, "net-a", NOW, None).is_ok()); + + // A clock that jumped forward must not mint an evergreen snapshot. + let future = snapshot(owner, "net-a", NOW + 86_400, 4); + assert_eq!( + future.validate(&owner, "net-a", NOW, None).unwrap_err(), + SnapshotRejection::Stale + ); + + // Small skew is tolerated. + let skewed = snapshot(owner, "net-a", NOW + 60, 4); + assert!(skewed.validate(&owner, "net-a", NOW, None).is_ok()); + } + + #[test] + fn the_network_fingerprint_ignores_ordering_but_not_membership() { + let a: MultiAddr = "/ip4/10.0.1.1/udp/9000/quic".parse().unwrap(); + let b: MultiAddr = "/ip4/10.0.2.1/udp/9000/quic".parse().unwrap(); + let c: MultiAddr = "/ip4/10.0.3.1/udp/9000/quic".parse().unwrap(); + + assert_eq!( + network_fingerprint(&[a.clone(), b.clone()]), + network_fingerprint(&[b.clone(), a.clone()]), + "reordering the configured list is not a change of network" + ); + assert_ne!( + network_fingerprint(&[a.clone(), b.clone()]), + network_fingerprint(&[a, b, c]), + "a different bootstrap set is a different network" + ); + assert_eq!( + network_fingerprint(&[]), + network_fingerprint(&[]), + "no configured peers still round-trips" + ); + } + + #[tokio::test] + async fn a_copied_snapshot_does_not_transplant_between_nodes() { + // The accident this actually guards: a data directory copied to a new + // host, or a cloned VM image, where the identity differs. + let dir = tempfile::tempdir().unwrap(); + snapshot(PeerId::random(), "net-a", NOW, 30) + .save_to_dir(dir.path()) + .await + .unwrap(); + + let loaded = RoutingSnapshot::load_from_dir(dir.path()) + .await + .unwrap() + .unwrap(); + assert_eq!( + loaded + .validate(&PeerId::random(), "net-a", NOW, None) + .unwrap_err(), + SnapshotRejection::ForeignOwner + ); + } +} diff --git a/src/network.rs b/src/network.rs index 9d0dde9..b786a7a 100644 --- a/src/network.rs +++ b/src/network.rs @@ -19,6 +19,7 @@ use crate::PeerId; use crate::adaptive::trust::{TrustRecord, TrustSnapshot}; use crate::adaptive::{AdaptiveDHT, AdaptiveDhtConfig, TrustEngine, TrustEvent}; use crate::bootstrap::cache::{CachedCloseGroupPeer, CloseGroupCache}; +use crate::bootstrap::{RoutingSnapshot, SnapshotPeer, network_fingerprint}; use crate::dht::core_engine::AddressType; use crate::dht_network_manager::{DhtNetworkConfig, DhtNetworkEvent, DhtNetworkManager}; use crate::error::{NetworkError, P2PError, P2pResult as Result}; @@ -178,6 +179,23 @@ const MAX_CONCURRENT_BOOTSTRAP_DIALS: usize = 4; /// dial every candidate so their routing table converges fully. const CLIENT_BOOTSTRAP_TARGET: usize = 6; +/// Maximum routing-snapshot peers dialled concurrently. +/// +/// Higher than [`MAX_CONCURRENT_BOOTSTRAP_DIALS`] because this set is an order +/// of magnitude larger — a full table rather than a handful of seeds — and the +/// whole value of restoring it is that the table is usable in seconds. Still +/// bounded, so a restart cannot open an unbounded number of simultaneous +/// QUIC+PQC handshakes. +const MAX_CONCURRENT_SNAPSHOT_DIALS: usize = 16; + +/// Wall-clock budget for the routing-snapshot dial phase. +/// +/// Startup must not be held hostage to a snapshot full of departed peers. When +/// the budget expires the remaining candidates are abandoned and the table +/// refills through ordinary discovery, which is exactly the behaviour of a node +/// that had no snapshot at all. +const SNAPSHOT_RESTORE_BUDGET: Duration = Duration::from_secs(20); + /// Serde helper — returns `true`. const fn default_true() -> bool { true @@ -291,6 +309,31 @@ pub struct NodeConfig { /// check. Default: one hour. #[serde(default = "default_close_group_cache_max_age")] pub close_group_cache_max_age: Option, + + /// Whether a validated routing snapshot seeds startup dials. + /// + /// **Defaults to `true`.** This is the behaviour that repairs a restarted + /// node's routing table in seconds rather than over the roughly daily + /// bucket-refresh cycle, and it is the point of persisting one at all. + /// + /// Set to `false` as an operational kill switch. That restores the previous + /// behaviour exactly — the close-group cache still seeds the neighbourhood, + /// and the snapshot is still written, so switching back needs no rebuild and + /// loses no data. + /// + /// **Consumer prerequisite.** A restored table is materially more complete, + /// and a consumer that decides record retention from routing state may treat + /// a table which has grown from 19 to 20 apparently-closer peers as licence + /// to delete a record with no holder confirmation. Any consumer must have + /// removed that path before adopting a version of this crate carrying + /// restoration. See the routing-snapshot module docs. + #[serde(default = "default_routing_snapshot_restore")] + pub routing_snapshot_restore: bool, +} + +/// Routing-snapshot restoration is on by default; see the field docs. +const fn default_routing_snapshot_restore() -> bool { + true } fn default_close_group_cache_max_age() -> Option { @@ -428,6 +471,7 @@ pub struct NodeConfigBuilder { /// Outer `None` means the builder setter was not called; inner `None` /// explicitly disables age enforcement. close_group_cache_max_age: Option>, + routing_snapshot_restore: Option, } impl Default for NodeConfigBuilder { @@ -447,6 +491,7 @@ impl Default for NodeConfigBuilder { adaptive_dht_config: None, close_group_cache_dir: None, close_group_cache_max_age: None, + routing_snapshot_restore: None, } } } @@ -575,6 +620,16 @@ impl NodeConfigBuilder { self } + /// Set whether a validated routing snapshot seeds startup dials. + /// + /// Defaults to `false`. Read the field documentation on + /// [`NodeConfig::routing_snapshot_restore`]. Defaults to `true`; setting + /// `false` is the kill switch that restores the previous startup behaviour. + pub fn routing_snapshot_restore(mut self, enabled: bool) -> Self { + self.routing_snapshot_restore = Some(enabled); + self + } + /// Build the [`NodeConfig`]. /// /// # Errors @@ -605,6 +660,9 @@ impl NodeConfigBuilder { close_group_cache_max_age: self .close_group_cache_max_age .unwrap_or_else(default_close_group_cache_max_age), + routing_snapshot_restore: self + .routing_snapshot_restore + .unwrap_or_else(default_routing_snapshot_restore), }) } } @@ -628,6 +686,7 @@ impl Default for NodeConfig { adaptive_dht_config: AdaptiveDhtConfig::default(), close_group_cache_dir: None, close_group_cache_max_age: default_close_group_cache_max_age(), + routing_snapshot_restore: default_routing_snapshot_restore(), } } } @@ -1382,11 +1441,13 @@ impl P2PNode { let peer_id = self.peer_id; let k_value = self.config.dht_config.k_value; let shutdown = self.close_group_cache_save_shutdown.clone(); + let fingerprint = network_fingerprint(&self.config.bootstrap_peers); *task = Some(tokio::spawn(periodic_close_group_cache_save( dht_manager, trust_engine, peer_id, k_value, + fingerprint, dir, interval, shutdown, @@ -2073,7 +2134,15 @@ impl P2PNode { } } - if serial_addr_sets.is_empty() && parallel_addr_sets.is_empty() { + // Priority 2: the routing snapshot, if one validated. Addresses already + // queued as close-group or configured candidates are skipped so a peer + // is never dialled twice. + let snapshot_addr_sets = self.routing_snapshot_dial_sets(&mut seen_addresses).await; + + if serial_addr_sets.is_empty() + && parallel_addr_sets.is_empty() + && snapshot_addr_sets.is_none() + { info!("No bootstrap peers configured"); return Ok(()); } @@ -2136,11 +2205,70 @@ impl P2PNode { // before we proceed to the DHT discovery phase below. } + // Phase C: the routing snapshot. + // + // The close group reconnects a neighbourhood; this rebuilds the rest of + // the table, which is what a node consults to decide whether anyone is + // closer to a given key than it is. Dialled concurrently and last: it is + // the widest set and the least urgent, and giving the close group and + // the configured peers the first attempts keeps cold-start latency for + // an unrestored node exactly as it was. + // + // Every peer here is dialled and identity-verified through the same path + // as any other candidate. Nothing from the file enters the routing table + // on the file's authority alone. + let mut snapshot_dial_candidates = 0usize; + let mut snapshot_dial_successes = 0usize; + if let Some(snapshot_sets) = snapshot_addr_sets { + snapshot_dial_candidates = snapshot_sets.len(); + let deadline = tokio::time::Instant::now() + SNAPSHOT_RESTORE_BUDGET; + let mut snapshot_stream = futures::stream::iter(snapshot_sets.into_iter().map( + |(expected_peer_id, addrs)| async move { + self.dial_bootstrap_addr_set( + &addrs, + identity_timeout, + "routing_snapshot", + Some(expected_peer_id), + ) + .await + }, + )) + .buffer_unordered(MAX_CONCURRENT_SNAPSHOT_DIALS); + + loop { + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + debug!( + snapshot_dial_successes, + snapshot_dial_candidates, + "Routing snapshot restore hit its time budget; the rest is left to \ + ordinary discovery" + ); + break; + } + match tokio::time::timeout(remaining, snapshot_stream.next()).await { + Ok(Some(Some(peer_id))) => { + snapshot_dial_successes += 1; + successful_connections += 1; + connected_peer_ids.push(peer_id); + } + Ok(Some(None)) => {} + Ok(None) => break, + Err(_) => { + debug!("Routing snapshot restore timed out awaiting a dial"); + break; + } + } + } + } + info!( cache_dial_candidates, cache_dial_successes, configured_dial_candidates, configured_dial_successes, + snapshot_dial_candidates, + snapshot_dial_successes, outbound_bootstrap_successes = successful_connections, outbound_reachable = successful_connections > 0, "Bootstrap reachability summary" @@ -2302,6 +2430,102 @@ impl P2PNode { None } + /// Load, validate and prepare the routing snapshot's dial candidates. + /// + /// Returns `None` when there is nothing usable — no configured directory, no + /// file, a rejected file, or restoration switched off. Every one of those is + /// logged with its reason: a node that quietly cold-starts on every restart + /// is indistinguishable from one that never had a snapshot, and that is + /// exactly the failure an operator needs to be able to see. + /// + /// Addresses already queued by an earlier priority are removed here, and + /// `seen_addresses` is extended, so no peer is dialled twice. + async fn routing_snapshot_dial_sets( + &self, + seen_addresses: &mut std::collections::HashSet, + ) -> Option)>> { + let dir = self.config.close_group_cache_dir.as_ref()?; + + let snapshot = match RoutingSnapshot::load_from_dir(dir).await { + Ok(Some(snapshot)) => snapshot, + Ok(None) => { + debug!("No routing snapshot on disk; starting from configured peers"); + return None; + } + Err(rejection) => { + warn!(%rejection, "Discarding routing snapshot"); + return None; + } + }; + + let now_epoch = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()); + let peers = match snapshot.validate( + &self.peer_id, + &network_fingerprint(&self.config.bootstrap_peers), + now_epoch, + self.config.close_group_cache_max_age, + ) { + Ok(peers) => peers, + Err(rejection) => { + warn!( + %rejection, + snapshot_peers = snapshot.peer_count(), + "Discarding routing snapshot" + ); + return None; + } + }; + + if !self.config.routing_snapshot_restore { + info!( + snapshot_peers = peers.len(), + "Routing snapshot is valid but restoration is switched off; \ + the routing table will refill at the ordinary refresh cadence" + ); + return None; + } + + let mut sets: Vec<(PeerId, Vec)> = Vec::new(); + for peer in peers { + if peer.peer_id == self.peer_id { + continue; + } + let new_addresses: Vec = peer + .addresses + .iter() + .filter(|addr| { + addr.dialable_socket_addr() + .is_some_and(|socket| !seen_addresses.contains(&socket)) + }) + .cloned() + .collect(); + if new_addresses.is_empty() { + continue; + } + for addr in &new_addresses { + if let Some(socket) = addr.socket_addr() { + seen_addresses.insert(socket); + } + } + sets.push((peer.peer_id, new_addresses)); + } + + if sets.is_empty() { + debug!("Routing snapshot added no candidates beyond those already queued"); + return None; + } + + info!( + snapshot_peers = peers.len(), + new_candidates = sets.len(), + age_secs = now_epoch.saturating_sub(snapshot.payload.saved_at_epoch_secs), + "Restoring routing table from snapshot" + ); + Some(sets) + } + /// Persist the current close group peers and their trust scores to disk. async fn save_close_group_cache( &self, @@ -2313,6 +2537,7 @@ impl P2PNode { self.adaptive_dht.trust_engine(), self.peer_id, self.config.dht_config.k_value, + &network_fingerprint(&self.config.bootstrap_peers), dir, save_reason, ) @@ -2331,6 +2556,7 @@ async fn save_close_group_cache_snapshot( trust_engine: &TrustEngine, peer_id: PeerId, k_value: usize, + network_fingerprint: &str, dir: &Path, save_reason: &'static str, ) -> anyhow::Result<()> { @@ -2373,15 +2599,78 @@ async fn save_close_group_cache_snapshot( peer_count, dir.display() ); + + // Write the whole routing table alongside the close group. The close group + // is what reconnects a neighbourhood; only the full table can reconstruct + // the knowledge a node uses to decide whether anyone is closer to a given + // key than it is. Written unconditionally, including when restoration is + // switched off, so the kill switch never costs the next start its snapshot. + // + // A failure to write the snapshot must not fail the close-group save that + // already succeeded, so it is logged and swallowed. + save_routing_snapshot( + dht_manager, + peer_id, + network_fingerprint, + now_epoch, + dir, + save_reason, + ) + .await; + Ok(()) } +/// Persist the whole routing table as a [`RoutingSnapshot`]. +/// +/// Best-effort and non-fatal: the close-group cache is the established path and +/// must not start failing because a newer, unread file could not be written. +async fn save_routing_snapshot( + dht_manager: &DhtNetworkManager, + peer_id: PeerId, + network_fingerprint: &str, + now_epoch: u64, + dir: &Path, + save_reason: &'static str, +) { + let peers: Vec = dht_manager + .routing_table_peers() + .await + .into_iter() + .map(|node| SnapshotPeer { + peer_id: node.peer_id, + addresses: node.addresses, + }) + .collect(); + let peer_count = peers.len(); + + let snapshot = + match RoutingSnapshot::new(peer_id, network_fingerprint.to_string(), now_epoch, peers) { + Ok(snapshot) => snapshot, + Err(error) => { + warn!(save_reason, %error, "Failed to build routing snapshot"); + return; + } + }; + + match snapshot.save_to_dir(dir).await { + Ok(()) => info!( + save_reason, + peer_count, + "Saved routing snapshot ({peer_count} peers) to {}", + dir.display() + ), + Err(error) => warn!(save_reason, %error, "Failed to save routing snapshot"), + } +} + /// Periodically persist the close group until cancelled. async fn periodic_close_group_cache_save( dht_manager: Arc, trust_engine: Arc, peer_id: PeerId, k_value: usize, + network_fingerprint: String, dir: PathBuf, interval: Duration, shutdown: CancellationToken, @@ -2400,6 +2689,7 @@ async fn periodic_close_group_cache_save( &trust_engine, peer_id, k_value, + &network_fingerprint, &dir, "periodic", ).await { @@ -2490,6 +2780,7 @@ mod tests { adaptive_dht_config: AdaptiveDhtConfig::default(), close_group_cache_dir: None, close_group_cache_max_age: default_close_group_cache_max_age(), + routing_snapshot_restore: default_routing_snapshot_restore(), } } From 4fe64611bf65edde385f992cf181fa1b60030923 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 17:29:44 +0900 Subject: [PATCH 2/8] docs(adr): record the routing-table snapshot decision ADR-017 states why a close-group cache cannot answer responsibility questions after a restart, and what the snapshot does instead. The reasoning is combinatorial: every peer in the bucket a key falls into is strictly closer to that key than self is, so a restored table answers correctly wherever those buckets are populated, and cannot where they are empty. That is why the snapshot carries every bucket rather than the nearest k peers, and why a per-bucket floor of nine is not enough at the wider width a storage consumer uses. Records the acceptance bindings and the reasons they are load-bearing, the dial-candidate contract, the deliberate absence of trust restoration, the bounded dial budget that degrades to today's behaviour, the alternatives that were measured and rejected, and the evidence that does not yet exist. --- ...7-routing-table-snapshot-across-restart.md | 70 +++++++++++++++++++ docs/adr/README.md | 1 + 2 files changed, 71 insertions(+) create mode 100644 docs/adr/ADR-017-routing-table-snapshot-across-restart.md diff --git a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md new file mode 100644 index 0000000..5b4c9a9 --- /dev/null +++ b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md @@ -0,0 +1,70 @@ +# ADR-017: Persist the Whole Routing Table Across a Restart + +## Status + +Proposed + +## Context + +`saorsa-core` persists exactly one thing across a restart: the close-group cache, which holds the `k` peers nearest to self. Everything else in the routing table is discarded and rebuilt by periodic bucket refresh at two buckets per 7.5 to 12.5 minutes, a cadence this crate's own source comment describes as approximately once-per-day full-table maintenance. + +Consumers above this crate do not ask "who are my neighbours". They ask "am I among the `w` closest to key `K`", which `find_closest_nodes_local_with_self` answers from whatever the local table happens to hold. That predicate has no notion of confidence and no failure value: if the table cannot name `w` peers closer to `K` than self, the answer is yes. A node that has just restarted therefore answers yes for most of the keyspace, and keeps doing so for hours. + +The effect is measurable in production rather than theoretical. On `ant-prod-01`, two independent restarted services were ranked against the 806-peer fleet by XOR distance to 200 keys each was actively claiming: median true rank 27 and 24, with **0 of 200 keys placing them in the true closest 7 or the true closest 9**. Fleet-wide, restarted nodes report 40 to 60% of their stored records as in range where a converged table reports about 1%. The observable costs attributed to this window are excess records taken in per service at each rollout, a warning flood from probing peers that are not the real holders, and pruning suppressed while the node believes it is responsible for what it holds. + +A close group cannot stand in for a routing table here, and the reason is combinatorial rather than a matter of degree. For a key `K`, write `c = CPL(self, K)`. Any peer `p` with `CPL(p, self) == c` agrees with self on bits `[0, c)` and differs at bit `c`; `K` agrees with self on `[0, c)` and also differs at bit `c`; therefore `p` agrees with `K` at bit `c` and is strictly closer to `K` than self is. Every peer in bucket `c` answers the question. A node holding `w` of them always answers correctly; a node whose bucket `c` is empty cannot, however many neighbours it has. Preserving peers across every bucket is what makes a restored table answer as the original did. + +Simulated on an 891-node network at the two widths a storage consumer uses, against correct shares of 1.01% and 2.24%: + +| restored table | width 9 claim | width 20 claim | +|---|---|---| +| converged table (steady state) | 1.05% | 2.01% | +| today: nearest 20 only | 41.4% | 95.6% | +| nine peers per bucket | 1.05% | 12.3% | +| whole capped table (~129 entries, ~25 KB) | 1.05% | 2.01% | + +## Decision + +Persist the whole capped routing table to its own snapshot file, and restore it as dial candidates at startup. + +1. **Separate file.** The snapshot is written alongside the close-group cache, not in place of it, so an older binary cannot mistake one for the other and a downgrade keeps working exactly as today. +2. **Every bucket, not the nearest `k`.** The snapshot carries the table up to its per-bucket capacity. Nine peers per bucket fixes the narrow width and leaves the wide one at 12.3%, so the shape is the whole table rather than a per-bucket floor. +3. **Bindings, all required before a snapshot is accepted.** Schema version; owning node id; a network fingerprint derived from the configured bootstrap set; an integrity checksum over the payload; and age, reusing the close-group cache's rules including the rejection of timestamps far in the future. The owner binding is load-bearing rather than hygiene: bucket indices are relative to the owner, so another node's snapshot describes a different partition of the id space. +4. **Restored peers are dial candidates only.** They are not inserted into the routing table and confer no authority until dialled and identity-verified through the ordinary path. Routing-table membership is an authorization fact for callers above this crate, and a file on disk must not be able to grant it. +5. **No trust scores are recorded or restored.** The close-group cache imports trust before dialling, which is defensible for `k` vetted neighbours. It is not defensible for a whole table: a file on disk must not decide that hundreds of unverified peers start above neutral. Trust is re-earned from live behaviour. +6. **Bounded cost, then fall back to today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger and the existing path is serial) under a wall-clock budget of 20 seconds. When the budget expires the remaining candidates are abandoned and the table refills at the ordinary refresh cadence, which is precisely the current behaviour. +7. **On by default, with a kill switch.** `NodeConfig::routing_snapshot_restore` defaults to `true`; setting it to `false` restores the pre-change startup path without a rollback. + +## Alternatives considered + +- **Keep the close-group cache and choose its 20 peers more cleverly.** Rejected on measurement: at that size composition is second-order. Nearest-20 claims 40.5% of the keyspace at width 9 and an arbitrary 20 claims 43.3%. Size dominates. +- **Persist a fixed floor of peers per bucket.** Rejected as insufficient rather than wrong. Nine per bucket answers width 9 correctly but leaves width 20 at 12.3%, and this crate does not know the widths its consumers use. +- **Estimate network size locally and refuse keys beyond an inferred horizon.** Rejected on two grounds. It admits every key out to true rank `slack × width`, which at the studied slack is rank 36, above the rank 24 to 27 band the production over-claim actually occupied: it caught 14.3% of that band with 6 of 10 simulated nodes catching none. It is also punishable, because a refusal resurfaces as an absent answer at the requester's audit, and peers near a victim's id can shrink its estimate. +- **Wait out the transient by tuning the consumer's retention timers.** Rejected: it treats a wrong answer as a scheduling problem, and it makes nodes act fastest exactly when their routing table is least trustworthy. + +## Consequences + +### Positive + +- A restarted node answers responsibility questions as its converged self did, at every width, instead of claiming most of the keyspace for hours. +- The fix is combinatorial, so it does not depend on network size, key distribution, or an estimator an adversary could move. +- Startup performs less discovery work overall: peers are read from disk rather than re-learned over a day of bucket refreshes. +- Consumers gain nothing new to configure. The predicate, its widths and its call sites are untouched. + +### Negative + +- A new on-disk artifact to version, validate and keep compatible. +- Startup dials a larger candidate set, bounded by the concurrency limit and the 20-second budget. +- A snapshot full of departed peers costs that budget and yields little, though never more than it. +- Peers restored from a snapshot are unverified until dialled, so the table refills slightly behind the file's contents rather than instantly. + +### Neutral + +- The close-group cache remains, unchanged, with its own trust import and its own validity rules. +- A brand-new node with no snapshot is unaffected and still refills at the ordinary refresh cadence. Accelerating that case is a separate change and is deliberately not in scope here. + +## Validation + +- Simulation over an 891-node network at widths 9 and 20, table above, reproducing the shape of the production over-claim and the effect of each candidate snapshot. +- Unit tests covering the bindings that decide acceptance (schema version, owner, network fingerprint, checksum, age), the dial-candidate contract, absence of trust restoration, budget expiry leaving the node in today's behaviour, and the kill switch. +- Not yet evidenced: any production or testnet measurement of this change. The over-claim itself is measured in production; the fix is measured only in simulation and tests, and a testnet run is the next step rather than something this ADR claims. diff --git a/docs/adr/README.md b/docs/adr/README.md index 72a4d4e..17d6c4d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -43,6 +43,7 @@ An Architecture Decision Record (ADR) is a document that captures an important a | [ADR-007](./ADR-007-adaptive-networking.md) | Adaptive Networking with ML | Accepted | Machine learning for dynamic routing optimization | | [ADR-008](./ADR-008-bootstrap-delegation.md) | Bootstrap Peer Discovery Scope | Superseded | Historical peer discovery design replaced by configured peers plus DHT discovery | | [ADR-016](./ADR-016-close-group-cache-validity.md) | Age-Bounded, Periodically Refreshed Close-Group Cache | Proposed | Bound persisted close-group age and refresh it safely during normal running | +| [ADR-017](./ADR-017-routing-table-snapshot-across-restart.md) | Persist the Whole Routing Table Across a Restart | Proposed | Restore every bucket as dial candidates so a restarted node stops claiming most of the keyspace | ### Messaging From 0bb0c122af9fc49a043f136e2dc7f50a57a9b6a5 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 18:02:17 +0900 Subject: [PATCH 3/8] fix(bootstrap): bound the snapshot restore and keep it out of discovery Review found the restore handed its successes to `bootstrap_from_peers`, which issues a serial FIND_NODE per seed and then serially dials every peer those queries return. The phase bounded its own dials at 16 concurrent under a 20 second budget and then fed ~130 peers into a path with no bound at all, so a restore could extend startup by tens of minutes. The queries were also redundant: a dialled, identity-verified peer is already admitted to the routing table, so this was re-discovering the table just restored. Snapshot successes now count towards reachability only. The budget no longer drops dials in flight. It stops new ones and lets the rest finish under the identity timeout, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. Loading is now bounded: regular files only, 512 KB ceiling checked before the read, and the peer and address caps re-applied after parsing so the bound holds for a file this process did not write. Restoration is once per process and skipped for clients. A re-bootstrap would have replayed it to rebuild the table the node already had, and a client never asks whether it is responsible for a key, so it keeps its existing six-peer startup bound. The post-bootstrap save no longer writes the snapshot. At that moment the table holds only what the restore re-dialled, so writing it replaced a complete snapshot with a partial one and every restart shrank it further. The periodic and shutdown saves still write it. Cut, on the same review: the network fingerprint, which hashed mutable bootstrap-address spellings rather than a stable network identity and would have invalidated every snapshot on a seed rotation; the payload checksum, which adds no authenticity for a locally written file that JSON parsing already accepts or rejects whole; and the config kill switch, which was new public API for a behaviour that rolls back by shipping the previous version. The snapshot type is now crate-private. The one-hour close-group cache age no longer governs the snapshot. That bound exists for trust scores on k neighbours; bucket coverage does not rot on the same timescale, and a longer maintenance window must not cost a node its table. Seven days, with the same future-skew rejection. Adds restore-path tests for candidate selection (self excluded, already-queued addresses skipped, undialable addresses dropped) and for the load bounds. --- ...7-routing-table-snapshot-across-restart.md | 41 +- src/bootstrap/mod.rs | 15 +- src/bootstrap/routing_snapshot.rs | 554 ++++++------------ src/network.rs | 351 +++++------ 4 files changed, 401 insertions(+), 560 deletions(-) diff --git a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md index 5b4c9a9..c044160 100644 --- a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md +++ b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md @@ -10,11 +10,11 @@ Proposed Consumers above this crate do not ask "who are my neighbours". They ask "am I among the `w` closest to key `K`", which `find_closest_nodes_local_with_self` answers from whatever the local table happens to hold. That predicate has no notion of confidence and no failure value: if the table cannot name `w` peers closer to `K` than self, the answer is yes. A node that has just restarted therefore answers yes for most of the keyspace, and keeps doing so for hours. -The effect is measurable in production rather than theoretical. On `ant-prod-01`, two independent restarted services were ranked against the 806-peer fleet by XOR distance to 200 keys each was actively claiming: median true rank 27 and 24, with **0 of 200 keys placing them in the true closest 7 or the true closest 9**. Fleet-wide, restarted nodes report 40 to 60% of their stored records as in range where a converged table reports about 1%. The observable costs attributed to this window are excess records taken in per service at each rollout, a warning flood from probing peers that are not the real holders, and pruning suppressed while the node believes it is responsible for what it holds. +The effect is measured in production rather than theoretical. On the Autonomi production fleet, two independent restarted services were ranked against the 806-peer fleet by XOR distance to 200 keys each was actively claiming: median true rank 27 and 24, with **0 of 200 keys placing them in the true closest 7 or the true closest 9**. Fleet-wide, restarted nodes reported 40 to 60% of their stored records as in range where a converged table reports about 1%. Those figures come from production telemetry held outside this repository; they are cited here as the motivation, and nothing in this ADR depends on their exact values. -A close group cannot stand in for a routing table here, and the reason is combinatorial rather than a matter of degree. For a key `K`, write `c = CPL(self, K)`. Any peer `p` with `CPL(p, self) == c` agrees with self on bits `[0, c)` and differs at bit `c`; `K` agrees with self on `[0, c)` and also differs at bit `c`; therefore `p` agrees with `K` at bit `c` and is strictly closer to `K` than self is. Every peer in bucket `c` answers the question. A node holding `w` of them always answers correctly; a node whose bucket `c` is empty cannot, however many neighbours it has. Preserving peers across every bucket is what makes a restored table answer as the original did. +A close group cannot stand in for a routing table, and the reason is combinatorial rather than a matter of degree. For a key `K`, write `c = CPL(self, K)`. Any peer `p` with `CPL(p, self) == c` agrees with self on bits `[0, c)` and differs at bit `c`; `K` agrees with self on `[0, c)` and also differs at bit `c`; therefore `p` agrees with `K` at bit `c` and is strictly closer to `K` than self is. Every peer in bucket `c` answers the question. A node holding `w` of them always answers correctly; a node whose bucket `c` is empty cannot, however many neighbours it has. Preserving peers across every bucket is what makes a restored table answer as the original did. -Simulated on an 891-node network at the two widths a storage consumer uses, against correct shares of 1.01% and 2.24%: +Simulated on an 891-node network against correct shares of 1.01% (width 9) and 2.24% (width 20). The simulation lives outside this repository, so these numbers are supporting evidence for the shape of the fix, not a claim this repository can reproduce: | restored table | width 9 claim | width 20 claim | |---|---|---| @@ -27,18 +27,22 @@ Simulated on an 891-node network at the two widths a storage consumer uses, agai Persist the whole capped routing table to its own snapshot file, and restore it as dial candidates at startup. -1. **Separate file.** The snapshot is written alongside the close-group cache, not in place of it, so an older binary cannot mistake one for the other and a downgrade keeps working exactly as today. +1. **Separate file.** The snapshot is written alongside `close_group_cache.json`, not in place of it, so an older binary cannot mistake one for the other and a downgrade behaves exactly as today. 2. **Every bucket, not the nearest `k`.** The snapshot carries the table up to its per-bucket capacity. Nine peers per bucket fixes the narrow width and leaves the wide one at 12.3%, so the shape is the whole table rather than a per-bucket floor. -3. **Bindings, all required before a snapshot is accepted.** Schema version; owning node id; a network fingerprint derived from the configured bootstrap set; an integrity checksum over the payload; and age, reusing the close-group cache's rules including the rejection of timestamps far in the future. The owner binding is load-bearing rather than hygiene: bucket indices are relative to the owner, so another node's snapshot describes a different partition of the id space. -4. **Restored peers are dial candidates only.** They are not inserted into the routing table and confer no authority until dialled and identity-verified through the ordinary path. Routing-table membership is an authorization fact for callers above this crate, and a file on disk must not be able to grant it. -5. **No trust scores are recorded or restored.** The close-group cache imports trust before dialling, which is defensible for `k` vetted neighbours. It is not defensible for a whole table: a file on disk must not decide that hundreds of unverified peers start above neutral. Trust is re-earned from live behaviour. -6. **Bounded cost, then fall back to today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger and the existing path is serial) under a wall-clock budget of 20 seconds. When the budget expires the remaining candidates are abandoned and the table refills at the ordinary refresh cadence, which is precisely the current behaviour. -7. **On by default, with a kill switch.** `NodeConfig::routing_snapshot_restore` defaults to `true`; setting it to `false` restores the pre-change startup path without a rollback. +3. **Two bindings, both required.** The schema version, so an unrecognised version is discarded rather than coerced; and the owning node id, which is load-bearing rather than hygiene, because bucket indices are relative to the owner and another node's snapshot describes a different partition of the id space. A snapshot older than seven days, or dated more than five minutes in the future, is refused. That age bound is the snapshot's own, deliberately not the close-group cache's one hour: the cache's bound exists for trust scores on `k` neighbours, while this file answers which parts of the id space the node knew about, which does not rot on the same timescale, and a maintenance window must not cost a node its table. +4. **Bounded load.** The file is refused if it is not a regular file or exceeds 512 KB, and the peer and per-peer address counts are capped after parsing as well as before writing, so a corrupt or hostile file cannot become an unbounded dial set. +5. **Restored peers are dial candidates only.** They are not inserted into the routing table and confer no authority until dialled and identity-verified through the ordinary path. Routing-table membership is an authorization fact for callers above this crate, and a file on disk must not be able to grant it. For the same reason the snapshot records **no trust scores**, unlike the close-group cache: pre-dial trust restoration is defensible for `k` vetted neighbours and not for hundreds of unverified peers. +6. **Restored peers do not seed DHT discovery.** A dialled, identity-verified peer is already admitted to the routing table by the connection path. Adding the restored set to the discovery seed list would issue a serial `FIND_NODE` per peer to rediscover the table just restored, and then serially dial everything those queries returned, so the phase's own bound would be defeated by the phase after it. +7. **Bounded cost, then today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger). A 20-second budget stops *new* dials; dials already in flight are allowed to finish, each bounded by the identity timeout, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. Whatever is not restored refills through ordinary discovery, which is exactly the behaviour of a node that had no snapshot. +8. **Once per process, and not for clients.** A re-bootstrap does not replay the snapshot, since the table it would restore is the one the node already has. `NodeMode::Client` skips restoration entirely and keeps its existing six-peer startup bound: a client does not serve the DHT, so it never asks the question this repairs. +9. **Written on the periodic and shutdown saves, not the post-bootstrap one.** Right after bootstrap the table holds only what the restore re-dialled, so persisting it then would replace a complete snapshot with a partial one and each restart would shrink it further. ## Alternatives considered - **Keep the close-group cache and choose its 20 peers more cleverly.** Rejected on measurement: at that size composition is second-order. Nearest-20 claims 40.5% of the keyspace at width 9 and an arbitrary 20 claims 43.3%. Size dominates. - **Persist a fixed floor of peers per bucket.** Rejected as insufficient rather than wrong. Nine per bucket answers width 9 correctly but leaves width 20 at 12.3%, and this crate does not know the widths its consumers use. +- **Bind the snapshot to a network fingerprint derived from the configured bootstrap list.** Rejected: it hashes mutable address spellings rather than a stable network identity, and routine seed rotation would invalidate every node's snapshot during exactly the rollout this repairs. A snapshot carried to another network costs failed dials inside the existing budget, which is the same cost as a cold start. +- **Checksum the payload.** Rejected: it provides no authenticity for a locally written file, while JSON parsing is already all-or-nothing, the write is atomic, and live identity verification governs what the contents can achieve. - **Estimate network size locally and refuse keys beyond an inferred horizon.** Rejected on two grounds. It admits every key out to true rank `slack × width`, which at the studied slack is rank 36, above the rank 24 to 27 band the production over-claim actually occupied: it caught 14.3% of that band with 6 of 10 simulated nodes catching none. It is also punishable, because a refusal resurfaces as an absent answer at the requester's audit, and peers near a victim's id can shrink its estimate. - **Wait out the transient by tuning the consumer's retention timers.** Rejected: it treats a wrong answer as a scheduling problem, and it makes nodes act fastest exactly when their routing table is least trustworthy. @@ -48,23 +52,24 @@ Persist the whole capped routing table to its own snapshot file, and restore it - A restarted node answers responsibility questions as its converged self did, at every width, instead of claiming most of the keyspace for hours. - The fix is combinatorial, so it does not depend on network size, key distribution, or an estimator an adversary could move. -- Startup performs less discovery work overall: peers are read from disk rather than re-learned over a day of bucket refreshes. -- Consumers gain nothing new to configure. The predicate, its widths and its call sites are untouched. +- Consumers gain nothing new to configure. The predicate, its widths and its call sites are untouched, and there is no new public API. ### Negative -- A new on-disk artifact to version, validate and keep compatible. -- Startup dials a larger candidate set, bounded by the concurrency limit and the 20-second budget. -- A snapshot full of departed peers costs that budget and yields little, though never more than it. -- Peers restored from a snapshot are unverified until dialled, so the table refills slightly behind the file's contents rather than instantly. +- A new on-disk artifact to version and keep compatible. +- Startup dials a larger candidate set. New dials stop at the 20-second budget, but a dial already in flight can still run to its identity timeout, so the phase can exceed the budget by that much. +- A snapshot full of departed peers spends that budget and yields little, though never more than it. +- Every periodic and shutdown close-group save now writes a second small file before returning. ### Neutral - The close-group cache remains, unchanged, with its own trust import and its own validity rules. - A brand-new node with no snapshot is unaffected and still refills at the ordinary refresh cadence. Accelerating that case is a separate change and is deliberately not in scope here. +- Restored peers are unverified until dialled, so the table refills behind the file's contents rather than instantly. ## Validation -- Simulation over an 891-node network at widths 9 and 20, table above, reproducing the shape of the production over-claim and the effect of each candidate snapshot. -- Unit tests covering the bindings that decide acceptance (schema version, owner, network fingerprint, checksum, age), the dial-candidate contract, absence of trust restoration, budget expiry leaving the node in today's behaviour, and the kill switch. -- Not yet evidenced: any production or testnet measurement of this change. The over-claim itself is measured in production; the fix is measured only in simulation and tests, and a testnet run is the next step rather than something this ADR claims. +- Unit tests in `src/bootstrap/routing_snapshot.rs` for the file contract: disk round trip, missing file treated as absence, truncated file reported, oversized file refused without being read, peer cap re-applied on load, foreign owner refused, unknown schema version refused, and staleness bounded on both sides of now. +- Unit tests in `src/network.rs` for the restore path's candidate selection: self excluded, addresses already queued by an earlier bootstrap priority not redialled, undialable addresses dropped, and a full table producing one candidate per peer. +- Not covered by tests in this PR: the dial phase itself against a live transport, including budget expiry and client-mode exclusion. Those need a multi-node harness. +- **No testnet or production measurement of this change exists.** The over-claim it targets is measured in production; the fix is evidenced by simulation and unit tests only. A dev testnet run is the next step, and nothing here claims fleet readiness. diff --git a/src/bootstrap/mod.rs b/src/bootstrap/mod.rs index 78af79e..753ac84 100644 --- a/src/bootstrap/mod.rs +++ b/src/bootstrap/mod.rs @@ -12,18 +12,11 @@ //! Persisted peer knowledge for warm-starting across restarts. //! -//! Two files, written side by side: -//! -//! - [`cache`] holds the `k` peers nearest to self — the close group. Kept so a -//! downgrade to an older binary still finds what it expects. -//! - [`routing_snapshot`] holds the whole routing table. A close group cannot -//! reconstruct a routing table, because the peers that answer "is anyone -//! closer to this key than me?" for a distant key are exactly the ones a -//! close group leaves out. See that module for why this is combinatorial -//! rather than a matter of degree. +//! Two files, written side by side: [`cache`] holds the `k` peers nearest to +//! self, and [`routing_snapshot`] holds the whole routing table, which is what +//! a node consults to decide whether anyone is closer to a given key than it is. pub mod cache; -pub mod routing_snapshot; +pub(crate) mod routing_snapshot; pub use cache::{CachedCloseGroupPeer, CloseGroupCache}; -pub use routing_snapshot::{RoutingSnapshot, SnapshotPeer, network_fingerprint}; diff --git a/src/bootstrap/routing_snapshot.rs b/src/bootstrap/routing_snapshot.rs index 6f766df..a8af56a 100644 --- a/src/bootstrap/routing_snapshot.rs +++ b/src/bootstrap/routing_snapshot.rs @@ -10,69 +10,21 @@ // distributed under these licenses is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -//! Routing snapshot: the whole routing table, persisted across a restart. -//! -//! # Why the close-group cache is not enough +//! The whole routing table, persisted across a restart. //! //! [`CloseGroupCache`](super::cache::CloseGroupCache) persists the `k` peers -//! nearest to self. That is the right shape for reconnecting to a close group, -//! and the wrong shape for reconstructing a routing table, because it drops -//! every peer that is not a neighbour — which is precisely the population a -//! node consults to answer "is anyone closer to this key than me?". -//! -//! The consequence is combinatorial, not statistical. For a key `K`, write -//! `c = CPL(self, K)` for the number of leading bits they share. Any peer `p` -//! with `CPL(p, self) == c` agrees with self on bits `[0, c)` and differs at -//! bit `c`; `K` agrees with self on `[0, c)` and also differs at bit `c`; -//! therefore `p` agrees with `K` at bit `c`, giving `CPL(p, K) >= c + 1`. -//! **Every peer in bucket `c` is strictly closer to `K` than self is.** -//! -//! So a node holding `w` peers in bucket `c` can always answer "no, I am not -//! among the `w` closest to `K`" — and a node whose bucket `c` is empty cannot, -//! however many neighbours it has. A snapshot must therefore preserve peers -//! across *every* bucket, not the nearest `k` overall. Preserving up to the -//! bucket capacity is what makes the restored table answer as the original did, -//! for every key, at every width. -//! -//! # What a snapshot is, and is not -//! -//! Restored peers are **dial candidates**. They are not inserted into the -//! routing table, are not trusted, and confer no authority until they have been -//! dialled and identity-verified through the ordinary path — routing-table -//! membership is an authorization fact for callers above this crate, and a file -//! on disk must never be able to grant it. -//! -//! For the same reason a snapshot records **no trust scores**. The close-group -//! cache does carry them, and imports them into the `TrustEngine` before its -//! peers are dialled, which is defensible for a set of `k` neighbours a node has -//! already vetted. It is not defensible for a whole-table file: pre-dial trust -//! restoration would let a file on disk decide that hundreds of unverified peers -//! start above neutral. A snapshot answers "where were my peers", never "how -//! much did I trust them" — trust is re-earned from live behaviour. -//! -//! # Bindings -//! -//! A snapshot is accepted only when every binding holds: +//! nearest to self, which is the right shape for reconnecting to a close group +//! and the wrong shape for reconstructing a routing table: the peers that +//! answer "is anyone closer to this key than me?" for a *distant* key are +//! exactly the ones a close group leaves out. For a key `K` sharing `c` leading +//! bits with self, every peer in bucket `c` is strictly closer to `K` than self +//! is, so a node that kept those peers answers correctly and a node whose +//! bucket `c` is empty cannot, however many neighbours it has. //! -//! - **Schema version.** An unknown version is discarded rather than guessed at. -//! - **Owner.** Bucket indices are relative to the owning node's id, so another -//! node's snapshot is not merely stale, it is *meaningless* — it describes a -//! different partition of the id space. This is the binding that matters most. -//! - **Network fingerprint.** Derived from the configured bootstrap set, so a -//! snapshot does not follow a node between networks. Bootstrap lists do change -//! legitimately; the cost of a mismatch is one cold start, which is exactly -//! today's behaviour, so failing closed here is cheap. -//! - **Integrity.** A checksum over the payload, so a truncated or bit-flipped -//! file is rejected instead of partially believed. -//! - **Age.** Reuses the close-group cache's rules, including rejecting -//! timestamps far in the future so a broken clock cannot make a snapshot look -//! fresh forever. -//! -//! None of these is a defence against an attacker who can write to the node's -//! data directory: such an attacker owns the node. They defend against the -//! accidents that actually happen — copied directories, cloned images, rolled -//! back filesystems, half-written files, and a snapshot outliving the network it -//! was taken on. +//! Restored peers are **dial candidates only**. They are dialled and +//! identity-verified through the ordinary path before they can enter the +//! routing table, and the snapshot carries no trust scores, because a file on +//! disk must not grant routing-table membership or above-neutral trust. use std::io::Write as _; use std::path::Path; @@ -83,235 +35,139 @@ use serde::{Deserialize, Serialize}; use crate::PeerId; use crate::address::MultiAddr; -/// A peer recorded in a routing snapshot. +/// Filename for the routing snapshot. +/// +/// Distinct from `close_group_cache.json`, which is still written alongside it, +/// so an older binary never reads a whole-table snapshot as a close group. +pub(crate) const ROUTING_SNAPSHOT_FILENAME: &str = "routing_snapshot.json"; + +/// Schema version. An unrecognised version is discarded, never coerced. +const SCHEMA_VERSION: u32 = 1; + +/// Maximum age of a snapshot that is still worth dialling. /// -/// Identity and addresses only. See the module docs for why no trust score is -/// carried: this file must not be able to promote unverified peers above -/// neutral before they have been dialled. +/// Deliberately not the close-group cache's one hour. That bound exists for +/// trust scores on `k` neighbours; this file answers which parts of the id +/// space the node knew about, which does not rot on the same timescale. A +/// maintenance window or a host move must not cost a node its table, and a +/// snapshot of departed peers already costs nothing beyond failed dials. +const MAX_AGE: Duration = Duration::from_secs(7 * 24 * 60 * 60); + +/// Tolerated wall-clock skew for a timestamp in the future, so a clock jump +/// cannot make a snapshot look fresh indefinitely. +const MAX_FUTURE_TIMESTAMP_SKEW: Duration = Duration::from_secs(5 * 60); + +/// Largest snapshot file that will be read. A full table is ~25 KB; anything +/// past this is not a snapshot this node wrote. +const MAX_SNAPSHOT_BYTES: u64 = 512 * 1024; + +/// Hard caps applied when writing and after parsing, so a corrupt or hostile +/// file cannot turn into an unbounded dial set. +const MAX_SNAPSHOT_PEERS: usize = 1024; +const MAX_ADDRESSES_PER_PEER: usize = 8; + +/// A peer recorded in a snapshot: identity and addresses, nothing else. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SnapshotPeer { +pub(crate) struct SnapshotPeer { /// Peer identity, re-verified on dial before it can enter the routing table. pub peer_id: PeerId, /// Addresses last known to reach this peer. pub addresses: Vec, } -/// Filename for the routing snapshot. -/// -/// Deliberately distinct from `close_group_cache.json`: an older binary must -/// never read this file and mistake a whole-table snapshot for a close group. -/// The two are written side by side so a downgrade keeps working. -pub const ROUTING_SNAPSHOT_FILENAME: &str = "routing_snapshot.json"; - -/// Schema version for [`RoutingSnapshot`]. -/// -/// Bump on any change to the payload's meaning. An unrecognised version is -/// discarded, never coerced. -pub const ROUTING_SNAPSHOT_SCHEMA_VERSION: u32 = 1; - -/// Maximum tolerated wall-clock skew for a snapshot timestamp in the future. -const MAX_FUTURE_TIMESTAMP_SKEW: Duration = Duration::from_secs(5 * 60); - /// Why a snapshot on disk was not used. /// -/// Every variant is a reason an operator may need to see: a node that silently -/// cold-starts every time looks identical to one with no snapshot at all. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum SnapshotRejection { +/// A node that silently cold-starts looks identical to one that never had a +/// snapshot, so every rejection is reportable. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub(crate) enum SnapshotRejection { /// The schema version is not one this build understands. + #[error("unknown schema version {found} (this build writes {expected})")] UnknownSchemaVersion { /// Version found in the file. found: u32, /// Version this build writes. expected: u32, }, - /// The snapshot was written by a different node. + /// The snapshot was written by a different node. Bucket indices are + /// relative to the owner, so another node's snapshot is meaningless here. + #[error("written by a different node")] ForeignOwner, - /// The snapshot was taken on a different network. - ForeignNetwork, - /// The checksum does not match the payload. - CorruptChecksum, - /// The snapshot is older than the configured maximum age, or its timestamp - /// is implausibly far in the future. + /// Older than [`MAX_AGE`], or dated implausibly far in the future. + #[error("stale or implausibly future-dated")] Stale, - /// The file could not be parsed at all. - Unreadable, + /// The file exists but could not be used. + #[error("unusable snapshot file: {0}")] + Unreadable(String), } -impl std::fmt::Display for SnapshotRejection { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::UnknownSchemaVersion { found, expected } => { - write!( - f, - "unknown schema version {found} (this build writes {expected})" - ) - } - Self::ForeignOwner => write!(f, "written by a different node"), - Self::ForeignNetwork => write!(f, "taken on a different network"), - Self::CorruptChecksum => write!(f, "checksum mismatch"), - Self::Stale => write!(f, "stale or implausibly future-dated"), - Self::Unreadable => write!(f, "unparseable"), - } - } -} - -/// The checksummed body of a snapshot. -/// -/// Split from the envelope so the checksum covers exactly the bytes whose -/// integrity is being asserted, and so adding an envelope field later cannot -/// silently change what was signed for. +/// A persisted routing table. #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RoutingSnapshotPayload { - /// Schema version of this payload. +pub(crate) struct RoutingSnapshot { + /// Schema version of this file. pub schema_version: u32, - /// Node that wrote the snapshot. Bucket indices are relative to this id. + /// Node that wrote it. Bucket indices are relative to this id. pub owner: PeerId, - /// Fingerprint of the network the snapshot was taken on. - pub network_fingerprint: String, - /// When the snapshot was written (seconds since UNIX epoch). + /// When it was written (seconds since UNIX epoch). pub saved_at_epoch_secs: u64, /// Every routing-table peer at the time of writing, across all buckets. pub peers: Vec, } -/// A persisted routing table, with the bindings needed to use it safely. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RoutingSnapshot { - /// The checksummed body. - pub payload: RoutingSnapshotPayload, - /// Hex-encoded BLAKE3 of the canonical payload encoding. - pub checksum: String, -} - -/// Derive a network fingerprint from the configured bootstrap addresses. -/// -/// Order-independent, so reordering the configured list is not a change of -/// network. An empty list yields a well-known fingerprint so that a node with no -/// configured bootstrap peers still round-trips its own snapshot. -#[must_use] -pub fn network_fingerprint(bootstrap_peers: &[MultiAddr]) -> String { - let mut rendered: Vec = bootstrap_peers.iter().map(ToString::to_string).collect(); - rendered.sort_unstable(); - let mut hasher = blake3::Hasher::new(); - hasher.update(b"saorsa-routing-snapshot-network-v1"); - for entry in &rendered { - hasher.update(entry.as_bytes()); - hasher.update(b"\n"); - } - hex::encode(hasher.finalize().as_bytes()) -} - -impl RoutingSnapshotPayload { - /// Canonical checksum over this payload. - /// - /// Computed from the serialized form so it covers every field without a - /// hand-maintained list that could drift as fields are added. - fn checksum(&self) -> anyhow::Result { - let encoded = serde_json::to_vec(self) - .map_err(|e| anyhow::anyhow!("failed to encode routing snapshot payload: {e}"))?; - let mut hasher = blake3::Hasher::new(); - hasher.update(b"saorsa-routing-snapshot-payload-v1"); - hasher.update(&encoded); - Ok(hex::encode(hasher.finalize().as_bytes())) - } -} - impl RoutingSnapshot { - /// Build a snapshot from the current routing table. - /// - /// # Errors - /// - /// Returns an error if the payload cannot be encoded for checksumming. - pub fn new( - owner: PeerId, - network_fingerprint: String, - saved_at_epoch_secs: u64, - peers: Vec, - ) -> anyhow::Result { - let payload = RoutingSnapshotPayload { - schema_version: ROUTING_SNAPSHOT_SCHEMA_VERSION, + /// Build a snapshot of the current routing table, capped. + pub fn new(owner: PeerId, saved_at_epoch_secs: u64, mut peers: Vec) -> Self { + peers.truncate(MAX_SNAPSHOT_PEERS); + for peer in &mut peers { + peer.addresses.truncate(MAX_ADDRESSES_PER_PEER); + } + Self { + schema_version: SCHEMA_VERSION, owner, - network_fingerprint, saved_at_epoch_secs, peers, - }; - let checksum = payload.checksum()?; - Ok(Self { payload, checksum }) - } - - /// Number of peers carried. - #[must_use] - pub fn peer_count(&self) -> usize { - self.payload.peers.len() + } } - /// Check every binding, returning the peers only if all of them hold. - /// - /// Checked cheapest-first, and integrity before meaning: there is no point - /// interpreting fields from a file that failed its checksum. + /// The peers this node may dial, or why the snapshot was not used. /// /// # Errors /// - /// Returns the first binding that failed, so the caller can log why a - /// snapshot was discarded rather than reporting a bare absence. - pub fn validate( + /// Returns the first binding that failed, so the caller can log why. + pub fn peers_for( &self, expected_owner: &PeerId, - expected_network: &str, now_epoch_secs: u64, - max_age: Option, ) -> Result<&[SnapshotPeer], SnapshotRejection> { - if self.payload.schema_version != ROUTING_SNAPSHOT_SCHEMA_VERSION { + if self.schema_version != SCHEMA_VERSION { return Err(SnapshotRejection::UnknownSchemaVersion { - found: self.payload.schema_version, - expected: ROUTING_SNAPSHOT_SCHEMA_VERSION, + found: self.schema_version, + expected: SCHEMA_VERSION, }); } - let Ok(expected_checksum) = self.payload.checksum() else { - return Err(SnapshotRejection::CorruptChecksum); - }; - if expected_checksum != self.checksum { - return Err(SnapshotRejection::CorruptChecksum); - } - if self.payload.owner != *expected_owner { + if self.owner != *expected_owner { return Err(SnapshotRejection::ForeignOwner); } - if self.payload.network_fingerprint != expected_network { - return Err(SnapshotRejection::ForeignNetwork); - } - if self.is_stale(now_epoch_secs, max_age) { + if self.is_stale(now_epoch_secs) { return Err(SnapshotRejection::Stale); } - Ok(&self.payload.peers) + Ok(&self.peers) } - /// Whether the snapshot is older than `max_age`, or dated implausibly far - /// in the future. - /// - /// `None` disables the maximum-age check; a future timestamp beyond the - /// tolerated skew is always rejected, so a broken clock cannot make a - /// snapshot look fresh indefinitely. - #[must_use] - pub fn is_stale(&self, now_epoch_secs: u64, max_age: Option) -> bool { - let future_skew = self - .payload - .saved_at_epoch_secs - .saturating_sub(now_epoch_secs); + /// Older than [`MAX_AGE`], or dated implausibly far in the future. + fn is_stale(&self, now_epoch_secs: u64) -> bool { + let future_skew = self.saved_at_epoch_secs.saturating_sub(now_epoch_secs); if future_skew > MAX_FUTURE_TIMESTAMP_SKEW.as_secs() { return true; } - max_age.is_some_and(|max_age| { - now_epoch_secs.saturating_sub(self.payload.saved_at_epoch_secs) > max_age.as_secs() - }) + now_epoch_secs.saturating_sub(self.saved_at_epoch_secs) > MAX_AGE.as_secs() } /// Write the snapshot to `{dir}/routing_snapshot.json`. /// - /// Atomic: written to a uniquely-named temporary file in the same directory - /// and persisted by rename, so a crash mid-write leaves either the previous - /// snapshot or none — never a half-written one. The checksum makes a - /// half-written file detectable even if the platform's rename is not atomic. + /// Written to a temporary file in the same directory and persisted by + /// rename, so a crash mid-write leaves either the previous snapshot or + /// none, never a half-written one. /// /// # Errors /// @@ -326,7 +182,7 @@ impl RoutingSnapshot { })?; let path = dir.join(ROUTING_SNAPSHOT_FILENAME); - let json = serde_json::to_string_pretty(self) + let json = serde_json::to_vec(self) .map_err(|e| anyhow::anyhow!("failed to serialize routing snapshot: {e}"))?; let dir_owned = dir.to_path_buf(); @@ -334,7 +190,7 @@ impl RoutingSnapshot { let mut tmp = tempfile::NamedTempFile::new_in(&dir_owned).map_err(|e| { anyhow::anyhow!("failed to create temp file in {}: {e}", dir_owned.display()) })?; - tmp.write_all(json.as_bytes()) + tmp.write_all(&json) .map_err(|e| anyhow::anyhow!("failed to write routing snapshot: {e}"))?; tmp.persist(&path).map_err(|e| { anyhow::anyhow!( @@ -350,24 +206,44 @@ impl RoutingSnapshot { /// Read the snapshot from `{dir}/routing_snapshot.json`. /// - /// Returns `Ok(None)` when there is no snapshot, and - /// `Err(SnapshotRejection::Unreadable)` when there is one that cannot be - /// parsed — a corrupt file is a fact worth logging, not an absence. + /// Returns `Ok(None)` when there is no snapshot. A file that exists but is + /// oversized, not a regular file, or unparseable is reported rather than + /// treated as absence, because the two need different operator responses. /// /// # Errors /// - /// Returns [`SnapshotRejection::Unreadable`] for an unreadable or - /// unparseable file. + /// Returns [`SnapshotRejection::Unreadable`] with the underlying reason. pub async fn load_from_dir(dir: &Path) -> Result, SnapshotRejection> { let path = dir.join(ROUTING_SNAPSHOT_FILENAME); - match tokio::fs::read_to_string(&path).await { - Ok(json) => match serde_json::from_str(&json) { - Ok(snapshot) => Ok(Some(snapshot)), - Err(_) => Err(SnapshotRejection::Unreadable), - }, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(_) => Err(SnapshotRejection::Unreadable), + + let metadata = match tokio::fs::metadata(&path).await { + Ok(metadata) => metadata, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(SnapshotRejection::Unreadable(e.to_string())), + }; + if !metadata.is_file() { + return Err(SnapshotRejection::Unreadable("not a regular file".into())); } + if metadata.len() > MAX_SNAPSHOT_BYTES { + return Err(SnapshotRejection::Unreadable(format!( + "{} bytes exceeds the {MAX_SNAPSHOT_BYTES} byte limit", + metadata.len() + ))); + } + + let bytes = tokio::fs::read(&path) + .await + .map_err(|e| SnapshotRejection::Unreadable(e.to_string()))?; + let mut snapshot: Self = serde_json::from_slice(&bytes) + .map_err(|e| SnapshotRejection::Unreadable(e.to_string()))?; + + // Re-apply the write-side caps: the bound has to hold for a file this + // process did not write. + snapshot.peers.truncate(MAX_SNAPSHOT_PEERS); + for peer in &mut snapshot.peers { + peer.addresses.truncate(MAX_ADDRESSES_PER_PEER); + } + Ok(Some(snapshot)) } } @@ -376,8 +252,6 @@ impl RoutingSnapshot { mod tests { use super::*; - const NOW: u64 = 1_700_000_000; - fn peer() -> SnapshotPeer { SnapshotPeer { peer_id: PeerId::random(), @@ -385,26 +259,24 @@ mod tests { } } - fn snapshot(owner: PeerId, network: &str, saved_at: u64, count: usize) -> RoutingSnapshot { - let peers = (0..count).map(|_| peer()).collect(); - RoutingSnapshot::new(owner, network.to_string(), saved_at, peers).unwrap() + fn snapshot(owner: PeerId, saved_at: u64, count: usize) -> RoutingSnapshot { + RoutingSnapshot::new(owner, saved_at, (0..count).map(|_| peer()).collect()) } #[tokio::test] async fn round_trips_through_disk() { - let owner = PeerId::random(); - let snap = snapshot(owner, "net-a", NOW, 40); let dir = tempfile::tempdir().unwrap(); + let owner = PeerId::random(); + let original = snapshot(owner, 1_000, 130); - snap.save_to_dir(dir.path()).await.unwrap(); + original.save_to_dir(dir.path()).await.unwrap(); let loaded = RoutingSnapshot::load_from_dir(dir.path()) .await .unwrap() - .unwrap(); + .expect("snapshot present"); - assert_eq!(loaded.peer_count(), 40); - let peers = loaded.validate(&owner, "net-a", NOW, None).unwrap(); - assert_eq!(peers.len(), 40); + assert_eq!(loaded.peers, original.peers); + assert_eq!(loaded.peers_for(&owner, 1_000).unwrap().len(), 130); } #[tokio::test] @@ -420,143 +292,95 @@ mod tests { #[tokio::test] async fn a_truncated_file_is_reported_not_silently_ignored() { - // Truncation must be distinguishable from "no snapshot": a node that - // cold-starts every time because its file is corrupt looks exactly like - // one that never had a snapshot. let dir = tempfile::tempdir().unwrap(); - let snap = snapshot(PeerId::random(), "net-a", NOW, 8); - snap.save_to_dir(dir.path()).await.unwrap(); + let owner = PeerId::random(); + snapshot(owner, 1_000, 4) + .save_to_dir(dir.path()) + .await + .unwrap(); let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); - let json = tokio::fs::read_to_string(&path).await.unwrap(); - let truncated = &json[..json.len() / 2]; - tokio::fs::write(&path, truncated).await.unwrap(); + let json = std::fs::read_to_string(&path).unwrap(); + std::fs::write(&path, &json[..json.len() / 2]).unwrap(); - assert_eq!( - RoutingSnapshot::load_from_dir(dir.path()) - .await - .unwrap_err(), - SnapshotRejection::Unreadable - ); + assert!(matches!( + RoutingSnapshot::load_from_dir(dir.path()).await, + Err(SnapshotRejection::Unreadable(_)) + )); } - #[test] - fn a_bit_flip_that_keeps_the_json_valid_fails_the_checksum() { - // The case a parser cannot catch: still-valid JSON, wrong contents. - let owner = PeerId::random(); - let mut snap = snapshot(owner, "net-a", NOW, 4); - snap.payload.saved_at_epoch_secs = NOW - 1; + #[tokio::test] + async fn an_oversized_file_is_refused_without_reading_it() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); + std::fs::write(&path, vec![b'x'; (MAX_SNAPSHOT_BYTES + 1) as usize]).unwrap(); - assert_eq!( - snap.validate(&owner, "net-a", NOW, None).unwrap_err(), - SnapshotRejection::CorruptChecksum - ); + assert!(matches!( + RoutingSnapshot::load_from_dir(dir.path()).await, + Err(SnapshotRejection::Unreadable(_)) + )); } - #[test] - fn another_nodes_snapshot_is_refused() { - // Bucket indices are relative to the owner, so a foreign snapshot is - // not stale data — it describes a different partition of the id space. - let snap = snapshot(PeerId::random(), "net-a", NOW, 4); - assert_eq!( - snap.validate(&PeerId::random(), "net-a", NOW, None) - .unwrap_err(), - SnapshotRejection::ForeignOwner - ); + #[tokio::test] + async fn a_file_with_too_many_peers_is_capped_on_load() { + let dir = tempfile::tempdir().unwrap(); + let owner = PeerId::random(); + // Build past the cap by hand: `new` caps on the write side. + let oversized = RoutingSnapshot { + schema_version: SCHEMA_VERSION, + owner, + saved_at_epoch_secs: 1_000, + peers: (0..MAX_SNAPSHOT_PEERS + 50).map(|_| peer()).collect(), + }; + let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); + std::fs::write(&path, serde_json::to_vec(&oversized).unwrap()).unwrap(); + + let loaded = RoutingSnapshot::load_from_dir(dir.path()) + .await + .unwrap() + .expect("snapshot present"); + assert_eq!(loaded.peers.len(), MAX_SNAPSHOT_PEERS); } #[test] - fn a_snapshot_from_another_network_is_refused() { + fn another_nodes_snapshot_is_refused() { let owner = PeerId::random(); - let snap = snapshot(owner, "net-a", NOW, 4); + let someone_else = PeerId::random(); assert_eq!( - snap.validate(&owner, "net-b", NOW, None).unwrap_err(), - SnapshotRejection::ForeignNetwork + snapshot(owner, 1_000, 4).peers_for(&someone_else, 1_000), + Err(SnapshotRejection::ForeignOwner) ); } #[test] fn an_unknown_schema_version_is_refused_rather_than_guessed_at() { let owner = PeerId::random(); - let mut snap = snapshot(owner, "net-a", NOW, 4); - snap.payload.schema_version = ROUTING_SNAPSHOT_SCHEMA_VERSION + 1; - snap.checksum = snap.payload.checksum().unwrap(); - - assert_eq!( - snap.validate(&owner, "net-a", NOW, None).unwrap_err(), - SnapshotRejection::UnknownSchemaVersion { - found: ROUTING_SNAPSHOT_SCHEMA_VERSION + 1, - expected: ROUTING_SNAPSHOT_SCHEMA_VERSION, - } - ); + let mut snap = snapshot(owner, 1_000, 4); + snap.schema_version = SCHEMA_VERSION + 1; + assert!(matches!( + snap.peers_for(&owner, 1_000), + Err(SnapshotRejection::UnknownSchemaVersion { .. }) + )); } #[test] fn staleness_is_bounded_on_both_sides_of_now() { let owner = PeerId::random(); + let saved_at = 1_000_000; + let snap = snapshot(owner, saved_at, 4); - let old = snapshot(owner, "net-a", NOW - 7200, 4); - assert_eq!( - old.validate(&owner, "net-a", NOW, Some(Duration::from_secs(3600))) - .unwrap_err(), - SnapshotRejection::Stale - ); - assert!(old.validate(&owner, "net-a", NOW, None).is_ok()); - - // A clock that jumped forward must not mint an evergreen snapshot. - let future = snapshot(owner, "net-a", NOW + 86_400, 4); - assert_eq!( - future.validate(&owner, "net-a", NOW, None).unwrap_err(), - SnapshotRejection::Stale - ); - - // Small skew is tolerated. - let skewed = snapshot(owner, "net-a", NOW + 60, 4); - assert!(skewed.validate(&owner, "net-a", NOW, None).is_ok()); - } - - #[test] - fn the_network_fingerprint_ignores_ordering_but_not_membership() { - let a: MultiAddr = "/ip4/10.0.1.1/udp/9000/quic".parse().unwrap(); - let b: MultiAddr = "/ip4/10.0.2.1/udp/9000/quic".parse().unwrap(); - let c: MultiAddr = "/ip4/10.0.3.1/udp/9000/quic".parse().unwrap(); - + // Fresh, and still usable well past the close-group cache's one hour. + assert!(snap.peers_for(&owner, saved_at).is_ok()); + assert!(snap.peers_for(&owner, saved_at + 6 * 60 * 60).is_ok()); + // Older than the maximum age. assert_eq!( - network_fingerprint(&[a.clone(), b.clone()]), - network_fingerprint(&[b.clone(), a.clone()]), - "reordering the configured list is not a change of network" + snap.peers_for(&owner, saved_at + MAX_AGE.as_secs() + 1), + Err(SnapshotRejection::Stale) ); - assert_ne!( - network_fingerprint(&[a.clone(), b.clone()]), - network_fingerprint(&[a, b, c]), - "a different bootstrap set is a different network" - ); - assert_eq!( - network_fingerprint(&[]), - network_fingerprint(&[]), - "no configured peers still round-trips" - ); - } - - #[tokio::test] - async fn a_copied_snapshot_does_not_transplant_between_nodes() { - // The accident this actually guards: a data directory copied to a new - // host, or a cloned VM image, where the identity differs. - let dir = tempfile::tempdir().unwrap(); - snapshot(PeerId::random(), "net-a", NOW, 30) - .save_to_dir(dir.path()) - .await - .unwrap(); - - let loaded = RoutingSnapshot::load_from_dir(dir.path()) - .await - .unwrap() - .unwrap(); + // Dated further in the future than tolerated skew. assert_eq!( - loaded - .validate(&PeerId::random(), "net-a", NOW, None) - .unwrap_err(), - SnapshotRejection::ForeignOwner + snap.peers_for(&owner, saved_at - MAX_FUTURE_TIMESTAMP_SKEW.as_secs() - 1), + Err(SnapshotRejection::Stale) ); } } diff --git a/src/network.rs b/src/network.rs index b786a7a..ca044bb 100644 --- a/src/network.rs +++ b/src/network.rs @@ -19,7 +19,7 @@ use crate::PeerId; use crate::adaptive::trust::{TrustRecord, TrustSnapshot}; use crate::adaptive::{AdaptiveDHT, AdaptiveDhtConfig, TrustEngine, TrustEvent}; use crate::bootstrap::cache::{CachedCloseGroupPeer, CloseGroupCache}; -use crate::bootstrap::{RoutingSnapshot, SnapshotPeer, network_fingerprint}; +use crate::bootstrap::routing_snapshot::{RoutingSnapshot, SnapshotPeer}; use crate::dht::core_engine::AddressType; use crate::dht_network_manager::{DhtNetworkConfig, DhtNetworkEvent, DhtNetworkManager}; use crate::error::{NetworkError, P2PError, P2pResult as Result}; @@ -32,7 +32,7 @@ use dashmap::DashMap; use futures::StreamExt; use parking_lot::Mutex as ParkingMutex; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -188,6 +188,15 @@ const CLIENT_BOOTSTRAP_TARGET: usize = 6; /// QUIC+PQC handshakes. const MAX_CONCURRENT_SNAPSHOT_DIALS: usize = 16; +/// Save reason for the snapshot written right after bootstrap. +/// +/// The close-group cache is written then, the routing snapshot is not: at that +/// moment the table holds only what the restore just re-dialled, so persisting +/// it would replace a complete snapshot with a partial one, and each restart +/// would shrink it further. The periodic save takes over once the table has +/// refilled, and the shutdown save is the authoritative one. +const POST_BOOTSTRAP_SAVE: &str = "post_bootstrap"; + /// Wall-clock budget for the routing-snapshot dial phase. /// /// Startup must not be held hostage to a snapshot full of departed peers. When @@ -309,31 +318,6 @@ pub struct NodeConfig { /// check. Default: one hour. #[serde(default = "default_close_group_cache_max_age")] pub close_group_cache_max_age: Option, - - /// Whether a validated routing snapshot seeds startup dials. - /// - /// **Defaults to `true`.** This is the behaviour that repairs a restarted - /// node's routing table in seconds rather than over the roughly daily - /// bucket-refresh cycle, and it is the point of persisting one at all. - /// - /// Set to `false` as an operational kill switch. That restores the previous - /// behaviour exactly — the close-group cache still seeds the neighbourhood, - /// and the snapshot is still written, so switching back needs no rebuild and - /// loses no data. - /// - /// **Consumer prerequisite.** A restored table is materially more complete, - /// and a consumer that decides record retention from routing state may treat - /// a table which has grown from 19 to 20 apparently-closer peers as licence - /// to delete a record with no holder confirmation. Any consumer must have - /// removed that path before adopting a version of this crate carrying - /// restoration. See the routing-snapshot module docs. - #[serde(default = "default_routing_snapshot_restore")] - pub routing_snapshot_restore: bool, -} - -/// Routing-snapshot restoration is on by default; see the field docs. -const fn default_routing_snapshot_restore() -> bool { - true } fn default_close_group_cache_max_age() -> Option { @@ -471,7 +455,6 @@ pub struct NodeConfigBuilder { /// Outer `None` means the builder setter was not called; inner `None` /// explicitly disables age enforcement. close_group_cache_max_age: Option>, - routing_snapshot_restore: Option, } impl Default for NodeConfigBuilder { @@ -491,7 +474,6 @@ impl Default for NodeConfigBuilder { adaptive_dht_config: None, close_group_cache_dir: None, close_group_cache_max_age: None, - routing_snapshot_restore: None, } } } @@ -620,16 +602,6 @@ impl NodeConfigBuilder { self } - /// Set whether a validated routing snapshot seeds startup dials. - /// - /// Defaults to `false`. Read the field documentation on - /// [`NodeConfig::routing_snapshot_restore`]. Defaults to `true`; setting - /// `false` is the kill switch that restores the previous startup behaviour. - pub fn routing_snapshot_restore(mut self, enabled: bool) -> Self { - self.routing_snapshot_restore = Some(enabled); - self - } - /// Build the [`NodeConfig`]. /// /// # Errors @@ -660,9 +632,6 @@ impl NodeConfigBuilder { close_group_cache_max_age: self .close_group_cache_max_age .unwrap_or_else(default_close_group_cache_max_age), - routing_snapshot_restore: self - .routing_snapshot_restore - .unwrap_or_else(default_routing_snapshot_restore), }) } } @@ -686,7 +655,6 @@ impl Default for NodeConfig { adaptive_dht_config: AdaptiveDhtConfig::default(), close_group_cache_dir: None, close_group_cache_max_age: default_close_group_cache_max_age(), - routing_snapshot_restore: default_routing_snapshot_restore(), } } } @@ -884,6 +852,9 @@ pub struct P2PNode { /// Dedicated cancellation token for periodic close-group-cache saves. /// Cancelled and joined before the authoritative shutdown snapshot. close_group_cache_save_shutdown: CancellationToken, + /// Whether the routing snapshot has already seeded a bootstrap in this + /// process, so a re-bootstrap does not replay it. + routing_snapshot_restored: AtomicBool, /// Periodic close-group-cache task, retained so shutdown can prevent a /// late periodic write from replacing the final snapshot. @@ -1007,6 +978,7 @@ impl P2PNode { start_time: Instant::now(), shutdown: CancellationToken::new(), close_group_cache_save_shutdown: CancellationToken::new(), + routing_snapshot_restored: AtomicBool::new(false), close_group_cache_save_handle: TokioMutex::new(None), adaptive_dht, is_bootstrapped: Arc::new(AtomicBool::new(false)), @@ -1441,13 +1413,11 @@ impl P2PNode { let peer_id = self.peer_id; let k_value = self.config.dht_config.k_value; let shutdown = self.close_group_cache_save_shutdown.clone(); - let fingerprint = network_fingerprint(&self.config.bootstrap_peers); *task = Some(tokio::spawn(periodic_close_group_cache_save( dht_manager, trust_engine, peer_id, k_value, - fingerprint, dir, interval, shutdown, @@ -2137,7 +2107,19 @@ impl P2PNode { // Priority 2: the routing snapshot, if one validated. Addresses already // queued as close-group or configured candidates are skipped so a peer // is never dialled twice. - let snapshot_addr_sets = self.routing_snapshot_dial_sets(&mut seen_addresses).await; + // + // Restored once per process. A client keeps its existing six-peer + // startup bound: it does not serve the DHT, so it never asks whether it + // is responsible for a key, which is the only question this repairs. + // A re-bootstrap of a running node skips it too — the table it would + // restore is the table the node already has. + let snapshot_addr_sets = if self.config.mode == NodeMode::Client + || self.routing_snapshot_restored.swap(true, Ordering::Relaxed) + { + None + } else { + self.routing_snapshot_dial_sets(&mut seen_addresses).await + }; if serial_addr_sets.is_empty() && parallel_addr_sets.is_empty() @@ -2207,12 +2189,23 @@ impl P2PNode { // Phase C: the routing snapshot. // - // The close group reconnects a neighbourhood; this rebuilds the rest of + // The close group reconnects a neighbourhood; this restores the rest of // the table, which is what a node consults to decide whether anyone is - // closer to a given key than it is. Dialled concurrently and last: it is - // the widest set and the least urgent, and giving the close group and - // the configured peers the first attempts keeps cold-start latency for - // an unrestored node exactly as it was. + // closer to a given key than it is. Dialled last, because the close + // group and the configured peers are what connectivity depends on, and + // concurrently, because the set is an order of magnitude larger. + // + // Two bounds, both deliberate: + // + // - New dials stop once the budget expires; dials already in flight are + // allowed to finish, each bounded by `identity_timeout`. Cancelling a + // handshake mid-flight would leave the far side holding a half-open + // connection, which is a worse outcome than waiting out one timeout. + // - Successes are NOT added to `connected_peer_ids`. A dialled, + // identity-verified peer is already admitted to the routing table by + // the connection path, so seeding DHT discovery with all of them would + // issue a serial FIND_NODE per peer to rediscover the table just + // restored, and then serially dial everything those queries returned. // // Every peer here is dialled and identity-verified through the same path // as any other candidate. Nothing from the file enters the routing table @@ -2224,6 +2217,9 @@ impl P2PNode { let deadline = tokio::time::Instant::now() + SNAPSHOT_RESTORE_BUDGET; let mut snapshot_stream = futures::stream::iter(snapshot_sets.into_iter().map( |(expected_peer_id, addrs)| async move { + if tokio::time::Instant::now() >= deadline { + return None; + } self.dial_bootstrap_addr_set( &addrs, identity_timeout, @@ -2235,31 +2231,21 @@ impl P2PNode { )) .buffer_unordered(MAX_CONCURRENT_SNAPSHOT_DIALS); - loop { - let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); - if remaining.is_zero() { - debug!( - snapshot_dial_successes, - snapshot_dial_candidates, - "Routing snapshot restore hit its time budget; the rest is left to \ - ordinary discovery" - ); - break; - } - match tokio::time::timeout(remaining, snapshot_stream.next()).await { - Ok(Some(Some(peer_id))) => { - snapshot_dial_successes += 1; - successful_connections += 1; - connected_peer_ids.push(peer_id); - } - Ok(Some(None)) => {} - Ok(None) => break, - Err(_) => { - debug!("Routing snapshot restore timed out awaiting a dial"); - break; - } + while let Some(outcome) = snapshot_stream.next().await { + if outcome.is_some() { + snapshot_dial_successes += 1; + successful_connections += 1; } } + + if snapshot_dial_successes < snapshot_dial_candidates { + debug!( + snapshot_dial_successes, + snapshot_dial_candidates, + "Some routing-snapshot peers were not restored; the rest is left to \ + ordinary discovery" + ); + } } info!( @@ -2344,7 +2330,7 @@ impl P2PNode { // Save close group cache after initial bootstrap so a crash before // graceful shutdown still preserves the newly-discovered close group. if let Some(ref dir) = self.config.close_group_cache_dir - && let Err(e) = self.save_close_group_cache(dir, "post_bootstrap").await + && let Err(e) = self.save_close_group_cache(dir, POST_BOOTSTRAP_SAVE).await { warn!("Failed to save close group cache after bootstrap: {e}"); } @@ -2430,28 +2416,21 @@ impl P2PNode { None } - /// Load, validate and prepare the routing snapshot's dial candidates. - /// - /// Returns `None` when there is nothing usable — no configured directory, no - /// file, a rejected file, or restoration switched off. Every one of those is - /// logged with its reason: a node that quietly cold-starts on every restart - /// is indistinguishable from one that never had a snapshot, and that is - /// exactly the failure an operator needs to be able to see. + /// Load the routing snapshot and turn it into dial candidates. /// - /// Addresses already queued by an earlier priority are removed here, and - /// `seen_addresses` is extended, so no peer is dialled twice. + /// Returns `None` when there is nothing usable — no configured directory, + /// no file, or a rejected one. A rejection is logged with its reason: a node + /// that quietly cold-starts on every restart looks exactly like one that + /// never had a snapshot, and an operator needs to tell those apart. async fn routing_snapshot_dial_sets( &self, - seen_addresses: &mut std::collections::HashSet, + seen_addresses: &mut HashSet, ) -> Option)>> { let dir = self.config.close_group_cache_dir.as_ref()?; let snapshot = match RoutingSnapshot::load_from_dir(dir).await { Ok(Some(snapshot)) => snapshot, - Ok(None) => { - debug!("No routing snapshot on disk; starting from configured peers"); - return None; - } + Ok(None) => return None, Err(rejection) => { warn!(%rejection, "Discarding routing snapshot"); return None; @@ -2461,66 +2440,23 @@ impl P2PNode { let now_epoch = SystemTime::now() .duration_since(UNIX_EPOCH) .map_or(0, |duration| duration.as_secs()); - let peers = match snapshot.validate( - &self.peer_id, - &network_fingerprint(&self.config.bootstrap_peers), - now_epoch, - self.config.close_group_cache_max_age, - ) { + let peers = match snapshot.peers_for(&self.peer_id, now_epoch) { Ok(peers) => peers, Err(rejection) => { - warn!( - %rejection, - snapshot_peers = snapshot.peer_count(), - "Discarding routing snapshot" - ); + warn!(%rejection, "Discarding routing snapshot"); return None; } }; - if !self.config.routing_snapshot_restore { - info!( - snapshot_peers = peers.len(), - "Routing snapshot is valid but restoration is switched off; \ - the routing table will refill at the ordinary refresh cadence" - ); - return None; - } - - let mut sets: Vec<(PeerId, Vec)> = Vec::new(); - for peer in peers { - if peer.peer_id == self.peer_id { - continue; - } - let new_addresses: Vec = peer - .addresses - .iter() - .filter(|addr| { - addr.dialable_socket_addr() - .is_some_and(|socket| !seen_addresses.contains(&socket)) - }) - .cloned() - .collect(); - if new_addresses.is_empty() { - continue; - } - for addr in &new_addresses { - if let Some(socket) = addr.socket_addr() { - seen_addresses.insert(socket); - } - } - sets.push((peer.peer_id, new_addresses)); - } - + let sets = snapshot_dial_sets(peers, &self.peer_id, seen_addresses); if sets.is_empty() { - debug!("Routing snapshot added no candidates beyond those already queued"); return None; } info!( snapshot_peers = peers.len(), new_candidates = sets.len(), - age_secs = now_epoch.saturating_sub(snapshot.payload.saved_at_epoch_secs), + age_secs = now_epoch.saturating_sub(snapshot.saved_at_epoch_secs), "Restoring routing table from snapshot" ); Some(sets) @@ -2537,7 +2473,6 @@ impl P2PNode { self.adaptive_dht.trust_engine(), self.peer_id, self.config.dht_config.k_value, - &network_fingerprint(&self.config.bootstrap_peers), dir, save_reason, ) @@ -2547,6 +2482,44 @@ impl P2PNode { // disconnect_all_peers and periodic_tasks are now in TransportHandle } +/// Turn snapshot peers into dial candidates, skipping self and anything an +/// earlier bootstrap priority already queued. +/// +/// Extends `seen_addresses` with what it returns, so a peer reachable through +/// the close-group cache and the snapshot is dialled once, not twice. Only +/// dialable (QUIC) addresses survive. +fn snapshot_dial_sets( + peers: &[SnapshotPeer], + self_id: &PeerId, + seen_addresses: &mut HashSet, +) -> Vec<(PeerId, Vec)> { + let mut sets: Vec<(PeerId, Vec)> = Vec::new(); + for peer in peers { + if peer.peer_id == *self_id { + continue; + } + let new_addresses: Vec = peer + .addresses + .iter() + .filter(|addr| { + addr.dialable_socket_addr() + .is_some_and(|socket| !seen_addresses.contains(&socket)) + }) + .cloned() + .collect(); + if new_addresses.is_empty() { + continue; + } + seen_addresses.extend( + new_addresses + .iter() + .filter_map(MultiAddr::dialable_socket_addr), + ); + sets.push((peer.peer_id, new_addresses)); + } + sets +} + /// Persist a close-group snapshot using owned subsystem handles. /// /// Keeping this separate from `P2PNode` allows the periodic task to own every @@ -2556,7 +2529,6 @@ async fn save_close_group_cache_snapshot( trust_engine: &TrustEngine, peer_id: PeerId, k_value: usize, - network_fingerprint: &str, dir: &Path, save_reason: &'static str, ) -> anyhow::Result<()> { @@ -2608,15 +2580,9 @@ async fn save_close_group_cache_snapshot( // // A failure to write the snapshot must not fail the close-group save that // already succeeded, so it is logged and swallowed. - save_routing_snapshot( - dht_manager, - peer_id, - network_fingerprint, - now_epoch, - dir, - save_reason, - ) - .await; + if save_reason != POST_BOOTSTRAP_SAVE { + save_routing_snapshot(dht_manager, peer_id, now_epoch, dir, save_reason).await; + } Ok(()) } @@ -2628,7 +2594,6 @@ async fn save_close_group_cache_snapshot( async fn save_routing_snapshot( dht_manager: &DhtNetworkManager, peer_id: PeerId, - network_fingerprint: &str, now_epoch: u64, dir: &Path, save_reason: &'static str, @@ -2644,22 +2609,9 @@ async fn save_routing_snapshot( .collect(); let peer_count = peers.len(); - let snapshot = - match RoutingSnapshot::new(peer_id, network_fingerprint.to_string(), now_epoch, peers) { - Ok(snapshot) => snapshot, - Err(error) => { - warn!(save_reason, %error, "Failed to build routing snapshot"); - return; - } - }; - + let snapshot = RoutingSnapshot::new(peer_id, now_epoch, peers); match snapshot.save_to_dir(dir).await { - Ok(()) => info!( - save_reason, - peer_count, - "Saved routing snapshot ({peer_count} peers) to {}", - dir.display() - ), + Ok(()) => debug!(save_reason, peer_count, "Saved routing snapshot"), Err(error) => warn!(save_reason, %error, "Failed to save routing snapshot"), } } @@ -2670,7 +2622,6 @@ async fn periodic_close_group_cache_save( trust_engine: Arc, peer_id: PeerId, k_value: usize, - network_fingerprint: String, dir: PathBuf, interval: Duration, shutdown: CancellationToken, @@ -2689,7 +2640,6 @@ async fn periodic_close_group_cache_save( &trust_engine, peer_id, k_value, - &network_fingerprint, &dir, "periodic", ).await { @@ -2780,7 +2730,6 @@ mod tests { adaptive_dht_config: AdaptiveDhtConfig::default(), close_group_cache_dir: None, close_group_cache_max_age: default_close_group_cache_max_age(), - routing_snapshot_restore: default_routing_snapshot_restore(), } } @@ -3994,4 +3943,74 @@ mod tests { "timestamp-only mutation on a signed message must fail signature verification" ); } + + fn snapshot_peer(addr: &str) -> SnapshotPeer { + SnapshotPeer { + peer_id: PeerId::random(), + addresses: vec![addr.parse().expect("valid multiaddr")], + } + } + + #[test] + fn snapshot_dial_sets_skips_self() { + let self_id = PeerId::random(); + let mut peers = vec![snapshot_peer("/ip4/10.0.0.1/udp/9000/quic")]; + peers[0].peer_id = self_id; + let mut seen = HashSet::new(); + + assert!(snapshot_dial_sets(&peers, &self_id, &mut seen).is_empty()); + assert!(seen.is_empty(), "self must not reserve an address"); + } + + #[test] + fn snapshot_dial_sets_skips_addresses_already_queued() { + let self_id = PeerId::random(); + let queued = snapshot_peer("/ip4/10.0.0.2/udp/9000/quic"); + let fresh = snapshot_peer("/ip4/10.0.0.3/udp/9000/quic"); + let mut seen: HashSet = queued + .addresses + .iter() + .filter_map(MultiAddr::dialable_socket_addr) + .collect(); + + let sets = snapshot_dial_sets(&[queued, fresh.clone()], &self_id, &mut seen); + + assert_eq!( + sets.len(), + 1, + "the already-queued peer must not be redialled" + ); + assert_eq!(sets[0].0, fresh.peer_id); + assert_eq!( + seen.len(), + 2, + "the new address is reserved for later phases" + ); + } + + #[test] + fn snapshot_dial_sets_drops_undialable_addresses() { + let self_id = PeerId::random(); + let peers = vec![snapshot_peer("/ip4/10.0.0.4/tcp/9000")]; + let mut seen = HashSet::new(); + + assert!( + snapshot_dial_sets(&peers, &self_id, &mut seen).is_empty(), + "only QUIC addresses are dialable today" + ); + } + + #[test] + fn snapshot_dial_sets_returns_every_other_peer_once() { + let self_id = PeerId::random(); + let peers: Vec = (0..130) + .map(|i| snapshot_peer(&format!("/ip4/10.1.{}.{}/udp/9000/quic", i / 256, i % 256))) + .collect(); + let mut seen = HashSet::new(); + + let sets = snapshot_dial_sets(&peers, &self_id, &mut seen); + + assert_eq!(sets.len(), 130); + assert_eq!(seen.len(), 130); + } } From e4252db34f93d30d29398b5b723aab1c9cdd8c02 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 18:16:37 +0900 Subject: [PATCH 4/8] fix(bootstrap): tighten the snapshot's load, save and dial bounds Second review round. Loading opened the file by path, checked its metadata, then read the path again. Those are two different files if anything replaces it in between, so neither the regular-file check nor the size ceiling was binding. It now opens once, checks that handle, and reads through a hard ceiling. Saving keyed off a human-readable save-reason string to decide whether the snapshot was written. The periodic and shutdown paths now call it explicitly and the string comparison is gone. A save is also skipped until the bootstrap phase that restores the previous snapshot has finished, and skipped if that restore recovered less than half of what it tried: a node that could not reach the network must not overwrite a full snapshot with the little it managed to re-dial. Without this a node stopped inside the restore window shrank its own snapshot a little further on every cycle. The dial budget stopped new peers but said nothing about how many addresses each one could try, so the phase's real bound was the budget plus up to eight attempts. A snapshot peer is now tried at no more than two addresses. Candidate selection deduplicated by socket address only, so a peer repeated in the file, or reachable at two addresses, could be dialled twice. It now deduplicates by peer id as well. ADR corrected on three points that overstated the code: the flat 1,024-peer ceiling was not mentioned, the phase bound was described as strictly 20 seconds, and "answers as its converged self did at every width" was stated as fact when it is the simulated expectation and no live test exists yet. --- ...7-routing-table-snapshot-across-restart.md | 17 +-- src/bootstrap/routing_snapshot.rs | 21 +++- src/network.rs | 119 ++++++++++++++---- 3 files changed, 120 insertions(+), 37 deletions(-) diff --git a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md index c044160..dc5cbf7 100644 --- a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md +++ b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md @@ -28,14 +28,14 @@ Simulated on an 891-node network against correct shares of 1.01% (width 9) and 2 Persist the whole capped routing table to its own snapshot file, and restore it as dial candidates at startup. 1. **Separate file.** The snapshot is written alongside `close_group_cache.json`, not in place of it, so an older binary cannot mistake one for the other and a downgrade behaves exactly as today. -2. **Every bucket, not the nearest `k`.** The snapshot carries the table up to its per-bucket capacity. Nine peers per bucket fixes the narrow width and leaves the wide one at 12.3%, so the shape is the whole table rather than a per-bucket floor. +2. **Every bucket, not the nearest `k`.** The snapshot carries the table up to its per-bucket capacity, subject to a flat ceiling of 1,024 peers and 8 addresses each. Nine peers per bucket fixes the narrow width and leaves the wide one at 12.3%, so the shape is the whole table rather than a per-bucket floor. The ceiling is well above the ~129 entries a fleet of this size produces; a network large enough to reach it would restore a truncated table, which is still strictly better than the close group alone. 3. **Two bindings, both required.** The schema version, so an unrecognised version is discarded rather than coerced; and the owning node id, which is load-bearing rather than hygiene, because bucket indices are relative to the owner and another node's snapshot describes a different partition of the id space. A snapshot older than seven days, or dated more than five minutes in the future, is refused. That age bound is the snapshot's own, deliberately not the close-group cache's one hour: the cache's bound exists for trust scores on `k` neighbours, while this file answers which parts of the id space the node knew about, which does not rot on the same timescale, and a maintenance window must not cost a node its table. -4. **Bounded load.** The file is refused if it is not a regular file or exceeds 512 KB, and the peer and per-peer address counts are capped after parsing as well as before writing, so a corrupt or hostile file cannot become an unbounded dial set. +4. **Bounded load.** The file is opened once and checked through that handle, never re-opened by path, so a file swapped in between the check and the read cannot bypass either. It is refused if it is not a regular file or exceeds 512 KB, the read itself is capped at the same ceiling, and the peer and per-peer address counts are re-applied after parsing, so a corrupt or hostile file cannot become an unbounded dial set. 5. **Restored peers are dial candidates only.** They are not inserted into the routing table and confer no authority until dialled and identity-verified through the ordinary path. Routing-table membership is an authorization fact for callers above this crate, and a file on disk must not be able to grant it. For the same reason the snapshot records **no trust scores**, unlike the close-group cache: pre-dial trust restoration is defensible for `k` vetted neighbours and not for hundreds of unverified peers. 6. **Restored peers do not seed DHT discovery.** A dialled, identity-verified peer is already admitted to the routing table by the connection path. Adding the restored set to the discovery seed list would issue a serial `FIND_NODE` per peer to rediscover the table just restored, and then serially dial everything those queries returned, so the phase's own bound would be defeated by the phase after it. -7. **Bounded cost, then today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger). A 20-second budget stops *new* dials; dials already in flight are allowed to finish, each bounded by the identity timeout, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. Whatever is not restored refills through ordinary discovery, which is exactly the behaviour of a node that had no snapshot. +7. **Bounded cost, then today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger). A 20-second budget stops *new* peers being dialled; dials already in flight are allowed to finish, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. The phase's bound is therefore the budget plus the last peer's attempts, which is why a snapshot peer is tried at no more than two addresses. Whatever is not restored refills through ordinary discovery, which is exactly the behaviour of a node that had no snapshot. 8. **Once per process, and not for clients.** A re-bootstrap does not replay the snapshot, since the table it would restore is the one the node already has. `NodeMode::Client` skips restoration entirely and keeps its existing six-peer startup bound: a client does not serve the DHT, so it never asks the question this repairs. -9. **Written on the periodic and shutdown saves, not the post-bootstrap one.** Right after bootstrap the table holds only what the restore re-dialled, so persisting it then would replace a complete snapshot with a partial one and each restart would shrink it further. +9. **Written only from a table that finished restoring.** The periodic and shutdown saves write the snapshot; the post-bootstrap save does not. A save is skipped entirely until the bootstrap phase that restores the previous snapshot has run, and if that restore recovered less than half of what it tried — a node that could not reach the network, rather than a table worth keeping — the older, fuller file is left in place. Without this, a node stopped or restarted inside the restore window would shrink its own snapshot a little further every time. ## Alternatives considered @@ -43,6 +43,7 @@ Persist the whole capped routing table to its own snapshot file, and restore it - **Persist a fixed floor of peers per bucket.** Rejected as insufficient rather than wrong. Nine per bucket answers width 9 correctly but leaves width 20 at 12.3%, and this crate does not know the widths its consumers use. - **Bind the snapshot to a network fingerprint derived from the configured bootstrap list.** Rejected: it hashes mutable address spellings rather than a stable network identity, and routine seed rotation would invalidate every node's snapshot during exactly the rollout this repairs. A snapshot carried to another network costs failed dials inside the existing budget, which is the same cost as a cold start. - **Checksum the payload.** Rejected: it provides no authenticity for a locally written file, while JSON parsing is already all-or-nothing, the write is atomic, and live identity verification governs what the contents can achieve. +- **A config kill switch for restoration.** Rejected: new public API for a behaviour that rolls back by shipping the previous version or deleting the file, and one more untested branch through startup. - **Estimate network size locally and refuse keys beyond an inferred horizon.** Rejected on two grounds. It admits every key out to true rank `slack × width`, which at the studied slack is rank 36, above the rank 24 to 27 band the production over-claim actually occupied: it caught 14.3% of that band with 6 of 10 simulated nodes catching none. It is also punishable, because a refusal resurfaces as an absent answer at the requester's audit, and peers near a victim's id can shrink its estimate. - **Wait out the transient by tuning the consumer's retention timers.** Rejected: it treats a wrong answer as a scheduling problem, and it makes nodes act fastest exactly when their routing table is least trustworthy. @@ -50,14 +51,14 @@ Persist the whole capped routing table to its own snapshot file, and restore it ### Positive -- A restarted node answers responsibility questions as its converged self did, at every width, instead of claiming most of the keyspace for hours. +- A restarted node is expected to answer responsibility questions as its converged self did, at both widths, instead of claiming most of the keyspace for hours. That is the simulated result and the intent of the design; it is not yet demonstrated against a live network, see Validation. - The fix is combinatorial, so it does not depend on network size, key distribution, or an estimator an adversary could move. - Consumers gain nothing new to configure. The predicate, its widths and its call sites are untouched, and there is no new public API. ### Negative - A new on-disk artifact to version and keep compatible. -- Startup dials a larger candidate set. New dials stop at the 20-second budget, but a dial already in flight can still run to its identity timeout, so the phase can exceed the budget by that much. +- Startup dials a larger candidate set. New peers stop being dialled at the 20-second budget, but attempts already in flight still run, so the phase can exceed the budget by one peer's two address attempts. - A snapshot full of departed peers spends that budget and yields little, though never more than it. - Every periodic and shutdown close-group save now writes a second small file before returning. @@ -70,6 +71,6 @@ Persist the whole capped routing table to its own snapshot file, and restore it ## Validation - Unit tests in `src/bootstrap/routing_snapshot.rs` for the file contract: disk round trip, missing file treated as absence, truncated file reported, oversized file refused without being read, peer cap re-applied on load, foreign owner refused, unknown schema version refused, and staleness bounded on both sides of now. -- Unit tests in `src/network.rs` for the restore path's candidate selection: self excluded, addresses already queued by an earlier bootstrap priority not redialled, undialable addresses dropped, and a full table producing one candidate per peer. -- Not covered by tests in this PR: the dial phase itself against a live transport, including budget expiry and client-mode exclusion. Those need a multi-node harness. +- Unit tests in `src/network.rs` for the restore path's candidate selection: self excluded, addresses already queued by an earlier bootstrap priority not redialled, undialable addresses dropped, repeated peers deduplicated, the per-peer dial list bounded, and a full table producing one candidate per peer. +- Not covered by tests in this PR: the dial phase itself against a live transport, including budget expiry, client-mode exclusion, and the readiness rule that decides whether a table may be persisted. Those need a multi-node harness. - **No testnet or production measurement of this change exists.** The over-claim it targets is measured in production; the fix is evidenced by simulation and unit tests only. A dev testnet run is the next step, and nothing here claims fleet readiness. diff --git a/src/bootstrap/routing_snapshot.rs b/src/bootstrap/routing_snapshot.rs index a8af56a..7bc1960 100644 --- a/src/bootstrap/routing_snapshot.rs +++ b/src/bootstrap/routing_snapshot.rs @@ -216,11 +216,19 @@ impl RoutingSnapshot { pub async fn load_from_dir(dir: &Path) -> Result, SnapshotRejection> { let path = dir.join(ROUTING_SNAPSHOT_FILENAME); - let metadata = match tokio::fs::metadata(&path).await { - Ok(metadata) => metadata, + // Open once and check the handle, not the path: checking the path and + // then reading it again is two different files if anything replaces it + // in between. The read is separately capped, so the size check cannot + // be sidestepped by a file that grows after it is opened. + let file = match tokio::fs::File::open(&path).await { + Ok(file) => file, Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), Err(e) => return Err(SnapshotRejection::Unreadable(e.to_string())), }; + let metadata = file + .metadata() + .await + .map_err(|e| SnapshotRejection::Unreadable(e.to_string()))?; if !metadata.is_file() { return Err(SnapshotRejection::Unreadable("not a regular file".into())); } @@ -231,9 +239,16 @@ impl RoutingSnapshot { ))); } - let bytes = tokio::fs::read(&path) + let mut bytes = Vec::new(); + let mut bounded = tokio::io::AsyncReadExt::take(file, MAX_SNAPSHOT_BYTES + 1); + tokio::io::AsyncReadExt::read_to_end(&mut bounded, &mut bytes) .await .map_err(|e| SnapshotRejection::Unreadable(e.to_string()))?; + if bytes.len() as u64 > MAX_SNAPSHOT_BYTES { + return Err(SnapshotRejection::Unreadable(format!( + "exceeds the {MAX_SNAPSHOT_BYTES} byte limit while reading" + ))); + } let mut snapshot: Self = serde_json::from_slice(&bytes) .map_err(|e| SnapshotRejection::Unreadable(e.to_string()))?; diff --git a/src/network.rs b/src/network.rs index ca044bb..4fb1627 100644 --- a/src/network.rs +++ b/src/network.rs @@ -188,14 +188,12 @@ const CLIENT_BOOTSTRAP_TARGET: usize = 6; /// QUIC+PQC handshakes. const MAX_CONCURRENT_SNAPSHOT_DIALS: usize = 16; -/// Save reason for the snapshot written right after bootstrap. +/// Addresses tried per snapshot peer. /// -/// The close-group cache is written then, the routing snapshot is not: at that -/// moment the table holds only what the restore just re-dialled, so persisting -/// it would replace a complete snapshot with a partial one, and each restart -/// would shrink it further. The periodic save takes over once the table has -/// refilled, and the shutdown save is the authoritative one. -const POST_BOOTSTRAP_SAVE: &str = "post_bootstrap"; +/// The budget below stops new peers, so the phase's real bound is the budget +/// plus the last peer's attempts. Two keeps that tail short while still +/// covering a peer whose first address has gone stale. +const MAX_SNAPSHOT_ADDRESSES_DIALLED: usize = 2; /// Wall-clock budget for the routing-snapshot dial phase. /// @@ -855,6 +853,9 @@ pub struct P2PNode { /// Whether the routing snapshot has already seeded a bootstrap in this /// process, so a re-bootstrap does not replay it. routing_snapshot_restored: AtomicBool, + /// Whether the routing table is finished restoring and may therefore be + /// persisted. Shared with the periodic save task. + routing_table_ready: Arc, /// Periodic close-group-cache task, retained so shutdown can prevent a /// late periodic write from replacing the final snapshot. @@ -979,6 +980,7 @@ impl P2PNode { shutdown: CancellationToken::new(), close_group_cache_save_shutdown: CancellationToken::new(), routing_snapshot_restored: AtomicBool::new(false), + routing_table_ready: Arc::new(AtomicBool::new(false)), close_group_cache_save_handle: TokioMutex::new(None), adaptive_dht, is_bootstrapped: Arc::new(AtomicBool::new(false)), @@ -1413,6 +1415,7 @@ impl P2PNode { let peer_id = self.peer_id; let k_value = self.config.dht_config.k_value; let shutdown = self.close_group_cache_save_shutdown.clone(); + let table_ready = Arc::clone(&self.routing_table_ready); *task = Some(tokio::spawn(periodic_close_group_cache_save( dht_manager, trust_engine, @@ -1420,6 +1423,7 @@ impl P2PNode { k_value, dir, interval, + table_ready, shutdown, ))); info!( @@ -1468,11 +1472,24 @@ impl P2PNode { warn!("Periodic close group cache task failed during shutdown: {error}"); } - // Save close group cache before tearing down the DHT and transport layers. - if let Some(ref dir) = self.config.close_group_cache_dir - && let Err(e) = self.save_close_group_cache(dir, "shutdown").await - { - warn!("Failed to save close group cache on shutdown: {e}"); + // Save close group cache and routing snapshot before tearing down the + // DHT and transport layers. + if let Some(ref dir) = self.config.close_group_cache_dir { + if let Err(e) = self.save_close_group_cache(dir, "shutdown").await { + warn!("Failed to save close group cache on shutdown: {e}"); + } + let now_epoch = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()); + save_routing_snapshot( + self.dht_manager(), + self.peer_id, + now_epoch, + dir, + &self.routing_table_ready, + "shutdown", + ) + .await; } // Signal the run loop to exit @@ -2248,6 +2265,16 @@ impl P2PNode { } } + // The table may now be persisted — unless the restore recovered less + // than half of what it tried, which is a node that could not reach the + // network rather than a table worth writing over the snapshot it came + // from. Keeping the older, fuller file is strictly better for the next + // start, and discovery will refill this one either way. + let restore_recovered_enough = snapshot_dial_candidates == 0 + || snapshot_dial_successes * 2 >= snapshot_dial_candidates; + self.routing_table_ready + .store(restore_recovered_enough, Ordering::Relaxed); + info!( cache_dial_candidates, cache_dial_successes, @@ -2330,7 +2357,7 @@ impl P2PNode { // Save close group cache after initial bootstrap so a crash before // graceful shutdown still preserves the newly-discovered close group. if let Some(ref dir) = self.config.close_group_cache_dir - && let Err(e) = self.save_close_group_cache(dir, POST_BOOTSTRAP_SAVE).await + && let Err(e) = self.save_close_group_cache(dir, "post_bootstrap").await { warn!("Failed to save close group cache after bootstrap: {e}"); } @@ -2494,8 +2521,9 @@ fn snapshot_dial_sets( seen_addresses: &mut HashSet, ) -> Vec<(PeerId, Vec)> { let mut sets: Vec<(PeerId, Vec)> = Vec::new(); + let mut seen_peers: HashSet = HashSet::new(); for peer in peers { - if peer.peer_id == *self_id { + if peer.peer_id == *self_id || !seen_peers.insert(peer.peer_id) { continue; } let new_addresses: Vec = peer @@ -2505,6 +2533,7 @@ fn snapshot_dial_sets( addr.dialable_socket_addr() .is_some_and(|socket| !seen_addresses.contains(&socket)) }) + .take(MAX_SNAPSHOT_ADDRESSES_DIALLED) .cloned() .collect(); if new_addresses.is_empty() { @@ -2572,18 +2601,6 @@ async fn save_close_group_cache_snapshot( dir.display() ); - // Write the whole routing table alongside the close group. The close group - // is what reconnects a neighbourhood; only the full table can reconstruct - // the knowledge a node uses to decide whether anyone is closer to a given - // key than it is. Written unconditionally, including when restoration is - // switched off, so the kill switch never costs the next start its snapshot. - // - // A failure to write the snapshot must not fail the close-group save that - // already succeeded, so it is logged and swallowed. - if save_reason != POST_BOOTSTRAP_SAVE { - save_routing_snapshot(dht_manager, peer_id, now_epoch, dir, save_reason).await; - } - Ok(()) } @@ -2591,13 +2608,28 @@ async fn save_close_group_cache_snapshot( /// /// Best-effort and non-fatal: the close-group cache is the established path and /// must not start failing because a newer, unread file could not be written. +/// +/// Skipped until `table_ready` is set, which happens when the bootstrap phase +/// that restores the previous snapshot has finished. Writing before that +/// replaces a complete snapshot with whatever has been re-dialled so far, and +/// a node stopped or restarted inside that window would shrink its own table a +/// little further every time. async fn save_routing_snapshot( dht_manager: &DhtNetworkManager, peer_id: PeerId, now_epoch: u64, dir: &Path, + table_ready: &AtomicBool, save_reason: &'static str, ) { + if !table_ready.load(Ordering::Relaxed) { + debug!( + save_reason, + "Skipping routing snapshot save: the table has not finished restoring" + ); + return; + } + let peers: Vec = dht_manager .routing_table_peers() .await @@ -2624,6 +2656,7 @@ async fn periodic_close_group_cache_save( k_value: usize, dir: PathBuf, interval: Duration, + table_ready: Arc, shutdown: CancellationToken, ) { let start = tokio::time::Instant::now() + interval; @@ -2645,6 +2678,17 @@ async fn periodic_close_group_cache_save( ).await { warn!("Periodic close group cache save failed: {error}"); } + let now_epoch = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()); + save_routing_snapshot( + &dht_manager, + peer_id, + now_epoch, + &dir, + &table_ready, + "periodic", + ).await; } } } @@ -4000,6 +4044,29 @@ mod tests { ); } + #[test] + fn snapshot_dial_sets_dedupes_repeated_peers_and_bounds_addresses() { + let self_id = PeerId::random(); + let mut peer = snapshot_peer("/ip4/10.0.0.5/udp/9000/quic"); + peer.addresses = (0..6) + .map(|i| { + format!("/ip4/10.0.0.5/udp/900{i}/quic") + .parse() + .expect("valid multiaddr") + }) + .collect(); + let mut seen = HashSet::new(); + + let sets = snapshot_dial_sets(&[peer.clone(), peer], &self_id, &mut seen); + + assert_eq!(sets.len(), 1, "the same peer twice is one candidate"); + assert_eq!( + sets[0].1.len(), + MAX_SNAPSHOT_ADDRESSES_DIALLED, + "the dial tail past the budget stays bounded" + ); + } + #[test] fn snapshot_dial_sets_returns_every_other_peer_once() { let self_id = PeerId::random(); From 4bcc35ff7ce78b801da49102669abdd7f90396e9 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 18:29:22 +0900 Subject: [PATCH 5/8] fix(bootstrap): never let a save shrink the routing snapshot Third review round. The readiness flag from the previous round could stick in either direction: a restore that recovered too little switched saving off for the life of the process, so the file went stale and the next restart was a cold one, and a later re-bootstrap could switch it back on because it reports no candidates when it skips the replay. Replaced with the invariant that was wanted in the first place: a save is skipped while the live table holds fewer peers than the snapshot restored at startup. It needs no lifecycle state, cannot be cleared by a re-bootstrap, and self-heals, because ordinary discovery refills the table past the floor and saving resumes on its own. Also corrects the ADR, which claimed the dial phase never exceeds its budget while the paragraph above it says in-flight attempts finish. --- ...7-routing-table-snapshot-across-restart.md | 6 +- src/network.rs | 65 +++++++++---------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md index dc5cbf7..e437ed1 100644 --- a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md +++ b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md @@ -35,7 +35,7 @@ Persist the whole capped routing table to its own snapshot file, and restore it 6. **Restored peers do not seed DHT discovery.** A dialled, identity-verified peer is already admitted to the routing table by the connection path. Adding the restored set to the discovery seed list would issue a serial `FIND_NODE` per peer to rediscover the table just restored, and then serially dial everything those queries returned, so the phase's own bound would be defeated by the phase after it. 7. **Bounded cost, then today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger). A 20-second budget stops *new* peers being dialled; dials already in flight are allowed to finish, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. The phase's bound is therefore the budget plus the last peer's attempts, which is why a snapshot peer is tried at no more than two addresses. Whatever is not restored refills through ordinary discovery, which is exactly the behaviour of a node that had no snapshot. 8. **Once per process, and not for clients.** A re-bootstrap does not replay the snapshot, since the table it would restore is the one the node already has. `NodeMode::Client` skips restoration entirely and keeps its existing six-peer startup bound: a client does not serve the DHT, so it never asks the question this repairs. -9. **Written only from a table that finished restoring.** The periodic and shutdown saves write the snapshot; the post-bootstrap save does not. A save is skipped entirely until the bootstrap phase that restores the previous snapshot has run, and if that restore recovered less than half of what it tried — a node that could not reach the network, rather than a table worth keeping — the older, fuller file is left in place. Without this, a node stopped or restarted inside the restore window would shrink its own snapshot a little further every time. +9. **A save never shrinks the snapshot.** The periodic and shutdown saves write the file; the post-bootstrap save does not. A save is skipped while the live table holds fewer peers than the snapshot restored at startup, because a node between opening and finishing its restore holds only what it has re-dialled so far, and one stopped in that window would otherwise replace a complete snapshot with a fragment and shrink its own file a little further on every cycle. The rule needs no other state and self-heals: once ordinary discovery refills the table past that floor, saving resumes. ## Alternatives considered @@ -59,7 +59,7 @@ Persist the whole capped routing table to its own snapshot file, and restore it - A new on-disk artifact to version and keep compatible. - Startup dials a larger candidate set. New peers stop being dialled at the 20-second budget, but attempts already in flight still run, so the phase can exceed the budget by one peer's two address attempts. -- A snapshot full of departed peers spends that budget and yields little, though never more than it. +- A snapshot full of departed peers spends that budget and yields little. No new peer is dialled after it, though attempts already in flight still finish. - Every periodic and shutdown close-group save now writes a second small file before returning. ### Neutral @@ -72,5 +72,5 @@ Persist the whole capped routing table to its own snapshot file, and restore it - Unit tests in `src/bootstrap/routing_snapshot.rs` for the file contract: disk round trip, missing file treated as absence, truncated file reported, oversized file refused without being read, peer cap re-applied on load, foreign owner refused, unknown schema version refused, and staleness bounded on both sides of now. - Unit tests in `src/network.rs` for the restore path's candidate selection: self excluded, addresses already queued by an earlier bootstrap priority not redialled, undialable addresses dropped, repeated peers deduplicated, the per-peer dial list bounded, and a full table producing one candidate per peer. -- Not covered by tests in this PR: the dial phase itself against a live transport, including budget expiry, client-mode exclusion, and the readiness rule that decides whether a table may be persisted. Those need a multi-node harness. +- Not covered by tests in this PR: the dial phase itself against a live transport, including budget expiry, client-mode exclusion, and the no-shrink rule on saves. Those need a multi-node harness. - **No testnet or production measurement of this change exists.** The over-claim it targets is measured in production; the fix is evidenced by simulation and unit tests only. A dev testnet run is the next step, and nothing here claims fleet readiness. diff --git a/src/network.rs b/src/network.rs index 4fb1627..6a904a2 100644 --- a/src/network.rs +++ b/src/network.rs @@ -36,7 +36,7 @@ use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tokio::sync::{Mutex as TokioMutex, RwLock, broadcast}; use tokio::time::Instant; @@ -853,9 +853,10 @@ pub struct P2PNode { /// Whether the routing snapshot has already seeded a bootstrap in this /// process, so a re-bootstrap does not replay it. routing_snapshot_restored: AtomicBool, - /// Whether the routing table is finished restoring and may therefore be - /// persisted. Shared with the periodic save task. - routing_table_ready: Arc, + /// How many peers the routing snapshot restored at startup, or zero when + /// none was. A save must not write a table smaller than this. Shared with + /// the periodic save task. + routing_snapshot_floor: Arc, /// Periodic close-group-cache task, retained so shutdown can prevent a /// late periodic write from replacing the final snapshot. @@ -980,7 +981,7 @@ impl P2PNode { shutdown: CancellationToken::new(), close_group_cache_save_shutdown: CancellationToken::new(), routing_snapshot_restored: AtomicBool::new(false), - routing_table_ready: Arc::new(AtomicBool::new(false)), + routing_snapshot_floor: Arc::new(AtomicUsize::new(0)), close_group_cache_save_handle: TokioMutex::new(None), adaptive_dht, is_bootstrapped: Arc::new(AtomicBool::new(false)), @@ -1415,7 +1416,7 @@ impl P2PNode { let peer_id = self.peer_id; let k_value = self.config.dht_config.k_value; let shutdown = self.close_group_cache_save_shutdown.clone(); - let table_ready = Arc::clone(&self.routing_table_ready); + let snapshot_floor = Arc::clone(&self.routing_snapshot_floor); *task = Some(tokio::spawn(periodic_close_group_cache_save( dht_manager, trust_engine, @@ -1423,7 +1424,7 @@ impl P2PNode { k_value, dir, interval, - table_ready, + snapshot_floor, shutdown, ))); info!( @@ -1486,7 +1487,7 @@ impl P2PNode { self.peer_id, now_epoch, dir, - &self.routing_table_ready, + &self.routing_snapshot_floor, "shutdown", ) .await; @@ -2265,16 +2266,6 @@ impl P2PNode { } } - // The table may now be persisted — unless the restore recovered less - // than half of what it tried, which is a node that could not reach the - // network rather than a table worth writing over the snapshot it came - // from. Keeping the older, fuller file is strictly better for the next - // start, and discovery will refill this one either way. - let restore_recovered_enough = snapshot_dial_candidates == 0 - || snapshot_dial_successes * 2 >= snapshot_dial_candidates; - self.routing_table_ready - .store(restore_recovered_enough, Ordering::Relaxed); - info!( cache_dial_candidates, cache_dial_successes, @@ -2475,6 +2466,10 @@ impl P2PNode { } }; + // A later save must not write a table smaller than this one. + self.routing_snapshot_floor + .store(peers.len(), Ordering::Relaxed); + let sets = snapshot_dial_sets(peers, &self.peer_id, seen_addresses); if sets.is_empty() { return None; @@ -2609,27 +2604,20 @@ async fn save_close_group_cache_snapshot( /// Best-effort and non-fatal: the close-group cache is the established path and /// must not start failing because a newer, unread file could not be written. /// -/// Skipped until `table_ready` is set, which happens when the bootstrap phase -/// that restores the previous snapshot has finished. Writing before that -/// replaces a complete snapshot with whatever has been re-dialled so far, and -/// a node stopped or restarted inside that window would shrink its own table a -/// little further every time. +/// Never writes a table smaller than the one restored at startup (`floor`). +/// Between opening and finishing its restore a node holds only what it has +/// re-dialled so far, and one stopped in that window would otherwise replace a +/// complete snapshot with a fragment, shrinking its own file a little further +/// on every cycle. The rule self-heals: as ordinary discovery refills the +/// table past the floor, saving resumes with no further state. async fn save_routing_snapshot( dht_manager: &DhtNetworkManager, peer_id: PeerId, now_epoch: u64, dir: &Path, - table_ready: &AtomicBool, + floor: &AtomicUsize, save_reason: &'static str, ) { - if !table_ready.load(Ordering::Relaxed) { - debug!( - save_reason, - "Skipping routing snapshot save: the table has not finished restoring" - ); - return; - } - let peers: Vec = dht_manager .routing_table_peers() .await @@ -2641,6 +2629,15 @@ async fn save_routing_snapshot( .collect(); let peer_count = peers.len(); + let floor = floor.load(Ordering::Relaxed); + if peer_count < floor { + debug!( + save_reason, + peer_count, floor, "Skipping routing snapshot save: it would shrink the snapshot" + ); + return; + } + let snapshot = RoutingSnapshot::new(peer_id, now_epoch, peers); match snapshot.save_to_dir(dir).await { Ok(()) => debug!(save_reason, peer_count, "Saved routing snapshot"), @@ -2656,7 +2653,7 @@ async fn periodic_close_group_cache_save( k_value: usize, dir: PathBuf, interval: Duration, - table_ready: Arc, + snapshot_floor: Arc, shutdown: CancellationToken, ) { let start = tokio::time::Instant::now() + interval; @@ -2686,7 +2683,7 @@ async fn periodic_close_group_cache_save( peer_id, now_epoch, &dir, - &table_ready, + &snapshot_floor, "periodic", ).await; } From 400def39b208db8e50b897476cc551c345e30553 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 18:41:28 +0900 Subject: [PATCH 6/8] fix(bootstrap): decide the snapshot floor before allowing any save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth review round, against the floor introduced in the third. The floor started at zero and was only set once the restore step ran, so a node stopped before that step — or one that failed to start — could write its empty or half-dialled table over a complete snapshot. It is now unresolved until the restore step decides it, and a save before that point does nothing. A client never restores, so it never resolves a floor and never writes a snapshot it would not read back. A permanent floor was also unreachable if the network genuinely shrank, if a bucket lost its last peer, or if k_value dropped: every later save was skipped and the file aged out, so the next restart cold-started anyway. The floor now applies for an hour after it is decided, which is the window it exists to protect. After that the live table is the node's best knowledge and the file keeps being refreshed. An empty table is never written, whatever the floor says. Declined from the same round: comparing per-bucket occupancy rather than a peer count before replacing the file. It defends a case the count misses — an equally sized table whose buckets shifted — at the cost of bucket arithmetic on the save path, and the failure it prevents costs one restart with a slightly worse table. The ADR now claims only what the count actually guarantees. --- ...7-routing-table-snapshot-across-restart.md | 2 +- src/network.rs | 100 ++++++++++++++---- 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md index e437ed1..84a6455 100644 --- a/docs/adr/ADR-017-routing-table-snapshot-across-restart.md +++ b/docs/adr/ADR-017-routing-table-snapshot-across-restart.md @@ -35,7 +35,7 @@ Persist the whole capped routing table to its own snapshot file, and restore it 6. **Restored peers do not seed DHT discovery.** A dialled, identity-verified peer is already admitted to the routing table by the connection path. Adding the restored set to the discovery seed list would issue a serial `FIND_NODE` per peer to rediscover the table just restored, and then serially dial everything those queries returned, so the phase's own bound would be defeated by the phase after it. 7. **Bounded cost, then today's behaviour.** Restoration dials at bounded concurrency (16, against 4 for bootstrap dials, because the set is an order of magnitude larger). A 20-second budget stops *new* peers being dialled; dials already in flight are allowed to finish, because cancelling a handshake mid-flight leaves the far side holding a half-open connection. The phase's bound is therefore the budget plus the last peer's attempts, which is why a snapshot peer is tried at no more than two addresses. Whatever is not restored refills through ordinary discovery, which is exactly the behaviour of a node that had no snapshot. 8. **Once per process, and not for clients.** A re-bootstrap does not replay the snapshot, since the table it would restore is the one the node already has. `NodeMode::Client` skips restoration entirely and keeps its existing six-peer startup bound: a client does not serve the DHT, so it never asks the question this repairs. -9. **A save never shrinks the snapshot.** The periodic and shutdown saves write the file; the post-bootstrap save does not. A save is skipped while the live table holds fewer peers than the snapshot restored at startup, because a node between opening and finishing its restore holds only what it has re-dialled so far, and one stopped in that window would otherwise replace a complete snapshot with a fragment and shrink its own file a little further on every cycle. The rule needs no other state and self-heals: once ordinary discovery refills the table past that floor, saving resumes. +9. **A save cannot shrink the snapshot while the table is still restoring.** The periodic and shutdown saves write the file; the post-bootstrap save does not. Nothing is written until the restore step has decided how many peers it recovered, so a node stopped before that step — and a client, which never restores — cannot overwrite a good file with an empty or partial table. For an hour after that decision, a save carrying fewer peers than the restore recovered is skipped, because a node stopped mid-restore holds only what it has re-dialled so far and would otherwise shrink its own file a little further on every cycle. After that hour the live table is the node's best knowledge, so the floor stops applying and the file keeps being refreshed rather than ageing out. An empty table is never written. ## Alternatives considered diff --git a/src/network.rs b/src/network.rs index 6a904a2..53d3655 100644 --- a/src/network.rs +++ b/src/network.rs @@ -36,7 +36,7 @@ use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tokio::sync::{Mutex as TokioMutex, RwLock, broadcast}; use tokio::time::Instant; @@ -188,6 +188,19 @@ const CLIENT_BOOTSTRAP_TARGET: usize = 6; /// QUIC+PQC handshakes. const MAX_CONCURRENT_SNAPSHOT_DIALS: usize = 16; +/// How long the restored peer count holds authority over saving. +/// +/// It exists to stop a node that is stopped mid-restore from writing the +/// fragment it has re-dialled so far over a complete snapshot. Once the table +/// has had this long to converge it is the node's best knowledge, and a smaller +/// table is a smaller network rather than an unfinished restore, so the floor +/// stops applying and the file keeps being refreshed. +const SNAPSHOT_FLOOR_ENFORCED_FOR: Duration = Duration::from_secs(60 * 60); + +/// The floor has not been decided yet: bootstrap has not reached the restore +/// step, or this is a client, which never restores and never writes one. +const SNAPSHOT_FLOOR_UNRESOLVED: usize = usize::MAX; + /// Addresses tried per snapshot peer. /// /// The budget below stops new peers, so the phase's real bound is the budget @@ -853,10 +866,13 @@ pub struct P2PNode { /// Whether the routing snapshot has already seeded a bootstrap in this /// process, so a re-bootstrap does not replay it. routing_snapshot_restored: AtomicBool, - /// How many peers the routing snapshot restored at startup, or zero when - /// none was. A save must not write a table smaller than this. Shared with - /// the periodic save task. + /// How many peers the routing snapshot restored at startup, zero when there + /// was none to restore, and [`SNAPSHOT_FLOOR_UNRESOLVED`] until bootstrap + /// has decided. A save must not write a table smaller than this while the + /// floor still applies. Shared with the periodic save task. routing_snapshot_floor: Arc, + /// When that floor was decided, in epoch seconds. Zero while unresolved. + routing_snapshot_floor_at: Arc, /// Periodic close-group-cache task, retained so shutdown can prevent a /// late periodic write from replacing the final snapshot. @@ -981,7 +997,8 @@ impl P2PNode { shutdown: CancellationToken::new(), close_group_cache_save_shutdown: CancellationToken::new(), routing_snapshot_restored: AtomicBool::new(false), - routing_snapshot_floor: Arc::new(AtomicUsize::new(0)), + routing_snapshot_floor: Arc::new(AtomicUsize::new(SNAPSHOT_FLOOR_UNRESOLVED)), + routing_snapshot_floor_at: Arc::new(AtomicU64::new(0)), close_group_cache_save_handle: TokioMutex::new(None), adaptive_dht, is_bootstrapped: Arc::new(AtomicBool::new(false)), @@ -1417,6 +1434,7 @@ impl P2PNode { let k_value = self.config.dht_config.k_value; let shutdown = self.close_group_cache_save_shutdown.clone(); let snapshot_floor = Arc::clone(&self.routing_snapshot_floor); + let snapshot_floor_at = Arc::clone(&self.routing_snapshot_floor_at); *task = Some(tokio::spawn(periodic_close_group_cache_save( dht_manager, trust_engine, @@ -1425,6 +1443,7 @@ impl P2PNode { dir, interval, snapshot_floor, + snapshot_floor_at, shutdown, ))); info!( @@ -1488,6 +1507,7 @@ impl P2PNode { now_epoch, dir, &self.routing_snapshot_floor, + &self.routing_snapshot_floor_at, "shutdown", ) .await; @@ -2434,6 +2454,21 @@ impl P2PNode { None } + /// Record how many peers the restore recovered from, and when. + /// + /// Until this is called a snapshot is never written, so a node stopped + /// before the restore step, and a client that never restores at all, cannot + /// overwrite a good file with an empty or partial table. + fn resolve_snapshot_floor(&self, restored_peers: usize) { + let now_epoch = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()); + self.routing_snapshot_floor + .store(restored_peers, Ordering::Relaxed); + self.routing_snapshot_floor_at + .store(now_epoch, Ordering::Relaxed); + } + /// Load the routing snapshot and turn it into dial candidates. /// /// Returns `None` when there is nothing usable — no configured directory, @@ -2448,9 +2483,13 @@ impl P2PNode { let snapshot = match RoutingSnapshot::load_from_dir(dir).await { Ok(Some(snapshot)) => snapshot, - Ok(None) => return None, + Ok(None) => { + self.resolve_snapshot_floor(0); + return None; + } Err(rejection) => { warn!(%rejection, "Discarding routing snapshot"); + self.resolve_snapshot_floor(0); return None; } }; @@ -2462,13 +2501,14 @@ impl P2PNode { Ok(peers) => peers, Err(rejection) => { warn!(%rejection, "Discarding routing snapshot"); + self.resolve_snapshot_floor(0); return None; } }; - // A later save must not write a table smaller than this one. - self.routing_snapshot_floor - .store(peers.len(), Ordering::Relaxed); + // A later save must not write a table smaller than this one, for as + // long as the floor applies. + self.resolve_snapshot_floor(peers.len()); let sets = snapshot_dial_sets(peers, &self.peer_id, seen_addresses); if sets.is_empty() { @@ -2604,20 +2644,31 @@ async fn save_close_group_cache_snapshot( /// Best-effort and non-fatal: the close-group cache is the established path and /// must not start failing because a newer, unread file could not be written. /// -/// Never writes a table smaller than the one restored at startup (`floor`). -/// Between opening and finishing its restore a node holds only what it has -/// re-dialled so far, and one stopped in that window would otherwise replace a -/// complete snapshot with a fragment, shrinking its own file a little further -/// on every cycle. The rule self-heals: as ordinary discovery refills the -/// table past the floor, saving resumes with no further state. +/// Writes nothing until the restore step has decided what the floor is, and +/// then nothing smaller than the table it restored while that floor still +/// applies. Between opening and finishing its restore a node holds only what it +/// has re-dialled so far, and one stopped in that window would otherwise +/// replace a complete snapshot with a fragment, shrinking its own file a little +/// further on every cycle. A client never resolves a floor and so never writes +/// a snapshot it would not read back. async fn save_routing_snapshot( dht_manager: &DhtNetworkManager, peer_id: PeerId, now_epoch: u64, dir: &Path, floor: &AtomicUsize, + floor_at: &AtomicU64, save_reason: &'static str, ) { + let floor_peers = floor.load(Ordering::Relaxed); + if floor_peers == SNAPSHOT_FLOOR_UNRESOLVED { + debug!( + save_reason, + "Skipping routing snapshot save: the restore step has not run" + ); + return; + } + let peers: Vec = dht_manager .routing_table_peers() .await @@ -2629,11 +2680,22 @@ async fn save_routing_snapshot( .collect(); let peer_count = peers.len(); - let floor = floor.load(Ordering::Relaxed); - if peer_count < floor { + if peer_count == 0 { + debug!( + save_reason, + "Skipping routing snapshot save: the table is empty" + ); + return; + } + let floor_applies = now_epoch.saturating_sub(floor_at.load(Ordering::Relaxed)) + < SNAPSHOT_FLOOR_ENFORCED_FOR.as_secs(); + if floor_applies && peer_count < floor_peers { debug!( save_reason, - peer_count, floor, "Skipping routing snapshot save: it would shrink the snapshot" + peer_count, + floor_peers, + "Skipping routing snapshot save: it would shrink the snapshot while the table is \ + still restoring" ); return; } @@ -2654,6 +2716,7 @@ async fn periodic_close_group_cache_save( dir: PathBuf, interval: Duration, snapshot_floor: Arc, + snapshot_floor_at: Arc, shutdown: CancellationToken, ) { let start = tokio::time::Instant::now() + interval; @@ -2684,6 +2747,7 @@ async fn periodic_close_group_cache_save( now_epoch, &dir, &snapshot_floor, + &snapshot_floor_at, "periodic", ).await; } From af7d75b427202192c1413e6c34e3ccf6abbd0226 Mon Sep 17 00:00:00 2001 From: grumbach Date: Fri, 14 Aug 2026 18:51:00 +0900 Subject: [PATCH 7/8] fix(bootstrap): publish the snapshot floor after its timestamp Both stores were relaxed and the count went first, so a save running concurrently with the restore step could see a resolved floor alongside the zero timestamp it was published with, read that as an expired floor, and write a partial table over a good snapshot. Timestamp first, count released after it, and the save acquires the count before reading the timestamp. --- src/network.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/network.rs b/src/network.rs index 53d3655..5c0c895 100644 --- a/src/network.rs +++ b/src/network.rs @@ -2463,10 +2463,13 @@ impl P2PNode { let now_epoch = SystemTime::now() .duration_since(UNIX_EPOCH) .map_or(0, |duration| duration.as_secs()); - self.routing_snapshot_floor - .store(restored_peers, Ordering::Relaxed); + // Publish the timestamp first and the count second with `Release`, so a + // save that sees a resolved floor cannot also see the zero timestamp it + // was published with and conclude the floor has already expired. self.routing_snapshot_floor_at .store(now_epoch, Ordering::Relaxed); + self.routing_snapshot_floor + .store(restored_peers, Ordering::Release); } /// Load the routing snapshot and turn it into dial candidates. @@ -2660,7 +2663,7 @@ async fn save_routing_snapshot( floor_at: &AtomicU64, save_reason: &'static str, ) { - let floor_peers = floor.load(Ordering::Relaxed); + let floor_peers = floor.load(Ordering::Acquire); if floor_peers == SNAPSHOT_FLOOR_UNRESOLVED { debug!( save_reason, From dac96dfe26e531b1f9b4cd96affcac773b851de0 Mon Sep 17 00:00:00 2001 From: Chris O'Neil Date: Sun, 16 Aug 2026 19:21:08 +0100 Subject: [PATCH 8/8] fix(bootstrap): refuse a special file at the snapshot path without blocking A FIFO placed at routing_snapshot.json stalled bootstrap indefinitely: the read-only open blocked until a writer appeared, before the regular-file check on the handle could reject it. Open with O_NONBLOCK and O_NOFOLLOW on Unix, so the open returns immediately, a symlink is refused outright, and the existing fstat-based rejection is actually reached. A path-level precheck was ruled out as TOCTOU-prone; the handle stays the single source of truth. Adds libc as a Unix-only dependency (already in the tree transitively) for the open flags and the mkfifo regression test. Requested in review on #152. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 1 + Cargo.toml | 3 ++ src/bootstrap/routing_snapshot.rs | 80 +++++++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eec2ff6..390719e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2480,6 +2480,7 @@ dependencies = [ "dirs 6.0.0", "futures", "hex", + "libc", "lru", "once_cell", "parking_lot", diff --git a/Cargo.toml b/Cargo.toml index ae07e11..e0e6809 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,9 @@ tokio-util = { version = "0.7", features = ["rt"] } # Fix wyz 0.5.0 compatibility issue with tap 1.0 (CI build failure) wyz = "=0.5.1" +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [dev-dependencies] tempfile = "3.17" diff --git a/src/bootstrap/routing_snapshot.rs b/src/bootstrap/routing_snapshot.rs index 7bc1960..af1afb1 100644 --- a/src/bootstrap/routing_snapshot.rs +++ b/src/bootstrap/routing_snapshot.rs @@ -207,8 +207,9 @@ impl RoutingSnapshot { /// Read the snapshot from `{dir}/routing_snapshot.json`. /// /// Returns `Ok(None)` when there is no snapshot. A file that exists but is - /// oversized, not a regular file, or unparseable is reported rather than - /// treated as absence, because the two need different operator responses. + /// oversized, not a regular file, a symlink, or unparseable is reported + /// rather than treated as absence, because the two need different operator + /// responses. /// /// # Errors /// @@ -220,7 +221,7 @@ impl RoutingSnapshot { // then reading it again is two different files if anything replaces it // in between. The read is separately capped, so the size check cannot // be sidestepped by a file that grows after it is opened. - let file = match tokio::fs::File::open(&path).await { + let file = match open_snapshot_file(&path).await { Ok(file) => file, Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), Err(e) => return Err(SnapshotRejection::Unreadable(e.to_string())), @@ -262,6 +263,37 @@ impl RoutingSnapshot { } } +/// Open the snapshot for reading without following a symlink and without +/// blocking on a special file. +/// +/// A plain `open` of a FIFO placed at this fixed path blocks until a writer +/// appears, which would stall bootstrap before the regular-file check on the +/// handle could reject it. `O_NONBLOCK` makes that open return immediately and +/// is inert for regular files, and `O_NOFOLLOW` refuses a symlink outright: +/// this node only ever writes a regular file here, by rename. +#[cfg(unix)] +async fn open_snapshot_file(path: &Path) -> std::io::Result { + use std::os::unix::fs::OpenOptionsExt as _; + + let path = path.to_path_buf(); + let file = tokio::task::spawn_blocking(move || { + std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_NONBLOCK | libc::O_NOFOLLOW) + .open(path) + }) + .await + .map_err(|e| std::io::Error::other(format!("snapshot open task panicked: {e}")))??; + Ok(tokio::fs::File::from_std(file)) +} + +/// Non-Unix fallback: a plain open, with the non-regular-file rejection still +/// enforced on the opened handle by the caller. +#[cfg(not(unix))] +async fn open_snapshot_file(path: &Path) -> std::io::Result { + tokio::fs::File::open(path).await +} + #[cfg(test)] #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod tests { @@ -357,6 +389,48 @@ mod tests { assert_eq!(loaded.peers.len(), MAX_SNAPSHOT_PEERS); } + /// Regression test: a plain blocking open of a FIFO at the snapshot path + /// hangs until a writer appears, so bootstrap never reached the + /// regular-file rejection. + #[cfg(unix)] + #[tokio::test] + async fn a_fifo_at_the_snapshot_path_is_refused_without_blocking() { + use std::os::unix::ffi::OsStrExt as _; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); + let c_path = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c_path.as_ptr(), 0o600) }, 0); + + let result = tokio::time::timeout( + Duration::from_secs(10), + RoutingSnapshot::load_from_dir(dir.path()), + ) + .await + .expect("loading must not block on a FIFO"); + assert!(matches!(result, Err(SnapshotRejection::Unreadable(_)))); + } + + #[cfg(unix)] + #[tokio::test] + async fn a_symlink_at_the_snapshot_path_is_refused() { + let dir = tempfile::tempdir().unwrap(); + let owner = PeerId::random(); + let target = dir.path().join("elsewhere.json"); + std::fs::write( + &target, + serde_json::to_vec(&snapshot(owner, 1_000, 4)).unwrap(), + ) + .unwrap(); + let path = dir.path().join(ROUTING_SNAPSHOT_FILENAME); + std::os::unix::fs::symlink(&target, &path).unwrap(); + + assert!(matches!( + RoutingSnapshot::load_from_dir(dir.path()).await, + Err(SnapshotRejection::Unreadable(_)) + )); + } + #[test] fn another_nodes_snapshot_is_refused() { let owner = PeerId::random();