Merge pull request #20 from Nuhvi/v3

feat: remove all unwrap and deny using it
pubky · Sep 27, 2024 · 9bcb7ca · 9bcb7ca
2 parents 3a4c331 + 4a18e1c
commit 9bcb7ca
Show file tree

Hide file tree

Showing 13 changed files with 121 additions and 99 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,12 @@
+# Changelog
+
+All notable changes to mainline dht will be documented in this file.
+
+## [Unreleased]
+
+### Changed
+
+- Removed all internal panic `#![deny(clippy::unwrap_used)]`
+- `Testnet::new(size)` returns a `Result<Testnet>`.
+- `Dht::local_addr()` returns a `Result<SocketAddr>`.
+ `AsyncDht::local_addr()` returns a `Result<SocketAddr>`.
diff --git a/Cargo.toml b/Cargo.toml
@@ -35,3 +35,6 @@ default = []
 
 [package.metadata.docs.rs]
 all-features = true
+
+[lints.clippy]
+unwrap_used = "deny"
diff --git a/clippy.toml b/clippy.toml
@@ -0,0 +1 @@
+allow-unwrap-in-tests = true
diff --git a/src/async_dht.rs b/src/async_dht.rs
@@ -10,6 +10,7 @@ use crate::{
         PutMutableRequestArguments, PutRequestSpecific, RequestTypeSpecific,
     },
     dht::{ActorMessage, Dht},
+    error::SocketAddrResult,
     rpc::{PutResult, ResponseSender},
     Result,
 };
@@ -29,8 +30,15 @@ impl AsyncDht {
     // === Getters ===
 
     /// Returns the local address of the udp socket this node is listening on.
-    pub fn local_addr(&self) -> Option<SocketAddr> {
-        self.0.local_addr()
+    ///
+    /// Returns an error if the actor is shutdown, or if the [std::net::UdpSocket::local_addr]
+    /// returned an IO error.
+    pub async fn local_addr(&self) -> Result<SocketAddr> {
+        let (sender, receiver) = flume::bounded::<SocketAddrResult>(1);
+
+        self.0 .0.send(ActorMessage::LocalAddr(sender))?;
+
+        Ok(receiver.recv_async().await??)
     }
 
     // === Public Methods ===
@@ -39,12 +47,10 @@ impl AsyncDht {
     pub async fn shutdown(&mut self) -> Result<()> {
         let (sender, receiver) = flume::bounded::<()>(1);
 
-        self.0.sender.send(ActorMessage::Shutdown(sender))?;
+        self.0 .0.send(ActorMessage::Shutdown(sender))?;
 
         receiver.recv_async().await?;
 
-        self.0.address = None;
-
         Ok(())
     }
 
@@ -69,7 +75,7 @@ impl AsyncDht {
 
         let request = RequestTypeSpecific::GetPeers(GetPeersRequestArguments { info_hash });
 
-        self.0.sender.send(ActorMessage::Get(
+        self.0 .0.send(ActorMessage::Get(
             info_hash,
             request,
             ResponseSender::Peers(sender),
@@ -98,7 +104,7 @@ impl AsyncDht {
         });
 
         self.0
-            .sender
+             .0
             .send(ActorMessage::Put(info_hash, request, sender))?;
 
         receiver.recv_async().await?
@@ -116,7 +122,7 @@ impl AsyncDht {
             salt: None,
         });
 
-        self.0.sender.send(ActorMessage::Get(
+        self.0 .0.send(ActorMessage::Get(
             target,
             request,
             ResponseSender::Immutable(sender),
@@ -127,7 +133,7 @@ impl AsyncDht {
 
     /// Put an immutable data to the DHT.
     pub async fn put_immutable(&self, value: Bytes) -> Result<Id> {
-        let target = Id::from_bytes(hash_immutable(&value)).unwrap();
+        let target: Id = hash_immutable(&value).into();
 
         let (sender, receiver) = flume::bounded::<PutResult>(1);
 
@@ -136,9 +142,7 @@ impl AsyncDht {
             v: value.clone().into(),
         });
 
-        self.0
-            .sender
-            .send(ActorMessage::Put(target, request, sender))?;
+        self.0 .0.send(ActorMessage::Put(target, request, sender))?;
 
         receiver.recv_async().await?
     }
@@ -158,7 +162,7 @@ impl AsyncDht {
 
         let request = RequestTypeSpecific::GetValue(GetValueRequestArguments { target, seq, salt });
 
-        let _ = self.0.sender.send(ActorMessage::Get(
+        let _ = self.0 .0.send(ActorMessage::Get(
             target,
             request,
             ResponseSender::Mutable(sender),
@@ -183,7 +187,7 @@ impl AsyncDht {
 
         let _ = self
             .0
-            .sender
+             .0
             .send(ActorMessage::Put(*item.target(), request, sender));
 
         receiver.recv_async().await?
@@ -207,7 +211,7 @@ mod test {
         async fn test() {
             let mut dht = Dht::client().unwrap().as_async();
 
-            dht.local_addr();
+            dht.local_addr().await.unwrap();
 
             let a = dht.clone();
 
@@ -223,7 +227,7 @@ mod test {
     #[test]
     fn announce_get_peer() {
         async fn test() {
-            let testnet = Testnet::new(10);
+            let testnet = Testnet::new(10).unwrap();
 
             let a = Dht::builder()
                 .bootstrap(&testnet.bootstrap)
@@ -258,7 +262,7 @@ mod test {
     #[test]
     fn put_get_immutable() {
         async fn test() {
-            let testnet = Testnet::new(10);
+            let testnet = Testnet::new(10).unwrap();
 
             let a = Dht::builder()
                 .bootstrap(&testnet.bootstrap)
@@ -287,7 +291,7 @@ mod test {
     #[test]
     fn put_get_mutable() {
         async fn test() {
-            let testnet = Testnet::new(10);
+            let testnet = Testnet::new(10).unwrap();
 
             let a = Dht::builder()
                 .bootstrap(&testnet.bootstrap)
@@ -328,7 +332,7 @@ mod test {
     #[test]
     fn put_get_mutable_no_more_recent_value() {
         async fn test() {
-            let testnet = Testnet::new(10);
+            let testnet = Testnet::new(10).unwrap();
 
             let a = Dht::builder()
                 .bootstrap(&testnet.bootstrap)
@@ -368,7 +372,7 @@ mod test {
     #[test]
     fn repeated_put_query() {
         async fn test() {
-            let testnet = Testnet::new(10);
+            let testnet = Testnet::new(10).unwrap();
 
             let a = Dht::builder()
                 .bootstrap(&testnet.bootstrap)

diff --git a/src/common/messages.rs b/src/common/messages.rs
@@ -437,13 +437,9 @@ impl Message {
                                     token: arguments.token,
                                     put_request_type: PutRequestSpecific::AnnouncePeer(
                                         AnnouncePeerRequestArguments {
-                                            implied_port: if arguments.implied_port.is_none() {
-                                                None
-                                            } else if arguments.implied_port.unwrap() != 0 {
-                                                Some(true)
-                                            } else {
-                                                Some(false)
-                                            },
+                                            implied_port: arguments
+                                                .implied_port
+                                                .map(|implied_port| implied_port != 0),
                                             info_hash: Id::from_bytes(&arguments.info_hash)?,
                                             port: arguments.port,
                                         },
@@ -452,7 +448,7 @@ impl Message {
                             }
                         }
                         internal::DHTRequestSpecific::PutValue { arguments } => {
-                            if arguments.k.is_some() {
+                            if let Some(k) = arguments.k {
                                 RequestSpecific {
                                     requester_id: Id::from_bytes(arguments.id)?,
 
@@ -462,10 +458,14 @@ impl Message {
                                             PutMutableRequestArguments {
                                                 target: Id::from_bytes(arguments.target)?,
                                                 v: arguments.v,
-                                                k: arguments.k.unwrap(),
+                                                k,
                                                 // Should panic if missing.
-                                                seq: arguments.seq.unwrap(),
-                                                sig: arguments.sig.unwrap(),
+                                                seq: arguments.seq.expect(
+                                                    "Put mutable message to have sequence number",
+                                                ),
+                                                sig: arguments.sig.expect(
+                                                    "Put mutable message to have a signature",
+                                                ),
                                                 salt: arguments.salt,
                                                 cas: arguments.cas,
                                             },

diff --git a/src/common/mutable.rs b/src/common/mutable.rs
@@ -53,9 +53,9 @@ impl MutableItem {
 
         let mut hasher = Sha1::new();
         hasher.update(&encoded);
-        let hash = hasher.digest().bytes();
+        let bytes = hasher.digest().bytes();
 
-        Id::from_bytes(hash).unwrap()
+        Id { bytes }
     }
 
     /// Set the cas number if needed.

diff --git a/src/common/routing_table.rs b/src/common/routing_table.rs
@@ -43,9 +43,7 @@ impl RoutingTable {
             return false;
         }
 
-        self.buckets.entry(distance).or_default();
-
-        let bucket = self.buckets.get_mut(&distance).unwrap();
+        let bucket = self.buckets.entry(distance).or_default();
 
         bucket.add(node)
     }