diff --git a/dash-spv/src/network/manager.rs b/dash-spv/src/network/manager.rs index 29b464009..ee7153ef1 100644 --- a/dash-spv/src/network/manager.rs +++ b/dash-spv/src/network/manager.rs @@ -860,7 +860,7 @@ impl PeerNetworkManager { return; } - // Send ping to all peers if needed + // Send ping to all peers if needed and disconnect unresponsive ones for (addr, peer) in self.pool.get_all_peers().await { let mut peer_guard = peer.write().await; if peer_guard.should_ping() { @@ -872,7 +872,11 @@ impl PeerNetworkManager { .await; } } - peer_guard.cleanup_old_pings(); + let has_expired = peer_guard.remove_expired_pings(); + drop(peer_guard); + if has_expired { + let _ = self.disconnect_peer(&addr, "ping timeout").await; + } } // Only save known peers if not in exclusive mode diff --git a/dash-spv/src/network/peer.rs b/dash-spv/src/network/peer.rs index 9a65005ac..dc3e484ed 100644 --- a/dash-spv/src/network/peer.rs +++ b/dash-spv/src/network/peer.rs @@ -762,8 +762,9 @@ impl Peer { true } - /// Clean up old pending pings that haven't received responses. - pub fn cleanup_old_pings(&mut self) { + /// Remove pending pings that have timed out. + /// Returns `true` if any pings were removed. + pub fn remove_expired_pings(&mut self) -> bool { const PING_TIMEOUT: Duration = Duration::from_secs(60); // 1 minute timeout for pings let now = SystemTime::now(); @@ -775,10 +776,13 @@ impl Peer { } } + let has_expired = !expired_nonces.is_empty(); for nonce in expired_nonces { self.pending_pings.remove(&nonce); tracing::warn!("Ping timeout for {} with nonce {}", self.address, nonce); } + + has_expired } /// Get ping/pong statistics. @@ -828,3 +832,39 @@ impl Peer { } } } + +#[cfg(test)] +mod tests { + use std::time::{Duration, SystemTime}; + + use super::Peer; + + #[test] + fn remove_expired_pings() { + let mut peer = Peer::dummy(); + let now = SystemTime::now(); + let expired = now - Duration::from_secs(61); + + // No pings at all + assert!(!peer.remove_expired_pings()); + + // Only recent pings — nothing removed + peer.pending_pings.insert(1, now); + peer.pending_pings.insert(2, now); + assert!(!peer.remove_expired_pings()); + assert_eq!(peer.pending_pings.len(), 2); + + // Add an expired ping — only it gets removed + peer.pending_pings.insert(3, expired); + assert!(peer.remove_expired_pings()); + assert_eq!(peer.pending_pings.len(), 2); + assert!(!peer.pending_pings.contains_key(&3)); + + // All expired — map ends up empty + peer.pending_pings.clear(); + peer.pending_pings.insert(10, expired); + peer.pending_pings.insert(20, expired); + assert!(peer.remove_expired_pings()); + assert!(peer.pending_pings.is_empty()); + } +} diff --git a/dash-spv/src/test_utils/network.rs b/dash-spv/src/test_utils/network.rs index 29075949f..00508ee0c 100644 --- a/dash-spv/src/test_utils/network.rs +++ b/dash-spv/src/test_utils/network.rs @@ -1,4 +1,5 @@ use crate::error::{NetworkError, NetworkResult}; +use crate::network::peer::Peer; use crate::network::{ Message, MessageDispatcher, MessageType, NetworkEvent, NetworkManager, NetworkRequest, RequestSender, @@ -8,11 +9,12 @@ use dashcore::network::constants::ServiceFlags; use dashcore::prelude::CoreBlockHeight; use dashcore::{ block::Header as BlockHeader, network::message::NetworkMessage, - network::message_blockdata::GetHeadersMessage, BlockHash, + network::message_blockdata::GetHeadersMessage, BlockHash, Network, }; use dashcore_hashes::Hash; use std::any::Any; use std::net::SocketAddr; +use std::time::Duration; use tokio::sync::broadcast; use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}; @@ -190,3 +192,10 @@ impl NetworkManager for MockNetworkManager { self.network_event_sender.subscribe() } } + +impl Peer { + pub fn dummy() -> Self { + let addr: SocketAddr = "127.0.0.1:9999".parse().unwrap(); + Peer::new(addr, Duration::from_secs(10), Network::Dash) + } +}