coinselection: option to exclude change cost from waste score

Useful when the transaction already accounted for the change cost.
Which happens when the user, or wallet process, set the coin control
custom change address equal to one of the existing output addresses.

In other words, when the user selects one of the transaction existing
outputs to be increased by the difference between inputs - output.

This will be connected to the transaction creation process in the
following-up commit.

Author: furszy

src/wallet/coinselection.cpp

@@ -449,7 +449,7 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in
     }
 }
 
-CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value)
+CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, std::optional<CAmount> change_cost, CAmount target, bool use_effective_value)
 {
     // This function should not be called with empty inputs as that would mean the selection failed
     assert(!inputs.empty());
@@ -465,11 +465,12 @@ CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmo
 
     if (change_cost) {
         // Consider the cost of making change and spending it in the future
-        // If we aren't making change, the caller should've set change_cost to 0
-        assert(change_cost > 0);
-        waste += change_cost;
+        // If we aren't making change, the caller should've set change_cost to std::nullopt
+        // change_cost will be 0 if the change cost is already covered
+        assert(change_cost >= 0);
+        waste += *change_cost;
     } else {
-        // When we are not making change (change_cost == 0), consider the excess we are throwing away to fees
+        // When we are not making change (change_cost == std::nullopt), consider the excess we are throwing away to fees
         assert(selected_effective_value >= target);
         waste += selected_effective_value - target;
     }
@@ -495,7 +496,7 @@ void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const
     if (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);
+        m_waste = GetSelectionWaste(m_selected_inputs, /*change_cost=*/std::nullopt, m_target, m_use_effective);
     }
 }
 

src/wallet/coinselection.h

@@ -280,13 +280,14 @@ typedef std::map<CoinEligibilityFilter, OutputGroupTypeMap> FilteredOutputGroups
  *
  * @param[in] inputs The selected inputs
  * @param[in] change_cost The cost of creating change and spending it in the future.
- *                        Only used if there is change, in which case it must be positive.
- *                        Must be 0 if there is no change.
+ *                        Only used if there is change, in which case it must be positive, or 0 for an already covered
+ *                        change cost (e.g. existent change output).
+ *                        Must be std::nullopt if there is no change.
  * @param[in] target The amount targeted by the coin selection algorithm.
  * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false).
  * @return The waste
  */
-[[nodiscard]] CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true);
+[[nodiscard]] CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, std::optional<CAmount> change_cost, CAmount target, bool use_effective_value = true);
 
 
 /** Choose a random change target for each transaction to make it harder to fingerprint the Core

src/wallet/test/coinselector_tests.cpp

@@ -840,7 +840,7 @@ BOOST_AUTO_TEST_CASE(waste_test)
     // Waste without change is the excess and difference between fee and long term fee
     add_coin(1 * COIN, 1, selection, fee, fee - fee_diff);
     add_coin(2 * COIN, 2, selection, fee, fee - fee_diff);
-    const CAmount waste_nochange1 = GetSelectionWaste(selection, 0, target);
+    const CAmount waste_nochange1 = GetSelectionWaste(selection, /*change_cost=*/std::nullopt, target);
     BOOST_CHECK_EQUAL(fee_diff * 2 + excess, waste_nochange1);
     selection.clear();
 
@@ -853,7 +853,7 @@ BOOST_AUTO_TEST_CASE(waste_test)
     // Waste without change and fee == long term fee is just the excess
     add_coin(1 * COIN, 1, selection, fee, fee);
     add_coin(2 * COIN, 2, selection, fee, fee);
-    BOOST_CHECK_EQUAL(excess, GetSelectionWaste(selection, 0, target));
+    BOOST_CHECK_EQUAL(excess, GetSelectionWaste(selection, /*change_cost=*/std::nullopt, target));
     selection.clear();
 
     // Waste will be greater when fee is greater, but long term fee is the same
@@ -876,7 +876,7 @@ BOOST_AUTO_TEST_CASE(waste_test)
     // With long term fee greater than fee, waste should be less than when long term fee is less than fee
     add_coin(1 * COIN, 1, selection, fee, fee + fee_diff);
     add_coin(2 * COIN, 2, selection, fee, fee + fee_diff);
-    const CAmount waste_nochange2 = GetSelectionWaste(selection, 0, target);
+    const CAmount waste_nochange2 = GetSelectionWaste(selection, /*change_cost=*/std::nullopt, target);
     BOOST_CHECK_EQUAL(fee_diff * -2 + excess, waste_nochange2);
     BOOST_CHECK_LT(waste_nochange2, waste_nochange1);
     selection.clear();
@@ -885,7 +885,7 @@ BOOST_AUTO_TEST_CASE(waste_test)
     add_coin(1 * COIN, 1, selection, fee, fee);
     add_coin(2 * COIN, 2, selection, fee, fee);
     const CAmount exact_target{in_amt - fee * 2};
-    BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/0, exact_target));
+    BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/std::nullopt, exact_target));
     selection.clear();
 
     // No Waste when (fee - long_term_fee) == (-cost_of_change), and no excess
@@ -899,15 +899,15 @@ BOOST_AUTO_TEST_CASE(waste_test)
     const CAmount new_target{in_amt - fee * 2 - fee_diff * 2};
     add_coin(1 * COIN, 1, selection, fee, fee + fee_diff);
     add_coin(2 * COIN, 2, selection, fee, fee + fee_diff);
-    BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/ 0, new_target));
+    BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/std::nullopt, new_target));
     selection.clear();
 
     // Negative waste when the long term fee is greater than the current fee and the selected value == target
     const CAmount exact_target1{3 * COIN - 2 * fee};
     const CAmount target_waste1{-2 * fee_diff}; // = (2 * fee) - (2 * (fee + fee_diff))
     add_coin(1 * COIN, 1, selection, fee, fee + fee_diff);
     add_coin(2 * COIN, 2, selection, fee, fee + fee_diff);
-    BOOST_CHECK_EQUAL(target_waste1, GetSelectionWaste(selection, /*change_cost=*/ 0, exact_target1));
+    BOOST_CHECK_EQUAL(target_waste1, GetSelectionWaste(selection, /*change_cost=*/std::nullopt, exact_target1));
     selection.clear();
 
     // Negative waste when the long term fee is greater than the current fee and change_cost < - (inputs * (fee - long_term_fee))
@@ -1071,7 +1071,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight)
         rand,
         /*change_output_size=*/34,
         /*min_change_target=*/CENT,
-        /*effective_feerate=*/CFeeRate(0),
+        /*effective_feerate=*/CFeeRate(1), // tiny feerate so the waste score isn't always 0.
         /*long_term_feerate=*/CFeeRate(0),
         /*tx_noinputs_size=*/10 + 34, // static header size + output size
         /*avoid_partial=*/false,
@@ -1089,10 +1089,10 @@ BOOST_AUTO_TEST_CASE(check_max_weight)
             target, cs_params, cc, [&](CWallet& wallet) {
                 CoinsResult available_coins;
                 for (int j = 0; j < 1515; ++j) {
-                    add_coin(available_coins, wallet, CAmount(0.033 * COIN), CFeeRate(0), 144, false, 0, true);
+                    add_coin(available_coins, wallet, CAmount(0.033 * COIN), cs_params.m_effective_feerate, 144, false, 0, true);
                 }
 
-                add_coin(available_coins, wallet, CAmount(50 * COIN), CFeeRate(0), 144, false, 0, true);
+                add_coin(available_coins, wallet, CAmount(50 * COIN), cs_params.m_effective_feerate, 144, false, 0, true);
                 return available_coins;
             },
             m_node);
@@ -1101,7 +1101,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight)
         // Verify that only the 50 BTC UTXO was selected
         const auto& selection_res = result->GetInputSet();
         BOOST_CHECK(selection_res.size() == 1);
-        BOOST_CHECK((*selection_res.begin())->GetEffectiveValue() == 50 * COIN);
+        BOOST_CHECK((*selection_res.begin())->txout.nValue == 50 * COIN);
     }
 
     {