Clippy&fmt

benches/sighash_bench.rs

@@ -1,6 +1,6 @@
-use std::rc::Rc;
+use std::{hint::black_box, rc::Rc, slice::from_ref};
 
-use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion};
+use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
 
 use bitcoin::{hashes::Hash, Network, ScriptBuf};
 use key_manager::{key_manager::KeyManager, key_type::BitcoinKeyType};
@@ -30,7 +30,7 @@ fn build_single_chain(n_txs: usize, km: &Rc<KeyManager>) -> Protocol {
     let mut protocol = Protocol::new("bench_single");
     let builder = ProtocolBuilder {};
 
-    let ext_output = OutputType::taproot(value, &key, &[script.clone()]).unwrap();
+    let ext_output = OutputType::taproot(value, &key, from_ref(&script)).unwrap();
     builder
         .add_external_connection(
             &mut protocol,
@@ -55,7 +55,7 @@ fn build_single_chain(n_txs: usize, km: &Rc<KeyManager>) -> Protocol {
                 &format!("tx_{i}"),
                 value,
                 &key,
-                &[script.clone()],
+                from_ref(&script),
                 &spend_all,
                 &format!("tx_{}", i + 1),
                 &tr_sighash,

src/builder/builder.rs

@@ -146,14 +146,14 @@ impl ProtocolBuilder {
 
         for (idx, speedup_data) in speedups_data.iter().enumerate() {
             let tx_name = &format!("tx_to_speedup_{idx}");
-            protocol.add_external_transaction(&tx_name)?;
+            protocol.add_external_transaction(tx_name)?;
 
             if let Some(utxo) = &speedup_data.utxo {
-                protocol.add_unknown_outputs(&tx_name, utxo.vout)?;
+                protocol.add_unknown_outputs(tx_name, utxo.vout)?;
                 let external_output = OutputType::segwit_key(utxo.amount, &utxo.pub_key)?;
                 protocol.add_connection(
                     &format!("speedup_{idx}"),
-                    &tx_name,
+                    tx_name,
                     external_output.into(),
                     "cpfp",
                     InputSpec::Auto(SighashType::ecdsa_all(), SpendMode::Segwit),
@@ -162,10 +162,10 @@ impl ProtocolBuilder {
                 )?;
             } else {
                 let partial_utxo = speedup_data.partial_utxo.as_ref().unwrap();
-                protocol.add_unknown_outputs(&tx_name, partial_utxo.1)?;
+                protocol.add_unknown_outputs(tx_name, partial_utxo.1)?;
                 protocol.add_connection(
                     &format!("speedup_{idx}"),
-                    &tx_name,
+                    tx_name,
                     speedup_data.output_type.as_ref().unwrap().clone().into(),
                     "cpfp",
                     InputSpec::Auto(
@@ -198,9 +198,9 @@ impl ProtocolBuilder {
         }
 
         let total_funding: u64 = funding_inputs.iter().map(|u| u.amount).sum();
-        let change_amount = total_funding.checked_sub(speedup_fee).ok_or_else(|| {
-            ProtocolBuilderError::InsufficientFunds(total_funding, speedup_fee)
-        })?;
+        let change_amount = total_funding
+            .checked_sub(speedup_fee)
+            .ok_or_else(|| ProtocolBuilderError::InsufficientFunds(total_funding, speedup_fee))?;
         let change_output = OutputType::segwit_key(change_amount, change_address)?;
         protocol.add_transaction_output("cpfp", &change_output)?;
 

src/builder/mod.rs

@@ -1,3 +1,4 @@
+#[allow(clippy::module_inception)]
 mod builder;
 mod check_params;
 mod protocol;

src/builder/protocol.rs

@@ -39,11 +39,11 @@ impl Protocol {
     }
 
     pub fn load(name: &str, storage: Rc<Storage>) -> Result<Option<Self>, ProtocolBuilderError> {
-        Ok(storage.get(&name, None)?)
+        Ok(storage.get(name, None)?)
     }
 
     pub fn save(&self, storage: Rc<Storage>) -> Result<(), ProtocolBuilderError> {
-        storage.set(&self.name, &self, None)?;
+        storage.set(&self.name, self, None)?;
         Ok(())
     }
 
@@ -137,6 +137,7 @@ impl Protocol {
         Ok(transaction.output.len() as u32)
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn add_connection(
         &mut self,
         connection_name: &str,
@@ -545,17 +546,16 @@ impl Protocol {
             .graph
             .get_output(transaction_name, output_index as usize)?
         {
-            match output_type {
-                OutputType::Taproot { leaves, .. } => return Ok((output_type, &leaves)),
-                _ => {}
+            if let OutputType::Taproot { leaves, .. } = output_type {
+                return Ok((output_type, leaves));
             }
         }
-        return Err(ProtocolBuilderError::CannotGetScriptForOutputType(
+        Err(ProtocolBuilderError::CannotGetScriptForOutputType(
             transaction_name.to_string(),
             output_index,
             0,
             "Output not found".to_string(),
-        ));
+        ))
     }
 
     pub fn get_script_to_spend(

src/cli.rs

@@ -1,4 +1,4 @@
-use std::{path::PathBuf, rc::Rc};
+use std::{path::PathBuf, rc::Rc, slice::from_ref};
 
 use anyhow::{Ok, Result};
 
@@ -395,7 +395,7 @@ impl Cli {
             from,
             value,
             &internal_key,
-            &[script.clone()],
+            from_ref(&script),
             &SpendMode::All {
                 key_path_sign: SignMode::Single,
             },
@@ -491,8 +491,8 @@ impl Cli {
             to,
             value,
             &public_key,
-            &[script.clone()],
-            &[script.clone()],
+            from_ref(&script),
+            from_ref(&script),
             &SpendMode::All {
                 key_path_sign: SignMode::Single,
             },

src/config.rs

@@ -26,6 +26,8 @@ pub struct Config {
 
 impl Config {
     pub fn new(config: Option<String>) -> Result<Config, ConfigError> {
-        Ok(bitvmx_settings::settings::load_config_file::<Config>(config)?)
+        Ok(bitvmx_settings::settings::load_config_file::<Config>(
+            config,
+        )?)
     }
 }

src/errors.rs

@@ -249,7 +249,7 @@ pub enum ProtocolBuilderError {
     DustOutput {
         value: Amount,
         dust_limit: Amount,
-        output_type: OutputType,
+        output_type: Box<OutputType>,
     },
 
     #[error("Uncompressed public key error: {0}")]

src/graph/estimate.rs

@@ -16,7 +16,7 @@ pub fn script_num_size(n: u32) -> usize {
 
     // Minimal number of bytes needed to hold n
     let bits = 32 - n.leading_zeros();
-    let len = ((bits + 7) / 8) as usize; // ceil(bits/8)
+    let len = bits.div_ceil(8) as usize; // ceil(bits/8)
 
     // Extra byte needed if the top bit of the most significant byte
     // is set, since CScriptNum encodes sign in that bit.
@@ -143,7 +143,7 @@ fn vbytes_from_parts(
 ) -> u64 {
     let s = stripped_size as u64;
     let w = total_witness_bytes_including_marker_flag as u64;
-    s + ((w + 3) / 4) // ceil(w/4)
+    s + w.div_ceil(4) // ceil(w/4)
 }
 
 /// Estimate the minimum relay fee (in sats) for `tx` at `feerate_sat_per_vb`,

src/graph/graph.rs

@@ -526,7 +526,7 @@ impl TransactionGraph {
                         Ok(acc
                             + parent.outputs[output_index]
                                 .get_value()
-                                .ok_or_else(|| GraphError::AmountTypeValueExpected)?
+                                .ok_or(GraphError::AmountTypeValueExpected)?
                                 .to_sat())
                     })?;
 
@@ -550,14 +550,12 @@ impl TransactionGraph {
 
             let total_subtracted = total_transaction_amount
                 .checked_add(minimum_relay_fee)
-                .ok_or_else(|| {
+                .ok_or({
                     GraphError::OverflowError(total_transaction_amount, minimum_relay_fee)
                 })?;
-            let recover_amount = total_parents_amount
-                .checked_sub(total_subtracted)
-                .ok_or_else(|| {
-                    GraphError::InsufficientFunds(total_parents_amount, total_subtracted)
-                })?;
+            let recover_amount = total_parents_amount.checked_sub(total_subtracted).ok_or(
+                GraphError::InsufficientFunds(total_parents_amount, total_subtracted),
+            )?;
             let recover_amount = Amount::from_sat(recover_amount);
 
             // Update OutputType value
@@ -671,7 +669,7 @@ impl TransactionGraph {
             } else {
                 let value = output
                     .get_value()
-                    .ok_or_else(|| GraphError::AmountTypeValueExpected)?;
+                    .ok_or(GraphError::AmountTypeValueExpected)?;
                 remaining = remaining.saturating_sub(value.to_sat()); // Saturating at 0
                 let current_amount = amounts.get(&parent_key).cloned().unwrap_or_default();
                 if value.to_sat() > current_amount.to_sat() {
@@ -726,7 +724,7 @@ impl TransactionGraph {
                     Ok::<u64, GraphError>(
                         i.output_type()?
                             .get_value()
-                            .ok_or_else(|| GraphError::AmountTypeValueExpected)?
+                            .ok_or(GraphError::AmountTypeValueExpected)?
                             .to_sat(),
                     )
                 })

src/graph/mod.rs

@@ -1,2 +1,3 @@
 pub mod estimate;
+#[allow(clippy::module_inception)]
 pub mod graph;

src/scripts.rs

@@ -78,14 +78,17 @@ impl KeyType {
             )),
         }
     }
+}
 
-    pub fn to_string(&self) -> String {
-        match self {
-            KeyType::WinternitzKey { .. } => "Winternitz".to_string(),
-            KeyType::LamportKey { .. } => "Lamport".to_string(),
-            KeyType::EcdsaKey { .. } => "Ecdsa".to_string(),
-            KeyType::XOnlyKey { .. } => "XOnly".to_string(),
-        }
+impl Display for KeyType {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let name = match self {
+            KeyType::WinternitzKey { .. } => "Winternitz",
+            KeyType::LamportKey { .. } => "Lamport",
+            KeyType::EcdsaKey => "Ecdsa",
+            KeyType::XOnlyKey => "XOnly",
+        };
+        write!(f, "{}", name)
     }
 }
 
@@ -425,7 +428,7 @@ pub fn ots_checklamport(public_key: &LamportPublicKey, keep_message: bool) -> Sc
         stack.define(1, format!("signature_{}", i).as_str());
     }
 
-    lamport_checksig(&mut stack, &public_key, keep_message);
+    lamport_checksig(&mut stack, public_key, keep_message);
 
     stack.get_script()
 }
@@ -514,7 +517,7 @@ pub fn verify_winternitz_signature_timelock(
     )?;
 
     protocol_script.add_stack_item(StackItem::new_schnorr_sig(true));
-    protocol_script.add_stack_item(StackItem::new_winternitz_sig(&public_key));
+    protocol_script.add_stack_item(StackItem::new_winternitz_sig(public_key));
 
     Ok(protocol_script)
 }
@@ -900,13 +903,13 @@ pub fn build_taproot_spend_info(
     let total_slots = 1 << (min_depth + 1); // 2^(min_depth + 1)
     let nodes_at_min_depth = total_slots - scripts_count;
     // Add leaves at minimum depth
-    for i in 0..nodes_at_min_depth {
-        tr_builder = tr_builder.add_leaf(min_depth, leaves[i].get_script().clone())?;
+    for leaf in &leaves[..nodes_at_min_depth] {
+        tr_builder = tr_builder.add_leaf(min_depth, leaf.get_script().clone())?;
     }
 
     // Add remaining leaves at minimum depth + 1
-    for i in nodes_at_min_depth..scripts_count {
-        tr_builder = tr_builder.add_leaf(min_depth + 1, leaves[i].get_script().clone())?;
+    for leaf in &leaves[nodes_at_min_depth..scripts_count] {
+        tr_builder = tr_builder.add_leaf(min_depth + 1, leaf.get_script().clone())?;
     }
 
     tr_builder
@@ -1461,7 +1464,7 @@ mod tests {
         let mut stack = StackTracker::new();
 
         for hash in signature.to_hashes().iter() {
-            stack.hexstr(&hex::encode(&hash));
+            stack.hexstr(&hex::encode(hash));
         }
 
         lamport_checksig(&mut stack, &public_key, false);
@@ -1487,7 +1490,7 @@ mod tests {
         let mut stack = StackTracker::new();
 
         for hash in signature.to_hashes().iter() {
-            stack.hexstr(&hex::encode(&hash));
+            stack.hexstr(&hex::encode(hash));
         }
 
         lamport_checksig(&mut stack, &public_key, true);

src/tests/autovalues_fees_test.rs

@@ -3,7 +3,12 @@ mod tests {
     use bitcoin::{hashes::Hash, ScriptBuf};
 
     use crate::{
-        builder::{Protocol, ProtocolBuilder}, errors::ProtocolBuilderError, graph::estimate::estimate_min_relay_fee, scripts::{ProtocolScript, SignMode, StackItem, verify_lamport_signatures}, tests::utils::TestContext, types::{
+        builder::{Protocol, ProtocolBuilder},
+        errors::ProtocolBuilderError,
+        graph::estimate::estimate_min_relay_fee,
+        scripts::{verify_lamport_signatures, ProtocolScript, SignMode, StackItem},
+        tests::utils::TestContext,
+        types::{
             connection::{InputSpec, OutputSpec},
             input::{InputArgs, SpendMode},
             output::OutputType,
@@ -66,7 +71,7 @@ mod tests {
 
         // Verify parent has output
         assert!(
-            parent_tx.output.len() > 0,
+            !parent_tx.output.is_empty(),
             "Parent transaction should have at least one output"
         );
 
@@ -77,7 +82,7 @@ mod tests {
 
         // Verify child consumed parent output by having an input
         assert!(
-            child_tx.input.len() > 0,
+            !child_tx.input.is_empty(),
             "Child should have at least one input from parent"
         );