Avoid selecting redundant UTXOs during coin selection

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

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));
+}