From c4de992690ae0511d5b611f7953a3da63e516ce1 Mon Sep 17 00:00:00 2001 From: Darius Date: Thu, 12 Oct 2023 02:09:03 -0400 Subject: [PATCH 1/9] feat: Refresh registration upon a change in external addresses. --- Cargo.lock | 2 +- Cargo.toml | 2 +- protocols/rendezvous/CHANGELOG.md | 6 +++ protocols/rendezvous/Cargo.toml | 2 +- protocols/rendezvous/src/client.rs | 22 ++++++++- protocols/rendezvous/tests/rendezvous.rs | 58 ++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96f5b8e73c3..1ee1cac0693 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2975,7 +2975,7 @@ dependencies = [ [[package]] name = "libp2p-rendezvous" -version = "0.13.0" +version = "0.13.1" dependencies = [ "async-trait", "asynchronous-codec", diff --git a/Cargo.toml b/Cargo.toml index 5334d279963..b7bb860a8f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -97,7 +97,7 @@ libp2p-plaintext = { version = "0.40.1", path = "transports/plaintext" } libp2p-pnet = { version = "0.23.0", path = "transports/pnet" } libp2p-quic = { version = "0.9.2", path = "transports/quic" } libp2p-relay = { version = "0.16.1", path = "protocols/relay" } -libp2p-rendezvous = { version = "0.13.0", path = "protocols/rendezvous" } +libp2p-rendezvous = { version = "0.13.1", path = "protocols/rendezvous" } libp2p-upnp = { version = "0.1.1", path = "protocols/upnp" } libp2p-request-response = { version = "0.25.1", path = "protocols/request-response" } libp2p-server = { version = "0.12.3", path = "misc/server" } diff --git a/protocols/rendezvous/CHANGELOG.md b/protocols/rendezvous/CHANGELOG.md index 15438502daa..525a812ffb3 100644 --- a/protocols/rendezvous/CHANGELOG.md +++ b/protocols/rendezvous/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.13.1 +- Refresh registration upon a change in external addresses. + See [PR XXXX] + +[PR XXXX]: https://github.com/libp2p/rust-libp2p/pull/XXXX + ## 0.13.0 - Changed the signature of the function `client::Behavior::register()`, diff --git a/protocols/rendezvous/Cargo.toml b/protocols/rendezvous/Cargo.toml index 3a61c21ffa2..0225f915346 100644 --- a/protocols/rendezvous/Cargo.toml +++ b/protocols/rendezvous/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-rendezvous" edition = "2021" rust-version = { workspace = true } description = "Rendezvous protocol for libp2p" -version = "0.13.0" +version = "0.13.1" authors = ["The COMIT guys "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/rendezvous/src/client.rs b/protocols/rendezvous/src/client.rs index 505635efda8..663eb0d8ed4 100644 --- a/protocols/rendezvous/src/client.rs +++ b/protocols/rendezvous/src/client.rs @@ -49,6 +49,8 @@ pub struct Behaviour { /// Storing these internally allows us to assist the [`libp2p_swarm::Swarm`] in dialing by returning addresses from [`NetworkBehaviour::handle_pending_outbound_connection`]. discovered_peers: HashMap<(PeerId, Namespace), Vec>, + registered_namespaces: HashMap<(PeerId, Namespace), Ttl>, + /// Tracks the expiry of registrations that we have discovered and stored in `discovered_peers` otherwise we have a memory leak. expiring_registrations: FuturesUnordered>, @@ -68,6 +70,7 @@ impl Behaviour { waiting_for_register: Default::default(), waiting_for_discovery: Default::default(), discovered_peers: Default::default(), + registered_namespaces: Default::default(), expiring_registrations: FuturesUnordered::from_iter(vec![ futures::future::pending().boxed() ]), @@ -104,7 +107,10 @@ impl Behaviour { /// Unregister ourselves from the given namespace with the given rendezvous peer. pub fn unregister(&mut self, namespace: Namespace, rendezvous_node: PeerId) { self.inner - .send_request(&rendezvous_node, Unregister(namespace)); + .send_request(&rendezvous_node, Unregister(namespace.clone())); + + self.registered_namespaces + .remove(&(rendezvous_node, namespace)); } /// Discover other peers at a given rendezvous peer. @@ -218,9 +224,18 @@ impl NetworkBehaviour for Behaviour { } fn on_swarm_event(&mut self, event: FromSwarm) { - self.external_addresses.on_swarm_event(&event); + let changed = self.external_addresses.on_swarm_event(&event); self.inner.on_swarm_event(event); + + if changed && self.external_addresses.iter().count() > 0 { + let registered = self.registered_namespaces.clone(); + for ((rz_node, ns), ttl) in registered { + if let Err(e) = self.register(ns.clone(), rz_node, Some(ttl)) { + log::error!("Error refreshing registration: {e}") + } + } + } } fn poll( @@ -348,6 +363,9 @@ impl Behaviour { if let Some((rendezvous_node, namespace)) = self.waiting_for_register.remove(request_id) { + self.registered_namespaces + .insert((rendezvous_node, namespace.clone()), ttl); + return Some(Event::Registered { rendezvous_node, ttl, diff --git a/protocols/rendezvous/tests/rendezvous.rs b/protocols/rendezvous/tests/rendezvous.rs index 992876d1971..21a3f5ffe04 100644 --- a/protocols/rendezvous/tests/rendezvous.rs +++ b/protocols/rendezvous/tests/rendezvous.rs @@ -20,6 +20,8 @@ use futures::stream::FuturesUnordered; use futures::StreamExt; +use libp2p_core::multiaddr::Protocol; +use libp2p_core::Multiaddr; use libp2p_identity as identity; use libp2p_rendezvous as rendezvous; use libp2p_rendezvous::client::RegisterError; @@ -162,6 +164,62 @@ async fn given_successful_registration_then_refresh_ttl() { } } +#[tokio::test] +async fn given_successful_registration_then_refresh_external_addrs() { + let _ = env_logger::try_init(); + let namespace = rendezvous::Namespace::from_static("some-namespace"); + let ([mut alice, mut bob], mut robert) = + new_server_with_connected_clients(rendezvous::server::Config::default()).await; + + let roberts_peer_id = *robert.local_peer_id(); + + alice + .behaviour_mut() + .register(namespace.clone(), roberts_peer_id, None) + .unwrap(); + + match libp2p_swarm_test::drive(&mut alice, &mut robert).await { + ( + [rendezvous::client::Event::Registered { .. }], + [rendezvous::server::Event::PeerRegistered { .. }], + ) => {} + events => panic!("Unexpected events: {events:?}"), + } + + bob.behaviour_mut() + .discover(Some(namespace.clone()), None, None, roberts_peer_id); + + match libp2p_swarm_test::drive(&mut bob, &mut robert).await { + ( + [rendezvous::client::Event::Discovered { .. }], + [rendezvous::server::Event::DiscoverServed { .. }], + ) => {} + events => panic!("Unexpected events: {events:?}"), + } + + let external_addr = Multiaddr::empty().with(Protocol::Memory(0)); + + alice.add_external_address(external_addr.clone()); + + match libp2p_swarm_test::drive(&mut alice, &mut robert).await { + ( + [rendezvous::client::Event::Registered { .. }], + [rendezvous::server::Event::PeerRegistered { .. }], + ) => {} + events => panic!("Unexpected events: {events:?}"), + } + + alice.remove_external_address(&external_addr); + + match libp2p_swarm_test::drive(&mut alice, &mut robert).await { + ( + [rendezvous::client::Event::Registered { .. }], + [rendezvous::server::Event::PeerRegistered { .. }], + ) => {} + events => panic!("Unexpected events: {events:?}"), + } +} + #[tokio::test] async fn given_invalid_ttl_then_unsuccessful_registration() { let _ = env_logger::try_init(); From eaca1d2044740afc5bde9b902730f68baeabdbce Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 02:23:36 -0400 Subject: [PATCH 2/9] chore: Update CHANGELOG.md --- protocols/rendezvous/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/protocols/rendezvous/CHANGELOG.md b/protocols/rendezvous/CHANGELOG.md index 525a812ffb3..b480d1e105e 100644 --- a/protocols/rendezvous/CHANGELOG.md +++ b/protocols/rendezvous/CHANGELOG.md @@ -1,8 +1,8 @@ -## 0.13.1 +## 0.13.1 - unreleased - Refresh registration upon a change in external addresses. - See [PR XXXX] + See [PR 4629] -[PR XXXX]: https://github.com/libp2p/rust-libp2p/pull/XXXX +[PR 4629]: https://github.com/libp2p/rust-libp2p/pull/4629 ## 0.13.0 From 1da1418d18cf5e3116e09c6c391c98183225be75 Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 02:28:51 -0400 Subject: [PATCH 3/9] chore: Remove redundant clone --- protocols/rendezvous/src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/rendezvous/src/client.rs b/protocols/rendezvous/src/client.rs index 663eb0d8ed4..0c9fcc040bd 100644 --- a/protocols/rendezvous/src/client.rs +++ b/protocols/rendezvous/src/client.rs @@ -231,7 +231,7 @@ impl NetworkBehaviour for Behaviour { if changed && self.external_addresses.iter().count() > 0 { let registered = self.registered_namespaces.clone(); for ((rz_node, ns), ttl) in registered { - if let Err(e) = self.register(ns.clone(), rz_node, Some(ttl)) { + if let Err(e) = self.register(ns, rz_node, Some(ttl)) { log::error!("Error refreshing registration: {e}") } } From 2cfa2e51ae57b5126cdc6d6fd320a9ff0142ca73 Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 02:32:02 -0400 Subject: [PATCH 4/9] chore: Update test --- protocols/rendezvous/tests/rendezvous.rs | 25 ++++++++++-------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/protocols/rendezvous/tests/rendezvous.rs b/protocols/rendezvous/tests/rendezvous.rs index 21a3f5ffe04..cd9515bb71c 100644 --- a/protocols/rendezvous/tests/rendezvous.rs +++ b/protocols/rendezvous/tests/rendezvous.rs @@ -186,17 +186,6 @@ async fn given_successful_registration_then_refresh_external_addrs() { events => panic!("Unexpected events: {events:?}"), } - bob.behaviour_mut() - .discover(Some(namespace.clone()), None, None, roberts_peer_id); - - match libp2p_swarm_test::drive(&mut bob, &mut robert).await { - ( - [rendezvous::client::Event::Discovered { .. }], - [rendezvous::server::Event::DiscoverServed { .. }], - ) => {} - events => panic!("Unexpected events: {events:?}"), - } - let external_addr = Multiaddr::empty().with(Protocol::Memory(0)); alice.add_external_address(external_addr.clone()); @@ -204,8 +193,11 @@ async fn given_successful_registration_then_refresh_external_addrs() { match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( [rendezvous::client::Event::Registered { .. }], - [rendezvous::server::Event::PeerRegistered { .. }], - ) => {} + [rendezvous::server::Event::PeerRegistered { registration, .. }], + ) => { + let record = registration.record; + assert!(record.addresses().contains(&external_addr)); + } events => panic!("Unexpected events: {events:?}"), } @@ -214,8 +206,11 @@ async fn given_successful_registration_then_refresh_external_addrs() { match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( [rendezvous::client::Event::Registered { .. }], - [rendezvous::server::Event::PeerRegistered { .. }], - ) => {} + [rendezvous::server::Event::PeerRegistered { registration, .. }], + ) => { + let record = registration.record; + assert!(!record.addresses().contains(&external_addr)); + } events => panic!("Unexpected events: {events:?}"), } } From d34f5fcbfd17b5215d196480747f879b72da501c Mon Sep 17 00:00:00 2001 From: Darius Clark Date: Fri, 13 Oct 2023 02:33:40 -0400 Subject: [PATCH 5/9] Update protocols/rendezvous/CHANGELOG.md --- protocols/rendezvous/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/rendezvous/CHANGELOG.md b/protocols/rendezvous/CHANGELOG.md index b480d1e105e..7f204c1c07b 100644 --- a/protocols/rendezvous/CHANGELOG.md +++ b/protocols/rendezvous/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.13.1 - unreleased - Refresh registration upon a change in external addresses. - See [PR 4629] + See [PR 4629]. [PR 4629]: https://github.com/libp2p/rust-libp2p/pull/4629 From 5d8b7675bfed25a44f08af66396ddf906bb3d4b6 Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 02:43:39 -0400 Subject: [PATCH 6/9] chore: Remove from map first to avoid cloning --- protocols/rendezvous/src/client.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/protocols/rendezvous/src/client.rs b/protocols/rendezvous/src/client.rs index 0c9fcc040bd..c16d015068c 100644 --- a/protocols/rendezvous/src/client.rs +++ b/protocols/rendezvous/src/client.rs @@ -106,11 +106,11 @@ impl Behaviour { /// Unregister ourselves from the given namespace with the given rendezvous peer. pub fn unregister(&mut self, namespace: Namespace, rendezvous_node: PeerId) { - self.inner - .send_request(&rendezvous_node, Unregister(namespace.clone())); - self.registered_namespaces - .remove(&(rendezvous_node, namespace)); + .retain(|(rz_node, ns), _| rz_node.ne(&rendezvous_node) && ns.ne(&namespace)); + + self.inner + .send_request(&rendezvous_node, Unregister(namespace)); } /// Discover other peers at a given rendezvous peer. From 6e1e3af617379de7c609f9658094854db0038d28 Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 03:02:16 -0400 Subject: [PATCH 7/9] chore: Remove unused var --- protocols/rendezvous/tests/rendezvous.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/rendezvous/tests/rendezvous.rs b/protocols/rendezvous/tests/rendezvous.rs index cd9515bb71c..59535bf2a2e 100644 --- a/protocols/rendezvous/tests/rendezvous.rs +++ b/protocols/rendezvous/tests/rendezvous.rs @@ -168,7 +168,7 @@ async fn given_successful_registration_then_refresh_ttl() { async fn given_successful_registration_then_refresh_external_addrs() { let _ = env_logger::try_init(); let namespace = rendezvous::Namespace::from_static("some-namespace"); - let ([mut alice, mut bob], mut robert) = + let ([mut alice, _], mut robert) = new_server_with_connected_clients(rendezvous::server::Config::default()).await; let roberts_peer_id = *robert.local_peer_id(); From 5616dfd803f00a08b365d05d8d1f1b71705c3701 Mon Sep 17 00:00:00 2001 From: Darius Date: Fri, 13 Oct 2023 03:06:36 -0400 Subject: [PATCH 8/9] chore: Minor change --- protocols/rendezvous/tests/rendezvous.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/rendezvous/tests/rendezvous.rs b/protocols/rendezvous/tests/rendezvous.rs index 59535bf2a2e..64cd294eefa 100644 --- a/protocols/rendezvous/tests/rendezvous.rs +++ b/protocols/rendezvous/tests/rendezvous.rs @@ -168,7 +168,7 @@ async fn given_successful_registration_then_refresh_ttl() { async fn given_successful_registration_then_refresh_external_addrs() { let _ = env_logger::try_init(); let namespace = rendezvous::Namespace::from_static("some-namespace"); - let ([mut alice, _], mut robert) = + let ([mut alice], mut robert) = new_server_with_connected_clients(rendezvous::server::Config::default()).await; let roberts_peer_id = *robert.local_peer_id(); From 90303b9cb5fded79ad5042bc433021a17c210240 Mon Sep 17 00:00:00 2001 From: Darius Date: Sun, 15 Oct 2023 03:42:13 -0400 Subject: [PATCH 9/9] chore: downgrade log level to warning --- protocols/rendezvous/src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/rendezvous/src/client.rs b/protocols/rendezvous/src/client.rs index c16d015068c..8459dc21c7e 100644 --- a/protocols/rendezvous/src/client.rs +++ b/protocols/rendezvous/src/client.rs @@ -232,7 +232,7 @@ impl NetworkBehaviour for Behaviour { let registered = self.registered_namespaces.clone(); for ((rz_node, ns), ttl) in registered { if let Err(e) = self.register(ns, rz_node, Some(ttl)) { - log::error!("Error refreshing registration: {e}") + log::warn!("refreshing registration failed: {e}") } } }