Skip to content

Commit

Permalink
feat: Implement missing common traits for public types
Browse files Browse the repository at this point in the history
It is recommended to eagerly implement some traits, listed in
https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits

While many central types already implement some of these traits,
this commit fills in the gap with the remaining ones.
  • Loading branch information
netrome committed Mar 21, 2024
1 parent b471b7d commit 119529f
Show file tree
Hide file tree
Showing 7 changed files with 340 additions and 15 deletions.
56 changes: 54 additions & 2 deletions p256k1/src/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use serde::{
de::{self, Visitor},
Deserialize, Deserializer, Serialize, Serializer,
};
use std::array::TryFromSliceError;
use std::{array::TryFromSliceError, hash::Hash};

use crate::_rename::{
secp256k1_ecdsa_sign, secp256k1_ecdsa_signature_parse_compact,
Expand All @@ -25,12 +25,15 @@ pub enum Error {
/// Error converting a scalar
Conversion(ConversionError),
}

impl Display for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
write!(f, "{:?}", self)
}
}

impl std::error::Error for Error {}

impl From<TryFromSliceError> for Error {
fn from(e: TryFromSliceError) -> Self {
Error::TryFrom(e.to_string())
Expand Down Expand Up @@ -105,6 +108,32 @@ impl Display for Signature {
}
}

impl PartialEq for Signature {
fn eq(&self, other: &Self) -> bool {
self.to_bytes().eq(&other.to_bytes())
}
}

impl Eq for Signature {}

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

impl Ord for Signature {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.to_bytes().cmp(&other.to_bytes())
}
}

impl Hash for Signature {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
state.write(&self.to_bytes())
}
}

impl Serialize for Signature {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
Expand Down Expand Up @@ -197,7 +226,7 @@ mod tests {
use super::*;
use rand_core::{OsRng, RngCore};
use sha2::{Digest, Sha256};
use std::thread;
use std::{collections::HashSet, thread};

#[test]
fn signature_generation() {
Expand Down Expand Up @@ -305,4 +334,27 @@ mod tests {

assert!(dsig.verify(&hash, &public_key));
}

#[test]
fn hash() {
let msg = [0u8; 32];
let private_keys = [1, 2, 3, 4, 5].map(Scalar::from);
let signatures = private_keys.map(|pk| Signature::new(&msg, &pk).unwrap());

let signatures_hash_set: HashSet<_> = signatures.into();

assert_eq!(signatures_hash_set.len(), 5);
}

#[test]
fn sort() {
let msg = [0u8; 32];
let private_keys = [1, 2, 3, 4, 5].map(Scalar::from);
let mut signatures = private_keys.map(|pk| Signature::new(&msg, &pk).unwrap());
signatures.sort();

for idx in 0..4 {
assert!(signatures[idx] < signatures[idx + 1]);
}
}
}
9 changes: 9 additions & 0 deletions p256k1/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ pub enum ConversionError {
/// Error converting a base58-related value
Base58(Base58Error),
}

impl Display for ConversionError {
fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
write!(f, "{:?}", self)
}
}

impl std::error::Error for Base58Error {}
impl std::error::Error for ConversionError {}
38 changes: 38 additions & 0 deletions p256k1/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use serde::{
de::{self, Visitor},
Deserialize, Deserializer, Serialize, Serializer,
};
use std::cmp::Ordering;

use crate::_rename::{
secp256k1_fe_add, secp256k1_fe_cmp_var, secp256k1_fe_get_b32, secp256k1_fe_inv,
Expand Down Expand Up @@ -44,6 +45,8 @@ impl Display for Error {
}
}

impl std::error::Error for Error {}

#[derive(Copy, Clone, Debug)]
/**
Element is a wrapper around libsecp256k1's internal secp256k1_fe struct. It provides a field element, which is like a scalar but not necessarily reduced modulo the group order
Expand Down Expand Up @@ -170,6 +173,23 @@ impl PartialEq for Element {

impl Eq for Element {}

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

impl Ord for Element {
fn cmp(&self, other: &Self) -> Ordering {
match unsafe { secp256k1_fe_cmp_var(&self.fe, &other.fe) } {
-1 => Ordering::Less,
0 => Ordering::Equal,
1 => Ordering::Greater,
_ => panic!("secp256k1_fe_cmp_var returned unexpected result"), // Unreachable
}
}
}

impl Serialize for Element {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
Expand Down Expand Up @@ -657,6 +677,24 @@ mod tests {
}
}

#[test]
fn cmp() {
let left = Element::from(1);
let right = Element::from(2);

assert!(left < right);
assert!(right > left);
}

#[test]
fn sort() {
let sorted = [1, 2, 3, 4, 5].map(Element::from);
let mut unsorted = [4, 2, 3, 1, 5].map(Element::from);
unsorted.sort();

assert_eq!(unsorted, sorted);
}

#[test]
fn base58() {
let mut rng = OsRng::default();
Expand Down
Loading

0 comments on commit 119529f

Please sign in to comment.