From d9501187ef3c4436b7baedb23a4d4f6b1f46a03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 16 Jan 2024 22:33:42 +0800 Subject: [PATCH 1/2] test(wallet): fix tests helpers to generate unique utxos --- crates/bdk/src/wallet/coin_selection.rs | 47 +++++++++++++------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/crates/bdk/src/wallet/coin_selection.rs b/crates/bdk/src/wallet/coin_selection.rs index 8829ba3a..4f810742 100644 --- a/crates/bdk/src/wallet/coin_selection.rs +++ b/crates/bdk/src/wallet/coin_selection.rs @@ -799,13 +799,14 @@ mod test { fn generate_random_utxos(rng: &mut StdRng, utxos_number: usize) -> Vec { let mut res = Vec::new(); - for _ in 0..utxos_number { + for i in 0..utxos_number { res.push(WeightedUtxo { satisfaction_weight: P2WPKH_SATISFACTION_SIZE, utxo: Utxo::Local(LocalOutput { - outpoint: OutPoint::from_str( - "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0", - ) + outpoint: OutPoint::from_str(&format!( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:{}", + i + )) .unwrap(), txout: TxOut { value: rng.gen_range(0..200000000), @@ -829,24 +830,26 @@ mod test { } fn generate_same_value_utxos(utxos_value: u64, utxos_number: usize) -> Vec { - let utxo = WeightedUtxo { - satisfaction_weight: P2WPKH_SATISFACTION_SIZE, - utxo: Utxo::Local(LocalOutput { - outpoint: OutPoint::from_str( - "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0", - ) - .unwrap(), - txout: TxOut { - value: utxos_value, - script_pubkey: ScriptBuf::new(), - }, - keychain: KeychainKind::External, - is_spent: false, - derivation_index: 42, - confirmation_time: ConfirmationTime::Unconfirmed { last_seen: 0 }, - }), - }; - vec![utxo; utxos_number] + (0..utxos_number) + .map(|i| WeightedUtxo { + satisfaction_weight: P2WPKH_SATISFACTION_SIZE, + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str(&format!( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:{}", + i + )) + .unwrap(), + txout: TxOut { + value: utxos_value, + script_pubkey: ScriptBuf::new(), + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + confirmation_time: ConfirmationTime::Unconfirmed { last_seen: 0 }, + }), + }) + .collect() } fn sum_random_utxos(mut rng: &mut StdRng, utxos: &mut Vec) -> u64 { From 5299db34cb9117ad1b66a6afcb51f6ca7e1f0d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 16 Jan 2024 15:51:15 +0800 Subject: [PATCH 2/2] fix(wallet): filter duplicates before coin selection --- crates/bdk/src/wallet/coin_selection.rs | 113 ++++++++++++++++++++++++ crates/bdk/src/wallet/mod.rs | 3 + 2 files changed, 116 insertions(+) diff --git a/crates/bdk/src/wallet/coin_selection.rs b/crates/bdk/src/wallet/coin_selection.rs index 4f810742..ac6084cf 100644 --- a/crates/bdk/src/wallet/coin_selection.rs +++ b/crates/bdk/src/wallet/coin_selection.rs @@ -100,6 +100,7 @@ //! # Ok::<(), anyhow::Error>(()) //! ``` +use crate::chain::collections::HashSet; use crate::types::FeeRate; use crate::wallet::utils::IsDust; use crate::Utxo; @@ -107,6 +108,7 @@ use crate::WeightedUtxo; use alloc::vec::Vec; use bitcoin::consensus::encode::serialize; +use bitcoin::OutPoint; use bitcoin::{Script, Weight}; use core::convert::TryInto; @@ -711,6 +713,25 @@ impl BranchAndBoundCoinSelection { } } +/// Remove duplicate UTXOs. +/// +/// If a UTXO appears in both `required` and `optional`, the appearance in `required` is kept. +pub(crate) fn filter_duplicates(required: I, optional: I) -> (I, I) +where + I: IntoIterator + FromIterator, +{ + let mut visited = HashSet::::new(); + let required = required + .into_iter() + .filter(|utxo| visited.insert(utxo.utxo.outpoint())) + .collect::(); + let optional = optional + .into_iter() + .filter(|utxo| visited.insert(utxo.utxo.outpoint())) + .collect::(); + (required, optional) +} + #[cfg(test)] mod test { use assert_matches::assert_matches; @@ -721,6 +742,7 @@ mod test { use super::*; use crate::types::*; + use crate::wallet::coin_selection::filter_duplicates; use crate::wallet::Vbytes; use rand::rngs::StdRng; @@ -1481,4 +1503,95 @@ mod test { }) ); } + + #[test] + fn test_filter_duplicates() { + fn utxo(txid: &str, value: u64) -> WeightedUtxo { + WeightedUtxo { + satisfaction_weight: 0, + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::new(bitcoin::hashes::Hash::hash(txid.as_bytes()), 0), + txout: TxOut { + value, + script_pubkey: ScriptBuf::new(), + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 0, + confirmation_time: ConfirmationTime::Confirmed { + height: 12345, + time: 12345, + }, + }), + } + } + + fn to_utxo_vec(utxos: &[(&str, u64)]) -> Vec { + let mut v = utxos + .iter() + .map(|&(txid, value)| utxo(txid, value)) + .collect::>(); + v.sort_by_key(|u| u.utxo.outpoint()); + v + } + + struct TestCase<'a> { + name: &'a str, + required: &'a [(&'a str, u64)], + optional: &'a [(&'a str, u64)], + exp_required: &'a [(&'a str, u64)], + exp_optional: &'a [(&'a str, u64)], + } + + let test_cases = [ + TestCase { + name: "no_duplicates", + required: &[("A", 1000), ("B", 2100)], + optional: &[("C", 1000)], + exp_required: &[("A", 1000), ("B", 2100)], + exp_optional: &[("C", 1000)], + }, + TestCase { + name: "duplicate_required_utxos", + required: &[("A", 3000), ("B", 1200), ("C", 1234), ("A", 3000)], + optional: &[("D", 2100)], + exp_required: &[("A", 3000), ("B", 1200), ("C", 1234)], + exp_optional: &[("D", 2100)], + }, + TestCase { + name: "duplicate_optional_utxos", + required: &[("A", 3000), ("B", 1200)], + optional: &[("C", 5000), ("D", 1300), ("C", 5000)], + exp_required: &[("A", 3000), ("B", 1200)], + exp_optional: &[("C", 5000), ("D", 1300)], + }, + TestCase { + name: "duplicate_across_required_and_optional_utxos", + required: &[("A", 3000), ("B", 1200), ("C", 2100)], + optional: &[("A", 3000), ("D", 1200), ("E", 5000)], + exp_required: &[("A", 3000), ("B", 1200), ("C", 2100)], + exp_optional: &[("D", 1200), ("E", 5000)], + }, + ]; + + for (i, t) in test_cases.into_iter().enumerate() { + println!("Case {}: {}", i, t.name); + let (required, optional) = + filter_duplicates(to_utxo_vec(t.required), to_utxo_vec(t.optional)); + assert_eq!( + required, + to_utxo_vec(t.exp_required), + "[{}:{}] unexpected `required` result", + i, + t.name + ); + assert_eq!( + optional, + to_utxo_vec(t.exp_optional), + "[{}:{}] unexpected `optional` result", + i, + t.name + ); + } + } } diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index ce152cb5..ef63c040 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -1469,6 +1469,9 @@ impl Wallet { } }; + let (required_utxos, optional_utxos) = + coin_selection::filter_duplicates(required_utxos, optional_utxos); + let coin_selection = coin_selection.coin_select( required_utxos, optional_utxos,