Implement stable comparison functionality

Currently we rely on the inner bytes with types that are passed across
the FFI boundry when implementing comparison functions (e.g. `Ord`,
`PartialEq`), this is incorrect because the bytes are opaque, meaning
the byte layout is not guaranteed across versions of `libsecp26k1`.

Implement stable comparison functionality by doing:

- Implement `core::cmp` traits by first coercing the data into a stable
  form e.g., by serializing it.
- Add fast comparison methods to `secp256k1-sys` types that wrap types
  from libsecp, add similar methods to types in `secp256k1` that wrap
  `secp256k1-sys` types (just call through to inner type).
- In `secp256k1-sys` feature gate the new `core::cmp` impls on
  `not(fuzzing)`, when fuzzing just derive the impls instead.

Any additional methods added to `secp256k1-sys` types are private,
justified by the fact the -sys is meant to be just a thin wrapper around
libsecp256k1, we don't want to commit to supporting additional API
functions.

secp256k1-sys/src/lib.rs

@@ -133,6 +133,7 @@ impl SchnorrSigExtraParams {
 
 /// Library-internal representation of a Secp256k1 public key
 #[repr(C)]
+#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
 pub struct PublicKey([c_uchar; 64]);
 impl_array_newtype!(PublicKey, c_uchar, 64);
 impl_raw_debug!(PublicKey);
@@ -169,10 +170,64 @@ impl PublicKey {
     pub fn underlying_bytes(self) -> [c_uchar; 64] {
         self.0
     }
+
+    /// Serializes this public key as a byte-encoded pair of values, in compressed form.
+    fn serialize(&self) -> [u8; 33] {
+        let mut buf = [0u8; 33];
+        let mut len = 33;
+        unsafe {
+            let ret = secp256k1_ec_pubkey_serialize(
+                secp256k1_context_no_precomp,
+                buf.as_mut_c_ptr(),
+                &mut len,
+                self,
+                SECP256K1_SER_COMPRESSED,
+            );
+            debug_assert_eq!(ret, 1);
+            debug_assert_eq!(len, 33);
+        };
+        buf
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialOrd for PublicKey {
+    fn partial_cmp(&self, other: &PublicKey) -> Option<core::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Ord for PublicKey {
+    fn cmp(&self, other: &PublicKey) -> core::cmp::Ordering {
+        let ret = unsafe {
+            secp256k1_ec_pubkey_cmp(secp256k1_context_no_precomp, self, other)
+        };
+        ret.cmp(&0i32)
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialEq for PublicKey {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == core::cmp::Ordering::Equal
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Eq for PublicKey {}
+
+#[cfg(not(fuzzing))]
+impl core::hash::Hash for PublicKey {
+    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+        let ser = self.serialize();
+        ser.hash(state);
+    }
 }
 
 /// Library-internal representation of a Secp256k1 signature
 #[repr(C)]
+#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
 pub struct Signature([c_uchar; 64]);
 impl_array_newtype!(Signature, c_uchar, 64);
 impl_raw_debug!(Signature);
@@ -209,9 +264,58 @@ impl Signature {
     pub fn underlying_bytes(self) -> [c_uchar; 64] {
         self.0
     }
+
+    /// Serializes the signature in compact format.
+    fn serialize(&self) -> [u8; 64] {
+        let mut buf = [0u8; 64];
+        unsafe {
+            let ret = secp256k1_ecdsa_signature_serialize_compact(
+                secp256k1_context_no_precomp,
+                buf.as_mut_c_ptr(),
+                self,
+            );
+            debug_assert!(ret == 1);
+        }
+        buf
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialOrd for Signature {
+    fn partial_cmp(&self, other: &Signature) -> Option<core::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Ord for Signature {
+    fn cmp(&self, other: &Signature) -> core::cmp::Ordering {
+        let this = self.serialize();
+        let that = other.serialize();
+        this.cmp(&that)
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialEq for Signature {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == core::cmp::Ordering::Equal
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Eq for Signature {}
+
+#[cfg(not(fuzzing))]
+impl core::hash::Hash for Signature {
+    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+        let ser = self.serialize();
+        ser.hash(state);
+    }
 }
 
 #[repr(C)]
+#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
 pub struct XOnlyPublicKey([c_uchar; 64]);
 impl_array_newtype!(XOnlyPublicKey, c_uchar, 64);
 impl_raw_debug!(XOnlyPublicKey);
@@ -248,9 +352,59 @@ impl XOnlyPublicKey {
     pub fn underlying_bytes(self) -> [c_uchar; 64] {
         self.0
     }
+
+    /// Serializes this key as a byte-encoded x coordinate value (32 bytes).
+    fn serialize(&self) -> [u8; 32] {
+        let mut buf = [0u8; 32];
+        unsafe {
+            let ret = secp256k1_xonly_pubkey_serialize(
+                secp256k1_context_no_precomp,
+                buf.as_mut_c_ptr(),
+                self,
+            );
+            assert_eq!(ret, 1);
+        };
+        buf
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialOrd for XOnlyPublicKey {
+    fn partial_cmp(&self, other: &XOnlyPublicKey) -> Option<core::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Ord for XOnlyPublicKey {
+    fn cmp(&self, other: &XOnlyPublicKey) -> core::cmp::Ordering {
+        let ret = unsafe {
+            secp256k1_xonly_pubkey_cmp(secp256k1_context_no_precomp, self, other)
+        };
+        ret.cmp(&0i32)
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialEq for XOnlyPublicKey {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == core::cmp::Ordering::Equal
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Eq for XOnlyPublicKey {}
+
+#[cfg(not(fuzzing))]
+impl core::hash::Hash for XOnlyPublicKey {
+    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+        let ser = self.serialize();
+        ser.hash(state);
+    }
 }
 
 #[repr(C)]
+#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
 pub struct KeyPair([c_uchar; 96]);
 impl_array_newtype!(KeyPair, c_uchar, 96);
 impl_raw_debug!(KeyPair);
@@ -287,6 +441,58 @@ impl KeyPair {
     pub fn underlying_bytes(self) -> [c_uchar; 96] {
         self.0
     }
+
+    /// Creates a new compressed public key from this key pair.
+    fn public_key(&self) -> PublicKey {
+        unsafe {
+            let mut pk = PublicKey::new();
+            let ret = secp256k1_keypair_pub(
+                secp256k1_context_no_precomp,
+                &mut pk,
+                self,
+            );
+            debug_assert_eq!(ret, 1);
+            pk
+        }
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialOrd for KeyPair {
+    fn partial_cmp(&self, other: &KeyPair) -> Option<core::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Ord for KeyPair {
+    fn cmp(&self, other: &KeyPair) -> core::cmp::Ordering {
+        let this = self.public_key();
+        let that = other.public_key();
+        this.cmp(&that)
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl PartialEq for KeyPair {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == core::cmp::Ordering::Equal
+    }
+}
+
+#[cfg(not(fuzzing))]
+impl Eq for KeyPair {}
+
+#[cfg(not(fuzzing))]
+impl core::hash::Hash for KeyPair {
+    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+        // To hash the key pair we just hash the serialized public key. Since any change to the
+        // secret key would also be a change to the public key this is a valid one way function from
+        // the key pair to the digest.
+        let pk = self.public_key();
+        let ser = pk.serialize();
+        ser.hash(state);
+    }
 }
 
 extern "C" {

secp256k1-sys/src/macros.rs

@@ -19,41 +19,40 @@ macro_rules! impl_array_newtype {
     ($thing:ident, $ty:ty, $len:expr) => {
         impl Copy for $thing {}
 
-        impl AsRef<[$ty; $len]> for $thing {
-            #[inline]
-            /// Gets a reference to the underlying array
-            fn as_ref(&self) -> &[$ty; $len] {
-                let &$thing(ref dat) = self;
-                dat
+        impl $thing {
+            /// Like `cmp::Ord` but faster and with no guarantees across library versions.
+            ///
+            /// The inner byte array of `Self` is passed across the FFI boundry, as such there are
+            /// no guarantees on its layout and it is subject to change across library versions,
+            /// even minor versions. For this reason comparison function implementations (e.g.
+            /// `Ord`, `PartialEq`) take measures to ensure the data will remain constant (e.g., by
+            /// serializing it to a guaranteed format). This means they may be slow, this function
+            /// provides a faster comparison if you know that your types come from the same library
+            /// version.
+            pub fn cmp_fast_unstable(&self, other: &Self) -> core::cmp::Ordering {
+                self.0.cmp(&other.0)
             }
-        }
 
-        impl PartialEq for $thing {
-            #[inline]
-            fn eq(&self, other: &$thing) -> bool {
-                &self[..] == &other[..]
+            /// Like `cmp::Eq` but faster and with no guarantees across library versions.
+            ///
+            /// The inner byte array of `Self` is passed across the FFI boundry, as such there are
+            /// no guarantees on its layout and it is subject to change across library versions,
+            /// even minor versions. For this reason comparison function implementations (e.g.
+            /// `Ord`, `PartialEq`) take measures to ensure the data will remain constant (e.g., by
+            /// serializing it to a guaranteed format). This means they may be slow, this function
+            /// provides a faster equality check if you know that your types come from the same
+            /// library version.
+            pub fn eq_fast_unstable(&self, other: &Self) -> bool {
+                self.0.eq(&other.0)
             }
         }
 
-        impl Eq for $thing {}
-
-        impl core::hash::Hash for $thing {
-            fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
-                (&self[..]).hash(state)
-            }
-        }
-
-        impl PartialOrd for $thing {
-            #[inline]
-            fn partial_cmp(&self, other: &$thing) -> Option<core::cmp::Ordering> {
-                self[..].partial_cmp(&other[..])
-            }
-        }
-
-        impl Ord for $thing {
+        impl AsRef<[$ty; $len]> for $thing {
             #[inline]
-            fn cmp(&self, other: &$thing) -> core::cmp::Ordering {
-                self[..].cmp(&other[..])
+            /// Gets a reference to the underlying array
+            fn as_ref(&self) -> &[$ty; $len] {
+                let &$thing(ref dat) = self;
+                dat
             }
         }
 

secp256k1-sys/src/recovery.rs

@@ -21,6 +21,7 @@ use core::fmt;
 
 /// Library-internal representation of a Secp256k1 signature + recovery ID
 #[repr(C)]
+#[derive(PartialOrd, Ord, PartialEq, Eq, Hash)]
 pub struct RecoverableSignature([c_uchar; 65]);
 impl_array_newtype!(RecoverableSignature, c_uchar, 65);
 
@@ -169,4 +170,4 @@ mod fuzz_dummy {
 }
 
 #[cfg(fuzzing)]
-pub use self::fuzz_dummy::*;
+pub use self::fuzz_dummy::*;

src/ecdsa/mod.rs

@@ -20,8 +20,9 @@ pub use serialized_signature::SerializedSignature;
 use crate::SECP256K1;
 
 /// An ECDSA signature
-#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
 pub struct Signature(pub(crate) ffi::Signature);
+impl_fast_comparisons!(Signature);
 
 impl fmt::Debug for Signature {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {