fix(kad): potentially incorrect return value of Addresses::remove

Cargo.toml

@@ -82,7 +82,7 @@ libp2p-floodsub = { version = "0.44.0", path = "protocols/floodsub" }
 libp2p-gossipsub = { version = "0.46.0", path = "protocols/gossipsub" }
 libp2p-identify = { version = "0.44.0", path = "protocols/identify" }
 libp2p-identity = { version = "0.2.7" }
-libp2p-kad = { version = "0.45.0", path = "protocols/kad" }
+libp2p-kad = { version = "0.45.1", path = "protocols/kad" }
 libp2p-mdns = { version = "0.45.0", path = "protocols/mdns" }
 libp2p-memory-connection-limits = { version = "0.2.0", path = "misc/memory-connection-limits" }
 libp2p-metrics = { version = "0.14.0", path = "misc/metrics" }

protocols/kad/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.45.1
+
+- Fix a bug where calling `Behaviour::remove_address` with an address not in the peer's bucket would remove the peer from the routing table if the bucket has only one address left.
+  See [PR 4816](https://github.com/libp2p/rust-libp2p/pull/4816)
+
 ## 0.45.0
 
 - Remove deprecated `kad::Config::set_connection_idle_timeout` in favor of `SwarmBuilder::idle_connection_timeout`.

protocols/kad/Cargo.toml

@@ -3,7 +3,7 @@ name = "libp2p-kad"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Kademlia protocol for libp2p"
-version = "0.45.0"
+version = "0.45.1"
 authors = ["Parity Technologies <[email protected]>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

protocols/kad/src/addresses.rs

@@ -67,7 +67,7 @@ impl Addresses {
     /// otherwise unreachable.
     #[allow(clippy::result_unit_err)]
     pub fn remove(&mut self, addr: &Multiaddr) -> Result<(), ()> {
-        if self.addrs.len() == 1 {
+        if self.addrs.len() == 1 && self.addrs[0] == *addr {
             return Err(());
         }
 
@@ -113,3 +113,67 @@ impl fmt::Debug for Addresses {
         f.debug_list().entries(self.addrs.iter()).finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn given_one_address_when_removing_different_one_returns_ok() {
+        let addrs = &[tcp_addr("1234")];
+        let mut addresses = make_addresses(addrs);
+
+        assert!(addresses.remove(&tcp_addr("4321")).is_ok());
+        // assert that content did not change since the removed value was not present in the addresses list
+        assert_eq!(addresses.addrs.as_slice(), addrs);
+    }
+
+    #[test]
+    fn given_one_address_when_removing_correct_one_returns_err() {
+        let addrs = &[tcp_addr("1234")];
+        let mut addresses = make_addresses(addrs);
+
+        assert!(addresses.remove(&tcp_addr("1234")).is_err());
+        // assert that content did not change since the removed value, while present in the addresses list, was the last one
+        assert_eq!(addresses.addrs.as_slice(), addrs);
+    }
+
+    #[test]
+    fn given_many_addresses_when_removing_different_one_does_not_remove_and_returns_ok() {
+        let addrs = &[tcp_addr("1234"), tcp_addr("4321")];
+        let mut addresses = make_addresses(addrs);
+
+        assert!(addresses.remove(&tcp_addr("5678")).is_ok());
+        // assert that content did not change since the removed value was not present in the addresses list
+        assert_eq!(addresses.addrs.as_slice(), addrs);
+    }
+
+    #[test]
+    fn given_many_addresses_when_removing_correct_one_removes_and_returns_ok() {
+        let mut addresses = make_addresses(&[tcp_addr("1234"), tcp_addr("4321")]);
+
+        assert!(addresses.remove(&tcp_addr("1234")).is_ok());
+        // assert that content did change since the removed value was present in the addresses list and was not the last one
+        assert_eq!(addresses.addrs.as_slice(), &[tcp_addr("4321")]);
+    }
+
+    /// Helper function to easily initialize Addresses struct
+    /// with multiple addresses
+    fn make_addresses(addrs: &[Multiaddr]) -> Addresses {
+        let mut addrs = addrs.into_iter();
+
+        let first = addrs.next().expect("Addresses must have at least one addr");
+        let mut addresses = Addresses::new(first.clone());
+
+        while let Some(addr) = addrs.next() {
+            addresses.insert(addr.clone());
+        }
+
+        addresses
+    }
+
+    /// Helper function to create a tcp Multiaddr with a specific port
+    fn tcp_addr(port: &str) -> Multiaddr {
+        format!("/ip4/127.0.0.1/tcp/{port}").parse().unwrap()
+    }
+}