Fix auto-connect reconnect on graceful peer disconnect

handle_disconnect() called remove_active_peer without scheduling a
reconnect, orphaning auto-connect peers on a clean upstream shutdown.
Mirror the pattern from the other three peer-removal paths (link-dead,
decrypt failure, peer restart) which all schedule reconnect after
removal.

Adds test_disconnect_schedules_reconnect regression test that verifies
handle_disconnect populates retry_pending for an auto-connect peer.
Visibility of handle_disconnect bumped to pub(in crate::node) for
direct unit-test access.

Fixes #60.
This commit is contained in:
Johnathan Corgan
2026-04-13 05:34:51 +00:00
parent 0b81f15369
commit 42834b8008
2 changed files with 49 additions and 2 deletions

View File

@@ -67,8 +67,12 @@ impl Node {
/// Handle a Disconnect notification from a peer.
///
/// The peer is signaling an orderly departure. We immediately remove
/// them from all state rather than waiting for timeout detection.
fn handle_disconnect(&mut self, from: &NodeAddr, payload: &[u8]) {
/// them from all state rather than waiting for timeout detection, and
/// schedule a reconnect if the peer is configured as auto-connect.
/// Without this, a graceful upstream shutdown orphans auto-connect
/// entries — other removal paths (link-dead, decrypt failure, peer
/// restart) all schedule reconnect.
pub(in crate::node) fn handle_disconnect(&mut self, from: &NodeAddr, payload: &[u8]) {
let disconnect = match crate::protocol::Disconnect::decode(payload) {
Ok(msg) => msg,
Err(e) => {
@@ -83,7 +87,13 @@ impl Node {
"Peer sent disconnect notification"
);
let addr = *from;
self.remove_active_peer(from);
let now_ms = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_millis() as u64)
.unwrap_or(0);
self.schedule_reconnect(addr, now_ms);
}
/// Remove an active peer and clean up all associated state.

View File

@@ -801,6 +801,43 @@ fn test_schedule_reconnect_fresh_state() {
assert_eq!(state.retry_after_ms, 1_000 + expected_delay);
}
/// Test that a graceful Disconnect from an auto-connect peer schedules reconnect.
///
/// Regression test for issue #60: `handle_disconnect` previously called
/// `remove_active_peer` without `schedule_reconnect`, orphaning auto-connect
/// entries on a clean upstream shutdown. Other peer-removal paths (link-dead,
/// decrypt failure, peer restart) all schedule reconnect.
#[test]
fn test_disconnect_schedules_reconnect() {
use crate::protocol::{Disconnect, DisconnectReason};
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:2121",
));
let mut node = Node::new(config).unwrap();
let payload = Disconnect::new(DisconnectReason::Shutdown).encode();
node.handle_disconnect(&peer_node_addr, &payload);
let state = node
.retry_pending
.get(&peer_node_addr)
.expect("handle_disconnect should schedule reconnect for auto-connect peer");
assert!(state.reconnect, "Entry should be marked as reconnect");
assert_eq!(
state.retry_count, 0,
"Fresh reconnect after disconnect should start at count=0"
);
}
/// Test that promote_connection clears retry_pending.
#[test]
fn test_promote_clears_retry_pending() {