From f65969e90f449bda037611b4f18ccb68a6fcb15e Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sat, 3 Dec 2022 12:02:49 +0100 Subject: [PATCH] perf(net): exclude bootnodes from update stream (#319) --- crates/net/discv4/src/lib.rs | 35 ++++++++++++++++++++++--- crates/net/network/src/config.rs | 15 ++++++++++- crates/net/network/src/manager.rs | 5 +++- crates/net/network/src/peers/manager.rs | 6 +++++ crates/net/network/src/session/mod.rs | 1 + 5 files changed, 56 insertions(+), 6 deletions(-) diff --git a/crates/net/discv4/src/lib.rs b/crates/net/discv4/src/lib.rs index 9371e35806..203762d549 100644 --- a/crates/net/discv4/src/lib.rs +++ b/crates/net/discv4/src/lib.rs @@ -382,13 +382,31 @@ impl Discv4Service { /// own ID in the DHT. This introduces the local node to the other nodes /// in the DHT and populates its routing table with the closest proven neighbours. /// - /// This is equivalent to adding all bootnodes via [`Self::add_node()`]. + /// This is similar to adding all bootnodes via [`Self::add_node`], but does not fire a + /// [`TableUpdate::Added`] event for the given bootnodes. So boot nodes don't appear in the + /// update stream, which is usually desirable, since bootnodes should not be connected to. + /// + /// If adding the configured boodnodes should result in a [`TableUpdate::Added`], see + /// [`Self::add_all_nodes`]. /// /// **Note:** This is a noop if there are no bootnodes. pub fn bootstrap(&mut self) { - for node in self.config.bootstrap_nodes.clone() { - debug!(target : "net::disc", ?node, "Adding bootstrap node"); - self.add_node(node); + for record in self.config.bootstrap_nodes.clone() { + debug!(target : "net::disc", ?record, "Adding bootstrap node"); + let key = kad_key(record.id); + let entry = NodeEntry { record, last_seen: Instant::now() }; + + // insert the boot node in the table + let _ = self.kbuckets.insert_or_update( + &key, + entry, + NodeStatus { + state: ConnectionState::Connected, + direction: ConnectionDirection::Outgoing, + }, + ); + + self.try_ping(record, PingReason::Normal); } } @@ -530,6 +548,15 @@ impl Discv4Service { } } + /// Adds all nodes + /// + /// See [Self::add_node] + pub fn add_all_nodes(&mut self, records: impl IntoIterator) { + for record in records.into_iter() { + self.add_node(record); + } + } + /// If the node's not in the table yet, this will start a ping to get it added on ping. pub fn add_node(&mut self, record: NodeRecord) { let key = kad_key(record.id); diff --git a/crates/net/network/src/config.rs b/crates/net/network/src/config.rs index 35b8ffe680..7ddf698770 100644 --- a/crates/net/network/src/config.rs +++ b/crates/net/network/src/config.rs @@ -3,7 +3,7 @@ use crate::{ peers::PeersConfig, session::SessionsConfig, }; -use reth_discv4::{Discv4Config, Discv4ConfigBuilder, DEFAULT_DISCOVERY_PORT}; +use reth_discv4::{Discv4Config, Discv4ConfigBuilder, NodeRecord, DEFAULT_DISCOVERY_PORT}; use reth_primitives::{Chain, ForkId, H256}; use secp256k1::SecretKey; use std::{ @@ -17,6 +17,8 @@ pub struct NetworkConfig { pub client: Arc, /// The node's secret key, from which the node's identity is derived. pub secret_key: SecretKey, + /// All boot nodes to start network discovery with. + pub boot_nodes: Vec, /// How to set up discovery. pub discovery_v4_config: Discv4Config, /// Address to use for discovery @@ -75,6 +77,8 @@ pub struct NetworkConfigBuilder { secret_key: SecretKey, /// How to set up discovery. discovery_v4_builder: Discv4ConfigBuilder, + /// All boot nodes to start network discovery with. + boot_nodes: Vec, /// Address to use for discovery discovery_addr: Option, /// Listener for incoming connections @@ -105,6 +109,7 @@ impl NetworkConfigBuilder { client, secret_key, discovery_v4_builder: Default::default(), + boot_nodes: vec![], discovery_addr: None, listener_addr: None, peers_config: None, @@ -153,12 +158,19 @@ impl NetworkConfigBuilder { self } + /// Sets the discv4 config to use. + pub fn boot_nodes(mut self, nodes: impl IntoIterator) -> Self { + self.boot_nodes = nodes.into_iter().collect(); + self + } + /// Consumes the type and creates the actual [`NetworkConfig`] pub fn build(self) -> NetworkConfig { let Self { client, secret_key, discovery_v4_builder, + boot_nodes, discovery_addr, listener_addr, peers_config, @@ -172,6 +184,7 @@ impl NetworkConfigBuilder { NetworkConfig { client, secret_key, + boot_nodes, discovery_v4_config: discovery_v4_builder.build(), discovery_addr: discovery_addr.unwrap_or_else(|| { SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, DEFAULT_DISCOVERY_PORT)) diff --git a/crates/net/network/src/manager.rs b/crates/net/network/src/manager.rs index 0475cc1aec..8e77fe8499 100644 --- a/crates/net/network/src/manager.rs +++ b/crates/net/network/src/manager.rs @@ -113,7 +113,7 @@ where let NetworkConfig { client, secret_key, - discovery_v4_config, + mut discovery_v4_config, discovery_addr, listener_addr, peers_config, @@ -121,6 +121,7 @@ where genesis_hash, block_import, network_mode, + boot_nodes, .. } = config; @@ -130,6 +131,8 @@ where let incoming = ConnectionListener::bind(listener_addr).await?; let listener_address = Arc::new(Mutex::new(incoming.local_address())); + // merge configured boot nodes + discovery_v4_config.bootstrap_nodes.extend(boot_nodes.clone()); let discovery = Discovery::new(discovery_addr, secret_key, discovery_v4_config).await?; // need to retrieve the addr here since provided port could be `0` let local_peer_id = discovery.local_id(); diff --git a/crates/net/network/src/peers/manager.rs b/crates/net/network/src/peers/manager.rs index 6d4378fa8d..76b1271929 100644 --- a/crates/net/network/src/peers/manager.rs +++ b/crates/net/network/src/peers/manager.rs @@ -13,6 +13,7 @@ use tokio::{ time::{Instant, Interval}, }; use tokio_stream::wrappers::UnboundedReceiverStream; +use tracing::trace; /// A communication channel to the [`PeersManager`] to apply manual changes to the peer set. pub struct PeersHandle { @@ -183,6 +184,7 @@ impl PeersManager { node.addr = addr; } Entry::Vacant(entry) => { + trace!(target : "net::peers", ?peer_id, ?addr, "discovered new node"); entry.insert(Peer::new(addr)); } } @@ -191,6 +193,7 @@ impl PeersManager { /// Removes the tracked node from the set. pub(crate) fn remove_discovered_node(&mut self, peer_id: PeerId) { if let Some(entry) = self.peers.remove(&peer_id) { + trace!(target : "net::peers", ?peer_id, "remove discovered node"); if entry.state.is_connected() { // TODO(mattsse): is this right to disconnect peers? self.connection_info.decr_state(entry.state); @@ -234,6 +237,9 @@ impl PeersManager { if peer.is_banned() { break } + + trace!(target : "net::peers", ?peer_id, addr=?peer.addr, "schedule outbound connection"); + peer.state = PeerConnectionState::Out; PeerAction::Connect { peer_id, remote_addr: peer.addr } }; diff --git a/crates/net/network/src/session/mod.rs b/crates/net/network/src/session/mod.rs index dc6bc699e3..4c0e3d24de 100644 --- a/crates/net/network/src/session/mod.rs +++ b/crates/net/network/src/session/mod.rs @@ -359,6 +359,7 @@ impl SessionManager { target : "net::session", ?session_id, ?remote_addr, + ?error, "disconnected pending session" ); self.remove_pending_session(&session_id);