diff --git a/.github/workflows/clippy-check.yml b/.github/workflows/clippy-check.yml index db95b1b9..987d0b54 100644 --- a/.github/workflows/clippy-check.yml +++ b/.github/workflows/clippy-check.yml @@ -23,4 +23,4 @@ jobs: uses: actions-rs/cargo@v1 with: command: lints - args: clippy --all-targets \ No newline at end of file + args: clippy --all-targets --all-features \ No newline at end of file diff --git a/src/ffi/error.rs b/src/ffi/error.rs index d38ce5eb..625659d7 100644 --- a/src/ffi/error.rs +++ b/src/ffi/error.rs @@ -16,6 +16,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ + convert::TryFrom, os::raw::{c_char, c_int}, ptr, slice, @@ -35,8 +36,12 @@ pub unsafe extern "C" fn lookup_error_message(code: c_int, buffer: *mut c_char, return NULL_POINTER; } - let error_message = get_error_message(code).to_string(); - let buffer = slice::from_raw_parts_mut(buffer as *mut u8, length as usize); + let error_message = get_error_message(code); + let length = match usize::try_from(length) { + Ok(l) => l, + Err(_) => return INTEGER_OVERFLOW, + }; + let buffer = slice::from_raw_parts_mut(buffer as *mut u8, length); if error_message.len() >= buffer.len() { return BUFFER_TOO_SMALL; @@ -48,7 +53,9 @@ pub unsafe extern "C" fn lookup_error_message(code: c_int, buffer: *mut c_char, // accidentally read into garbage. buffer[error_message.len()] = 0; - error_message.len() as c_int + // Explicitly truncate usize to c_int (not possible anyway) because adding #[allow(clippy::cast-possible-wrap)] + // attribute requires unstable rust feature + c_int::try_from(error_message.len()).unwrap_or(c_int::MAX) } pub const OK: i32 = 0; @@ -57,6 +64,7 @@ pub const BUFFER_TOO_SMALL: i32 = -2; pub const INVALID_SECRET_KEY_SER: i32 = -1000; pub const SIGNING_ERROR: i32 = -1100; pub const STR_CONV_ERR: i32 = -2000; +pub const INTEGER_OVERFLOW: i32 = -3000; pub fn get_error_message(code: i32) -> &'static str { match code { @@ -66,6 +74,7 @@ pub fn get_error_message(code: i32) -> &'static str { INVALID_SECRET_KEY_SER => "Invalid secret key representation.", SIGNING_ERROR => "Error creating signature", STR_CONV_ERR => "String conversion error", + INTEGER_OVERFLOW => "Integer overflowed", _ => "Unknown error code.", } } @@ -92,7 +101,7 @@ mod test { unsafe { let mut buffer = [0u8; 1000]; assert_eq!( - lookup_error_message(OK, buffer.as_mut_ptr() as *mut i8, 1000) as usize, + usize::try_from(lookup_error_message(OK, buffer.as_mut_ptr() as *mut i8, 1000)).unwrap(), get_error_message(OK).len() ); assert_eq!( diff --git a/src/ffi/keys.rs b/src/ffi/keys.rs index b53dc29d..a2d53698 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -295,7 +295,7 @@ mod test { #[test] pub fn test_random_keypair_with_valid_params() { let mut priv_key: KeyArray = [0; KEY_LENGTH]; - let priv_key_before = priv_key.clone(); + let priv_key_before = priv_key; let mut pub_key: KeyArray = [0; KEY_LENGTH]; // Public keys is null. A new private key is set @@ -304,7 +304,7 @@ mod test { } assert_ne!(priv_key, priv_key_before); - let priv_key_before = priv_key.clone(); + let priv_key_before = priv_key; // Both are not null. unsafe { random_keypair(&mut priv_key, &mut pub_key); @@ -361,50 +361,41 @@ mod test { let mut signature = [0; KEY_LENGTH]; let mut err_code = 0i32; unsafe { - assert_eq!( - verify( - null_mut(), - msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, - &mut err_code - ), - false - ); - assert_eq!( - verify(&pub_key, null_mut(), &mut pub_nonce, &mut signature, &mut err_code), - false - ); - assert_eq!( - verify( - &pub_key, - msg.as_ptr() as *const c_char, - null_mut(), - &mut signature, - &mut err_code - ), - false - ); - assert_eq!( - verify( - &pub_key, - msg.as_ptr() as *const c_char, - &mut pub_nonce, - null_mut(), - &mut err_code - ), - false - ); - assert_eq!( - verify( - &pub_key, - msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, - null_mut() - ), - false - ); + assert!(!verify( + null_mut(), + msg.as_ptr() as *const c_char, + &mut pub_nonce, + &mut signature, + &mut err_code + ),); + assert!(!verify( + &pub_key, + null_mut(), + &mut pub_nonce, + &mut signature, + &mut err_code + ),); + assert!(!verify( + &pub_key, + msg.as_ptr() as *const c_char, + null_mut(), + &mut signature, + &mut err_code + ),); + assert!(!verify( + &pub_key, + msg.as_ptr() as *const c_char, + &mut pub_nonce, + null_mut(), + &mut err_code + ),); + assert!(!verify( + &pub_key, + msg.as_ptr() as *const c_char, + &mut pub_nonce, + &mut signature, + null_mut() + ),); } } @@ -419,16 +410,13 @@ mod test { unsafe { random_keypair(&mut priv_key, &mut pub_key); sign(&priv_key, msg.as_ptr() as *const c_char, &mut pub_nonce, &mut signature); - assert_eq!( - verify( - &pub_key, - msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, - &mut err_code - ), - true - ); + assert!(verify( + &pub_key, + msg.as_ptr() as *const c_char, + &mut pub_nonce, + &mut signature, + &mut err_code + )); } } } diff --git a/src/hash/blake2.rs b/src/hash/blake2.rs index b9a7123a..29caf05b 100644 --- a/src/hash/blake2.rs +++ b/src/hash/blake2.rs @@ -34,6 +34,7 @@ use digest::{ pub struct Blake256(VarBlake2b); impl Blake256 { + /// Constructs a `Blake256` hashing context with parameters that allow hash keying, salting and personalization. pub fn with_params(key: &[u8], salt: &[u8], persona: &[u8]) -> Self { Self(VarBlake2b::with_params( key, diff --git a/src/musig.rs b/src/musig.rs index 2b717f36..797ae7a9 100644 --- a/src/musig.rs +++ b/src/musig.rs @@ -179,7 +179,7 @@ where /// produces a byte array of the correct length. fn calculate_common(&self) -> K { let mut common = D::new(); - for k in self.pub_keys.iter() { + for k in &self.pub_keys { common = common.chain(k.as_bytes()); } K::from_bytes(&common.finalize()) diff --git a/src/range_proof.rs b/src/range_proof.rs index b919e974..d513353b 100644 --- a/src/range_proof.rs +++ b/src/range_proof.rs @@ -76,7 +76,7 @@ pub trait RangeProofService { proof_message: &[u8; REWIND_USER_MESSAGE_LENGTH], ) -> Result; - /// Rewind a rewindable range proof to reveal the committed value and the 19 byte proof message + /// Rewind a rewindable range proof to reveal the committed value and the 19 byte proof message. fn rewind_proof_value_only( &self, proof: &Self::P, @@ -96,6 +96,7 @@ pub trait RangeProofService { ) -> Result, RangeProofError>; } +/// Rewind data extracted from a rangeproof containing the committed value and the 19 byte proof message. #[derive(Debug, PartialEq)] pub struct RewindResult { pub committed_value: u64, @@ -103,6 +104,7 @@ pub struct RewindResult { } impl RewindResult { + /// Creates a new `RewindResult` pub fn new(committed_value: u64, proof_message: [u8; REWIND_USER_MESSAGE_LENGTH]) -> Self { Self { committed_value, @@ -111,6 +113,8 @@ impl RewindResult { } } +/// Rewind data extracted from a rangeproof containing the committed value, a 19 byte proof message and the blinding +/// factor. #[derive(Debug, PartialEq)] pub struct FullRewindResult where K: SecretKey @@ -123,6 +127,7 @@ where K: SecretKey impl FullRewindResult where K: SecretKey { + /// Creates a new `FullRewindResult` pub fn new(committed_value: u64, proof_message: [u8; REWIND_USER_MESSAGE_LENGTH], blinding_factor: K) -> Self { Self { committed_value, diff --git a/src/ristretto/musig.rs b/src/ristretto/musig.rs index d0657094..14aa626a 100644 --- a/src/ristretto/musig.rs +++ b/src/ristretto/musig.rs @@ -179,12 +179,14 @@ impl RistrettoMuSig { /// Add a public key to the MuSig ceremony. Public keys can only be added in the `Initialization` state and the /// MuSig state will only progress to the next state (nonce hash collection) once exactly `n` unique public keys /// have been added. + #[must_use] pub fn add_public_key(self, key: &RistrettoPublicKey) -> Self { let key = key.clone(); self.handle_event(MuSigEvent::AddKey(key)) } /// Set the message to be signed in this MuSig ceremony + #[must_use] pub fn set_message(self, msg: &[u8]) -> Self { let msg = D::digest(msg).to_vec(); self.handle_event(MuSigEvent::SetMessage(msg)) @@ -192,6 +194,7 @@ impl RistrettoMuSig { /// Adds a Round 1 public nonce commitment to the MuSig state. Once _n_ commitments have been collected, the /// MuSig state will automatically switch to nonce collection. + #[must_use] pub fn add_nonce_commitment(self, pub_key: &RistrettoPublicKey, commitment: MessageHash) -> Self { self.handle_event(MuSigEvent::AddNonceHash(pub_key, commitment)) } @@ -200,6 +203,7 @@ impl RistrettoMuSig { /// ceremonies. This risk is mitigated by the MuSig object taking ownership of the nonce, meaning that you need /// to explicitly call `clone()` on your nonce if you want to use it elsewhere. /// The MuSig state will automatically switch to `SignatureCollection` once _n_ valid nonces have been collected. + #[must_use] pub fn add_nonce(self, pub_key: &RistrettoPublicKey, nonce: RistrettoPublicKey) -> Self { self.handle_event(MuSigEvent::AddNonce(pub_key, nonce)) } @@ -210,6 +214,7 @@ impl RistrettoMuSig { /// aggregate signature will automatically be valid). This is slower than just checking the aggregate signature, /// but you will also know exactly _which_ signature failed. /// Otherwise pass `false` to `should_validate` and verify the aggregate signature. + #[must_use] pub fn add_signature(self, s: &RistrettoSchnorr, should_validate: bool) -> Self { self.handle_event(MuSigEvent::AddPartialSig(Box::new(s.clone()), should_validate)) } @@ -403,7 +408,7 @@ impl Initialization { if self.message.is_some() { return MuSigState::Failed(MuSigError::MessageAlreadySet); } - self.message = Some(msg.to_vec()); + self.message = Some(msg); MuSigState::Initialization(self) } } @@ -668,7 +673,7 @@ mod test { for r in public_nonces[1..].iter() { r_agg = r_agg + r; } - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); if let Some(v) = msg { musig = musig.set_message(v); } @@ -697,7 +702,7 @@ mod test { for (p, h) in data.pub_keys.iter().zip(&data.nonce_hashes) { musig = musig.add_nonce_commitment(p, h.clone()); } - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); if let Some(v) = msg { musig = musig.set_message(v); } @@ -734,13 +739,12 @@ mod test { /// verified, and the sum of partial signatures is returned independently fn create_final_musig(n: usize, msg: &[u8]) -> (RistrettoMuSig, MuSigTestData, RistrettoSchnorr) { let (mut musig, data) = create_round_three_musig(n, Some(msg)); - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); // Add the partial signatures - for s in data.partial_sigs.iter() { - musig = musig.add_signature(&s, true); - assert_eq!( - musig.has_failed(), - false, + for s in &data.partial_sigs { + musig = musig.add_signature(s, true); + assert!( + !musig.has_failed(), "Partial signature addition failed. {:?}", musig.failure_reason() ); @@ -760,7 +764,7 @@ mod test { let (_, p2) = RistrettoPublicKey::random_keypair(&mut rng); let (_, p3) = RistrettoPublicKey::random_keypair(&mut rng); let musig = musig.add_public_key(&p1).add_public_key(&p2); - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); let musig = musig.add_public_key(&p3); assert!(musig.has_failed()); assert_eq!(musig.failure_reason(), Some(MuSigError::InvalidStateTransition)); @@ -788,7 +792,7 @@ mod test { fn add_msg_in_round_one() { let (mut musig, _) = create_round_one_musig(5, None); musig = musig.set_message(b"Hello Discworld"); - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); // Haven't collected nonces yet, so challenge is still undefined assert_eq!(musig.get_challenge(), None); } @@ -800,7 +804,7 @@ mod test { let (k1, p1) = RistrettoPublicKey::random_keypair(&mut rng); let (_, p2) = RistrettoPublicKey::random_keypair(&mut rng); let mut musig = musig.add_public_key(&p1).add_public_key(&p2); - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed()); musig = musig.add_nonce_commitment(&p1, k1.to_vec()); assert!(musig.has_failed()); assert_eq!(musig.failure_reason(), Some(MuSigError::InvalidStateTransition)); @@ -821,7 +825,7 @@ mod test { fn must_wait_for_all_nonce_hashes() { let (mut musig, data) = create_round_one_musig(3, None); musig = musig.add_nonce_commitment(&data.pub_keys[2], data.nonce_hashes[2].clone()); - assert_eq!(musig.has_failed(), false); + assert!(!musig.has_failed(),); // Try add nonce before all hashes have been collected musig = musig.add_nonce(&data.pub_keys[2], data.public_nonces[2].clone()); assert_eq!(musig.failure_reason(), Some(MuSigError::InvalidStateTransition)); @@ -977,15 +981,15 @@ mod test_joint_key { let (_, p2) = RistrettoPublicKey::random_keypair(&mut rng); let (_, p3) = RistrettoPublicKey::random_keypair(&mut rng); // Add first key - assert_eq!(jk.key_exists(&p1), false); + assert!(!jk.key_exists(&p1),); assert_eq!(jk.add_key(p1).unwrap(), 1); - assert_eq!(jk.is_full(), false); + assert!(!jk.is_full(),); // Add second key - assert_eq!(jk.key_exists(&p2), false); + assert!(!jk.key_exists(&p2),); assert_eq!(jk.add_key(p2).unwrap(), 2); assert!(jk.is_full()); // Try add third key - assert_eq!(jk.key_exists(&p3), false); + assert!(!jk.key_exists(&p3),); assert_eq!(jk.add_key(p3).err(), Some(MuSigError::TooManyParticipants)); } @@ -996,15 +1000,15 @@ mod test_joint_key { let (_, p1) = RistrettoPublicKey::random_keypair(&mut rng); let (_, p2) = RistrettoPublicKey::random_keypair(&mut rng); // Add first key - assert_eq!(jk.key_exists(&p1), false); + assert!(!jk.key_exists(&p1),); assert_eq!(jk.add_key(p1.clone()).unwrap(), 1); - assert_eq!(jk.is_full(), false); + assert!(!jk.is_full(),); // Add second key - assert_eq!(jk.key_exists(&p2), false); + assert!(!jk.key_exists(&p2),); assert_eq!(jk.add_key(p2).unwrap(), 2); - assert_eq!(jk.is_full(), false); + assert!(!jk.is_full(),); // Try add third key - assert_eq!(jk.key_exists(&p1), true); + assert!(jk.key_exists(&p1)); assert_eq!(jk.add_key(p1).err(), Some(MuSigError::DuplicatePubKey)); } diff --git a/src/ristretto/pedersen.rs b/src/ristretto/pedersen.rs index e75342c3..8c67d873 100644 --- a/src/ristretto/pedersen.rs +++ b/src/ristretto/pedersen.rs @@ -41,6 +41,8 @@ lazy_static! { pub type PedersenCommitment = HomomorphicCommitment; +/// Generates Pederson commitments `k.G + v.H` using the provided base +/// [RistrettoPoints](curve25519_dalek::ristretto::RistrettoPoints). #[derive(Debug, PartialEq, Eq, Clone)] #[allow(non_snake_case)] pub struct PedersenCommitmentFactory { @@ -57,8 +59,8 @@ impl PedersenCommitmentFactory { } } -/// The default Ristretto Commitment factory uses the Base point for x25519 and its first Blake256 hash. impl Default for PedersenCommitmentFactory { + /// The default Ristretto Commitment factory uses the Base point for x25519 and its first Blake256 hash. fn default() -> Self { PedersenCommitmentFactory::new(RISTRETTO_PEDERSEN_G, *RISTRETTO_PEDERSEN_H) } diff --git a/src/ristretto/ristretto_com_sig.rs b/src/ristretto/ristretto_com_sig.rs index 4d476fd3..5138b575 100644 --- a/src/ristretto/ristretto_com_sig.rs +++ b/src/ristretto/ristretto_com_sig.rs @@ -97,7 +97,6 @@ use crate::{ /// let factory = PedersenCommitmentFactory::default(); /// assert!(sig.verify_challenge(&commitment, &e, &factory)); /// ``` - pub type RistrettoComSig = CommitmentSignature; #[cfg(test)] diff --git a/src/ristretto/ristretto_keys.rs b/src/ristretto/ristretto_keys.rs index a36f453c..4caca579 100644 --- a/src/ristretto/ristretto_keys.rs +++ b/src/ristretto/ristretto_keys.rs @@ -82,8 +82,8 @@ impl SecretKey for RistrettoSecretKey { } } -/// Clear the secret key value in memory when it goes out of scope impl Drop for RistrettoSecretKey { + /// Clear the secret key value in memory when it goes out of scope fn drop(&mut self) { self.0.zeroize() } @@ -131,7 +131,7 @@ impl<'a, 'b> Mul<&'b RistrettoPublicKey> for &'a RistrettoSecretKey { type Output = RistrettoPublicKey; fn mul(self, rhs: &'b RistrettoPublicKey) -> RistrettoPublicKey { - let p = &self.0 * &rhs.point; + let p = self.0 * rhs.point; RistrettoPublicKey::new_from_pk(p) } } @@ -140,7 +140,7 @@ impl<'a, 'b> Add<&'b RistrettoSecretKey> for &'a RistrettoSecretKey { type Output = RistrettoSecretKey; fn add(self, rhs: &'b RistrettoSecretKey) -> RistrettoSecretKey { - let k = &self.0 + &rhs.0; + let k = self.0 + rhs.0; RistrettoSecretKey(k) } } @@ -149,7 +149,7 @@ impl<'a, 'b> Sub<&'b RistrettoSecretKey> for &'a RistrettoSecretKey { type Output = RistrettoSecretKey; fn sub(self, rhs: &'b RistrettoSecretKey) -> RistrettoSecretKey { - RistrettoSecretKey(&self.0 - &rhs.0) + RistrettoSecretKey(self.0 - rhs.0) } } @@ -362,7 +362,7 @@ impl<'a, 'b> Add<&'b RistrettoPublicKey> for &'a RistrettoPublicKey { type Output = RistrettoPublicKey; fn add(self, rhs: &'b RistrettoPublicKey) -> RistrettoPublicKey { - let p_sum = &self.point + &rhs.point; + let p_sum = self.point + rhs.point; RistrettoPublicKey::new_from_pk(p_sum) } } @@ -371,7 +371,7 @@ impl<'a, 'b> Sub<&'b RistrettoPublicKey> for &'a RistrettoPublicKey { type Output = RistrettoPublicKey; fn sub(self, rhs: &RistrettoPublicKey) -> RistrettoPublicKey { - let p_sum = &self.point - &rhs.point; + let p_sum = self.point - rhs.point; RistrettoPublicKey::new_from_pk(p_sum) } } @@ -380,7 +380,7 @@ impl<'a, 'b> Mul<&'b RistrettoSecretKey> for &'a RistrettoPublicKey { type Output = RistrettoPublicKey; fn mul(self, rhs: &'b RistrettoSecretKey) -> RistrettoPublicKey { - let p = &rhs.0 * &self.point; + let p = rhs.0 * self.point; RistrettoPublicKey::new_from_pk(p) } } diff --git a/src/ristretto/test_common.rs b/src/ristretto/test_common.rs index df6c351e..45dcdbf4 100644 --- a/src/ristretto/test_common.rs +++ b/src/ristretto/test_common.rs @@ -24,7 +24,8 @@ use crate::{ keys::{PublicKey, SecretKey}, ristretto::{RistrettoPublicKey, RistrettoSecretKey}, }; -pub fn get_keypair() -> (RistrettoSecretKey, RistrettoPublicKey) { + +pub(crate) fn get_keypair() -> (RistrettoSecretKey, RistrettoPublicKey) { let mut rng = rand::thread_rng(); let k = RistrettoSecretKey::random(&mut rng); let pk = RistrettoPublicKey::from_secret_key(&k); diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index 8b863955..b9958274 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -19,6 +19,12 @@ pub enum SchnorrSignatureError { InvalidChallenge, } +/// # SchnorrSignature +/// +/// Provides a Schnorr signature that is agnostic to a specific public/private key implementation. +/// For a concrete implementation see [RistrettoSchnorr](crate::ristretto::RistrettoSchnorr). +/// +/// More details on Schnorr signatures can be found at [TLU](https://tlu.tarilabs.com/cryptography/introduction-schnorr-signatures). #[allow(non_snake_case)] #[derive(PartialEq, Eq, Copy, Debug, Clone, Serialize, Deserialize, Hash)] pub struct SchnorrSignature { @@ -31,6 +37,7 @@ where P: PublicKey, K: SecretKey, { + /// Create a new `SchnorrSignature`. pub fn new(public_nonce: P, signature: K) -> Self { SchnorrSignature { public_nonce, @@ -38,10 +45,13 @@ where } } - pub fn calc_signature_verifier(&self) -> P { + /// Calculates the signature verifier `s.G`. This must be equal to `R + eK`. + fn calc_signature_verifier(&self) -> P { P::from_secret_key(&self.signature) } + /// Sign a challenge with the given `secret` and private `nonce`. Returns an SchnorrSignatureError if `::from_bytes(challenge)` returns an error. pub fn sign(secret: K, nonce: K, challenge: &[u8]) -> Result where K: Add + Mul + Mul { // s = r + e.k @@ -55,6 +65,8 @@ where Ok(Self::new(public_nonce, s)) } + /// Returns true if this signature is valid for a public key and challenge, otherwise false. This will always return + /// false if `::from_bytes(challenge)` returns an error. pub fn verify_challenge<'a>(&self, public_key: &'a P, challenge: &[u8]) -> bool where for<'b> &'b K: Mul<&'a P, Output = P>, @@ -67,6 +79,7 @@ where self.verify(public_key, &e) } + /// Returns true if this signature is valid for a public key and challenge scalar, otherwise false. pub fn verify<'a>(&self, public_key: &'a P, challenge: &K) -> bool where for<'b> &'b K: Mul<&'a P, Output = P>, @@ -78,12 +91,12 @@ where lhs == rhs } - #[inline] + /// Returns a reference to the `s` signature component. pub fn get_signature(&self) -> &K { &self.signature } - #[inline] + /// Returns a reference to the public nonce component. pub fn get_public_nonce(&self) -> &P { &self.public_nonce } @@ -131,15 +144,16 @@ where } } -/// Provide an efficient ordering algorithm for Schnorr signatures. It's probably not a good idea to implement `Ord` -/// for secret keys, but in this instance, the signature is publicly known and is simply a scalar, so we use the byte -/// representation of the scalar as the canonical ordering metric. This conversion is done if and only if the public -/// nonces are already equal, otherwise the public nonce ordering determines the SchnorrSignature order. impl Ord for SchnorrSignature where P: Eq + Ord, K: Eq + ByteArray, { + /// Provide an efficient ordering algorithm for Schnorr signatures. It's probably not a good idea to implement `Ord` + /// for secret keys, but in this instance, the signature is publicly known and is simply a scalar, so we use the + /// byte representation of the scalar as the canonical ordering metric. This conversion is done if and only if + /// the public nonces are already equal, otherwise the public nonce ordering determines the SchnorrSignature + /// order. fn cmp(&self, other: &Self) -> Ordering { match self.public_nonce.cmp(&other.public_nonce) { Ordering::Equal => self.signature.as_bytes().cmp(other.signature.as_bytes()), diff --git a/src/wasm/commitments.rs b/src/wasm/commitments.rs index 9d4f34be..a9b4d58a 100644 --- a/src/wasm/commitments.rs +++ b/src/wasm/commitments.rs @@ -150,7 +150,7 @@ mod test { #[wasm_bindgen_test] fn it_returns_false_for_zero_length_input() { - assert!(!opens("", 123, &"")); + assert!(!opens("", 123, "")); } #[wasm_bindgen_test] diff --git a/src/wasm/key_utils.rs b/src/wasm/key_utils.rs index e18badb7..5314ffc6 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -158,7 +158,7 @@ pub(super) fn sign_with_key(k: &RistrettoSecretKey, e: &[u8], r: Option<&Ristret let sig = match RistrettoSchnorr::sign(k.clone(), r, e) { Ok(s) => s, Err(e) => { - result.error = format!("Could not create signature. {}", e.to_string()); + result.error = format!("Could not create signature. {}", e); return; }, }; @@ -314,10 +314,10 @@ pub(crate) fn sign_comsig_with_key( None => RistrettoSecretKey::random(&mut OsRng), }; - let sig = match RistrettoComSig::sign(&private_key_a, &private_key_x, &r_2, &r_1, e, &factory) { + let sig = match RistrettoComSig::sign(private_key_a, private_key_x, &r_2, &r_1, e, &factory) { Ok(s) => s, Err(e) => { - result.error = format!("Could not create signature. {}", e.to_string()); + result.error = format!("Could not create signature. {}", e); return; }, }; @@ -548,7 +548,7 @@ mod test { #[wasm_bindgen_test] fn it_returns_error_if_invalid() { - assert_eq!(sign("", SAMPLE_CHALLENGE).error.is_empty(), false); + assert!(!sign("", SAMPLE_CHALLENGE).error.is_empty()); assert!(!sign(&["0"; 32].join(""), SAMPLE_CHALLENGE).error.is_empty()); } @@ -585,17 +585,13 @@ mod test { #[wasm_bindgen_test] fn it_returns_error_if_invalid() { let (_, key) = key_hex(); - assert_eq!( - sign_challenge_with_nonce(&key, "", &hash_hex(SAMPLE_CHALLENGE)) - .error - .is_empty(), - false - ); - assert_eq!( - sign_challenge_with_nonce(&["0"; 33].join(""), &key, &hash_hex(SAMPLE_CHALLENGE)) + assert!(!sign_challenge_with_nonce(&key, "", &hash_hex(SAMPLE_CHALLENGE)) + .error + .is_empty()); + assert!( + !sign_challenge_with_nonce(&["0"; 33].join(""), &key, &hash_hex(SAMPLE_CHALLENGE)) .error - .is_empty(), - false + .is_empty() ); } @@ -634,30 +630,29 @@ mod test { fn it_errors_given_invalid_data() { fn it_errors(pub_nonce: &str, signature: &str, pub_key: &str, msg: &str) { let result = check_signature(pub_nonce, signature, pub_key, msg); - assert_eq!( - result.error.is_empty(), - false, + assert!( + !result.error.is_empty(), "check_signature did not fail with args ({}, {}, {}, {})", pub_nonce, signature, pub_key, msg ); - assert_eq!(result.result, false); + assert!(!result.result); } it_errors("", "", "", SAMPLE_CHALLENGE); let (sig, pk, _) = create_signature(SAMPLE_CHALLENGE); - it_errors(&sig.get_public_nonce().to_hex(), &"", &pk.to_hex(), SAMPLE_CHALLENGE); + it_errors(&sig.get_public_nonce().to_hex(), "", &pk.to_hex(), SAMPLE_CHALLENGE); } #[wasm_bindgen_test] fn it_fails_if_verification_is_invalid() { fn it_fails(pub_nonce: &str, signature: &str, pub_key: &str, msg: &str) { let result = check_signature(pub_nonce, signature, pub_key, msg); - assert_eq!(result.error.is_empty(), true,); - assert_eq!(result.result, false); + assert!(result.error.is_empty()); + assert!(!result.result); } let (sig, pk, _) = create_signature(SAMPLE_CHALLENGE); @@ -708,10 +703,10 @@ mod test { fn it_fails_if_given_invalid_data() { fn it_fails(a: &str, x: &str, msg: &str) { let result = sign_comsig(a, x, msg); - assert_eq!(result.error.is_empty(), false); - assert_eq!(result.public_nonce.is_some(), false); - assert_eq!(result.v.is_some(), false); - assert_eq!(result.u.is_some(), false); + assert!(!result.error.is_empty()); + assert!(result.public_nonce.is_none()); + assert!(result.v.is_none()); + assert!(result.u.is_none()); } let (sk, _) = random_keypair(); @@ -775,10 +770,10 @@ mod test { fn it_fails_if_given_invalid_data() { fn it_fails(a: &str, x: &str, n_a: &str, n_x: &str) { let result = sign_comsig_challenge_with_nonce(a, x, n_a, n_x); - assert_eq!(result.error.is_empty(), false); - assert_eq!(result.public_nonce.is_some(), false); - assert_eq!(result.v.is_some(), false); - assert_eq!(result.u.is_some(), false); + assert!(!result.error.is_empty()); + assert!(result.public_nonce.is_none()); + assert!(result.v.is_none()); + assert!(result.u.is_none()); } let (sk, _) = random_keypair(); @@ -794,7 +789,7 @@ mod test { let (r2, _) = random_keypair(); let expected_p_nonce = PedersenCommitmentFactory::default().commit(&r1, &r2); let result = sign_comsig_challenge_with_nonce(&sk.to_hex(), &sk.to_hex(), &r1.to_hex(), &r2.to_hex()); - assert_eq!(result.error.is_empty(), true); + assert!(result.error.is_empty()); assert_eq!( PedersenCommitment::from_hex(&result.public_nonce.unwrap()).unwrap(), expected_p_nonce @@ -822,16 +817,15 @@ mod test { fn it_errors(nonce_commit: &str, signature_u: &str, signature_v: &str, commitment: &str) { let result = check_comsig_signature(nonce_commit, signature_u, signature_v, commitment, SAMPLE_CHALLENGE); - assert_eq!( - result.error.is_empty(), - false, + assert!( + !result.error.is_empty(), "check_comsig_signature did not fail with args ({}, {}, {}, {})", nonce_commit, signature_u, signature_v, commitment ); - assert_eq!(result.result, false); + assert!(!result.result); } it_errors("", "", "", ""); @@ -845,8 +839,8 @@ mod test { fn it_fails_if_verification_is_invalid() { fn it_fails(pub_nonce_commit: &str, signature_u: &str, signature_v: &str, commit: &str, msg: &str) { let result = check_comsig_signature(pub_nonce_commit, signature_u, signature_v, commit, msg); - assert_eq!(result.error.is_empty(), true); - assert_eq!(result.result, false); + assert!(result.error.is_empty()); + assert!(!result.result); } let (sig, commit) = create_commsig(SAMPLE_CHALLENGE); @@ -894,7 +888,7 @@ mod test { #[wasm_bindgen_test] fn fail_case() { fn it_fails(private_key_hex: &str) { - assert_eq!(secret_key_from_hex_bytes(private_key_hex).as_bool().unwrap(), false); + assert!(!secret_key_from_hex_bytes(private_key_hex).as_bool().unwrap()); } it_fails(""); @@ -921,10 +915,10 @@ mod test { use super::*; fn it_fails JsValue>(subject: F, k_a: &str, k_b: &str) { - assert_eq!((subject)(k_a, k_b).as_bool().unwrap(), false); + assert!(!(subject)(k_a, k_b).as_bool().unwrap()); } fn it_succeeds JsValue>(subject: F, k_a: &str, k_b: &str) -> RistrettoSecretKey { - (subject)(&k_a, &k_b).into_serde::().unwrap() + (subject)(k_a, k_b).into_serde::().unwrap() } #[wasm_bindgen_test] diff --git a/src/wasm/keyring.rs b/src/wasm/keyring.rs index 306f6217..565ad8f4 100644 --- a/src/wasm/keyring.rs +++ b/src/wasm/keyring.rs @@ -40,6 +40,8 @@ use crate::{ }, }; +/// KeyRing is an in-memory key-value store for secret keys. Each secret key has a user-defined id associated with it. +/// Additionally, it provides methods to sign and verify signatures using these stored keys. #[wasm_bindgen] #[derive(Default)] pub struct KeyRing { @@ -226,7 +228,7 @@ mod test { let sk_a = kr.expect_private_key("a"); let pk_a = kr.expect_public_key("a"); - assert_eq!(*pk_a, RistrettoPublicKey::from_secret_key(&sk_a)); + assert_eq!(*pk_a, RistrettoPublicKey::from_secret_key(sk_a)); let sk_b = kr.expect_private_key("b"); assert_ne!(sk_a, sk_b); @@ -257,7 +259,7 @@ mod test { let kr = new_keyring(); let sig = sign(&kr, "a").unwrap(); let pk = kr.expect_public_key("a"); - assert!(sig.verify_challenge(&pk, &hash(SAMPLE_CHALLENGE))); + assert!(sig.verify_challenge(pk, &hash(SAMPLE_CHALLENGE))); } } @@ -267,30 +269,30 @@ mod test { #[wasm_bindgen_test] fn it_returns_false_if_key_doesnt_exist() { let kr = new_keyring(); - assert_eq!(kr.opens("doesnt-exist", 0, ""), false); - assert_eq!(kr.opens("doesnt-exist", u64::MAX, ""), false); + assert!(!kr.opens("doesnt-exist", 0, ""),); + assert!(!kr.opens("doesnt-exist", u64::MAX, ""),); let c = create_commitment(kr.expect_private_key("a"), 0); - assert_eq!(kr.opens("doesnt-exist", 0, &c.to_hex()), false); + assert!(!kr.opens("doesnt-exist", 0, &c.to_hex()),); } #[wasm_bindgen_test] fn it_returns_false_does_not_open_commitment() { let kr = new_keyring(); let c = create_commitment(&RistrettoSecretKey::random(&mut OsRng), 123); - assert_eq!(kr.opens("a", 123, &c.to_hex()), false); + assert!(!kr.opens("a", 123, &c.to_hex()),); let c = create_commitment(kr.expect_private_key("a"), 123); - assert_eq!(kr.opens("a", 321, &c.to_hex()), false); + assert!(!kr.opens("a", 321, &c.to_hex()),); let c = create_commitment(kr.expect_private_key("a"), 123); - assert_eq!(kr.opens("b", 123, &c.to_hex()), false); + assert!(!kr.opens("b", 123, &c.to_hex()),); } #[wasm_bindgen_test] fn it_returns_true_if_commitment_opened() { let kr = new_keyring(); let c = create_commitment(kr.expect_private_key("a"), 123); - assert_eq!(kr.opens("a", 123, &c.to_hex()), true); + assert!(kr.opens("a", 123, &c.to_hex())); } } diff --git a/src/wasm/range_proofs.rs b/src/wasm/range_proofs.rs index 89a9f36e..0d956180 100644 --- a/src/wasm/range_proofs.rs +++ b/src/wasm/range_proofs.rs @@ -54,6 +54,7 @@ pub struct RangeProofFactory { #[wasm_bindgen] impl RangeProofFactory { + /// Create a new `RangeProofFactory` pub fn new() -> Self { let cf = PedersenCommitmentFactory::default(); let rpf = DalekRangeProofService::new(64, &cf).unwrap(); @@ -90,7 +91,7 @@ impl RangeProofFactory { let proof = match from_hex(proof) { Ok(v) => v, Err(e) => { - result.error = format!("Range proof is invalid. {}", e.to_string()); + result.error = format!("Range proof is invalid. {}", e); return JsValue::from_serde(&result).unwrap(); }, }; @@ -117,7 +118,7 @@ mod test { fn it_fails_with_invalid_hex_input() { let factory = RangeProofFactory::new(); let result = factory.create_proof("", 123).into_serde::().unwrap(); - assert_eq!(result.error.is_empty(), false); + assert!(!result.error.is_empty()); assert!(result.proof.is_empty()); }