From 324535e76d2d35bdf2048b1530328244bbded960 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 15 Mar 2026 18:43:11 +0000 Subject: [PATCH] Make auto-connect peers retry indefinitely on initial connection failure Previously, static peers configured with AutoConnect gave up after 6 attempts (1 initial + 5 retries). If the remote peer was offline at startup, the node permanently abandoned the connection. The reconnect path (after MMP link-dead) already retried indefinitely but only activated after a peer had been previously connected. Remove the reconnect parameter from schedule_retry() and always set reconnect=true when creating retry entries, since only auto-connect peers reach this code path. The 300s backoff cap prevents resource waste. The max_retries=0 config still works as an explicit kill switch. --- src/node/handlers/timeout.rs | 2 +- src/node/lifecycle.rs | 2 +- src/node/retry.rs | 8 +++----- src/node/tests/unit.rs | 32 +++++++++++++++++--------------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index a724425..89f3904 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -51,7 +51,7 @@ impl Node { if conn.is_outbound() && let Some(identity) = conn.expected_identity() { - self.schedule_retry(*identity.node_addr(), now_ms, false); + self.schedule_retry(*identity.node_addr(), now_ms); } } self.cleanup_stale_connection(link_id, now_ms); diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index 0502793..e92c8dd 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -475,7 +475,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - self.schedule_retry(*pending.peer_identity.node_addr(), now_ms, false); + self.schedule_retry(*pending.peer_identity.node_addr(), now_ms); } } } diff --git a/src/node/retry.rs b/src/node/retry.rs index 919a1d4..c5354da 100644 --- a/src/node/retry.rs +++ b/src/node/retry.rs @@ -59,11 +59,10 @@ impl Node { &mut self, node_addr: NodeAddr, now_ms: u64, - reconnect: bool, ) { let retry_cfg = &self.config.node.retry; let max_retries = retry_cfg.max_retries; - if max_retries == 0 && !reconnect { + if max_retries == 0 { return; } @@ -112,12 +111,11 @@ impl Node { if let Some(pc) = peer_config { let mut state = RetryState::new(pc); state.retry_count = 1; - state.reconnect = reconnect; + state.reconnect = true; let delay = state.backoff_ms(base_interval_ms, max_backoff_ms); state.retry_after_ms = now_ms + delay; debug!( peer = %self.peer_display_name(&node_addr), - reconnect = reconnect, delay_secs = delay / 1000, "First connection attempt failed, scheduling retry" ); @@ -237,7 +235,7 @@ impl Node { ); // 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, false); + self.schedule_retry(node_addr, now_ms); } } } diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index af18ea3..ea8d5a4 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -568,11 +568,12 @@ fn test_schedule_retry_creates_entry() { assert!(node.retry_pending.is_empty()); - node.schedule_retry(peer_node_addr, 1000, false); + 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); + assert!(state.reconnect, "Auto-connect peers always get reconnect=true"); // 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); @@ -595,20 +596,20 @@ fn test_schedule_retry_increments() { let mut node = Node::new(config).unwrap(); // First failure - node.schedule_retry(peer_node_addr, 1000, false); + 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, false); + 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 that auto-connect peers retry indefinitely (never exhaust). #[test] -fn test_schedule_retry_max_retries_exhausted() { +fn test_schedule_retry_auto_connect_never_exhausts() { let peer_identity = Identity::generate(); let peer_npub = peer_identity.npub(); let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); @@ -623,19 +624,20 @@ fn test_schedule_retry_max_retries_exhausted() { let mut node = Node::new(config).unwrap(); - // Attempts 1 and 2 should schedule retries - node.schedule_retry(peer_node_addr, 1000, false); + // All attempts should keep the entry alive despite max_retries=2 + node.schedule_retry(peer_node_addr, 1000); assert!(node.retry_pending.contains_key(&peer_node_addr)); - node.schedule_retry(peer_node_addr, 2000, false); + 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, false); + // Attempt 3 would have exhausted before, but now retries indefinitely + node.schedule_retry(peer_node_addr, 3000); assert!( - !node.retry_pending.contains_key(&peer_node_addr), - "Should be removed after max retries exhausted" + node.retry_pending.contains_key(&peer_node_addr), + "Auto-connect peers should never exhaust retries" ); + assert_eq!(node.retry_pending.get(&peer_node_addr).unwrap().retry_count, 3); } /// Test that schedule_retry does nothing when max_retries is 0. @@ -655,7 +657,7 @@ fn test_schedule_retry_disabled() { let mut node = Node::new(config).unwrap(); - node.schedule_retry(peer_node_addr, 1000, false); + node.schedule_retry(peer_node_addr, 1000); assert!( node.retry_pending.is_empty(), "No retry should be scheduled when max_retries=0" @@ -671,7 +673,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, false); + node.schedule_retry(peer_node_addr, 1000); assert!( node.retry_pending.is_empty(), "No retry for unconfigured peer" @@ -693,7 +695,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, false); + node.schedule_retry(node_addr, 3000); assert!( node.retry_pending.is_empty(), "No retry for already-connected peer"