From 525a3abf31263ef63d3b091cd35e31a942a1fe51 Mon Sep 17 00:00:00 2001 From: Benno Zeeman Date: Fri, 20 Oct 2023 14:49:57 +0200 Subject: [PATCH 1/4] fix: deduplicate autonat add_server addresses fixes #4699 --- protocols/request-response/src/lib.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/protocols/request-response/src/lib.rs b/protocols/request-response/src/lib.rs index 9520a0b686d..04152f6d0d6 100644 --- a/protocols/request-response/src/lib.rs +++ b/protocols/request-response/src/lib.rs @@ -437,8 +437,19 @@ where /// by [`NetworkBehaviour::handle_pending_outbound_connection`]. /// /// Addresses added in this way are only removed by `remove_address`. - pub fn add_address(&mut self, peer: &PeerId, address: Multiaddr) { - self.addresses.entry(*peer).or_default().push(address); + /// + /// Returns true if the address was added, false otherwise (i.e. if the + /// address is already in the list). + pub fn add_address(&mut self, peer: &PeerId, address: Multiaddr) -> bool { + let addrs = self.addresses.entry(*peer).or_default(); + + // Add only if address is not already in the list. + if addrs.iter().all(|a| *a != address) { + addrs.push(address); + true + } else { + false + } } /// Removes an address of a peer previously added via `add_address`. From 98355b6b88ab5cb8f033d1cfb9915876372c25c7 Mon Sep 17 00:00:00 2001 From: Benno Zeeman Date: Mon, 23 Oct 2023 14:39:05 +0200 Subject: [PATCH 2/4] keep peers in HashSet instead of SmallVec This prevents us from adding duplicates into the list of peer addresses we have. --- protocols/request-response/src/lib.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/protocols/request-response/src/lib.rs b/protocols/request-response/src/lib.rs index 04152f6d0d6..05a1b17e990 100644 --- a/protocols/request-response/src/lib.rs +++ b/protocols/request-response/src/lib.rs @@ -326,7 +326,7 @@ where /// reachable addresses, if any. connected: HashMap>, /// Externally managed addresses via `add_address` and `remove_address`. - addresses: HashMap>, + addresses: HashMap>, /// Requests that have not yet been sent and are waiting for a connection /// to be established. pending_outbound_requests: HashMap; 10]>>, @@ -441,15 +441,7 @@ where /// Returns true if the address was added, false otherwise (i.e. if the /// address is already in the list). pub fn add_address(&mut self, peer: &PeerId, address: Multiaddr) -> bool { - let addrs = self.addresses.entry(*peer).or_default(); - - // Add only if address is not already in the list. - if addrs.iter().all(|a| *a != address) { - addrs.push(address); - true - } else { - false - } + self.addresses.entry(*peer).or_default().insert(address) } /// Removes an address of a peer previously added via `add_address`. From 0c4b66761bfdd7b56e06fe56ed61a83e75ef61ce Mon Sep 17 00:00:00 2001 From: Benno Zeeman Date: Mon, 23 Oct 2023 14:39:34 +0200 Subject: [PATCH 3/4] update request-response CHANGELOG.md for #4700 --- protocols/request-response/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/protocols/request-response/CHANGELOG.md b/protocols/request-response/CHANGELOG.md index 872d1320e7e..34dc0704198 100644 --- a/protocols/request-response/CHANGELOG.md +++ b/protocols/request-response/CHANGELOG.md @@ -3,6 +3,9 @@ - Remove `request_response::Config::set_connection_keep_alive` in favor of `SwarmBuilder::idle_connection_timeout`. See [PR 4679](https://github.com/libp2p/rust-libp2p/pull/4679). +- Keep peer addresses in `HashSet` instead of `SmallVec` to prevent adding duplicate addresses. + See [PR 4700](https://github.com/libp2p/rust-libp2p/pull/4700). + ## 0.25.2 - Deprecate `request_response::Config::set_connection_keep_alive` in favor of `SwarmBuilder::idle_connection_timeout`. From f9db317d98b2ca6112dfc96166905779eb42d595 Mon Sep 17 00:00:00 2001 From: Benno Zeeman Date: Tue, 24 Oct 2023 16:47:42 +0200 Subject: [PATCH 4/4] change (&HashSet)::into_iter to equivalent iter --- protocols/request-response/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/request-response/src/lib.rs b/protocols/request-response/src/lib.rs index 05a1b17e990..6503460f672 100644 --- a/protocols/request-response/src/lib.rs +++ b/protocols/request-response/src/lib.rs @@ -734,7 +734,7 @@ where addresses.extend(connections.iter().filter_map(|c| c.remote_address.clone())) } if let Some(more) = self.addresses.get(&peer) { - addresses.extend(more.into_iter().cloned()); + addresses.extend(more.iter().cloned()); } Ok(addresses)