diff --git a/src/config.rs b/src/config.rs index f8dfa0f..c171c39 100644 --- a/src/config.rs +++ b/src/config.rs @@ -54,8 +54,22 @@ pub struct IdentityConfig { pub nsec: Option, } +/// Default maximum connection retry attempts. +const DEFAULT_MAX_RETRIES: u32 = 5; + +/// Default base retry interval in seconds (backoff: 5s, 10s, 20s, 40s, 80s). +const DEFAULT_BASE_RETRY_INTERVAL_SECS: u64 = 5; + +fn default_max_retries() -> u32 { + DEFAULT_MAX_RETRIES +} + +fn default_base_retry_interval_secs() -> u64 { + DEFAULT_BASE_RETRY_INTERVAL_SECS +} + /// Node configuration (`node.*`). -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct NodeConfig { /// Identity configuration (`node.identity.*`). #[serde(default)] @@ -64,6 +78,27 @@ pub struct NodeConfig { /// Leaf-only mode (`node.leaf_only`). #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub leaf_only: bool, + + /// Maximum connection retry attempts for auto-connect peers. + /// 0 disables retries. Default: 5. + #[serde(default = "default_max_retries")] + pub max_retries: u32, + + /// Base retry interval in seconds for exponential backoff. + /// Actual delay is base * 2^attempt. Default: 5. + #[serde(default = "default_base_retry_interval_secs")] + pub base_retry_interval_secs: u64, +} + +impl Default for NodeConfig { + fn default() -> Self { + Self { + identity: IdentityConfig::default(), + leaf_only: false, + max_retries: DEFAULT_MAX_RETRIES, + base_retry_interval_secs: DEFAULT_BASE_RETRY_INTERVAL_SECS, + } + } } /// Default TUN device name. @@ -324,7 +359,7 @@ impl PeerAddress { /// /// Peers are identified by their Nostr public key (npub) and can have /// multiple transport addresses for reaching them. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct PeerConfig { /// The peer's Nostr public key in npub (bech32) or hex format. diff --git a/src/node/handlers.rs b/src/node/handlers.rs index 79cdb8e..a4d592b 100644 --- a/src/node/handlers.rs +++ b/src/node/handlers.rs @@ -38,6 +38,11 @@ impl Node { } _ = tick.tick() => { self.check_timeouts(); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + self.process_pending_retries(now_ms).await; } } } @@ -563,6 +568,7 @@ impl Node { self.peers.insert(peer_node_addr, new_peer); self.peers_by_index .insert((transport_id, our_index.as_u32()), peer_node_addr); + self.retry_pending.remove(&peer_node_addr); info!( node_addr = %peer_node_addr, @@ -592,7 +598,32 @@ impl Node { }) } } else { - // No cross-connection, normal promotion + // No existing promoted peer, but check for pending outbound + // connection to the same peer. A completed handshake always wins + // over a pending one — just clean it up immediately rather than + // waiting for the 30s handshake timeout. + let pending_to_same_peer: Vec = self + .connections + .iter() + .filter(|(_, conn)| { + conn.expected_identity() + .map(|id| *id.node_addr() == peer_node_addr) + .unwrap_or(false) + }) + .map(|(lid, _)| *lid) + .collect(); + + for pending_link_id in pending_to_same_peer { + info!( + node_addr = %peer_node_addr, + pending_link_id = %pending_link_id, + promoted_link_id = %link_id, + "Cleaning up pending connection superseded by completed handshake" + ); + self.cleanup_stale_connection(pending_link_id, current_time_ms); + } + + // Normal promotion if self.max_peers > 0 && self.peers.len() >= self.max_peers { let _ = self.index_allocator.free(our_index); return Err(NodeError::MaxPeersExceeded { max: self.max_peers }); @@ -613,6 +644,7 @@ impl Node { self.peers.insert(peer_node_addr, new_peer); self.peers_by_index .insert((transport_id, our_index.as_u32()), peer_node_addr); + self.retry_pending.remove(&peer_node_addr); info!( node_addr = %peer_node_addr, @@ -745,24 +777,47 @@ impl Node { .collect(); for link_id in stale { + // Log and schedule retry before cleanup (need connection state) + if let Some(conn) = self.connections.get(&link_id) { + let direction = conn.direction(); + let idle_ms = conn.idle_time(now_ms); + if conn.is_failed() { + info!( + link_id = %link_id, + direction = %direction, + "Failed handshake connection cleaned up" + ); + } else { + info!( + link_id = %link_id, + direction = %direction, + idle_secs = idle_ms / 1000, + "Stale handshake connection timed out" + ); + } + + // Schedule retry for failed outbound auto-connect peers + if conn.is_outbound() { + if let Some(identity) = conn.expected_identity() { + self.schedule_retry(*identity.node_addr(), now_ms); + } + } + } self.cleanup_stale_connection(link_id, now_ms); } } - /// Remove a stale or failed handshake connection and all associated state. + /// Remove a handshake connection and all associated state. /// /// Frees the session index, removes pending_outbound entry, and cleans up - /// the link and address mapping. - fn cleanup_stale_connection(&mut self, link_id: LinkId, now_ms: u64) { + /// the link and address mapping. Does not log — callers provide context-appropriate + /// log messages. + fn cleanup_stale_connection(&mut self, link_id: LinkId, _now_ms: u64) { let conn = match self.connections.remove(&link_id) { Some(c) => c, None => return, }; - let direction = conn.direction(); - let idle_ms = conn.idle_time(now_ms); - let is_failed = conn.is_failed(); - // Free session index and pending_outbound if allocated if let Some(idx) = conn.our_index() { if let Some(tid) = conn.transport_id() { @@ -773,20 +828,5 @@ impl Node { // Remove link and addr_to_link self.remove_link(&link_id); - - if is_failed { - info!( - link_id = %link_id, - direction = %direction, - "Failed handshake connection cleaned up" - ); - } else { - info!( - link_id = %link_id, - direction = %direction, - idle_secs = idle_ms / 1000, - "Stale handshake connection timed out" - ); - } } } diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index e541e48..f9a79a2 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -34,7 +34,7 @@ impl Node { /// Initiate a connection to a single peer. /// /// Creates a link, starts the Noise handshake, and sends the first message. - async fn initiate_peer_connection(&mut self, peer_config: &crate::config::PeerConfig) -> Result<(), NodeError> { + pub(super) async fn initiate_peer_connection(&mut self, peer_config: &crate::config::PeerConfig) -> Result<(), NodeError> { // Parse the peer's npub to get their identity let peer_identity = PeerIdentity::from_npub(&peer_config.npub).map_err(|e| { NodeError::InvalidPeerNpub { diff --git a/src/node/mod.rs b/src/node/mod.rs index 2c42550..c3ce9cf 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -6,6 +6,7 @@ mod handlers; mod lifecycle; +mod retry; #[cfg(test)] mod tests; @@ -253,6 +254,13 @@ pub struct Node { // === Rate Limiting === /// Rate limiter for msg1 processing (DoS protection). msg1_rate_limiter: HandshakeRateLimiter, + + // === Connection Retry === + /// Retry state for peers whose outbound connections have failed. + /// Keyed by NodeAddr. Entries are created when a handshake times out + /// or fails, and removed on successful promotion or when max retries + /// are exhausted. + retry_pending: HashMap, } impl Node { @@ -309,6 +317,7 @@ impl Node { peers_by_index: HashMap::new(), pending_outbound: HashMap::new(), msg1_rate_limiter: HandshakeRateLimiter::new(), + retry_pending: HashMap::new(), }) } @@ -356,6 +365,7 @@ impl Node { peers_by_index: HashMap::new(), pending_outbound: HashMap::new(), msg1_rate_limiter: HandshakeRateLimiter::new(), + retry_pending: HashMap::new(), } } diff --git a/src/node/retry.rs b/src/node/retry.rs new file mode 100644 index 0000000..2936911 --- /dev/null +++ b/src/node/retry.rs @@ -0,0 +1,236 @@ +//! Connection retry logic 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. + +use super::Node; +use crate::config::PeerConfig; +use crate::identity::NodeAddr; +use crate::PeerIdentity; +use tracing::{debug, info, warn}; + +/// Maximum backoff cap in milliseconds (5 minutes). +const MAX_BACKOFF_MS: u64 = 300_000; + +/// Tracks retry state for a peer across connection attempts. +pub struct RetryState { + /// The peer config to use for initiating retries. + pub peer_config: PeerConfig, + + /// Number of retries attempted so far. + pub retry_count: u32, + + /// Timestamp (Unix ms) when the next retry should be attempted. + pub retry_after_ms: u64, +} + +impl RetryState { + /// Create a new retry state for a peer. + pub fn new(peer_config: PeerConfig) -> Self { + Self { + peer_config, + retry_count: 0, + retry_after_ms: 0, + } + } + + /// Calculate the backoff delay in milliseconds for the current retry count. + /// + /// Uses exponential backoff: `base_interval_ms * 2^retry_count`, + /// capped at `MAX_BACKOFF_MS`. + pub fn backoff_ms(&self, base_interval_ms: u64) -> u64 { + let multiplier = 1u64.checked_shl(self.retry_count).unwrap_or(u64::MAX); + base_interval_ms.saturating_mul(multiplier).min(MAX_BACKOFF_MS) + } +} + +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. Does nothing if the peer is already connected + /// or has a connection in progress. + pub(super) fn schedule_retry(&mut self, node_addr: NodeAddr, now_ms: u64) { + let max_retries = self.config.node.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 = self.config.node.base_retry_interval_secs * 1000; + + if let Some(state) = self.retry_pending.get_mut(&node_addr) { + // Already tracking — increment + state.retry_count += 1; + if state.retry_count > max_retries { + info!( + node_addr = %node_addr, + attempts = state.retry_count, + "Max retries exhausted, giving up on peer" + ); + self.retry_pending.remove(&node_addr); + return; + } + let delay = state.backoff_ms(base_interval_ms); + state.retry_after_ms = now_ms + delay; + info!( + node_addr = %node_addr, + retry = state.retry_count, + 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; + let delay = state.backoff_ms(base_interval_ms); + state.retry_after_ms = now_ms + delay; + info!( + node_addr = %node_addr, + delay_secs = delay / 1000, + "First connection attempt failed, scheduling retry" + ); + self.retry_pending.insert(node_addr, state); + } + // If not found in auto_connect_peers, no retry (one-shot connection) + } + } + + /// 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(super) async fn process_pending_retries(&mut self, now_ms: u64) { + if self.retry_pending.is_empty() { + return; + } + + // Collect retries that are due + let due: Vec = self + .retry_pending + .iter() + .filter(|(_, state)| now_ms >= state.retry_after_ms) + .map(|(addr, _)| *addr) + .collect(); + + for node_addr in due { + // Peer may have connected inbound while we waited + if self.peers.contains_key(&node_addr) { + self.retry_pending.remove(&node_addr); + continue; + } + + let state = match self.retry_pending.get(&node_addr) { + Some(s) => s, + None => continue, + }; + + info!( + node_addr = %node_addr, + retry = state.retry_count, + "Attempting connection retry" + ); + + let peer_config = state.peer_config.clone(); + + match self.initiate_peer_connection(&peer_config).await { + Ok(()) => { + debug!( + node_addr = %node_addr, + "Retry connection initiated" + ); + // Don't remove from retry_pending — wait for promotion + // (success) or next timeout (failure triggers schedule_retry again) + } + Err(e) => { + warn!( + node_addr = %node_addr, + error = %e, + "Retry connection initiation failed" + ); + // Immediate failure counts as an attempt — schedule next retry + self.schedule_retry(node_addr, now_ms); + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::PeerConfig; + + #[test] + fn test_backoff_exponential() { + let state = RetryState { + peer_config: PeerConfig::default(), + retry_count: 0, + retry_after_ms: 0, + }; + // base = 5000ms + assert_eq!(state.backoff_ms(5000), 5000); // 5s * 2^0 + + let state = RetryState { + retry_count: 1, + ..state + }; + assert_eq!(state.backoff_ms(5000), 10_000); // 5s * 2^1 + + let state = RetryState { + retry_count: 2, + ..state + }; + assert_eq!(state.backoff_ms(5000), 20_000); // 5s * 2^2 + + let state = RetryState { + retry_count: 3, + ..state + }; + assert_eq!(state.backoff_ms(5000), 40_000); // 5s * 2^3 + + let state = RetryState { + retry_count: 4, + ..state + }; + assert_eq!(state.backoff_ms(5000), 80_000); // 5s * 2^4 + } + + #[test] + fn test_backoff_cap() { + let state = RetryState { + peer_config: PeerConfig::default(), + retry_count: 20, // 2^20 * 5000 would be huge + retry_after_ms: 0, + }; + assert_eq!(state.backoff_ms(5000), MAX_BACKOFF_MS); + } + + #[test] + fn test_backoff_zero_base() { + let state = RetryState { + peer_config: PeerConfig::default(), + retry_count: 3, + retry_after_ms: 0, + }; + assert_eq!(state.backoff_ms(0), 0); + } +} diff --git a/src/node/tests.rs b/src/node/tests.rs index b68da44..0253acc 100644 --- a/src/node/tests.rs +++ b/src/node/tests.rs @@ -1162,3 +1162,307 @@ async fn test_failed_connection_cleanup() { assert_eq!(node.link_count(), 0, "Failed link should be removed"); assert_eq!(node.index_allocator.count(), 0, "Session index should be freed"); } + +/// Test that promoting a connection cleans up a pending outbound to the same peer. +/// +/// Simulates the scenario where node A has a pending outbound handshake to B +/// (unanswered because B wasn't running), then B starts and initiates to A. +/// When A promotes B's inbound connection, it should immediately clean up the +/// stale pending outbound rather than waiting for the 30s timeout. +#[test] +fn test_promote_cleans_up_pending_outbound_to_same_peer() { + let mut node = make_node(); + let transport_id = TransportId::new(1); + + // Generate peer B's identity (shared between the two connections) + let peer_b_full = Identity::generate(); + let peer_b_identity = PeerIdentity::from_pubkey_full(peer_b_full.pubkey_full()); + let peer_b_node_addr = *peer_b_identity.node_addr(); + + // --- Set up the pending outbound to B (link_id 1) --- + // This simulates A having sent msg1 to B before B was running. + let pending_link_id = LinkId::new(1); + let pending_time_ms = 1000; + let mut pending_conn = + PeerConnection::outbound(pending_link_id, peer_b_identity.clone(), pending_time_ms); + + let our_keypair = node.identity.keypair(); + let _msg1 = pending_conn.start_handshake(our_keypair, pending_time_ms).unwrap(); + + let pending_index = node.index_allocator.allocate().unwrap(); + pending_conn.set_our_index(pending_index); + pending_conn.set_transport_id(transport_id); + let pending_addr = TransportAddr::from_string("10.0.0.2:4000"); + pending_conn.set_source_addr(pending_addr.clone()); + + let pending_link = Link::connectionless( + pending_link_id, + transport_id, + pending_addr.clone(), + LinkDirection::Outbound, + Duration::from_millis(100), + ); + node.links.insert(pending_link_id, pending_link); + node.addr_to_link + .insert((transport_id, pending_addr.clone()), pending_link_id); + node.connections.insert(pending_link_id, pending_conn); + node.pending_outbound + .insert((transport_id, pending_index.as_u32()), pending_link_id); + + // Verify pending state + assert_eq!(node.connection_count(), 1); + assert_eq!(node.link_count(), 1); + assert_eq!(node.index_allocator.count(), 1); + + // --- Set up the completing inbound from B (link_id 2) --- + // Simulate B's outbound arriving at A and completing the handshake. + // We use make_completed_connection's pattern but with B's known identity. + let completing_link_id = LinkId::new(2); + let completing_time_ms = 2000; + + let mut completing_conn = PeerConnection::outbound( + completing_link_id, + peer_b_identity.clone(), + completing_time_ms, + ); + + let our_keypair = node.identity.keypair(); + let msg1 = completing_conn + .start_handshake(our_keypair, completing_time_ms) + .unwrap(); + + // B responds + let mut resp_conn = PeerConnection::inbound(LinkId::new(999), completing_time_ms); + let peer_keypair = peer_b_full.keypair(); + let msg2 = resp_conn + .receive_handshake_init(peer_keypair, &msg1, completing_time_ms) + .unwrap(); + + completing_conn + .complete_handshake(&msg2, completing_time_ms) + .unwrap(); + + let completing_index = node.index_allocator.allocate().unwrap(); + completing_conn.set_our_index(completing_index); + completing_conn.set_their_index(SessionIndex::new(99)); + completing_conn.set_transport_id(transport_id); + completing_conn.set_source_addr(TransportAddr::from_string("10.0.0.2:4001")); + + node.add_connection(completing_conn).unwrap(); + + // Now 2 connections, 1 link (pending has link, completing doesn't yet need one for this test) + assert_eq!(node.connection_count(), 2); + assert_eq!(node.index_allocator.count(), 2); + + // --- Promote the completing connection --- + let result = node + .promote_connection(completing_link_id, peer_b_identity.clone(), completing_time_ms) + .unwrap(); + + assert!(matches!(result, PromotionResult::Promoted(_))); + + // The pending outbound should have been cleaned up + assert_eq!( + node.connection_count(), + 0, + "Pending outbound should be cleaned up during promotion" + ); + assert_eq!(node.peer_count(), 1, "Promoted peer should exist"); + assert!( + !node + .pending_outbound + .contains_key(&(transport_id, pending_index.as_u32())), + "pending_outbound entry for stale connection should be freed" + ); + assert_eq!( + node.index_allocator.count(), + 1, + "Only the promoted peer's index should remain" + ); + assert!( + node.addr_to_link + .get(&(transport_id, pending_addr)) + .is_none(), + "addr_to_link for stale connection should be cleaned up" + ); + + // Verify the promoted peer is correct + let peer = node.get_peer(&peer_b_node_addr).unwrap(); + assert_eq!(peer.link_id(), completing_link_id); +} + +/// Test that schedule_retry creates a retry entry for auto-connect peers. +#[test] +fn test_schedule_retry_creates_entry() { + let peer_identity = Identity::generate(); + let peer_npub = peer_identity.npub(); + let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); + + let mut config = Config::new(); + config.peers.push(crate::config::PeerConfig::new( + peer_npub, + "udp", + "10.0.0.2:4000", + )); + + let mut node = Node::new(config).unwrap(); + + assert!(node.retry_pending.is_empty()); + + node.schedule_retry(peer_node_addr, 1000); + + assert_eq!(node.retry_pending.len(), 1); + let state = node.retry_pending.get(&peer_node_addr).unwrap(); + assert_eq!(state.retry_count, 1); + // Default base = 5s, 2^1 = 10s, but first retry is 2^0... let me check: + // retry_count is set to 1, backoff_ms(5000) = 5000 * 2^1 = 10000 + assert_eq!(state.retry_after_ms, 1000 + 10_000); +} + +/// Test that schedule_retry increments on subsequent calls. +#[test] +fn test_schedule_retry_increments() { + let peer_identity = Identity::generate(); + let peer_npub = peer_identity.npub(); + let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); + + let mut config = Config::new(); + config.peers.push(crate::config::PeerConfig::new( + peer_npub, + "udp", + "10.0.0.2:4000", + )); + + let mut node = Node::new(config).unwrap(); + + // First failure + node.schedule_retry(peer_node_addr, 1000); + assert_eq!(node.retry_pending.get(&peer_node_addr).unwrap().retry_count, 1); + + // Second failure + node.schedule_retry(peer_node_addr, 11_000); + let state = node.retry_pending.get(&peer_node_addr).unwrap(); + assert_eq!(state.retry_count, 2); + // backoff_ms(5000) with retry_count=2 = 5000 * 4 = 20000 + assert_eq!(state.retry_after_ms, 11_000 + 20_000); +} + +/// Test that schedule_retry gives up after max_retries. +#[test] +fn test_schedule_retry_max_retries_exhausted() { + let peer_identity = Identity::generate(); + let peer_npub = peer_identity.npub(); + let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); + + let mut config = Config::new(); + config.node.max_retries = 2; + config.peers.push(crate::config::PeerConfig::new( + peer_npub, + "udp", + "10.0.0.2:4000", + )); + + let mut node = Node::new(config).unwrap(); + + // Attempts 1 and 2 should schedule retries + node.schedule_retry(peer_node_addr, 1000); + assert!(node.retry_pending.contains_key(&peer_node_addr)); + + node.schedule_retry(peer_node_addr, 2000); + assert!(node.retry_pending.contains_key(&peer_node_addr)); + + // Attempt 3 exceeds max_retries=2, should remove entry + node.schedule_retry(peer_node_addr, 3000); + assert!( + !node.retry_pending.contains_key(&peer_node_addr), + "Should be removed after max retries exhausted" + ); +} + +/// Test that schedule_retry does nothing when max_retries is 0. +#[test] +fn test_schedule_retry_disabled() { + let peer_identity = Identity::generate(); + let peer_npub = peer_identity.npub(); + let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); + + let mut config = Config::new(); + config.node.max_retries = 0; + config.peers.push(crate::config::PeerConfig::new( + peer_npub, + "udp", + "10.0.0.2:4000", + )); + + let mut node = Node::new(config).unwrap(); + + node.schedule_retry(peer_node_addr, 1000); + assert!( + node.retry_pending.is_empty(), + "No retry should be scheduled when max_retries=0" + ); +} + +/// Test that schedule_retry does nothing for non-auto-connect peers. +#[test] +fn test_schedule_retry_ignores_non_autoconnect() { + let peer_identity = Identity::generate(); + let peer_node_addr = *peer_identity.node_addr(); + + // No peers configured at all + let mut node = make_node(); + + node.schedule_retry(peer_node_addr, 1000); + assert!( + node.retry_pending.is_empty(), + "No retry for unconfigured peer" + ); +} + +/// Test that schedule_retry does nothing if peer is already connected. +#[test] +fn test_schedule_retry_skips_connected_peer() { + let mut node = make_node(); + let transport_id = TransportId::new(1); + + // Promote a peer so it's in the peers map + let link_id = LinkId::new(1); + let (conn, identity) = make_completed_connection(&mut node, link_id, transport_id, 1000); + let node_addr = *identity.node_addr(); + node.add_connection(conn).unwrap(); + node.promote_connection(link_id, identity, 2000).unwrap(); + 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); + assert!( + node.retry_pending.is_empty(), + "No retry for already-connected peer" + ); +} + +/// Test that promote_connection clears retry_pending. +#[test] +fn test_promote_clears_retry_pending() { + let mut node = make_node(); + let transport_id = TransportId::new(1); + + let link_id = LinkId::new(1); + let (conn, identity) = make_completed_connection(&mut node, link_id, transport_id, 1000); + let node_addr = *identity.node_addr(); + + // Simulate a retry entry existing for this peer + node.retry_pending.insert( + node_addr, + super::retry::RetryState::new(crate::config::PeerConfig::default()), + ); + assert_eq!(node.retry_pending.len(), 1); + + node.add_connection(conn).unwrap(); + node.promote_connection(link_id, identity, 2000).unwrap(); + + assert!( + !node.retry_pending.contains_key(&node_addr), + "retry_pending should be cleared on successful promotion" + ); +} diff --git a/src/peer/connection.rs b/src/peer/connection.rs index 35eeb05..f131aa9 100644 --- a/src/peer/connection.rs +++ b/src/peer/connection.rs @@ -96,9 +96,6 @@ pub struct PeerConnection { /// When the last handshake message was sent/received. last_activity: u64, - /// Number of retries attempted. - retry_count: u32, - // === Statistics === /// Link statistics during handshake. link_stats: LinkStats, @@ -140,7 +137,7 @@ impl PeerConnection { noise_session: None, started_at: current_time_ms, last_activity: current_time_ms, - retry_count: 0, + link_stats: LinkStats::new(), our_index: None, their_index: None, @@ -163,7 +160,7 @@ impl PeerConnection { noise_session: None, started_at: current_time_ms, last_activity: current_time_ms, - retry_count: 0, + link_stats: LinkStats::new(), our_index: None, their_index: None, @@ -190,7 +187,7 @@ impl PeerConnection { noise_session: None, started_at: current_time_ms, last_activity: current_time_ms, - retry_count: 0, + link_stats: LinkStats::new(), our_index: None, their_index: None, @@ -266,11 +263,6 @@ impl PeerConnection { current_time_ms.saturating_sub(self.last_activity) } - /// Number of retries. - pub fn retry_count(&self) -> u32 { - self.retry_count - } - /// Get link statistics. pub fn link_stats(&self) -> &LinkStats { &self.link_stats @@ -465,11 +457,6 @@ impl PeerConnection { self.noise_handshake = None; } - /// Increment retry counter. - pub fn increment_retry(&mut self) { - self.retry_count += 1; - } - /// Update last activity timestamp. pub fn touch(&mut self, current_time_ms: u64) { self.last_activity = current_time_ms; @@ -482,10 +469,6 @@ impl PeerConnection { self.idle_time(current_time_ms) > timeout_ms } - /// Check if max retries exceeded. - pub fn max_retries_exceeded(&self, max_retries: u32) -> bool { - self.retry_count >= max_retries - } } impl fmt::Debug for PeerConnection { @@ -502,7 +485,6 @@ impl fmt::Debug for PeerConnection { .field("transport_id", &self.transport_id) .field("started_at", &self.started_at) .field("last_activity", &self.last_activity) - .field("retry_count", &self.retry_count) .finish() } } @@ -544,7 +526,6 @@ mod tests { assert_eq!(conn.handshake_state(), HandshakeState::Initial); assert!(conn.expected_identity().is_some()); assert_eq!(conn.started_at(), 1000); - assert_eq!(conn.retry_count(), 0); } #[test] @@ -619,22 +600,6 @@ mod tests { assert!(conn.is_timed_out(2500, 1000)); } - #[test] - fn test_retry_tracking() { - let identity = make_peer_identity(); - let mut conn = PeerConnection::outbound(LinkId::new(1), identity, 1000); - - assert_eq!(conn.retry_count(), 0); - assert!(!conn.max_retries_exceeded(3)); - - conn.increment_retry(); - conn.increment_retry(); - conn.increment_retry(); - - assert_eq!(conn.retry_count(), 3); - assert!(conn.max_retries_exceeded(3)); - } - #[test] fn test_connection_failure() { let identity = make_peer_identity();