From 3a626cc51e2396744a3058148ebaf0cf158014f2 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 7 Jul 2023 22:05:07 -0400 Subject: [PATCH] Port common, and most of crypto, to a more aggressive clippy --- .github/workflows/tests.yml | 3 +- clippy-config | 70 ++++++++++++++++++++ common/db/src/lib.rs | 19 +++--- common/std-shims/src/io.rs | 14 ++-- common/std-shims/src/lib.rs | 15 +---- common/std-shims/src/sync.rs | 18 +++--- crypto/ciphersuite/src/ed448.rs | 6 +- crypto/ciphersuite/src/kp256.rs | 4 +- crypto/ciphersuite/src/lib.rs | 2 + crypto/dalek-ff-group/src/field.rs | 65 ++++++++++--------- crypto/dalek-ff-group/src/lib.rs | 71 +++++++++++---------- crypto/dleq/src/cross_group/aos.rs | 24 +++---- crypto/dleq/src/cross_group/bits.rs | 35 +++++----- crypto/dleq/src/cross_group/mod.rs | 63 +++++++++--------- crypto/dleq/src/cross_group/scalar.rs | 3 + crypto/dleq/src/cross_group/schnorr.rs | 10 +-- crypto/dleq/src/lib.rs | 16 ++--- crypto/dleq/src/tests/cross_group/scalar.rs | 2 +- crypto/dleq/src/tests/mod.rs | 6 +- crypto/ed448/src/backend.rs | 6 +- crypto/ed448/src/field.rs | 2 +- crypto/ed448/src/lib.rs | 1 + crypto/ed448/src/point.rs | 71 +++++++++++---------- crypto/ed448/src/scalar.rs | 9 +-- crypto/ff-group-tests/src/field.rs | 14 ++-- crypto/ff-group-tests/src/lib.rs | 1 + crypto/multiexp/src/batch.rs | 11 ++-- crypto/multiexp/src/lib.rs | 6 +- crypto/schnorr/src/aggregate.rs | 3 +- crypto/schnorr/src/lib.rs | 10 +-- crypto/schnorr/src/tests/mod.rs | 2 +- crypto/transcript/src/lib.rs | 26 ++++---- crypto/transcript/src/merlin.rs | 7 +- crypto/transcript/src/tests.rs | 34 ++++++---- 34 files changed, 367 insertions(+), 282 deletions(-) create mode 100644 clippy-config diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index cdc1e11c..68d4bcd6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,7 +25,8 @@ jobs: rust-components: clippy - name: Run Clippy - run: cargo clippy --all-features --all-targets -- -D warnings -A clippy::items_after_test_module + # Allow dbg_macro when run locally, yet not when pushed + run: cargo clippy --all-features --all-targets -- -D clippy::dbg_macro $(grep "\S" ../../clippy-config | grep -v "#") deny: runs-on: ubuntu-latest diff --git a/clippy-config b/clippy-config new file mode 100644 index 00000000..d66a4503 --- /dev/null +++ b/clippy-config @@ -0,0 +1,70 @@ +# No warnings allowed +-D warnings + +# Non-default groups +-D clippy::nursery +-D clippy::pedantic + +# Stylistic preferrence +-A clippy::option-if-let-else + +# Too many false/irrelevant positives +-A clippy::redundant-pub-crate +-A clippy::similar_names + +# Frequently used +-A clippy::wildcard-imports + +# Used to avoid doing &* on copy-able items, with the * being the concern +-A clippy::explicit-deref-methods + +# Lints from clippy::restrictions + +# These are relevant for crates we want to be no-std, eventually, and aren't +# relevant for the rest +-D clippy::std_instead_of_alloc +-D clippy::std_instead_of_core +-D clippy::alloc_instead_of_core + +# Safety +-D clippy::as_conversions +-D clippy::float_cmp_const +-D clippy::disallowed_script_idents +-D clippy::wildcard_enum_match_arm + +# Clarity +-D clippy::assertions_on_result_states +-D clippy::deref_by_slicing +-D clippy::empty_structs_with_brackets +-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 + +# Quality +-D clippy::format_push_string +-D clippy::string_to_string + +# Flagged on tests being named test_ +-A clippy::module-name-repetitions + +# Flagged on items passed by value which implemented Copy +-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 + +# TODO: Enable this +# -D clippy::cargo + +# Not in nightly yet +# -D clippy::redundant_type_annotations +# -D clippy::big_endian_bytes +# -D clippy::host_endian_bytes diff --git a/common/db/src/lib.rs b/common/db/src/lib.rs index 1ff5bb82..fd1dd6bf 100644 --- a/common/db/src/lib.rs +++ b/common/db/src/lib.rs @@ -1,6 +1,8 @@ use core::fmt::Debug; +extern crate alloc; +use alloc::sync::Arc; use std::{ - sync::{Arc, RwLock}, + sync::RwLock, collections::{HashSet, HashMap}, }; @@ -23,7 +25,7 @@ pub trait Db: 'static + Send + Sync + Clone + Debug + Get { fn key(db_dst: &'static [u8], item_dst: &'static [u8], key: impl AsRef<[u8]>) -> Vec { let db_len = u8::try_from(db_dst.len()).unwrap(); let dst_len = u8::try_from(item_dst.len()).unwrap(); - [[db_len].as_ref(), db_dst, [dst_len].as_ref(), item_dst, key.as_ref()].concat().to_vec() + [[db_len].as_ref(), db_dst, [dst_len].as_ref(), item_dst, key.as_ref()].concat() } fn txn(&mut self) -> Self::Transaction<'_>; } @@ -38,7 +40,7 @@ impl<'a> Get for MemDbTxn<'a> { if self.2.contains(key.as_ref()) { return None; } - self.1.get(key.as_ref()).cloned().or(self.0 .0.read().unwrap().get(key.as_ref()).cloned()) + self.1.get(key.as_ref()).cloned().or_else(|| self.0 .0.read().unwrap().get(key.as_ref()).cloned()) } } impl<'a> DbTxn for MemDbTxn<'a> { @@ -66,22 +68,23 @@ impl<'a> DbTxn for MemDbTxn<'a> { pub struct MemDb(Arc, Vec>>>); impl PartialEq for MemDb { - fn eq(&self, other: &MemDb) -> bool { + fn eq(&self, other: &Self) -> bool { *self.0.read().unwrap() == *other.0.read().unwrap() } } impl Eq for MemDb {} impl Default for MemDb { - fn default() -> MemDb { - MemDb(Arc::new(RwLock::new(HashMap::new()))) + fn default() -> Self { + Self(Arc::new(RwLock::new(HashMap::new()))) } } impl MemDb { /// Create a new in-memory database. - pub fn new() -> MemDb { - MemDb::default() + #[must_use] + pub fn new() -> Self { + Self::default() } } diff --git a/common/std-shims/src/io.rs b/common/std-shims/src/io.rs index 4d33f135..f94d67c8 100644 --- a/common/std-shims/src/io.rs +++ b/common/std-shims/src/io.rs @@ -24,14 +24,17 @@ mod shims { } impl Error { - pub fn new(kind: ErrorKind, error: E) -> Error { - Error { kind, error: Box::new(error) } + #[must_use] + pub fn new(kind: ErrorKind, error: E) -> Self { + Self { kind, error: Box::new(error) } } - pub fn kind(&self) -> ErrorKind { + #[must_use] + pub const fn kind(&self) -> ErrorKind { self.kind } + #[must_use] pub fn into_inner(self) -> Option> { Some(self.error) } @@ -53,10 +56,7 @@ mod shims { impl Read for &[u8] { fn read(&mut self, buf: &mut [u8]) -> Result { - let mut read = buf.len(); - if self.len() < buf.len() { - read = self.len(); - } + let read = self.len().min(buf.len()); buf[.. read].copy_from_slice(&self[.. read]); *self = &self[read ..]; Ok(read) diff --git a/common/std-shims/src/lib.rs b/common/std-shims/src/lib.rs index 3eb2ec72..80a841fa 100644 --- a/common/std-shims/src/lib.rs +++ b/common/std-shims/src/lib.rs @@ -2,33 +2,20 @@ #![doc = include_str!("../README.md")] #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(not(feature = "std"))] -#[allow(unused_imports)] -#[doc(hidden)] -#[macro_use] -pub extern crate alloc; +extern crate alloc; pub mod sync; pub mod collections; pub mod io; pub mod vec { - #[cfg(not(feature = "std"))] pub use alloc::vec::*; - #[cfg(feature = "std")] - pub use std::vec::*; } pub mod str { - #[cfg(not(feature = "std"))] pub use alloc::str::*; - #[cfg(feature = "std")] - pub use std::str::*; } pub mod string { - #[cfg(not(feature = "std"))] pub use alloc::string::*; - #[cfg(feature = "std")] - pub use std::string::*; } diff --git a/common/std-shims/src/sync.rs b/common/std-shims/src/sync.rs index 05cf80ab..0685f4b4 100644 --- a/common/std-shims/src/sync.rs +++ b/common/std-shims/src/sync.rs @@ -32,24 +32,24 @@ mod oncelock_shim { pub struct OnceLock(Mutex, Option); impl OnceLock { - pub const fn new() -> OnceLock { - OnceLock(Mutex::new(false), None) + pub const fn new() -> Self { + Self(Mutex::new(false), None) } // These return a distinct Option in case of None so another caller using get_or_init doesn't // transform it from None to Some pub fn get(&self) -> Option<&T> { - if !*self.0.lock() { - None - } else { + if *self.0.lock() { self.1.as_ref() + } else { + None } } pub fn get_mut(&mut self) -> Option<&mut T> { - if !*self.0.lock() { - None - } else { + if *self.0.lock() { self.1.as_mut() + } else { + None } } @@ -57,7 +57,7 @@ mod oncelock_shim { let mut lock = self.0.lock(); if !*lock { unsafe { - (core::ptr::addr_of!(self.1) as *mut Option<_>).write_unaligned(Some(f())); + core::ptr::addr_of!(self.1).cast_mut().write_unaligned(Some(f())); } } *lock = true; diff --git a/crypto/ciphersuite/src/ed448.rs b/crypto/ciphersuite/src/ed448.rs index 8a927251..0417d12a 100644 --- a/crypto/ciphersuite/src/ed448.rs +++ b/crypto/ciphersuite/src/ed448.rs @@ -1,8 +1,8 @@ use zeroize::Zeroize; use digest::{ - typenum::U114, core_api::BlockSizeUser, Update, Output, OutputSizeUser, FixedOutput, - ExtendableOutput, XofReader, HashMarker, Digest, + typenum::U114, generic_array::GenericArray, core_api::BlockSizeUser, Update, Output, + OutputSizeUser, FixedOutput, ExtendableOutput, XofReader, HashMarker, Digest, }; use sha3::Shake256; @@ -37,7 +37,7 @@ impl Update for Shake256_114 { } impl FixedOutput for Shake256_114 { fn finalize_fixed(self) -> Output { - let mut res = Default::default(); + let mut res = GenericArray::default(); FixedOutput::finalize_into(self, &mut res); res } diff --git a/crypto/ciphersuite/src/kp256.rs b/crypto/ciphersuite/src/kp256.rs index e97a856b..37fdb2e4 100644 --- a/crypto/ciphersuite/src/kp256.rs +++ b/crypto/ciphersuite/src/kp256.rs @@ -134,7 +134,7 @@ fn test_secp256k1() { ) .to_repr() .iter() - .cloned() + .copied() .collect::>(), hex::decode("acc83278035223c1ba464e2d11bfacfc872b2b23e1041cf5f6130da21e4d8068").unwrap() ); @@ -167,7 +167,7 @@ f4e8cf80aec3f888d997900ac7e3e349944b5a6b47649fc32186d2f1238103c6\ ) .to_repr() .iter() - .cloned() + .copied() .collect::>(), hex::decode("f871dfcf6bcd199342651adc361b92c941cb6a0d8c8c1a3b91d79e2c1bf3722d").unwrap() ); diff --git a/crypto/ciphersuite/src/lib.rs b/crypto/ciphersuite/src/lib.rs index 18ce1290..317ab153 100644 --- a/crypto/ciphersuite/src/lib.rs +++ b/crypto/ciphersuite/src/lib.rs @@ -1,3 +1,5 @@ +#![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")] #![cfg_attr(not(feature = "std"), no_std)] diff --git a/crypto/dalek-ff-group/src/field.rs b/crypto/dalek-ff-group/src/field.rs index fb25281c..0b952906 100644 --- a/crypto/dalek-ff-group/src/field.rs +++ b/crypto/dalek-ff-group/src/field.rs @@ -1,5 +1,5 @@ use core::{ - ops::{DerefMut, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, + ops::{Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, iter::{Sum, Product}, }; @@ -72,7 +72,7 @@ math!( macro_rules! from_wrapper { ($uint: ident) => { impl From<$uint> for FieldElement { - fn from(a: $uint) -> FieldElement { + fn from(a: $uint) -> Self { Self(ResidueType::new(&U256::from(a))) } } @@ -106,19 +106,19 @@ impl Field for FieldElement { fn random(mut rng: impl RngCore) -> Self { let mut bytes = [0; 64]; rng.fill_bytes(&mut bytes); - FieldElement(reduce(U512::from_le_bytes(bytes))) + Self(reduce(U512::from_le_bytes(bytes))) } fn square(&self) -> Self { - FieldElement(self.0.square()) + Self(self.0.square()) } fn double(&self) -> Self { - FieldElement(self.0.add(&self.0)) + Self(self.0.add(&self.0)) } fn invert(&self) -> CtOption { - const NEG_2: FieldElement = - FieldElement(ResidueType::new(&MODULUS.saturating_sub(&U256::from_u8(2)))); + #[allow(clippy::use_self)] + const NEG_2: FieldElement = Self(ResidueType::new(&MODULUS.saturating_sub(&U256::from_u8(2)))); CtOption::new(self.pow(NEG_2), !self.is_zero()) } @@ -130,7 +130,7 @@ impl Field for FieldElement { CtOption::new(candidate, candidate.square().ct_eq(self)) } - fn sqrt_ratio(u: &FieldElement, v: &FieldElement) -> (Choice, FieldElement) { + fn sqrt_ratio(u: &Self, v: &Self) -> (Choice, Self) { let i = SQRT_M1; let u = *u; @@ -163,7 +163,7 @@ impl PrimeField for FieldElement { const NUM_BITS: u32 = 255; const CAPACITY: u32 = 254; - const TWO_INV: Self = FieldElement(ResidueType::new(&U256::from_u8(2)).invert().0); + const TWO_INV: Self = Self(ResidueType::new(&U256::from_u8(2)).invert().0); // This was calculated with the method from the ff crate docs // SageMath GF(modulus).primitive_element() @@ -174,15 +174,15 @@ impl PrimeField for FieldElement { // This was calculated via the formula from the ff crate docs // Self::MULTIPLICATIVE_GENERATOR ** ((modulus - 1) >> Self::S) - const ROOT_OF_UNITY: Self = FieldElement(ResidueType::new(&U256::from_be_hex( + const ROOT_OF_UNITY: Self = Self(ResidueType::new(&U256::from_be_hex( "2b8324804fc1df0b2b4d00993dfbd7a72f431806ad2fe478c4ee1b274a0ea0b0", ))); // Self::ROOT_OF_UNITY.invert() - const ROOT_OF_UNITY_INV: Self = FieldElement(Self::ROOT_OF_UNITY.0.invert().0); + const ROOT_OF_UNITY_INV: Self = Self(Self::ROOT_OF_UNITY.0.invert().0); // This was calculated via the formula from the ff crate docs // Self::MULTIPLICATIVE_GENERATOR ** (2 ** Self::S) - const DELTA: Self = FieldElement(ResidueType::new(&U256::from_be_hex( + const DELTA: Self = Self(ResidueType::new(&U256::from_be_hex( "0000000000000000000000000000000000000000000000000000000000000010", ))); @@ -217,24 +217,26 @@ impl PrimeFieldBits for FieldElement { impl FieldElement { /// Interpret the value as a little-endian integer, square it, and reduce it into a FieldElement. - pub fn from_square(value: [u8; 32]) -> FieldElement { + #[must_use] + pub fn from_square(value: [u8; 32]) -> Self { let value = U256::from_le_bytes(value); - FieldElement(reduce(U512::from(value.mul_wide(&value)))) + Self(reduce(U512::from(value.mul_wide(&value)))) } /// Perform an exponentation. - pub fn pow(&self, other: FieldElement) -> FieldElement { - let mut table = [FieldElement::ONE; 16]; + #[must_use] + pub fn pow(&self, other: Self) -> Self { + let mut table = [Self::ONE; 16]; table[1] = *self; for i in 2 .. 16 { table[i] = table[i - 1] * self; } - let mut res = FieldElement::ONE; + let mut res = Self::ONE; let mut bits = 0; for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let mut bit = u8_from_bool(bit.deref_mut()); + let mut bit = u8_from_bool(&mut bit); bits |= bit; bit.zeroize(); @@ -257,7 +259,8 @@ impl FieldElement { /// The result is only a valid square root if the Choice is true. /// RFC 8032 simply fails if there isn't a square root, leaving any return value undefined. /// Ristretto explicitly returns 0 or sqrt((SQRT_M1 * u) / v). - pub fn sqrt_ratio_i(u: FieldElement, v: FieldElement) -> (Choice, FieldElement) { + #[must_use] + pub fn sqrt_ratio_i(u: Self, v: Self) -> (Choice, Self) { let i = SQRT_M1; let v3 = v.square() * v; @@ -288,9 +291,9 @@ impl FieldElement { } } -impl Sum for FieldElement { - fn sum>(iter: I) -> FieldElement { - let mut res = FieldElement::ZERO; +impl Sum for FieldElement { + fn sum>(iter: I) -> Self { + let mut res = Self::ZERO; for item in iter { res += item; } @@ -298,15 +301,15 @@ impl Sum for FieldElement { } } -impl<'a> Sum<&'a FieldElement> for FieldElement { - fn sum>(iter: I) -> FieldElement { - iter.cloned().sum() +impl<'a> Sum<&'a Self> for FieldElement { + fn sum>(iter: I) -> Self { + iter.copied().sum() } } -impl Product for FieldElement { - fn product>(iter: I) -> FieldElement { - let mut res = FieldElement::ONE; +impl Product for FieldElement { + fn product>(iter: I) -> Self { + let mut res = Self::ONE; for item in iter { res *= item; } @@ -314,9 +317,9 @@ impl Product for FieldElement { } } -impl<'a> Product<&'a FieldElement> for FieldElement { - fn product>(iter: I) -> FieldElement { - iter.cloned().product() +impl<'a> Product<&'a Self> for FieldElement { + fn product>(iter: I) -> Self { + iter.copied().product() } } diff --git a/crypto/dalek-ff-group/src/lib.rs b/crypto/dalek-ff-group/src/lib.rs index 21ed119e..ac40b099 100644 --- a/crypto/dalek-ff-group/src/lib.rs +++ b/crypto/dalek-ff-group/src/lib.rs @@ -1,10 +1,12 @@ +#![allow(clippy::tests_outside_test_module)] + #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![no_std] // Prevents writing new code, in what should be a simple wrapper, which requires std #![doc = include_str!("../README.md")] use core::{ borrow::Borrow, - ops::{Deref, DerefMut, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, + ops::{Deref, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, iter::{Iterator, Sum, Product}, hash::{Hash, Hasher}, }; @@ -50,6 +52,7 @@ fn u8_from_bool(bit_ref: &mut bool) -> u8 { let bit_ref = black_box(bit_ref); let mut bit = black_box(*bit_ref); + #[allow(clippy::as_conversions, clippy::cast_lossless)] let res = black_box(bit as u8); bit.zeroize(); debug_assert!((res | 1) == 1); @@ -172,8 +175,8 @@ math_neg!(Scalar, Scalar, DScalar::add, DScalar::sub, DScalar::mul); macro_rules! from_wrapper { ($uint: ident) => { impl From<$uint> for Scalar { - fn from(a: $uint) -> Scalar { - Scalar(DScalar::from(a)) + fn from(a: $uint) -> Self { + Self(DScalar::from(a)) } } }; @@ -190,18 +193,19 @@ const MODULUS: U256 = U256::from_be_hex("1000000000000000000000000000000014def9dea2f79cd65812631a5cf5d3ed"); impl Scalar { - pub fn pow(&self, other: Scalar) -> Scalar { - let mut table = [Scalar::ONE; 16]; + #[must_use] + pub fn pow(&self, other: Self) -> Self { + let mut table = [Self::ONE; 16]; table[1] = *self; for i in 2 .. 16 { table[i] = table[i - 1] * self; } - let mut res = Scalar::ONE; + let mut res = Self::ONE; let mut bits = 0; for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let mut bit = u8_from_bool(bit.deref_mut()); + let mut bit = u8_from_bool(&mut bit); bits |= bit; bit.zeroize(); @@ -219,23 +223,25 @@ impl Scalar { } /// Perform wide reduction on a 64-byte array to create a Scalar without bias. - pub fn from_bytes_mod_order_wide(bytes: &[u8; 64]) -> Scalar { + #[must_use] + pub fn from_bytes_mod_order_wide(bytes: &[u8; 64]) -> Self { Self(DScalar::from_bytes_mod_order_wide(bytes)) } /// Derive a Scalar without bias from a digest via wide reduction. - pub fn from_hash + HashMarker>(hash: D) -> Scalar { + #[must_use] + pub fn from_hash + HashMarker>(hash: D) -> Self { let mut output = [0u8; 64]; output.copy_from_slice(&hash.finalize()); - let res = Scalar(DScalar::from_bytes_mod_order_wide(&output)); + let res = Self(DScalar::from_bytes_mod_order_wide(&output)); output.zeroize(); res } } impl Field for Scalar { - const ZERO: Scalar = Scalar(DScalar::from_bits([0; 32])); - const ONE: Scalar = Scalar(DScalar::from_bits({ + const ZERO: Self = Self(DScalar::from_bits([0; 32])); + const ONE: Self = Self(DScalar::from_bits({ let mut bytes = [0; 32]; bytes[0] = 1; bytes @@ -259,10 +265,10 @@ impl Field for Scalar { fn sqrt(&self) -> CtOption { let mod_3_8 = MODULUS.saturating_add(&U256::from_u8(3)).wrapping_div(&U256::from_u8(8)); - let mod_3_8 = Scalar::from_repr(mod_3_8.to_le_bytes()).unwrap(); + let mod_3_8 = Self::from_repr(mod_3_8.to_le_bytes()).unwrap(); let sqrt_m1 = MODULUS.saturating_sub(&U256::from_u8(1)).wrapping_div(&U256::from_u8(4)); - let sqrt_m1 = Scalar::from(2u8).pow(Scalar::from_repr(sqrt_m1.to_le_bytes()).unwrap()); + let sqrt_m1 = Self::from(2u8).pow(Self::from_repr(sqrt_m1.to_le_bytes()).unwrap()); let tv1 = self.pow(mod_3_8); let tv2 = tv1 * sqrt_m1; @@ -284,14 +290,14 @@ impl PrimeField for Scalar { const CAPACITY: u32 = 252; // 2.invert() - const TWO_INV: Scalar = Scalar(DScalar::from_bits([ + const TWO_INV: Self = Self(DScalar::from_bits([ 247, 233, 122, 46, 141, 49, 9, 44, 107, 206, 123, 81, 239, 124, 111, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, ])); // This was calculated with the method from the ff crate docs // SageMath GF(modulus).primitive_element() - const MULTIPLICATIVE_GENERATOR: Scalar = Scalar(DScalar::from_bits({ + const MULTIPLICATIVE_GENERATOR: Self = Self(DScalar::from_bits({ let mut bytes = [0; 32]; bytes[0] = 2; bytes @@ -302,26 +308,26 @@ impl PrimeField for Scalar { // This was calculated via the formula from the ff crate docs // Self::MULTIPLICATIVE_GENERATOR ** ((modulus - 1) >> Self::S) - const ROOT_OF_UNITY: Scalar = Scalar(DScalar::from_bits([ + const ROOT_OF_UNITY: Self = Self(DScalar::from_bits([ 212, 7, 190, 235, 223, 117, 135, 190, 254, 131, 206, 66, 83, 86, 240, 14, 122, 194, 193, 171, 96, 109, 61, 125, 231, 129, 121, 224, 16, 115, 74, 9, ])); // Self::ROOT_OF_UNITY.invert() - const ROOT_OF_UNITY_INV: Scalar = Scalar(DScalar::from_bits([ + const ROOT_OF_UNITY_INV: Self = Self(DScalar::from_bits([ 25, 204, 55, 113, 58, 237, 138, 153, 215, 24, 41, 96, 139, 163, 238, 5, 134, 61, 62, 84, 159, 146, 194, 130, 24, 126, 134, 31, 239, 140, 181, 6, ])); // This was calculated via the formula from the ff crate docs // Self::MULTIPLICATIVE_GENERATOR ** (2 ** Self::S) - const DELTA: Scalar = Scalar(DScalar::from_bits([ + const DELTA: Self = Self(DScalar::from_bits([ 16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ])); fn from_repr(bytes: [u8; 32]) -> CtOption { let scalar = DScalar::from_canonical_bytes(bytes); // TODO: This unwrap_or_else isn't constant time, yet we don't exactly have an alternative... - CtOption::new(Scalar(scalar.unwrap_or_else(DScalar::zero)), choice(black_box(scalar).is_some())) + CtOption::new(Self(scalar.unwrap_or_else(DScalar::zero)), choice(black_box(scalar).is_some())) } fn to_repr(&self) -> [u8; 32] { self.0.to_bytes() @@ -337,7 +343,7 @@ impl PrimeField for Scalar { // methods does not // We do not use one of its methods to ensure we write via zeroize for mut bit in bits.iter_mut() { - bit.deref_mut().zeroize(); + bit.zeroize(); } res } @@ -355,33 +361,33 @@ impl PrimeFieldBits for Scalar { } fn char_le_bits() -> FieldBits { - let mut bytes = (Scalar::ZERO - Scalar::ONE).to_repr(); + let mut bytes = (Self::ZERO - Self::ONE).to_repr(); bytes[0] += 1; debug_assert_eq!(DScalar::from_bytes_mod_order(bytes), DScalar::zero()); bytes.into() } } -impl Sum for Scalar { - fn sum>(iter: I) -> Scalar { +impl Sum for Scalar { + fn sum>(iter: I) -> Self { Self(DScalar::sum(iter)) } } -impl<'a> Sum<&'a Scalar> for Scalar { - fn sum>(iter: I) -> Scalar { +impl<'a> Sum<&'a Self> for Scalar { + fn sum>(iter: I) -> Self { Self(DScalar::sum(iter)) } } -impl Product for Scalar { - fn product>(iter: I) -> Scalar { +impl Product for Scalar { + fn product>(iter: I) -> Self { Self(DScalar::product(iter)) } } -impl<'a> Product<&'a Scalar> for Scalar { - fn product>(iter: I) -> Scalar { +impl<'a> Product<&'a Self> for Scalar { + fn product>(iter: I) -> Self { Self(DScalar::product(iter)) } } @@ -502,8 +508,9 @@ dalek_group!( ); impl EdwardsPoint { - pub fn mul_by_cofactor(&self) -> EdwardsPoint { - EdwardsPoint(self.0.mul_by_cofactor()) + #[must_use] + pub fn mul_by_cofactor(&self) -> Self { + Self(self.0.mul_by_cofactor()) } } diff --git a/crypto/dleq/src/cross_group/aos.rs b/crypto/dleq/src/cross_group/aos.rs index 4cba3c89..ce521d99 100644 --- a/crypto/dleq/src/cross_group/aos.rs +++ b/crypto/dleq/src/cross_group/aos.rs @@ -37,12 +37,12 @@ pub(crate) enum Re { impl Re { #[allow(non_snake_case)] - pub(crate) fn R_default() -> Re { - Re::R(G0::identity(), G1::identity()) + pub(crate) fn R_default() -> Self { + Self::R(G0::identity(), G1::identity()) } - pub(crate) fn e_default() -> Re { - Re::e(G0::Scalar::ZERO) + pub(crate) const fn e_default() -> Self { + Self::e(G0::Scalar::ZERO) } } @@ -122,13 +122,13 @@ where #[allow(non_snake_case)] let mut R = original_R; - for i in ((actual + 1) .. (actual + RING_LEN + 1)).map(|i| i % RING_LEN) { + for i in ((actual + 1) ..= (actual + RING_LEN)).map(|i| i % RING_LEN) { let e = Self::nonces(transcript.clone(), R); if i == 0 { match Re_0 { Re::R(ref mut R0_0, ref mut R1_0) => { *R0_0 = R.0; - *R1_0 = R.1 + *R1_0 = R.1; } Re::e(ref mut e_0) => *e_0 = e.0, } @@ -144,15 +144,15 @@ where r.0.zeroize(); r.1.zeroize(); break; - // Generate a decoy response - } else { - s[i] = (G0::Scalar::random(&mut *rng), G1::Scalar::random(&mut *rng)); } + // Generate a decoy response + s[i] = (G0::Scalar::random(&mut *rng), G1::Scalar::random(&mut *rng)); + R = Self::R(generators, s[i], ring[i], e); } - Aos { Re_0, s } + Self { Re_0, s } } // Assumes the ring has already been transcripted in some form. Critically insecure if it hasn't @@ -234,7 +234,7 @@ where match Re_0 { Re::R(ref mut R0, ref mut R1) => { *R0 = read_point(r)?; - *R1 = read_point(r)? + *R1 = read_point(r)?; } Re::e(ref mut e) => *e = read_scalar(r)?, } @@ -244,6 +244,6 @@ where *s = (read_scalar(r)?, read_scalar(r)?); } - Ok(Aos { Re_0, s }) + Ok(Self { Re_0, s }) } } diff --git a/crypto/dleq/src/cross_group/bits.rs b/crypto/dleq/src/cross_group/bits.rs index 1995fad1..9e54c43f 100644 --- a/crypto/dleq/src/cross_group/bits.rs +++ b/crypto/dleq/src/cross_group/bits.rs @@ -28,42 +28,39 @@ pub(crate) enum BitSignature { impl BitSignature { pub(crate) const fn to_u8(&self) -> u8 { match self { - BitSignature::ClassicLinear => 0, - BitSignature::ConciseLinear => 1, - BitSignature::EfficientLinear => 2, - BitSignature::CompromiseLinear => 3, + Self::ClassicLinear => 0, + Self::ConciseLinear => 1, + Self::EfficientLinear => 2, + Self::CompromiseLinear => 3, } } - pub(crate) const fn from(algorithm: u8) -> BitSignature { + pub(crate) const fn from(algorithm: u8) -> Self { match algorithm { - 0 => BitSignature::ClassicLinear, - 1 => BitSignature::ConciseLinear, - 2 => BitSignature::EfficientLinear, - 3 => BitSignature::CompromiseLinear, + 0 => Self::ClassicLinear, + 1 => Self::ConciseLinear, + 2 => Self::EfficientLinear, + 3 => Self::CompromiseLinear, _ => panic!("Unknown algorithm"), } } pub(crate) const fn bits(&self) -> usize { match self { - BitSignature::ClassicLinear => 1, - BitSignature::ConciseLinear => 2, - BitSignature::EfficientLinear => 1, - BitSignature::CompromiseLinear => 2, + Self::ClassicLinear | Self::EfficientLinear => 1, + Self::ConciseLinear | Self::CompromiseLinear => 2, } } pub(crate) const fn ring_len(&self) -> usize { + #[allow(clippy::as_conversions, clippy::cast_possible_truncation)] // Needed for const 2_usize.pow(self.bits() as u32) } fn aos_form(&self) -> Re { match self { - BitSignature::ClassicLinear => Re::e_default(), - BitSignature::ConciseLinear => Re::e_default(), - BitSignature::EfficientLinear => Re::R_default(), - BitSignature::CompromiseLinear => Re::R_default(), + Self::ClassicLinear | Self::ConciseLinear => Re::e_default(), + Self::EfficientLinear | Self::CompromiseLinear => Re::R_default(), } } } @@ -139,7 +136,7 @@ where bits.zeroize(); Self::shift(pow_2); - Bits { commitments, signature } + Self { commitments, signature } } pub(crate) fn verify( @@ -174,7 +171,7 @@ where #[cfg(feature = "serialize")] pub(crate) fn read(r: &mut R) -> std::io::Result { - Ok(Bits { + Ok(Self { commitments: (read_point(r)?, read_point(r)?), signature: Aos::read(r, BitSignature::from(SIGNATURE).aos_form())?, }) diff --git a/crypto/dleq/src/cross_group/mod.rs b/crypto/dleq/src/cross_group/mod.rs index c08ff7b2..0162a644 100644 --- a/crypto/dleq/src/cross_group/mod.rs +++ b/crypto/dleq/src/cross_group/mod.rs @@ -1,8 +1,6 @@ use core::ops::{Deref, DerefMut}; #[cfg(feature = "serialize")] -use std::io::{Read, Write}; - -use thiserror::Error; +use std::io::{self, Read, Write}; use rand_core::{RngCore, CryptoRng}; @@ -42,6 +40,7 @@ fn u8_from_bool(bit_ref: &mut bool) -> u8 { let bit_ref = black_box(bit_ref); let mut bit = black_box(*bit_ref); + #[allow(clippy::as_conversions, clippy::cast_lossless)] let res = black_box(bit as u8); bit.zeroize(); debug_assert!((res | 1) == 1); @@ -51,15 +50,15 @@ fn u8_from_bool(bit_ref: &mut bool) -> u8 { } #[cfg(feature = "serialize")] -pub(crate) fn read_point(r: &mut R) -> std::io::Result { +pub(crate) fn read_point(r: &mut R) -> io::Result { let mut repr = G::Repr::default(); r.read_exact(repr.as_mut())?; let point = G::from_bytes(&repr); let Some(point) = Option::::from(point) else { - Err(std::io::Error::new(std::io::ErrorKind::Other, "invalid point"))? + Err(io::Error::new(io::ErrorKind::Other, "invalid point"))? }; if point.to_bytes().as_ref() != repr.as_ref() { - Err(std::io::Error::new(std::io::ErrorKind::Other, "non-canonical point"))?; + Err(io::Error::new(io::ErrorKind::Other, "non-canonical point"))?; } Ok(point) } @@ -78,11 +77,11 @@ pub struct Generators { impl Generators { /// Create a new set of generators. - pub fn new(primary: G, alt: G) -> Option> { + pub fn new(primary: G, alt: G) -> Option { if primary == alt { None?; } - Some(Generators { primary, alt }) + Some(Self { primary, alt }) } fn transcript(&self, transcript: &mut T) { @@ -93,21 +92,27 @@ impl Generators { } /// Error for cross-group DLEq proofs. -#[derive(Error, PartialEq, Eq, Debug)] -pub enum DLEqError { - /// Invalid proof of knowledge. - #[error("invalid proof of knowledge")] - InvalidProofOfKnowledge, - /// Invalid proof length. - #[error("invalid proof length")] - InvalidProofLength, - /// Invalid challenge. - #[error("invalid challenge")] - InvalidChallenge, - /// Invalid proof. - #[error("invalid proof")] - InvalidProof, +#[allow(clippy::std_instead_of_core)] +mod dleq_error { + use thiserror::Error; + + #[derive(Error, PartialEq, Eq, Debug)] + pub enum DLEqError { + /// Invalid proof of knowledge. + #[error("invalid proof of knowledge")] + InvalidProofOfKnowledge, + /// Invalid proof length. + #[error("invalid proof length")] + InvalidProofLength, + /// Invalid challenge. + #[error("invalid challenge")] + InvalidChallenge, + /// Invalid proof. + #[error("invalid proof")] + InvalidProof, + } } +pub use dleq_error::DLEqError; // This should never be directly instantiated and uses a u8 to represent internal values // Any external usage is likely invalid @@ -335,7 +340,7 @@ where these_bits.zeroize(); - let proof = __DLEqProof { bits, remainder, poks }; + let proof = Self { bits, remainder, poks }; debug_assert_eq!( proof.reconstruct_keys(), (generators.0.primary * f.0.deref(), generators.1.primary * f.1.deref()) @@ -412,10 +417,8 @@ where Self::transcript(transcript, generators, keys); let batch_capacity = match BitSignature::from(SIGNATURE) { - BitSignature::ClassicLinear => 3, - BitSignature::ConciseLinear => 3, - BitSignature::EfficientLinear => (self.bits.len() + 1) * 3, - BitSignature::CompromiseLinear => (self.bits.len() + 1) * 3, + BitSignature::ClassicLinear | BitSignature::ConciseLinear => 3, + BitSignature::EfficientLinear | BitSignature::CompromiseLinear => (self.bits.len() + 1) * 3, }; let mut batch = (BatchVerifier::new(batch_capacity), BatchVerifier::new(batch_capacity)); @@ -439,7 +442,7 @@ where /// Write a Cross-Group Discrete Log Equality proof to a type satisfying std::io::Write. #[cfg(feature = "serialize")] - pub fn write(&self, w: &mut W) -> std::io::Result<()> { + pub fn write(&self, w: &mut W) -> io::Result<()> { for bit in &self.bits { bit.write(w)?; } @@ -452,7 +455,7 @@ where /// Read a Cross-Group Discrete Log Equality proof from a type satisfying std::io::Read. #[cfg(feature = "serialize")] - pub fn read(r: &mut R) -> std::io::Result { + pub fn read(r: &mut R) -> io::Result { let capacity = usize::try_from(G0::Scalar::CAPACITY.min(G1::Scalar::CAPACITY)).unwrap(); let bits_per_group = BitSignature::from(SIGNATURE).bits(); @@ -466,6 +469,6 @@ where remainder = Some(Bits::read(r)?); } - Ok(__DLEqProof { bits, remainder, poks: (SchnorrPoK::read(r)?, SchnorrPoK::read(r)?) }) + Ok(Self { bits, remainder, poks: (SchnorrPoK::read(r)?, SchnorrPoK::read(r)?) }) } } diff --git a/crypto/dleq/src/cross_group/scalar.rs b/crypto/dleq/src/cross_group/scalar.rs index 1b8eb4e5..3a0c45a0 100644 --- a/crypto/dleq/src/cross_group/scalar.rs +++ b/crypto/dleq/src/cross_group/scalar.rs @@ -7,6 +7,7 @@ use zeroize::Zeroize; use crate::cross_group::u8_from_bool; /// Convert a uniform scalar into one usable on both fields, clearing the top bits as needed. +#[must_use] pub fn scalar_normalize( mut scalar: F0, ) -> (F0, F1) { @@ -49,6 +50,7 @@ pub fn scalar_normalize( } /// Helper to convert a scalar between fields. Returns None if the scalar isn't mutually valid. +#[must_use] pub fn scalar_convert( mut scalar: F0, ) -> Option { @@ -60,6 +62,7 @@ pub fn scalar_convert( } /// Create a mutually valid scalar from bytes via bit truncation to not introduce bias. +#[must_use] pub fn mutual_scalar_from_bytes( bytes: &[u8], ) -> (F0, F1) { diff --git a/crypto/dleq/src/cross_group/schnorr.rs b/crypto/dleq/src/cross_group/schnorr.rs index ec560388..2dd691ed 100644 --- a/crypto/dleq/src/cross_group/schnorr.rs +++ b/crypto/dleq/src/cross_group/schnorr.rs @@ -47,13 +47,13 @@ where transcript: &mut T, generator: G, private_key: &Zeroizing, - ) -> SchnorrPoK { + ) -> Self { let nonce = Zeroizing::new(G::Scalar::random(rng)); #[allow(non_snake_case)] let R = generator * nonce.deref(); - SchnorrPoK { + Self { R, - s: (SchnorrPoK::hra(transcript, generator, R, generator * private_key.deref()) * + s: (Self::hra(transcript, generator, R, generator * private_key.deref()) * private_key.deref()) + nonce.deref(), } @@ -85,7 +85,7 @@ where } #[cfg(feature = "serialize")] - pub fn read(r: &mut R) -> std::io::Result> { - Ok(SchnorrPoK { R: read_point(r)?, s: read_scalar(r)? }) + pub fn read(r: &mut R) -> std::io::Result { + Ok(Self { R: read_point(r)?, s: read_scalar(r)? }) } } diff --git a/crypto/dleq/src/lib.rs b/crypto/dleq/src/lib.rs index 40f578b9..f62380c4 100644 --- a/crypto/dleq/src/lib.rs +++ b/crypto/dleq/src/lib.rs @@ -131,7 +131,7 @@ where transcript: &mut T, generators: &[G], scalar: &Zeroizing, - ) -> DLEqProof { + ) -> Self { let r = Zeroizing::new(G::Scalar::random(rng)); transcript.domain_separate(b"dleq"); @@ -144,7 +144,7 @@ where // r + ca let s = (c * scalar.deref()) + r.deref(); - DLEqProof { c, s } + Self { c, s } } // Transcript a specific generator/nonce/point (G/R/A), as used when verifying a proof. @@ -194,8 +194,8 @@ where /// Read a DLEq proof from something implementing Read. #[cfg(feature = "serialize")] - pub fn read(r: &mut R) -> io::Result> { - Ok(DLEqProof { c: read_scalar(r)?, s: read_scalar(r)? }) + pub fn read(r: &mut R) -> io::Result { + Ok(Self { c: read_scalar(r)?, s: read_scalar(r)? }) } /// Serialize a DLEq proof to a `Vec`. @@ -235,7 +235,7 @@ where transcript: &mut T, generators: &[Vec], scalars: &[Zeroizing], - ) -> MultiDLEqProof { + ) -> Self { assert_eq!( generators.len(), scalars.len(), @@ -268,7 +268,7 @@ where s.push((c * scalar.deref()) + nonce.deref()); } - MultiDLEqProof { c, s } + Self { c, s } } /// Verify each series of points share a discrete logarithm against their matching series of @@ -317,13 +317,13 @@ where /// Read a multi-DLEq proof from something implementing Read. #[cfg(feature = "serialize")] - pub fn read(r: &mut R, discrete_logs: usize) -> io::Result> { + pub fn read(r: &mut R, discrete_logs: usize) -> io::Result { let c = read_scalar(r)?; let mut s = vec![]; for _ in 0 .. discrete_logs { s.push(read_scalar(r)?); } - Ok(MultiDLEqProof { c, s }) + Ok(Self { c, s }) } /// Serialize a multi-DLEq proof to a `Vec`. diff --git a/crypto/dleq/src/tests/cross_group/scalar.rs b/crypto/dleq/src/tests/cross_group/scalar.rs index 07d9d457..7cc3a973 100644 --- a/crypto/dleq/src/tests/cross_group/scalar.rs +++ b/crypto/dleq/src/tests/cross_group/scalar.rs @@ -27,7 +27,7 @@ fn test_scalar() { // The initial scalar should equal the new scalar with Ed25519's capacity let mut initial_bytes = initial.to_repr().to_vec(); // Drop the first 4 bits to hit 252 - initial_bytes[0] &= 0b00001111; + initial_bytes[0] &= 0b0000_1111; let k_bytes = k.to_repr().to_vec(); assert_eq!(initial_bytes, k_bytes); diff --git a/crypto/dleq/src/tests/mod.rs b/crypto/dleq/src/tests/mod.rs index 104c2238..b9c1f5ca 100644 --- a/crypto/dleq/src/tests/mod.rs +++ b/crypto/dleq/src/tests/mod.rs @@ -77,7 +77,7 @@ fn test_dleq() { assert!(proof .verify( &mut transcript(), - generators[.. i].iter().cloned().rev().collect::>().as_ref(), + generators[.. i].iter().copied().rev().collect::>().as_ref(), &keys[.. i] ) .is_err()); @@ -86,7 +86,7 @@ fn test_dleq() { .verify( &mut transcript(), &generators[.. i], - keys[.. i].iter().cloned().rev().collect::>().as_ref() + keys[.. i].iter().copied().rev().collect::>().as_ref() ) .is_err()); } @@ -117,7 +117,7 @@ fn test_multi_dleq() { // 0: 0 // 1: 1, 2 // 2: 2, 3, 4 - let key_generators = generators[i .. (i + i + 1)].to_vec(); + let key_generators = generators[i ..= i + i].to_vec(); let mut these_pub_keys = vec![]; for generator in &key_generators { these_pub_keys.push(generator * key.deref()); diff --git a/crypto/ed448/src/backend.rs b/crypto/ed448/src/backend.rs index 4b889d94..6b1d4795 100644 --- a/crypto/ed448/src/backend.rs +++ b/crypto/ed448/src/backend.rs @@ -12,6 +12,7 @@ pub(crate) fn u8_from_bool(bit_ref: &mut bool) -> u8 { let bit_ref = black_box(bit_ref); let mut bit = black_box(*bit_ref); + #[allow(clippy::as_conversions, clippy::cast_lossless)] let res = black_box(bit as u8); bit.zeroize(); debug_assert!((res | 1) == 1); @@ -80,7 +81,7 @@ macro_rules! field { $DELTA: expr, ) => { use core::{ - ops::{DerefMut, Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, + ops::{Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, iter::{Sum, Product}, }; @@ -139,6 +140,7 @@ macro_rules! field { impl $FieldName { /// Perform an exponentation. + #[must_use] pub fn pow(&self, other: $FieldName) -> $FieldName { let mut table = [Self(Residue::ONE); 16]; table[1] = *self; @@ -150,7 +152,7 @@ macro_rules! field { let mut bits = 0; for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let mut bit = u8_from_bool(bit.deref_mut()); + let mut bit = u8_from_bool(&mut bit); bits |= bit; bit.zeroize(); diff --git a/crypto/ed448/src/field.rs b/crypto/ed448/src/field.rs index f14141c7..09ba3764 100644 --- a/crypto/ed448/src/field.rs +++ b/crypto/ed448/src/field.rs @@ -20,7 +20,7 @@ pub struct FieldElement(pub(crate) ResidueType); impl DefaultIsZeroes for FieldElement {} // 2**448 - 2**224 - 1 -pub(crate) const MODULUS: U448 = U448::from_be_hex(MODULUS_STR); +const MODULUS: U448 = U448::from_be_hex(MODULUS_STR); const WIDE_MODULUS: U896 = U896::from_be_hex(concat!( "00000000000000000000000000000000000000000000000000000000", diff --git a/crypto/ed448/src/lib.rs b/crypto/ed448/src/lib.rs index e596d399..17f92117 100644 --- a/crypto/ed448/src/lib.rs +++ b/crypto/ed448/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(clippy::tests_outside_test_module)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![doc = include_str!("../README.md")] #![no_std] diff --git a/crypto/ed448/src/point.rs b/crypto/ed448/src/point.rs index fa250f90..554d2f74 100644 --- a/crypto/ed448/src/point.rs +++ b/crypto/ed448/src/point.rs @@ -1,5 +1,5 @@ use core::{ - ops::{DerefMut, Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, + ops::{Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, iter::Sum, }; @@ -72,7 +72,7 @@ impl ConstantTimeEq for Point { } impl PartialEq for Point { - fn eq(&self, other: &Point) -> bool { + fn eq(&self, other: &Self) -> bool { self.ct_eq(other).into() } } @@ -81,7 +81,7 @@ impl Eq for Point {} impl ConditionallySelectable for Point { fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self { - Point { + Self { x: FieldElement::conditional_select(&a.x, &b.x, choice), y: FieldElement::conditional_select(&a.y, &b.y, choice), z: FieldElement::conditional_select(&a.z, &b.z, choice), @@ -90,8 +90,9 @@ impl ConditionallySelectable for Point { } impl Add for Point { - type Output = Point; + type Output = Self; fn add(self, other: Self) -> Self { + // add-2008-bbjlp // 12 muls, 7 additions, 4 negations let xcp = self.x * other.x; let ycp = self.y * other.y; @@ -105,7 +106,7 @@ impl Add for Point { #[allow(non_snake_case)] let G_ = B + E; - Point { + Self { x: zcp * F * ((self.x + self.y) * (other.x + other.y) - xcp - ycp), y: zcp * G_ * (ycp - xcp), z: F * G_, @@ -114,33 +115,33 @@ impl Add for Point { } impl AddAssign for Point { - fn add_assign(&mut self, other: Point) { + fn add_assign(&mut self, other: Self) { *self = *self + other; } } -impl Add<&Point> for Point { - type Output = Point; - fn add(self, other: &Point) -> Point { +impl Add<&Self> for Point { + type Output = Self; + fn add(self, other: &Self) -> Self { self + *other } } -impl AddAssign<&Point> for Point { - fn add_assign(&mut self, other: &Point) { +impl AddAssign<&Self> for Point { + fn add_assign(&mut self, other: &Self) { *self += *other; } } impl Neg for Point { - type Output = Point; + type Output = Self; fn neg(self) -> Self { - Point { x: -self.x, y: self.y, z: self.z } + Self { x: -self.x, y: self.y, z: self.z } } } impl Sub for Point { - type Output = Point; + type Output = Self; #[allow(clippy::suspicious_arithmetic_impl)] fn sub(self, other: Self) -> Self { self + other.neg() @@ -148,20 +149,20 @@ impl Sub for Point { } impl SubAssign for Point { - fn sub_assign(&mut self, other: Point) { + fn sub_assign(&mut self, other: Self) { *self = *self - other; } } -impl Sub<&Point> for Point { - type Output = Point; - fn sub(self, other: &Point) -> Point { +impl Sub<&Self> for Point { + type Output = Self; + fn sub(self, other: &Self) -> Self { self - *other } } -impl SubAssign<&Point> for Point { - fn sub_assign(&mut self, other: &Point) { +impl SubAssign<&Self> for Point { + fn sub_assign(&mut self, other: &Self) { *self -= *other; } } @@ -180,7 +181,7 @@ impl Group for Point { } } fn identity() -> Self { - Point { x: FieldElement::ZERO, y: FieldElement::ONE, z: FieldElement::ONE } + Self { x: FieldElement::ZERO, y: FieldElement::ONE, z: FieldElement::ONE } } fn generator() -> Self { G @@ -198,12 +199,12 @@ impl Group for Point { let F = xsq + ysq; #[allow(non_snake_case)] let J = F - zsq.double(); - Point { x: J * (xy.square() - xsq - ysq), y: F * (xsq - ysq), z: F * J } + Self { x: J * (xy.square() - xsq - ysq), y: F * (xsq - ysq), z: F * J } } } -impl Sum for Point { - fn sum>(iter: I) -> Point { +impl Sum for Point { + fn sum>(iter: I) -> Self { let mut res = Self::identity(); for i in iter { res += i; @@ -212,17 +213,17 @@ impl Sum for Point { } } -impl<'a> Sum<&'a Point> for Point { - fn sum>(iter: I) -> Point { - Point::sum(iter.cloned()) +impl<'a> Sum<&'a Self> for Point { + fn sum>(iter: I) -> Self { + Self::sum(iter.copied()) } } impl Mul for Point { - type Output = Point; - fn mul(self, mut other: Scalar) -> Point { + type Output = Self; + fn mul(self, mut other: Scalar) -> Self { // Precompute the optimal amount that's a multiple of 2 - let mut table = [Point::identity(); 16]; + let mut table = [Self::identity(); 16]; table[1] = self; for i in 2 .. 16 { table[i] = table[i - 1] + self; @@ -232,7 +233,7 @@ impl Mul for Point { let mut bits = 0; for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let mut bit = u8_from_bool(bit.deref_mut()); + let mut bit = u8_from_bool(&mut bit); bits |= bit; bit.zeroize(); @@ -258,8 +259,8 @@ impl MulAssign for Point { } impl Mul<&Scalar> for Point { - type Output = Point; - fn mul(self, other: &Scalar) -> Point { + type Output = Self; + fn mul(self, other: &Scalar) -> Self { self * *other } } @@ -291,14 +292,14 @@ impl GroupEncoding for Point { recover_x(y).and_then(|mut x| { x.conditional_negate(x.is_odd().ct_eq(&!sign)); let not_negative_zero = !(x.is_zero() & sign); - let point = Point { x, y, z: FieldElement::ONE }; + let point = Self { x, y, z: FieldElement::ONE }; CtOption::new(point, not_negative_zero & point.is_torsion_free()) }) }) } fn from_bytes_unchecked(bytes: &Self::Repr) -> CtOption { - Point::from_bytes(bytes) + Self::from_bytes(bytes) } fn to_bytes(&self) -> Self::Repr { diff --git a/crypto/ed448/src/scalar.rs b/crypto/ed448/src/scalar.rs index b0078285..9f6adbe3 100644 --- a/crypto/ed448/src/scalar.rs +++ b/crypto/ed448/src/scalar.rs @@ -15,12 +15,12 @@ type ResidueType = Residue; /// Ed448 Scalar field element. #[derive(Clone, Copy, PartialEq, Eq, Default, Debug)] -pub struct Scalar(pub(crate) ResidueType); +pub struct Scalar(ResidueType); impl DefaultIsZeroes for Scalar {} // 2**446 - 13818066809895115352007386748515426880336692474882178609894547503885 -pub(crate) const MODULUS: U448 = U448::from_be_hex(MODULUS_STR); +const MODULUS: U448 = U448::from_be_hex(MODULUS_STR); const WIDE_MODULUS: U896 = U896::from_be_hex(concat!( "00000000000000000000000000000000000000000000000000000000", @@ -53,9 +53,10 @@ field!( impl Scalar { /// Perform a wide reduction to obtain a non-biased Scalar. - pub fn wide_reduce(bytes: [u8; 114]) -> Scalar { + #[must_use] + pub fn wide_reduce(bytes: [u8; 114]) -> Self { let wide = U1024::from_le_slice(&[bytes.as_ref(), &[0; 14]].concat()); - Scalar(Residue::new(&U448::from_le_slice( + Self(Residue::new(&U448::from_le_slice( &wide.rem(&WIDE_REDUCTION_MODULUS).to_le_bytes()[.. 56], ))) } diff --git a/crypto/ff-group-tests/src/field.rs b/crypto/ff-group-tests/src/field.rs index e34f4c81..0b1b9c72 100644 --- a/crypto/ff-group-tests/src/field.rs +++ b/crypto/ff-group-tests/src/field.rs @@ -39,9 +39,10 @@ pub fn test_add() { /// Perform basic tests on sum. pub fn test_sum() { - assert_eq!((&[] as &[F]).iter().sum::(), F::ZERO, "[].sum() != 0"); - assert_eq!([F::ZERO].iter().sum::(), F::ZERO, "[0].sum() != 0"); - assert_eq!([F::ONE].iter().sum::(), F::ONE, "[1].sum() != 1"); + let empty_slice: &[F] = &[]; + assert_eq!(empty_slice.iter().sum::(), F::ZERO, "[].sum() != 0"); + assert_eq!(core::iter::once(F::ZERO).sum::(), F::ZERO, "[0].sum() != 0"); + assert_eq!(core::iter::once(F::ONE).sum::(), F::ONE, "[1].sum() != 1"); let two = F::ONE + F::ONE; assert_eq!([F::ONE, F::ONE].iter().sum::(), two, "[1, 1].sum() != 2"); @@ -79,9 +80,10 @@ pub fn test_mul() { /// Perform basic tests on product. pub fn test_product() { - assert_eq!((&[] as &[F]).iter().product::(), F::ONE, "[].product() != 1"); - assert_eq!([F::ZERO].iter().product::(), F::ZERO, "[0].product() != 0"); - assert_eq!([F::ONE].iter().product::(), F::ONE, "[1].product() != 1"); + let empty_slice: &[F] = &[]; + assert_eq!(empty_slice.iter().product::(), F::ONE, "[].product() != 1"); + assert_eq!(core::iter::once(F::ZERO).product::(), F::ZERO, "[0].product() != 0"); + assert_eq!(core::iter::once(F::ONE).product::(), F::ONE, "[1].product() != 1"); assert_eq!([F::ONE, F::ONE].iter().product::(), F::ONE, "[1, 1].product() != 2"); let two = F::ONE + F::ONE; diff --git a/crypto/ff-group-tests/src/lib.rs b/crypto/ff-group-tests/src/lib.rs index 0f14eb69..0b59832a 100644 --- a/crypto/ff-group-tests/src/lib.rs +++ b/crypto/ff-group-tests/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(clippy::tests_outside_test_module)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![doc = include_str!("../README.md")] diff --git a/crypto/multiexp/src/batch.rs b/crypto/multiexp/src/batch.rs index 09c0ebce..0d00a3a0 100644 --- a/crypto/multiexp/src/batch.rs +++ b/crypto/multiexp/src/batch.rs @@ -18,7 +18,7 @@ fn flat( where ::Scalar: PrimeFieldBits + Zeroize, { - Zeroizing::new(slice.iter().flat_map(|pairs| pairs.1.iter()).cloned().collect::>()) + Zeroizing::new(slice.iter().flat_map(|pairs| pairs.1.iter()).copied().collect::>()) } /// A batch verifier intended to verify a series of statements are each equivalent to zero. @@ -35,9 +35,11 @@ where ::Scalar: PrimeFieldBits + Zeroize, { /// Create a new batch verifier, expected to verify the following amount of statements. - /// This is a size hint and is not required to be accurate. - pub fn new(capacity: usize) -> BatchVerifier { - BatchVerifier(Zeroizing::new(Vec::with_capacity(capacity))) + /// + /// `capacity` is a size hint and is not required to be accurate. + #[must_use] + pub fn new(capacity: usize) -> Self { + Self(Zeroizing::new(Vec::with_capacity(capacity))) } /// Queue a statement for batch verification. @@ -110,6 +112,7 @@ where /// /// This function will only return the ID of one invalid statement, even if multiple are invalid. // A constant time variant may be beneficial for robust protocols + #[must_use] pub fn blame_vartime(&self) -> Option { let mut slice = self.0.as_slice(); while slice.len() > 1 { diff --git a/crypto/multiexp/src/lib.rs b/crypto/multiexp/src/lib.rs index b660ebb7..e84db139 100644 --- a/crypto/multiexp/src/lib.rs +++ b/crypto/multiexp/src/lib.rs @@ -2,7 +2,6 @@ #![doc = include_str!("../README.md")] #![cfg_attr(not(feature = "std"), no_std)] -use core::ops::DerefMut; #[cfg(not(feature = "std"))] #[macro_use] extern crate alloc; @@ -39,6 +38,7 @@ fn u8_from_bool(bit_ref: &mut bool) -> u8 { let bit_ref = black_box(bit_ref); let mut bit = black_box(*bit_ref); + #[allow(clippy::as_conversions, clippy::cast_lossless)] let res = black_box(bit as u8); bit.zeroize(); debug_assert!((res | 1) == 1); @@ -62,7 +62,7 @@ where groupings.push(vec![0; (bits.len() + (w_usize - 1)) / w_usize]); for (i, mut bit) in bits.iter_mut().enumerate() { - let mut bit = u8_from_bool(bit.deref_mut()); + let mut bit = u8_from_bool(&mut bit); groupings[p][i / w_usize] |= bit << (i % w_usize); bit.zeroize(); } @@ -124,7 +124,7 @@ Pippenger 6 is more efficient at 250 with 655µs per Pippenger 7 is more efficient at 475 with 500µs per Pippenger 8 is more efficient at 875 with 499µs per */ -fn algorithm(len: usize) -> Algorithm { +const fn algorithm(len: usize) -> Algorithm { #[cfg(not(debug_assertions))] if len == 0 { Algorithm::Null diff --git a/crypto/schnorr/src/aggregate.rs b/crypto/schnorr/src/aggregate.rs index d5b1d5dc..99d6f572 100644 --- a/crypto/schnorr/src/aggregate.rs +++ b/crypto/schnorr/src/aggregate.rs @@ -85,7 +85,7 @@ impl SchnorrAggregate { Rs.push(C::read_G(reader)?); } - Ok(SchnorrAggregate { Rs, s: C::read_F(reader)? }) + Ok(Self { Rs, s: C::read_F(reader)? }) } /// Write a SchnorrAggregate to something implementing Write. @@ -155,6 +155,7 @@ impl SchnorrAggregator { /// /// The DST used here must prevent a collision with whatever hash function produced the /// challenges. + #[must_use] pub fn new(dst: &'static [u8]) -> Self { let mut res = Self { digest: DigestTranscript::::new(dst), sigs: vec![] }; res.digest.domain_separate(b"signatures"); diff --git a/crypto/schnorr/src/lib.rs b/crypto/schnorr/src/lib.rs index 77d033d0..54691cb3 100644 --- a/crypto/schnorr/src/lib.rs +++ b/crypto/schnorr/src/lib.rs @@ -48,7 +48,7 @@ pub struct SchnorrSignature { impl SchnorrSignature { /// Read a SchnorrSignature from something implementing Read. pub fn read(reader: &mut R) -> io::Result { - Ok(SchnorrSignature { R: C::read_G(reader)?, s: C::read_F(reader)? }) + Ok(Self { R: C::read_G(reader)?, s: C::read_F(reader)? }) } /// Write a SchnorrSignature to something implementing Read. @@ -69,12 +69,8 @@ impl SchnorrSignature { /// This challenge must be properly crafted, which means being binding to the public key, nonce, /// and any message. Failure to do so will let a malicious adversary to forge signatures for /// different keys/messages. - pub fn sign( - private_key: &Zeroizing, - nonce: Zeroizing, - challenge: C::F, - ) -> SchnorrSignature { - SchnorrSignature { + pub fn sign(private_key: &Zeroizing, nonce: Zeroizing, challenge: C::F) -> Self { + Self { // Uses deref instead of * as * returns C::F yet deref returns &C::F, preventing a copy R: C::generator() * nonce.deref(), s: (challenge * private_key.deref()) + nonce.deref(), diff --git a/crypto/schnorr/src/tests/mod.rs b/crypto/schnorr/src/tests/mod.rs index 42509ffd..47bd9bc3 100644 --- a/crypto/schnorr/src/tests/mod.rs +++ b/crypto/schnorr/src/tests/mod.rs @@ -106,7 +106,7 @@ pub(crate) fn aggregate() { keys .iter() .map(|key| C::generator() * key.deref()) - .zip(challenges.iter().cloned()) + .zip(challenges.iter().copied()) .collect::>() .as_ref(), )); diff --git a/crypto/transcript/src/lib.rs b/crypto/transcript/src/lib.rs index e9e99005..7f28ec11 100644 --- a/crypto/transcript/src/lib.rs +++ b/crypto/transcript/src/lib.rs @@ -61,15 +61,15 @@ enum DigestTranscriptMember { } impl DigestTranscriptMember { - fn as_u8(&self) -> u8 { + const fn as_u8(&self) -> u8 { match self { - DigestTranscriptMember::Name => 0, - DigestTranscriptMember::Domain => 1, - DigestTranscriptMember::Label => 2, - DigestTranscriptMember::Value => 3, - DigestTranscriptMember::Challenge => 4, - DigestTranscriptMember::Continued => 5, - DigestTranscriptMember::Challenged => 6, + Self::Name => 0, + Self::Domain => 1, + Self::Label => 2, + Self::Value => 3, + Self::Challenge => 4, + Self::Continued => 5, + Self::Challenged => 6, } } } @@ -103,8 +103,9 @@ impl DigestTranscript { impl Transcript for DigestTranscript { type Challenge = Output; + #[must_use] fn new(name: &'static [u8]) -> Self { - let mut res = DigestTranscript(D::new()); + let mut res = Self(D::new()); res.append(DigestTranscriptMember::Name, name); res } @@ -139,10 +140,7 @@ impl Transcript for DigestTranscript { // Digest doesn't implement Zeroize // Implement Zeroize for DigestTranscript by writing twice the block size to the digest in an // attempt to overwrite the internal hash state/any leftover bytes -impl Zeroize for DigestTranscript -where - D: BlockSizeUser, -{ +impl Zeroize for DigestTranscript { fn zeroize(&mut self) { // Update in 4-byte chunks to reduce call quantity and enable word-level update optimizations const WORD_SIZE: usize = 4; @@ -187,7 +185,7 @@ where choice.zeroize(); } - mark_read(self) + mark_read(self); } } diff --git a/crypto/transcript/src/merlin.rs b/crypto/transcript/src/merlin.rs index ce4ee4d9..f2e2bf89 100644 --- a/crypto/transcript/src/merlin.rs +++ b/crypto/transcript/src/merlin.rs @@ -29,7 +29,7 @@ impl Transcript for MerlinTranscript { type Challenge = [u8; 64]; fn new(name: &'static [u8]) -> Self { - MerlinTranscript(merlin::Transcript::new(name)) + Self(merlin::Transcript::new(name)) } fn domain_separate(&mut self, label: &'static [u8]) { @@ -37,10 +37,7 @@ impl Transcript for MerlinTranscript { } fn append_message>(&mut self, label: &'static [u8], message: M) { - assert!( - label != "dom-sep".as_bytes(), - "\"dom-sep\" is reserved for the domain_separate function", - ); + assert!(label != b"dom-sep", "\"dom-sep\" is reserved for the domain_separate function",); self.0.append_message(label, message.as_ref()); } diff --git a/crypto/transcript/src/tests.rs b/crypto/transcript/src/tests.rs index 93651b03..8a56a043 100644 --- a/crypto/transcript/src/tests.rs +++ b/crypto/transcript/src/tests.rs @@ -84,20 +84,26 @@ where assert!(t().rng_seed(b"a") != t().rng_seed(b"b")); } -#[test] -fn test_digest() { - test_transcript::>(); - test_transcript::>(); -} +#[allow(clippy::module_inception)] +#[cfg(test)] +mod tests { + use super::*; -#[cfg(feature = "recommended")] -#[test] -fn test_recommended() { - test_transcript::(); -} + #[test] + fn test_digest() { + test_transcript::>(); + test_transcript::>(); + } -#[cfg(feature = "merlin")] -#[test] -fn test_merlin() { - test_transcript::(); + #[cfg(feature = "recommended")] + #[test] + fn test_recommended() { + test_transcript::(); + } + + #[cfg(feature = "merlin")] + #[test] + fn test_merlin() { + test_transcript::(); + } }