Stack allocated PeerId

core/Cargo.toml

@@ -12,7 +12,6 @@ categories = ["network-programming", "asynchronous"]
 [dependencies]
 asn1_der = "0.6.1"
 bs58 = "0.4.0"
-bytes = "0.5"
 ed25519-dalek = "1.0.1"
 either = "1.5"
 fnv = "1.0"

core/src/network/peer.rs

@@ -555,7 +555,7 @@ where
         }
 
         self.network.pool.add(connection, connected)
-            .map(|_id| ConnectedPeer {
+            .map(move |_id| ConnectedPeer {
                 network: self.network,
                 peer_id: self.peer_id,
             })

core/src/peer_id.rs

@@ -19,11 +19,10 @@
 // DEALINGS IN THE SOFTWARE.
 
 use crate::PublicKey;
-use bytes::Bytes;
-use thiserror::Error;
-use multihash::{Code, Multihash, MultihashDigest};
+use multihash::{Code, Error, Multihash, MultihashDigest};
 use rand::Rng;
-use std::{convert::TryFrom, borrow::Borrow, fmt, hash, str::FromStr, cmp};
+use std::{convert::TryFrom, fmt, str::FromStr};
+use thiserror::Error;
 
 /// Public keys with byte-lengths smaller than `MAX_INLINE_KEY_LENGTH` will be
 /// automatically used as the peer id using an identity multihash.
@@ -32,10 +31,9 @@ const MAX_INLINE_KEY_LENGTH: usize = 42;
 /// Identifier of a peer of the network.
 ///
 /// The data is a multihash of the public key of the peer.
-// TODO: maybe keep things in decoded version?
-#[derive(Clone, Eq)]
+#[derive(Clone, Copy, Eq, Hash, Ord, PartialEq, PartialOrd)]
 pub struct PeerId {
-    multihash: Bytes,
+    multihash: Multihash,
 }
 
 impl fmt::Debug for PeerId {
@@ -52,21 +50,6 @@ impl fmt::Display for PeerId {
     }
 }
 
-impl cmp::PartialOrd for PeerId {
-    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
-        Some(Ord::cmp(self, other))
-    }
-}
-
-impl cmp::Ord for PeerId {
-    fn cmp(&self, other: &Self) -> cmp::Ordering {
-        // must use borrow, because as_bytes is not consistent with equality
-        let lhs: &[u8] = self.borrow();
-        let rhs: &[u8] = other.borrow();
-        lhs.cmp(rhs)
-    }
-}
-
 impl PeerId {
     /// Builds a `PeerId` from a public key.
     pub fn from_public_key(key: PublicKey) -> PeerId {
@@ -78,18 +61,15 @@ impl PeerId {
             Code::Sha2_256
         };
 
-        let multihash = hash_algorithm.digest(&key_enc).to_bytes().into();
+        let multihash = hash_algorithm.digest(&key_enc);
 
         PeerId { multihash }
     }
 
-    /// Checks whether `data` is a valid `PeerId`. If so, returns the `PeerId`. If not, returns
-    /// back the data as an error.
-    pub fn from_bytes(data: Vec<u8>) -> Result<PeerId, Vec<u8>> {
-        match Multihash::from_bytes(&data) {
-            Ok(multihash) => PeerId::from_multihash(multihash).map_err(|_| data),
-            Err(_err) => Err(data),
-        }
+    /// Parses a `PeerId` from bytes.
+    pub fn from_bytes(data: &[u8]) -> Result<PeerId, Error> {
+        Ok(PeerId::from_multihash(Multihash::from_bytes(&data)?)
+            .map_err(|mh| Error::UnsupportedCode(mh.code()))?)
     }
 
     /// Tries to turn a `Multihash` into a `PeerId`.
@@ -99,9 +79,9 @@ impl PeerId {
     /// peer ID, it is returned as an `Err`.
     pub fn from_multihash(multihash: Multihash) -> Result<PeerId, Multihash> {
         match Code::try_from(multihash.code()) {
-            Ok(Code::Sha2_256) => Ok(PeerId { multihash: multihash.to_bytes().into() }),
+            Ok(Code::Sha2_256) => Ok(PeerId { multihash }),
             Ok(Code::Identity) if multihash.digest().len() <= MAX_INLINE_KEY_LENGTH
-                => Ok(PeerId { multihash: multihash.to_bytes().into() }),
+                => Ok(PeerId { multihash }),
             _ => Err(multihash)
         }
     }
@@ -113,54 +93,29 @@ impl PeerId {
         let peer_id = rand::thread_rng().gen::<[u8; 32]>();
         PeerId {
             multihash: Multihash::wrap(Code::Identity.into(), &peer_id)
-                .expect("The digest size is never too large").to_bytes().into()
+                .expect("The digest size is never too large")
         }
     }
 
     /// Returns a raw bytes representation of this `PeerId`.
-    ///
-    /// **NOTE:** This byte representation is not necessarily consistent with
-    /// equality of peer IDs. That is, two peer IDs may be considered equal
-    /// while having a different byte representation as per `into_bytes`.
-    pub fn into_bytes(self) -> Vec<u8> {
-        self.multihash.to_vec()
-    }
-
-    /// Returns a raw bytes representation of this `PeerId`.
-    ///
-    /// **NOTE:** This byte representation is not necessarily consistent with
-    /// equality of peer IDs. That is, two peer IDs may be considered equal
-    /// while having a different byte representation as per `as_bytes`.
-    pub fn as_bytes(&self) -> &[u8] {
-        &self.multihash
+    pub fn to_bytes(&self) -> Vec<u8> {
+        self.multihash.to_bytes()
     }
 
     /// Returns a base-58 encoded string of this `PeerId`.
     pub fn to_base58(&self) -> String {
-        bs58::encode(self.borrow() as &[u8]).into_string()
+        bs58::encode(self.to_bytes()).into_string()
     }
 
     /// Checks whether the public key passed as parameter matches the public key of this `PeerId`.
     ///
     /// Returns `None` if this `PeerId`s hash algorithm is not supported when encoding the
     /// given public key, otherwise `Some` boolean as the result of an equality check.
     pub fn is_public_key(&self, public_key: &PublicKey) -> Option<bool> {
-        let multihash = Multihash::from_bytes(&self.multihash)
-           .expect("Internal multihash is always a valid");
-        let alg = Code::try_from(multihash.code())
+        let alg = Code::try_from(self.multihash.code())
             .expect("Internal multihash is always a valid `Code`");
         let enc = public_key.clone().into_protobuf_encoding();
-        Some(alg.digest(&enc) == multihash)
-    }
-}
-
-impl hash::Hash for PeerId {
-    fn hash<H>(&self, state: &mut H)
-    where
-        H: hash::Hasher
-    {
-        let digest = self.borrow() as &[u8];
-        hash::Hash::hash(digest, state)
+        Some(alg.digest(&enc) == self.multihash)
     }
 }
 
@@ -174,7 +129,7 @@ impl TryFrom<Vec<u8>> for PeerId {
     type Error = Vec<u8>;
 
     fn try_from(value: Vec<u8>) -> Result<Self, Self::Error> {
-        PeerId::from_bytes(value)
+        PeerId::from_bytes(&value).map_err(|_| value)
     }
 }
 
@@ -186,33 +141,21 @@ impl TryFrom<Multihash> for PeerId {
     }
 }
 
-impl PartialEq<PeerId> for PeerId {
-    fn eq(&self, other: &PeerId) -> bool {
-        let self_digest = self.borrow() as &[u8];
-        let other_digest = other.borrow() as &[u8];
-        self_digest == other_digest
-    }
-}
-
-impl Borrow<[u8]> for PeerId {
-    fn borrow(&self) -> &[u8] {
+impl AsRef<Multihash> for PeerId {
+    fn as_ref(&self) -> &Multihash {
         &self.multihash
     }
 }
 
-/// **NOTE:** This byte representation is not necessarily consistent with
-/// equality of peer IDs. That is, two peer IDs may be considered equal
-/// while having a different byte representation as per `AsRef<[u8]>`.
-impl AsRef<[u8]> for PeerId {
-    fn as_ref(&self) -> &[u8] {
-        self.as_bytes()
+impl From<PeerId> for Multihash {
+    fn from(peer_id: PeerId) -> Self {
+        peer_id.multihash
     }
 }
 
-impl From<PeerId> for Multihash {
+impl From<PeerId> for Vec<u8> {
     fn from(peer_id: PeerId) -> Self {
-        Multihash::from_bytes(&peer_id.multihash)
-            .expect("PeerIds always contain valid Multihashes")
+        peer_id.to_bytes()
     }
 }
 
@@ -230,7 +173,7 @@ impl FromStr for PeerId {
     #[inline]
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let bytes = bs58::decode(s).into_vec()?;
-        PeerId::from_bytes(bytes).map_err(|_| ParseError::MultiHash)
+        PeerId::from_bytes(&bytes).map_err(|_| ParseError::MultiHash)
     }
 }
 
@@ -248,7 +191,7 @@ mod tests {
     #[test]
     fn peer_id_into_bytes_then_from_bytes() {
         let peer_id = identity::Keypair::generate_ed25519().public().into_peer_id();
-        let second = PeerId::from_bytes(peer_id.clone().into_bytes()).unwrap();
+        let second = PeerId::from_bytes(&peer_id.to_bytes()).unwrap();
         assert_eq!(peer_id, second);
     }
 
@@ -263,7 +206,7 @@ mod tests {
     fn random_peer_id_is_valid() {
         for _ in 0 .. 5000 {
             let peer_id = PeerId::random();
-            assert_eq!(peer_id, PeerId::from_bytes(peer_id.clone().into_bytes()).unwrap());
+            assert_eq!(peer_id, PeerId::from_bytes(&peer_id.to_bytes()).unwrap());
         }
     }
 }

protocols/floodsub/src/protocol.rs

@@ -61,7 +61,7 @@ where
             let mut messages = Vec::with_capacity(rpc.publish.len());
             for publish in rpc.publish.into_iter() {
                 messages.push(FloodsubMessage {
-                    source: PeerId::from_bytes(publish.from.unwrap_or_default()).map_err(|_| {
+                    source: PeerId::from_bytes(&publish.from.unwrap_or_default()).map_err(|_| {
                         FloodsubDecodeError::InvalidPeerId
                     })?,
                     data: publish.data.unwrap_or_default(),
@@ -179,7 +179,7 @@ impl FloodsubRpc {
             publish: self.messages.into_iter()
                 .map(|msg| {
                     rpc_proto::Message {
-                        from: Some(msg.source.into_bytes()),
+                        from: Some(msg.source.to_bytes()),
                         data: Some(msg.data),
                         seqno: Some(msg.sequence_number),
                         topic_ids: msg.topics

protocols/gossipsub/src/behaviour.rs

@@ -1100,7 +1100,7 @@ impl Gossipsub {
 
                 let signature = {
                     let message = rpc_proto::Message {
-                        from: Some(author.clone().into_bytes()),
+                        from: Some(author.clone().to_bytes()),
                         data: Some(data.clone()),
                         seqno: Some(sequence_number.to_be_bytes().to_vec()),
                         topic_ids: topics.clone().into_iter().map(|t| t.into()).collect(),

protocols/gossipsub/src/config.rs

@@ -139,7 +139,7 @@ impl Default for GossipsubConfig {
                 let mut source_string = if let Some(peer_id) = message.source.as_ref() {
                     peer_id.to_base58()
                 } else {
-                    PeerId::from_bytes(vec![0, 1, 0])
+                    PeerId::from_bytes(&[0, 1, 0])
                         .expect("Valid peer id")
                         .to_base58()
                 };

protocols/gossipsub/src/protocol.rs

@@ -137,7 +137,7 @@ impl GossipsubCodec {
             }
         };
 
-        let source = match PeerId::from_bytes(from.clone()) {
+        let source = match PeerId::from_bytes(&from) {
             Ok(v) => v,
             Err(_) => {
                 debug!("Signature verification failed: Invalid Peer Id");
@@ -161,7 +161,7 @@ impl GossipsubCodec {
             .map(|key| PublicKey::from_protobuf_encoding(&key))
         {
             Some(Ok(key)) => key,
-            _ => match PublicKey::from_protobuf_encoding(&source.as_bytes()[2..]) {
+            _ => match PublicKey::from_protobuf_encoding(&source.to_bytes()[2..]) {
                 Ok(v) => v,
                 Err(_) => {
                     warn!("Signature verification failed: No valid public key supplied");
@@ -200,7 +200,7 @@ impl Encoder for GossipsubCodec {
 
         for message in item.messages.into_iter() {
             let message = rpc_proto::Message {
-                from: message.source.map(|m| m.into_bytes()),
+                from: message.source.map(|m| m.to_bytes()),
                 data: Some(message.data),
                 seqno: message.sequence_number.map(|s| s.to_be_bytes().to_vec()),
                 topic_ids: message.topics.into_iter().map(TopicHash::into).collect(),
@@ -372,7 +372,7 @@ impl Decoder for GossipsubCodec {
 
             let source = if verify_source {
                 Some(
-                    PeerId::from_bytes(message.from.unwrap_or_default()).map_err(|_| {
+                    PeerId::from_bytes(&message.from.unwrap_or_default()).map_err(|_| {
                         io::Error::new(io::ErrorKind::InvalidData, "Invalid Peer Id")
                     })?,
                 )