fix(mdns): Don't expire mDNS nodes on connection close (#3367)

mDNS records should not be expiring when an unrelated connection timeout with said peer is reached.

Fixes #3309.

protocols/mdns/CHANGELOG.md

@@ -6,7 +6,12 @@
 
 - Update to `libp2p-swarm` `v0.42.0`.
 
+- Don't expire mDNS records when the last connection was closed.
+  mDNS records will only be expired when the TTL is reached and the DNS record is no longer valid.
+  See [PR 3367].
+
 [PR 3153]: https://github.com/libp2p/rust-libp2p/pull/3153
+[PR 3367]: https://github.com/libp2p/rust-libp2p/pull/3367
 
 # 0.42.0
 

protocols/mdns/src/behaviour.rs

@@ -28,7 +28,7 @@ use crate::Config;
 use futures::Stream;
 use if_watch::IfEvent;
 use libp2p_core::{Multiaddr, PeerId};
-use libp2p_swarm::behaviour::{ConnectionClosed, FromSwarm};
+use libp2p_swarm::behaviour::FromSwarm;
 use libp2p_swarm::{
     dummy, ListenAddresses, NetworkBehaviour, NetworkBehaviourAction, PollParameters,
     THandlerOutEvent,
@@ -199,22 +199,14 @@ where
         self.listen_addresses.on_swarm_event(&event);
 
         match event {
-            FromSwarm::ConnectionClosed(ConnectionClosed {
-                peer_id,
-                remaining_established,
-                ..
-            }) => {
-                if remaining_established == 0 {
-                    self.expire_node(&peer_id);
-                }
-            }
             FromSwarm::NewListener(_) => {
                 log::trace!("waking interface state because listening address changed");
                 for iface in self.iface_states.values_mut() {
                     iface.fire_timer();
                 }
             }
-            FromSwarm::ConnectionEstablished(_)
+            FromSwarm::ConnectionClosed(_)
+            | FromSwarm::ConnectionEstablished(_)
             | FromSwarm::DialFailure(_)
             | FromSwarm::AddressChange(_)
             | FromSwarm::ListenFailure(_)

protocols/mdns/tests/use-async-std.rs

@@ -55,6 +55,24 @@ async fn test_expired_async_std() -> Result<(), Box<dyn Error>> {
         .map_err(|e| Box::new(e) as Box<dyn Error>)
 }
 
+#[async_std::test]
+async fn test_no_expiration_on_close_async_std() -> Result<(), Box<dyn Error>> {
+    env_logger::try_init().ok();
+    let config = Config {
+        ttl: Duration::from_secs(120),
+        query_interval: Duration::from_secs(10),
+        ..Default::default()
+    };
+
+    async_std::future::timeout(
+        Duration::from_secs(6),
+        run_no_expiration_on_close_test(config),
+    )
+    .await
+    .map(|_| ())
+    .map_err(|e| Box::new(e) as Box<dyn Error>)
+}
+
 async fn create_swarm(config: Config) -> Result<Swarm<Behaviour>, Box<dyn Error>> {
     let id_keys = identity::Keypair::generate_ed25519();
     let peer_id = PeerId::from(id_keys.public());
@@ -126,3 +144,66 @@ async fn run_peer_expiration_test(config: Config) -> Result<(), Box<dyn Error>>
         }
     }
 }
+
+async fn run_no_expiration_on_close_test(config: Config) -> Result<(), Box<dyn Error>> {
+    let mut a = create_swarm(config.clone()).await?;
+    let mut b = create_swarm(config).await?;
+
+    #[derive(PartialEq)]
+    enum State {
+        Initial,
+        Dialed,
+        Closed,
+    }
+
+    let mut state = State::Initial;
+
+    loop {
+        futures::select! {
+            ev = a.select_next_some() => match ev {
+                SwarmEvent::Behaviour(Event::Discovered(peers)) => {
+                    if state == State::Initial {
+                        for (peer, addr) in peers {
+                            if peer == *b.local_peer_id() {
+                                // Connect to all addresses of b to 'expire' all of them
+                                a.dial(addr)?;
+                                state = State::Dialed;
+                                break;
+                            }
+                        }
+                    }
+                }
+                SwarmEvent::ConnectionEstablished { peer_id, .. } => {
+                    if peer_id == *b.local_peer_id() {
+                        if state == State::Dialed {
+                            // We disconnect the connection that was initiated
+                            // in the discovered event
+                            a.disconnect_peer_id(peer_id).unwrap();
+                        } else if state == State::Closed {
+                            // If the connection attempt after connection close
+                            // succeeded the mDNS record wasn't expired by
+                            // connection close
+                            return Ok(())
+                        }
+                    }
+                }
+                SwarmEvent::ConnectionClosed { peer_id, num_established, .. } => {
+                    if peer_id == *b.local_peer_id() && num_established == 0 {
+                        // Dial a second time to make sure connection is still
+                        // possible only via the peer id
+                        state = State::Closed;
+
+                        // Either wait for the expiration event to give mDNS enough time to expire
+                        // or timeout after 1 second of not receiving the expiration event
+                        let _ = async_std::future::timeout(Duration::from_secs(1), a.select_next_some()).await;
+
+                        // If the record expired this will fail because the peer has no addresses
+                        a.dial(peer_id).expect("Expected peer addresses to not expire after connection close");
+                    }
+                }
+                _ => {}
+            },
+            _ = b.select_next_some() => {}
+        }
+    }
+}