diff --git a/wallet/src/account/utxo_selector/mod.rs b/wallet/src/account/utxo_selector/mod.rs index dc03b8b84..83172a804 100644 --- a/wallet/src/account/utxo_selector/mod.rs +++ b/wallet/src/account/utxo_selector/mod.rs @@ -253,8 +253,17 @@ fn select_coins_srd( // Now check if we are above the target if selected_eff_value >= target_value { - // Result found, add it. - while let Some(group) = heap.pop() { + // Result found. The heap is ordered by effective value, so add only the most + // valuable groups needed to reach the target. Groups that were drawn earlier but + // turned out to be redundant (because a larger one was drawn afterwards) are left + // out instead of being included in the selection. + let mut added_eff_value = Amount::ZERO; + while added_eff_value < target_value { + let group = heap + .pop() + .expect("heap total reached the target, so it can't be emptied first"); + added_eff_value = (added_eff_value + group.0.get_effective_value(pay_fees)) + .ok_or(UtxoSelectorError::AmountArithmeticError)?; result.add_input(&group.0, pay_fees)?; } return Ok(result); @@ -799,7 +808,9 @@ fn select_random_coins( results .into_iter() - .min_by_key(|res| res.waste) + // Pick the least wasteful result; when several results have the same waste, prefer the + // one that selects fewer inputs so we don't end up with redundant UTXOs. + .min_by_key(|res| (res.waste, res.num_selected_inputs())) .ok_or_else(|| errors.pop().unwrap_or(UtxoSelectorError::NoSolutionFound)) } diff --git a/wallet/src/account/utxo_selector/tests.rs b/wallet/src/account/utxo_selector/tests.rs index 10b6f3ecd..c338fdba0 100644 --- a/wallet/src/account/utxo_selector/tests.rs +++ b/wallet/src/account/utxo_selector/tests.rs @@ -450,3 +450,36 @@ fn test_srd_solver_max_weight(#[case] seed: Seed) { assert_eq!(result.unwrap_err(), UtxoSelectorError::MaxWeightExceeded); } + +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn test_srd_solver_omits_redundant_utxos(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + + // One large UTXO that on its own covers the target, plus several small UTXOs that + // together can't reach it. Whatever order SRD draws them in, the large UTXO has to be + // drawn to reach the target, and once it is none of the small ones are needed. + let target_value = Amount::from_atoms(50); + let mut groups = vec![]; + add_output(Amount::from_atoms(100), &mut groups); + for _ in 0..10 { + add_output(Amount::from_atoms(1), &mut groups); + } + + let cost_of_change = Amount::ZERO; + let max_weight = 100; + let result = select_coins_srd( + &groups, + target_value, + &mut rng, + cost_of_change, + max_weight, + PayFee::PayFeeWithThisCurrency, + ) + .unwrap(); + + // Only the large UTXO should be selected; the redundant small ones must be left out. + assert_eq!(result.num_selected_inputs(), 1); + assert_eq!(result.effective_value, Amount::from_atoms(100)); +}