Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove derives for ffi wrapped types #515

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ 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_fast_comparisons!(Signature);

impl fmt::Debug for Signature {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -197,6 +199,40 @@ impl Signature {
}
}

#[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_compact();
let that = other.serialize_compact();
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_compact();
ser.hash(state);
}
}

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

Expand Down
82 changes: 80 additions & 2 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ pub const ONE_KEY: SecretKey = SecretKey(constants::ONE);
#[cfg_attr(fuzzing, derive(PartialOrd, Ord, PartialEq, Eq, Hash))]
#[repr(transparent)]
pub struct PublicKey(ffi::PublicKey);
impl_fast_comparisons!(PublicKey);

impl fmt::LowerHex for PublicKey {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -798,9 +799,11 @@ 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);
impl_fast_comparisons!(KeyPair);

impl KeyPair {
/// Obtains a raw const pointer suitable for use with FFI functions.
Expand Down Expand Up @@ -1007,6 +1010,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,8 +1176,10 @@ 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_fast_comparisons!(XOnlyPublicKey);

impl fmt::LowerHex for XOnlyPublicKey {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -1364,6 +1407,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
29 changes: 29 additions & 0 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,32 @@ macro_rules! write_err {
}
}
}

/// Implements fast unstable versions of Ord, PartialOrd, Eq, PartialEq, and Hash.
macro_rules! impl_fast_comparisons {
($ty:ident) => {
impl $ty {
/// Equivalent to `Ord` but faster and not stable across library versions.
///
/// The `Ord` implementation for `Self` is stable but slow because we first serialize
/// `self` and `other` before comparing them. The `Ord` implementation for FFI types
/// compares the inner bytes directly. The inner bytes are passed across the FFI boundry
/// and as such there are no guarantees to the layout of the bytes. The layout may
/// change unexpectedly between versions of the library, even minor versions.
pub fn cmp_fast_unstable(&self, other: &Self) -> core::cmp::Ordering {
self.0.cmp(&other.0)
}

/// Equivalent to `Eq` but faster and not stable across library versions.
///
/// The `Eq` implementation for `Self` is stable but slow because we first serialize
/// `self` and `other` before comparing them. The `Eq` implementation for FFI types
/// compares the inner bytes directly. The inner bytes are passed across the FFI boundry
/// and as such there are no guarantees to the layout of the bytes. The layout may
/// change unexpectedly between versions of the library, even minor versions.
pub fn eq_fast_unstable(&self, other: &Self) -> bool {
self.0.eq(&other.0)
}
}
}
}