From 48a98d691a92750093a15eb85c4a48ab5878e54b Mon Sep 17 00:00:00 2001 From: Rob Date: Thu, 5 Dec 2024 10:31:25 -0500 Subject: [PATCH] potentially fix Libp2p restart test --- .../src/traits/networking/combined_network.rs | 3 -- .../src/traits/networking/libp2p_network.rs | 38 ++++++++++--------- .../src/traits/networking/memory_network.rs | 1 - .../src/traits/networking/push_cdn_network.rs | 1 - crates/types/src/traits/network.rs | 1 - .../types/src/traits/node_implementation.rs | 3 -- 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/crates/hotshot/src/traits/networking/combined_network.rs b/crates/hotshot/src/traits/networking/combined_network.rs index 0639dc2f8f..f73c016ed4 100644 --- a/crates/hotshot/src/traits/networking/combined_network.rs +++ b/crates/hotshot/src/traits/networking/combined_network.rs @@ -255,7 +255,6 @@ pub struct UnderlyingCombinedNetworks( impl TestableNetworkingImplementation for CombinedNetworks { fn generator( expected_node_count: usize, - test_id: usize, da_committee_size: usize, reliability_config: Option>, secondary_network_delay: Duration, @@ -263,14 +262,12 @@ impl TestableNetworkingImplementation for CombinedNetwor let generators = ( as TestableNetworkingImplementation>::generator( expected_node_count, - test_id, da_committee_size, None, Duration::default(), ), as TestableNetworkingImplementation>::generator( expected_node_count, - test_id, da_committee_size, reliability_config, Duration::default(), diff --git a/crates/hotshot/src/traits/networking/libp2p_network.rs b/crates/hotshot/src/traits/networking/libp2p_network.rs index 94acc37d20..81b0f74688 100644 --- a/crates/hotshot/src/traits/networking/libp2p_network.rs +++ b/crates/hotshot/src/traits/networking/libp2p_network.rs @@ -172,7 +172,7 @@ pub struct Libp2pNetwork { } /// Generate the expected [testing] multiaddr from a node index -fn multiaddr_from_node_index(i: usize) -> Multiaddr { +fn multiaddr_from_node_index(i: usize, bind_addresses: &[Multiaddr]) -> Multiaddr { // Generate the node's private key from the node ID let peers_hotshot_private_key = T::SignatureKey::generated_from_seed_indexed([0u8; 32], i as u64).1; @@ -182,12 +182,10 @@ fn multiaddr_from_node_index(i: usize) -> Multiaddr { .expect("Failed to derive libp2p keypair"); // Generate the multiaddress using the peer id and port - Multiaddr::from_str(&format!( - "/ip4/127.0.0.1/udp/{}/quic-v1/p2p/{}", - 48000 + i, - peers_libp2p_keypair.public().to_peer_id() - )) - .expect("Failed to create multiaddr") + bind_addresses[i] + .clone() + .with_p2p(peers_libp2p_keypair.public().to_peer_id()) + .expect("Failed to append libp2p peer id to multiaddr") } #[cfg(feature = "hotshot-testing")] @@ -204,7 +202,6 @@ impl TestableNetworkingImplementation for Libp2pNetwork { #[allow(clippy::panic, clippy::too_many_lines)] fn generator( expected_node_count: usize, - test_id: usize, da_committee_size: usize, reliability_config: Option>, _secondary_network_delay: Duration, @@ -214,16 +211,23 @@ impl TestableNetworkingImplementation for Libp2pNetwork { "DA committee size must be less than or equal to total # nodes" ); + // Generate the bind addresses each node will use + let mut bind_addresses = Vec::new(); + for _ in 0..expected_node_count { + bind_addresses.push( + Multiaddr::from_str(&format!( + "/ip4/127.0.0.1/udp/{}/quic-v1", + portpicker::pick_unused_port().expect("All ports are in use") + )) + .expect("Failed to create multiaddr"), + ); + } + // NOTE uncomment this for easier debugging Box::pin({ move |node_id| { - // The port is 48000 + the node id. This way it's deterministic and easy to connect nodes together - let port = 48000 + node_id; - - // Create the bind address - let bind_address = - Multiaddr::from_str(&format!("/ip4/127.0.0.{test_id}/udp/{port}/quic-v1")) - .unwrap(); + // Get our bind address from the list + let bind_address = bind_addresses[usize::try_from(node_id).unwrap()].clone(); // Deterministically generate the private key from the node ID let hotshot_private_key = @@ -272,7 +276,7 @@ impl TestableNetworkingImplementation for Libp2pNetwork { // Create the list of known peers let known_peers = (0..expected_node_count) - .map(|i| multiaddr_from_node_index::(i)) + .map(|i| multiaddr_from_node_index::(i, &bind_addresses)) .collect(); // Collect the `PeerConfig`s of all nodes @@ -434,7 +438,7 @@ impl Libp2pNetwork { // Spawn the network node with a copy of our config let (mut rx, network_handle) = spawn_network_node::(config.clone()) .await - .map_err(|e| NetworkError::ConfigError(format!("failed to spawn network node: {e}")))?; + .with_context(|| "failed to spawn network node")?; // Subscribe only to the global topic (DA messages are direct-broadcasted) let subscribed_topics = HashSet::from_iter(vec![GLOBAL_TOPIC.to_string()]); diff --git a/crates/hotshot/src/traits/networking/memory_network.rs b/crates/hotshot/src/traits/networking/memory_network.rs index 7b11d58be4..297f116430 100644 --- a/crates/hotshot/src/traits/networking/memory_network.rs +++ b/crates/hotshot/src/traits/networking/memory_network.rs @@ -180,7 +180,6 @@ impl TestableNetworkingImplementation { fn generator( _expected_node_count: usize, - _test_id: usize, da_committee_size: usize, reliability_config: Option>, _secondary_network_delay: Duration, diff --git a/crates/hotshot/src/traits/networking/push_cdn_network.rs b/crates/hotshot/src/traits/networking/push_cdn_network.rs index 9681d8104a..f3a068530f 100644 --- a/crates/hotshot/src/traits/networking/push_cdn_network.rs +++ b/crates/hotshot/src/traits/networking/push_cdn_network.rs @@ -281,7 +281,6 @@ impl TestableNetworkingImplementation #[allow(clippy::too_many_lines)] fn generator( _expected_node_count: usize, - _test_id: usize, da_committee_size: usize, _reliability_config: Option>, _secondary_network_delay: Duration, diff --git a/crates/types/src/traits/network.rs b/crates/types/src/traits/network.rs index c6b4ea673c..1211cc93e2 100644 --- a/crates/types/src/traits/network.rs +++ b/crates/types/src/traits/network.rs @@ -287,7 +287,6 @@ where #[allow(clippy::type_complexity)] fn generator( expected_node_count: usize, - test_id: usize, da_committee_size: usize, reliability_config: Option>, secondary_network_delay: Duration, diff --git a/crates/types/src/traits/node_implementation.rs b/crates/types/src/traits/node_implementation.rs index 8489ef41af..49b814672a 100644 --- a/crates/types/src/traits/node_implementation.rs +++ b/crates/types/src/traits/node_implementation.rs @@ -19,7 +19,6 @@ use std::{ use async_trait::async_trait; use committable::Committable; -use rand::Rng; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use url::Url; use vbs::version::StaticVersionType; @@ -149,8 +148,6 @@ where ) -> AsyncGenerator> { >::generator( expected_node_count, - // Generate a unique-ish test id between 0 and 255 - rand::thread_rng().gen_range(0..255), da_committee_size, reliability_config.clone(), secondary_network_delay,