Meaningful changes from aggressive-clippy

I do want to enable a few specific lints, yet aggressive-clippy as a whole
isn't worthwhile.

coins/monero/src/lib.rs

@@ -140,7 +140,7 @@ pub struct Commitment {
 }
 
 impl Commitment {
-  /// The zero commitment, defined as a mask of 1 (as to not be the identity) and a 0 amount.
+  /// A commitment to zero, defined with a mask of 1 (as to not be the identity).
   pub fn zero() -> Commitment {
     Commitment { mask: Scalar::one(), amount: 0 }
   }

coins/monero/src/merkle.rs

@@ -2,7 +2,7 @@ use std_shims::vec::Vec;
 
 use crate::hash;
 
-pub fn merkle_root(root: [u8; 32], leafs: &[[u8; 32]]) -> [u8; 32] {
+pub(crate) fn merkle_root(root: [u8; 32], leafs: &[[u8; 32]]) -> [u8; 32] {
   match leafs.len() {
     0 => root,
     1 => hash(&[root, leafs[0]].concat()),

coins/monero/src/ringct/borromean.rs

@@ -35,10 +35,10 @@ impl BorromeanSignatures {
   }
 
   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)

coins/monero/src/ringct/bulletproofs/core.rs

@@ -50,7 +50,7 @@ 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
@@ -118,9 +118,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

coins/monero/src/ringct/bulletproofs/original.rs

@@ -190,7 +190,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());

coins/monero/src/ringct/bulletproofs/plus.rs

@@ -196,7 +196,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 +220,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 +235,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));
     }
 

coins/monero/src/ringct/bulletproofs/scalar_vector.rs

@@ -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<_>>())
   }
 }
 

coins/monero/src/ringct/mlsag.rs

@@ -19,7 +19,7 @@ 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)

coins/monero/src/rpc/mod.rs

@@ -299,9 +299,9 @@ 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)

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"))
 }
 

coins/monero/src/transaction.rs

@@ -345,14 +345,13 @@ impl Transaction {
       hashes.extend(hash(&buf));
       buf.clear();
 
-      match self.rct_signatures.prunable {
-        RctPrunable::Null => buf.resize(32, 0),
+      hashes.extend(&match self.rct_signatures.prunable {
+        RctPrunable::Null => [0; 32],
         _ => {
           self.rct_signatures.prunable.write(&mut buf, self.rct_signatures.rct_type()).unwrap();
-          buf = hash(&buf).to_vec();
+          hash(&buf)
         }
-      }
-      hashes.extend(&buf);
+      });
 
       hash(&hashes)
     }

coins/monero/src/wallet/mod.rs

@@ -241,9 +241,13 @@ 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 {

coins/monero/src/wallet/scan.rs

@@ -263,6 +263,7 @@ impl<O: Clone + Zeroize> Timelocked<O> {
   }
 
   /// 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 +272,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 +282,7 @@ impl<O: Clone + Zeroize> Timelocked<O> {
     }
   }
 
+  #[must_use]
   pub fn ignore_timelock(&self) -> Vec<O> {
     self.1.clone()
   }
@@ -293,16 +296,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![]);
     };
 
@@ -453,7 +451,7 @@ impl Scanner {
     };
 
     let mut res = vec![];
-    for tx in txs.drain(..) {
+    for tx in txs {
       if let Some(timelock) = map(self.scan_transaction(&tx), index) {
         res.push(timelock);
       }

coins/monero/src/wallet/send/mod.rs

@@ -296,7 +296,7 @@ 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,
@@ -382,7 +382,7 @@ 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));
     }
@@ -562,11 +562,12 @@ 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.
   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 +607,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 +657,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![] },
         },
@@ -713,13 +714,18 @@ 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 in a transaction with a distinct set of inputs, yet no honest
+  /// transaction which doesn't satisfy this Eventuality will contain it.
   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 +758,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.
@@ -815,7 +822,7 @@ impl Eventuality {
       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> {

coins/monero/src/wallet/send/multisig.rs

@@ -274,7 +274,7 @@ impl SignMachine<Transaction> for TransactionSignMachine {
     // 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 +325,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);
     }
 
@@ -430,7 +430,9 @@ impl SignatureMachine<Transaction> for TransactionSignatureMachine {
           pseudo_outs.push(pseudo_out);
         }
       }
-      _ => unreachable!("attempted to sign a multisig TX which wasn't CLSAG"),
+      RctPrunable::MlsagBorromean { .. } | RctPrunable::MlsagBulletproofs { .. } => {
+        unreachable!("attempted to sign a multisig TX which wasn't CLSAG")
+      }
     }
     Ok(tx)
   }

coins/monero/tests/add_data.rs

@@ -12,8 +12,7 @@ test!(
       let arbitrary_data = vec![b'\0'; MAX_ARBITRARY_DATA_SIZE - 1];
 
       // make sure we can add to tx
-      let result = builder.add_data(arbitrary_data.clone());
-      assert!(result.is_ok());
+      builder.add_data(arbitrary_data.clone()).unwrap();
 
       builder.add_payment(addr, 5);
       (builder.build().unwrap(), (arbitrary_data,))
@@ -37,8 +36,7 @@ test!(
 
       // Add data multiple times
       for data in &data {
-        let result = builder.add_data(data.clone());
-        assert!(result.is_ok());
+        builder.add_data(data.clone()).unwrap();
       }
 
       builder.add_payment(addr, 5);
@@ -65,7 +63,7 @@ test!(
       // Reduce data size and retry. The data will now be 255 bytes long (including the added
       // marker), exactly
       data.pop();
-      assert!(builder.add_data(data.clone()).is_ok());
+      builder.add_data(data.clone()).unwrap();
 
       builder.add_payment(addr, 5);
       (builder.build().unwrap(), data)

coins/monero/tests/send.rs

@@ -37,8 +37,8 @@ test!(
     },
   ),
   (
-    |rpc, mut builder: Builder, addr, mut outputs: Vec<ReceivedOutput>| async move {
-      for output in outputs.drain(..) {
+    |rpc, mut builder: Builder, addr, outputs: Vec<ReceivedOutput>| async move {
+      for output in outputs {
         builder.add_input(SpendableOutput::from(&rpc, output).await.unwrap());
       }
       builder.add_payment(addr, 6);

coins/monero/tests/wallet2_compatibility.rs

@@ -70,7 +70,7 @@ async fn from_wallet_rpc_to_self(spec: AddressSpec) {
 
   // make an addr
   let (_, view_pair, _) = runner::random_address();
-  let addr = Address::from_str(&view_pair.address(Network::Mainnet, spec).to_string()[..]).unwrap();
+  let addr = Address::from_str(&view_pair.address(Network::Mainnet, spec).to_string()).unwrap();
 
   // refresh & make a tx
   wallet_rpc.refresh(None).await.unwrap();
@@ -103,7 +103,9 @@ async fn from_wallet_rpc_to_self(spec: AddressSpec) {
       assert_eq!(output.metadata.payment_id, payment_id);
       assert_eq!(output.metadata.subaddress, None);
     }
-    _ => assert_eq!(output.metadata.subaddress, None),
+    AddressSpec::Standard | AddressSpec::Featured { .. } => {
+      assert_eq!(output.metadata.subaddress, None)
+    }
   }
   assert_eq!(output.commitment().amount, 1000000000000);
 }
@@ -228,7 +230,7 @@ test!(
       for _ in 0 .. 2 {
         // Subtract 1 since we prefix data with 127
         let data = vec![b'a'; MAX_TX_EXTRA_NONCE_SIZE - 1];
-        assert!(builder.add_data(data).is_ok());
+        builder.add_data(data).unwrap();
       }
 
       (builder.build().unwrap(), (wallet_rpc,))