Skip to content

Commit

Permalink
Remove derives for ffi wrapped types
Browse files Browse the repository at this point in the history
Currently we are deriving various traits that call down to the traits
implemented in secp265k1-sys that in turn call down to implementations
on the inner byte array. Since we cannot guarantee the inner byte arrays
in secp256k1-sys this results in trait implementations that may not be
stable across versions of the library.

For `XOnlyPublicKey`, `KeyPair`, and `ecdsa::Signature` manually
implement `PartialEq`, `Eq`, `PartialOrd`, `Ord`, and `Hash` by first
converting the type into a form that is guaranteed to be stable across
library versions e.g., by serializing it to a known format.
  • Loading branch information
tcharding committed Nov 16, 2022
1 parent 5a54694 commit 64a22fe
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 3 deletions.
32 changes: 31 additions & 1 deletion src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ pub use serialized_signature::SerializedSignature;
use crate::SECP256K1;

/// An ECDSA signature
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Copy, Clone)]
#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
pub struct Signature(pub(crate) ffi::Signature);

impl fmt::Debug for Signature {
Expand Down Expand Up @@ -197,6 +198,35 @@ impl Signature {
}
}

impl PartialOrd for Signature {
fn partial_cmp(&self, other: &Signature) -> Option<core::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl Ord for Signature {
fn cmp(&self, other: &Signature) -> core::cmp::Ordering {
let this = self.serialize_compact();
let that = other.serialize_compact();
this.cmp(&that)
}
}

impl PartialEq for Signature {
fn eq(&self, other: &Self) -> bool {
self.cmp(other) == core::cmp::Ordering::Equal
}
}

impl Eq for Signature {}

impl core::hash::Hash for Signature {
fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
let ser = self.serialize_compact();
ser.hash(state);
}
}

impl CPtr for Signature {
type Target = ffi::Signature;

Expand Down
79 changes: 77 additions & 2 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,8 @@ impl core::hash::Hash for PublicKey {
/// ```
/// [`bincode`]: https://docs.rs/bincode
/// [`cbor`]: https://docs.rs/cbor
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Copy, Clone)]
#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
pub struct KeyPair(ffi::KeyPair);
impl_display_secret!(KeyPair);

Expand Down Expand Up @@ -1007,6 +1008,44 @@ impl KeyPair {
}
}

#[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 keypair 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 keypair to the digest.
let pk = self.public_key();
let ser = pk.serialize();
ser.hash(state);
}
}

impl From<KeyPair> for SecretKey {
#[inline]
fn from(pair: KeyPair) -> Self {
Expand Down Expand Up @@ -1135,7 +1174,8 @@ impl CPtr for KeyPair {
/// ```
/// [`bincode`]: https://docs.rs/bincode
/// [`cbor`]: https://docs.rs/cbor
#[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)]
#[derive(Copy, Clone, Debug)]
#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
pub struct XOnlyPublicKey(ffi::XOnlyPublicKey);

impl fmt::LowerHex for XOnlyPublicKey {
Expand Down Expand Up @@ -1364,6 +1404,41 @@ impl XOnlyPublicKey {
}
}

#[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 {
ffi::secp256k1_xonly_pubkey_cmp(ffi::secp256k1_context_no_precomp, self.as_c_ptr(), other.as_c_ptr())
};
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);
}
}

/// Represents the parity passed between FFI function calls.
#[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)]
pub enum Parity {
Expand Down

0 comments on commit 64a22fe

Please sign in to comment.