Use stronger typing for the public key of identify (#236)

* Use stronger typing for the public key of identify

* Add PartialEq/Eq implementations

* Fix tests

* Also change is_public_key()

core/src/lib.rs

@@ -233,7 +233,7 @@ pub mod upgrade;
 pub use self::connection_reuse::ConnectionReuse;
 pub use self::multiaddr::{AddrComponent, Multiaddr};
 pub use self::muxing::StreamMuxer;
-pub use self::peer_id::PeerId;
+pub use self::peer_id::{PeerId, PublicKeyBytes, PublicKeyBytesSlice};
 pub use self::swarm::{swarm, SwarmController, SwarmFuture};
 pub use self::transport::{MuxedTransport, Transport};
 pub use self::upgrade::{ConnectionUpgrade, Endpoint};

core/src/peer_id.rs

@@ -22,6 +22,56 @@ use bs58;
 use multihash;
 use std::{fmt, str::FromStr};
 
+/// The raw bytes of a public key.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct PublicKeyBytes(pub Vec<u8>);
+
+impl PublicKeyBytes {
+    /// Turns this into a `PublicKeyBytesSlice`.
+    #[inline]
+    pub fn as_slice(&self) -> PublicKeyBytesSlice {
+        PublicKeyBytesSlice(&self.0)
+    }
+
+    /// Turns this into a `PeerId`.
+    #[inline]
+    pub fn to_peer_id(&self) -> PeerId {
+        self.as_slice().into()
+    }
+}
+
+/// The raw bytes of a public key.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub struct PublicKeyBytesSlice<'a>(pub &'a [u8]);
+
+impl<'a> PublicKeyBytesSlice<'a> {
+    /// Turns this into a `PublicKeyBytes`.
+    #[inline]
+    pub fn to_owned(&self) -> PublicKeyBytes {
+        PublicKeyBytes(self.0.to_owned())
+    }
+
+    /// Turns this into a `PeerId`.
+    #[inline]
+    pub fn to_peer_id(&self) -> PeerId {
+        PeerId::from_public_key(*self)
+    }
+}
+
+impl<'a> PartialEq<PublicKeyBytes> for PublicKeyBytesSlice<'a> {
+    #[inline]
+    fn eq(&self, other: &PublicKeyBytes) -> bool {
+        self.0 == &other.0[..]
+    }
+}
+
+impl<'a> PartialEq<PublicKeyBytesSlice<'a>> for PublicKeyBytes {
+    #[inline]
+    fn eq(&self, other: &PublicKeyBytesSlice<'a>) -> bool {
+        self.0 == &other.0[..]
+    }
+}
+
 /// Identifier of a peer of the network.
 ///
 /// The data is a multihash of the public key of the peer.
@@ -40,8 +90,8 @@ impl fmt::Debug for PeerId {
 impl PeerId {
     /// Builds a `PeerId` from a public key.
     #[inline]
-    pub fn from_public_key(public_key: &[u8]) -> PeerId {
+    pub fn from_public_key(public_key: PublicKeyBytesSlice) -> PeerId {
-        let data = multihash::encode(multihash::Hash::SHA2256, public_key)
+        let data = multihash::encode(multihash::Hash::SHA2256, public_key.0)
             .expect("sha2-256 is always supported");
         PeerId { multihash: data }
     }
@@ -57,12 +107,16 @@ impl PeerId {
     }
 
     /// Returns a raw bytes representation of this `PeerId`.
+    ///
+    /// Note that this is not the same as the public key of the peer.
     #[inline]
     pub fn into_bytes(self) -> Vec<u8> {
         self.multihash
     }
 
     /// Returns a raw bytes representation of this `PeerId`.
+    ///
+    /// Note that this is not the same as the public key of the peer.
     #[inline]
     pub fn as_bytes(&self) -> &[u8] {
         &self.multihash
@@ -86,11 +140,11 @@ impl 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: &[u8]) -> Option<bool> {
+    pub fn is_public_key(&self, public_key: PublicKeyBytesSlice) -> Option<bool> {
         let alg = multihash::decode(&self.multihash)
             .expect("our inner value should always be valid")
             .alg;
-        match multihash::encode(alg, public_key) {
+        match multihash::encode(alg, public_key.0) {
             Ok(compare) => Some(compare == self.multihash),
             Err(multihash::Error::UnsupportedType) => None,
             Err(_) => Some(false),
@@ -98,6 +152,20 @@ impl PeerId {
     }
 }
 
+impl From<PublicKeyBytes> for PeerId {
+    #[inline]
+    fn from(pubkey: PublicKeyBytes) -> PeerId {
+        PublicKeyBytesSlice(&pubkey.0).into()
+    }
+}
+
+impl<'a> From<PublicKeyBytesSlice<'a>> for PeerId {
+    #[inline]
+    fn from(pubkey: PublicKeyBytesSlice<'a>) -> PeerId {
+        PeerId::from_public_key(pubkey)
+    }
+}
+
 quick_error! {
     #[derive(Debug)]
     pub enum ParseError {

identify/src/protocol.rs

@@ -20,7 +20,7 @@
 
 use bytes::{Bytes, BytesMut};
 use futures::{future, Future, Sink, Stream};
-use libp2p_core::{ConnectionUpgrade, Endpoint};
+use libp2p_core::{ConnectionUpgrade, Endpoint, PublicKeyBytes};
 use log::Level;
 use multiaddr::Multiaddr;
 use protobuf::Message as ProtobufMessage;
@@ -83,7 +83,7 @@ where
         let mut message = structs_proto::Identify::new();
         message.set_agentVersion(info.agent_version);
         message.set_protocolVersion(info.protocol_version);
-        message.set_publicKey(info.public_key);
+        message.set_publicKey(info.public_key.0);
         message.set_listenAddrs(listen_addrs);
         message.set_observedAddr(observed_addr.to_bytes());
         message.set_protocols(RepeatedField::from_vec(info.protocols));
@@ -100,8 +100,8 @@ where
 /// Information sent from the listener to the dialer.
 #[derive(Debug, Clone)]
 pub struct IdentifyInfo {
-    /// Public key of the node in the DER format.
+    /// Public key of the node.
-    pub public_key: Vec<u8>,
+    pub public_key: PublicKeyBytes,
     /// Version of the "global" protocol, eg. `ipfs/1.0.0` or `polkadot/1.0.0`.
     pub protocol_version: String,
     /// Name and version of the client. Can be thought as similar to the `User-Agent` header
@@ -213,7 +213,7 @@ fn parse_proto_msg(msg: BytesMut) -> Result<(IdentifyInfo, Multiaddr), IoError>
             let observed_addr = bytes_to_multiaddr(msg.take_observedAddr())?;
 
             let info = IdentifyInfo {
-                public_key: msg.take_publicKey(),
+                public_key: PublicKeyBytes(msg.take_publicKey()),
                 protocol_version: msg.take_protocolVersion(),
                 agent_version: msg.take_agentVersion(),
                 listen_addrs: listen_addrs,
@@ -235,7 +235,7 @@ mod tests {
     use self::libp2p_tcp_transport::TcpConfig;
     use self::tokio_core::reactor::Core;
     use futures::{Future, Stream};
-    use libp2p_core::Transport;
+    use libp2p_core::{Transport, PublicKeyBytes};
     use std::sync::mpsc;
     use std::thread;
     use {IdentifyInfo, IdentifyOutput, IdentifyProtocolConfig};
@@ -263,7 +263,7 @@ mod tests {
                 .and_then(|identify| match identify {
                     IdentifyOutput::Sender { sender, .. } => sender.send(
                         IdentifyInfo {
-                            public_key: vec![1, 2, 3, 4, 5, 7],
+                            public_key: PublicKeyBytes(vec![1, 2, 3, 4, 5, 7]),
                             protocol_version: "proto_version".to_owned(),
                             agent_version: "agent_version".to_owned(),
                             listen_addrs: vec![
@@ -295,7 +295,7 @@ mod tests {
                         observed_addr,
                         "/ip4/100.101.102.103/tcp/5000".parse().unwrap()
                     );
-                    assert_eq!(info.public_key, &[1, 2, 3, 4, 5, 7]);
+                    assert_eq!(info.public_key.0, &[1, 2, 3, 4, 5, 7]);
                     assert_eq!(info.protocol_version, "proto_version");
                     assert_eq!(info.agent_version, "agent_version");
                     assert_eq!(

identify/src/transport.rs

@@ -390,7 +390,7 @@ fn process_identify_info<P>(
 where
     P: Peerstore,
 {
-    let peer_id = PeerId::from_public_key(&info.public_key);
+    let peer_id: PeerId = info.public_key.to_peer_id();
     peerstore
         .peer_or_create(&peer_id)
         .add_addr(client_addr, ttl);
@@ -408,7 +408,7 @@ mod tests {
     use futures::{Future, Stream};
     use libp2p_peerstore::memory_peerstore::MemoryPeerstore;
     use libp2p_peerstore::{PeerAccess, PeerId, Peerstore};
-    use libp2p_core::Transport;
+    use libp2p_core::{Transport, PublicKeyBytesSlice};
     use multiaddr::{AddrComponent, Multiaddr};
     use std::io::Error as IoError;
     use std::iter;
@@ -450,7 +450,7 @@ mod tests {
             }
         }
 
-        let peer_id = PeerId::from_public_key(&vec![1, 2, 3, 4]);
+        let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3, 4]));
 
         let peerstore = MemoryPeerstore::empty();
         peerstore.peer_or_create(&peer_id).add_addr(

kad/src/protocol.rs

@@ -308,7 +308,7 @@ mod tests {
     use self::tokio_core::reactor::Core;
     use futures::{Future, Sink, Stream};
     use libp2p_peerstore::PeerId;
-    use libp2p_core::Transport;
+    use libp2p_core::{Transport, PublicKeyBytesSlice};
     use protocol::{ConnectionType, KadMsg, KademliaProtocolConfig, Peer};
     use std::sync::mpsc;
     use std::thread;
@@ -332,7 +332,7 @@ mod tests {
         test_one(KadMsg::FindNodeRes {
             closer_peers: vec![
                 Peer {
-                    node_id: PeerId::from_public_key(&[93, 80, 12, 250]),
+                    node_id: PeerId::from_public_key(PublicKeyBytesSlice(&[93, 80, 12, 250])),
                     multiaddrs: vec!["/ip4/100.101.102.103/tcp/20105".parse().unwrap()],
                     connection_ty: ConnectionType::Connected,
                 },

libp2p/examples/floodsub.rs

@@ -31,7 +31,7 @@ use futures::Stream;
 use futures::future::Future;
 use std::{env, mem};
 use libp2p::core::{either::EitherOutput, upgrade};
-use libp2p::core::{Multiaddr, Transport};
+use libp2p::core::{Multiaddr, Transport, PublicKeyBytesSlice};
 use libp2p::peerstore::PeerId;
 use libp2p::tcp::TcpConfig;
 use tokio_core::reactor::Core;
@@ -91,7 +91,7 @@ fn main() {
     // or substream to our server.
     let my_id = {
         let key = (0..2048).map(|_| rand::random::<u8>()).collect::<Vec<_>>();
-        PeerId::from_public_key(&key)
+        PeerId::from_public_key(PublicKeyBytesSlice(&key))
     };
 
     let (floodsub_upgrade, floodsub_rx) = libp2p::floodsub::FloodSubUpgrade::new(my_id);

libp2p/examples/kademlia.rs

@@ -33,7 +33,7 @@ use libp2p::Multiaddr;
 use std::env;
 use std::sync::Arc;
 use std::time::Duration;
-use libp2p::core::Transport;
+use libp2p::core::{Transport, PublicKeyBytesSlice};
 use libp2p::core::{upgrade, either::EitherOutput};
 use libp2p::tcp::TcpConfig;
 use tokio_core::reactor::Core;
@@ -96,7 +96,7 @@ fn main() {
     // incoming connections, and that will automatically apply secio and multiplex on top
     // of any opened stream.
 
-    let my_peer_id = PeerId::from_public_key(include_bytes!("test-rsa-public-key.der"));
+    let my_peer_id = PeerId::from_public_key(PublicKeyBytesSlice(include_bytes!("test-rsa-public-key.der")));
     println!("Local peer id is: {:?}", my_peer_id);
 
     // Let's put this `transport` into a Kademlia *swarm*. The swarm will handle all the incoming

libp2p/examples/random_peerid.rs

@@ -21,12 +21,12 @@
 extern crate libp2p;
 extern crate rand;
 
-use libp2p::PeerId;
+use libp2p::{PeerId, core::PublicKeyBytesSlice};
 
 fn main() {
     let pid = {
         let key = (0..2048).map(|_| rand::random::<u8>()).collect::<Vec<_>>();
-        PeerId::from_public_key(&key)
+        PeerId::from_public_key(PublicKeyBytesSlice(&key))
     };
     println!("{}", pid.to_base58());
 }

peerstore/src/json_peerstore.rs

@@ -148,7 +148,7 @@ mod tests {
         let temp_file = self::tempfile::NamedTempFile::new().unwrap();
         let peer_store = ::json_peerstore::JsonPeerstore::new(temp_file.path()).unwrap();
 
-        let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+        let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
         let addr = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
 
         peer_store

peerstore/src/lib.rs

@@ -38,17 +38,19 @@
 //!
 //! ```
 //! extern crate multiaddr;
+//! extern crate libp2p_core;
 //! extern crate libp2p_peerstore;
 //!
 //! # fn main() {
+//! use libp2p_core::{PeerId, PublicKeyBytesSlice};
 //! use libp2p_peerstore::memory_peerstore::MemoryPeerstore;
-//! use libp2p_peerstore::{PeerId, Peerstore, PeerAccess};
+//! use libp2p_peerstore::{Peerstore, PeerAccess};
 //! use multiaddr::Multiaddr;
 //! use std::time::Duration;
 //!
 //! // In this example we use a `MemoryPeerstore`, but you can easily swap it for another backend.
 //! let mut peerstore = MemoryPeerstore::empty();
-//! let peer_id = PeerId::from_public_key(&[1, 2, 3, 4]);
+//! let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3, 4]));
 //!
 //! // Let's write some information about a peer.
 //! {

peerstore/src/peerstore_tests.rs

@@ -33,13 +33,14 @@ macro_rules! peerstore_tests {
         use std::thread;
         use std::time::Duration;
         use {Peerstore, PeerAccess, PeerId};
+        use libp2p_core::PublicKeyBytesSlice;
         use multiaddr::Multiaddr;
 
         #[test]
         fn initially_empty() {
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
             assert_eq!(peer_store.peers().count(), 0);
             assert!(peer_store.peer(&peer_id).is_none());
         }
@@ -48,7 +49,7 @@ macro_rules! peerstore_tests {
         fn set_then_get_addr() {
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
             let addr = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
 
             peer_store.peer_or_create(&peer_id).add_addr(addr.clone(), Duration::from_millis(5000));
@@ -62,7 +63,7 @@ macro_rules! peerstore_tests {
             // Add an already-expired address to a peer.
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
             let addr = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
 
             peer_store.peer_or_create(&peer_id).add_addr(addr.clone(), Duration::from_millis(0));
@@ -76,7 +77,7 @@ macro_rules! peerstore_tests {
         fn clear_addrs() {
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
             let addr = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
 
             peer_store.peer_or_create(&peer_id)
@@ -91,7 +92,7 @@ macro_rules! peerstore_tests {
         fn no_update_ttl() {
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
 
             let addr1 = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
             let addr2 = "/ip4/0.0.0.1/tcp/0".parse::<Multiaddr>().unwrap();
@@ -112,7 +113,7 @@ macro_rules! peerstore_tests {
         fn force_update_ttl() {
             $($stmt;)*
             let peer_store = $create_peerstore;
-            let peer_id = PeerId::from_public_key(&[1, 2, 3]);
+            let peer_id = PeerId::from_public_key(PublicKeyBytesSlice(&[1, 2, 3]));
 
             let addr1 = "/ip4/0.0.0.0/tcp/0".parse::<Multiaddr>().unwrap();
             let addr2 = "/ip4/0.0.0.1/tcp/0".parse::<Multiaddr>().unwrap();

secio/src/lib.rs

@@ -99,7 +99,7 @@ pub use self::error::SecioError;
 use bytes::{Bytes, BytesMut};
 use futures::stream::MapErr as StreamMapErr;
 use futures::{Future, Poll, Sink, StartSend, Stream};
-use libp2p_core::{Multiaddr, PeerId};
+use libp2p_core::{Multiaddr, PeerId, PublicKeyBytes, PublicKeyBytesSlice};
 use ring::signature::{Ed25519KeyPair, RSAKeyPair};
 use ring::rand::SystemRandom;
 use rw_stream_sink::RwStreamSink;
@@ -210,10 +210,10 @@ impl SecioKeyPair {
     pub fn to_peer_id(&self) -> PeerId {
         match self.inner {
             SecioKeyPairInner::Rsa { ref public, .. } => {
-                PeerId::from_public_key(&public)
+                PublicKeyBytesSlice(&public).into()
             },
             SecioKeyPairInner::Ed25519 { ref key_pair } => {
-                PeerId::from_public_key(key_pair.public_key_bytes())
+                PublicKeyBytesSlice(key_pair.public_key_bytes()).into()
             },
         }
     }
@@ -236,7 +236,7 @@ enum SecioKeyPairInner {
 }
 
 /// Public key used by the remote.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum SecioPublicKey {
     /// DER format.
     Rsa(Vec<u8>),
@@ -246,13 +246,28 @@ pub enum SecioPublicKey {
 }
 
 impl SecioPublicKey {
+    /// Turns this public key into a raw representation.
+    #[inline]
+    pub fn as_raw(&self) -> PublicKeyBytesSlice {
+        match self {
+            SecioPublicKey::Rsa(ref data) => PublicKeyBytesSlice(data),
+            SecioPublicKey::Ed25519(ref data) => PublicKeyBytesSlice(data),
+        }
+    }
+
+    /// Turns this public key into a raw representation.
+    #[inline]
+    pub fn into_raw(self) -> PublicKeyBytes {
+        match self {
+            SecioPublicKey::Rsa(data) => PublicKeyBytes(data),
+            SecioPublicKey::Ed25519(data) => PublicKeyBytes(data),
+        }
+    }
+
     /// Builds a `PeerId` corresponding to the public key of the node.
     #[inline]
     pub fn to_peer_id(&self) -> PeerId {
-        match self {
+        self.as_raw().into()
-            &SecioPublicKey::Rsa(ref data) => PeerId::from_public_key(data),
-            &SecioPublicKey::Ed25519(ref data) => PeerId::from_public_key(data),
-        }
     }
 }
 
@@ -263,6 +278,13 @@ impl From<SecioPublicKey> for PeerId {
     }
 }
 
+impl From<SecioPublicKey> for PublicKeyBytes {
+    #[inline]
+    fn from(key: SecioPublicKey) -> PublicKeyBytes {
+        key.into_raw()
+    }
+}
+
 impl<S> libp2p_core::ConnectionUpgrade<S> for SecioConfig
 where
     S: AsyncRead + AsyncWrite + 'static,