refactor: clean-up discv5 configuration (#9143)

bin/reth/src/commands/debug_cmd/execution.rs

@@ -34,7 +34,7 @@ use reth_stages::{
 };
 use reth_static_file::StaticFileProducer;
 use reth_tasks::TaskExecutor;
-use std::{net::SocketAddr, path::PathBuf, sync::Arc};
+use std::{path::PathBuf, sync::Arc};
 use tokio::sync::watch;
 use tracing::*;
 
@@ -130,11 +130,6 @@ impl Command {
             .network
             .network_config(config, provider_factory.chain_spec(), secret_key, default_peers_path)
             .with_task_executor(Box::new(task_executor))
-            .listener_addr(SocketAddr::new(self.network.addr, self.network.port))
-            .discovery_addr(SocketAddr::new(
-                self.network.discovery.addr,
-                self.network.discovery.port,
-            ))
             .build(provider_factory)
             .start_network()
             .await?;

bin/reth/src/commands/debug_cmd/in_memory_merkle.rs

@@ -26,7 +26,7 @@ use reth_revm::database::StateProviderDatabase;
 use reth_stages::StageId;
 use reth_tasks::TaskExecutor;
 use reth_trie::{updates::TrieKey, StateRoot};
-use std::{net::SocketAddr, path::PathBuf, sync::Arc};
+use std::{path::PathBuf, sync::Arc};
 use tracing::*;
 
 /// `reth debug in-memory-merkle` command
@@ -64,11 +64,6 @@ impl Command {
             .network
             .network_config(config, provider_factory.chain_spec(), secret_key, default_peers_path)
             .with_task_executor(Box::new(task_executor))
-            .listener_addr(SocketAddr::new(self.network.addr, self.network.port))
-            .discovery_addr(SocketAddr::new(
-                self.network.discovery.addr,
-                self.network.discovery.port,
-            ))
             .build(provider_factory)
             .start_network()
             .await?;

bin/reth/src/commands/debug_cmd/merkle.rs

@@ -30,7 +30,7 @@ use reth_stages::{
     ExecInput, Stage, StageCheckpoint,
 };
 use reth_tasks::TaskExecutor;
-use std::{net::SocketAddr, path::PathBuf, sync::Arc};
+use std::{path::PathBuf, sync::Arc};
 use tracing::*;
 
 /// `reth debug merkle` command
@@ -69,11 +69,6 @@ impl Command {
             .network
             .network_config(config, provider_factory.chain_spec(), secret_key, default_peers_path)
             .with_task_executor(Box::new(task_executor))
-            .listener_addr(SocketAddr::new(self.network.addr, self.network.port))
-            .discovery_addr(SocketAddr::new(
-                self.network.discovery.addr,
-                self.network.discovery.port,
-            ))
             .build(provider_factory)
             .start_network()
             .await?;

bin/reth/src/commands/debug_cmd/replay_engine.rs

@@ -27,7 +27,7 @@ use reth_stages::Pipeline;
 use reth_static_file::StaticFileProducer;
 use reth_tasks::TaskExecutor;
 use reth_transaction_pool::noop::NoopTransactionPool;
-use std::{net::SocketAddr, path::PathBuf, sync::Arc, time::Duration};
+use std::{path::PathBuf, sync::Arc, time::Duration};
 use tokio::sync::oneshot;
 use tracing::*;
 
@@ -65,11 +65,6 @@ impl Command {
             .network
             .network_config(config, provider_factory.chain_spec(), secret_key, default_peers_path)
             .with_task_executor(Box::new(task_executor))
-            .listener_addr(SocketAddr::new(self.network.addr, self.network.port))
-            .discovery_addr(SocketAddr::new(
-                self.network.discovery.addr,
-                self.network.discovery.port,
-            ))
             .build(provider_factory)
             .start_network()
             .await?;

bin/reth/src/commands/p2p/mod.rs

@@ -4,13 +4,12 @@ use crate::{
     args::{
         get_secret_key,
         utils::{chain_help, chain_value_parser, hash_or_num_value_parser, SUPPORTED_CHAINS},
-        DatabaseArgs, DiscoveryArgs, NetworkArgs,
+        DatabaseArgs, NetworkArgs,
     },
     utils::get_single_header,
 };
 use backon::{ConstantBuilder, Retryable};
 use clap::{Parser, Subcommand};
-use discv5::ListenConfig;
 use reth_chainspec::ChainSpec;
 use reth_config::Config;
 use reth_db::create_db;
@@ -19,11 +18,7 @@ use reth_network_p2p::bodies::client::BodiesClient;
 use reth_node_core::args::DatadirArgs;
 use reth_primitives::BlockHashOrNumber;
 use reth_provider::{providers::StaticFileProvider, ProviderFactory};
-use std::{
-    net::{IpAddr, SocketAddrV4, SocketAddrV6},
-    path::PathBuf,
-    sync::Arc,
-};
+use std::{path::PathBuf, sync::Arc};
 
 /// `reth p2p` command
 #[derive(Debug, Parser)]
@@ -113,47 +108,7 @@ impl Command {
             .disable_discv4_discovery_if(self.chain.chain.is_optimism())
             .boot_nodes(boot_nodes.clone())
             .apply(|builder| {
-                self.network
-                    .discovery
-                    .apply_to_builder(builder, rlpx_socket)
-                    .map_discv5_config_builder(|builder| {
-                        let DiscoveryArgs {
-                            discv5_addr,
-                            discv5_addr_ipv6,
-                            discv5_port,
-                            discv5_port_ipv6,
-                            discv5_lookup_interval,
-                            discv5_bootstrap_lookup_interval,
-                            discv5_bootstrap_lookup_countdown,
-                            ..
-                        } = self.network.discovery;
-
-                        // Use rlpx address if none given
-                        let discv5_addr_ipv4 = discv5_addr.or(match self.network.addr {
-                            IpAddr::V4(ip) => Some(ip),
-                            IpAddr::V6(_) => None,
-                        });
-                        let discv5_addr_ipv6 = discv5_addr_ipv6.or(match self.network.addr {
-                            IpAddr::V4(_) => None,
-                            IpAddr::V6(ip) => Some(ip),
-                        });
-
-                        builder
-                            .discv5_config(
-                                discv5::ConfigBuilder::new(ListenConfig::from_two_sockets(
-                                    discv5_addr_ipv4
-                                        .map(|addr| SocketAddrV4::new(addr, discv5_port)),
-                                    discv5_addr_ipv6.map(|addr| {
-                                        SocketAddrV6::new(addr, discv5_port_ipv6, 0, 0)
-                                    }),
-                                ))
-                                .build(),
-                            )
-                            .add_unsigned_boot_nodes(boot_nodes.into_iter())
-                            .lookup_interval(discv5_lookup_interval)
-                            .bootstrap_lookup_interval(discv5_bootstrap_lookup_interval)
-                            .bootstrap_lookup_countdown(discv5_bootstrap_lookup_countdown)
-                    })
+                self.network.discovery.apply_to_builder(builder, rlpx_socket, boot_nodes)
             })
             .build(Arc::new(ProviderFactory::new(
                 noop_db,

crates/net/network/src/config.rs

@@ -410,36 +410,6 @@ impl NetworkConfigBuilder {
         }
     }
 
-    /// Calls a closure on [`reth_discv5::ConfigBuilder`], if discv5 discovery is enabled and the
-    /// builder has been set.
-    /// ```
-    /// use reth_chainspec::MAINNET;
-    /// use reth_network::NetworkConfigBuilder;
-    /// use reth_provider::test_utils::NoopProvider;
-    /// use secp256k1::{rand::thread_rng, SecretKey};
-    ///
-    /// let sk = SecretKey::new(&mut thread_rng());
-    /// let fork_id = MAINNET.latest_fork_id();
-    /// let network_config = NetworkConfigBuilder::new(sk)
-    ///     .map_discv5_config_builder(|builder| builder.fork(b"eth", fork_id))
-    ///     .build(NoopProvider::default());
-    /// ```
-    pub fn map_discv5_config_builder(
-        mut self,
-        f: impl FnOnce(reth_discv5::ConfigBuilder) -> reth_discv5::ConfigBuilder,
-    ) -> Self {
-        if let Some(mut builder) = self.discovery_v5_builder {
-            if let Some(network_stack_id) = NetworkStackId::id(&self.chain_spec) {
-                let fork_id = self.chain_spec.latest_fork_id();
-                builder = builder.fork(network_stack_id, fork_id);
-            }
-
-            self.discovery_v5_builder = Some(f(builder));
-        }
-
-        self
-    }
-
     /// Adds a new additional protocol to the `RLPx` sub-protocol list.
     pub fn add_rlpx_sub_protocol(mut self, protocol: impl IntoRlpxSubProtocol) -> Self {
         self.extra_protocols.push(protocol);
@@ -479,7 +449,7 @@ impl NetworkConfigBuilder {
             secret_key,
             mut dns_discovery_config,
             discovery_v4_builder,
-            discovery_v5_builder,
+            mut discovery_v5_builder,
             boot_nodes,
             discovery_addr,
             listener_addr,
@@ -496,6 +466,15 @@ impl NetworkConfigBuilder {
             transactions_manager_config,
         } = self;
 
+        discovery_v5_builder = discovery_v5_builder.map(|mut builder| {
+            if let Some(network_stack_id) = NetworkStackId::id(&chain_spec) {
+                let fork_id = chain_spec.latest_fork_id();
+                builder = builder.fork(network_stack_id, fork_id)
+            }
+
+            builder
+        });
+
         let listener_addr = listener_addr.unwrap_or(DEFAULT_DISCOVERY_ADDRESS);
 
         let mut hello_message =

crates/node-core/src/args/network.rs

@@ -4,9 +4,9 @@ use crate::version::P2P_CLIENT_VERSION;
 use clap::Args;
 use reth_chainspec::{net::mainnet_nodes, ChainSpec};
 use reth_config::Config;
-use reth_discv4::{DEFAULT_DISCOVERY_ADDR, DEFAULT_DISCOVERY_PORT};
+use reth_discv4::{NodeRecord, DEFAULT_DISCOVERY_ADDR, DEFAULT_DISCOVERY_PORT};
 use reth_discv5::{
-    DEFAULT_COUNT_BOOTSTRAP_LOOKUPS, DEFAULT_DISCOVERY_V5_PORT,
+    discv5::ListenConfig, DEFAULT_COUNT_BOOTSTRAP_LOOKUPS, DEFAULT_DISCOVERY_V5_PORT,
     DEFAULT_SECONDS_BOOTSTRAP_LOOKUP_INTERVAL, DEFAULT_SECONDS_LOOKUP_INTERVAL,
 };
 use reth_net_nat::NatResolver;
@@ -21,7 +21,7 @@ use reth_network::{
 use reth_network_peers::TrustedPeer;
 use secp256k1::SecretKey;
 use std::{
-    net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr},
+    net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6},
     ops::Not,
     path::PathBuf,
     sync::Arc,
@@ -173,23 +173,17 @@ impl NetworkArgs {
             // apply discovery settings
             .apply(|builder| {
                 let rlpx_socket = (self.addr, self.port).into();
-                self.discovery.apply_to_builder(builder, rlpx_socket)
-            })
-            // modify discv5 settings if enabled in previous step
-            .map_discv5_config_builder(|builder| {
-                let DiscoveryArgs {
-                    discv5_lookup_interval,
-                    discv5_bootstrap_lookup_interval,
-                    discv5_bootstrap_lookup_countdown,
-                    ..
-                } = self.discovery;
-
-                builder
-                    .add_unsigned_boot_nodes(chain_bootnodes)
-                    .lookup_interval(discv5_lookup_interval)
-                    .bootstrap_lookup_interval(discv5_bootstrap_lookup_interval)
-                    .bootstrap_lookup_countdown(discv5_bootstrap_lookup_countdown)
+                self.discovery.apply_to_builder(builder, rlpx_socket, chain_bootnodes)
             })
+            .listener_addr(SocketAddr::new(
+                self.addr, // set discovery port based on instance number
+                self.port,
+            ))
+            .discovery_addr(SocketAddr::new(
+                self.discovery.addr,
+                // set discovery port based on instance number
+                self.discovery.port,
+            ))
     }
 
     /// If `no_persist_peers` is false then this returns the path to the persistent peers file path.
@@ -211,6 +205,17 @@ impl NetworkArgs {
         self.discovery = self.discovery.with_unused_discovery_port();
         self
     }
+
+    /// Change networking port numbers based on the instance number.
+    /// Ports are updated to `previous_value + instance - 1`
+    ///
+    /// # Panics
+    /// Warning: if `instance` is zero in debug mode, this will panic.
+    pub fn adjust_instance_ports(&mut self, instance: u16) {
+        debug_assert_ne!(instance, 0, "instance must be non-zero");
+        self.port += instance - 1;
+        self.discovery.adjust_instance_ports(instance);
+    }
 }
 
 impl Default for NetworkArgs {
@@ -309,6 +314,7 @@ impl DiscoveryArgs {
         &self,
         mut network_config_builder: NetworkConfigBuilder,
         rlpx_tcp_socket: SocketAddr,
+        boot_nodes: impl IntoIterator<Item = NodeRecord>,
     ) -> NetworkConfigBuilder {
         if self.disable_discovery || self.disable_dns_discovery {
             network_config_builder = network_config_builder.disable_dns_discovery();
@@ -319,19 +325,72 @@ impl DiscoveryArgs {
         }
 
         if !self.disable_discovery && self.enable_discv5_discovery {
-            network_config_builder =
-                network_config_builder.discovery_v5(reth_discv5::Config::builder(rlpx_tcp_socket));
+            network_config_builder = network_config_builder
+                .discovery_v5(self.discovery_v5_builder(rlpx_tcp_socket, boot_nodes));
         }
 
         network_config_builder
     }
 
+    /// Creates a [`reth_discv5::ConfigBuilder`] filling it with the values from this struct.
+    pub fn discovery_v5_builder(
+        &self,
+        rlpx_tcp_socket: SocketAddr,
+        boot_nodes: impl IntoIterator<Item = NodeRecord>,
+    ) -> reth_discv5::ConfigBuilder {
+        let Self {
+            discv5_addr,
+            discv5_addr_ipv6,
+            discv5_port,
+            discv5_port_ipv6,
+            discv5_lookup_interval,
+            discv5_bootstrap_lookup_interval,
+            discv5_bootstrap_lookup_countdown,
+            ..
+        } = self;
+
+        // Use rlpx address if none given
+        let discv5_addr_ipv4 = discv5_addr.or(match rlpx_tcp_socket {
+            SocketAddr::V4(addr) => Some(*addr.ip()),
+            SocketAddr::V6(_) => None,
+        });
+        let discv5_addr_ipv6 = discv5_addr_ipv6.or(match rlpx_tcp_socket {
+            SocketAddr::V4(_) => None,
+            SocketAddr::V6(addr) => Some(*addr.ip()),
+        });
+
+        reth_discv5::Config::builder(rlpx_tcp_socket)
+            .discv5_config(
+                reth_discv5::discv5::ConfigBuilder::new(ListenConfig::from_two_sockets(
+                    discv5_addr_ipv4.map(|addr| SocketAddrV4::new(addr, *discv5_port)),
+                    discv5_addr_ipv6.map(|addr| SocketAddrV6::new(addr, *discv5_port_ipv6, 0, 0)),
+                ))
+                .build(),
+            )
+            .add_unsigned_boot_nodes(boot_nodes)
+            .lookup_interval(*discv5_lookup_interval)
+            .bootstrap_lookup_interval(*discv5_bootstrap_lookup_interval)
+            .bootstrap_lookup_countdown(*discv5_bootstrap_lookup_countdown)
+    }
+
     /// Set the discovery port to zero, to allow the OS to assign a random unused port when
     /// discovery binds to the socket.
     pub const fn with_unused_discovery_port(mut self) -> Self {
         self.port = 0;
         self
     }
+
+    /// Change networking port numbers based on the instance number.
+    /// Ports are updated to `previous_value + instance - 1`
+    ///
+    /// # Panics
+    /// Warning: if `instance` is zero in debug mode, this will panic.
+    pub fn adjust_instance_ports(&mut self, instance: u16) {
+        debug_assert_ne!(instance, 0, "instance must be non-zero");
+        self.port += instance - 1;
+        self.discv5_port += instance - 1;
+        self.discv5_port_ipv6 += instance - 1;
+    }
 }
 
 impl Default for DiscoveryArgs {

crates/node-core/src/node_config.rs

@@ -390,6 +390,7 @@ impl NodeConfig {
     /// [`RpcServerArgs::adjust_instance_ports`] method.
     pub fn adjust_instance_ports(&mut self) {
         self.rpc.adjust_instance_ports(self.instance);
+        self.network.adjust_instance_ports(self.instance);
     }
 
     /// Sets networking and RPC ports to zero, causing the OS to choose random unused ports when