Skip to content

Commit

Permalink
Merge #449: Re-implement public key ordering using underlying FFI fun…
Browse files Browse the repository at this point in the history
…ctions

13af519 Make key comparison non-fuzzable (Dr Maxim Orlovsky)
7396604 Implement PublicKey ordering using FFI (Dr Maxim Orlovsky)
0faf404 Benchmark for key ordering (Dr Maxim Orlovsky)
999d165 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 13af519
  apoelstra:
    ACK 13af519

Tree-SHA512: 3054fcbc1707679f54466cdc91162c286394ad691e4f5c8ee18635a22b0854a4e60f1186ef3ca1532aacd8a637d0a153601ec203947e9e58dfcebf1bcb619955
  • Loading branch information
apoelstra committed Jun 15, 2022
2 parents 4dacf55 + 13af519 commit aba2663
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
13 changes: 13 additions & 0 deletions secp256k1-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 31 additions & 3 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -2069,6 +2075,7 @@ mod test {
assert_eq!(Ok(sksum), sum1);
}

#[cfg(not(fuzzing))]
#[test]
fn pubkey_equal() {
let pk1 = PublicKey::from_slice(
Expand All @@ -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));
Expand Down Expand Up @@ -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();
})
}
}

0 comments on commit aba2663

Please sign in to comment.