From f93106af6bc966a6096abeeaf68255a9ba648279 Mon Sep 17 00:00:00 2001 From: Luke Parker <lukeparker5132@gmail.com> Date: Sat, 8 Jul 2023 00:56:43 -0400 Subject: [PATCH] Mostly lint Monero --- clippy-config | 4 + coins/monero/build.rs | 4 +- coins/monero/generators/src/hash_to_point.rs | 4 +- coins/monero/generators/src/lib.rs | 1 + coins/monero/generators/src/varint.rs | 2 + coins/monero/src/block.rs | 12 +- coins/monero/src/lib.rs | 56 ++++--- coins/monero/src/merkle.rs | 3 +- coins/monero/src/ringct/borromean.rs | 20 +-- coins/monero/src/ringct/bulletproofs/core.rs | 16 +- coins/monero/src/ringct/bulletproofs/mod.rs | 27 ++-- .../src/ringct/bulletproofs/original.rs | 21 ++- coins/monero/src/ringct/bulletproofs/plus.rs | 28 +--- .../src/ringct/bulletproofs/scalar_vector.rs | 34 ++--- coins/monero/src/ringct/clsag/mod.rs | 64 ++++---- coins/monero/src/ringct/clsag/multisig.rs | 19 ++- coins/monero/src/ringct/hash_to_point.rs | 1 + coins/monero/src/ringct/mlsag.rs | 7 +- coins/monero/src/ringct/mod.rs | 141 +++++++++--------- coins/monero/src/rpc/http.rs | 4 +- coins/monero/src/rpc/mod.rs | 53 ++++--- coins/monero/src/serialize.rs | 2 +- coins/monero/src/tests/clsag.rs | 28 ++-- coins/monero/src/transaction.rs | 53 ++++--- coins/monero/src/wallet/address.rs | 71 +++++---- coins/monero/src/wallet/decoys.rs | 7 +- coins/monero/src/wallet/extra.rs | 52 ++++--- coins/monero/src/wallet/mod.rs | 21 ++- coins/monero/src/wallet/scan.rs | 81 +++++----- coins/monero/src/wallet/seed/classic.rs | 16 +- coins/monero/src/wallet/seed/mod.rs | 53 ++++--- coins/monero/src/wallet/send/builder.rs | 6 +- coins/monero/src/wallet/send/mod.rs | 86 ++++++----- coins/monero/src/wallet/send/multisig.rs | 14 +- common/std-shims/src/sync.rs | 1 + 35 files changed, 553 insertions(+), 459 deletions(-) diff --git a/clippy-config b/clippy-config index f0164aab..a1786a8e 100644 --- a/clippy-config +++ b/clippy-config @@ -15,7 +15,11 @@ -A clippy::redundant_pub_crate -A clippy::similar_names +# Flags on any debug_assert using an RNG +-A clippy::debug_assert_with_mut_call + # Frequently used +-A clippy::large_types_passed_by_value -A clippy::wildcard_imports -A clippy::too_many_lines diff --git a/coins/monero/build.rs b/coins/monero/build.rs index 34c34b6b..9194c4b9 100644 --- a/coins/monero/build.rs +++ b/coins/monero/build.rs @@ -28,10 +28,10 @@ fn serialize(generators_string: &mut String, points: &[EdwardsPoint]) { fn generators(prefix: &'static str, path: &str) { let generators = bulletproofs_generators(prefix.as_bytes()); #[allow(non_snake_case)] - let mut G_str = "".to_string(); + let mut G_str = String::new(); serialize(&mut G_str, &generators.G); #[allow(non_snake_case)] - let mut H_str = "".to_string(); + let mut H_str = String::new(); serialize(&mut H_str, &generators.H); let path = Path::new(&env::var("OUT_DIR").unwrap()).join(path); diff --git a/coins/monero/generators/src/hash_to_point.rs b/coins/monero/generators/src/hash_to_point.rs index 3f9156b9..9ef0c3cd 100644 --- a/coins/monero/generators/src/hash_to_point.rs +++ b/coins/monero/generators/src/hash_to_point.rs @@ -8,8 +8,10 @@ use dalek_ff_group::FieldElement; use crate::hash; /// Monero's hash to point function, as named `ge_fromfe_frombytes_vartime`. +#[allow(clippy::many_single_char_names)] +#[must_use] pub fn hash_to_point(bytes: [u8; 32]) -> EdwardsPoint { - #[allow(non_snake_case)] + #[allow(non_snake_case, clippy::unreadable_literal)] let A = FieldElement::from(486662u64); let v = FieldElement::from_square(hash(&bytes)).double(); diff --git a/coins/monero/generators/src/lib.rs b/coins/monero/generators/src/lib.rs index 7f630f36..66bef901 100644 --- a/coins/monero/generators/src/lib.rs +++ b/coins/monero/generators/src/lib.rs @@ -61,6 +61,7 @@ pub struct Generators { } /// Generate generators as needed for Bulletproofs(+), as Monero does. +#[must_use] pub fn bulletproofs_generators(dst: &'static [u8]) -> Generators { let mut res = Generators { G: [EdwardsPoint::identity(); MAX_MN], H: [EdwardsPoint::identity(); MAX_MN] }; diff --git a/coins/monero/generators/src/varint.rs b/coins/monero/generators/src/varint.rs index 2e82816e..32141bac 100644 --- a/coins/monero/generators/src/varint.rs +++ b/coins/monero/generators/src/varint.rs @@ -1,6 +1,8 @@ use std_shims::io::{self, Write}; const VARINT_CONTINUATION_MASK: u8 = 0b1000_0000; + +#[allow(clippy::trivially_copy_pass_by_ref)] // &u64 is needed for API consistency pub(crate) fn write_varint<W: Write>(varint: &u64, w: &mut W) -> io::Result<()> { let mut varint = *varint; while { diff --git a/coins/monero/src/block.rs b/coins/monero/src/block.rs index 751b04f7..a2c1289e 100644 --- a/coins/monero/src/block.rs +++ b/coins/monero/src/block.rs @@ -33,14 +33,15 @@ impl BlockHeader { w.write_all(&self.nonce.to_le_bytes()) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<BlockHeader> { - Ok(BlockHeader { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { major_version: read_varint(r)?, minor_version: read_varint(r)?, timestamp: read_varint(r)?, @@ -58,6 +59,7 @@ pub struct Block { } impl Block { + #[must_use] pub fn number(&self) -> usize { match self.miner_tx.prefix.inputs.get(0) { Some(Input::Gen(number)) => (*number).try_into().unwrap(), @@ -91,6 +93,7 @@ impl Block { out } + #[must_use] pub fn hash(&self) -> [u8; 32] { let hash = hash(&self.serialize_hashable()); if hash == CORRECT_BLOCK_HASH_202612 { @@ -100,14 +103,15 @@ impl Block { hash } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<Block> { - Ok(Block { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { header: BlockHeader::read(r)?, miner_tx: Transaction::read(r)?, txs: (0 .. read_varint(r)?).map(|_| read_bytes(r)).collect::<Result<_, _>>()?, diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index 74b42f81..7617a975 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -60,39 +60,42 @@ pub enum Protocol { impl Protocol { /// Amount of ring members under this protocol version. - pub fn ring_len(&self) -> usize { + #[must_use] + pub const fn ring_len(&self) -> usize { match self { - Protocol::v14 => 11, - Protocol::v16 => 16, - Protocol::Custom { ring_len, .. } => *ring_len, + Self::v14 => 11, + Self::v16 => 16, + Self::Custom { ring_len, .. } => *ring_len, } } /// Whether or not the specified version uses Bulletproofs or Bulletproofs+. /// /// This method will likely be reworked when versions not using Bulletproofs at all are added. - pub fn bp_plus(&self) -> bool { + #[must_use] + pub const fn bp_plus(&self) -> bool { match self { - Protocol::v14 => false, - Protocol::v16 => true, - Protocol::Custom { bp_plus, .. } => *bp_plus, + Self::v14 => false, + Self::v16 => true, + Self::Custom { bp_plus, .. } => *bp_plus, } } // TODO: Make this an Option when we support pre-RCT protocols - pub fn optimal_rct_type(&self) -> RctType { + #[must_use] + pub const fn optimal_rct_type(&self) -> RctType { match self { - Protocol::v14 => RctType::Clsag, - Protocol::v16 => RctType::BulletproofsPlus, - Protocol::Custom { optimal_rct_type, .. } => *optimal_rct_type, + Self::v14 => RctType::Clsag, + Self::v16 => RctType::BulletproofsPlus, + Self::Custom { optimal_rct_type, .. } => *optimal_rct_type, } } pub(crate) fn write<W: io::Write>(&self, w: &mut W) -> io::Result<()> { match self { - Protocol::v14 => w.write_all(&[0, 14]), - Protocol::v16 => w.write_all(&[0, 16]), - Protocol::Custom { ring_len, bp_plus, optimal_rct_type } => { + Self::v14 => w.write_all(&[0, 14]), + Self::v16 => w.write_all(&[0, 16]), + Self::Custom { ring_len, bp_plus, optimal_rct_type } => { // Custom, version 0 w.write_all(&[1, 0])?; w.write_all(&u16::try_from(*ring_len).unwrap().to_le_bytes())?; @@ -102,17 +105,17 @@ impl Protocol { } } - pub(crate) fn read<R: io::Read>(r: &mut R) -> io::Result<Protocol> { + pub(crate) fn read<R: io::Read>(r: &mut R) -> io::Result<Self> { Ok(match read_byte(r)? { // Monero protocol 0 => match read_byte(r)? { - 14 => Protocol::v14, - 16 => Protocol::v16, + 14 => Self::v14, + 16 => Self::v16, _ => Err(io::Error::new(io::ErrorKind::Other, "unrecognized monero protocol"))?, }, // Custom 1 => match read_byte(r)? { - 0 => Protocol::Custom { + 0 => Self::Custom { ring_len: read_u16(r)?.into(), bp_plus: match read_byte(r)? { 0 => false, @@ -140,22 +143,26 @@ pub struct Commitment { } impl Commitment { - /// The zero commitment, defined as a mask of 1 (as to not be the identity) and a 0 amount. - pub fn zero() -> Commitment { - Commitment { mask: Scalar::one(), amount: 0 } + /// A commitment to zero, defined with a mask of 1 (as to not be the identity). + #[must_use] + pub fn zero() -> Self { + Self { mask: Scalar::one(), amount: 0 } } - pub fn new(mask: Scalar, amount: u64) -> Commitment { - Commitment { mask, amount } + #[must_use] + pub const fn new(mask: Scalar, amount: u64) -> Self { + Self { mask, amount } } /// Calculate a Pedersen commitment, as a point, from the transparent structure. + #[must_use] pub fn calculate(&self) -> EdwardsPoint { (&self.mask * &ED25519_BASEPOINT_TABLE) + (Scalar::from(self.amount) * H()) } } /// Support generating a random scalar using a modern rand, as dalek's is notoriously dated. +#[must_use] pub fn random_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Scalar { let mut r = [0; 64]; rng.fill_bytes(&mut r); @@ -167,6 +174,7 @@ pub(crate) fn hash(data: &[u8]) -> [u8; 32] { } /// Hash the provided data to a scalar via keccak256(data) % l. +#[must_use] pub fn hash_to_scalar(data: &[u8]) -> Scalar { let scalar = Scalar::from_bytes_mod_order(hash(data)); // Monero will explicitly error in this case diff --git a/coins/monero/src/merkle.rs b/coins/monero/src/merkle.rs index 1671adf2..b1c5faa2 100644 --- a/coins/monero/src/merkle.rs +++ b/coins/monero/src/merkle.rs @@ -2,7 +2,8 @@ use std_shims::vec::Vec; use crate::hash; -pub fn merkle_root(root: [u8; 32], leafs: &[[u8; 32]]) -> [u8; 32] { +#[must_use] +pub(crate) fn merkle_root(root: [u8; 32], leafs: &[[u8; 32]]) -> [u8; 32] { match leafs.len() { 0 => root, 1 => hash(&[root, leafs[0]].concat()), diff --git a/coins/monero/src/ringct/borromean.rs b/coins/monero/src/ringct/borromean.rs index d0bf5b2a..c07acb62 100644 --- a/coins/monero/src/ringct/borromean.rs +++ b/coins/monero/src/ringct/borromean.rs @@ -26,19 +26,15 @@ pub struct BorromeanSignatures { } impl BorromeanSignatures { - pub fn read<R: Read>(r: &mut R) -> io::Result<BorromeanSignatures> { - Ok(BorromeanSignatures { - s0: read_array(read_bytes, r)?, - s1: read_array(read_bytes, r)?, - ee: read_bytes(r)?, - }) + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { s0: read_array(read_bytes, r)?, s1: read_array(read_bytes, r)?, ee: read_bytes(r)? }) } pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { - for s0 in self.s0.iter() { + for s0 in &self.s0 { w.write_all(s0)?; } - for s1 in self.s1.iter() { + for s1 in &self.s1 { w.write_all(s1)?; } w.write_all(&self.ee) @@ -79,11 +75,8 @@ pub struct BorromeanRange { } impl BorromeanRange { - pub fn read<R: Read>(r: &mut R) -> io::Result<BorromeanRange> { - Ok(BorromeanRange { - sigs: BorromeanSignatures::read(r)?, - bit_commitments: read_array(read_point, r)?, - }) + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { sigs: BorromeanSignatures::read(r)?, bit_commitments: read_array(read_point, r)? }) } pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { self.sigs.write(w)?; @@ -91,6 +84,7 @@ impl BorromeanRange { } #[cfg(feature = "experimental")] + #[must_use] pub fn verify(&self, commitment: &EdwardsPoint) -> bool { if &self.bit_commitments.iter().sum::<EdwardsPoint>() != commitment { return false; diff --git a/coins/monero/src/ringct/bulletproofs/core.rs b/coins/monero/src/ringct/bulletproofs/core.rs index 1f30567f..66908dfa 100644 --- a/coins/monero/src/ringct/bulletproofs/core.rs +++ b/coins/monero/src/ringct/bulletproofs/core.rs @@ -50,13 +50,13 @@ pub(crate) fn vector_exponent( pub(crate) fn hash_cache(cache: &mut Scalar, mash: &[[u8; 32]]) -> Scalar { let slice = - &[cache.to_bytes().as_ref(), mash.iter().cloned().flatten().collect::<Vec<_>>().as_ref()] + &[cache.to_bytes().as_ref(), mash.iter().copied().flatten().collect::<Vec<_>>().as_ref()] .concat(); *cache = hash_to_scalar(slice); *cache } -pub(crate) fn MN(outputs: usize) -> (usize, usize, usize) { +pub(crate) const fn MN(outputs: usize) -> (usize, usize, usize) { let mut logM = 0; let mut M; while { @@ -78,10 +78,8 @@ pub(crate) fn bit_decompose(commitments: &[Commitment]) -> (ScalarVector, Scalar for j in 0 .. M { for i in (0 .. N).rev() { - let mut bit = Choice::from(0); - if j < sv.len() { - bit = Choice::from((sv[j][i / 8] >> (i % 8)) & 1); - } + let bit = + if j < sv.len() { Choice::from((sv[j][i / 8] >> (i % 8)) & 1) } else { Choice::from(0) }; aL.0[(j * N) + i] = Scalar::conditional_select(&Scalar::ZERO, &Scalar::ONE, bit); aR.0[(j * N) + i] = Scalar::conditional_select(&-Scalar::ONE, &Scalar::ZERO, bit); } @@ -118,9 +116,9 @@ pub(crate) fn LR_statements( let mut res = a .0 .iter() - .cloned() - .zip(G_i.iter().cloned()) - .chain(b.0.iter().cloned().zip(H_i.iter().cloned())) + .copied() + .zip(G_i.iter().copied()) + .chain(b.0.iter().copied().zip(H_i.iter().copied())) .collect::<Vec<_>>(); res.push((cL, U)); res diff --git a/coins/monero/src/ringct/bulletproofs/mod.rs b/coins/monero/src/ringct/bulletproofs/mod.rs index c4d17369..07dc4d07 100644 --- a/coins/monero/src/ringct/bulletproofs/mod.rs +++ b/coins/monero/src/ringct/bulletproofs/mod.rs @@ -62,14 +62,14 @@ impl Bulletproofs { rng: &mut R, outputs: &[Commitment], plus: bool, - ) -> Result<Bulletproofs, TransactionError> { + ) -> Result<Self, TransactionError> { if outputs.len() > MAX_OUTPUTS { return Err(TransactionError::TooManyOutputs)?; } Ok(if !plus { - Bulletproofs::Original(OriginalStruct::prove(rng, outputs)) + Self::Plus(PlusStruct::prove(rng, outputs)) } else { - Bulletproofs::Plus(PlusStruct::prove(rng, outputs)) + Self::Original(OriginalStruct::prove(rng, outputs)) }) } @@ -77,8 +77,8 @@ impl Bulletproofs { #[must_use] pub fn verify<R: RngCore + CryptoRng>(&self, rng: &mut R, commitments: &[EdwardsPoint]) -> bool { match self { - Bulletproofs::Original(bp) => bp.verify(rng, commitments), - Bulletproofs::Plus(bp) => bp.verify(rng, commitments), + Self::Original(bp) => bp.verify(rng, commitments), + Self::Plus(bp) => bp.verify(rng, commitments), } } @@ -94,8 +94,8 @@ impl Bulletproofs { commitments: &[EdwardsPoint], ) -> bool { match self { - Bulletproofs::Original(bp) => bp.batch_verify(rng, verifier, id, commitments), - Bulletproofs::Plus(bp) => bp.batch_verify(rng, verifier, id, commitments), + Self::Original(bp) => bp.batch_verify(rng, verifier, id, commitments), + Self::Plus(bp) => bp.batch_verify(rng, verifier, id, commitments), } } @@ -105,7 +105,7 @@ impl Bulletproofs { specific_write_vec: F, ) -> io::Result<()> { match self { - Bulletproofs::Original(bp) => { + Self::Original(bp) => { write_point(&bp.A, w)?; write_point(&bp.S, w)?; write_point(&bp.T1, w)?; @@ -119,7 +119,7 @@ impl Bulletproofs { write_scalar(&bp.t, w) } - Bulletproofs::Plus(bp) => { + Self::Plus(bp) => { write_point(&bp.A, w)?; write_point(&bp.A1, w)?; write_point(&bp.B, w)?; @@ -140,6 +140,7 @@ impl Bulletproofs { self.write_core(w, |points, w| write_vec(write_point, points, w)) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized).unwrap(); @@ -147,8 +148,8 @@ impl Bulletproofs { } /// Read Bulletproofs. - pub fn read<R: Read>(r: &mut R) -> io::Result<Bulletproofs> { - Ok(Bulletproofs::Original(OriginalStruct { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self::Original(OriginalStruct { A: read_point(r)?, S: read_point(r)?, T1: read_point(r)?, @@ -164,8 +165,8 @@ impl Bulletproofs { } /// Read Bulletproofs+. - pub fn read_plus<R: Read>(r: &mut R) -> io::Result<Bulletproofs> { - Ok(Bulletproofs::Plus(PlusStruct { + pub fn read_plus<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self::Plus(PlusStruct { A: read_point(r)?, A1: read_point(r)?, B: read_point(r)?, diff --git a/coins/monero/src/ringct/bulletproofs/original.rs b/coins/monero/src/ringct/bulletproofs/original.rs index 3fd15ef2..1dbff606 100644 --- a/coins/monero/src/ringct/bulletproofs/original.rs +++ b/coins/monero/src/ringct/bulletproofs/original.rs @@ -36,10 +36,8 @@ pub struct OriginalStruct { } impl OriginalStruct { - pub(crate) fn prove<R: RngCore + CryptoRng>( - rng: &mut R, - commitments: &[Commitment], - ) -> OriginalStruct { + #[allow(clippy::many_single_char_names)] + pub(crate) fn prove<R: RngCore + CryptoRng>(rng: &mut R, commitments: &[Commitment]) -> Self { let (logMN, M, MN) = MN(commitments.len()); let (aL, aR) = bit_decompose(commitments); @@ -134,8 +132,8 @@ impl OriginalStruct { let L_i = prove_multiexp(&LR_statements(&aL, G_R, &bR, H_L, cL, U)); let R_i = prove_multiexp(&LR_statements(&aR, G_L, &bL, H_R, cR, U)); - L.push(L_i); - R.push(R_i); + L.push(*L_i); + R.push(*R_i); let w = hash_cache(&mut cache, &[L_i.compress().to_bytes(), R_i.compress().to_bytes()]); let winv = w.invert().unwrap(); @@ -149,15 +147,15 @@ impl OriginalStruct { } } - let res = OriginalStruct { + let res = Self { A: *A, S: *S, T1: *T1, T2: *T2, taux: *taux, mu: *mu, - L: L.drain(..).map(|L| *L).collect(), - R: R.drain(..).map(|R| *R).collect(), + L, + R, a: *a[0], b: *b[0], t: *t, @@ -166,6 +164,7 @@ impl OriginalStruct { res } + #[allow(clippy::many_single_char_names)] #[must_use] fn verify_core<ID: Copy + Zeroize, R: RngCore + CryptoRng>( &self, @@ -190,7 +189,7 @@ impl OriginalStruct { } // Rebuild all challenges - let (mut cache, commitments) = hash_commitments(commitments.iter().cloned()); + let (mut cache, commitments) = hash_commitments(commitments.iter().copied()); let y = hash_cache(&mut cache, &[self.A.compress().to_bytes(), self.S.compress().to_bytes()]); let z = hash_to_scalar(&y.to_bytes()); @@ -223,7 +222,7 @@ impl OriginalStruct { let A = normalize(&self.A); let S = normalize(&self.S); - let commitments = commitments.iter().map(|c| c.mul_by_cofactor()).collect::<Vec<_>>(); + let commitments = commitments.iter().map(EdwardsPoint::mul_by_cofactor).collect::<Vec<_>>(); // Verify it let mut proof = Vec::with_capacity(4 + commitments.len()); diff --git a/coins/monero/src/ringct/bulletproofs/plus.rs b/coins/monero/src/ringct/bulletproofs/plus.rs index 0cccc549..5b12812d 100644 --- a/coins/monero/src/ringct/bulletproofs/plus.rs +++ b/coins/monero/src/ringct/bulletproofs/plus.rs @@ -56,10 +56,8 @@ pub struct PlusStruct { } impl PlusStruct { - pub(crate) fn prove<R: RngCore + CryptoRng>( - rng: &mut R, - commitments: &[Commitment], - ) -> PlusStruct { + #[allow(clippy::many_single_char_names)] + pub(crate) fn prove<R: RngCore + CryptoRng>(rng: &mut R, commitments: &[Commitment]) -> Self { let generators = GENERATORS(); let (logMN, M, MN) = MN(commitments.len()); @@ -113,12 +111,12 @@ impl PlusStruct { let mut L_i = LR_statements(&(&aL * yinvpow[aL.len()]), G_R, &bR, H_L, cL, H()); L_i.push((dL, G)); let L_i = prove_multiexp(&L_i); - L.push(L_i); + L.push(*L_i); let mut R_i = LR_statements(&(&aR * ypow[aR.len()]), G_L, &bL, H_R, cR, H()); R_i.push((dR, G)); let R_i = prove_multiexp(&R_i); - R.push(R_i); + R.push(*R_i); let w = hash_cache(&mut cache, &[L_i.compress().to_bytes(), R_i.compress().to_bytes()]); let winv = w.invert().unwrap(); @@ -158,20 +156,12 @@ impl PlusStruct { eta.zeroize(); alpha1.zeroize(); - let res = PlusStruct { - A: *A, - A1: *A1, - B: *B, - r1: *r1, - s1: *s1, - d1: *d1, - L: L.drain(..).map(|L| *L).collect(), - R: R.drain(..).map(|R| *R).collect(), - }; + let res = Self { A: *A, A1: *A1, B: *B, r1: *r1, s1: *s1, d1: *d1, L, R }; debug_assert!(res.verify(rng, &commitments_points)); res } + #[allow(clippy::many_single_char_names)] #[must_use] fn verify_core<ID: Copy + Zeroize, R: RngCore + CryptoRng>( &self, @@ -196,7 +186,7 @@ impl PlusStruct { } // Rebuild all challenges - let (mut cache, commitments) = hash_plus(commitments.iter().cloned()); + let (mut cache, commitments) = hash_plus(commitments.iter().copied()); let y = hash_cache(&mut cache, &[self.A.compress().to_bytes()]); let yinv = y.invert().unwrap(); let z = hash_to_scalar(&y.to_bytes()); @@ -220,8 +210,6 @@ impl PlusStruct { let A1 = normalize(&self.A1); let B = normalize(&self.B); - let mut commitments = commitments.iter().map(|c| c.mul_by_cofactor()).collect::<Vec<_>>(); - // Verify it let mut proof = Vec::with_capacity(logMN + 5 + (2 * (MN + logMN))); @@ -237,7 +225,7 @@ impl PlusStruct { let esq = e * e; let minus_esq = -esq; let commitment_weight = minus_esq * yMNy; - for (i, commitment) in commitments.drain(..).enumerate() { + for (i, commitment) in commitments.iter().map(EdwardsPoint::mul_by_cofactor).enumerate() { proof.push((commitment_weight * zpow[i], commitment)); } diff --git a/coins/monero/src/ringct/bulletproofs/scalar_vector.rs b/coins/monero/src/ringct/bulletproofs/scalar_vector.rs index 8fc9f18e..823de5f7 100644 --- a/coins/monero/src/ringct/bulletproofs/scalar_vector.rs +++ b/coins/monero/src/ringct/bulletproofs/scalar_vector.rs @@ -13,9 +13,9 @@ pub(crate) struct ScalarVector(pub(crate) Vec<Scalar>); macro_rules! math_op { ($Op: ident, $op: ident, $f: expr) => { impl $Op<Scalar> for ScalarVector { - type Output = ScalarVector; - fn $op(self, b: Scalar) -> ScalarVector { - ScalarVector(self.0.iter().map(|a| $f((a, &b))).collect()) + type Output = Self; + fn $op(self, b: Scalar) -> Self { + Self(self.0.iter().map(|a| $f((a, &b))).collect()) } } @@ -27,16 +27,16 @@ macro_rules! math_op { } impl $Op<ScalarVector> for ScalarVector { - type Output = ScalarVector; - fn $op(self, b: ScalarVector) -> ScalarVector { + type Output = Self; + fn $op(self, b: Self) -> Self { debug_assert_eq!(self.len(), b.len()); - ScalarVector(self.0.iter().zip(b.0.iter()).map($f).collect()) + Self(self.0.iter().zip(b.0.iter()).map($f).collect()) } } - impl $Op<&ScalarVector> for &ScalarVector { + impl $Op<Self> for &ScalarVector { type Output = ScalarVector; - fn $op(self, b: &ScalarVector) -> ScalarVector { + fn $op(self, b: Self) -> ScalarVector { debug_assert_eq!(self.len(), b.len()); ScalarVector(self.0.iter().zip(b.0.iter()).map($f).collect()) } @@ -48,11 +48,11 @@ math_op!(Sub, sub, |(a, b): (&Scalar, &Scalar)| *a - *b); math_op!(Mul, mul, |(a, b): (&Scalar, &Scalar)| *a * *b); impl ScalarVector { - pub(crate) fn new(len: usize) -> ScalarVector { - ScalarVector(vec![Scalar::ZERO; len]) + pub(crate) fn new(len: usize) -> Self { + Self(vec![Scalar::ZERO; len]) } - pub(crate) fn powers(x: Scalar, len: usize) -> ScalarVector { + pub(crate) fn powers(x: Scalar, len: usize) -> Self { debug_assert!(len != 0); let mut res = Vec::with_capacity(len); @@ -60,16 +60,16 @@ impl ScalarVector { for i in 1 .. len { res.push(res[i - 1] * x); } - ScalarVector(res) + Self(res) } - pub(crate) fn even_powers(x: Scalar, pow: usize) -> ScalarVector { + pub(crate) fn even_powers(x: Scalar, pow: usize) -> Self { debug_assert!(pow != 0); // Verify pow is a power of two debug_assert_eq!(((pow - 1) & pow), 0); let xsq = x * x; - let mut res = ScalarVector(Vec::with_capacity(pow / 2)); + let mut res = Self(Vec::with_capacity(pow / 2)); res.0.push(xsq); let mut prev = 2; @@ -89,9 +89,9 @@ impl ScalarVector { self.0.len() } - pub(crate) fn split(self) -> (ScalarVector, ScalarVector) { + pub(crate) fn split(self) -> (Self, Self) { let (l, r) = self.0.split_at(self.0.len() / 2); - (ScalarVector(l.to_vec()), ScalarVector(r.to_vec())) + (Self(l.to_vec()), Self(r.to_vec())) } } @@ -119,7 +119,7 @@ impl Mul<&[EdwardsPoint]> for &ScalarVector { type Output = EdwardsPoint; fn mul(self, b: &[EdwardsPoint]) -> EdwardsPoint { debug_assert_eq!(self.len(), b.len()); - multiexp(&self.0.iter().cloned().zip(b.iter().cloned()).collect::<Vec<_>>()) + multiexp(&self.0.iter().copied().zip(b.iter().copied()).collect::<Vec<_>>()) } } diff --git a/coins/monero/src/ringct/clsag/mod.rs b/coins/monero/src/ringct/clsag/mod.rs index 2bccc8ff..692efdf9 100644 --- a/coins/monero/src/ringct/clsag/mod.rs +++ b/coins/monero/src/ringct/clsag/mod.rs @@ -30,27 +30,31 @@ pub use multisig::{ClsagDetails, ClsagAddendum, ClsagMultisig}; #[cfg(feature = "multisig")] pub(crate) use multisig::add_key_image_share; -/// Errors returned when CLSAG signing fails. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "std", derive(thiserror::Error))] -pub enum ClsagError { - #[cfg_attr(feature = "std", error("internal error ({0})"))] - InternalError(&'static str), - #[cfg_attr(feature = "std", error("invalid ring"))] - InvalidRing, - #[cfg_attr(feature = "std", error("invalid ring member (member {0}, ring size {1})"))] - InvalidRingMember(u8, u8), - #[cfg_attr(feature = "std", error("invalid commitment"))] - InvalidCommitment, - #[cfg_attr(feature = "std", error("invalid key image"))] - InvalidImage, - #[cfg_attr(feature = "std", error("invalid D"))] - InvalidD, - #[cfg_attr(feature = "std", error("invalid s"))] - InvalidS, - #[cfg_attr(feature = "std", error("invalid c1"))] - InvalidC1, +#[allow(clippy::std_instead_of_core)] +mod clsag_error { + /// Errors returned when CLSAG signing fails. + #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[cfg_attr(feature = "std", derive(thiserror::Error))] + pub enum ClsagError { + #[cfg_attr(feature = "std", error("internal error ({0})"))] + InternalError(&'static str), + #[cfg_attr(feature = "std", error("invalid ring"))] + InvalidRing, + #[cfg_attr(feature = "std", error("invalid ring member (member {0}, ring size {1})"))] + InvalidRingMember(u8, u8), + #[cfg_attr(feature = "std", error("invalid commitment"))] + InvalidCommitment, + #[cfg_attr(feature = "std", error("invalid key image"))] + InvalidImage, + #[cfg_attr(feature = "std", error("invalid D"))] + InvalidD, + #[cfg_attr(feature = "std", error("invalid s"))] + InvalidS, + #[cfg_attr(feature = "std", error("invalid c1"))] + InvalidC1, + } } +pub use clsag_error::ClsagError; /// Input being signed for. #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] @@ -62,7 +66,7 @@ pub struct ClsagInput { } impl ClsagInput { - pub fn new(commitment: Commitment, decoys: Decoys) -> Result<ClsagInput, ClsagError> { + pub fn new(commitment: Commitment, decoys: Decoys) -> Result<Self, ClsagError> { let n = decoys.len(); if n > u8::MAX.into() { Err(ClsagError::InternalError("max ring size in this library is u8 max"))?; @@ -77,7 +81,7 @@ impl ClsagInput { Err(ClsagError::InvalidCommitment)?; } - Ok(ClsagInput { commitment, decoys }) + Ok(Self { commitment, decoys }) } } @@ -205,6 +209,7 @@ pub struct Clsag { impl Clsag { // Sign core is the extension of core as needed for signing, yet is shared between single signer // and multisig, hence why it's still core + #[allow(clippy::many_single_char_names)] pub(crate) fn sign_core<R: RngCore + CryptoRng>( rng: &mut R, I: &EdwardsPoint, @@ -213,7 +218,7 @@ impl Clsag { msg: &[u8; 32], A: EdwardsPoint, AH: EdwardsPoint, - ) -> (Clsag, EdwardsPoint, Scalar, Scalar) { + ) -> (Self, EdwardsPoint, Scalar, Scalar) { let r: usize = input.decoys.i.into(); let pseudo_out = Commitment::new(mask, input.commitment.amount).calculate(); @@ -228,18 +233,19 @@ impl Clsag { let ((D, p, c), c1) = core(&input.decoys.ring, I, &pseudo_out, msg, &D, &s, Mode::Sign(r, A, AH)); - (Clsag { D, s, c1 }, pseudo_out, p, c * z) + (Self { D, s, c1 }, pseudo_out, p, c * z) } /// Generate CLSAG signatures for the given inputs. /// inputs is of the form (private key, key image, input). /// sum_outputs is for the sum of the outputs' commitment masks. + #[must_use] pub fn sign<R: RngCore + CryptoRng>( rng: &mut R, mut inputs: Vec<(Zeroizing<Scalar>, EdwardsPoint, ClsagInput)>, sum_outputs: Scalar, msg: [u8; 32], - ) -> Vec<(Clsag, EdwardsPoint)> { + ) -> Vec<(Self, EdwardsPoint)> { let mut res = Vec::with_capacity(inputs.len()); let mut sum_pseudo_outs = Scalar::zero(); for i in 0 .. inputs.len() { @@ -251,7 +257,7 @@ impl Clsag { } let mut nonce = Zeroizing::new(random_scalar(rng)); - let (mut clsag, pseudo_out, p, c) = Clsag::sign_core( + let (mut clsag, pseudo_out, p, c) = Self::sign_core( rng, &inputs[i].1, &inputs[i].2, @@ -308,7 +314,7 @@ impl Clsag { Ok(()) } - pub(crate) fn fee_weight(ring_len: usize) -> usize { + pub(crate) const fn fee_weight(ring_len: usize) -> usize { (ring_len * 32) + 32 + 32 } @@ -318,7 +324,7 @@ impl Clsag { write_point(&self.D, w) } - pub fn read<R: Read>(decoys: usize, r: &mut R) -> io::Result<Clsag> { - Ok(Clsag { s: read_raw_vec(read_scalar, decoys, r)?, c1: read_scalar(r)?, D: read_point(r)? }) + pub fn read<R: Read>(decoys: usize, r: &mut R) -> io::Result<Self> { + Ok(Self { s: read_raw_vec(read_scalar, decoys, r)?, c1: read_scalar(r)?, D: read_point(r)? }) } } diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index 2dcacb56..302b759e 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/coins/monero/src/ringct/clsag/multisig.rs @@ -1,6 +1,9 @@ use core::{ops::Deref, fmt::Debug}; -use std_shims::io::{self, Read, Write}; -use std::sync::{Arc, RwLock}; +use std_shims::{ + sync::Arc, + io::{self, Read, Write}, +}; +use std::sync::RwLock; use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha20Rng; @@ -48,7 +51,7 @@ impl ClsagInput { // if in use transcript.append_message(b"member", [u8::try_from(i).expect("ring size exceeded 255")]); transcript.append_message(b"key", pair[0].compress().to_bytes()); - transcript.append_message(b"commitment", pair[1].compress().to_bytes()) + transcript.append_message(b"commitment", pair[1].compress().to_bytes()); } // Doesn't include the commitment's parts as the above ring + index includes the commitment @@ -65,8 +68,9 @@ pub struct ClsagDetails { } impl ClsagDetails { - pub fn new(input: ClsagInput, mask: Scalar) -> ClsagDetails { - ClsagDetails { input, mask } + #[must_use] + pub fn new(input: ClsagInput, mask: Scalar) -> Self { + Self { input, mask } } } @@ -112,12 +116,13 @@ pub struct ClsagMultisig { } impl ClsagMultisig { + #[must_use] pub fn new( transcript: RecommendedTranscript, output_key: EdwardsPoint, details: Arc<RwLock<Option<ClsagDetails>>>, - ) -> ClsagMultisig { - ClsagMultisig { + ) -> Self { + Self { transcript, H: hash_to_point(output_key), diff --git a/coins/monero/src/ringct/hash_to_point.rs b/coins/monero/src/ringct/hash_to_point.rs index 65bea9db..2170d187 100644 --- a/coins/monero/src/ringct/hash_to_point.rs +++ b/coins/monero/src/ringct/hash_to_point.rs @@ -3,6 +3,7 @@ use curve25519_dalek::edwards::EdwardsPoint; pub use monero_generators::{hash_to_point as raw_hash_to_point}; /// Monero's hash to point function, as named `ge_fromfe_frombytes_vartime`. +#[must_use] pub fn hash_to_point(key: EdwardsPoint) -> EdwardsPoint { raw_hash_to_point(key.compress().to_bytes()) } diff --git a/coins/monero/src/ringct/mlsag.rs b/coins/monero/src/ringct/mlsag.rs index 6999251f..31d6affb 100644 --- a/coins/monero/src/ringct/mlsag.rs +++ b/coins/monero/src/ringct/mlsag.rs @@ -19,20 +19,21 @@ pub struct Mlsag { impl Mlsag { pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { - for ss in self.ss.iter() { + for ss in &self.ss { write_raw_vec(write_scalar, ss, w)?; } write_scalar(&self.cc, w) } - pub fn read<R: Read>(mixins: usize, r: &mut R) -> io::Result<Mlsag> { - Ok(Mlsag { + pub fn read<R: Read>(mixins: usize, r: &mut R) -> io::Result<Self> { + Ok(Self { ss: (0 .. mixins).map(|_| read_array(read_scalar, r)).collect::<Result<_, _>>()?, cc: read_scalar(r)?, }) } #[cfg(feature = "experimental")] + #[must_use] pub fn verify( &self, msg: &[u8; 32], diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index eeadfa07..0dbff450 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -27,6 +27,7 @@ use crate::{ }; /// Generate a key image for a given key. Defined as `x * hash_to_point(xG)`. +#[must_use] pub fn generate_key_image(secret: &Zeroizing<Scalar>) -> EdwardsPoint { hash_to_point(&ED25519_BASEPOINT_TABLE * secret.deref()) * secret.deref() } @@ -38,21 +39,21 @@ pub enum EncryptedAmount { } impl EncryptedAmount { - pub fn read<R: Read>(compact: bool, r: &mut R) -> io::Result<EncryptedAmount> { - Ok(if !compact { - EncryptedAmount::Original { mask: read_bytes(r)?, amount: read_bytes(r)? } + pub fn read<R: Read>(compact: bool, r: &mut R) -> io::Result<Self> { + Ok(if compact { + Self::Compact { amount: read_bytes(r)? } } else { - EncryptedAmount::Compact { amount: read_bytes(r)? } + Self::Original { mask: read_bytes(r)?, amount: read_bytes(r)? } }) } pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { match self { - EncryptedAmount::Original { mask, amount } => { + Self::Original { mask, amount } => { w.write_all(mask)?; w.write_all(amount) } - EncryptedAmount::Compact { amount } => w.write_all(amount), + Self::Compact { amount } => w.write_all(amount), } } } @@ -77,40 +78,38 @@ pub enum RctType { } impl RctType { - pub fn to_byte(self) -> u8 { + #[must_use] + pub const fn to_byte(self) -> u8 { match self { - RctType::Null => 0, - RctType::MlsagAggregate => 1, - RctType::MlsagIndividual => 2, - RctType::Bulletproofs => 3, - RctType::BulletproofsCompactAmount => 4, - RctType::Clsag => 5, - RctType::BulletproofsPlus => 6, + Self::Null => 0, + Self::MlsagAggregate => 1, + Self::MlsagIndividual => 2, + Self::Bulletproofs => 3, + Self::BulletproofsCompactAmount => 4, + Self::Clsag => 5, + Self::BulletproofsPlus => 6, } } + #[must_use] pub fn from_byte(byte: u8) -> Option<Self> { Some(match byte { - 0 => RctType::Null, - 1 => RctType::MlsagAggregate, - 2 => RctType::MlsagIndividual, - 3 => RctType::Bulletproofs, - 4 => RctType::BulletproofsCompactAmount, - 5 => RctType::Clsag, - 6 => RctType::BulletproofsPlus, + 0 => Self::Null, + 1 => Self::MlsagAggregate, + 2 => Self::MlsagIndividual, + 3 => Self::Bulletproofs, + 4 => Self::BulletproofsCompactAmount, + 5 => Self::Clsag, + 6 => Self::BulletproofsPlus, _ => None?, }) } - pub fn compact_encrypted_amounts(&self) -> bool { + #[must_use] + pub const fn compact_encrypted_amounts(&self) -> bool { match self { - RctType::Null => false, - RctType::MlsagAggregate => false, - RctType::MlsagIndividual => false, - RctType::Bulletproofs => false, - RctType::BulletproofsCompactAmount => true, - RctType::Clsag => true, - RctType::BulletproofsPlus => true, + Self::Null | Self::MlsagAggregate | Self::MlsagIndividual | Self::Bulletproofs => false, + Self::BulletproofsCompactAmount | Self::Clsag | Self::BulletproofsPlus => true, } } } @@ -124,7 +123,7 @@ pub struct RctBase { } impl RctBase { - pub(crate) fn fee_weight(outputs: usize) -> usize { + pub(crate) const fn fee_weight(outputs: usize) -> usize { 1 + 8 + (outputs * (8 + 32)) } @@ -132,7 +131,12 @@ impl RctBase { w.write_all(&[rct_type.to_byte()])?; match rct_type { RctType::Null => Ok(()), - _ => { + RctType::MlsagAggregate | + RctType::MlsagIndividual | + RctType::Bulletproofs | + RctType::BulletproofsCompactAmount | + RctType::Clsag | + RctType::BulletproofsPlus => { write_varint(&self.fee, w)?; if rct_type == RctType::MlsagIndividual { write_raw_vec(write_point, &self.pseudo_outs, w)?; @@ -145,14 +149,12 @@ impl RctBase { } } - pub fn read<R: Read>(inputs: usize, outputs: usize, r: &mut R) -> io::Result<(RctBase, RctType)> { + pub fn read<R: Read>(inputs: usize, outputs: usize, r: &mut R) -> io::Result<(Self, RctType)> { let rct_type = RctType::from_byte(read_byte(r)?) .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid RCT type"))?; match rct_type { - RctType::Null => {} - RctType::MlsagAggregate => {} - RctType::MlsagIndividual => {} + RctType::Null | RctType::MlsagAggregate | RctType::MlsagIndividual => {} RctType::Bulletproofs | RctType::BulletproofsCompactAmount | RctType::Clsag | @@ -170,9 +172,9 @@ impl RctBase { Ok(( if rct_type == RctType::Null { - RctBase { fee: 0, pseudo_outs: vec![], encrypted_amounts: vec![], commitments: vec![] } + Self { fee: 0, pseudo_outs: vec![], encrypted_amounts: vec![], commitments: vec![] } } else { - RctBase { + Self { fee: read_varint(r)?, pseudo_outs: if rct_type == RctType::MlsagIndividual { read_raw_vec(read_point, inputs, r)? @@ -217,12 +219,12 @@ impl RctPrunable { pub fn write<W: Write>(&self, w: &mut W, rct_type: RctType) -> io::Result<()> { match self { - RctPrunable::Null => Ok(()), - RctPrunable::MlsagBorromean { borromean, mlsags } => { + Self::Null => Ok(()), + Self::MlsagBorromean { borromean, mlsags } => { write_raw_vec(BorromeanRange::write, borromean, w)?; write_raw_vec(Mlsag::write, mlsags, w) } - RctPrunable::MlsagBulletproofs { bulletproofs, mlsags, pseudo_outs } => { + Self::MlsagBulletproofs { bulletproofs, mlsags, pseudo_outs } => { if rct_type == RctType::Bulletproofs { w.write_all(&1u32.to_le_bytes())?; } else { @@ -233,7 +235,7 @@ impl RctPrunable { write_raw_vec(Mlsag::write, mlsags, w)?; write_raw_vec(write_point, pseudo_outs, w) } - RctPrunable::Clsag { bulletproofs, clsags, pseudo_outs } => { + Self::Clsag { bulletproofs, clsags, pseudo_outs } => { w.write_all(&[1])?; bulletproofs.write(w)?; @@ -243,6 +245,7 @@ impl RctPrunable { } } + #[must_use] pub fn serialize(&self, rct_type: RctType) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized, rct_type).unwrap(); @@ -254,31 +257,29 @@ impl RctPrunable { decoys: &[usize], outputs: usize, r: &mut R, - ) -> io::Result<RctPrunable> { + ) -> io::Result<Self> { Ok(match rct_type { - RctType::Null => RctPrunable::Null, - RctType::MlsagAggregate | RctType::MlsagIndividual => RctPrunable::MlsagBorromean { + RctType::Null => Self::Null, + RctType::MlsagAggregate | RctType::MlsagIndividual => Self::MlsagBorromean { borromean: read_raw_vec(BorromeanRange::read, outputs, r)?, mlsags: decoys.iter().map(|d| Mlsag::read(*d, r)).collect::<Result<_, _>>()?, }, - RctType::Bulletproofs | RctType::BulletproofsCompactAmount => { - RctPrunable::MlsagBulletproofs { - bulletproofs: { - if (if rct_type == RctType::Bulletproofs { - u64::from(read_u32(r)?) - } else { - read_varint(r)? - }) != 1 - { - Err(io::Error::new(io::ErrorKind::Other, "n bulletproofs instead of one"))?; - } - Bulletproofs::read(r)? - }, - mlsags: decoys.iter().map(|d| Mlsag::read(*d, r)).collect::<Result<_, _>>()?, - pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?, - } - } - RctType::Clsag | RctType::BulletproofsPlus => RctPrunable::Clsag { + RctType::Bulletproofs | RctType::BulletproofsCompactAmount => Self::MlsagBulletproofs { + bulletproofs: { + if (if rct_type == RctType::Bulletproofs { + u64::from(read_u32(r)?) + } else { + read_varint(r)? + }) != 1 + { + Err(io::Error::new(io::ErrorKind::Other, "n bulletproofs instead of one"))?; + } + Bulletproofs::read(r)? + }, + mlsags: decoys.iter().map(|d| Mlsag::read(*d, r)).collect::<Result<_, _>>()?, + pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?, + }, + RctType::Clsag | RctType::BulletproofsPlus => Self::Clsag { bulletproofs: { if read_varint(r)? != 1 { Err(io::Error::new(io::ErrorKind::Other, "n bulletproofs instead of one"))?; @@ -295,12 +296,10 @@ impl RctPrunable { pub(crate) fn signature_write<W: Write>(&self, w: &mut W) -> io::Result<()> { match self { - RctPrunable::Null => panic!("Serializing RctPrunable::Null for a signature"), - RctPrunable::MlsagBorromean { borromean, .. } => { - borromean.iter().try_for_each(|rs| rs.write(w)) - } - RctPrunable::MlsagBulletproofs { bulletproofs, .. } => bulletproofs.signature_write(w), - RctPrunable::Clsag { bulletproofs, .. } => bulletproofs.signature_write(w), + Self::Null => panic!("Serializing RctPrunable::Null for a signature"), + Self::MlsagBorromean { borromean, .. } => borromean.iter().try_for_each(|rs| rs.write(w)), + Self::MlsagBulletproofs { bulletproofs, .. } => bulletproofs.signature_write(w), + Self::Clsag { bulletproofs, .. } => bulletproofs.signature_write(w), } } } @@ -313,6 +312,7 @@ pub struct RctSignatures { impl RctSignatures { /// RctType for a given RctSignatures struct. + #[must_use] pub fn rct_type(&self) -> RctType { match &self.prunable { RctPrunable::Null => RctType::Null, @@ -376,14 +376,15 @@ impl RctSignatures { self.prunable.write(w, rct_type) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(decoys: Vec<usize>, outputs: usize, r: &mut R) -> io::Result<RctSignatures> { + pub fn read<R: Read>(decoys: Vec<usize>, outputs: usize, r: &mut R) -> io::Result<Self> { let base = RctBase::read(decoys.len(), outputs, r)?; - Ok(RctSignatures { base: base.0, prunable: RctPrunable::read(base.1, &decoys, outputs, r)? }) + Ok(Self { base: base.0, prunable: RctPrunable::read(base.1, &decoys, outputs, r)? }) } } diff --git a/coins/monero/src/rpc/http.rs b/coins/monero/src/rpc/http.rs index f2eb1328..39c371e2 100644 --- a/coins/monero/src/rpc/http.rs +++ b/coins/monero/src/rpc/http.rs @@ -17,7 +17,7 @@ impl HttpRpc { /// /// A daemon requiring authentication can be used via including the username and password in the /// URL. - pub fn new(mut url: String) -> Result<Rpc<HttpRpc>, RpcError> { + pub fn new(mut url: String) -> Result<Rpc<Self>, RpcError> { // Parse out the username and password let userpass = if url.contains('@') { let url_clone = url; @@ -47,7 +47,7 @@ impl HttpRpc { None }; - Ok(Rpc(HttpRpc { client: Client::new(), userpass, url })) + Ok(Rpc(Self { client: Client::new(), userpass, url })) } } diff --git a/coins/monero/src/rpc/mod.rs b/coins/monero/src/rpc/mod.rs index d0936271..048a7c66 100644 --- a/coins/monero/src/rpc/mod.rs +++ b/coins/monero/src/rpc/mod.rs @@ -28,7 +28,7 @@ mod http; pub use http::*; #[derive(Deserialize, Debug)] -pub struct EmptyResponse {} +pub struct EmptyResponse; #[derive(Deserialize, Debug)] pub struct JsonRpcResponse<T> { result: T, @@ -47,26 +47,31 @@ struct TransactionsResponse { txs: Vec<TransactionResponse>, } -#[derive(Clone, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "std", derive(thiserror::Error))] -pub enum RpcError { - #[cfg_attr(feature = "std", error("internal error ({0})"))] - InternalError(&'static str), - #[cfg_attr(feature = "std", error("connection error"))] - ConnectionError, - #[cfg_attr(feature = "std", error("invalid node"))] - InvalidNode, - #[cfg_attr(feature = "std", error("unsupported protocol version ({0})"))] - UnsupportedProtocol(usize), - #[cfg_attr(feature = "std", error("transactions not found"))] - TransactionsNotFound(Vec<[u8; 32]>), - #[cfg_attr(feature = "std", error("invalid point ({0})"))] - InvalidPoint(String), - #[cfg_attr(feature = "std", error("pruned transaction"))] - PrunedTransaction, - #[cfg_attr(feature = "std", error("invalid transaction ({0:?})"))] - InvalidTransaction([u8; 32]), +#[allow(clippy::std_instead_of_core)] +mod rpc_error { + use std_shims::{vec::Vec, string::String}; + #[derive(Clone, PartialEq, Eq, Debug)] + #[cfg_attr(feature = "std", derive(thiserror::Error))] + pub enum RpcError { + #[cfg_attr(feature = "std", error("internal error ({0})"))] + InternalError(&'static str), + #[cfg_attr(feature = "std", error("connection error"))] + ConnectionError, + #[cfg_attr(feature = "std", error("invalid node"))] + InvalidNode, + #[cfg_attr(feature = "std", error("unsupported protocol version ({0})"))] + UnsupportedProtocol(usize), + #[cfg_attr(feature = "std", error("transactions not found"))] + TransactionsNotFound(Vec<[u8; 32]>), + #[cfg_attr(feature = "std", error("invalid point ({0})"))] + InvalidPoint(String), + #[cfg_attr(feature = "std", error("pruned transaction"))] + PrunedTransaction, + #[cfg_attr(feature = "std", error("invalid transaction ({0:?})"))] + InvalidTransaction([u8; 32]), + } } +pub use rpc_error::RpcError; fn rpc_hex(value: &str) -> Result<Vec<u8>, RpcError> { hex::decode(value).map_err(|_| RpcError::InvalidNode) @@ -299,15 +304,15 @@ impl<R: RpcConnection> Rpc<R> { match self.get_block(self.get_block_hash(number).await?).await { Ok(block) => { // Make sure this is actually the block for this number - match block.miner_tx.prefix.inputs[0] { - Input::Gen(actual) => { - if usize::try_from(actual).unwrap() == number { + match block.miner_tx.prefix.inputs.get(0) { + Some(Input::Gen(actual)) => { + if usize::try_from(*actual).unwrap() == number { Ok(block) } else { Err(RpcError::InvalidNode) } } - _ => Err(RpcError::InvalidNode), + Some(Input::ToKey { .. }) | None => Err(RpcError::InvalidNode), } } e => e, diff --git a/coins/monero/src/serialize.rs b/coins/monero/src/serialize.rs index d3e983b0..29baaeed 100644 --- a/coins/monero/src/serialize.rs +++ b/coins/monero/src/serialize.rs @@ -125,7 +125,7 @@ pub(crate) fn read_point<R: Read>(r: &mut R) -> io::Result<EdwardsPoint> { pub(crate) fn read_torsion_free_point<R: Read>(r: &mut R) -> io::Result<EdwardsPoint> { read_point(r) .ok() - .filter(|point| point.is_torsion_free()) + .filter(EdwardsPoint::is_torsion_free) .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid point")) } diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index 70b1b5fe..04275230 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -1,6 +1,8 @@ use core::ops::Deref; #[cfg(feature = "multisig")] -use std::sync::{Arc, RwLock}; +use std_shims::sync::Arc; +#[cfg(feature = "multisig")] +use std::sync::RwLock; use zeroize::Zeroizing; use rand_core::{RngCore, OsRng}; @@ -45,13 +47,12 @@ fn clsag() { for i in 0 .. RING_LEN { let dest = Zeroizing::new(random_scalar(&mut OsRng)); let mask = random_scalar(&mut OsRng); - let amount; - if i == real { + let amount = if i == real { secrets = (dest.clone(), mask); - amount = AMOUNT; + AMOUNT } else { - amount = OsRng.next_u64(); - } + OsRng.next_u64() + }; ring .push([dest.deref() * &ED25519_BASEPOINT_TABLE, Commitment::new(mask, amount).calculate()]); } @@ -90,16 +91,15 @@ fn clsag_multisig() { for i in 0 .. RING_LEN { let dest; let mask; - let amount; - if i != u64::from(RING_INDEX) { - dest = &random_scalar(&mut OsRng) * &ED25519_BASEPOINT_TABLE; - mask = random_scalar(&mut OsRng); - amount = OsRng.next_u64(); - } else { + let amount = if i == u64::from(RING_INDEX) { dest = keys[&Participant::new(1).unwrap()].group_key().0; mask = randomness; - amount = AMOUNT; - } + AMOUNT + } else { + dest = &random_scalar(&mut OsRng) * &ED25519_BASEPOINT_TABLE; + mask = random_scalar(&mut OsRng); + OsRng.next_u64() + }; ring.push([dest, Commitment::new(mask, amount).calculate()]); } diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 882c8322..07d04ccf 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -25,7 +25,7 @@ pub enum Input { impl Input { // Worst-case predictive len - pub(crate) fn fee_weight(ring_len: usize) -> usize { + pub(crate) const fn fee_weight(ring_len: usize) -> usize { // Uses 1 byte for the VarInt amount due to amount being 0 // Uses 1 byte for the VarInt encoding of the length of the ring as well 1 + 1 + 1 + (8 * ring_len) + 32 @@ -33,12 +33,12 @@ impl Input { pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { match self { - Input::Gen(height) => { + Self::Gen(height) => { w.write_all(&[255])?; write_varint(height, w) } - Input::ToKey { amount, key_offsets, key_image } => { + Self::ToKey { amount, key_offsets, key_image } => { w.write_all(&[2])?; write_varint(&amount.unwrap_or(0), w)?; write_vec(write_varint, key_offsets, w)?; @@ -47,19 +47,20 @@ impl Input { } } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut res = vec![]; self.write(&mut res).unwrap(); res } - pub fn read<R: Read>(interpret_as_rct: bool, r: &mut R) -> io::Result<Input> { + pub fn read<R: Read>(interpret_as_rct: bool, r: &mut R) -> io::Result<Self> { Ok(match read_byte(r)? { - 255 => Input::Gen(read_varint(r)?), + 255 => Self::Gen(read_varint(r)?), 2 => { let amount = read_varint(r)?; let amount = if (amount == 0) && interpret_as_rct { None } else { Some(amount) }; - Input::ToKey { + Self::ToKey { amount, key_offsets: read_vec(read_varint, r)?, key_image: read_torsion_free_point(r)?, @@ -81,7 +82,7 @@ pub struct Output { } impl Output { - pub(crate) fn fee_weight() -> usize { + pub(crate) const fn fee_weight() -> usize { 1 + 1 + 32 + 1 } @@ -95,13 +96,14 @@ impl Output { Ok(()) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut res = Vec::with_capacity(8 + 1 + 32); self.write(&mut res).unwrap(); res } - pub fn read<R: Read>(interpret_as_rct: bool, r: &mut R) -> io::Result<Output> { + pub fn read<R: Read>(interpret_as_rct: bool, r: &mut R) -> io::Result<Self> { let amount = read_varint(r)?; let amount = if interpret_as_rct { if amount != 0 { @@ -121,7 +123,7 @@ impl Output { ))?, }; - Ok(Output { + Ok(Self { amount, key: CompressedEdwardsY(read_bytes(r)?), view_tag: if view_tag { Some(read_byte(r)?) } else { None }, @@ -137,22 +139,22 @@ pub enum Timelock { } impl Timelock { - fn from_raw(raw: u64) -> Timelock { + fn from_raw(raw: u64) -> Self { if raw == 0 { - Timelock::None + Self::None } else if raw < 500_000_000 { - Timelock::Block(usize::try_from(raw).unwrap()) + Self::Block(usize::try_from(raw).unwrap()) } else { - Timelock::Time(raw) + Self::Time(raw) } } fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { write_varint( &match self { - Timelock::None => 0, - Timelock::Block(block) => (*block).try_into().unwrap(), - Timelock::Time(time) => *time, + Self::None => 0, + Self::Block(block) => (*block).try_into().unwrap(), + Self::Time(time) => *time, }, w, ) @@ -162,9 +164,9 @@ impl Timelock { impl PartialOrd for Timelock { fn partial_cmp(&self, other: &Self) -> Option<Ordering> { match (self, other) { - (Timelock::None, _) => Some(Ordering::Less), - (Timelock::Block(a), Timelock::Block(b)) => a.partial_cmp(b), - (Timelock::Time(a), Timelock::Time(b)) => a.partial_cmp(b), + (Self::None, _) => Some(Ordering::Less), + (Self::Block(a), Self::Block(b)) => a.partial_cmp(b), + (Self::Time(a), Self::Time(b)) => a.partial_cmp(b), _ => None, } } @@ -200,13 +202,14 @@ impl TransactionPrefix { w.write_all(&self.extra) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut res = vec![]; self.write(&mut res).unwrap(); res } - pub fn read<R: Read>(r: &mut R) -> io::Result<TransactionPrefix> { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { let version = read_varint(r)?; // TODO: Create an enum out of version if (version == 0) || (version > 2) { @@ -221,7 +224,7 @@ impl TransactionPrefix { } let is_miner_tx = matches!(inputs[0], Input::Gen { .. }); - let mut prefix = TransactionPrefix { + let mut prefix = Self { version, timelock, inputs, @@ -232,6 +235,7 @@ impl TransactionPrefix { Ok(prefix) } + #[must_use] pub fn hash(&self) -> [u8; 32] { hash(&self.serialize()) } @@ -273,13 +277,14 @@ impl Transaction { } } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut res = Vec::with_capacity(2048); self.write(&mut res).unwrap(); res } - pub fn read<R: Read>(r: &mut R) -> io::Result<Transaction> { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { let prefix = TransactionPrefix::read(r)?; let mut signatures = vec![]; let mut rct_signatures = RctSignatures { @@ -328,9 +333,10 @@ impl Transaction { Err(io::Error::new(io::ErrorKind::Other, "Tried to deserialize unknown version"))?; } - Ok(Transaction { prefix, signatures, rct_signatures }) + Ok(Self { prefix, signatures, rct_signatures }) } + #[must_use] pub fn hash(&self) -> [u8; 32] { let mut buf = Vec::with_capacity(2048); if self.prefix.version == 1 { @@ -359,6 +365,7 @@ impl Transaction { } /// Calculate the hash of this transaction as needed for signing it. + #[must_use] pub fn signature_hash(&self) -> [u8; 32] { let mut buf = Vec::with_capacity(2048); let mut sig_hash = Vec::with_capacity(96); diff --git a/coins/monero/src/wallet/address.rs b/coins/monero/src/wallet/address.rs index bfd0096e..6c5093b5 100644 --- a/coins/monero/src/wallet/address.rs +++ b/coins/monero/src/wallet/address.rs @@ -32,18 +32,21 @@ pub struct SubaddressIndex { } impl SubaddressIndex { - pub const fn new(account: u32, address: u32) -> Option<SubaddressIndex> { + #[must_use] + pub const fn new(account: u32, address: u32) -> Option<Self> { if (account == 0) && (address == 0) { return None; } - Some(SubaddressIndex { account, address }) + Some(Self { account, address }) } - pub fn account(&self) -> u32 { + #[must_use] + pub const fn account(&self) -> u32 { self.account } - pub fn address(&self) -> u32 { + #[must_use] + pub const fn address(&self) -> u32 { self.address } } @@ -58,23 +61,25 @@ pub enum AddressSpec { } impl AddressType { - pub fn is_subaddress(&self) -> bool { - matches!(self, AddressType::Subaddress) || - matches!(self, AddressType::Featured { subaddress: true, .. }) + #[must_use] + pub const fn is_subaddress(&self) -> bool { + matches!(self, Self::Subaddress) || matches!(self, Self::Featured { subaddress: true, .. }) } - pub fn payment_id(&self) -> Option<[u8; 8]> { - if let AddressType::Integrated(id) = self { + #[must_use] + pub const fn payment_id(&self) -> Option<[u8; 8]> { + if let Self::Integrated(id) = self { Some(*id) - } else if let AddressType::Featured { payment_id, .. } = self { + } else if let Self::Featured { payment_id, .. } = self { *payment_id } else { None } } - pub fn is_guaranteed(&self) -> bool { - matches!(self, AddressType::Featured { guaranteed: true, .. }) + #[must_use] + pub const fn is_guaranteed(&self) -> bool { + matches!(self, Self::Featured { guaranteed: true, .. }) } } @@ -142,8 +147,9 @@ impl<B: AddressBytes> AddressMeta<B> { } /// Create an address's metadata. - pub fn new(network: Network, kind: AddressType) -> Self { - AddressMeta { _bytes: PhantomData, network, kind } + #[must_use] + pub const fn new(network: Network, kind: AddressType) -> Self { + Self { _bytes: PhantomData, network, kind } } // Returns an incomplete instantiation in the case of Integrated/Featured addresses @@ -160,7 +166,7 @@ impl<B: AddressBytes> AddressMeta<B> { } _ => None, } { - meta = Some(AddressMeta::new(network, kind)); + meta = Some(Self::new(network, kind)); break; } } @@ -168,15 +174,18 @@ impl<B: AddressBytes> AddressMeta<B> { meta.ok_or(AddressError::InvalidByte) } - pub fn is_subaddress(&self) -> bool { + #[must_use] + pub const fn is_subaddress(&self) -> bool { self.kind.is_subaddress() } - pub fn payment_id(&self) -> Option<[u8; 8]> { + #[must_use] + pub const fn payment_id(&self) -> Option<[u8; 8]> { self.kind.payment_id() } - pub fn is_guaranteed(&self) -> bool { + #[must_use] + pub const fn is_guaranteed(&self) -> bool { self.kind.is_guaranteed() } } @@ -216,8 +225,9 @@ impl<B: AddressBytes> ToString for Address<B> { } impl<B: AddressBytes> Address<B> { - pub fn new(meta: AddressMeta<B>, spend: EdwardsPoint, view: EdwardsPoint) -> Self { - Address { meta, spend, view } + #[must_use] + pub const fn new(meta: AddressMeta<B>, spend: EdwardsPoint, view: EdwardsPoint) -> Self { + Self { meta, spend, view } } pub fn from_str_raw(s: &str) -> Result<Self, AddressError> { @@ -267,7 +277,7 @@ impl<B: AddressBytes> Address<B> { id.copy_from_slice(&raw[(read - 8) .. read]); } - Ok(Address { meta, spend, view }) + Ok(Self { meta, spend, view }) } pub fn from_str(network: Network, s: &str) -> Result<Self, AddressError> { @@ -280,30 +290,35 @@ impl<B: AddressBytes> Address<B> { }) } - pub fn network(&self) -> Network { + #[must_use] + pub const fn network(&self) -> Network { self.meta.network } - pub fn is_subaddress(&self) -> bool { + #[must_use] + pub const fn is_subaddress(&self) -> bool { self.meta.is_subaddress() } - pub fn payment_id(&self) -> Option<[u8; 8]> { + #[must_use] + pub const fn payment_id(&self) -> Option<[u8; 8]> { self.meta.payment_id() } - pub fn is_guaranteed(&self) -> bool { + #[must_use] + pub const fn is_guaranteed(&self) -> bool { self.meta.is_guaranteed() } } /// Instantiation of the Address type with Monero's network bytes. pub type MoneroAddress = Address<MoneroAddressBytes>; -// Allow re-interpreting of an arbitrary address as a monero address so it can be used with the +// Allow re-interpreting of an arbitrary address as a Monero address so it can be used with the // rest of this library. Doesn't use From as it was conflicting with From<T> for T. impl MoneroAddress { - pub fn from<B: AddressBytes>(address: Address<B>) -> MoneroAddress { - MoneroAddress::new( + #[must_use] + pub const fn from<B: AddressBytes>(address: Address<B>) -> Self { + Self::new( AddressMeta::new(address.meta.network, address.meta.kind), address.spend, address.view, diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index 81eeedab..27046ad1 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -31,7 +31,7 @@ const TIP_APPLICATION: f64 = (LOCK_WINDOW * BLOCK_TIME) as f64; static DISTRIBUTION_CELL: OnceLock<Mutex<Vec<u64>>> = OnceLock::new(); #[allow(non_snake_case)] fn DISTRIBUTION() -> &'static Mutex<Vec<u64>> { - DISTRIBUTION_CELL.get_or_init(|| Mutex::new(Vec::with_capacity(3000000))) + DISTRIBUTION_CELL.get_or_init(|| Mutex::new(Vec::with_capacity(3_000_000))) } #[allow(clippy::too_many_arguments)] @@ -141,6 +141,7 @@ pub struct Decoys { } impl Decoys { + #[must_use] pub fn len(&self) -> usize { self.offsets.len() } @@ -152,7 +153,7 @@ impl Decoys { ring_len: usize, height: usize, inputs: &[SpendableOutput], - ) -> Result<Vec<Decoys>, RpcError> { + ) -> Result<Vec<Self>, RpcError> { #[cfg(not(feature = "std"))] let mut distribution = DISTRIBUTION().lock(); #[cfg(feature = "std")] @@ -265,7 +266,7 @@ impl Decoys { // members } - res.push(Decoys { + res.push(Self { // Binary searches for the real spend since we don't know where it sorted to i: u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(), offsets: offset(&ring.iter().map(|output| output.0).collect::<Vec<_>>()), diff --git a/coins/monero/src/wallet/extra.rs b/coins/monero/src/wallet/extra.rs index 2a694b6f..8cf051f3 100644 --- a/coins/monero/src/wallet/extra.rs +++ b/coins/monero/src/wallet/extra.rs @@ -30,14 +30,14 @@ pub enum PaymentId { } impl BitXor<[u8; 8]> for PaymentId { - type Output = PaymentId; + type Output = Self; - fn bitxor(self, bytes: [u8; 8]) -> PaymentId { + fn bitxor(self, bytes: [u8; 8]) -> Self { match self { // Don't perform the xor since this isn't intended to be encrypted with xor - PaymentId::Unencrypted(_) => self, - PaymentId::Encrypted(id) => { - PaymentId::Encrypted((u64::from_le_bytes(id) ^ u64::from_le_bytes(bytes)).to_le_bytes()) + Self::Unencrypted(_) => self, + Self::Encrypted(id) => { + Self::Encrypted((u64::from_le_bytes(id) ^ u64::from_le_bytes(bytes)).to_le_bytes()) } } } @@ -46,11 +46,11 @@ impl BitXor<[u8; 8]> for PaymentId { impl PaymentId { pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { match self { - PaymentId::Unencrypted(id) => { + Self::Unencrypted(id) => { w.write_all(&[PAYMENT_ID_MARKER])?; w.write_all(id)?; } - PaymentId::Encrypted(id) => { + Self::Encrypted(id) => { w.write_all(&[ENCRYPTED_PAYMENT_ID_MARKER])?; w.write_all(id)?; } @@ -58,10 +58,10 @@ impl PaymentId { Ok(()) } - pub fn read<R: Read>(r: &mut R) -> io::Result<PaymentId> { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { Ok(match read_byte(r)? { - 0 => PaymentId::Unencrypted(read_bytes(r)?), - 1 => PaymentId::Encrypted(read_bytes(r)?), + 0 => Self::Unencrypted(read_bytes(r)?), + 1 => Self::Encrypted(read_bytes(r)?), _ => Err(io::Error::new(io::ErrorKind::Other, "unknown payment ID type"))?, }) } @@ -79,20 +79,20 @@ pub enum ExtraField { impl ExtraField { pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> { match self { - ExtraField::PublicKey(key) => { + Self::PublicKey(key) => { w.write_all(&[1])?; w.write_all(&key.compress().to_bytes())?; } - ExtraField::Nonce(data) => { + Self::Nonce(data) => { w.write_all(&[2])?; write_vec(write_byte, data, w)?; } - ExtraField::MergeMining(height, merkle) => { + Self::MergeMining(height, merkle) => { w.write_all(&[3])?; write_varint(&u64::try_from(*height).unwrap(), w)?; w.write_all(merkle)?; } - ExtraField::PublicKeys(keys) => { + Self::PublicKeys(keys) => { w.write_all(&[4])?; write_vec(write_point, keys, w)?; } @@ -100,22 +100,22 @@ impl ExtraField { Ok(()) } - pub fn read<R: Read>(r: &mut R) -> io::Result<ExtraField> { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { Ok(match read_byte(r)? { - 1 => ExtraField::PublicKey(read_point(r)?), - 2 => ExtraField::Nonce({ + 1 => Self::PublicKey(read_point(r)?), + 2 => Self::Nonce({ let nonce = read_vec(read_byte, r)?; if nonce.len() > MAX_TX_EXTRA_NONCE_SIZE { Err(io::Error::new(io::ErrorKind::Other, "too long nonce"))?; } nonce }), - 3 => ExtraField::MergeMining( + 3 => Self::MergeMining( usize::try_from(read_varint(r)?) .map_err(|_| io::Error::new(io::ErrorKind::Other, "varint for height exceeds usize"))?, read_bytes(r)?, ), - 4 => ExtraField::PublicKeys(read_vec(read_point, r)?), + 4 => Self::PublicKeys(read_vec(read_point, r)?), _ => Err(io::Error::new(io::ErrorKind::Other, "unknown extra field"))?, }) } @@ -124,6 +124,7 @@ impl ExtraField { #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub struct Extra(Vec<ExtraField>); impl Extra { + #[must_use] pub fn keys(&self) -> Option<(EdwardsPoint, Option<Vec<EdwardsPoint>>)> { let mut key = None; let mut additional = None; @@ -131,7 +132,7 @@ impl Extra { match field.clone() { ExtraField::PublicKey(this_key) => key = key.or(Some(this_key)), ExtraField::PublicKeys(these_additional) => { - additional = additional.or(Some(these_additional)) + additional = additional.or(Some(these_additional)); } _ => (), } @@ -140,6 +141,7 @@ impl Extra { key.map(|key| (key, additional)) } + #[must_use] pub fn payment_id(&self) -> Option<PaymentId> { for field in &self.0 { if let ExtraField::Nonce(data) = field { @@ -149,6 +151,7 @@ impl Extra { None } + #[must_use] pub fn data(&self) -> Vec<Vec<u8>> { let mut res = vec![]; for field in &self.0 { @@ -161,8 +164,8 @@ impl Extra { res } - pub(crate) fn new(key: EdwardsPoint, additional: Vec<EdwardsPoint>) -> Extra { - let mut res = Extra(Vec::with_capacity(3)); + pub(crate) fn new(key: EdwardsPoint, additional: Vec<EdwardsPoint>) -> Self { + let mut res = Self(Vec::with_capacity(3)); res.push(ExtraField::PublicKey(key)); if !additional.is_empty() { res.push(ExtraField::PublicKeys(additional)); @@ -198,14 +201,15 @@ impl Extra { Ok(()) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut buf = vec![]; self.write(&mut buf).unwrap(); buf } - pub fn read<R: Read>(r: &mut R) -> io::Result<Extra> { - let mut res = Extra(vec![]); + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + let mut res = Self(vec![]); let mut field; while { field = ExtraField::read(r); diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index f953edd1..0d03f985 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -80,7 +80,7 @@ pub(crate) fn shared_key( // uniqueness || let shared_key = if let Some(uniqueness) = uniqueness { - [uniqueness.as_ref(), &output_derivation].concat().to_vec() + [uniqueness.as_ref(), &output_derivation].concat() } else { output_derivation }; @@ -141,14 +141,17 @@ pub struct ViewPair { } impl ViewPair { - pub fn new(spend: EdwardsPoint, view: Zeroizing<Scalar>) -> ViewPair { - ViewPair { spend, view } + #[must_use] + pub const fn new(spend: EdwardsPoint, view: Zeroizing<Scalar>) -> Self { + Self { spend, view } } - pub fn spend(&self) -> EdwardsPoint { + #[must_use] + pub const fn spend(&self) -> EdwardsPoint { self.spend } + #[must_use] pub fn view(&self) -> EdwardsPoint { self.view.deref() * &ED25519_BASEPOINT_TABLE } @@ -173,6 +176,7 @@ impl ViewPair { } /// Returns an address with the provided specification. + #[must_use] pub fn address(&self, network: Network, spec: AddressSpec) -> MoneroAddress { let mut spend = self.spend; let mut view: EdwardsPoint = self.view.deref() * &ED25519_BASEPOINT_TABLE; @@ -241,15 +245,20 @@ impl ZeroizeOnDrop for Scanner {} impl Scanner { /// Create a Scanner from a ViewPair. + /// /// burning_bug is a HashSet of used keys, intended to prevent key reuse which would burn funds. + /// /// When an output is successfully scanned, the output key MUST be saved to disk. + /// /// When a new scanner is created, ALL saved output keys must be passed in to be secure. + /// /// If None is passed, a modified shared key derivation is used which is immune to the burning /// bug (specifically the Guaranteed feature from Featured Addresses). - pub fn from_view(pair: ViewPair, burning_bug: Option<HashSet<CompressedEdwardsY>>) -> Scanner { + #[must_use] + pub fn from_view(pair: ViewPair, burning_bug: Option<HashSet<CompressedEdwardsY>>) -> Self { let mut subaddresses = HashMap::new(); subaddresses.insert(pair.spend.compress(), None); - Scanner { pair, subaddresses, burning_bug } + Self { pair, subaddresses, burning_bug } } /// Register a subaddress. diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 4816ed86..1ca2394b 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -32,14 +32,15 @@ impl AbsoluteId { w.write_all(&[self.o]) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = Vec::with_capacity(32 + 1); self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<AbsoluteId> { - Ok(AbsoluteId { tx: read_bytes(r)?, o: read_byte(r)? }) + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { tx: read_bytes(r)?, o: read_byte(r)? }) } } @@ -60,14 +61,15 @@ impl OutputData { w.write_all(&self.commitment.amount.to_le_bytes()) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = Vec::with_capacity(32 + 32 + 32 + 8); self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<OutputData> { - Ok(OutputData { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { key: read_point(r)?, key_offset: read_scalar(r)?, commitment: Commitment::new(read_scalar(r)?, read_u64(r)?), @@ -108,13 +110,14 @@ impl Metadata { Ok(()) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = Vec::with_capacity(1 + 8 + 1); self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<Metadata> { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { let subaddress = if read_byte(r)? == 1 { Some( SubaddressIndex::new(read_u32(r)?, read_u32(r)?) @@ -124,7 +127,7 @@ impl Metadata { None }; - Ok(Metadata { + Ok(Self { subaddress, payment_id: read_bytes(r)?, arbitrary_data: { @@ -148,18 +151,22 @@ pub struct ReceivedOutput { } impl ReceivedOutput { - pub fn key(&self) -> EdwardsPoint { + #[must_use] + pub const fn key(&self) -> EdwardsPoint { self.data.key } - pub fn key_offset(&self) -> Scalar { + #[must_use] + pub const fn key_offset(&self) -> Scalar { self.data.key_offset } + #[must_use] pub fn commitment(&self) -> Commitment { self.data.commitment.clone() } + #[must_use] pub fn arbitrary_data(&self) -> &[Vec<u8>] { &self.metadata.arbitrary_data } @@ -170,14 +177,15 @@ impl ReceivedOutput { self.metadata.write(w) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut serialized = vec![]; self.write(&mut serialized).unwrap(); serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<ReceivedOutput> { - Ok(ReceivedOutput { + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { absolute: AbsoluteId::read(r)?, data: OutputData::read(r)?, metadata: Metadata::read(r)?, @@ -209,24 +217,28 @@ impl SpendableOutput { pub async fn from<RPC: RpcConnection>( rpc: &Rpc<RPC>, output: ReceivedOutput, - ) -> Result<SpendableOutput, RpcError> { - let mut output = SpendableOutput { output, global_index: 0 }; + ) -> Result<Self, RpcError> { + let mut output = Self { output, global_index: 0 }; output.refresh_global_index(rpc).await?; Ok(output) } - pub fn key(&self) -> EdwardsPoint { + #[must_use] + pub const fn key(&self) -> EdwardsPoint { self.output.key() } - pub fn key_offset(&self) -> Scalar { + #[must_use] + pub const fn key_offset(&self) -> Scalar { self.output.key_offset() } + #[must_use] pub fn commitment(&self) -> Commitment { self.output.commitment() } + #[must_use] pub fn arbitrary_data(&self) -> &[Vec<u8>] { self.output.arbitrary_data() } @@ -242,8 +254,8 @@ impl SpendableOutput { serialized } - pub fn read<R: Read>(r: &mut R) -> io::Result<SpendableOutput> { - Ok(SpendableOutput { output: ReceivedOutput::read(r)?, global_index: read_u64(r)? }) + pub fn read<R: Read>(r: &mut R) -> io::Result<Self> { + Ok(Self { output: ReceivedOutput::read(r)?, global_index: read_u64(r)? }) } } @@ -258,11 +270,13 @@ impl<O: Clone + Zeroize> Drop for Timelocked<O> { impl<O: Clone + Zeroize> ZeroizeOnDrop for Timelocked<O> {} impl<O: Clone + Zeroize> Timelocked<O> { - pub fn timelock(&self) -> Timelock { + #[must_use] + pub const fn timelock(&self) -> Timelock { self.0 } /// Return the outputs if they're not timelocked, or an empty vector if they are. + #[must_use] pub fn not_locked(&self) -> Vec<O> { if self.0 == Timelock::None { return self.1.clone(); @@ -271,6 +285,7 @@ impl<O: Clone + Zeroize> Timelocked<O> { } /// Returns None if the Timelocks aren't comparable. Returns Some(vec![]) if none are unlocked. + #[must_use] pub fn unlocked(&self, timelock: Timelock) -> Option<Vec<O>> { // If the Timelocks are comparable, return the outputs if they're now unlocked if self.0 <= timelock { @@ -280,6 +295,7 @@ impl<O: Clone + Zeroize> Timelocked<O> { } } + #[must_use] pub fn ignore_timelock(&self) -> Vec<O> { self.1.clone() } @@ -293,16 +309,11 @@ impl Scanner { return Timelocked(tx.prefix.timelock, vec![]); } - let extra = Extra::read::<&[u8]>(&mut tx.prefix.extra.as_ref()); - let extra = if let Ok(extra) = extra { - extra - } else { + let Ok(extra) = Extra::read::<&[u8]>(&mut tx.prefix.extra.as_ref()) else { return Timelocked(tx.prefix.timelock, vec![]); }; - let (tx_key, additional) = if let Some((tx_key, additional)) = extra.keys() { - (tx_key, additional) - } else { + let Some((tx_key, additional)) = extra.keys() else { return Timelocked(tx.prefix.timelock, vec![]); }; @@ -324,17 +335,17 @@ impl Scanner { let output_key = output_key.unwrap(); for key in [Some(Some(&tx_key)), additional.as_ref().map(|additional| additional.get(o))] { - let key = if let Some(Some(key)) = key { - key - } else if let Some(None) = key { - // This is non-standard. There were additional keys, yet not one for this output - // https://github.com/monero-project/monero/ - // blob/04a1e2875d6e35e27bb21497988a6c822d319c28/ - // src/cryptonote_basic/cryptonote_format_utils.cpp#L1062 - // TODO: Should this return? Where does Monero set the trap handler for this exception? - continue; - } else { - break; + let Some(Some(key)) = key else { + if let Some(None) = key { + // This is non-standard. There were additional keys, yet not one for this output + // https://github.com/monero-project/monero/ + // blob/04a1e2875d6e35e27bb21497988a6c822d319c28/ + // src/cryptonote_basic/cryptonote_format_utils.cpp#L1062 + // TODO: Should this return? Where does Monero set the trap handler for this exception? + continue; + } else { + break; + } }; let (view_tag, shared_key, payment_id_xor) = shared_key( if self.burning_bug.is_none() { Some(uniqueness(&tx.prefix.inputs)) } else { None }, diff --git a/coins/monero/src/wallet/seed/classic.rs b/coins/monero/src/wallet/seed/classic.rs index 74e87c8b..7d34c5f3 100644 --- a/coins/monero/src/wallet/seed/classic.rs +++ b/coins/monero/src/wallet/seed/classic.rs @@ -33,8 +33,8 @@ struct WordList { } impl WordList { - fn new(word_list: Vec<&'static str>, prefix_length: usize) -> WordList { - let mut lang = WordList { + fn new(word_list: Vec<&'static str>, prefix_length: usize) -> Self { + let mut lang = Self { word_list, word_map: HashMap::new(), trimmed_word_map: HashMap::new(), @@ -74,10 +74,10 @@ fn LANGUAGES() -> &'static HashMap<Language, WordList> { #[cfg(test)] pub(crate) fn trim_by_lang(word: &str, lang: Language) -> String { - if lang != Language::EnglishOld { - word.chars().take(LANGUAGES()[&lang].unique_prefix_length).collect() - } else { + if lang == Language::EnglishOld { word.to_string() + } else { + word.chars().take(LANGUAGES()[&lang].unique_prefix_length).collect() } } @@ -239,11 +239,11 @@ pub(crate) fn seed_to_bytes(words: &str) -> Result<(Language, Zeroizing<[u8; 32] #[derive(Clone, PartialEq, Eq, Zeroize)] pub struct ClassicSeed(Zeroizing<String>); impl ClassicSeed { - pub(crate) fn new<R: RngCore + CryptoRng>(rng: &mut R, lang: Language) -> ClassicSeed { + pub(crate) fn new<R: RngCore + CryptoRng>(rng: &mut R, lang: Language) -> Self { key_to_seed(lang, Zeroizing::new(random_scalar(rng))) } - pub fn from_string(words: Zeroizing<String>) -> Result<ClassicSeed, SeedError> { + pub fn from_string(words: Zeroizing<String>) -> Result<Self, SeedError> { let (lang, entropy) = seed_to_bytes(&words)?; // Make sure this is a valid scalar @@ -257,7 +257,7 @@ impl ClassicSeed { Ok(Self::from_entropy(lang, entropy).unwrap()) } - pub fn from_entropy(lang: Language, entropy: Zeroizing<[u8; 32]>) -> Option<ClassicSeed> { + pub fn from_entropy(lang: Language, entropy: Zeroizing<[u8; 32]>) -> Option<Self> { Scalar::from_canonical_bytes(*entropy).map(|scalar| key_to_seed(lang, Zeroizing::new(scalar))) } diff --git a/coins/monero/src/wallet/seed/mod.rs b/coins/monero/src/wallet/seed/mod.rs index 3e4aca6c..9199118a 100644 --- a/coins/monero/src/wallet/seed/mod.rs +++ b/coins/monero/src/wallet/seed/mod.rs @@ -7,21 +7,25 @@ use rand_core::{RngCore, CryptoRng}; pub(crate) mod classic; use classic::{CLASSIC_SEED_LENGTH, CLASSIC_SEED_LENGTH_WITH_CHECKSUM, ClassicSeed}; -/// Error when decoding a seed. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "std", derive(thiserror::Error))] -pub enum SeedError { - #[cfg_attr(feature = "std", error("invalid number of words in seed"))] - InvalidSeedLength, - #[cfg_attr(feature = "std", error("unknown language"))] - UnknownLanguage, - #[cfg_attr(feature = "std", error("invalid checksum"))] - InvalidChecksum, - #[cfg_attr(feature = "std", error("english old seeds don't support checksums"))] - EnglishOldWithChecksum, - #[cfg_attr(feature = "std", error("invalid seed"))] - InvalidSeed, +#[allow(clippy::std_instead_of_core)] +mod seed_error { + /// Error when decoding a seed. + #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[cfg_attr(feature = "std", derive(thiserror::Error))] + pub enum SeedError { + #[cfg_attr(feature = "std", error("invalid number of words in seed"))] + InvalidSeedLength, + #[cfg_attr(feature = "std", error("unknown language"))] + UnknownLanguage, + #[cfg_attr(feature = "std", error("invalid checksum"))] + InvalidChecksum, + #[cfg_attr(feature = "std", error("english old seeds don't support checksums"))] + EnglishOldWithChecksum, + #[cfg_attr(feature = "std", error("invalid seed"))] + InvalidSeed, + } } +pub use seed_error::SeedError; #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] pub enum Language { @@ -50,43 +54,46 @@ pub enum Seed { impl fmt::Debug for Seed { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Seed::Classic(_) => f.debug_struct("Seed::Classic").finish_non_exhaustive(), + Self::Classic(_) => f.debug_struct("Seed::Classic").finish_non_exhaustive(), } } } impl Seed { /// Create a new seed. - pub fn new<R: RngCore + CryptoRng>(rng: &mut R, lang: Language) -> Seed { - Seed::Classic(ClassicSeed::new(rng, lang)) + #[must_use] + pub fn new<R: RngCore + CryptoRng>(rng: &mut R, lang: Language) -> Self { + Self::Classic(ClassicSeed::new(rng, lang)) } /// Parse a seed from a String. - pub fn from_string(words: Zeroizing<String>) -> Result<Seed, SeedError> { + pub fn from_string(words: Zeroizing<String>) -> Result<Self, SeedError> { match words.split_whitespace().count() { CLASSIC_SEED_LENGTH | CLASSIC_SEED_LENGTH_WITH_CHECKSUM => { - ClassicSeed::from_string(words).map(Seed::Classic) + ClassicSeed::from_string(words).map(Self::Classic) } _ => Err(SeedError::InvalidSeedLength)?, } } /// Create a Seed from entropy. - pub fn from_entropy(lang: Language, entropy: Zeroizing<[u8; 32]>) -> Option<Seed> { - ClassicSeed::from_entropy(lang, entropy).map(Seed::Classic) + #[must_use] + pub fn from_entropy(lang: Language, entropy: Zeroizing<[u8; 32]>) -> Option<Self> { + ClassicSeed::from_entropy(lang, entropy).map(Self::Classic) } /// Convert a seed to a String. pub fn to_string(&self) -> Zeroizing<String> { match self { - Seed::Classic(seed) => seed.to_string(), + Self::Classic(seed) => seed.to_string(), } } /// Return the entropy for this seed. + #[must_use] pub fn entropy(&self) -> Zeroizing<[u8; 32]> { match self { - Seed::Classic(seed) => seed.entropy(), + Self::Classic(seed) => seed.entropy(), } } } diff --git a/coins/monero/src/wallet/send/builder.rs b/coins/monero/src/wallet/send/builder.rs index 7c575750..32075336 100644 --- a/coins/monero/src/wallet/send/builder.rs +++ b/coins/monero/src/wallet/send/builder.rs @@ -25,7 +25,7 @@ struct SignableTransactionBuilderInternal { impl SignableTransactionBuilderInternal { // Takes in the change address so users don't miss that they have to manually set one // If they don't, all leftover funds will become part of the fee - fn new(protocol: Protocol, fee: Fee, change_address: Option<Change>) -> Self { + const fn new(protocol: Protocol, fee: Fee, change_address: Option<Change>) -> Self { Self { protocol, fee, @@ -81,15 +81,17 @@ impl Eq for SignableTransactionBuilder {} impl Zeroize for SignableTransactionBuilder { fn zeroize(&mut self) { - self.0.write().unwrap().zeroize() + self.0.write().unwrap().zeroize(); } } +#[allow(clippy::return_self_not_must_use)] impl SignableTransactionBuilder { fn shallow_copy(&self) -> Self { Self(self.0.clone()) } + #[must_use] pub fn new(protocol: Protocol, fee: Fee, change_address: Option<Change>) -> Self { Self(Arc::new(RwLock::new(SignableTransactionBuilderInternal::new( protocol, diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index 7eb1fcf2..87afe196 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -72,7 +72,7 @@ impl SendOutput { output: (usize, (MoneroAddress, u64)), ecdh: EdwardsPoint, R: EdwardsPoint, - ) -> (SendOutput, Option<[u8; 8]>) { + ) -> (Self, Option<[u8; 8]>) { let o = output.0; let output = output.1; @@ -80,7 +80,7 @@ impl SendOutput { shared_key(Some(unique).filter(|_| output.0.is_guaranteed()), ecdh, o); ( - SendOutput { + Self { R, view_tag, dest: ((&shared_key * &ED25519_BASEPOINT_TABLE) + output.0.spend), @@ -98,16 +98,16 @@ impl SendOutput { r: &Zeroizing<Scalar>, unique: [u8; 32], output: (usize, (MoneroAddress, u64)), - ) -> (SendOutput, Option<[u8; 8]>) { + ) -> (Self, Option<[u8; 8]>) { let address = output.1 .0; - SendOutput::internal( + Self::internal( unique, output, r.deref() * address.view, - if !address.is_subaddress() { - r.deref() * &ED25519_BASEPOINT_TABLE - } else { + if address.is_subaddress() { r.deref() * address.spend + } else { + r.deref() * &ED25519_BASEPOINT_TABLE }, ) } @@ -116,8 +116,8 @@ impl SendOutput { ecdh: EdwardsPoint, unique: [u8; 32], output: (usize, (MoneroAddress, u64)), - ) -> (SendOutput, Option<[u8; 8]>) { - SendOutput::internal(unique, output, ecdh, ED25519_BASEPOINT_POINT) + ) -> (Self, Option<[u8; 8]>) { + Self::internal(unique, output, ecdh, ED25519_BASEPOINT_POINT) } } @@ -211,6 +211,7 @@ pub struct Fee { } impl Fee { + #[must_use] pub fn calculate(&self, weight: usize) -> u64 { ((((self.per_weight * u64::try_from(weight).unwrap()) - 1) / self.mask) + 1) * self.mask } @@ -261,8 +262,9 @@ impl fmt::Debug for Change { impl Change { /// Create a change output specification from a ViewPair, as needed to maintain privacy. - pub fn new(view: &ViewPair, guaranteed: bool) -> Change { - Change { + #[must_use] + pub fn new(view: &ViewPair, guaranteed: bool) -> Self { + Self { address: view.address( Network::Mainnet, if !guaranteed { @@ -275,10 +277,12 @@ impl Change { } } - /// Create a fingerprintable change output specification which will harm privacy. Only use this - /// if you know what you're doing. - pub fn fingerprintable(address: MoneroAddress) -> Change { - Change { address, view: None } + /// Create a fingerprintable change output specification which will harm privacy. + /// + /// Only use this if you know what you're doing. + #[must_use] + pub const fn fingerprintable(address: MoneroAddress) -> Self { + Self { address, view: None } } } @@ -296,17 +300,17 @@ impl SignableTransaction { protocol: Protocol, r_seed: Option<Zeroizing<[u8; 32]>>, inputs: Vec<SpendableOutput>, - mut payments: Vec<(MoneroAddress, u64)>, + payments: Vec<(MoneroAddress, u64)>, change_address: Option<Change>, data: Vec<Vec<u8>>, fee_rate: Fee, - ) -> Result<SignableTransaction, TransactionError> { + ) -> Result<Self, TransactionError> { // Make sure there's only one payment ID let mut has_payment_id = { let mut payment_ids = 0; let mut count = |addr: MoneroAddress| { if addr.payment_id().is_some() { - payment_ids += 1 + payment_ids += 1; } }; for payment in &payments { @@ -382,15 +386,16 @@ impl SignableTransaction { Err(TransactionError::TooManyOutputs)?; } - let mut payments = payments.drain(..).map(InternalPayment::Payment).collect::<Vec<_>>(); + let mut payments = payments.into_iter().map(InternalPayment::Payment).collect::<Vec<_>>(); if let Some(change) = change_address { payments.push(InternalPayment::Change(change, in_amount - out_amount)); } - Ok(SignableTransaction { protocol, r_seed, inputs, payments, data, fee }) + Ok(Self { protocol, r_seed, inputs, payments, data, fee }) } - pub fn fee(&self) -> u64 { + #[must_use] + pub const fn fee(&self) -> u64 { self.fee } @@ -444,11 +449,8 @@ impl SignableTransaction { 0; // We need additional keys if we have any subaddresses - let mut additional = subaddresses; - // Unless the above change view key path is taken - if (payments.len() == 2) && has_change_view { - additional = false; - } + // UNLESS there's only two payments and we have the view-key for the change output + let additional = if (payments.len() == 2) && has_change_view { false } else { subaddresses }; let modified_change_ecdh = subaddresses && (!additional); // If we're using the aR rewrite, update tx_public_key from rG to rB @@ -562,11 +564,13 @@ impl SignableTransaction { } /// Returns the eventuality of this transaction. + /// /// The eventuality is defined as the TX extra/outputs this transaction will create, if signed /// with the specified seed. This eventuality can be compared to on-chain transactions to see /// if the transaction has already been signed and published. + #[must_use] pub fn eventuality(&self) -> Option<Eventuality> { - let inputs = self.inputs.iter().map(|input| input.key()).collect::<Vec<_>>(); + let inputs = self.inputs.iter().map(SpendableOutput::key).collect::<Vec<_>>(); let (tx_key, additional, outputs, id) = Self::prepare_payments( self.r_seed.as_ref()?, &inputs, @@ -606,7 +610,7 @@ impl SignableTransaction { let (tx_key, additional, outputs, id) = Self::prepare_payments( &r_seed, - &self.inputs.iter().map(|input| input.key()).collect::<Vec<_>>(), + &self.inputs.iter().map(SpendableOutput::key).collect::<Vec<_>>(), &mut self.payments, uniqueness, ); @@ -656,7 +660,7 @@ impl SignableTransaction { fee, encrypted_amounts, pseudo_outs: vec![], - commitments: commitments.iter().map(|commitment| commitment.calculate()).collect(), + commitments: commitments.iter().map(Commitment::calculate).collect(), }, prunable: RctPrunable::Clsag { bulletproofs: bp, clsags: vec![], pseudo_outs: vec![] }, }, @@ -704,7 +708,9 @@ impl SignableTransaction { clsags.append(&mut clsag_pairs.iter().map(|clsag| clsag.0.clone()).collect::<Vec<_>>()); pseudo_outs.append(&mut clsag_pairs.iter().map(|clsag| clsag.1).collect::<Vec<_>>()); } - _ => unreachable!("attempted to sign a TX which wasn't CLSAG"), + RctPrunable::MlsagBorromean { .. } | RctPrunable::MlsagBulletproofs { .. } => { + unreachable!("attempted to sign a TX which wasn't CLSAG") + } } Ok(tx) } @@ -713,13 +719,19 @@ impl SignableTransaction { impl Eventuality { /// Enables building a HashMap of Extra -> Eventuality for efficiently checking if an on-chain /// transaction may match this eventuality. + /// /// This extra is cryptographically bound to: /// 1) A specific set of inputs (via their output key) /// 2) A specific seed for the ephemeral keys + /// + /// This extra may be used with a transaction with a distinct set of inputs, yet no honest + /// transaction which doesn't satisfy this Eventuality will contain it. + #[must_use] pub fn extra(&self) -> &[u8] { &self.extra } + #[must_use] pub fn matches(&self, tx: &Transaction) -> bool { if self.payments.len() != tx.prefix.outputs.len() { return false; @@ -752,9 +764,10 @@ impl Eventuality { } // TODO: Remove this when the following for loop is updated - if !rct_type.compact_encrypted_amounts() { - panic!("created an Eventuality for a very old RctType we don't support proving for"); - } + assert!( + rct_type.compact_encrypted_amounts(), + "created an Eventuality for a very old RctType we don't support proving for" + ); for (o, (expected, actual)) in outputs.iter().zip(tx.prefix.outputs.iter()).enumerate() { // Verify the output, commitment, and encrypted amount. @@ -804,18 +817,19 @@ impl Eventuality { write_vec(write_byte, &self.extra, w) } + #[must_use] pub fn serialize(&self) -> Vec<u8> { let mut buf = Vec::with_capacity(128); self.write(&mut buf).unwrap(); buf } - pub fn read<R: io::Read>(r: &mut R) -> io::Result<Eventuality> { + pub fn read<R: io::Read>(r: &mut R) -> io::Result<Self> { fn read_address<R: io::Read>(r: &mut R) -> io::Result<MoneroAddress> { String::from_utf8(read_vec(read_byte, r)?) .ok() .and_then(|str| MoneroAddress::from_str_raw(&str).ok()) - .ok_or(io::Error::new(io::ErrorKind::Other, "invalid address")) + .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid address")) } fn read_payment<R: io::Read>(r: &mut R) -> io::Result<InternalPayment> { @@ -836,7 +850,7 @@ impl Eventuality { }) } - Ok(Eventuality { + Ok(Self { protocol: Protocol::read(r)?, r_seed: Zeroizing::new(read_bytes::<_, 32>(r)?), inputs: read_vec(read_point, r)?, diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index 3694d507..d5ed4ad7 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -1,9 +1,10 @@ use std_shims::{ + sync::Arc, vec::Vec, io::{self, Read}, collections::HashMap, }; -use std::sync::{Arc, RwLock}; +use std::sync::RwLock; use zeroize::Zeroizing; @@ -267,14 +268,15 @@ impl SignMachine<Transaction> for TransactionSignMachine { mut commitments: HashMap<Participant, Self::Preprocess>, msg: &[u8], ) -> Result<(TransactionSignatureMachine, Self::SignatureShare), FrostError> { - if !msg.is_empty() { - panic!("message was passed to the TransactionMachine when it generates its own"); - } + assert!( + msg.is_empty(), + "message was passed to the TransactionMachine when it generates its own" + ); // Find out who's included // This may not be a valid set of signers yet the algorithm machine will error if it's not commitments.remove(&self.i); // Remove, if it was included for some reason - let mut included = commitments.keys().cloned().collect::<Vec<_>>(); + let mut included = commitments.keys().copied().collect::<Vec<_>>(); included.push(self.i); included.sort_unstable(); @@ -325,7 +327,7 @@ impl SignMachine<Transaction> for TransactionSignMachine { // Remove our preprocess which shouldn't be here. It was just the easiest way to implement the // above - for map in commitments.iter_mut() { + for map in &mut commitments { map.remove(&self.i); } diff --git a/common/std-shims/src/sync.rs b/common/std-shims/src/sync.rs index 0685f4b4..eeb911c9 100644 --- a/common/std-shims/src/sync.rs +++ b/common/std-shims/src/sync.rs @@ -1,4 +1,5 @@ pub use core::sync::*; +pub use alloc::sync::*; mod mutex_shim { #[cfg(feature = "std")]