From 7e7fad5734a9dff2779c9fd8335bdd3cebeb8965 Mon Sep 17 00:00:00 2001 From: blacktemplar Date: Tue, 3 Nov 2020 23:43:10 +0000 Subject: [PATCH] Ignore RPC messages of disconnected peers and remove old peers based on disconnection time (#1854) ## Issue Addressed NA ## Proposed Changes Lets the networking behavior ignore messages of peers that are not connected. Furthermore, old peers are not removed from the peerdb based on score anymore but based on the disconnection time. --- beacon_node/eth2_libp2p/src/behaviour/mod.rs | 11 +++++++++ .../eth2_libp2p/src/peer_manager/mod.rs | 4 ++++ .../eth2_libp2p/src/peer_manager/peerdb.rs | 24 +++++++++---------- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/beacon_node/eth2_libp2p/src/behaviour/mod.rs b/beacon_node/eth2_libp2p/src/behaviour/mod.rs index 46a53c7ca..f39fcb768 100644 --- a/beacon_node/eth2_libp2p/src/behaviour/mod.rs +++ b/beacon_node/eth2_libp2p/src/behaviour/mod.rs @@ -589,6 +589,17 @@ impl Behaviour { fn on_rpc_event(&mut self, message: RPCMessage) { let peer_id = message.peer_id; + + if !self.peer_manager.is_connected(&peer_id) { + //ignore this event + debug!( + self.log, + "Ignoring rpc message of disconnected peer"; + "peer" => peer_id.to_string() + ); + return; + } + let handler_id = message.conn_id; // The METADATA and PING RPC responses are handled within the behaviour and not propagated match message.event { diff --git a/beacon_node/eth2_libp2p/src/peer_manager/mod.rs b/beacon_node/eth2_libp2p/src/peer_manager/mod.rs index 182f7a204..5a8fb92d7 100644 --- a/beacon_node/eth2_libp2p/src/peer_manager/mod.rs +++ b/beacon_node/eth2_libp2p/src/peer_manager/mod.rs @@ -316,6 +316,10 @@ impl PeerManager { self.network_globals.peers.read().is_banned(peer_id) } + pub fn is_connected(&self, peer_id: &PeerId) -> bool { + self.network_globals.peers.read().is_connected(peer_id) + } + /// Reports whether the peer limit is reached in which case we stop allowing new incoming /// connections. pub fn peer_limit_reached(&self) -> bool { diff --git a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs index 7b805b0a1..97dd6a43e 100644 --- a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs +++ b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs @@ -537,16 +537,15 @@ impl PeerDB { pub fn shrink_to_fit(&mut self) { // Remove excess banned peers while self.banned_peers_count.banned_peers() > MAX_BANNED_PEERS { - if let Some(to_drop) = if let Some((id, info)) = self + if let Some(to_drop) = if let Some((id, info, _)) = self .peers .iter() - .filter(|(_, info)| info.is_banned()) - .min_by(|(_, info_a), (_, info_b)| { - info_a - .score() - .partial_cmp(&info_b.score()) - .unwrap_or(std::cmp::Ordering::Equal) - }) { + .filter_map(|(id, info)| match info.connection_status() { + PeerConnectionStatus::Banned { since } => Some((id, info, since)), + _ => None, + }) + .min_by_key(|(_, _, since)| *since) + { self.banned_peers_count .remove_banned_peer(info.seen_addresses()); Some(id.clone()) @@ -571,12 +570,11 @@ impl PeerDB { .peers .iter() .filter(|(_, info)| info.is_disconnected()) - .min_by(|(_, info_a), (_, info_b)| { - info_a - .score() - .partial_cmp(&info_b.score()) - .unwrap_or(std::cmp::Ordering::Equal) + .filter_map(|(id, info)| match info.connection_status() { + PeerConnectionStatus::Disconnected { since } => Some((id, since)), + _ => None, }) + .min_by_key(|(_, since)| *since) .map(|(id, _)| id.clone()) { debug!(self.log, "Removing old disconnected peer"; "peer_id" => to_drop.to_string());