Skip to content

Commit

Permalink
refactor(swarm-test): don't implicitly add external addresses
Browse files Browse the repository at this point in the history
This is a safer default than always adding the memory address as external. Kademlia for example depends on whether we have an external address.

Motivated-by: #4735.

Pull-Request: #4743.
  • Loading branch information
thomaseizinger authored Oct 30, 2023
1 parent 35a92df commit 9bd127f
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 101 deletions.
16 changes: 8 additions & 8 deletions misc/allow-block-list/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ mod tests {
async fn cannot_dial_blocked_peer() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

dialer.behaviour_mut().block_peer(*listener.local_peer_id());

Expand All @@ -298,7 +298,7 @@ mod tests {
async fn can_dial_unblocked_peer() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

dialer.behaviour_mut().block_peer(*listener.local_peer_id());
dialer
Expand All @@ -312,7 +312,7 @@ mod tests {
async fn blocked_peer_cannot_dial_us() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

listener.behaviour_mut().block_peer(*dialer.local_peer_id());
dial(&mut dialer, &listener).unwrap();
Expand All @@ -334,7 +334,7 @@ mod tests {
async fn connections_get_closed_upon_blocked() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;
dialer.connect(&mut listener).await;

dialer.behaviour_mut().block_peer(*listener.local_peer_id());
Expand All @@ -360,7 +360,7 @@ mod tests {
async fn cannot_dial_peer_unless_allowed() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

let DialError::Denied { cause } = dial(&mut dialer, &listener).unwrap_err() else {
panic!("unexpected dial error")
Expand All @@ -375,7 +375,7 @@ mod tests {
async fn cannot_dial_disallowed_peer() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

dialer.behaviour_mut().allow_peer(*listener.local_peer_id());
dialer
Expand All @@ -392,7 +392,7 @@ mod tests {
async fn not_allowed_peer_cannot_dial_us() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;

dialer
.dial(
Expand Down Expand Up @@ -429,7 +429,7 @@ mod tests {
async fn connections_get_closed_upon_disallow() {
let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
let mut listener = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::default());
listener.listen().await;
listener.listen().with_memory_addr_external().await;
dialer.behaviour_mut().allow_peer(*listener.local_peer_id());
listener.behaviour_mut().allow_peer(*dialer.local_peer_id());

Expand Down
2 changes: 1 addition & 1 deletion misc/connection-limits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ mod tests {
});

async_std::task::block_on(async {
let (listen_addr, _) = swarm1.listen().await;
let (listen_addr, _) = swarm1.listen().with_memory_addr_external().await;

for _ in 0..limit {
swarm2.connect(&mut swarm1).await;
Expand Down
2 changes: 1 addition & 1 deletion protocols/autonat/tests/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ async fn test_confidence() {
// Randomly test either for public or for private status the confidence.
let test_public = rand::random::<bool>();
if test_public {
client.listen().await;
client.listen().with_memory_addr_external().await;
} else {
let unreachable_addr = "/ip4/127.0.0.1/tcp/42".parse().unwrap();
client.behaviour_mut().probe_address(unreachable_addr);
Expand Down
14 changes: 4 additions & 10 deletions protocols/dcutr/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,9 @@ async fn connect() {
let mut src = build_client();

// Have all swarms listen on a local TCP address.
let (memory_addr, relay_addr) = relay.listen().await;
relay.remove_external_address(&memory_addr);
relay.add_external_address(relay_addr.clone());

let (dst_mem_addr, dst_tcp_addr) = dst.listen().await;
let (src_mem_addr, _) = src.listen().await;

dst.remove_external_address(&dst_mem_addr);
src.remove_external_address(&src_mem_addr);
let (_, relay_tcp_addr) = relay.listen().with_tcp_addr_external().await;
let (_, dst_tcp_addr) = dst.listen().await;
src.listen().await;

assert!(src.external_addresses().next().is_none());
assert!(dst.external_addresses().next().is_none());
Expand All @@ -58,7 +52,7 @@ async fn connect() {

async_std::task::spawn(relay.loop_on_next());

let dst_relayed_addr = relay_addr
let dst_relayed_addr = relay_tcp_addr
.with(Protocol::P2p(relay_peer_id))
.with(Protocol::P2pCircuit)
.with(Protocol::P2p(dst_peer_id));
Expand Down
2 changes: 1 addition & 1 deletion protocols/gossipsub/tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ async fn build_node() -> Swarm<gossipsub::Behaviour> {
.unwrap();
gossipsub::Behaviour::new(MessageAuthenticity::Author(peer_id), config).unwrap()
});
swarm.listen().await;
swarm.listen().with_memory_addr_external().await;

swarm
}
Expand Down
7 changes: 4 additions & 3 deletions protocols/identify/tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ async fn periodic_identify() {
});
let swarm2_peer_id = *swarm2.local_peer_id();

let (swarm1_memory_listen, swarm1_tcp_listen_addr) = swarm1.listen().await;
let (swarm1_memory_listen, swarm1_tcp_listen_addr) =
swarm1.listen().with_memory_addr_external().await;
let (swarm2_memory_listen, swarm2_tcp_listen_addr) = swarm2.listen().await;
swarm2.connect(&mut swarm1).await;

Expand Down Expand Up @@ -92,7 +93,7 @@ async fn identify_push() {
)
});

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

// First, let the periodic identify do its thing.
Expand Down Expand Up @@ -142,7 +143,7 @@ async fn discover_peer_after_disconnect() {
)
});

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

let swarm1_peer_id = *swarm1.local_peer_id();
Expand Down
17 changes: 7 additions & 10 deletions protocols/kad/tests/client_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async fn server_gets_added_to_routing_table_by_client() {
let mut client = Swarm::new_ephemeral(MyBehaviour::new);
let mut server = Swarm::new_ephemeral(MyBehaviour::new);

server.listen().await;
server.listen().with_memory_addr_external().await;
client.connect(&mut server).await;

let server_peer_id = *server.local_peer_id();
Expand All @@ -37,7 +37,7 @@ async fn two_servers_add_each_other_to_routing_table() {
let mut server1 = Swarm::new_ephemeral(MyBehaviour::new);
let mut server2 = Swarm::new_ephemeral(MyBehaviour::new);

server2.listen().await;
server2.listen().with_memory_addr_external().await;
server1.connect(&mut server2).await;

let server1_peer_id = *server1.local_peer_id();
Expand All @@ -54,7 +54,7 @@ async fn two_servers_add_each_other_to_routing_table() {
other => panic!("Unexpected events: {other:?}"),
}

server1.listen().await;
server1.listen().with_memory_addr_external().await;
server2.connect(&mut server1).await;

async_std::task::spawn(server1.loop_on_next());
Expand All @@ -79,14 +79,11 @@ async fn adding_an_external_addresses_activates_server_mode_on_existing_connecti

let (memory_addr, _) = server.listen().await;

// Remove memory address to simulate a server that doesn't know its external address.
server.remove_external_address(&memory_addr);
client.dial(memory_addr.clone()).unwrap();
// Do the usual identify send/receive dance. This triggers a mode change to Mode::Client.

// Do the usual identify send/receive dance.
match libp2p_swarm_test::drive(&mut client, &mut server).await {
([Identify(_), Identify(_)], [Kad(ModeChanged { new_mode }), Identify(_), Identify(_)]) => {
assert_eq!(new_mode, Mode::Client);
}
([Identify(_), Identify(_)], [Identify(_), Identify(_)]) => {}
other => panic!("Unexpected events: {other:?}"),
}

Expand Down Expand Up @@ -115,7 +112,7 @@ async fn set_client_to_server_mode() {

let mut server = Swarm::new_ephemeral(MyBehaviour::new);

server.listen().await;
server.listen().with_memory_addr_external().await;
client.connect(&mut server).await;

let server_peer_id = *server.local_peer_id();
Expand Down
2 changes: 1 addition & 1 deletion protocols/perf/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async fn perf() {
let server_peer_id = *server.local_peer_id();
let mut client = Swarm::new_ephemeral(|_| client::Behaviour::new());

server.listen().await;
server.listen().with_memory_addr_external().await;
client.connect(&mut server).await;

tokio::task::spawn(server.loop_on_next());
Expand Down
4 changes: 2 additions & 2 deletions protocols/ping/tests/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn ping_pong() {
let mut swarm2 = Swarm::new_ephemeral(|_| ping::Behaviour::new(cfg.clone()));

async_std::task::block_on(async {
swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

for _ in 0..count.get() {
Expand Down Expand Up @@ -67,7 +67,7 @@ fn unsupported_doesnt_fail() {
let mut swarm2 = Swarm::new_ephemeral(|_| ping::Behaviour::new(ping::Config::new()));

let result = async_std::task::block_on(async {
swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;
let swarm1_peer_id = *swarm1.local_peer_id();
async_std::task::spawn(swarm1.loop_on_next());
Expand Down
8 changes: 4 additions & 4 deletions protocols/rendezvous/tests/rendezvous.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,15 +429,15 @@ async fn new_server_with_connected_clients<const N: usize>(

async fn new_client() -> Swarm<rendezvous::client::Behaviour> {
let mut client = Swarm::new_ephemeral(rendezvous::client::Behaviour::new);
client.listen().await; // we need to listen otherwise we don't have addresses to register
client.listen().with_memory_addr_external().await; // we need to listen otherwise we don't have addresses to register

client
}

async fn new_server(config: rendezvous::server::Config) -> Swarm<rendezvous::server::Behaviour> {
let mut server = Swarm::new_ephemeral(|_| rendezvous::server::Behaviour::new(config));

server.listen().await;
server.listen().with_memory_addr_external().await;

server
}
Expand All @@ -447,7 +447,7 @@ async fn new_combined_node() -> Swarm<Combined> {
client: rendezvous::client::Behaviour::new(identity),
server: rendezvous::server::Behaviour::new(rendezvous::server::Config::default()),
});
node.listen().await;
node.listen().with_memory_addr_external().await;

node
}
Expand All @@ -458,7 +458,7 @@ async fn new_impersonating_client() -> Swarm<rendezvous::client::Behaviour> {
// As such, the best we can do is hand eve a completely different keypair from what she is using to authenticate her connection.
let someone_else = identity::Keypair::generate_ed25519();
let mut eve = Swarm::new_ephemeral(move |_| rendezvous::client::Behaviour::new(someone_else));
eve.listen().await;
eve.listen().with_memory_addr_external().await;

eve
}
Expand Down
12 changes: 6 additions & 6 deletions protocols/request-response/tests/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ async fn report_outbound_failure_on_read_response() {
let (peer1_id, mut swarm1) = new_swarm();
let (peer2_id, mut swarm2) = new_swarm();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

let server_task = async move {
Expand Down Expand Up @@ -75,7 +75,7 @@ async fn report_outbound_failure_on_write_request() {
let (peer1_id, mut swarm1) = new_swarm();
let (_peer2_id, mut swarm2) = new_swarm();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

// Expects no events because `Event::Request` is produced after `read_request`.
Expand Down Expand Up @@ -117,7 +117,7 @@ async fn report_outbound_timeout_on_read_response() {
let (peer1_id, mut swarm1) = new_swarm_with_timeout(Duration::from_millis(200));
let (peer2_id, mut swarm2) = new_swarm_with_timeout(Duration::from_millis(100));

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

let server_task = async move {
Expand Down Expand Up @@ -161,7 +161,7 @@ async fn report_inbound_failure_on_read_request() {
let (peer1_id, mut swarm1) = new_swarm();
let (_peer2_id, mut swarm2) = new_swarm();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

// Expects no events because `Event::Request` is produced after `read_request`.
Expand Down Expand Up @@ -196,7 +196,7 @@ async fn report_inbound_failure_on_write_response() {
let (peer1_id, mut swarm1) = new_swarm();
let (peer2_id, mut swarm2) = new_swarm();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

// Expects OutboundFailure::Io failure with `FailOnWriteResponse` error
Expand Down Expand Up @@ -261,7 +261,7 @@ async fn report_inbound_timeout_on_write_response() {
let (peer1_id, mut swarm1) = new_swarm_with_timeout(Duration::from_millis(100));
let (peer2_id, mut swarm2) = new_swarm_with_timeout(Duration::from_millis(200));

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

// Expects InboundFailure::Timeout
Expand Down
6 changes: 3 additions & 3 deletions protocols/request-response/tests/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ async fn ping_protocol() {
});
let peer2_id = *swarm2.local_peer_id();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

let expected_ping = ping.clone();
Expand Down Expand Up @@ -190,7 +190,7 @@ async fn emits_inbound_connection_closed_failure() {
});
let peer2_id = *swarm2.local_peer_id();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

swarm2.behaviour_mut().send_request(&peer1_id, ping.clone());
Expand Down Expand Up @@ -255,7 +255,7 @@ async fn emits_inbound_connection_closed_if_channel_is_dropped() {
});
let peer2_id = *swarm2.local_peer_id();

swarm1.listen().await;
swarm1.listen().with_memory_addr_external().await;
swarm2.connect(&mut swarm1).await;

swarm2.behaviour_mut().send_request(&peer1_id, ping.clone());
Expand Down
Loading

0 comments on commit 9bd127f

Please sign in to comment.