Merge rust-bitcoin/rust-secp256k1#449: Re-implement public key ordering using underlying FFI functions

13af51926a17f1f6b8a5797f6eb7ce0f1280b581 Make key comparison non-fuzzable (Dr Maxim Orlovsky)
739660499b0b417a438864f3e78a3ad7117f20f8 Implement PublicKey ordering using FFI (Dr Maxim Orlovsky)
0faf404f0e6ab88a840185f9b899c9c0a569a0d3 Benchmark for key ordering (Dr Maxim Orlovsky)
999d165c68f448a6444fb51895c6e75c7226d297 FFI for pubkey comparison ops (Dr Maxim Orlovsky)

Pull request description:

  Re-base #309 for @dr-orlovsky on request by @Kixunil.

  To do the rebase I just had to change instances of cfg_attr to use `v0_5_0` instead of `v0_4_1` e.g.,
  ```
      #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_xonly_pubkey_cmp")]
  ```

  And drop the changes to `src/schnorrsig.rs`, all these changes are covered by the changes in `key.rs` I believe.

ACKs for top commit:
  Kixunil:
    ACK 13af51926a17f1f6b8a5797f6eb7ce0f1280b581
  apoelstra:
    ACK 13af51926a17f1f6b8a5797f6eb7ce0f1280b581

Tree-SHA512: 3054fcbc1707679f54466cdc91162c286394ad691e4f5c8ee18635a22b0854a4e60f1186ef3ca1532aacd8a637d0a153601ec203947e9e58dfcebf1bcb619955

Author: william2332-limf

secp256k1-sys/src/lib.rs

@@ -434,6 +434,12 @@ extern "C" {
                                       pk: *mut PublicKey) -> c_int;
 
 
+    #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_ec_pubkey_cmp")]
+    pub fn secp256k1_ec_pubkey_cmp(cx: *const Context,
+                                   pubkey1: *const PublicKey,
+                                   pubkey2: *const PublicKey)
+                                   -> c_int;
+
     #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_ec_pubkey_tweak_add")]
     pub fn secp256k1_ec_pubkey_tweak_add(cx: *const Context,
                                          pk: *mut PublicKey,
@@ -540,6 +546,13 @@ extern "C" {
         pubkey: *const PublicKey,
     ) -> c_int;
 
+    #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_xonly_pubkey_cmp")]
+    pub fn secp256k1_xonly_pubkey_cmp(
+        cx: *const Context,
+        pubkey1: *const XOnlyPublicKey,
+        pubkey2: *const XOnlyPublicKey
+    ) -> c_int;
+
     #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_xonly_pubkey_tweak_add")]
     pub fn secp256k1_xonly_pubkey_tweak_add(
         cx: *const Context,

src/key.rs

@@ -101,6 +101,7 @@ pub const ONE_KEY: SecretKey = SecretKey([0, 0, 0, 0, 0, 0, 0, 0,
 /// [`bincode`]: https://docs.rs/bincode
 /// [`cbor`]: https://docs.rs/cbor
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)]
+#[cfg_attr(feature = "fuzzing", derive(PartialOrd, Ord))]
 #[repr(transparent)]
 pub struct PublicKey(ffi::PublicKey);
 
@@ -763,15 +764,20 @@ impl<'de> serde::Deserialize<'de> for PublicKey {
     }
 }
 
+#[cfg(not(fuzzing))]
 impl PartialOrd for PublicKey {
     fn partial_cmp(&self, other: &PublicKey) -> Option<core::cmp::Ordering> {
-        self.serialize().partial_cmp(&other.serialize())
+        Some(self.cmp(other))
     }
 }
 
+#[cfg(not(fuzzing))]
 impl Ord for PublicKey {
     fn cmp(&self, other: &PublicKey) -> core::cmp::Ordering {
-        self.serialize().cmp(&other.serialize())
+        let ret = unsafe {
+            ffi::secp256k1_ec_pubkey_cmp(ffi::secp256k1_context_no_precomp, self.as_c_ptr(), other.as_c_ptr())
+        };
+        ret.cmp(&0i32)
     }
 }
 
@@ -2069,6 +2075,7 @@ mod test {
         assert_eq!(Ok(sksum), sum1);
     }
 
+    #[cfg(not(fuzzing))]
     #[test]
     fn pubkey_equal() {
         let pk1 = PublicKey::from_slice(
@@ -2079,7 +2086,7 @@ mod test {
             &hex!("02e6642fd69bd211f93f7f1f36ca51a26a5290eb2dd1b0d8279a87bb0d480c8443"),
         ).unwrap();
 
-        assert!(pk1 == pk2);
+        assert_eq!(pk1, pk2);
         assert!(pk1 <= pk2);
         assert!(pk2 <= pk1);
         assert!(!(pk2 < pk1));
@@ -2430,3 +2437,24 @@ mod test {
         assert_tokens(&pk.readable(), &[Token::String(PK_STR)]);
     }
 }
+
+#[cfg(all(test, feature = "unstable"))]
+mod benches {
+    use test::Bencher;
+    use std::collections::BTreeSet;
+    use crate::PublicKey;
+    use crate::constants::GENERATOR_X;
+
+    #[bench]
+    fn bench_pk_ordering(b: &mut Bencher) {
+        let mut map = BTreeSet::new();
+        let mut g_slice = [02u8; 33];
+        g_slice[1..].copy_from_slice(&GENERATOR_X);
+        let g = PublicKey::from_slice(&g_slice).unwrap();
+        let mut pk = g;
+        b.iter(|| {
+            map.insert(pk);
+            pk = pk.combine(&pk).unwrap();
+        })
+    }
+}