From f538333246476f71028e29e227d671a1f2f15047 Mon Sep 17 00:00:00 2001 From: Khushal Date: Tue, 23 Dec 2025 11:32:23 +0530 Subject: [PATCH] refactor: minimize cloning in wallet list_* methods --- examples/wallet_basic.rs | 8 ++--- src/bin/taker.rs | 10 ++++-- src/taker/api.rs | 4 +-- src/taker/api2.rs | 2 +- src/utill.rs | 5 +-- src/wallet/api.rs | 69 ++++++++++++++++--------------------- src/wallet/spend.rs | 19 +++++----- tests/abort2_case1.rs | 13 +++++-- tests/standard_swap.rs | 10 ++++-- tests/test_framework/mod.rs | 4 +-- tests/utxo_behavior.rs | 12 +++---- 11 files changed, 84 insertions(+), 72 deletions(-) diff --git a/examples/wallet_basic.rs b/examples/wallet_basic.rs index 89fc45fee..703982c35 100644 --- a/examples/wallet_basic.rs +++ b/examples/wallet_basic.rs @@ -156,10 +156,10 @@ fn main() -> Result<(), Box> { // Categorize UTXOs by type println!("\nUTXO Categories:"); - let regular_utxos = wallet.list_descriptor_utxo_spend_info(); - let swap_utxos = wallet.list_swap_coin_utxo_spend_info(); - let fidelity_utxos = wallet.list_fidelity_spend_info(); - let swept_utxos = wallet.list_swept_incoming_swap_utxos(); + let regular_utxos: Vec<_> = wallet.list_descriptor_utxo_spend_info().into_iter().map(|(a, b)| (a.clone(), b.clone())).collect(); + let swap_utxos: Vec<_> = wallet.list_swap_coin_utxo_spend_info().into_iter().map(|(a, b)| (a.clone(), b.clone())).collect(); + let fidelity_utxos: Vec<_> = wallet.list_fidelity_spend_info().into_iter().map(|(a, b)| (a.clone(), b.clone())).collect(); + let swept_utxos: Vec<_> = wallet.list_swept_incoming_swap_utxos().into_iter().map(|(a, b)| (a.clone(), b.clone())).collect(); println!(" Regular UTXOs: {}", regular_utxos.len()); println!(" Swap UTXOs: {}", swap_utxos.len()); println!(" Fidelity UTXOs: {}", fidelity_utxos.len()); diff --git a/src/bin/taker.rs b/src/bin/taker.rs index 3fc6ae275..c152cd071 100644 --- a/src/bin/taker.rs +++ b/src/bin/taker.rs @@ -305,7 +305,10 @@ fn main() -> Result<(), TakerError> { let manually_selected_outpoints = if cfg!(not(feature = "integration-test")) { Some( coinswap::utill::interactive_select( - taker.get_wallet().list_all_utxo_spend_info(), + taker.get_wallet().list_all_utxo_spend_info() + .into_iter() + .map(|(utxo, info)| (utxo.clone(), info.clone())) + .collect::>(), amount, )? .iter() @@ -394,7 +397,10 @@ fn main() -> Result<(), TakerError> { Some( coinswap::utill::interactive_select( - taker.get_wallet().list_all_utxo_spend_info(), + taker.get_wallet().list_all_utxo_spend_info() + .into_iter() + .map(|(utxo, info)| (utxo.clone(), info.clone())) + .collect::>(), target_amount, )? .iter() diff --git a/src/taker/api.rs b/src/taker/api.rs index 669ae3f40..97714b950 100644 --- a/src/taker/api.rs +++ b/src/taker/api.rs @@ -356,7 +356,7 @@ impl Taker { swap_params: SwapParams, ) -> Result, TakerError> { let swap_start_time = std::time::Instant::now(); - let initial_utxoset = self.wallet.list_all_utxo(); + let initial_utxoset = self.wallet.list_all_utxo().into_iter().cloned().collect::>(); self.ongoing_swap_state.swap_params = swap_params.clone(); // Check if we have enough balance - try regular first, then swap @@ -576,7 +576,7 @@ impl Taker { .wallet .list_descriptor_utxo_spend_info() .into_iter() - .map(|(utxo, _)| utxo) + .map(|(utxo, _)| utxo.clone()) .collect::>(); let initial_outpoints = initial_utxos diff --git a/src/taker/api2.rs b/src/taker/api2.rs index c1732563b..4e41d30be 100644 --- a/src/taker/api2.rs +++ b/src/taker/api2.rs @@ -344,7 +344,7 @@ impl Taker { swap_params: SwapParams, ) -> Result, TakerError> { let swap_start_time = std::time::Instant::now(); - let initial_utxoset = self.wallet.list_all_utxo(); + let initial_utxoset = self.wallet.list_all_utxo().into_iter().cloned().collect::>(); let available = self.wallet.get_balances()?.spendable; diff --git a/src/utill.rs b/src/utill.rs index 48537565a..f51363b0c 100644 --- a/src/utill.rs +++ b/src/utill.rs @@ -415,10 +415,11 @@ pub struct UTXO { impl UTXO { /// Creates an UTXO from detailed internal utxo data - pub fn from_utxo_data(data: (ListUnspentResultEntry, UTXOSpendInfo)) -> Self { + pub fn from_utxo_data(data: (&ListUnspentResultEntry, &UTXOSpendInfo)) -> Self { let addr = data .0 .address + .clone() .expect("address always expected") .assume_checked() .to_string(); @@ -1089,7 +1090,7 @@ pub fn interactive_select( let total_selected: Amount = selected_utxo.iter().map(|(u, _)| u.amount).sum(); println!("Total selected amount: {} BTC", total_selected.to_btc()); - Ok(selected_utxo) + Ok(selected_utxo.into_iter().map(|(a, b)| (a.clone(), b.clone())).collect()) } #[cfg(test)] diff --git a/src/wallet/api.rs b/src/wallet/api.rs index 783f0edea..4ef2bfa8d 100644 --- a/src/wallet/api.rs +++ b/src/wallet/api.rs @@ -971,36 +971,35 @@ impl Wallet { } /// Returns a list of all UTXOs tracked by the wallet. Including fidelity, live_contracts and swap coins. - pub fn list_all_utxo(&self) -> Vec { + pub fn list_all_utxo(&self) -> Vec<&ListUnspentResultEntry> { self.list_all_utxo_spend_info() - .iter() - .map(|(utxo, _)| utxo.clone()) + .into_iter() + .map(|(utxo, _)| utxo) .collect() } /// Returns a list all utxos with their spend info tracked by the wallet. /// Optionally takes in an Utxo list to reduce RPC calls. If None is given, the /// full list of utxo is fetched from core rpc. - pub fn list_all_utxo_spend_info(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_all_utxo_spend_info(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let processed_utxos = self .store .utxo_cache .values() - .map(|(utxo, spend_info)| (utxo.clone(), spend_info.clone())) + .map(|(utxo, spend_info)| (utxo, spend_info)) .collect(); processed_utxos } /// Lists live contract UTXOs along with their Spend info. - pub fn list_live_contract_spend_info(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_live_contract_spend_info(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| { matches!(x.1, UTXOSpendInfo::HashlockContract { .. }) || matches!(x.1, UTXOSpendInfo::TimelockContract { .. }) }) - .cloned() .collect(); filtered_utxos } @@ -1008,62 +1007,57 @@ impl Wallet { /// Lists live timelock contract UTXOs along with their Spend info. pub fn list_live_timelock_contract_spend_info( &self, - ) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + ) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| matches!(x.1, UTXOSpendInfo::TimelockContract { .. })) - .cloned() .collect(); filtered_utxos } /// Lists all live hashlock contract UTXOs along with their Spend info. pub fn list_live_hashlock_contract_spend_info( &self, - ) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + ) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| matches!(x.1, UTXOSpendInfo::HashlockContract { .. })) - .cloned() .collect(); filtered_utxos } /// Lists fidelity UTXOs along with their Spend info. - pub fn list_fidelity_spend_info(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_fidelity_spend_info(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| matches!(x.1, UTXOSpendInfo::FidelityBondCoin { .. })) - .cloned() .collect(); filtered_utxos } /// Lists descriptor UTXOs along with their Spend info. - pub fn list_descriptor_utxo_spend_info(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_descriptor_utxo_spend_info(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| matches!(x.1, UTXOSpendInfo::SeedCoin { .. })) - .cloned() .collect(); filtered_utxos } /// Lists swap coin UTXOs along with their Spend info. - pub fn list_swap_coin_utxo_spend_info(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_swap_coin_utxo_spend_info(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| { matches!( x.1, UTXOSpendInfo::IncomingSwapCoin { .. } | UTXOSpendInfo::OutgoingSwapCoin { .. } ) }) - .cloned() .collect(); filtered_utxos } @@ -1071,22 +1065,20 @@ impl Wallet { /// Lists all incoming swapcoin UTXOs along with their Spend info. pub fn list_incoming_swap_coin_utxo_spend_info( &self, - ) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + ) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|x| matches!(x.1, UTXOSpendInfo::IncomingSwapCoin { .. })) - .cloned() .collect(); filtered_utxos } /// Lists all swept incoming swapcoin UTXOs along with their Spend info. - pub fn list_swept_incoming_swap_utxos(&self) -> Vec<(ListUnspentResultEntry, UTXOSpendInfo)> { + pub fn list_swept_incoming_swap_utxos(&self) -> Vec<(&ListUnspentResultEntry, &UTXOSpendInfo)> { let all_valid_utxo = self.list_all_utxo_spend_info(); let filtered_utxos: Vec<_> = all_valid_utxo - .iter() + .into_iter() .filter(|(_, spend_info)| matches!(spend_info, UTXOSpendInfo::SweptCoin { .. })) - .cloned() .collect(); filtered_utxos } @@ -1223,7 +1215,7 @@ impl Wallet { if utxo.descriptor.is_none() { continue; } - let descriptor = utxo.descriptor.expect("its not none"); + let descriptor = utxo.clone().descriptor.expect("its not none"); let ret = get_hd_path_from_descriptor(&descriptor); if ret.is_none() { continue; @@ -1653,7 +1645,7 @@ impl Wallet { const CHANGE_OUTPUT_WEIGHT: u64 = Amount::SIZE as u64 + 1 + P2WPKH_SPK_SIZE as u64; // ~31 bytes let locked_utxos = self.list_lock_unspent()?; - let filter_locked = |utxos: Vec<(ListUnspentResultEntry, UTXOSpendInfo)>| { + fn filter_locked<'a>(utxos: Vec<(&'a ListUnspentResultEntry, &'a UTXOSpendInfo)>, locked_utxos: &[OutPoint]) -> Vec<(&'a ListUnspentResultEntry, &'a UTXOSpendInfo)> { utxos .into_iter() .filter(|(utxo, _)| { @@ -1661,11 +1653,11 @@ impl Wallet { !locked_utxos.contains(&outpoint) }) .collect::>() - }; + } // Get regular and swap UTXOs separately - let available_regular_utxos = filter_locked(self.list_descriptor_utxo_spend_info()); - let available_swap_utxos = filter_locked(self.list_swept_incoming_swap_utxos()); + let available_regular_utxos = filter_locked(self.list_descriptor_utxo_spend_info(), &locked_utxos); + let available_swap_utxos = filter_locked(self.list_swept_incoming_swap_utxos(), &locked_utxos); // Assert that no non-spendable UTXOs are included after filtering assert!( @@ -1817,9 +1809,8 @@ impl Wallet { grouped_addresses.insert( 0, manual_unspents - .clone() - .into_iter() - .cloned() + .iter() + .map(|(utxo, spend_info)| ((*utxo).clone(), (*spend_info).clone())) .collect::>(), ); @@ -1979,7 +1970,7 @@ impl Wallet { for utxo in seed_coin_utxo { if utxo.0.txid == txid && utxo.0.vout == vout { - return Ok(Some(utxo.1)); + return Ok(Some(utxo.1.clone())); } } @@ -2238,7 +2229,7 @@ impl Wallet { internal_address ); let sweep_tx = self.spend_coins( - &[(utxo.clone(), spend_info)], + &[(utxo.clone(), spend_info.clone())], Destination::Sweep(internal_address.clone()), feerate, )?; diff --git a/src/wallet/spend.rs b/src/wallet/spend.rs index a452664ad..3f2208b1d 100644 --- a/src/wallet/spend.rs +++ b/src/wallet/spend.rs @@ -76,7 +76,7 @@ impl Wallet { } } - let tx = self.spend_coins(&coins, destination, feerate)?; + let tx = self.spend_coins(&coins[..], destination, feerate)?; Ok(tx) } @@ -113,7 +113,7 @@ impl Wallet { let utxo = match self .list_fidelity_spend_info() .iter() - .find(|(_, spend_info)| *spend_info == expired_fidelity_spend_info) + .find(|(_, spend_info)| **spend_info == expired_fidelity_spend_info) { Some((utxo, _)) => utxo, None => { @@ -133,9 +133,9 @@ impl Wallet { return Ok(()); } } - .clone(); + .to_owned(); - let tx = self.spend_coins(&[(utxo, expired_fidelity_spend_info)], destination, feerate)?; + let tx = self.spend_coins(&[(utxo.clone(), expired_fidelity_spend_info)], destination, feerate)?; let txid = self.send_tx(&tx)?; log::info!("Fidelity redeem transaction broadcasted. txid: {txid}"); @@ -174,8 +174,9 @@ impl Wallet { && input_value == og_sc.contract_tx.output[0].value { let destination = Destination::Sweep(destination_address.clone()); - let coins = vec![(utxo, spend_info)]; - let tx = self.spend_coins(&coins, destination, feerate)?; + let coin = (utxo.clone(), spend_info.clone()); + let coins = [coin]; + let tx = self.spend_coins(&coins[..], destination, feerate)?; return Ok(tx); } } @@ -202,9 +203,9 @@ impl Wallet { && input_value == ic_sc.contract_tx.output[0].value { let destination = Destination::Sweep(destination_address.clone()); - let coin = (utxo, spend_info); - let coins = vec![coin]; - let tx = self.spend_coins(&coins, destination, feerate)?; + let coin = (utxo.clone(), spend_info.clone()); + let coins = [coin]; + let tx = self.spend_coins(&coins[..], destination, feerate)?; return Ok(tx); } } diff --git a/tests/abort2_case1.rs b/tests/abort2_case1.rs index fa1c02ac1..87df4ed7a 100644 --- a/tests/abort2_case1.rs +++ b/tests/abort2_case1.rs @@ -1,10 +1,14 @@ #![cfg(feature = "integration-test")] use bitcoin::Amount; -use bitcoind::bitcoincore_rpc::RpcApi; +use bitcoind::bitcoincore_rpc::{ + RpcApi, + json::{ListUnspentResultEntry} +}; use coinswap::{ maker::{start_maker_server, MakerBehavior}, taker::{SwapParams, TakerBehavior}, utill::MIN_FEE_RATE, + wallet::UTXOSpendInfo, }; use std::sync::Arc; mod test_framework; @@ -306,8 +310,13 @@ fn maker_abort2_case1() { .unwrap()[0] .to_owned(); + let swap_utxos: Vec<(ListUnspentResultEntry, UTXOSpendInfo)> = + swap_coins + .iter() + .map(|(e, s)| ((*e).clone(), (*s).clone())) + .collect(); let tx = taker_wallet_mut - .spend_from_wallet(MIN_FEE_RATE, Destination::Sweep(addr), &swap_coins) + .spend_from_wallet(MIN_FEE_RATE, Destination::Sweep(addr), &swap_utxos) .unwrap(); assert_eq!( diff --git a/tests/standard_swap.rs b/tests/standard_swap.rs index 264233b60..a0ea79b91 100644 --- a/tests/standard_swap.rs +++ b/tests/standard_swap.rs @@ -4,11 +4,12 @@ use coinswap::{ maker::{start_maker_server, MakerBehavior}, taker::{SwapParams, TakerBehavior}, utill::MIN_FEE_RATE, - wallet::{AddressType, Destination}, + wallet::{AddressType, Destination, UTXOSpendInfo}, }; use std::sync::Arc; use bitcoind::bitcoincore_rpc::RpcApi; +use bitcoind::bitcoincore_rpc::json::ListUnspentResultEntry; mod test_framework; use test_framework::*; @@ -254,8 +255,13 @@ fn test_standard_coinswap() { .unwrap()[0] .to_owned(); + let owned_swap_coins: Vec<(ListUnspentResultEntry, UTXOSpendInfo)> = + swap_coins + .iter() + .map(|(e, s)| ((*e).clone(), (*s).clone())) + .collect(); let tx = taker_wallet_mut - .spend_from_wallet(MIN_FEE_RATE, Destination::Sweep(addr), &swap_coins) + .spend_from_wallet(MIN_FEE_RATE, Destination::Sweep(addr), &owned_swap_coins) .unwrap(); assert_eq!( diff --git a/tests/test_framework/mod.rs b/tests/test_framework/mod.rs index 26e293597..483534449 100644 --- a/tests/test_framework/mod.rs +++ b/tests/test_framework/mod.rs @@ -244,7 +244,7 @@ pub fn fund_and_verify_taker( // Get initial state before funding let wallet = taker.get_wallet_mut(); wallet.sync_and_save().unwrap(); - let initial_utxo_set = wallet.list_all_utxo(); + let initial_utxo_set: Vec<_> = wallet.list_all_utxo().into_iter().cloned().collect(); let initial_balances = wallet.get_balances().unwrap(); let initial_external_index = *wallet.get_external_index(); let mut new_txids = Vec::new(); @@ -339,7 +339,7 @@ pub fn fund_and_verify_maker( makers.iter().enumerate().for_each(|(maker_index, &maker)| { let mut wallet = maker.get_wallet().write().unwrap(); - let initial_utxo_set = wallet.list_all_utxo(); + let initial_utxo_set: Vec<_> = wallet.list_all_utxo().into_iter().cloned().collect(); let initial_balances = wallet.get_balances().unwrap(); let mut new_txids = Vec::new(); diff --git a/tests/utxo_behavior.rs b/tests/utxo_behavior.rs index 5e1783da9..a7941146c 100644 --- a/tests/utxo_behavior.rs +++ b/tests/utxo_behavior.rs @@ -524,7 +524,8 @@ fn test_maunal_coinselection() { println!("\n === Post-Swap UTXO Analysis ==="); // Get regular UTXOs (SeedCoin) - let regular_utxos = taker.get_wallet_mut().list_descriptor_utxo_spend_info(); + let wallet = taker.get_wallet_mut(); + let regular_utxos = wallet.list_descriptor_utxo_spend_info(); println!("\n ๐Ÿ“Š Regular UTXOs: {}", regular_utxos.len()); for (utxo, spend_info) in ®ular_utxos { @@ -536,7 +537,7 @@ fn test_maunal_coinselection() { } // Get swept incoming swap UTXOs - let swept_utxos = taker.get_wallet_mut().list_swept_incoming_swap_utxos(); + let swept_utxos = wallet.list_swept_incoming_swap_utxos(); println!("\n ๐Ÿงน Swept Swap UTXOs: {}", swept_utxos.len()); @@ -574,7 +575,7 @@ fn test_maunal_coinselection() { println!( "\n Taker Wallet Balances: {:?}", - taker.get_wallet().get_balances() + wallet.get_balances() ); println!( @@ -651,10 +652,7 @@ fn test_maunal_coinselection() { _ => None, }; - let result = - taker - .get_wallet_mut() - .coin_select(target_amount, MIN_FEE_RATE, manual_outpoints); + let result = wallet.coin_select(target_amount, MIN_FEE_RATE, manual_outpoints); match (result, expected) { (Ok(selection), "Ok") => { println!(