diff --git a/clippy-config b/clippy-config index d66a4503..f0164aab 100644 --- a/clippy-config +++ b/clippy-config @@ -5,18 +5,22 @@ -D clippy::nursery -D clippy::pedantic +# Not worth the effort +-A clippy::implicit_hasher + # Stylistic preferrence --A clippy::option-if-let-else +-A clippy::option_if_let_else # Too many false/irrelevant positives --A clippy::redundant-pub-crate +-A clippy::redundant_pub_crate -A clippy::similar_names # Frequently used --A clippy::wildcard-imports +-A clippy::wildcard_imports +-A clippy::too_many_lines # Used to avoid doing &* on copy-able items, with the * being the concern --A clippy::explicit-deref-methods +-A clippy::explicit_deref_methods # Lints from clippy::restrictions @@ -39,7 +43,6 @@ -D clippy::get_unwrap -D clippy::if_then_some_else_none -D clippy::rest_pat_in_fully_bound_structs --D clippy::self_named_module_files -D clippy::semicolon_inside_block -D clippy::tests_outside_test_module @@ -48,18 +51,18 @@ -D clippy::string_to_string # Flagged on tests being named test_ --A clippy::module-name-repetitions +-A clippy::module_name_repetitions # Flagged on items passed by value which implemented Copy --A clippy::needless-pass-by-value +-A clippy::needless_pass_by_value # Flagged on embedded functions defined when needed/relevant -A clippy::items_after_statements # These potentially should be enabled in the future --A clippy::missing-errors-doc --A clippy::missing-panics-doc --A clippy::doc-markdown +-A clippy::missing_errors_doc +-A clippy::missing_panics_doc +-A clippy::doc_markdown # TODO: Enable this # -D clippy::cargo diff --git a/crypto/ciphersuite/src/lib.rs b/crypto/ciphersuite/src/lib.rs index 317ab153..18ed0454 100644 --- a/crypto/ciphersuite/src/lib.rs +++ b/crypto/ciphersuite/src/lib.rs @@ -1,4 +1,3 @@ -#![allow(clippy::self_named_module_files)] // False positive? #![allow(clippy::tests_outside_test_module)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![doc = include_str!("lib.md")] diff --git a/crypto/dkg/src/encryption.rs b/crypto/dkg/src/encryption.rs index 99b039b8..20f2acdf 100644 --- a/crypto/dkg/src/encryption.rs +++ b/crypto/dkg/src/encryption.rs @@ -1,8 +1,6 @@ use core::{ops::Deref, fmt}; use std::{io, collections::HashMap}; -use thiserror::Error; - use zeroize::{Zeroize, Zeroizing}; use rand_core::{RngCore, CryptoRng}; @@ -70,7 +68,7 @@ impl EncryptionKeyMessage { } #[cfg(any(test, feature = "tests"))] - pub(crate) fn enc_key(&self) -> C::G { + pub(crate) const fn enc_key(&self) -> C::G { self.enc_key } } @@ -110,6 +108,7 @@ fn cipher(context: &str, ecdh: &Zeroizing) -> ChaCha20 { transcript.append_message(b"shared_key", ecdh.as_ref()); ecdh.as_mut().zeroize(); + #[allow(clippy::redundant_closure_for_method_calls)] // Not redundant due to typing let zeroize = |buf: &mut [u8]| buf.zeroize(); let mut key = Cc20Key::default(); @@ -329,13 +328,19 @@ fn encryption_key_transcript(context: &str) -> RecommendedTranscript { transcript } -#[derive(Clone, Copy, PartialEq, Eq, Debug, Error)] -pub(crate) enum DecryptionError { - #[error("accused provided an invalid signature")] - InvalidSignature, - #[error("accuser provided an invalid decryption key")] - InvalidProof, +#[allow(clippy::std_instead_of_core)] +mod decryption_error { + use thiserror::Error; + + #[derive(Clone, Copy, PartialEq, Eq, Debug, Error)] + pub(crate) enum DecryptionError { + #[error("accused provided an invalid signature")] + InvalidSignature, + #[error("accuser provided an invalid decryption key")] + InvalidProof, + } } +pub(crate) use decryption_error::DecryptionError; // A simple box for managing encryption. #[derive(Clone)] @@ -381,7 +386,7 @@ impl Encryption { } } - pub(crate) fn registration(&self, msg: M) -> EncryptionKeyMessage { + pub(crate) const fn registration(&self, msg: M) -> EncryptionKeyMessage { EncryptionKeyMessage { msg, enc_key: self.enc_pub_key } } @@ -390,9 +395,10 @@ impl Encryption { participant: Participant, msg: EncryptionKeyMessage, ) -> M { - if self.enc_keys.contains_key(&participant) { - panic!("Re-registering encryption key for a participant"); - } + assert!( + !self.enc_keys.contains_key(&participant), + "Re-registering encryption key for a participant" + ); self.enc_keys.insert(participant, msg.enc_key); msg.msg } diff --git a/crypto/dkg/src/frost.rs b/crypto/dkg/src/frost.rs index 868a7109..40e8ef63 100644 --- a/crypto/dkg/src/frost.rs +++ b/crypto/dkg/src/frost.rs @@ -73,7 +73,7 @@ impl ReadWrite for Commitments { commitments.push(read_G()?); } - Ok(Commitments { commitments, cached_msg, sig: SchnorrSignature::read(reader)? }) + Ok(Self { commitments, cached_msg, sig: SchnorrSignature::read(reader)? }) } fn write(&self, writer: &mut W) -> io::Result<()> { @@ -87,14 +87,16 @@ impl ReadWrite for Commitments { pub struct KeyGenMachine { params: ThresholdParams, context: String, - _curve: PhantomData, + curve: PhantomData, } impl KeyGenMachine { /// Create a new machine to generate a key. - // The context string should be unique among multisigs. - pub fn new(params: ThresholdParams, context: String) -> KeyGenMachine { - KeyGenMachine { params, context, _curve: PhantomData } + /// + /// The context string should be unique among multisigs. + #[must_use] + pub const fn new(params: ThresholdParams, context: String) -> Self { + Self { params, context, curve: PhantomData } } /// Start generating a key according to the FROST DKG spec. @@ -171,7 +173,6 @@ fn polynomial( /// channel. /// /// If any participant sends multiple secret shares to another participant, they are faulty. - // This should presumably be written as SecretShare(Zeroizing). // It's unfortunately not possible as F::Repr doesn't have Zeroize as a bound. // The encryption system also explicitly uses Zeroizing so it can ensure anything being @@ -195,7 +196,7 @@ impl fmt::Debug for SecretShare { } impl Zeroize for SecretShare { fn zeroize(&mut self) { - self.0.as_mut().zeroize() + self.0.as_mut().zeroize(); } } // Still manually implement ZeroizeOnDrop to ensure these don't stick around. @@ -213,7 +214,7 @@ impl ReadWrite for SecretShare { fn read(reader: &mut R, _: ThresholdParams) -> io::Result { let mut repr = F::Repr::default(); reader.read_exact(repr.as_mut())?; - Ok(SecretShare(repr)) + Ok(Self(repr)) } fn write(&self, writer: &mut W) -> io::Result<()> { @@ -353,7 +354,7 @@ impl Zeroize for KeyMachine { fn zeroize(&mut self) { self.params.zeroize(); self.secret.zeroize(); - for (_, commitments) in self.commitments.iter_mut() { + for commitments in self.commitments.values_mut() { commitments.zeroize(); } self.encryption.zeroize(); @@ -466,7 +467,7 @@ impl KeyMachine { ); } - let KeyMachine { commitments, encryption, params, secret } = self; + let Self { commitments, encryption, params, secret } = self; Ok(BlameMachine { commitments, encryption, @@ -499,7 +500,7 @@ impl fmt::Debug for BlameMachine { impl Zeroize for BlameMachine { fn zeroize(&mut self) { - for (_, commitments) in self.commitments.iter_mut() { + for commitments in self.commitments.values_mut() { commitments.zeroize(); } self.encryption.zeroize(); @@ -517,6 +518,7 @@ impl BlameMachine { /// territory of consensus protocols. This library does not handle that nor does it provide any /// tooling to do so. This function is solely intended to force users to acknowledge they're /// completing the protocol, not processing any blame. + #[allow(clippy::missing_const_for_fn)] // False positive pub fn complete(self) -> ThresholdCore { self.result } @@ -536,10 +538,9 @@ impl BlameMachine { Err(DecryptionError::InvalidProof) => return recipient, }; - let share = match Option::::from(C::F::from_repr(share_bytes.0)) { - Some(share) => share, + let Some(share) = Option::::from(C::F::from_repr(share_bytes.0)) else { // If this isn't a valid scalar, the sender is faulty - None => return sender, + return sender; }; // If this isn't a valid share, the sender is faulty diff --git a/crypto/dkg/src/lib.rs b/crypto/dkg/src/lib.rs index 303bd120..79ced53b 100644 --- a/crypto/dkg/src/lib.rs +++ b/crypto/dkg/src/lib.rs @@ -3,9 +3,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use core::fmt::{self, Debug}; - -#[cfg(feature = "std")] -use thiserror::Error; +extern crate alloc; use zeroize::Zeroize; @@ -35,23 +33,25 @@ pub mod tests; pub struct Participant(pub(crate) u16); impl Participant { /// Create a new Participant identifier from a u16. - pub fn new(i: u16) -> Option { + #[must_use] + pub const fn new(i: u16) -> Option { if i == 0 { None } else { - Some(Participant(i)) + Some(Self(i)) } } /// Convert a Participant identifier to bytes. #[allow(clippy::wrong_self_convention)] - pub fn to_bytes(&self) -> [u8; 2] { + #[must_use] + pub const fn to_bytes(&self) -> [u8; 2] { self.0.to_le_bytes() } } impl From for u16 { - fn from(participant: Participant) -> u16 { + fn from(participant: Participant) -> Self { participant.0 } } @@ -63,49 +63,58 @@ impl fmt::Display for Participant { } /// Various errors possible during key generation. -#[derive(Clone, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "std", derive(Error))] -pub enum DkgError { - /// A parameter was zero. - #[cfg_attr(feature = "std", error("a parameter was 0 (threshold {0}, participants {1})"))] - ZeroParameter(u16, u16), - /// The threshold exceeded the amount of participants. - #[cfg_attr(feature = "std", error("invalid threshold (max {1}, got {0})"))] - InvalidThreshold(u16, u16), - /// Invalid participant identifier. - #[cfg_attr( - feature = "std", - error("invalid participant (0 < participant <= {0}, yet participant is {1})") - )] - InvalidParticipant(u16, Participant), +#[allow(clippy::std_instead_of_core)] +mod dkg_error { + use core::fmt::Debug; + use thiserror::Error; + use super::Participant; - /// Invalid signing set. - #[cfg_attr(feature = "std", error("invalid signing set"))] - InvalidSigningSet, - /// Invalid amount of participants. - #[cfg_attr(feature = "std", error("invalid participant quantity (expected {0}, got {1})"))] - InvalidParticipantQuantity(usize, usize), - /// A participant was duplicated. - #[cfg_attr(feature = "std", error("duplicated participant ({0})"))] - DuplicatedParticipant(Participant), - /// A participant was missing. - #[cfg_attr(feature = "std", error("missing participant {0}"))] - MissingParticipant(Participant), + #[derive(Clone, PartialEq, Eq, Debug)] + #[cfg_attr(feature = "std", derive(Error))] + pub enum DkgError { + /// A parameter was zero. + #[cfg_attr(feature = "std", error("a parameter was 0 (threshold {0}, participants {1})"))] + ZeroParameter(u16, u16), + /// The threshold exceeded the amount of participants. + #[cfg_attr(feature = "std", error("invalid threshold (max {1}, got {0})"))] + InvalidThreshold(u16, u16), + /// Invalid participant identifier. + #[cfg_attr( + feature = "std", + error("invalid participant (0 < participant <= {0}, yet participant is {1})") + )] + InvalidParticipant(u16, Participant), - /// An invalid proof of knowledge was provided. - #[cfg_attr(feature = "std", error("invalid proof of knowledge (participant {0})"))] - InvalidProofOfKnowledge(Participant), - /// An invalid DKG share was provided. - #[cfg_attr(feature = "std", error("invalid share (participant {participant}, blame {blame})"))] - InvalidShare { participant: Participant, blame: Option }, + /// Invalid signing set. + #[cfg_attr(feature = "std", error("invalid signing set"))] + InvalidSigningSet, + /// Invalid amount of participants. + #[cfg_attr(feature = "std", error("invalid participant quantity (expected {0}, got {1})"))] + InvalidParticipantQuantity(usize, usize), + /// A participant was duplicated. + #[cfg_attr(feature = "std", error("duplicated participant ({0})"))] + DuplicatedParticipant(Participant), + /// A participant was missing. + #[cfg_attr(feature = "std", error("missing participant {0}"))] + MissingParticipant(Participant), + + /// An invalid proof of knowledge was provided. + #[cfg_attr(feature = "std", error("invalid proof of knowledge (participant {0})"))] + InvalidProofOfKnowledge(Participant), + /// An invalid DKG share was provided. + #[cfg_attr(feature = "std", error("invalid share (participant {participant}, blame {blame})"))] + InvalidShare { participant: Participant, blame: Option }, + } } +pub use dkg_error::DkgError; #[cfg(feature = "std")] mod lib { pub use super::*; use core::ops::Deref; - use std::{io, sync::Arc, collections::HashMap}; + use alloc::sync::Arc; + use std::{io, collections::HashMap}; use zeroize::Zeroizing; @@ -158,7 +167,7 @@ mod lib { impl ThresholdParams { /// Create a new set of parameters. - pub fn new(t: u16, n: u16, i: Participant) -> Result> { + pub fn new(t: u16, n: u16, i: Participant) -> Result> { if (t == 0) || (n == 0) { Err(DkgError::ZeroParameter(t, n))?; } @@ -170,24 +179,28 @@ mod lib { Err(DkgError::InvalidParticipant(n, i))?; } - Ok(ThresholdParams { t, n, i }) + Ok(Self { t, n, i }) } /// Return the threshold for a multisig with these parameters. - pub fn t(&self) -> u16 { + #[must_use] + pub const fn t(&self) -> u16 { self.t } /// Return the amount of participants for a multisig with these parameters. - pub fn n(&self) -> u16 { + #[must_use] + pub const fn n(&self) -> u16 { self.n } /// Return the participant index of the share with these parameters. - pub fn i(&self) -> Participant { + #[must_use] + pub const fn i(&self) -> Participant { self.i } } /// Calculate the lagrange coefficient for a signing set. + #[must_use] pub fn lagrange(i: Participant, included: &[Participant]) -> F { let i_f = F::from(u64::from(u16::from(i))); @@ -239,20 +252,21 @@ mod lib { self.params.zeroize(); self.secret_share.zeroize(); self.group_key.zeroize(); - for (_, share) in self.verification_shares.iter_mut() { + for share in self.verification_shares.values_mut() { share.zeroize(); } } } impl ThresholdCore { + #[must_use] pub(crate) fn new( params: ThresholdParams, secret_share: Zeroizing, verification_shares: HashMap, - ) -> ThresholdCore { + ) -> Self { let t = (1 ..= params.t()).map(Participant).collect::>(); - ThresholdCore { + Self { params, secret_share, group_key: t.iter().map(|i| verification_shares[i] * lagrange::(*i, &t)).sum(), @@ -261,17 +275,17 @@ mod lib { } /// Parameters for these keys. - pub fn params(&self) -> ThresholdParams { + pub const fn params(&self) -> ThresholdParams { self.params } /// Secret share for these keys. - pub fn secret_share(&self) -> &Zeroizing { + pub const fn secret_share(&self) -> &Zeroizing { &self.secret_share } /// Group key for these keys. - pub fn group_key(&self) -> C::G { + pub const fn group_key(&self) -> C::G { self.group_key } @@ -304,7 +318,7 @@ mod lib { } /// Read keys from a type satisfying std::io::Read. - pub fn read(reader: &mut R) -> io::Result> { + pub fn read(reader: &mut R) -> io::Result { { let different = || io::Error::new(io::ErrorKind::Other, "deserializing ThresholdCore for another curve"); @@ -332,7 +346,7 @@ mod lib { read_u16()?, read_u16()?, Participant::new(read_u16()?) - .ok_or(io::Error::new(io::ErrorKind::Other, "invalid participant index"))?, + .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid participant index"))?, ) }; @@ -343,7 +357,7 @@ mod lib { verification_shares.insert(l, ::read_G(reader)?); } - Ok(ThresholdCore::new( + Ok(Self::new( ThresholdParams::new(t, n, i) .map_err(|_| io::Error::new(io::ErrorKind::Other, "invalid parameters"))?, secret_share, @@ -395,10 +409,10 @@ mod lib { self.group_key.zeroize(); self.included.zeroize(); self.secret_share.zeroize(); - for (_, share) in self.original_verification_shares.iter_mut() { + for share in self.original_verification_shares.values_mut() { share.zeroize(); } - for (_, share) in self.verification_shares.iter_mut() { + for share in self.verification_shares.values_mut() { share.zeroize(); } } @@ -406,8 +420,9 @@ mod lib { impl ThresholdKeys { /// Create a new set of ThresholdKeys from a ThresholdCore. - pub fn new(core: ThresholdCore) -> ThresholdKeys { - ThresholdKeys { core: Arc::new(core), offset: None } + #[must_use] + pub fn new(core: ThresholdCore) -> Self { + Self { core: Arc::new(core), offset: None } } /// Offset the keys by a given scalar to allow for various account and privacy schemes. @@ -415,7 +430,7 @@ mod lib { /// This offset is ephemeral and will not be included when these keys are serialized. It also /// accumulates, so calling offset multiple times will produce a offset of the offsets' sum. #[must_use] - pub fn offset(&self, offset: C::F) -> ThresholdKeys { + pub fn offset(&self, offset: C::F) -> Self { let mut res = self.clone(); // Carry any existing offset // Enables schemes like Monero's subaddresses which have a per-subaddress offset and then a @@ -425,7 +440,7 @@ mod lib { } /// Return the current offset in-use for these keys. - pub fn current_offset(&self) -> Option { + pub const fn current_offset(&self) -> Option { self.offset } @@ -469,7 +484,7 @@ mod lib { ); let mut verification_shares = self.verification_shares(); - for (i, share) in verification_shares.iter_mut() { + for (i, share) in &mut verification_shares { *share *= lagrange::(*i, &included); } @@ -492,19 +507,19 @@ mod lib { } impl From> for ThresholdKeys { - fn from(keys: ThresholdCore) -> ThresholdKeys { - ThresholdKeys::new(keys) + fn from(keys: ThresholdCore) -> Self { + Self::new(keys) } } impl ThresholdView { /// Return the offset for this view. - pub fn offset(&self) -> C::F { + pub const fn offset(&self) -> C::F { self.offset } /// Return the group key. - pub fn group_key(&self) -> C::G { + pub const fn group_key(&self) -> C::G { self.group_key } @@ -514,7 +529,7 @@ mod lib { } /// Return the interpolated, offset secret share. - pub fn secret_share(&self) -> &Zeroizing { + pub const fn secret_share(&self) -> &Zeroizing { &self.secret_share } diff --git a/crypto/dkg/src/promote.rs b/crypto/dkg/src/promote.rs index 8f42c19d..3abab8b5 100644 --- a/crypto/dkg/src/promote.rs +++ b/crypto/dkg/src/promote.rs @@ -1,7 +1,7 @@ use core::{marker::PhantomData, ops::Deref}; +use alloc::sync::Arc; use std::{ io::{self, Read, Write}, - sync::Arc, collections::HashMap, }; @@ -45,11 +45,8 @@ impl GeneratorProof { self.proof.write(writer) } - pub fn read(reader: &mut R) -> io::Result> { - Ok(GeneratorProof { - share: ::read_G(reader)?, - proof: DLEqProof::read(reader)?, - }) + pub fn read(reader: &mut R) -> io::Result { + Ok(Self { share: ::read_G(reader)?, proof: DLEqProof::read(reader)? }) } pub fn serialize(&self) -> Vec { @@ -70,16 +67,13 @@ pub struct GeneratorPromotion { _c2: PhantomData, } -impl GeneratorPromotion -where - C2: Ciphersuite, -{ +impl> GeneratorPromotion { /// Begin promoting keys from one generator to another. Returns a proof this share was properly /// promoted. pub fn promote( rng: &mut R, base: ThresholdKeys, - ) -> (GeneratorPromotion, GeneratorProof) { + ) -> (Self, GeneratorProof) { // Do a DLEqProof for the new generator let proof = GeneratorProof { share: C2::generator() * base.secret_share().deref(), @@ -91,7 +85,7 @@ where ), }; - (GeneratorPromotion { base, proof, _c2: PhantomData:: }, proof) + (Self { base, proof, _c2: PhantomData:: }, proof) } /// Complete promotion by taking in the proofs from all other participants. diff --git a/crypto/dkg/src/tests/mod.rs b/crypto/dkg/src/tests/mod.rs index f78489ad..6afcab88 100644 --- a/crypto/dkg/src/tests/mod.rs +++ b/crypto/dkg/src/tests/mod.rs @@ -25,7 +25,7 @@ pub const PARTICIPANTS: u16 = 5; pub const THRESHOLD: u16 = ((PARTICIPANTS * 2) / 3) + 1; /// Clone a map without a specific value. -pub fn clone_without( +pub fn clone_without( map: &HashMap, without: &K, ) -> HashMap { @@ -40,7 +40,7 @@ pub fn clone_without( pub fn recover_key(keys: &HashMap>) -> C::F { let first = keys.values().next().expect("no keys provided"); assert!(keys.len() >= first.params().t().into(), "not enough keys provided"); - let included = keys.keys().cloned().collect::>(); + let included = keys.keys().copied().collect::>(); let group_private = keys.iter().fold(C::F::ZERO, |accum, (i, keys)| { accum + (lagrange::(*i, &included) * keys.secret_share().deref()) @@ -95,6 +95,7 @@ pub fn test_ciphersuite(rng: &mut R) { test_generator_promotion::<_, C>(rng); } +#[allow(clippy::tests_outside_test_module)] #[test] fn test_with_ristretto() { test_ciphersuite::<_, ciphersuite::Ristretto>(&mut rand_core::OsRng); diff --git a/crypto/dkg/src/tests/musig.rs b/crypto/dkg/src/tests/musig.rs index bbe47342..81ff9442 100644 --- a/crypto/dkg/src/tests/musig.rs +++ b/crypto/dkg/src/tests/musig.rs @@ -24,9 +24,9 @@ pub fn test_musig(rng: &mut R) { const CONTEXT: &[u8] = b"MuSig Test"; // Empty signing set - assert!(musig::(CONTEXT, &Zeroizing::new(C::F::ZERO), &[]).is_err()); + musig::(CONTEXT, &Zeroizing::new(C::F::ZERO), &[]).unwrap_err(); // Signing set we're not part of - assert!(musig::(CONTEXT, &Zeroizing::new(C::F::ZERO), &[C::generator()]).is_err()); + musig::(CONTEXT, &Zeroizing::new(C::F::ZERO), &[C::generator()]).unwrap_err(); // Test with n keys { @@ -55,7 +55,8 @@ pub fn test_musig(rng: &mut R) { } } +#[allow(clippy::tests_outside_test_module)] #[test] fn musig_literal() { - test_musig::<_, ciphersuite::Ristretto>(&mut rand_core::OsRng) + test_musig::<_, ciphersuite::Ristretto>(&mut rand_core::OsRng); } diff --git a/crypto/dkg/src/tests/promote.rs b/crypto/dkg/src/tests/promote.rs index 99c00433..1204d5b3 100644 --- a/crypto/dkg/src/tests/promote.rs +++ b/crypto/dkg/src/tests/promote.rs @@ -14,7 +14,7 @@ use crate::{ #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] struct AltGenerator { - _curve: PhantomData, + curve: PhantomData, } impl Ciphersuite for AltGenerator { diff --git a/crypto/frost/src/algorithm.rs b/crypto/frost/src/algorithm.rs index 073b483f..541a06e2 100644 --- a/crypto/frost/src/algorithm.rs +++ b/crypto/frost/src/algorithm.rs @@ -97,8 +97,8 @@ mod sealed { impl Transcript for IetfTranscript { type Challenge = Vec; - fn new(_: &'static [u8]) -> IetfTranscript { - IetfTranscript(vec![]) + fn new(_: &'static [u8]) -> Self { + Self(vec![]) } fn domain_separate(&mut self, _: &[u8]) {} @@ -147,8 +147,9 @@ pub type IetfSchnorr = Schnorr; impl> Schnorr { /// Construct a Schnorr algorithm continuing the specified transcript. - pub fn new(transcript: T) -> Schnorr { - Schnorr { transcript, c: None, _hram: PhantomData } + #[must_use] + pub const fn new(transcript: T) -> Self { + Self { transcript, c: None, _hram: PhantomData } } } @@ -156,8 +157,9 @@ impl> IetfSchnorr { /// Construct a IETF-compatible Schnorr algorithm. /// /// Please see the `IetfSchnorr` documentation for the full details of this. - pub fn ietf() -> IetfSchnorr { - Schnorr::new(IetfTranscript(vec![])) + #[must_use] + pub const fn ietf() -> Self { + Self::new(IetfTranscript(vec![])) } } diff --git a/crypto/frost/src/curve/mod.rs b/crypto/frost/src/curve/mod.rs index 1fe0c22f..0f3a3cc5 100644 --- a/crypto/frost/src/curve/mod.rs +++ b/crypto/frost/src/curve/mod.rs @@ -46,6 +46,7 @@ pub trait Curve: Ciphersuite { const CONTEXT: &'static [u8]; /// Hash the given dst and data to a byte vector. Used to instantiate H4 and H5. + #[must_use] fn hash(dst: &[u8], data: &[u8]) -> Output { Self::H::digest([Self::CONTEXT, dst, data].concat()) } @@ -53,26 +54,31 @@ pub trait Curve: Ciphersuite { /// Field element from hash. Used during key gen and by other crates under Serai as a general /// utility. Used to instantiate H1 and H3. #[allow(non_snake_case)] + #[must_use] fn hash_to_F(dst: &[u8], msg: &[u8]) -> Self::F { ::hash_to_F(&[Self::CONTEXT, dst].concat(), msg) } /// Hash the message for the binding factor. H4 from the IETF draft. + #[must_use] fn hash_msg(msg: &[u8]) -> Output { Self::hash(b"msg", msg) } /// Hash the commitments for the binding factor. H5 from the IETF draft. + #[must_use] fn hash_commitments(commitments: &[u8]) -> Output { Self::hash(b"com", commitments) } /// Hash the commitments and message to calculate the binding factor. H1 from the IETF draft. + #[must_use] fn hash_binding_factor(binding: &[u8]) -> Self::F { ::hash_to_F(b"rho", binding) } /// Securely generate a random nonce. H3 from the IETF draft. + #[must_use] fn random_nonce( secret: &Zeroizing, rng: &mut R, diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index c6f6916f..0d648619 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -1,11 +1,8 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![doc = include_str!("../README.md")] -use core::fmt::Debug; use std::collections::HashMap; -use thiserror::Error; - /// Distributed key generation protocol. pub use dkg::{self, Participant, ThresholdParams, ThresholdCore, ThresholdKeys, ThresholdView}; @@ -23,25 +20,32 @@ pub mod sign; #[cfg(any(test, feature = "tests"))] pub mod tests; -/// Various errors possible during signing. -#[derive(Clone, Copy, PartialEq, Eq, Debug, Error)] -pub enum FrostError { - #[error("invalid participant (0 < participant <= {0}, yet participant is {1})")] - InvalidParticipant(u16, Participant), - #[error("invalid signing set ({0})")] - InvalidSigningSet(&'static str), - #[error("invalid participant quantity (expected {0}, got {1})")] - InvalidParticipantQuantity(usize, usize), - #[error("duplicated participant ({0})")] - DuplicatedParticipant(Participant), - #[error("missing participant {0}")] - MissingParticipant(Participant), +#[allow(clippy::std_instead_of_core)] +mod frost_error { + use core::fmt::Debug; + use thiserror::Error; + use dkg::Participant; + /// Various errors possible during signing. + #[derive(Clone, Copy, PartialEq, Eq, Debug, Error)] + pub enum FrostError { + #[error("invalid participant (0 < participant <= {0}, yet participant is {1})")] + InvalidParticipant(u16, Participant), + #[error("invalid signing set ({0})")] + InvalidSigningSet(&'static str), + #[error("invalid participant quantity (expected {0}, got {1})")] + InvalidParticipantQuantity(usize, usize), + #[error("duplicated participant ({0})")] + DuplicatedParticipant(Participant), + #[error("missing participant {0}")] + MissingParticipant(Participant), - #[error("invalid preprocess (participant {0})")] - InvalidPreprocess(Participant), - #[error("invalid share (participant {0})")] - InvalidShare(Participant), + #[error("invalid preprocess (participant {0})")] + InvalidPreprocess(Participant), + #[error("invalid share (participant {0})")] + InvalidShare(Participant), + } } +pub use frost_error::FrostError; /// Validate a map of values to have the expected participants. pub fn validate_map( diff --git a/crypto/frost/src/nonce.rs b/crypto/frost/src/nonce.rs index 6b156212..27eed028 100644 --- a/crypto/frost/src/nonce.rs +++ b/crypto/frost/src/nonce.rs @@ -59,8 +59,8 @@ pub(crate) struct Nonce(pub(crate) [Zeroizing; 2]); #[derive(Copy, Clone, PartialEq, Eq)] pub(crate) struct GeneratorCommitments(pub(crate) [C::G; 2]); impl GeneratorCommitments { - fn read(reader: &mut R) -> io::Result> { - Ok(GeneratorCommitments([::read_G(reader)?, ::read_G(reader)?])) + fn read(reader: &mut R) -> io::Result { + Ok(Self([::read_G(reader)?, ::read_G(reader)?])) } fn write(&self, writer: &mut W) -> io::Result<()> { @@ -82,7 +82,7 @@ impl NonceCommitments { rng: &mut R, secret_share: &Zeroizing, generators: &[C::G], - ) -> (Nonce, NonceCommitments) { + ) -> (Nonce, Self) { let nonce = Nonce::([ C::random_nonce(secret_share, &mut *rng), C::random_nonce(secret_share, &mut *rng), @@ -96,11 +96,11 @@ impl NonceCommitments { ])); } - (nonce, NonceCommitments { generators: commitments }) + (nonce, Self { generators: commitments }) } - fn read(reader: &mut R, generators: &[C::G]) -> io::Result> { - Ok(NonceCommitments { + fn read(reader: &mut R, generators: &[C::G]) -> io::Result { + Ok(Self { generators: (0 .. generators.len()) .map(|_| GeneratorCommitments::read(reader)) .collect::>()?, @@ -146,7 +146,7 @@ impl Commitments { secret_share: &Zeroizing, planned_nonces: &[Vec], context: &[u8], - ) -> (Vec>, Commitments) { + ) -> (Vec>, Self) { let mut nonces = vec![]; let mut commitments = vec![]; @@ -168,18 +168,18 @@ impl Commitments { commitments.push(these_commitments); } - let dleq = if !dleq_generators.is_empty() { + let dleq = if dleq_generators.is_empty() { + None + } else { Some(MultiDLEqProof::prove( rng, &mut dleq_transcript::(context), &dleq_generators, &dleq_nonces, )) - } else { - None }; - (nonces, Commitments { nonces: commitments, dleq }) + (nonces, Self { nonces: commitments, dleq }) } pub(crate) fn transcript(&self, t: &mut T) { @@ -219,17 +219,17 @@ impl Commitments { } } - let dleq = if !dleq_generators.is_empty() { + let dleq = if dleq_generators.is_empty() { + None + } else { let dleq = MultiDLEqProof::read(reader, dleq_generators.len())?; dleq .verify(&mut dleq_transcript::(context), &dleq_generators, &dleq_nonces) .map_err(|_| io::Error::new(io::ErrorKind::Other, "invalid DLEq proof"))?; Some(dleq) - } else { - None }; - Ok(Commitments { nonces, dleq }) + Ok(Self { nonces, dleq }) } pub(crate) fn write(&self, writer: &mut W) -> io::Result<()> { @@ -256,7 +256,7 @@ impl BindingFactor { } pub(crate) fn calculate_binding_factors(&mut self, transcript: &mut T) { - for (l, binding) in self.0.iter_mut() { + for (l, binding) in &mut self.0 { let mut transcript = transcript.clone(); transcript.append_message(b"participant", C::F::from(u64::from(u16::from(*l))).to_repr()); // It *should* be perfectly fine to reuse a binding factor for multiple nonces diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 86070add..c7a689bf 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -53,8 +53,8 @@ struct Params> { } impl> Params { - fn new(algorithm: A, keys: ThresholdKeys) -> Params { - Params { algorithm, keys } + const fn new(algorithm: A, keys: ThresholdKeys) -> Self { + Self { algorithm, keys } } fn multisig_params(&self) -> ThresholdParams { @@ -111,8 +111,8 @@ pub struct AlgorithmMachine> { impl> AlgorithmMachine { /// Creates a new machine to generate a signature with the specified keys. - pub fn new(algorithm: A, keys: ThresholdKeys) -> AlgorithmMachine { - AlgorithmMachine { params: Params::new(algorithm, keys) } + pub const fn new(algorithm: A, keys: ThresholdKeys) -> Self { + Self { params: Params::new(algorithm, keys) } } fn seeded_preprocess( diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index f20196ce..cc5065bf 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/crypto/frost/src/tests/mod.rs @@ -27,7 +27,7 @@ pub const PARTICIPANTS: u16 = 5; pub const THRESHOLD: u16 = ((PARTICIPANTS * 2) / 3) + 1; /// Clone a map without a specific value. -pub fn clone_without( +pub fn clone_without( map: &HashMap, without: &K, ) -> HashMap { @@ -57,11 +57,7 @@ pub fn algorithm_machines>( keys .iter() .filter_map(|(i, keys)| { - if included.contains(i) { - Some((*i, AlgorithmMachine::new(algorithm.clone(), keys.clone()))) - } else { - None - } + included.contains(i).then(|| (*i, AlgorithmMachine::new(algorithm.clone(), keys.clone()))) }) .collect() } @@ -177,8 +173,8 @@ pub fn sign( machines, |rng, machines| { // Cache and rebuild half of the machines - let mut included = machines.keys().cloned().collect::>(); - for i in included.drain(..) { + let included = machines.keys().copied().collect::>(); + for i in included { if (rng.next_u64() % 2) == 0 { let cache = machines.remove(&i).unwrap().cache(); machines.insert( @@ -208,13 +204,13 @@ pub fn test_schnorr_with_keys>( /// Test a basic Schnorr signature. pub fn test_schnorr>(rng: &mut R) { let keys = key_gen(&mut *rng); - test_schnorr_with_keys::<_, _, H>(&mut *rng, keys) + test_schnorr_with_keys::<_, _, H>(&mut *rng, keys); } /// Test a basic Schnorr signature, yet with MuSig. pub fn test_musig_schnorr>(rng: &mut R) { let keys = musig_key_gen(&mut *rng); - test_schnorr_with_keys::<_, _, H>(&mut *rng, keys) + test_schnorr_with_keys::<_, _, H>(&mut *rng, keys); } /// Test an offset Schnorr signature. @@ -226,7 +222,7 @@ pub fn test_offset_schnorr>(rng: &m let offset = C::F::from(5); let offset_key = group_key + (C::generator() * offset); - for (_, keys) in keys.iter_mut() { + for keys in keys.values_mut() { *keys = keys.offset(offset); assert_eq!(keys.group_key(), offset_key); } diff --git a/crypto/frost/src/tests/nonces.rs b/crypto/frost/src/tests/nonces.rs index 0091d97d..3310cd6d 100644 --- a/crypto/frost/src/tests/nonces.rs +++ b/crypto/frost/src/tests/nonces.rs @@ -26,8 +26,8 @@ struct MultiNonce { } impl MultiNonce { - fn new() -> MultiNonce { - MultiNonce { + fn new() -> Self { + Self { transcript: RecommendedTranscript::new(b"FROST MultiNonce Algorithm Test"), nonces: None, } @@ -173,16 +173,10 @@ pub fn test_invalid_commitment(rng: &mut R) { let mut preprocess = preprocesses.remove(&faulty).unwrap(); // Mutate one of the commitments - let nonce = - preprocess.commitments.nonces.get_mut(usize::try_from(rng.next_u64()).unwrap() % 2).unwrap(); + let nonce = &mut preprocess.commitments.nonces[usize::try_from(rng.next_u64()).unwrap() % 2]; let generators_len = nonce.generators.len(); - *nonce - .generators - .get_mut(usize::try_from(rng.next_u64()).unwrap() % generators_len) - .unwrap() - .0 - .get_mut(usize::try_from(rng.next_u64()).unwrap() % 2) - .unwrap() = C::G::random(&mut *rng); + nonce.generators[usize::try_from(rng.next_u64()).unwrap() % generators_len].0 + [usize::try_from(rng.next_u64()).unwrap() % 2] = C::G::random(&mut *rng); // The commitments are validated at time of deserialization (read_preprocess) // Accordingly, serialize it and read it again to make sure that errors diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index 74ab8069..e188a966 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/crypto/frost/src/tests/vectors.rs @@ -1,8 +1,8 @@ use core::ops::Deref; -use std::collections::HashMap; #[cfg(test)] -use std::str::FromStr; +use core::str::FromStr; +use std::collections::HashMap; use zeroize::Zeroizing; @@ -45,11 +45,12 @@ pub struct Vectors { // Vectors are expected to be formatted per the IETF proof of concept // The included vectors are direcly from // https://github.com/cfrg/draft-irtf-cfrg-frost/tree/draft-irtf-cfrg-frost-11/poc +#[allow(clippy::fallible_impl_from)] #[cfg(test)] impl From for Vectors { - fn from(value: serde_json::Value) -> Vectors { + fn from(value: serde_json::Value) -> Self { let to_str = |value: &serde_json::Value| value.as_str().unwrap().to_string(); - Vectors { + Self { threshold: u16::from_str(value["config"]["NUM_PARTICIPANTS"].as_str().unwrap()).unwrap(), group_secret: to_str(&value["inputs"]["group_secret_key"]), @@ -166,8 +167,9 @@ pub fn test_with_vectors>( } let mut commitments = HashMap::new(); - let mut machines = machines - .drain(..) + #[allow(clippy::needless_collect)] // Fails to compile without it due to borrow checking + let machines = machines + .into_iter() .enumerate() .map(|(c, (i, machine))| { let nonce = |i| { @@ -224,8 +226,8 @@ pub fn test_with_vectors>( .collect::>(); let mut shares = HashMap::new(); - let mut machines = machines - .drain(..) + let machines = machines + .into_iter() .enumerate() .map(|(c, (i, machine))| { let (machine, share) = machine @@ -244,7 +246,7 @@ pub fn test_with_vectors>( }) .collect::>(); - for (i, machine) in machines.drain() { + for (i, machine) in machines { let sig = machine.complete(clone_without(&shares, i)).unwrap(); let mut serialized = sig.R.to_bytes().as_ref().to_vec(); serialized.extend(sig.s.to_repr().as_ref()); @@ -265,7 +267,7 @@ pub fn test_with_vectors>( unimplemented!() } fn fill_bytes(&mut self, dest: &mut [u8]) { - dest.copy_from_slice(&self.0.remove(0)) + dest.copy_from_slice(&self.0.remove(0)); } fn try_fill_bytes(&mut self, _: &mut [u8]) -> Result<(), rand_core::Error> { unimplemented!() @@ -347,7 +349,7 @@ pub fn test_with_vectors>( machines.push((i, AlgorithmMachine::new(IetfSchnorr::::ietf(), keys[i].clone()))); } - for (i, machine) in machines.drain(..) { + for (i, machine) in machines { let (_, preprocess) = machine.preprocess(&mut frosts.clone()); // Calculate the expected nonces diff --git a/crypto/schnorrkel/src/lib.rs b/crypto/schnorrkel/src/lib.rs index 7d3b3333..e55a43ef 100644 --- a/crypto/schnorrkel/src/lib.rs +++ b/crypto/schnorrkel/src/lib.rs @@ -62,12 +62,9 @@ impl Schnorrkel { /// Create a new algorithm with the specified context. /// /// If the context is greater than or equal to 4 GB in size, this will panic. - pub fn new(context: &'static [u8]) -> Schnorrkel { - Schnorrkel { - context, - schnorr: Schnorr::new(MerlinTranscript::new(b"FROST Schnorrkel")), - msg: None, - } + #[must_use] + pub fn new(context: &'static [u8]) -> Self { + Self { context, schnorr: Schnorr::new(MerlinTranscript::new(b"FROST Schnorrkel")), msg: None } } } diff --git a/crypto/schnorrkel/src/tests.rs b/crypto/schnorrkel/src/tests.rs index 2b01ad43..6cd64f03 100644 --- a/crypto/schnorrkel/src/tests.rs +++ b/crypto/schnorrkel/src/tests.rs @@ -21,5 +21,5 @@ fn test() { let signature = sign(&mut OsRng, Schnorrkel::new(CONTEXT), keys, machines, MSG); let key = PublicKey::from_bytes(key.to_bytes().as_ref()).unwrap(); - key.verify(&mut SigningContext::new(CONTEXT).bytes(MSG), &signature).unwrap() + key.verify(&mut SigningContext::new(CONTEXT).bytes(MSG), &signature).unwrap(); }