diff --git a/crates/app/src/app/types.rs b/crates/app/src/app/types.rs index db6d2154..b8df6d5d 100644 --- a/crates/app/src/app/types.rs +++ b/crates/app/src/app/types.rs @@ -1327,13 +1327,13 @@ pub(super) fn update_peer_record( pub(super) fn compute_peer_backoff_secs(failures: u32) -> i64 { use rand::Rng; - const SECONDS_PER_BACKOFF: u64 = 10; - const MAX_BACKOFF_EXPONENT: u32 = 10; - let exp = failures.min(MAX_BACKOFF_EXPONENT); - let max = SECONDS_PER_BACKOFF.saturating_mul(1u64 << exp); + // The drift-prone ceiling/constants live in the shared + // `henyey_common::peer_backoff` module; only the (non-drift-prone) unbiased + // sampling stays here. The ceiling is always >= 1, so the range is never + // empty. + let ceiling = henyey_common::peer_backoff::connect_backoff_ceiling_secs(failures); let mut rng = rand::thread_rng(); - let jitter = rng.gen_range(1..=max.max(1)); - jitter as i64 + rng.gen_range(1..=ceiling) as i64 } pub(super) fn current_epoch_seconds() -> i64 { diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 5e4d419c..51805781 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -63,6 +63,7 @@ pub mod memory; pub mod meta; pub mod meta_walk; pub mod network; +pub mod peer_backoff; pub mod protocol; pub mod resource; pub mod spawn; diff --git a/crates/common/src/peer_backoff.rs b/crates/common/src/peer_backoff.rs new file mode 100644 index 00000000..7850b97c --- /dev/null +++ b/crates/common/src/peer_backoff.rs @@ -0,0 +1,92 @@ +//! Shared connect-backoff schedule for the overlay peer-retry logic. +//! +//! This module is the single source of truth for the drift-prone parts of the +//! peer connect-backoff schedule: the two schedule constants and the +//! deterministic backoff ceiling. Prior to consolidation the schedule was +//! copied verbatim at three call sites (`henyey-overlay`'s `PeerManager` and +//! the overlay tick loop, plus `henyey-app`'s persisted peer-record update), +//! so a change to one copy would silently leave the others stale. Routing all +//! three through this module makes any future schedule edit a single-point +//! change that all sites read, and pins the constants so drift fails loudly. +//! +//! Mirrors stellar-core `PeerManager::computeBackoff` +//! (`stellar-core/src/overlay/PeerManager.cpp:365-410`) and `OVERLAY_SPEC +//! §10.3-1`: a random delay in +//! `[1, 2^min(num_failures, MAX_BACKOFF_EXPONENT) * SECONDS_PER_BACKOFF]` +//! seconds, doubling the ceiling per consecutive failure and capped at +//! exponent 10 (~2.84h ceiling). +//! +//! Only the *ceiling* (the `min(n, MAX_BACKOFF_EXPONENT)` cap and the +//! `<<`/`*` arithmetic) and the constants live here — the drift-prone parts. +//! The unbiased uniform sampling (`gen_range(1..=ceiling)`) stays at each call +//! site, so `henyey-common` need not depend on `rand`. + +/// Seconds per backoff unit. +/// +/// Matches stellar-core `PeerManager.cpp:365-410` and `OVERLAY_SPEC §10.3-1`. +pub const SECONDS_PER_BACKOFF: u64 = 10; + +/// Maximum backoff exponent. The ceiling doubles per consecutive failure up to +/// this exponent, then stays clamped. +/// +/// Matches stellar-core `PeerManager.cpp:365-410` and `OVERLAY_SPEC §10.3-1`. +pub const MAX_BACKOFF_EXPONENT: u32 = 10; + +/// Deterministic upper bound (inclusive, in seconds) for the connect-backoff +/// delay given a consecutive failure count. +/// +/// Returns `2^min(num_failures, MAX_BACKOFF_EXPONENT) * SECONDS_PER_BACKOFF`. +/// The value is always `>= SECONDS_PER_BACKOFF` (>= 10), so callers can safely +/// sample `gen_range(1..=ceiling)` without an empty-range guard. +/// +/// Call sites sample uniformly in `[1, ceiling]` to produce the actual delay +/// (see module docs); this function centralizes only the drift-prone ceiling. +pub fn connect_backoff_ceiling_secs(num_failures: u32) -> u64 { + let exponent = num_failures.min(MAX_BACKOFF_EXPONENT); + (1u64 << exponent) * SECONDS_PER_BACKOFF +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The ceiling doubles per failure from 10s and clamps at exponent 10 + /// (10240s), exercising the `min(n, MAX_BACKOFF_EXPONENT)` cap. + #[test] + fn test_ceiling_schedule() { + let expected = [10u64, 20, 40, 80, 160, 320, 640, 1280, 2560, 5120, 10240]; + for (n, &want) in expected.iter().enumerate() { + assert_eq!( + connect_backoff_ceiling_secs(n as u32), + want, + "ceiling for {n} failures" + ); + } + // Beyond MAX_BACKOFF_EXPONENT the ceiling stays clamped. + assert_eq!(connect_backoff_ceiling_secs(11), 10240); + assert_eq!(connect_backoff_ceiling_secs(1000), 10240); + assert_eq!(connect_backoff_ceiling_secs(u32::MAX), 10240); + } + + /// Pin the schedule constants to their spec values. This is the load-bearing + /// drift guard: after consolidation there is exactly one definition of each + /// constant and all three call sites read it, so any future schedule edit + /// flips this test and the sites can no longer diverge from each other. + #[test] + fn test_schedule_constants_pinned() { + assert_eq!(SECONDS_PER_BACKOFF, 10); + assert_eq!(MAX_BACKOFF_EXPONENT, 10); + } + + /// The ceiling is always `>= 1` (in fact `>= 10`) for every failure count, + /// so a call site sampling `gen_range(1..=ceiling)` always sees a non-empty + /// range and never yields 0. (The sampling itself stays at each site and is + /// exercised by those sites' tests, e.g. the overlay tick-loop backoff + /// bounds test — `henyey-common` is intentionally `rand`-free.) + #[test] + fn test_ceiling_never_below_one() { + for &n in &[0u32, 5, 10, 1000, u32::MAX] { + assert!(connect_backoff_ceiling_secs(n) >= 1); + } + } +} diff --git a/crates/overlay/src/manager/tick.rs b/crates/overlay/src/manager/tick.rs index 948a543a..6747474c 100644 --- a/crates/overlay/src/manager/tick.rs +++ b/crates/overlay/src/manager/tick.rs @@ -53,11 +53,11 @@ pub(super) const PEER_IP_RESOLVE_RETRY_DELAY: Duration = Duration::from_secs(10) /// Replaces the old flat `OUTBOUND_CONNECT_RETRY_DELAY` (10s forever), which /// re-dialed dead preferred peers every ~20s indefinitely (#3770). fn compute_connect_backoff(num_failures: u32) -> Duration { - const SECONDS_PER_BACKOFF: u32 = 10; - const MAX_BACKOFF_EXPONENT: u32 = 10; - let backoff_count = num_failures.min(MAX_BACKOFF_EXPONENT); - let ceiling_secs = (1u32 << backoff_count) * SECONDS_PER_BACKOFF; - Duration::from_secs(rand::thread_rng().gen_range(1..=ceiling_secs) as u64) + // The drift-prone ceiling/constants live in the shared + // `henyey_common::peer_backoff` module; only the (non-drift-prone) unbiased + // sampling stays here. + let ceiling_secs = henyey_common::peer_backoff::connect_backoff_ceiling_secs(num_failures); + Duration::from_secs(rand::thread_rng().gen_range(1..=ceiling_secs)) } /// Result of a background DNS resolution of configured peers. @@ -1852,10 +1852,7 @@ mod tests { /// so the test can safely advance simulated time past any possible /// jittered draw, without depending on (non-seedable) RNG internals. fn max_connect_backoff_secs(num_failures: u32) -> u64 { - const SECONDS_PER_BACKOFF: u32 = 10; - const MAX_BACKOFF_EXPONENT: u32 = 10; - let backoff_count = num_failures.min(MAX_BACKOFF_EXPONENT); - ((1u32 << backoff_count) * SECONDS_PER_BACKOFF) as u64 + henyey_common::peer_backoff::connect_backoff_ceiling_secs(num_failures) } /// Repeated failures against the same preferred peer must accumulate a diff --git a/crates/overlay/src/peer_manager.rs b/crates/overlay/src/peer_manager.rs index 7468e5c9..6ad4cbce 100644 --- a/crates/overlay/src/peer_manager.rs +++ b/crates/overlay/src/peer_manager.rs @@ -45,12 +45,6 @@ pub const MAX_FAILURES: u32 = 10; #[cfg(test)] pub const REALLY_DEAD_NUM_FAILURES_CUTOFF: u32 = 120; -/// Seconds per backoff unit. -const SECONDS_PER_BACKOFF: u64 = 10; - -/// Maximum backoff exponent. -const MAX_BACKOFF_EXPONENT: u32 = 10; - pub use henyey_common::StoredPeerType; /// Filter for querying peers. @@ -603,14 +597,14 @@ fn apply_type_update(record: &mut PeerRecord, update: TypeUpdate) { /// Compute backoff duration based on failure count. fn compute_backoff(num_failures: u32) -> Duration { - let backoff_count = num_failures.min(MAX_BACKOFF_EXPONENT); - let max_seconds = (1u64 << backoff_count) * SECONDS_PER_BACKOFF; - // Sample uniformly in [1, max_seconds] (inclusive), matching the spec's + // Sample uniformly in [1, ceiling] (inclusive), matching the spec's // random_uniform(1, max_seconds). `gen_range` is unbiased, unlike the // previous `rand::random() % max_seconds` which had modulo bias. See - // OVERLAY_SPEC §10.3 (claim OVERLAY §10.3-1). max_seconds is always >= 1 - // since backoff_count >= 0 and SECONDS_PER_BACKOFF >= 1. - let random_seconds = rand::thread_rng().gen_range(1..=max_seconds); + // OVERLAY_SPEC §10.3 (claim OVERLAY §10.3-1). The ceiling is always >= 1, + // so the range is never empty. The drift-prone ceiling/constants live in + // the shared `henyey_common::peer_backoff` module. + let ceiling = henyey_common::peer_backoff::connect_backoff_ceiling_secs(num_failures); + let random_seconds = rand::thread_rng().gen_range(1..=ceiling); Duration::from_secs(random_seconds) }