diff --git a/src/node/dataplane/dispatch.rs b/src/node/dataplane/dispatch.rs index 89ddf14..4d8bdd3 100644 --- a/src/node/dataplane/dispatch.rs +++ b/src/node/dataplane/dispatch.rs @@ -93,7 +93,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - self.schedule_reconnect(addr, now_ms); + self.note_link_dead(addr, now_ms); } /// Remove an active peer and clean up all associated state. diff --git a/src/node/dataplane/encrypted.rs b/src/node/dataplane/encrypted.rs index 2d69adc..e06880a 100644 --- a/src/node/dataplane/encrypted.rs +++ b/src/node/dataplane/encrypted.rs @@ -534,7 +534,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - self.schedule_reconnect(addr, now_ms); + self.note_link_dead(addr, now_ms); } } } diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 0000442..8b6875f 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -417,7 +417,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - self.schedule_reconnect(peer, now_ms); + self.note_link_dead(peer, now_ms); } InboundDecision::Promote => {} } diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index dc175ca..e1f9866 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -506,7 +506,7 @@ impl Node { "Removing peer: link dead timeout" ); self.remove_active_peer(&peer); - self.schedule_reconnect(peer, now_ms); + self.note_link_dead(peer, now_ms); } MmpAction::Heartbeat { peer } => { if let Some(p) = self.peers.get_mut(&peer) { diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index d61270e..af88741 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -77,7 +77,7 @@ impl Node { let stale = self.stale_connections(now_ms, timeout_ms); for action in self.fmp.poll_timeouts(stale) { match action { - ConnAction::ScheduleRetry { peer } => self.schedule_retry(peer, now_ms), + ConnAction::ScheduleRetry { peer } => self.note_handshake_timeout(peer, now_ms), ConnAction::Teardown { link } => { // Log before cleanup (needs live connection state). if let Some(conn) = self.connections.get(&link) { diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index a24ed7b..5209473 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -3,7 +3,10 @@ pub(crate) mod supervisor; use super::{Node, NodeError, NodeState}; -use supervisor::{Action, Child, Event, SupervisorFsm}; +use supervisor::{Action, Child, Event, PeeringDesired, SupervisorFsm}; + +use super::peering::reconcile::{Budget, DiscoveryPools, Gate, Observed, PeeringAction, Policy}; +use super::peering::retry::MAX_RETRY_CONNECTIONS_PER_TICK; use crate::config::{ConnectPolicy, PeerAddress, PeerConfig}; use crate::node::acl::PeerAclContext; @@ -158,7 +161,7 @@ impl Node { error = %err, "Failed to initiate connection for newly added runtime peer" ); - self.schedule_retry(*identity.node_addr(), Self::now_ms()); + self.note_handshake_timeout(*identity.node_addr(), Self::now_ms()); } } @@ -203,7 +206,7 @@ impl Node { error = %err, "Refreshed peer addresses did not initiate a direct connection" ); - self.schedule_retry(node_addr, Self::now_ms()); + self.note_handshake_timeout(node_addr, Self::now_ms()); } } } @@ -242,20 +245,62 @@ impl Node { self.register_identity(*identity.node_addr(), identity.pubkey_full()); } - // Collect peer configs to avoid borrow conflicts - let peer_configs: Vec<_> = self.config().auto_connect_peers().cloned().collect(); + // Collect the auto-connect peer configs and build the mandatory-floor + // reconcile inputs. This is the startup-gate seam (design §3 D4): the + // substrate is up (transports created, before TUN) but the published + // NodeState is still `Starting`, so pass `Gate::Reconciling` EXPLICITLY + // rather than deriving it from the published state (which would map to + // `NotRunning` and dial nothing). This preserves today's startup dial + // position exactly. + let auto_connect_peers: Vec = + self.config().auto_connect_peers().cloned().collect(); - if peer_configs.is_empty() { + if auto_connect_peers.is_empty() { debug!("No static peers configured"); return; } + // Recover the full `PeerConfig` for each emitted floor `Connect` from its + // candidate identity (the core carries identity + a placeholder address; + // the driver dial needs the config to expand addresses). + let configs_by_addr: HashMap = auto_connect_peers + .iter() + .filter_map(|pc| { + PeerIdentity::from_npub(&pc.npub) + .ok() + .map(|id| (*id.node_addr(), pc.clone())) + }) + .collect(); + debug!( - count = peer_configs.len(), + count = configs_by_addr.len(), "Initiating static peer connections" ); - for peer_config in peer_configs { + let policy = self.build_peering_policy(auto_connect_peers); + let observed = self.observe_peering(); + let budget = self.build_peering_budget(); + let now_ms = Self::now_ms(); + let actions = self.peering.reconciler.reconcile( + &policy, + &observed, + &budget, + &DiscoveryPools::default(), + now_ms, + Gate::Reconciling, + ); + + for action in actions { + let PeeringAction::Connect(candidate) = action else { + continue; + }; + let Some(identity) = candidate.identity else { + continue; + }; + let node_addr = *identity.node_addr(); + let Some(peer_config) = configs_by_addr.get(&node_addr).cloned() else { + continue; + }; if let Err(e) = self.initiate_peer_connection(&peer_config).await { warn!( npub = %peer_config.npub, @@ -266,9 +311,7 @@ impl Node { // Schedule a retry so transient address-resolution failures // (e.g. cached endpoints stale, NAT rebinds, all addresses // currently unreachable) recover without a daemon restart. - if let Ok(peer_identity) = PeerIdentity::from_npub(&peer_config.npub) { - self.schedule_retry(*peer_identity.node_addr(), Self::now_ms()); - } + self.note_handshake_timeout(node_addr, now_ms); // No-transport failures most often mean the cached overlay // advert is pointing at a dead post-NAT-rebind address. The // advert cache is read-only inside fetch_advert, so retries @@ -774,7 +817,10 @@ impl Node { Err(err) => { warn!(peer_npub = %peer_npub, error = %err, "Failed to adopt NAT traversal"); if let Ok(peer_identity) = PeerIdentity::from_npub(&peer_npub) { - self.schedule_retry(*peer_identity.node_addr(), Self::now_ms()); + self.note_handshake_timeout( + *peer_identity.node_addr(), + Self::now_ms(), + ); } } } @@ -863,7 +909,7 @@ impl Node { continue; } - self.schedule_retry(node_addr, now_ms); + self.note_handshake_timeout(node_addr, now_ms); if let Some(cooldown_until_ms) = decision.cooldown_until_ms && let Some(state) = self.peering.reconciler.retry_pending.get_mut(&node_addr) @@ -1053,7 +1099,7 @@ impl Node { // Clean up link and schedule retry self.remove_link(&pending.link_id); self.links.remove(&pending.link_id); - self.schedule_retry(*pending.peer_identity.node_addr(), Self::now_ms()); + self.note_handshake_timeout(*pending.peer_identity.node_addr(), Self::now_ms()); } } } @@ -1857,11 +1903,18 @@ impl Node { // The rx loop owns the bounded wait; the FSM's timer is // carried for observability only. No-op here. } - Action::SetPeeringDesired(_) | Action::SuspendReplenish => { - // §8 reconciler drain-gate. Documented no-op in this commit: - // the homeostatic reconciler that consumes these lands in - // Step 1b. Without it there is nothing to reconnect the peers - // the drain closes, so the gate is implicitly satisfied. + Action::SetPeeringDesired(PeeringDesired::Empty) => { + // §8 reconciler drain-gate (obligation O4): clear the queued + // retry schedule so the disconnects the drain itself causes + // cannot leave reconnect entries behind. + self.peering.reconciler.retry_pending.clear(); + } + Action::SuspendReplenish => { + // §8 reconciler drain-gate: no extra latch needed. The whole + // drain window is `Gate::Suspended` + // (`Gate::from_state(NodeState::Draining)`), so the per-tick + // retry-dial reconcile and every peer-loss reflex read that + // gate and self-suppress for the duration of the drain. } Action::SpawnChild(_) | Action::StopChild(_) | Action::PublishState(_) => { // Drain entry never emits child or publish-state actions @@ -2363,6 +2416,79 @@ impl Node { self.supervisor.nostr_rendezvous.set_startup_sweep_done(); } + /// Build the reconciler [`Policy`] from config. `auto_connect_peers` is + /// filled by the caller: the startup floor and the reflex wrappers pass the + /// configured auto-connect set; the per-tick retry-dial slot passes an empty + /// set so the config floor stays silent (cadence contract, design §3 D4). + pub(in crate::node) fn build_peering_policy( + &self, + auto_connect_peers: Vec, + ) -> Policy { + let cfg = self.config(); + let retry = &cfg.node.retry; + let nostr = &cfg.node.rendezvous.nostr; + Policy { + auto_connect_peers, + max_peers: self.max_peers(), + max_connections: self.max_connections(), + max_links: self.max_links(), + retry_base_interval_ms: retry.base_interval_secs.saturating_mul(1000), + retry_max_backoff_ms: retry.max_backoff_secs.saturating_mul(1000), + retry_max_retries: retry.max_retries, + handshake_timeout_ms: cfg + .node + .rate_limit + .handshake_timeout_secs + .saturating_mul(1000), + open_discovery_enabled: nostr.enabled + && nostr.policy == crate::config::NostrRendezvousPolicy::Open, + open_discovery_max_pending: nostr.open_discovery_max_pending, + open_discovery_expires_ms: nostr + .advert_ttl_secs + .saturating_mul(1000) + .saturating_mul(OPEN_DISCOVERY_RETRY_LIFETIME_MULTIPLIER), + } + } + + /// Snapshot the live dataplane maps into the reconciler's [`Observed`] input. + /// + /// For the mandatory-floor + retry-dial cutover (C3b) only the `connected` + /// and `connecting` sets are read by the core; the count/in-flight fields + /// are populated by the overlay/opportunistic cutovers that follow. + pub(in crate::node) fn observe_peering(&self) -> Observed { + let connected: HashSet = self.peers.keys().copied().collect(); + let connecting: HashSet = self + .connections + .values() + .filter_map(|conn| conn.expected_identity().map(|id| *id.node_addr())) + .collect(); + Observed { + connected, + connecting, + ..Observed::default() + } + } + + /// Build the admission [`Budget`] from the live maps. This is the surviving + /// home for the slot arithmetic; the shared helpers it wraps stay until the + /// overlay/opportunistic cutovers consume them (obligation O5). + pub(in crate::node) fn build_peering_budget(&self) -> Budget { + let peer_slots = if self.max_peers() == 0 { + usize::MAX + } else { + self.max_peers().saturating_sub(self.peers.len()) + }; + Budget { + handshake_slots: self.outbound_handshake_slots(), + link_slots: self.outbound_link_slots(), + peer_slots, + admission_ok: self.outbound_admission_check(), + discovery_per_tick: MAX_DISCOVERY_CONNECTS_PER_TICK, + retry_per_tick: MAX_RETRY_CONNECTIONS_PER_TICK, + per_peer_cap: MAX_PARALLEL_PATH_CANDIDATES_PER_PEER, + } + } + fn available_outbound_slots(&self) -> usize { let connection_used = self .connections diff --git a/src/node/peering/driver.rs b/src/node/peering/driver.rs new file mode 100644 index 0000000..1117bb2 --- /dev/null +++ b/src/node/peering/driver.rs @@ -0,0 +1,148 @@ +//! Thin async driver for the peering reconciler. +//! +//! These `impl Node` methods are the I/O edge of the sans-IO +//! [`super::reconcile::PeeringReconciler`]: they snapshot the live dataplane +//! maps into the reconciler's plain-data inputs, invoke the pure core, and +//! perform the dial / advert-refetch I/O each [`PeeringAction`] names. They also +//! host the two gate-guarded reflex wrappers every peer-loss call site routes +//! through, so drain suppression and the connected-guard live in one place. +//! +//! The `Policy` / `Observed` / `Budget` builders these methods consume live in +//! [`crate::node::lifecycle`] next to the surviving budget helpers and limit +//! constants they wrap. + +use crate::identity::NodeAddr; +use crate::node::{Node, NodeError}; +use tracing::warn; + +use super::reconcile::{DiscoveryPools, Gate, PeeringAction}; + +impl Node { + /// Reflex: an outbound handshake timed out (replaces the old + /// `Node::schedule_retry` call sites). + /// + /// Replicates `schedule_retry`'s connected-guard (obligation O2) — the pure + /// core cannot observe the peers map, so the driver drops the event when the + /// peer is already connected — then feeds the gate-guarded reconciler reflex + /// with the gate derived from the live published state (obligation O4). + pub(in crate::node) fn note_handshake_timeout(&mut self, node_addr: NodeAddr, now_ms: u64) { + if self.peers.contains_key(&node_addr) { + return; + } + let policy = + self.build_peering_policy(self.config().auto_connect_peers().cloned().collect()); + let gate = Gate::from_state(self.supervisor.state); + let _ = self + .peering + .reconciler + .on_handshake_timeout(node_addr, now_ms, &policy, gate); + } + + /// Reflex: a link went dead / a peer was lost (replaces the old + /// `Node::schedule_reconnect` call sites). + /// + /// No connected-guard — the peer is already gone by the time a link-dead / + /// disconnect event fires (`schedule_reconnect` had none). The gate is + /// derived from the live published state so a drain self-suppresses the + /// reconnect (obligation O4, design §8 correctness trap). + pub(in crate::node) fn note_link_dead(&mut self, node_addr: NodeAddr, now_ms: u64) { + let policy = + self.build_peering_policy(self.config().auto_connect_peers().cloned().collect()); + let gate = Gate::from_state(self.supervisor.state); + let _ = self + .peering + .reconciler + .on_link_dead(node_addr, now_ms, &policy, gate); + } + + /// Process pending retries whose time has arrived (replaces the old + /// `Node::process_pending_retries` body). + /// + /// The pure retry-dial phase owns the decision — drop expired entries, refuse + /// to grow when admission binds, dial the first `retry_per_tick` due entries + /// (bumping their `retry_after_ms` past the handshake window). This driver + /// performs the advert-refetch + dial I/O each emitted `Connect` names, and + /// on an immediate dial error feeds the `on_handshake_timeout` reflex so the + /// optimistic re-fire suppression is overwritten by proper backoff + /// (obligation O3). During a drain the gate is `Suspended`, so the reconcile + /// clears the schedule and emits nothing (the design §8 gate). + pub(in crate::node) async fn process_pending_retries(&mut self, now_ms: u64) { + if self.peering.reconciler.retry_pending.is_empty() { + return; + } + + // Retry-dial cadence slot: empty config floor (design §3 D4) and empty + // discovery pools, so only the retry-dial phase acts. + let policy = self.build_peering_policy(Vec::new()); + let observed = self.observe_peering(); + let budget = self.build_peering_budget(); + let gate = Gate::from_state(self.supervisor.state); + let actions = self.peering.reconciler.reconcile( + &policy, + &observed, + &budget, + &DiscoveryPools::default(), + now_ms, + gate, + ); + + for action in actions { + let PeeringAction::Connect(candidate) = action else { + continue; + }; + let Some(identity) = candidate.identity else { + continue; + }; + let node_addr = *identity.node_addr(); + let Some(peer_config) = self + .peering + .reconciler + .retry_pending + .get(&node_addr) + .map(|state| state.peer_config.clone()) + else { + continue; + }; + + // Refresh the peer's overlay advert before retrying. The cache is + // read-only on hit, so a retry without a refetch dials the same + // cached endpoint — and the most common reason a peer landed in the + // retry schedule is that endpoint just stopped working (NAT rebind, + // port change, peer restart). Cheap (one Filter fetch, bounded by + // the retry backoff cadence). + if let Some(bootstrap) = self.supervisor.nostr_rendezvous.engine_arc() { + let _ = bootstrap + .refetch_advert_for_stale_check(&peer_config.npub) + .await; + } + + match self.initiate_peer_connection(&peer_config).await { + // The core already pushed `retry_after_ms` past the handshake + // window; a successful promotion clears the entry, a later + // timeout re-fires the reflex with proper backoff. + Ok(()) => {} + Err(e) => { + warn!( + peer = %self.peer_display_name(&node_addr), + error = %e, + "Retry connection initiation failed" + ); + // No-transport failures usually mean the cached overlay + // advert is stale; force a re-fetch so the next tick picks up + // fresh endpoints. + if matches!(e, NodeError::NoTransportForType(_)) + && let Some(bootstrap) = self.supervisor.nostr_rendezvous.engine_arc() + { + let npub = peer_config.npub.clone(); + tokio::spawn(async move { + let _ = bootstrap.refetch_advert_for_stale_check(&npub).await; + }); + } + // Immediate failure counts as an attempt: overwrite the + // optimistic re-fire suppression with backoff (obligation O3). + self.note_handshake_timeout(node_addr, now_ms); + } + } + } + } +} diff --git a/src/node/peering/mod.rs b/src/node/peering/mod.rs index fbe63bb..b047a54 100644 --- a/src/node/peering/mod.rs +++ b/src/node/peering/mod.rs @@ -9,7 +9,6 @@ //! connection is created per re-dial, so the escalating backoff count must //! persist in the reconciler, not per-connection. -pub(in crate::node) mod retry; - -#[allow(dead_code)] // Wired by the driver cutover in the following commits; unused until then. +pub(in crate::node) mod driver; pub(in crate::node) mod reconcile; +pub(in crate::node) mod retry; diff --git a/src/node/peering/reconcile.rs b/src/node/peering/reconcile.rs index 005dfa1..1037f98 100644 --- a/src/node/peering/reconcile.rs +++ b/src/node/peering/reconcile.rs @@ -142,12 +142,19 @@ pub(crate) struct Budget { #[derive(Clone, Debug, Default)] pub(crate) struct Observed { /// `self.peers.len()`. + // The scalar counts are carried for the ceiling's future set-point use and + // driver observability; the C3b core reads admission via `Budget`, so they + // are not yet read by any layer. + #[allow(dead_code)] // populated for observability; read by later cutovers pub peers: usize, /// `self.connections.len()`. + #[allow(dead_code)] // populated for observability; read by later cutovers pub connections: usize, /// `self.links.len()`. + #[allow(dead_code)] // populated for observability; read by later cutovers pub links: usize, /// `self.pending_connects.len()`. + #[allow(dead_code)] // populated for observability; read by later cutovers pub pending_connects: usize, /// The `peers` map keys (fully authenticated peers). pub connected: HashSet, @@ -164,9 +171,13 @@ pub(crate) struct Observed { /// anonymous-capable. #[derive(Clone, Debug)] pub(crate) struct Candidate { + // transport_id / remote_addr are read by the opportunistic-growth driver + // cutover (C5); the mandatory-floor + retry cutover (C3b) dials by identity. /// The transport to dial over. + #[allow(dead_code)] // wired by the opportunistic cutover (C5) pub transport_id: TransportId, /// The remote address to dial. + #[allow(dead_code)] // wired by the opportunistic cutover (C5) pub remote_addr: TransportAddr, /// The peer identity; `None` is an anonymous first-contact leg (design §7). pub identity: Option, @@ -225,11 +236,17 @@ pub(crate) struct DiscoveryPools { pub(crate) struct Policy { /// `config.peers()` filtered by `is_auto_connect()`. pub auto_connect_peers: Vec, + // The limit triple is enforced through `Budget` (the admission arithmetic the + // driver pre-computes), so the core does not read these directly; they are + // carried on the policy for completeness and future set-point use. /// `node.limits.max_peers`. + #[allow(dead_code)] // ceiling is enforced via Budget; carried for completeness pub max_peers: usize, /// `node.limits.max_connections`. + #[allow(dead_code)] // ceiling is enforced via Budget; carried for completeness pub max_connections: usize, /// `node.limits.max_links`. + #[allow(dead_code)] // ceiling is enforced via Budget; carried for completeness pub max_links: usize, /// `node.retry.base_interval_secs * 1000`. pub retry_base_interval_ms: u64, @@ -265,8 +282,10 @@ pub(crate) enum PeeringAction { /// only; the durable mutation is on the reconciler's `retry_pending`. ScheduleRetry { /// The peer whose retry was scheduled. + #[allow(dead_code)] // observability payload; the durable mutation is internal peer: NodeAddr, /// The backoff delay applied. + #[allow(dead_code)] // observability payload; the durable mutation is internal backoff_ms: u64, }, } diff --git a/src/node/peering/retry.rs b/src/node/peering/retry.rs index 246fd4c..64cab49 100644 --- a/src/node/peering/retry.rs +++ b/src/node/peering/retry.rs @@ -1,18 +1,18 @@ -//! Connection retry logic for auto-connect peers. +//! Cross-attempt retry state for auto-connect peers. //! -//! When an outbound handshake fails (timeout or send error), the node can -//! automatically retry with exponential backoff. Retry state lives on Node -//! (not PeerConnection) because each retry creates a fresh connection. +//! [`RetryState`] is the durable per-peer schedule entry the peering reconciler +//! owns (it lives in [`crate::node::peering::reconcile::PeeringReconciler`], not +//! on a per-connection object, because a fresh connection is created per re-dial +//! so the escalating backoff count must persist across attempts). The decision +//! logic that reads and mutates it — the retry-dial phase and the +//! `on_handshake_timeout` / `on_link_dead` reflexes — lives in the sans-IO +//! reconciler core; the driver wrappers that feed it (retry-dial I/O, the +//! gate-guarded reflex call sites) live in [`super::driver`]. -use crate::PeerIdentity; use crate::config::PeerConfig; -use crate::identity::NodeAddr; -use crate::node::{Node, NodeError}; -use crate::proto::fmp::backoff_ms; -use tracing::{debug, info, warn}; -// MAX_BACKOFF_MS is now derived from config: node.retry.max_backoff_secs * 1000 -const MAX_RETRY_CONNECTIONS_PER_TICK: usize = 16; +/// Per-tick cap on retry-dial connection attempts (design §6 ceiling). +pub(in crate::node) const MAX_RETRY_CONNECTIONS_PER_TICK: usize = 16; /// Tracks retry state for a peer across connection attempts. pub struct RetryState { @@ -47,299 +47,3 @@ impl RetryState { } } } - -impl Node { - /// Schedule a retry for a failed outbound connection, if applicable. - /// - /// Only schedules if the peer is an auto-connect peer and max retries - /// have not been exhausted (unless `reconnect` is true, which retries - /// indefinitely). Does nothing if the peer is already connected or has - /// a connection in progress. - pub(in crate::node) fn schedule_retry(&mut self, node_addr: NodeAddr, now_ms: u64) { - let retry_cfg = &self.config().node.retry; - let max_retries = retry_cfg.max_retries; - if max_retries == 0 { - return; - } - - // Don't retry if peer is already connected - if self.peers.contains_key(&node_addr) { - return; - } - - let base_interval_ms = retry_cfg.base_interval_secs * 1000; - let max_backoff_ms = retry_cfg.max_backoff_secs * 1000; - let peer_name = self.peer_display_name(&node_addr); - - if let Some(state) = self.peering.reconciler.retry_pending.get_mut(&node_addr) { - // Already tracking — increment - state.retry_count += 1; - if !state.reconnect && state.retry_count > max_retries { - info!( - peer = %peer_name, - attempts = state.retry_count, - "Max retries exhausted, giving up on peer" - ); - self.peering.reconciler.retry_pending.remove(&node_addr); - return; - } - let delay = backoff_ms(state.retry_count, base_interval_ms, max_backoff_ms); - state.retry_after_ms = now_ms + delay; - debug!( - peer = %peer_name, - retry = state.retry_count, - reconnect = state.reconnect, - delay_secs = delay / 1000, - "Scheduling connection retry" - ); - } else { - // First failure — find the matching PeerConfig - let peer_config = self - .config() - .auto_connect_peers() - .find(|pc| { - PeerIdentity::from_npub(&pc.npub) - .map(|id| *id.node_addr() == node_addr) - .unwrap_or(false) - }) - .cloned(); - - if let Some(pc) = peer_config { - let mut state = RetryState::new(pc); - state.retry_count = 1; - state.reconnect = true; - let delay = backoff_ms(state.retry_count, base_interval_ms, max_backoff_ms); - state.retry_after_ms = now_ms + delay; - debug!( - peer = %self.peer_display_name(&node_addr), - delay_secs = delay / 1000, - "First connection attempt failed, scheduling retry" - ); - self.peering - .reconciler - .retry_pending - .insert(node_addr, state); - } - // If not found in auto_connect_peers, no retry (one-shot connection) - } - } - - /// Schedule auto-reconnect for a peer removed by MMP dead timeout. - /// - /// Looks up the peer in auto-connect config and checks `auto_reconnect`. - /// If enabled, feeds the peer into the retry system with unlimited retries. - /// - /// If a retry entry already exists (e.g. from a previous failed handshake - /// attempt during an earlier reconnect cycle), the existing retry count is - /// preserved and incremented rather than reset to zero. This ensures - /// exponential backoff accumulates across repeated link-dead events instead - /// of resetting to the base interval on every peer removal. - pub(in crate::node) fn schedule_reconnect(&mut self, node_addr: NodeAddr, now_ms: u64) { - // Find peer in auto-connect config - let peer_config = self - .config() - .auto_connect_peers() - .find(|pc| { - PeerIdentity::from_npub(&pc.npub) - .map(|id| *id.node_addr() == node_addr) - .unwrap_or(false) - }) - .cloned(); - - let Some(pc) = peer_config else { - return; // Not an auto-connect peer, no reconnect - }; - - if !pc.auto_reconnect { - debug!( - peer = %self.peer_display_name(&node_addr), - "Auto-reconnect disabled for peer, skipping" - ); - return; - } - - let base_interval_ms = self.config().node.retry.base_interval_secs * 1000; - let max_backoff_ms = self.config().node.retry.max_backoff_secs * 1000; - let peer_name = self.peer_display_name(&node_addr); - - // If we already have accumulated backoff from previous failed attempts, - // preserve and bump it rather than resetting to zero. This prevents the - // exponential backoff from being discarded on each link-dead cycle. - if let Some(state) = self.peering.reconciler.retry_pending.get_mut(&node_addr) { - state.reconnect = true; - state.retry_count += 1; - let delay = backoff_ms(state.retry_count, base_interval_ms, max_backoff_ms); - state.retry_after_ms = now_ms + delay; - debug!( - peer = %peer_name, - retry = state.retry_count, - delay_secs = delay / 1000, - "Scheduling auto-reconnect after link-dead removal (backoff preserved)" - ); - return; - } - - let mut state = RetryState::new(pc); - state.reconnect = true; - let delay = backoff_ms(state.retry_count, base_interval_ms, max_backoff_ms); - state.retry_after_ms = now_ms + delay; - - debug!( - peer = %peer_name, - delay_secs = delay / 1000, - "Scheduling auto-reconnect after link-dead removal" - ); - - self.peering - .reconciler - .retry_pending - .insert(node_addr, state); - } - - /// Process pending retries whose time has arrived. - /// - /// For each due retry, initiates a fresh connection attempt. The retry - /// entry stays in `retry_pending` until the connection succeeds (cleared - /// in `promote_connection`) or max retries are exhausted (cleared in - /// `schedule_retry`). - pub(in crate::node) async fn process_pending_retries(&mut self, now_ms: u64) { - if self.peering.reconciler.retry_pending.is_empty() { - return; - } - - let expired: Vec = self - .peering - .reconciler - .retry_pending - .iter() - .filter_map(|(addr, state)| { - state - .expires_at_ms - .filter(|expires_at_ms| now_ms >= *expires_at_ms) - .map(|_| *addr) - }) - .collect(); - for node_addr in expired { - self.peering.reconciler.retry_pending.remove(&node_addr); - info!( - peer = %self.peer_display_name(&node_addr), - "Retry window expired, dropping pending retry state" - ); - } - if self.peering.reconciler.retry_pending.is_empty() { - return; - } - - if !self.outbound_admission_check() { - debug!( - peers = self.peers.len(), - max_peers = self.max_peers(), - retry_pending = self.peering.reconciler.retry_pending.len(), - "Suppressing auto-reconnect retries: at capacity" - ); - return; - } - - // Collect retries that are due - let due: Vec = self - .peering - .reconciler - .retry_pending - .iter() - .filter(|(_, state)| now_ms >= state.retry_after_ms) - .map(|(addr, _)| *addr) - .collect(); - let deferred = due.len().saturating_sub(MAX_RETRY_CONNECTIONS_PER_TICK); - if deferred > 0 { - debug!( - due = due.len(), - processing = MAX_RETRY_CONNECTIONS_PER_TICK, - deferred, - "Retry processing budget exhausted; deferring remaining peers" - ); - } - - for node_addr in due.into_iter().take(MAX_RETRY_CONNECTIONS_PER_TICK) { - // Peer may have connected inbound while we waited - if self.peers.contains_key(&node_addr) { - self.peering.reconciler.retry_pending.remove(&node_addr); - continue; - } - - let state = match self.peering.reconciler.retry_pending.get(&node_addr) { - Some(s) => s, - None => continue, - }; - - debug!( - peer = %self.peer_display_name(&node_addr), - retry = state.retry_count, - "Attempting connection retry" - ); - - let peer_config = state.peer_config.clone(); - - // Refresh the peer's overlay advert before retrying. The cache is - // read-only on hit (see fetch_advert), so every retry without a - // refetch dials the same cached endpoint — and the most common - // reason a peer ended up in retry_pending is that the cached - // endpoint just stopped working (NAT rebind, port change, peer - // restart on a different port). Without this refresh the retry - // loop dials the same dead address forever. - // - // refetch_advert_for_stale_check uses the relay's advert as - // ground truth: replaces the cache if there's a newer one, - // evicts if the relay has nothing, otherwise leaves it. Cheap - // (one Filter fetch with 2s timeout) and bounded by the retry - // backoff cadence. - if let Some(bootstrap) = self.supervisor.nostr_rendezvous.engine_arc() { - let _ = bootstrap - .refetch_advert_for_stale_check(&peer_config.npub) - .await; - } - - match self.initiate_peer_connection(&peer_config).await { - Ok(()) => { - // Push retry_after_ms past the handshake timeout window so - // we don't re-fire on the next tick. If the handshake - // succeeds, promote_connection() clears retry_pending. If - // it times out, check_timeouts() calls schedule_retry() - // which bumps the counter and applies proper backoff. - let hs_timeout_ms = self.config().node.rate_limit.handshake_timeout_secs * 1000; - if let Some(state) = self.peering.reconciler.retry_pending.get_mut(&node_addr) { - state.retry_after_ms = now_ms + hs_timeout_ms; - } - debug!( - peer = %self.peer_display_name(&node_addr), - "Retry connection initiated, suppressing re-fire for {}s", - self.config().node.rate_limit.handshake_timeout_secs, - ); - } - Err(e) => { - warn!( - peer = %self.peer_display_name(&node_addr), - error = %e, - "Retry connection initiation failed" - ); - // No-transport failures usually mean the cached overlay - // advert is stale (peer rebound NAT, switched relay, etc.). - // The advert cache is read-only inside fetch_advert, so - // every retry returns the same dead address until the - // entry expires. Force a re-fetch so the next retry tick - // picks up fresh endpoints. - if matches!(e, NodeError::NoTransportForType(_)) - && let Some(bootstrap) = self.supervisor.nostr_rendezvous.engine_arc() - { - let npub = peer_config.npub.clone(); - tokio::spawn(async move { - let _ = bootstrap.refetch_advert_for_stale_check(&npub).await; - }); - } - // Immediate failure counts as an attempt — schedule next retry - // (reconnect flag is preserved on existing retry_pending entry) - self.schedule_retry(node_addr, now_ms); - } - } - } - } -} diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 865dde5..69d9e61 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -772,7 +772,7 @@ fn test_schedule_retry_creates_entry() { assert!(node.peering.reconciler.retry_pending.is_empty()); - node.schedule_retry(peer_node_addr, 1000); + node.note_handshake_timeout(peer_node_addr, 1000); assert_eq!(node.peering.reconciler.retry_pending.len(), 1); let state = node @@ -808,7 +808,7 @@ fn test_schedule_retry_increments() { let mut node = Node::new(config).unwrap(); // First failure - node.schedule_retry(peer_node_addr, 1000); + node.note_handshake_timeout(peer_node_addr, 1000); assert_eq!( node.peering .reconciler @@ -820,7 +820,7 @@ fn test_schedule_retry_increments() { ); // Second failure - node.schedule_retry(peer_node_addr, 11_000); + node.note_handshake_timeout(peer_node_addr, 11_000); let state = node .peering .reconciler @@ -857,6 +857,10 @@ async fn test_process_pending_retries_is_budgeted_per_tick() { addrs.push(node_addr); } + // The retry-dial tick runs only under a `Reconciling` gate (it fires from the + // rx loop, which spins only after start() reaches Running). Put the node in + // Running so the retry-dial budget — the property under test — is exercised. + node.supervisor.state = crate::node::NodeState::Running; node.process_pending_retries(1).await; let processed = addrs @@ -894,7 +898,7 @@ fn test_schedule_retry_auto_connect_never_exhausts() { let mut node = Node::new(config).unwrap(); // All attempts should keep the entry alive despite max_retries=2 - node.schedule_retry(peer_node_addr, 1000); + node.note_handshake_timeout(peer_node_addr, 1000); assert!( node.peering .reconciler @@ -902,7 +906,7 @@ fn test_schedule_retry_auto_connect_never_exhausts() { .contains_key(&peer_node_addr) ); - node.schedule_retry(peer_node_addr, 2000); + node.note_handshake_timeout(peer_node_addr, 2000); assert!( node.peering .reconciler @@ -911,7 +915,7 @@ fn test_schedule_retry_auto_connect_never_exhausts() { ); // Attempt 3 would have exhausted before, but now retries indefinitely - node.schedule_retry(peer_node_addr, 3000); + node.note_handshake_timeout(peer_node_addr, 3000); assert!( node.peering .reconciler @@ -947,7 +951,7 @@ fn test_schedule_retry_disabled() { let mut node = Node::new(config).unwrap(); - node.schedule_retry(peer_node_addr, 1000); + node.note_handshake_timeout(peer_node_addr, 1000); assert!( node.peering.reconciler.retry_pending.is_empty(), "No retry should be scheduled when max_retries=0" @@ -963,7 +967,7 @@ fn test_schedule_retry_ignores_non_autoconnect() { // No peers configured at all let mut node = make_node(); - node.schedule_retry(peer_node_addr, 1000); + node.note_handshake_timeout(peer_node_addr, 1000); assert!( node.peering.reconciler.retry_pending.is_empty(), "No retry for unconfigured peer" @@ -985,7 +989,7 @@ fn test_schedule_retry_skips_connected_peer() { assert_eq!(node.peer_count(), 1); // Scheduling a retry for an already-connected peer should be a no-op - node.schedule_retry(node_addr, 3000); + node.note_handshake_timeout(node_addr, 3000); assert!( node.peering.reconciler.retry_pending.is_empty(), "No retry for already-connected peer" @@ -1304,6 +1308,9 @@ async fn test_process_pending_retries_drops_expired_entries() { .retry_pending .insert(peer_node_addr, state); + // Retry-dial runs only under a `Reconciling` gate; put the node in Running so + // the expired-entry drop (the property under test) is reached. + node.supervisor.state = crate::node::NodeState::Running; node.process_pending_retries(1_000).await; assert!( @@ -1339,8 +1346,8 @@ fn test_schedule_reconnect_preserves_backoff() { let mut node = Node::new(config).unwrap(); // Simulate two stale handshake timeouts incrementing the retry count. - node.schedule_retry(peer_node_addr, 1_000); // count=1, delay=10s - node.schedule_retry(peer_node_addr, 11_000); // count=2, delay=20s + node.note_handshake_timeout(peer_node_addr, 1_000); // count=1, delay=10s + node.note_handshake_timeout(peer_node_addr, 11_000); // count=2, delay=20s { let state = node .peering @@ -1354,7 +1361,7 @@ fn test_schedule_reconnect_preserves_backoff() { // Now simulate a link-dead removal triggering schedule_reconnect. // The existing retry entry (count=2) should be preserved and bumped to 3, // NOT reset to 0 as it was before the fix. - node.schedule_reconnect(peer_node_addr, 31_000); + node.note_link_dead(peer_node_addr, 31_000); let state = node .peering @@ -1396,7 +1403,7 @@ fn test_schedule_reconnect_fresh_state() { let mut node = Node::new(config).unwrap(); // No prior retry entry — first reconnect should use base delay. - node.schedule_reconnect(peer_node_addr, 1_000); + node.note_link_dead(peer_node_addr, 1_000); let state = node .peering @@ -1801,6 +1808,9 @@ async fn process_pending_retries_gated_at_capacity() { let before_peers = node.peer_count(); let before_connections = node.connection_count(); + // Running gate so the admission short-circuit (not the startup gate) is what + // suppresses the dial — the fingerprint this test asserts on. + node.supervisor.state = crate::node::NodeState::Running; node.process_pending_retries(1_000).await; // At capacity: gate short-circuits before due-list collection. The