From 8ceadaac5aec4b462463ef4082d6af577a3158b1 Mon Sep 17 00:00:00 2001 From: stormshield-frb <144998884+stormshield-frb@users.noreply.github.com> Date: Thu, 26 Sep 2024 16:17:47 +0200 Subject: [PATCH] fix(swarm): don't report `NewExternalAddrCandidate` if already confirmed (#5582) ## Description Currently, `NewExternalAddrCandidate` events are emitted for every connections. However, we continue to get this event even when `autonat` has already confirmed that this address is external. So we should not continue to advertise the "candidate" event. ## Notes & open questions We have made the changes in the `swarm` instead of `identify` because it does not make it necessary to duplicate the `ConfirmedExternalAddr` vector in the `identify` Behaviour. Moreover, if any future Behaviour emit `NewExternalAddrCandidate`, the same rule will also be applied. I had to edit the `autonat_v2` tests which were always expecting a `NewExternalAddrCandidate` but the address was already confirmed. ## Change checklist - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have added tests that prove my fix is effective or that my feature works - [x] A changelog entry has been made in the appropriate crates --------- Co-authored-by: Darius Clark Co-authored-by: Guillaume Michel --- Cargo.lock | 2 +- Cargo.toml | 2 +- protocols/autonat/tests/autonatv2.rs | 38 +++++++++++++--------------- swarm/CHANGELOG.md | 5 ++++ swarm/Cargo.toml | 2 +- swarm/src/lib.rs | 14 +++++----- 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c3498a0635..79eb2eb4ad5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3329,7 +3329,7 @@ dependencies = [ [[package]] name = "libp2p-swarm" -version = "0.45.1" +version = "0.45.2" dependencies = [ "async-std", "criterion", diff --git a/Cargo.toml b/Cargo.toml index c9fe928096d..32f72c5e252 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,7 +102,7 @@ libp2p-rendezvous = { version = "0.15.0", path = "protocols/rendezvous" } libp2p-request-response = { version = "0.27.0", path = "protocols/request-response" } libp2p-server = { version = "0.12.7", path = "misc/server" } libp2p-stream = { version = "0.2.0-alpha", path = "protocols/stream" } -libp2p-swarm = { version = "0.45.1", path = "swarm" } +libp2p-swarm = { version = "0.45.2", path = "swarm" } libp2p-swarm-derive = { version = "=0.35.0", path = "swarm-derive" } # `libp2p-swarm-derive` may not be compatible with different `libp2p-swarm` non-breaking releases. E.g. `libp2p-swarm` might introduce a new enum variant `FromSwarm` (which is `#[non-exhaustive]`) in a non-breaking release. Older versions of `libp2p-swarm-derive` would not forward this enum variant within the `NetworkBehaviour` hierarchy. Thus the version pinning is required. libp2p-swarm-test = { version = "0.4.0", path = "swarm-test" } libp2p-tcp = { version = "0.42.0", path = "transports/tcp" } diff --git a/protocols/autonat/tests/autonatv2.rs b/protocols/autonat/tests/autonatv2.rs index abd0c4bd8eb..3d792172445 100644 --- a/protocols/autonat/tests/autonatv2.rs +++ b/protocols/autonat/tests/autonatv2.rs @@ -1,5 +1,6 @@ use libp2p_autonat::v2::client::{self, Config}; use libp2p_autonat::v2::server; +use libp2p_core::multiaddr::Protocol; use libp2p_core::transport::TransportError; use libp2p_core::Multiaddr; use libp2p_swarm::{ @@ -21,17 +22,10 @@ async fn confirm_successful() { let cor_server_peer = *alice.local_peer_id(); let cor_client_peer = *bob.local_peer_id(); - let bob_external_addrs = Arc::new(bob.external_addresses().cloned().collect::>()); - let alice_bob_external_addrs = bob_external_addrs.clone(); + let bob_tcp_listeners = Arc::new(tcp_listeners(&bob)); + let alice_bob_tcp_listeners = bob_tcp_listeners.clone(); let alice_task = async { - let _ = alice - .wait(|event| match event { - SwarmEvent::NewExternalAddrCandidate { .. } => Some(()), - _ => None, - }) - .await; - let (dialed_peer_id, dialed_connection_id) = alice .wait(|event| match event { SwarmEvent::Dialing { @@ -76,10 +70,10 @@ async fn confirm_successful() { }) .await; - assert_eq!(tested_addr, bob_external_addrs.first().cloned().unwrap()); + assert_eq!(tested_addr, bob_tcp_listeners.first().cloned().unwrap()); assert_eq!(data_amount, 0); assert_eq!(client, cor_client_peer); - assert_eq!(&all_addrs[..], &bob_external_addrs[..]); + assert_eq!(&all_addrs[..], &bob_tcp_listeners[..]); assert!(result.is_ok(), "Result: {result:?}"); }; @@ -122,7 +116,7 @@ async fn confirm_successful() { .await; assert_eq!( tested_addr, - alice_bob_external_addrs.first().cloned().unwrap() + alice_bob_tcp_listeners.first().cloned().unwrap() ); assert_eq!(bytes_sent, 0); assert_eq!(server, cor_server_peer); @@ -446,7 +440,7 @@ async fn new_client() -> Swarm { identity.public().clone(), )), }); - node.listen().with_tcp_addr_external().await; + node.listen().await; node } @@ -490,13 +484,6 @@ async fn bootstrap() -> (Swarm, Swarm) { let cor_client_peer = *bob.local_peer_id(); let alice_task = async { - let _ = alice - .wait(|event| match event { - SwarmEvent::NewExternalAddrCandidate { .. } => Some(()), - _ => None, - }) - .await; - let (dialed_peer_id, dialed_connection_id) = alice .wait(|event| match event { SwarmEvent::Dialing { @@ -566,3 +553,14 @@ async fn bootstrap() -> (Swarm, Swarm) { tokio::join!(alice_task, bob_task); (alice, bob) } + +fn tcp_listeners(swarm: &Swarm) -> Vec { + swarm + .listeners() + .filter(|addr| { + addr.iter() + .any(|protocol| matches!(protocol, Protocol::Tcp(_))) + }) + .cloned() + .collect::>() +} diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index e7931a60de2..c5d10872d40 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.45.2 + +- Don't report `NewExternalAddrCandidate` for confirmed external addresses. + See [PR 5582](https://github.com/libp2p/rust-libp2p/pull/5582). + ## 0.45.1 - Update `libp2p-swarm-derive` to version `0.35.0`, see [PR 5545] diff --git a/swarm/Cargo.toml b/swarm/Cargo.toml index 3d0b1a84eee..cdee67f3fb3 100644 --- a/swarm/Cargo.toml +++ b/swarm/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-swarm" edition = "2021" rust-version = { workspace = true } description = "The libp2p swarm" -version = "0.45.1" +version = "0.45.2" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 81b1ca1a68d..12280e99f07 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1140,12 +1140,14 @@ where self.pending_handler_event = Some((peer_id, handler, event)); } ToSwarm::NewExternalAddrCandidate(addr) => { - self.behaviour - .on_swarm_event(FromSwarm::NewExternalAddrCandidate( - NewExternalAddrCandidate { addr: &addr }, - )); - self.pending_swarm_events - .push_back(SwarmEvent::NewExternalAddrCandidate { address: addr }); + if !self.confirmed_external_addr.contains(&addr) { + self.behaviour + .on_swarm_event(FromSwarm::NewExternalAddrCandidate( + NewExternalAddrCandidate { addr: &addr }, + )); + self.pending_swarm_events + .push_back(SwarmEvent::NewExternalAddrCandidate { address: addr }); + } } ToSwarm::ExternalAddrConfirmed(addr) => { self.add_external_address(addr.clone());