feat: add with_unused_ports method to rpc and network args (#6109)

bin/reth/src/args/network_args.rs

@@ -122,6 +122,21 @@ impl NetworkArgs {
 
         Some(peers_file)
     }
+
+    /// Sets the p2p port to zero, to allow the OS to assign a random unused port when
+    /// the network components bind to a socket.
+    pub fn with_unused_p2p_port(mut self) -> Self {
+        self.port = 0;
+        self
+    }
+
+    /// Sets the p2p and discovery ports to zero, allowing the OD to assign a random unused port
+    /// when network components bind to sockets.
+    pub fn with_unused_ports(mut self) -> Self {
+        self = self.with_unused_p2p_port();
+        self.discovery = self.discovery.with_unused_discovery_port();
+        self
+    }
 }
 
 impl Default for NetworkArgs {
@@ -183,6 +198,13 @@ impl DiscoveryArgs {
         }
         network_config_builder
     }
+
+    /// Set the discovery port to zero, to allow the OS to assign a random unused port when
+    /// discovery binds to the socket.
+    pub fn with_unused_discovery_port(mut self) -> Self {
+        self.port = 0;
+        self
+    }
 }
 
 impl Default for DiscoveryArgs {

bin/reth/src/args/rpc_server_args.rs

@@ -17,6 +17,7 @@ use clap::{
     Arg, Args, Command,
 };
 use futures::TryFutureExt;
+use rand::Rng;
 use reth_network_api::{NetworkInfo, Peers};
 use reth_node_api::EngineTypes;
 use reth_provider::{
@@ -220,6 +221,49 @@ impl RpcServerArgs {
         self.ipcpath = format!("{}-{}", self.ipcpath, instance);
     }
 
+    /// Set the http port to zero, to allow the OS to assign a random unused port when the rpc
+    /// server binds to a socket.
+    pub fn with_http_unused_port(mut self) -> Self {
+        self.http_port = 0;
+        self
+    }
+
+    /// Set the ws port to zero, to allow the OS to assign a random unused port when the rpc
+    /// server binds to a socket.
+    pub fn with_ws_unused_port(mut self) -> Self {
+        self.ws_port = 0;
+        self
+    }
+
+    /// Set the auth port to zero, to allow the OS to assign a random unused port when the rpc
+    /// server binds to a socket.
+    pub fn with_auth_unused_port(mut self) -> Self {
+        self.auth_port = 0;
+        self
+    }
+
+    /// Append a random string to the ipc path, to prevent possible collisions when multiple nodes
+    /// are being run on the same machine.
+    pub fn with_ipc_random_path(mut self) -> Self {
+        let random_string: String = rand::thread_rng()
+            .sample_iter(rand::distributions::Alphanumeric)
+            .take(8)
+            .map(char::from)
+            .collect();
+        self.ipcpath = format!("{}-{}", self.ipcpath, random_string);
+        self
+    }
+
+    /// Configure all ports to be set to a random unused port when bound, and set the IPC path to a
+    /// random path.
+    pub fn with_unused_ports(mut self) -> Self {
+        self = self.with_http_unused_port();
+        self = self.with_ws_unused_port();
+        self = self.with_auth_unused_port();
+        self = self.with_ipc_random_path();
+        self
+    }
+
     /// Configures and launches _all_ servers.
     ///
     /// Returns the handles for the launched regular RPC server(s) (if any) and the server handle

bin/reth/src/builder/mod.rs

@@ -246,7 +246,7 @@ pub struct NodeConfig {
 impl NodeConfig {
     /// Creates a testing [NodeConfig], causing the database to be launched ephemerally.
     pub fn test() -> Self {
-        Self {
+        let mut test = Self {
             database: DatabaseBuilder::test(),
             config: None,
             chain: MAINNET.clone(),
@@ -263,7 +263,11 @@ impl NodeConfig {
             pruning: PruningArgs::default(),
             #[cfg(feature = "optimism")]
             rollup: crate::args::RollupArgs::default(),
-        }
+        };
+
+        // set all ports to zero by default for test instances
+        test = test.with_unused_ports();
+        test
     }
 
     /// Set the datadir for the node
@@ -350,12 +354,6 @@ impl NodeConfig {
         self
     }
 
-    /// Set the node instance number
-    pub fn with_instance_number(mut self, instance: u16) -> Self {
-        self.instance = instance;
-        self
-    }
-
     /// Set the rollup args for the node
     #[cfg(feature = "optimism")]
     pub fn with_rollup(mut self, rollup: crate::args::RollupArgs) -> Self {
@@ -949,6 +947,14 @@ impl NodeConfig {
     fn adjust_instance_ports(&mut self) {
         self.rpc.adjust_instance_ports(self.instance);
     }
+
+    /// Sets networking and RPC ports to zero, causing the OS to choose random unused ports when
+    /// sockets are bound.
+    fn with_unused_ports(mut self) -> Self {
+        self.rpc = self.rpc.with_unused_ports();
+        self.network = self.network.with_unused_ports();
+        self
+    }
 }
 
 impl Default for NodeConfig {
@@ -1389,8 +1395,7 @@ impl NodeHandle {
 ///     // Create a node builder with an http rpc server enabled
 ///     let rpc_args = RpcServerArgs::default().with_http();
 ///
-///     /// Set the node instance number to 2
-///     let builder = NodeConfig::test().with_rpc(rpc_args).with_instance(2);
+///     let builder = NodeConfig::test().with_rpc(rpc_args);
 ///
 ///     // Spawn the builder, returning a handle to the node
 ///     let (_handle, _manager) = spawn_node(builder).await.unwrap();
@@ -1413,11 +1418,7 @@ mod tests {
         // this launches a test node with http
         let rpc_args = RpcServerArgs::default().with_http();
 
-        // NOTE: tests here manually set an instance number. The alternative would be to use an
-        // atomic counter. This works for `cargo test` but if tests would be run in `nextest` then
-        // they would become flaky. So new tests should manually set a unique instance number.
-        let (handle, _manager) =
-            spawn_node(NodeConfig::test().with_rpc(rpc_args).with_instance(1)).await.unwrap();
+        let (handle, _manager) = spawn_node(NodeConfig::test().with_rpc(rpc_args)).await.unwrap();
 
         // call a function on the node
         let client = handle.rpc_server_handles().rpc.http_client().unwrap();
@@ -1430,7 +1431,7 @@ mod tests {
     #[tokio::test]
     async fn rpc_handles_none_without_http() {
         // this launches a test node _without_ http
-        let (handle, _manager) = spawn_node(NodeConfig::test().with_instance(2)).await.unwrap();
+        let (handle, _manager) = spawn_node(NodeConfig::test()).await.unwrap();
 
         // ensure that the `http_client` is none
         let maybe_client = handle.rpc_server_handles().rpc.http_client();
@@ -1442,13 +1443,10 @@ mod tests {
         // spawn_test_node takes roughly 1 second per node, so this test takes ~4 seconds
         let num_nodes = 4;
 
-        // this reserves instances 3-6
-        let starting_instance = 3;
         // contains handles and managers
         let mut handles = Vec::new();
-        for i in 0..num_nodes {
-            let handle =
-                spawn_node(NodeConfig::test().with_instance(starting_instance + i)).await.unwrap();
+        for _ in 0..num_nodes {
+            let handle = spawn_node(NodeConfig::test()).await.unwrap();
             handles.push(handle);
         }
     }
@@ -1478,7 +1476,7 @@ mod tests {
         let genesis_hash = spec.genesis_hash();
 
         // create node config
-        let node_config = NodeConfig::test().with_rpc(rpc_args).with_instance(7).with_chain(spec);
+        let node_config = NodeConfig::test().with_rpc(rpc_args).with_chain(spec);
 
         let (handle, _manager) = spawn_node(node_config).await.unwrap();
 
@@ -1546,7 +1544,7 @@ mod tests {
         let genesis_hash = spec.genesis_hash();
 
         // create node config
-        let node_config = NodeConfig::test().with_rpc(rpc_args).with_instance(8).with_chain(spec);
+        let node_config = NodeConfig::test().with_rpc(rpc_args).with_chain(spec);
 
         let (handle, _manager) = spawn_node(node_config).await.unwrap();
 
@@ -1615,7 +1613,7 @@ mod tests {
         let genesis_hash = spec.genesis_hash();
 
         // create node config
-        let node_config = NodeConfig::test().with_rpc(rpc_args).with_instance(9).with_chain(spec);
+        let node_config = NodeConfig::test().with_rpc(rpc_args).with_chain(spec);
 
         let (handle, _manager) = spawn_node(node_config).await.unwrap();
 
@@ -1683,7 +1681,7 @@ mod tests {
         let genesis_hash = spec.genesis_hash();
 
         // create node config
-        let node_config = NodeConfig::test().with_rpc(rpc_args).with_instance(10).with_chain(spec);
+        let node_config = NodeConfig::test().with_rpc(rpc_args).with_chain(spec);
 
         let (handle, _manager) = spawn_node(node_config).await.unwrap();