wallet: Prefer smaller change amount in tiebreaker

Requires bitcoin#25647

src/wallet/coinselection.cpp

@@ -408,9 +408,9 @@ CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng)
 
 void SelectionResult::ComputeAndSetWaste(const CAmount min_change, const CAmount change_cost, const CAmount change_fee)
 {
-    const CAmount change = GetChange(min_change, change_fee);
+    m_change = ComputeAndSetChange(min_change, change_fee);
 
-    if (change > 0) {
+    if (m_change > 0) {
         m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective);
     } else {
         m_waste = GetSelectionWaste(m_selected_inputs, 0, m_target, m_use_effective);
@@ -470,8 +470,16 @@ bool SelectionResult::operator<(SelectionResult other) const
 {
     Assert(m_waste.has_value());
     Assert(other.m_waste.has_value());
-    // As this operator is only used in std::min_element, we want the result that has more inputs when waste are equal.
-    return *m_waste < *other.m_waste || (*m_waste == *other.m_waste && m_selected_inputs.size() > other.m_selected_inputs.size());
+    // As this operator is only used in std::min_element, we want the result that has more inputs when waste are equal, and other things equal we want to reveal fewer funds to the counter party as well as unconfirm less liquidity
+    if (*m_waste != *other.m_waste) {
+        return *m_waste < *other.m_waste;
+    } else if (m_selected_inputs.size() != other.m_selected_inputs.size()) {
+        return (m_selected_inputs.size() > other.m_selected_inputs.size());
+    } else {
+        Assert(m_change.has_value());
+        Assert(other.m_change.has_value());
+        return m_change < other.m_change;
+    }
 }
 
 std::string COutput::ToString() const
@@ -492,7 +500,12 @@ std::string GetAlgorithmName(const SelectionAlgorithm algo)
     assert(false);
 }
 
-CAmount SelectionResult::GetChange(const CAmount min_change, const CAmount change_fee) const
+CAmount SelectionResult::GetChange() const
+{
+    return *Assert(m_change);
+}
+
+CAmount SelectionResult::ComputeAndSetChange(const CAmount min_change, const CAmount change_fee)
 {
     // change_budget = SUM(inputs) - SUM(outputs) - fees
     // 1) With SFFO we don't pay any fees
@@ -504,11 +517,8 @@ CAmount SelectionResult::GetChange(const CAmount min_change, const CAmount chang
                                   ? GetSelectedEffectiveValue() - m_target - change_fee
                                   : GetSelectedValue() - m_target;
 
-    if (change_budget < min_change) {
-        return 0;
-    }
-
-    return change_budget;
+    m_change = (change_budget < min_change) ? 0 : change_budget;
+    return *m_change;
 }
 
 } // namespace wallet

src/wallet/coinselection.h

@@ -287,6 +287,8 @@ struct SelectionResult
     std::set<COutput> m_selected_inputs;
     /** The target the algorithm selected for. Equal to the recipient amount plus non-input fees */
     CAmount m_target;
+    /** The computed change for this selection result under the selection attempt's circumstances */
+    std::optional<CAmount> m_change;
     /** The algorithm used to produce this result */
     SelectionAlgorithm m_algo;
     /** Whether the input values for calculations should be the effective value (true) or normal value (false) */
@@ -320,9 +322,17 @@ struct SelectionResult
     /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */
     std::vector<COutput> GetShuffledInputVector() const;
 
+    /** Sorting order for SelectionResults
+     * 1) Prefer lower waste,
+     * 2) at equal waste prefer higher input count,
+     * 3) at equal waste and input count prefer lower change
+     */
     bool operator<(SelectionResult other) const;
 
-    /** Get the amount for the change output after paying needed fees.
+    /** Get m_change */
+    CAmount GetChange() const;
+
+    /** Calculate the amount for the change output after paying needed fees.
      *
      * The change amount is not 100% precise due to discrepancies in fee calculation.
      * The final change amount (if any) should be corrected after calculating the final tx fees.
@@ -339,7 +349,7 @@ struct SelectionResult
      * @returns Amount for change output, 0 when there is no change.
      *
      */
-    CAmount GetChange(const CAmount min_change, const CAmount change_fee) const;
+    CAmount ComputeAndSetChange(const CAmount min_change, const CAmount change_fee);
 
     CAmount GetTarget() const { return m_target; }
 

src/wallet/spend.cpp

@@ -792,7 +792,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
     }
     TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->GetAlgo()).c_str(), result->GetTarget(), result->GetWaste(), result->GetSelectedValue());
 
-    const CAmount change_amount = result->GetChange(coin_selection_params.min_change, coin_selection_params.m_change_fee);
+    const CAmount change_amount = result->ComputeAndSetChange(coin_selection_params.min_change, coin_selection_params.m_change_fee);
     if (change_amount > 0) {
         CTxOut newTxOut(change_amount, scriptChange);
         if (nChangePosInOut == -1)