core/identity: Implement Hash and Ord for PublicKey

core/src/identity.rs

@@ -381,54 +381,20 @@ mod tests {
     }
 
     #[test]
-    fn public_key_hash() {
-        use std::collections::*;
+    fn public_key_implements_hash() {
+        use std::hash::Hash;
 
-        let mut keypairs = Vec::new();
+        fn assert_implements_hash<T: Hash>() {}
 
-        for _ in 0..8 {
-            keypairs.push(Keypair::generate_ed25519());
-            keypairs.push(Keypair::generate_secp256k1());
-            keypairs.push(Keypair::generate_ecdsa());
-        }
-
-        let public_keys: Vec<_> = keypairs
-            .iter()
-            .map(|k| k.public())
-            .collect();
-
-        let public_keys_hash_set: HashSet<PublicKey> = HashSet::from_iter(
-            public_keys
-                .iter()
-                .map(|p| p.clone())
-        );
-
-        assert_eq!(public_keys_hash_set.len(), public_keys.len());
+        assert_implements_hash::<PublicKey>();
     }
 
     #[test]
-    fn public_key_ord() {
-        use std::collections::*;
-
-        let mut keypairs = Vec::new();
-
-        for _ in 0..8 {
-            keypairs.push(Keypair::generate_ed25519());
-            keypairs.push(Keypair::generate_secp256k1());
-            keypairs.push(Keypair::generate_ecdsa());
-        }
-
-        let public_keys: Vec<_> = keypairs
-            .iter()
-            .map(|k| k.public())
-            .collect();
+    fn public_key_implements_ord() {
+        use std::cmp::Ord;
 
-        let public_keys_b_tree_set: BTreeSet<PublicKey> = BTreeSet::from_iter(
-            public_keys
-                .iter()
-                .map(|p| p.clone())
-        );
+        fn assert_implements_ord<T: Ord>() {}
 
-        assert_eq!(public_keys_b_tree_set.len(), public_keys.len());
+        assert_implements_ord::<PublicKey>();
     }
 }

core/src/identity/ecdsa.rs

@@ -223,6 +223,7 @@ impl fmt::Debug for PublicKey {
     }
 }
 
+#[allow(clippy::derive_hash_xor_eq)]
 impl hash::Hash for PublicKey {
     fn hash<H: hash::Hasher>(&self, state: &mut H) {
         self.to_bytes().hash(state);
@@ -232,6 +233,24 @@ impl hash::Hash for PublicKey {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test::*;
+    use quickcheck::*;
+
+    #[derive(Clone, Debug)]
+    struct SomePublicKey(PublicKey);
+
+    impl Arbitrary for SomePublicKey {
+        fn arbitrary<G: Gen>(g: &mut G) -> SomePublicKey {
+            loop {
+                let mut bytes = Vec::<u8>::arbitrary(g);
+                bytes.resize(33, 0);
+                let public_key = PublicKey::from_bytes(&bytes);
+                if let Ok(public_key) = public_key {
+                    return SomePublicKey(public_key);
+                }
+            }
+        }
+    }
 
     #[test]
     fn sign_verify() {
@@ -249,4 +268,13 @@ mod tests {
         let invalid_msg = "h3ll0 w0rld".as_bytes();
         assert!(!pk.verify(invalid_msg, &sig));
     }
+
+    #[test]
+    fn ecdsa_public_key_eq_implies_hash() {
+        fn prop(SomePublicKey(pub1): SomePublicKey, SomePublicKey(pub2): SomePublicKey) -> bool {
+            pub1 != pub2 || hash(&pub1) == hash(&pub2)
+        }
+        QuickCheck::new()
+            .quickcheck(prop as fn(_, _) -> _);
+    }
 }

core/src/identity/ed25519.rs

@@ -130,6 +130,7 @@ impl fmt::Debug for PublicKey {
 
 // Remove (derive instead) when this PR is merged:
 // https://github.com/dalek-cryptography/ed25519-dalek/pull/176
+#[allow(clippy::derive_hash_xor_eq)]
 impl hash::Hash for PublicKey {
     fn hash<H: hash::Hasher>(&self, state: &mut H) {
         self.0.as_bytes().hash(state);
@@ -220,8 +221,25 @@ impl SecretKey {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test::*;
     use quickcheck::*;
 
+    #[derive(Clone, Debug)]
+    struct SomePublicKey(PublicKey);
+
+    impl Arbitrary for SomePublicKey {
+        fn arbitrary<G: Gen>(g: &mut G) -> SomePublicKey {
+            loop {
+                let mut bytes = Vec::<u8>::arbitrary(g);
+                bytes.resize(32, 0);
+                let public_key = PublicKey::decode(&mut bytes);
+                if let Ok(public_key) = public_key {
+                    return SomePublicKey(public_key);
+                }
+            }
+        }
+    }
+
     fn eq_keypairs(kp1: &Keypair, kp2: &Keypair) -> bool {
         kp1.public() == kp2.public() && kp1.0.secret.as_bytes() == kp2.0.secret.as_bytes()
     }
@@ -264,4 +282,13 @@ mod tests {
         let invalid_msg = "h3ll0 w0rld".as_bytes();
         assert!(!pk.verify(invalid_msg, &sig));
     }
+
+    #[test]
+    fn ecdsa_public_key_eq_implies_hash() {
+        fn prop(SomePublicKey(pub1): SomePublicKey, SomePublicKey(pub2): SomePublicKey) -> bool {
+            pub1 != pub2 || hash(&pub1) == hash(&pub2)
+        }
+        QuickCheck::new()
+            .quickcheck(prop as fn(_, _) -> _);
+    }
 }

core/src/identity/rsa.rs

@@ -70,7 +70,7 @@ impl Keypair {
 }
 
 /// An RSA public key.
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct PublicKey(Vec<u8>);
 
 impl PublicKey {
@@ -305,30 +305,33 @@ impl DerDecodable<'_> for Asn1SubjectPublicKeyInfo {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test::*;
     use quickcheck::*;
     use rand07::seq::SliceRandom;
-    use std::fmt;
 
     const KEY1: &'static [u8] = include_bytes!("test/rsa-2048.pk8");
     const KEY2: &'static [u8] = include_bytes!("test/rsa-3072.pk8");
     const KEY3: &'static [u8] = include_bytes!("test/rsa-4096.pk8");
 
-    #[derive(Clone)]
+    #[derive(Clone, Debug)]
     struct SomeKeypair(Keypair);
 
-    impl fmt::Debug for SomeKeypair {
-        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-            write!(f, "SomeKeypair")
-        }
-    }
-
     impl Arbitrary for SomeKeypair {
         fn arbitrary<G: Gen>(g: &mut G) -> SomeKeypair {
             let mut key = [KEY1, KEY2, KEY3].choose(g).unwrap().to_vec();
             SomeKeypair(Keypair::from_pkcs8(&mut key).unwrap())
         }
     }
 
+    #[derive(Clone, Debug)]
+    struct SomePublicKey(PublicKey);
+
+    impl Arbitrary for SomePublicKey {
+        fn arbitrary<G: Gen>(g: &mut G) -> SomePublicKey {
+            SomePublicKey(SomeKeypair::arbitrary(g).0.public())
+        }
+    }
+
     #[test]
     fn rsa_from_pkcs8() {
         assert!(Keypair::from_pkcs8(&mut KEY1.to_vec()).is_ok());
@@ -356,4 +359,13 @@ mod tests {
             .tests(10)
             .quickcheck(prop as fn(_, _) -> _);
     }
+
+    #[test]
+    fn rsa_public_key_eq_implies_hash() {
+        fn prop(SomePublicKey(pub1): SomePublicKey, SomePublicKey(pub2): SomePublicKey) -> bool {
+            pub1 != pub2 || hash(&pub1) == hash(&pub2)
+        }
+        QuickCheck::new()
+            .quickcheck(prop as fn(_, _) -> _);
+    }
 }

core/src/identity/secp256k1.rs

@@ -165,6 +165,7 @@ impl fmt::Debug for PublicKey {
     }
 }
 
+#[allow(clippy::derive_hash_xor_eq)]
 impl hash::Hash for PublicKey {
     fn hash<H: hash::Hasher>(&self, state: &mut H) {
         self.encode().hash(state);
@@ -219,6 +220,35 @@ impl PublicKey {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test::*;
+    use quickcheck::*;
+
+    #[derive(Clone, Debug)]
+    struct SomeSecretKey(SecretKey);
+
+    impl Arbitrary for SomeSecretKey {
+        fn arbitrary<G: Gen>(g: &mut G) -> SomeSecretKey {
+            loop {
+                let mut bytes = Vec::<u8>::arbitrary(g);
+                bytes.resize(32, 0);
+                let secret_key = SecretKey::from_bytes(&mut bytes);
+                if let Ok(secret_key) = secret_key {
+                    return SomeSecretKey(secret_key);
+                }
+            }
+        }
+    }
+
+    #[derive(Clone, Debug)]
+    struct SomePublicKey(PublicKey);
+
+    impl Arbitrary for SomePublicKey {
+        fn arbitrary<G: Gen>(g: &mut G) -> SomePublicKey {
+            let secret_key = SomeSecretKey::arbitrary(g).0;
+            let keypair: Keypair = secret_key.into();
+            SomePublicKey(keypair.public().clone())
+        }
+    }
 
     #[test]
     fn secp256k1_secret_from_bytes() {
@@ -229,4 +259,13 @@ mod tests {
         assert_eq!(sk1.0.serialize(), sk2.0.serialize());
         assert_eq!(sk_bytes, [0; 32]);
     }
+
+    #[test]
+    fn ecdsa_public_key_eq_implies_hash() {
+        fn prop(SomePublicKey(pub1): SomePublicKey, SomePublicKey(pub2): SomePublicKey) -> bool {
+            pub1 != pub2 || hash(&pub1) == hash(&pub2)
+        }
+        QuickCheck::new()
+            .quickcheck(prop as fn(_, _) -> _);
+    }
 }

core/src/lib.rs

@@ -77,6 +77,9 @@ pub use translation::address_translation;
 pub use transport::Transport;
 pub use upgrade::{InboundUpgrade, OutboundUpgrade, ProtocolName, UpgradeError, UpgradeInfo};
 
+#[cfg(test)]
+mod test;
+
 use std::{future::Future, pin::Pin};
 
 /// Implemented on objects that can run a `Future` in the background.

core/src/test.rs

@@ -0,0 +1,8 @@
+use std::hash::{Hash, Hasher};
+use std::collections::hash_map::DefaultHasher;
+
+pub fn hash<T: Hash>(t: &T) -> u64 {
+    let mut s = DefaultHasher::new();
+    t.hash(&mut s);
+    s.finish()
+}