From 52a39032ae45e1e33388f0ff8334fd3a8f4121e0 Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Thu, 18 Jul 2024 12:12:56 +0530 Subject: [PATCH] chore(network): remove bad node checks at places as we will never connect to bad nodes --- sn_networking/src/cmd.rs | 1 - sn_networking/src/driver.rs | 4 +-- sn_networking/src/event/swarm.rs | 44 ++++++++++++------------------ sn_networking/src/relay_manager.rs | 25 +++-------------- 4 files changed, 23 insertions(+), 51 deletions(-) diff --git a/sn_networking/src/cmd.rs b/sn_networking/src/cmd.rs index 349342501b..ba2014827e 100644 --- a/sn_networking/src/cmd.rs +++ b/sn_networking/src/cmd.rs @@ -734,7 +734,6 @@ impl SwarmDriver { SwarmCmd::RecordNodeIssue { peer_id, issue } => { cmd_string = "RecordNodeIssues"; - let _ = self.bad_nodes_ongoing_verifications.remove(&peer_id); self.record_node_issue(peer_id, issue); } SwarmCmd::IsPeerShunned { target, sender } => { diff --git a/sn_networking/src/driver.rs b/sn_networking/src/driver.rs index 0d1ca693a5..50476fcf06 100644 --- a/sn_networking/src/driver.rs +++ b/sn_networking/src/driver.rs @@ -56,7 +56,7 @@ use sn_protocol::{ }; use sn_transfers::PaymentQuote; use std::{ - collections::{btree_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet}, + collections::{btree_map::Entry, BTreeMap, HashMap, HashSet}, fmt::Debug, net::SocketAddr, num::NonZeroUsize, @@ -615,7 +615,6 @@ impl NetworkBuilder { handled_times: 0, hard_disk_write_error: 0, bad_nodes: Default::default(), - bad_nodes_ongoing_verifications: Default::default(), quotes_history: Default::default(), replication_targets: Default::default(), }; @@ -665,7 +664,6 @@ pub struct SwarmDriver { handled_times: usize, pub(crate) hard_disk_write_error: usize, pub(crate) bad_nodes: BadNodes, - pub(crate) bad_nodes_ongoing_verifications: BTreeSet, pub(crate) quotes_history: BTreeMap, pub(crate) replication_targets: BTreeMap, } diff --git a/sn_networking/src/event/swarm.rs b/sn_networking/src/event/swarm.rs index b659d9fe64..3efbcbc692 100644 --- a/sn_networking/src/event/swarm.rs +++ b/sn_networking/src/event/swarm.rs @@ -187,7 +187,6 @@ impl SwarmDriver { &peer_id, &addrs, &info.protocols, - &self.bad_nodes, ); } @@ -254,32 +253,25 @@ impl SwarmDriver { // If we are not local, we care only for peers that we dialed and thus are reachable. if self.local || has_dialed { - // To reduce the bad_node check resource usage, - // during the connection establish process, only check cached black_list - // The periodical check, which involves network queries shall filter - // out bad_nodes eventually. - if let Some((_issues, true)) = self.bad_nodes.get(&peer_id) { - info!("Peer {peer_id:?} is considered as bad, blocking it."); - } else { - self.remove_bootstrap_from_full(peer_id); - - // Avoid have `direct link format` addrs co-exists with `relay` addr - if has_relayed { - addrs.retain(|multiaddr| { - multiaddr.iter().any(|p| matches!(p, Protocol::P2pCircuit)) - }); - } + // A bad node cannot establish a connection with us. So we can add it to the RT directly. + self.remove_bootstrap_from_full(peer_id); + + // Avoid have `direct link format` addrs co-exists with `relay` addr + if has_relayed { + addrs.retain(|multiaddr| { + multiaddr.iter().any(|p| matches!(p, Protocol::P2pCircuit)) + }); + } - trace!(%peer_id, ?addrs, "identify: attempting to add addresses to routing table"); + trace!(%peer_id, ?addrs, "identify: attempting to add addresses to routing table"); - // Attempt to add the addresses to the routing table. - for multiaddr in addrs { - let _routing_update = self - .swarm - .behaviour_mut() - .kademlia - .add_address(&peer_id, multiaddr); - } + // Attempt to add the addresses to the routing table. + for multiaddr in addrs { + let _routing_update = self + .swarm + .behaviour_mut() + .kademlia + .add_address(&peer_id, multiaddr); } } trace!( @@ -665,7 +657,7 @@ impl SwarmDriver { } // skip if the peer is a relay server that we're connected to - if self.relay_manager.keep_alive_peer(peer_id, &self.bad_nodes) { + if self.relay_manager.keep_alive_peer(peer_id) { return true; // retain peer } diff --git a/sn_networking/src/relay_manager.rs b/sn_networking/src/relay_manager.rs index 54c2b6db7f..3bdbd9be60 100644 --- a/sn_networking/src/relay_manager.rs +++ b/sn_networking/src/relay_manager.rs @@ -57,19 +57,10 @@ impl RelayManager { self.enable_client = enable; } - /// Should we keep this peer alive? - /// If a peer is considered as a bad node, closing it's connection would remove that server from the listen addr. - #[allow(clippy::nonminimal_bool)] - pub(crate) fn keep_alive_peer(&self, peer_id: &PeerId, bad_nodes: &BadNodes) -> bool { - let is_not_bad = if let Some((_, is_bad)) = bad_nodes.get(peer_id) { - !*is_bad - } else { - true - }; - - // we disconnect from bad server - (self.connected_relays.contains_key(peer_id) && is_not_bad) - || (self.waiting_for_reservation.contains_key(peer_id) && is_not_bad) + /// Should we keep this peer alive? Closing a connection to that peer would remove that server from the listen addr. + pub(crate) fn keep_alive_peer(&self, peer_id: &PeerId) -> bool { + self.connected_relays.contains_key(peer_id) + || self.waiting_for_reservation.contains_key(peer_id) // but servers provide connections to bad nodes. || self.reserved_by.contains(peer_id) } @@ -81,20 +72,12 @@ impl RelayManager { peer_id: &PeerId, addrs: &HashSet, stream_protocols: &Vec, - bad_nodes: &BadNodes, ) { if self.candidates.len() >= MAX_POTENTIAL_CANDIDATES { trace!("Got max relay candidates"); return; } - if let Some((_, is_bad)) = bad_nodes.get(peer_id) { - if *is_bad { - debug!("Not adding peer {peer_id:?} as relay candidate as it is a bad node."); - return; - } - } - if Self::does_it_support_relay_server_protocol(stream_protocols) { // todo: collect and manage multiple addrs if let Some(addr) = addrs.iter().next() {