From 999d165c68f448a6444fb51895c6e75c7226d297 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sat, 19 Jun 2021 11:18:10 +0200 Subject: [PATCH 1/4] FFI for pubkey comparison ops --- secp256k1-sys/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 6f61439..6d6a6c7 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -379,6 +379,12 @@ extern "C" { tweak: *const c_uchar) -> 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_keypair_sec")] pub fn secp256k1_keypair_sec(cx: *const Context, output_seckey: *mut c_uchar, @@ -390,6 +396,12 @@ extern "C" { output_pubkey: *mut PublicKey, keypair: *const KeyPair) -> 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(not(fuzzing))] From 0faf404f0e6ab88a840185f9b899c9c0a569a0d3 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sun, 20 Jun 2021 17:58:50 +0200 Subject: [PATCH 2/4] Benchmark for key ordering --- src/key.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/key.rs b/src/key.rs index 4f715d8..4eec0f6 100644 --- a/src/key.rs +++ b/src/key.rs @@ -2204,3 +2204,24 @@ mod test { assert_eq!(got, want) } } + +#[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(); + }) + } +} From 739660499b0b417a438864f3e78a3ad7117f20f8 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sat, 19 Jun 2021 11:25:06 +0200 Subject: [PATCH 3/4] Implement PublicKey ordering using FFI Instead of selializing the key we can call down to the ffi layer to do ordering. Co-authored-by: Tobin C. Harding --- src/key.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/key.rs b/src/key.rs index 4eec0f6..7bf5997 100644 --- a/src/key.rs +++ b/src/key.rs @@ -673,13 +673,16 @@ impl<'de> serde::Deserialize<'de> for PublicKey { impl PartialOrd for PublicKey { fn partial_cmp(&self, other: &PublicKey) -> Option { - self.serialize().partial_cmp(&other.serialize()) + Some(self.cmp(other)) } } 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) } } From 13af51926a17f1f6b8a5797f6eb7ce0f1280b581 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Tue, 29 Jun 2021 13:19:18 +0200 Subject: [PATCH 4/4] Make key comparison non-fuzzable Feature guard the custom implementations of `Ord` and `PartialOrd` on `cfg(not(fuzzing))`. When fuzzing, auto-derive implementations. Co-authored-by: Tobin C. Harding --- secp256k1-sys/src/lib.rs | 25 +++++++++++++------------ src/key.rs | 6 +++++- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 6d6a6c7..cc9047f 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -379,12 +379,6 @@ extern "C" { tweak: *const c_uchar) -> 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_keypair_sec")] pub fn secp256k1_keypair_sec(cx: *const Context, output_seckey: *mut c_uchar, @@ -396,12 +390,6 @@ extern "C" { output_pubkey: *mut PublicKey, keypair: *const KeyPair) -> 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(not(fuzzing))] @@ -446,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, @@ -552,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, diff --git a/src/key.rs b/src/key.rs index 7bf5997..e72f4a8 100644 --- a/src/key.rs +++ b/src/key.rs @@ -82,6 +82,7 @@ pub const ONE_KEY: SecretKey = SecretKey([0, 0, 0, 0, 0, 0, 0, 0, /// # } /// ``` #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] +#[cfg_attr(feature = "fuzzing", derive(PartialOrd, Ord))] #[repr(transparent)] pub struct PublicKey(ffi::PublicKey); @@ -671,12 +672,14 @@ impl<'de> serde::Deserialize<'de> for PublicKey { } } +#[cfg(not(fuzzing))] impl PartialOrd for PublicKey { fn partial_cmp(&self, other: &PublicKey) -> Option { Some(self.cmp(other)) } } +#[cfg(not(fuzzing))] impl Ord for PublicKey { fn cmp(&self, other: &PublicKey) -> core::cmp::Ordering { let ret = unsafe { @@ -1899,6 +1902,7 @@ mod test { assert_eq!(Ok(sksum), sum1); } + #[cfg(not(fuzzing))] #[test] fn pubkey_equal() { let pk1 = PublicKey::from_slice( @@ -1909,7 +1913,7 @@ mod test { &hex!("02e6642fd69bd211f93f7f1f36ca51a26a5290eb2dd1b0d8279a87bb0d480c8443"), ).unwrap(); - assert!(pk1 == pk2); + assert_eq!(pk1, pk2); assert!(pk1 <= pk2); assert!(pk2 <= pk1); assert!(!(pk2 < pk1));