refactor(kad): make Kademlia::add_address return Result

Cargo.toml

@@ -83,7 +83,7 @@ libp2p-floodsub = { version = "0.43.0", path = "protocols/floodsub" }
 libp2p-gossipsub = { version = "0.45.1", path = "protocols/gossipsub" }
 libp2p-identify = { version = "0.43.0", path = "protocols/identify" }
 libp2p-identity = { version = "0.2.3" }
-libp2p-kad = { version = "0.44.5", path = "protocols/kad" }
+libp2p-kad = { version = "0.45.0", path = "protocols/kad" }
 libp2p-mdns = { version = "0.44.0", path = "protocols/mdns" }
 libp2p-memory-connection-limits = { version = "0.1.0", path = "misc/memory-connection-limits" }
 libp2p-metrics = { version = "0.13.1", path = "misc/metrics" }

examples/distributed-key-value-store/src/main.rs

@@ -104,7 +104,7 @@ async fn main() -> Result<(), Box<dyn Error>> {
             },
             SwarmEvent::Behaviour(MyBehaviourEvent::Mdns(mdns::Event::Discovered(list))) => {
                 for (peer_id, multiaddr) in list {
-                    swarm.behaviour_mut().kademlia.add_address(&peer_id, multiaddr);
+                    swarm.behaviour_mut().kademlia.add_address(&peer_id, multiaddr)?;
                 }
             }
             SwarmEvent::Behaviour(MyBehaviourEvent::Kademlia(KademliaEvent::OutboundQueryProgressed { result, ..})) => {

examples/file-sharing/src/network.rs

@@ -355,7 +355,8 @@ impl EventLoop {
                     self.swarm
                         .behaviour_mut()
                         .kademlia
-                        .add_address(&peer_id, peer_addr.clone());
+                        .add_address(&peer_id, peer_addr.clone())
+                        .expect("Adding a peer to the DHT");
                     match self.swarm.dial(peer_addr.with(Protocol::P2p(peer_id))) {
                         Ok(()) => {
                             e.insert(sender);

examples/ipfs-kad/src/main.rs

@@ -60,7 +60,7 @@ async fn main() -> Result<(), Box<dyn Error>> {
         // into the `transport` resolves the `dnsaddr` when Kademlia tries
         // to dial these nodes.
         for peer in &BOOTNODES {
-            behaviour.add_address(&peer.parse()?, "/dnsaddr/bootstrap.libp2p.io".parse()?);
+            behaviour.add_address(&peer.parse()?, "/dnsaddr/bootstrap.libp2p.io".parse()?)?;
         }
 
         SwarmBuilder::with_async_std_executor(transport, behaviour, local_peer_id).build()

misc/server/src/behaviour.rs

@@ -44,7 +44,9 @@ impl Behaviour {
             );
             let bootaddr = Multiaddr::from_str("/dnsaddr/bootstrap.libp2p.io").unwrap();
             for peer in &BOOTNODES {
-                kademlia.add_address(&PeerId::from_str(peer).unwrap(), bootaddr.clone());
+                kademlia
+                    .add_address(&PeerId::from_str(peer).unwrap(), bootaddr.clone())
+                    .expect("to add bootnode to the DHT");
             }
             kademlia.bootstrap().unwrap();
             Some(kademlia)

protocols/kad/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 0.45.0 - unreleased
+
+- make `Kademlia::add_address` return `Result`.
+  See [PR 4280].
+
+[PR 4280]: https://github.com/libp2p/rust-libp2p/pull/4280
+
 ## 0.44.5 - unreleased
 - Migrate to `quick-protobuf-codec` crate for codec logic.
   See [PR 4501].

protocols/kad/Cargo.toml

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

protocols/kad/src/behaviour.rs

@@ -524,7 +524,11 @@ where
     ///
     /// If the routing table has been updated as a result of this operation,
     /// a [`KademliaEvent::RoutingUpdated`] event is emitted.
-    pub fn add_address(&mut self, peer: &PeerId, address: Multiaddr) -> RoutingUpdate {
+    pub fn add_address(
+        &mut self,
+        peer: &PeerId,
+        address: Multiaddr,
+    ) -> Result<RoutingUpdateOk, RoutingUpdateError> {
         let key = kbucket::Key::from(*peer);
         match self.kbuckets.entry(&key) {
             kbucket::Entry::Present(mut entry, _) => {
@@ -543,11 +547,11 @@ where
                         },
                     ))
                 }
-                RoutingUpdate::Success
+                Ok(RoutingUpdateOk::Success)
             }
             kbucket::Entry::Pending(mut entry, _) => {
                 entry.value().insert(address);
-                RoutingUpdate::Pending
+                Ok(RoutingUpdateOk::Pending)
             }
             kbucket::Entry::Absent(entry) => {
                 let addresses = Addresses::new(address);
@@ -571,23 +575,23 @@ where
                                     .expect("Not kbucket::Entry::SelfEntry."),
                             },
                         ));
-                        RoutingUpdate::Success
+                        Ok(RoutingUpdateOk::Success)
                     }
                     kbucket::InsertResult::Full => {
-                        debug!("Bucket full. Peer not added to routing table: {}", peer);
-                        RoutingUpdate::Failed
+                        debug!("Bucket full. Peer not added to routing table: {peer}");
+                        Err(RoutingUpdateError::BucketFull(*peer))
                     }
                     kbucket::InsertResult::Pending { disconnected } => {
                         self.queued_events.push_back(ToSwarm::Dial {
                             opts: DialOpts::peer_id(disconnected.into_preimage())
                                 .condition(dial_opts::PeerCondition::NotDialing)
                                 .build(),
                         });
-                        RoutingUpdate::Pending
+                        Ok(RoutingUpdateOk::Pending)
                     }
                 }
             }
-            kbucket::Entry::SelfEntry => RoutingUpdate::Failed,
+            kbucket::Entry::SelfEntry => Err(RoutingUpdateError::SelfEntry),
         }
     }
 
@@ -3290,9 +3294,9 @@ impl fmt::Display for NoKnownPeers {
 
 impl std::error::Error for NoKnownPeers {}
 
-/// The possible outcomes of [`Kademlia::add_address`].
+/// The possible success outcomes of [`Kademlia::add_address`].
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum RoutingUpdate {
+pub enum RoutingUpdateOk {
     /// The given peer and address has been added to the routing
     /// table.
     Success,
@@ -3302,12 +3306,18 @@ pub enum RoutingUpdate {
     /// in the routing table, [`KademliaEvent::RoutingUpdated`]
     /// is eventually emitted.
     Pending,
-    /// The routing table update failed, either because the
-    /// corresponding bucket for the peer is full and the
-    /// pending slot(s) are occupied, or because the given
-    /// peer ID is deemed invalid (e.g. refers to the local
-    /// peer ID).
-    Failed,
+}
+
+/// The routing table update failed, either because the
+/// corresponding bucket for the peer is full and the
+/// pending slot(s) are occupied, or because the given
+/// peer ID refers to the local peer ID.
+#[derive(Debug, Clone, Error)]
+pub enum RoutingUpdateError {
+    #[error("peer `{0}` is not added to the DHT as the corresponding bucket is full")]
+    BucketFull(PeerId),
+    #[error("cannot add local peer to the DHT")]
+    SelfEntry,
 }
 
 #[derive(PartialEq, Copy, Clone, Debug)]

protocols/kad/src/behaviour/test.rs

@@ -109,7 +109,8 @@ fn build_connected_nodes_with_config(
             swarms[i]
                 .1
                 .behaviour_mut()
-                .add_address(peer_id, addr.clone());
+                .add_address(peer_id, addr.clone())
+                .unwrap();
         }
         if j % step == 0 {
             i += step;
@@ -131,7 +132,12 @@ fn build_fully_connected_nodes_with_config(
 
     for (_addr, swarm) in swarms.iter_mut() {
         for (addr, peer) in &swarm_addr_and_peer_id {
-            swarm.behaviour_mut().add_address(peer, addr.clone());
+            if swarm.local_peer_id() != peer {
+                swarm
+                    .behaviour_mut()
+                    .add_address(peer, addr.clone())
+                    .unwrap();
+            }
         }
     }
 
@@ -327,7 +333,8 @@ fn unresponsive_not_returned_direct() {
     for _ in 0..10 {
         swarms[0]
             .behaviour_mut()
-            .add_address(&PeerId::random(), Protocol::Udp(10u16).into());
+            .add_address(&PeerId::random(), Protocol::Udp(10u16).into())
+            .unwrap();
     }
 
     // Ask first to search a random value.
@@ -373,7 +380,8 @@ fn unresponsive_not_returned_indirect() {
         swarms[0]
             .1
             .behaviour_mut()
-            .add_address(&PeerId::random(), multiaddr![Udp(10u16)]);
+            .add_address(&PeerId::random(), multiaddr![Udp(10u16)])
+            .unwrap();
     }
 
     // Connect second to first.
@@ -382,7 +390,8 @@ fn unresponsive_not_returned_indirect() {
     swarms[1]
         .1
         .behaviour_mut()
-        .add_address(&first_peer_id, first_address);
+        .add_address(&first_peer_id, first_address)
+        .unwrap();
 
     // Drop the swarm addresses.
     let mut swarms = swarms
@@ -434,11 +443,13 @@ fn get_record_not_found() {
     swarms[0]
         .1
         .behaviour_mut()
-        .add_address(&swarm_ids[1], second);
+        .add_address(&swarm_ids[1], second)
+        .unwrap();
     swarms[1]
         .1
         .behaviour_mut()
-        .add_address(&swarm_ids[2], third);
+        .add_address(&swarm_ids[2], third)
+        .unwrap();
 
     // Drop the swarm addresses.
     let mut swarms = swarms
@@ -515,7 +526,8 @@ fn put_record() {
                 single_swarm
                     .1
                     .behaviour_mut()
-                    .add_address(swarm.1.local_peer_id(), swarm.0.clone());
+                    .add_address(swarm.1.local_peer_id(), swarm.0.clone())
+                    .unwrap();
             }
 
             let mut swarms = vec![single_swarm];
@@ -747,7 +759,11 @@ fn get_record() {
             *Swarm::local_peer_id(&swarms[i + 1].1),
             swarms[i + 1].0.clone(),
         );
-        swarms[i].1.behaviour_mut().add_address(&peer_id, address);
+        swarms[i]
+            .1
+            .behaviour_mut()
+            .add_address(&peer_id, address)
+            .unwrap();
     }
 
     // Drop the swarm addresses.
@@ -886,7 +902,8 @@ fn add_provider() {
                 single_swarm
                     .1
                     .behaviour_mut()
-                    .add_address(swarm.1.local_peer_id(), swarm.0.clone());
+                    .add_address(swarm.1.local_peer_id(), swarm.0.clone())
+                    .unwrap();
             }
 
             let mut swarms = vec![single_swarm];
@@ -1117,11 +1134,13 @@ fn disjoint_query_does_not_finish_before_all_paths_did() {
     alice
         .1
         .behaviour_mut()
-        .add_address(trudy.1.local_peer_id(), trudy.0.clone());
+        .add_address(trudy.1.local_peer_id(), trudy.0.clone())
+        .unwrap();
     alice
         .1
         .behaviour_mut()
-        .add_address(bob.1.local_peer_id(), bob.0.clone());
+        .add_address(bob.1.local_peer_id(), bob.0.clone())
+        .unwrap();
 
     // Drop the swarm addresses.
     let (mut alice, mut bob, mut trudy) = (alice.1, bob.1, trudy.1);
@@ -1442,7 +1461,11 @@ fn get_providers_limit<const N: usize>() {
                 *Swarm::local_peer_id(&swarms[i + 1].1),
                 swarms[i + 1].0.clone(),
             );
-            swarms[i].1.behaviour_mut().add_address(&peer_id, address);
+            swarms[i]
+                .1
+                .behaviour_mut()
+                .add_address(&peer_id, address)
+                .unwrap();
         }
 
         // Drop the swarm addresses.

protocols/kad/src/lib.rs

@@ -70,7 +70,7 @@ pub use behaviour::{
     GetClosestPeersResult, GetProvidersError, GetProvidersOk, GetProvidersResult, GetRecordError,
     GetRecordOk, GetRecordResult, InboundRequest, Mode, NoKnownPeers, PeerRecord, PutRecordContext,
     PutRecordError, PutRecordOk, PutRecordPhase, PutRecordResult, QueryInfo, QueryMut, QueryRef,
-    QueryResult, QueryStats, RoutingUpdate,
+    QueryResult, QueryStats, RoutingUpdateError, RoutingUpdateOk,
 };
 pub use behaviour::{
     Kademlia, KademliaBucketInserts, KademliaCaching, KademliaConfig, KademliaEvent,