From b495cc0e72fcee3354a6f5657b66679aad06cd7f Mon Sep 17 00:00:00 2001 From: Oliver Anyanwu Date: Sat, 11 Jul 2026 16:23:42 +0200 Subject: [PATCH] Prefer confirmed utxos during coin selection Since the mempool started limiting the size of transaction clusters, a wallet that builds a transaction on top of unconfirmed (in-mempool) utxos can produce a transaction that the mempool then rejects because the resulting cluster is too big. As a first step towards taking cluster limits into account, prefer confirmed utxos during selection. select_inputs_for_send_request now also gathers the confirmed-only utxos and select_coins_preferring_confirmed tries to satisfy the target from those first, falling back to the full set (which also includes the unconfirmed utxos) only when the confirmed ones are not enough. The fallback path is identical to the previous behaviour, so nothing regresses when confirmed utxos can't cover the amount. This addresses the first item of #2066; querying the mempool for the resulting cluster size and retrying is left for a follow-up. --- wallet/src/account/mod.rs | 69 ++++++++++++++++------- wallet/src/account/utxo_selector/mod.rs | 43 ++++++++++++++ wallet/src/account/utxo_selector/tests.rs | 67 ++++++++++++++++++++++ 3 files changed, 158 insertions(+), 21 deletions(-) diff --git a/wallet/src/account/mod.rs b/wallet/src/account/mod.rs index 59b3ca04d4..3765dc369a 100644 --- a/wallet/src/account/mod.rs +++ b/wallet/src/account/mod.rs @@ -84,7 +84,7 @@ use wallet_types::{ use crate::{ SendRequest, WalletError, WalletResult, - account::utxo_selector::{OutputGroup, select_coins}, + account::utxo_selector::{OutputGroup, select_coins_preferring_confirmed}, destination_getters::{HtlcSpendingCondition, get_tx_output_destination}, key_chain::{AccountKeyChains, KeyChainError, VRFAccountKeyChains}, send_request::{ @@ -112,7 +112,7 @@ pub use self::{ utxo_selector::{CoinSelectionAlgo, UtxoSelectorError}, }; -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct CurrentFeeRate { pub current_fee_rate: FeeRate, pub consolidate_fee_rate: FeeRate, @@ -277,39 +277,58 @@ impl Account { Ok(()) })?; - let (utxos, selection_algo) = if input_utxos.is_empty() { + let (utxos, confirmed_utxos, selection_algo) = if input_utxos.is_empty() { + let utxo_types = UtxoType::Transfer | UtxoType::LockThenTransfer | UtxoType::IssueNft; + let utxos = self.get_utxos( + utxo_types, + median_time, + UtxoState::Confirmed | UtxoState::InMempool | UtxoState::Inactive, + WithLocked::Unlocked, + ); + // Prefer confirmed utxos during selection: spending an unconfirmed utxo forms a + // mempool cluster, which is size limited, so building on top of confirmed utxos + // reduces the chance of producing a tx that the mempool would reject (see issue + // #2066). The confirmed-only set is tried first, with a fallback to the full set. + let confirmed_utxos = self.get_utxos( + utxo_types, + median_time, + UtxoState::Confirmed.into(), + WithLocked::Unlocked, + ); ( - self.get_utxos( - UtxoType::Transfer | UtxoType::LockThenTransfer | UtxoType::IssueNft, - median_time, - UtxoState::Confirmed | UtxoState::InMempool | UtxoState::Inactive, - WithLocked::Unlocked, - ), + utxos, + Some(confirmed_utxos), selection_algo.unwrap_or(CoinSelectionAlgo::Randomize), ) } else { let selection_algo = selection_algo.unwrap_or(CoinSelectionAlgo::UsePreselected); - match input_utxos { + let utxos = match input_utxos { SelectedInputs::Utxos(input_utxos) => { let current_block_info = BlockInfo { height: self.account_info.best_block_height(), timestamp: median_time, }; - ( - self.output_cache.find_utxos(current_block_info, input_utxos)?, - selection_algo, - ) + self.output_cache.find_utxos(current_block_info, input_utxos)? } - SelectedInputs::Inputs(ref inputs) => ( - inputs.iter().map(|(outpoint, utxo)| (outpoint.clone(), utxo)).collect(), - selection_algo, - ), - } + SelectedInputs::Inputs(ref inputs) => { + inputs.iter().map(|(outpoint, utxo)| (outpoint.clone(), utxo)).collect() + } + }; + (utxos, None, selection_algo) }; let current_fee_rate = fee_rates.current_fee_rate; let mut utxos_by_currency = self.utxo_output_groups_by_currency(fee_rates, &pay_fee_with_currency, utxos)?; + let mut confirmed_utxos_by_currency = confirmed_utxos + .map(|confirmed_utxos| { + self.utxo_output_groups_by_currency( + fee_rates, + &pay_fee_with_currency, + confirmed_utxos, + ) + }) + .transpose()?; let amount_to_be_paid_in_currency_with_fees = output_currency_amounts.remove(&pay_fee_with_currency).unwrap_or(Amount::ZERO); @@ -342,6 +361,9 @@ impl Account { .iter() .map(|(currency, output_amount)| -> WalletResult<_> { let utxos = utxos_by_currency.remove(currency).unwrap_or(vec![]); + let confirmed_utxos = confirmed_utxos_by_currency + .as_mut() + .map(|by_currency| by_currency.remove(currency).unwrap_or_default()); let preselected_amount = preselected_inputs .amounts .remove(currency) @@ -354,7 +376,8 @@ impl Account { change_address.clone(), )?; - let selection_result = select_coins( + let selection_result = select_coins_preferring_confirmed( + confirmed_utxos, utxos, output_amount.sub(preselected_amount).unwrap_or(Amount::ZERO), PayFee::DoNotPayFeeWithThisCurrency, @@ -393,6 +416,9 @@ impl Account { .try_collect()?; let utxos = utxos_by_currency.remove(&pay_fee_with_currency).unwrap_or(vec![]); + let confirmed_utxos = confirmed_utxos_by_currency + .as_mut() + .map(|by_currency| by_currency.remove(&pay_fee_with_currency).unwrap_or_default()); let num_inputs = selected_inputs .values() @@ -438,7 +464,8 @@ impl Account { change_address.clone(), )?; - let selection_result = select_coins( + let selection_result = select_coins_preferring_confirmed( + confirmed_utxos, utxos, (amount_to_be_paid_in_currency_with_fees - preselected_amount).unwrap_or(Amount::ZERO), PayFee::PayFeeWithThisCurrency, diff --git a/wallet/src/account/utxo_selector/mod.rs b/wallet/src/account/utxo_selector/mod.rs index dc03b8b84b..c38c8f142f 100644 --- a/wallet/src/account/utxo_selector/mod.rs +++ b/wallet/src/account/utxo_selector/mod.rs @@ -734,6 +734,49 @@ pub fn select_coins( } } +/// Select coins, preferring confirmed utxos. +/// +/// A transaction that spends an unconfirmed (in-mempool) utxo forms a cluster with the +/// transaction that produced it, and the mempool limits how big such a cluster may be. To +/// reduce the chance of building a transaction that the mempool would reject, this first tries +/// to reach the target using `confirmed_pool` alone. Only if that doesn't work (for example +/// because the confirmed utxos aren't enough) does it fall back to `full_pool`, which also +/// contains the unconfirmed utxos. +/// +/// `confirmed_pool` is `None` when there is no preference to apply, for example when the caller +/// selected the inputs explicitly; in that case `full_pool` is used directly. +pub fn select_coins_preferring_confirmed( + confirmed_pool: Option>, + full_pool: Vec, + selection_target: Amount, + pay_fees: PayFee, + cost_of_change: Amount, + coin_selection_algo: CoinSelectionAlgo, + max_tx_weight: usize, +) -> Result { + if let Some(confirmed_pool) = confirmed_pool + && let Ok(result) = select_coins( + confirmed_pool, + selection_target, + pay_fees, + cost_of_change, + coin_selection_algo, + max_tx_weight, + ) + { + return Ok(result); + } + + select_coins( + full_pool, + selection_target, + pay_fees, + cost_of_change, + coin_selection_algo, + max_tx_weight, + ) +} + fn select_random_coins( mut utxo_pool: Vec, selection_target: Amount, diff --git a/wallet/src/account/utxo_selector/tests.rs b/wallet/src/account/utxo_selector/tests.rs index 10b6f3ecd5..2784cd05e7 100644 --- a/wallet/src/account/utxo_selector/tests.rs +++ b/wallet/src/account/utxo_selector/tests.rs @@ -450,3 +450,70 @@ fn test_srd_solver_max_weight(#[case] seed: Seed) { assert_eq!(result.unwrap_err(), UtxoSelectorError::MaxWeightExceeded); } + +#[test] +fn test_prefer_confirmed_uses_confirmed_when_sufficient() { + // The confirmed pool alone can cover the target, so it must be used and the full pool + // (deliberately given a different, larger value) must be left untouched. + let mut confirmed = vec![]; + add_output(Amount::from_atoms(60), &mut confirmed); + let mut full = vec![]; + add_output(Amount::from_atoms(1000), &mut full); + + let result = select_coins_preferring_confirmed( + Some(confirmed), + full, + Amount::from_atoms(50), + PayFee::PayFeeWithThisCurrency, + Amount::ZERO, + CoinSelectionAlgo::Randomize, + 100, + ) + .unwrap(); + + assert_eq!(result.effective_value, Amount::from_atoms(60)); +} + +#[test] +fn test_prefer_confirmed_falls_back_when_confirmed_insufficient() { + // The confirmed pool can't reach the target on its own, so selection must fall back to the + // full pool, which additionally contains a larger (unconfirmed) utxo. + let mut confirmed = vec![]; + add_output(Amount::from_atoms(10), &mut confirmed); + let mut full = vec![]; + add_output(Amount::from_atoms(10), &mut full); + add_output(Amount::from_atoms(90), &mut full); + + let result = select_coins_preferring_confirmed( + Some(confirmed), + full, + Amount::from_atoms(50), + PayFee::PayFeeWithThisCurrency, + Amount::ZERO, + CoinSelectionAlgo::Randomize, + 100, + ) + .unwrap(); + + assert!(result.effective_value >= Amount::from_atoms(50)); +} + +#[test] +fn test_prefer_confirmed_without_preference_uses_full_pool() { + // With no confirmed pool the full pool is used directly. + let mut full = vec![]; + add_output(Amount::from_atoms(70), &mut full); + + let result = select_coins_preferring_confirmed( + None, + full, + Amount::from_atoms(50), + PayFee::PayFeeWithThisCurrency, + Amount::ZERO, + CoinSelectionAlgo::Randomize, + 100, + ) + .unwrap(); + + assert_eq!(result.effective_value, Amount::from_atoms(70)); +}