fix(swarm): don't report NewExternalAddrCandidate if already confir…

…med (#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

<!-- Please add a Changelog entry in the appropriate crates and bump the
crate versions if needed. See
<https://github.com/libp2p/rust-libp2p/blob/master/docs/release.md#development-between-releases>-->

- [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 <[email protected]>
Co-authored-by: Guillaume Michel <[email protected]>

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" }

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::<Vec<_>>());
-    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<CombinedClient> {
             identity.public().clone(),
         )),
     });
-    node.listen().with_tcp_addr_external().await;
+    node.listen().await;
     node
 }
 
@@ -490,13 +484,6 @@ async fn bootstrap() -> (Swarm<CombinedServer>, Swarm<CombinedClient>) {
     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<CombinedServer>, Swarm<CombinedClient>) {
     tokio::join!(alice_task, bob_task);
     (alice, bob)
 }
+
+fn tcp_listeners<T: NetworkBehaviour>(swarm: &Swarm<T>) -> Vec<Multiaddr> {
+    swarm
+        .listeners()
+        .filter(|addr| {
+            addr.iter()
+                .any(|protocol| matches!(protocol, Protocol::Tcp(_)))
+        })
+        .cloned()
+        .collect::<Vec<_>>()
+}

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]

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 <[email protected]>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

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());