Skip to content

Avoid unnecessary cloning in list_* methods #668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
a-khushal wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Avoid unnecessary cloning in list_* methods #668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions examples/wallet_basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {

// 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());
Expand Down
10 changes: 8 additions & 2 deletions src/bin/taker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>(),
amount,
)?
.iter()
Expand Down Expand Up @@ -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::<Vec<_>>(),
target_amount,
)?
.iter()
Expand Down
4 changes: 2 additions & 2 deletions src/taker/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ impl Taker {
swap_params: SwapParams,
) -> Result<Option<SwapReport>, 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::<Vec<_>>();
self.ongoing_swap_state.swap_params = swap_params.clone();

// Check if we have enough balance - try regular first, then swap
Expand Down Expand Up @@ -576,7 +576,7 @@ impl Taker {
.wallet
.list_descriptor_utxo_spend_info()
.into_iter()
.map(|(utxo, _)| utxo)
.map(|(utxo, _)| utxo.clone())
.collect::<Vec<_>>();

let initial_outpoints = initial_utxos
Expand Down
2 changes: 1 addition & 1 deletion src/taker/api2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ impl Taker {
swap_params: SwapParams,
) -> Result<Option<SwapReport>, 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::<Vec<_>>();

let available = self.wallet.get_balances()?.spendable;

Expand Down
5 changes: 3 additions & 2 deletions src/utill.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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)]
Expand Down
69 changes: 30 additions & 39 deletions src/wallet/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -971,122 +971,114 @@ 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<ListUnspentResultEntry> {
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
}

/// 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
}

/// 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
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1653,19 +1645,19 @@ 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, _)| {
let outpoint = OutPoint::new(utxo.txid, utxo.vout);
!locked_utxos.contains(&outpoint)
})
.collect::<Vec<_>>()
};
}

// 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!(
Expand Down Expand Up @@ -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::<Vec<_>>(),
);

Expand Down Expand Up @@ -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()));
}
}

Expand Down Expand Up @@ -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,
)?;
Expand Down
19 changes: 10 additions & 9 deletions src/wallet/spend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl Wallet {
}
}

let tx = self.spend_coins(&coins, destination, feerate)?;
let tx = self.spend_coins(&coins[..], destination, feerate)?;

Ok(tx)
}
Expand Down Expand Up @@ -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 => {
Expand All @@ -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}");
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down
13 changes: 11 additions & 2 deletions tests/abort2_case1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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!(
Expand Down
Loading