wallet: Add coin control option "add_excess_to_recipient_position"

If this coin selection RPC option is set, then the excess value for a changeless BnB tx will be added to the selected recipient output position instead of added to fees.

src/bench/coin_selection.cpp

@@ -117,7 +117,7 @@ static void BnBExhaustion(benchmark::Bench& bench)
     bench.run([&] {
         // Benchmark
         CAmount target = make_hard_case(17, utxo_pool);
-        SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_WEIGHT); // Should exhaust
+        SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_WEIGHT, /*add_excess_to_target=*/false); // Should exhaust
 
         // Cleanup
         utxo_pool.clear();

src/rpc/client.cpp

@@ -148,6 +148,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "fundrawtransaction", 1, "solving_data"},
     { "fundrawtransaction", 1, "change_target"},
     { "fundrawtransaction", 1, "enable_algos"},
+    { "fundrawtransaction", 1, "add_excess_to_recipient_position"},
     { "fundrawtransaction", 2, "iswitness" },
     { "walletcreatefundedpsbt", 0, "inputs" },
     { "walletcreatefundedpsbt", 1, "outputs" },
@@ -168,6 +169,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "walletcreatefundedpsbt", 3, "solving_data"},
     { "walletcreatefundedpsbt", 3, "change_target"},
     { "walletcreatefundedpsbt", 3, "enable_algos"},
+    { "walletcreatefundedpsbt", 3, "add_excess_to_recipient_position"},
     { "walletcreatefundedpsbt", 4, "bip32derivs" },
     { "walletprocesspsbt", 1, "sign" },
     { "walletprocesspsbt", 3, "bip32derivs" },
@@ -214,6 +216,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "send", 4, "solving_data"},
     { "send", 4, "change_target"},
     { "send", 4, "enable_algos"},
+    { "send", 4, "add_excess_to_recipient_position"},
     { "sendall", 0, "recipients" },
     { "sendall", 1, "conf_target" },
     { "sendall", 3, "fee_rate"},

src/wallet/coincontrol.h

@@ -121,6 +121,8 @@ class CCoinControl
     std::optional<CAmount> m_change_target;
     //! Enable corresponding coin selection algorithms
     std::bitset<size_t(SelectionAlgorithm::NUM_ELEMENTS)> m_enable_algos;
+    //! If set, add any excess from changeless spends to the specified recipient output index instead of to fees and do not count it as waste.
+    std::optional<uint32_t> m_add_excess_to_recipient_position;
 
     CCoinControl();
 

src/wallet/coinselection.cpp

@@ -85,13 +85,14 @@ struct {
  * @param const CAmount& cost_of_change This is the cost of creating and spending a change output.
  *        This plus selection_target is the upper bound of the range.
  * @param int max_weight The maximum weight available for the input set.
+ * @param bool add_excess_to_target When true do not count excess as waste and add to the result target
  * @returns The result of this coin selection algorithm, or std::nullopt
  */
 
 static const size_t TOTAL_TRIES = 100000;
 
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
-                                             int max_weight)
+                                             int max_weight, const bool add_excess_to_target)
 {
     SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     CAmount curr_value = 0;
@@ -194,6 +195,11 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
     for (const size_t& i : best_selection) {
         result.AddInput(utxo_pool.at(i));
     }
+
+    if (add_excess_to_target) {
+        auto excess = result.ResetTargetToSelectedValue();
+        best_waste -= excess;
+    }
     result.ComputeAndSetWaste(cost_of_change, cost_of_change, CAmount{0});
     assert(best_waste == result.GetWaste());
 
@@ -851,6 +857,13 @@ void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const
     }
 }
 
+CAmount SelectionResult::ResetTargetToSelectedValue()
+{
+    CAmount excess = (m_use_effective ? GetSelectedEffectiveValue(): GetSelectedValue()) - m_target;
+    m_target += excess;
+    return excess;
+}
+
 void SelectionResult::SetAlgoCompleted(bool algo_completed)
 {
     m_algo_completed = algo_completed;

src/wallet/coinselection.h

@@ -190,6 +190,11 @@ struct CoinSelectionParams {
      * to its SelectionAlgorithm enum value set to true. By default all are enabled.
     */
     std::bitset<size_t(SelectionAlgorithm::NUM_ELEMENTS)> m_enable_algos;
+    /***
+     * When set, excess value for changeless results will be added to the target amount at the given position
+     * and not counted as waste. Otherwise excess value will be be applied to fees and counted as waste.
+    */
+    std::optional<uint32_t> m_add_excess_to_recipient_position;
 
     CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size,
                         CAmount min_change_target, CFeeRate effective_feerate,
@@ -396,6 +401,9 @@ struct SelectionResult
     /** How much individual inputs overestimated the bump fees for shared ancestries */
     void SetBumpFeeDiscount(const CAmount discount);
 
+    /** Reset target to the current selected amount */
+    CAmount ResetTargetToSelectedValue();
+
     /** Calculates and stores the waste for this selection via GetSelectionWaste */
     void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee);
     [[nodiscard]] CAmount GetWaste() const;
@@ -454,7 +462,7 @@ struct SelectionResult
 };
 
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
-                                             int max_weight);
+                                             int max_weight, const bool add_excess_to_target);
 
 util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight);
 

src/wallet/rpc/spend.cpp

@@ -537,6 +537,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
                 {"input_weights", UniValueType(UniValue::VARR)},
                 {"change_target", UniValueType()}, // will be checked by AmountFromValue() below
                 {"enable_algos", UniValueType(UniValue::VARR)},
+                {"add_excess_to_recipient_position", UniValue::VNUM},
             },
             true, true);
 
@@ -631,6 +632,14 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
                 }
             }
         }
+
+        if (options.exists("add_excess_to_recipient_position")) {
+            coinControl.m_add_excess_to_recipient_position = options["add_excess_to_recipient_position"].getInt<uint32_t>();
+            if (coinControl.m_add_excess_to_recipient_position.value() >= recipients.size()) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Cannot add excess to the recipient output at index %d; the output does not exist.", coinControl.m_add_excess_to_recipient_position.value()));
+            }
+        }
+
       }
     } else {
         // if options is null and not a bool
@@ -810,6 +819,7 @@ RPCHelpMan fundrawtransaction()
                                     {"algo", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "One of: \"bnb\", \"cg\", \"knapsack\" or \"srd\"."},
                                 }
                              },
+                             {"add_excess_to_recipient_position", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index where excess fees are added. If not set, excess value from changeless transactions is added to fees"},
                         },
                         FundTxDoc()),
                         RPCArgOptions{
@@ -1270,6 +1280,7 @@ RPCHelpMan send()
                             {"algo", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "One of: \"bnb\", \"cg\", \"knapsack\" or \"srd\"."},
                         }
                     },
+                    {"add_excess_to_recipient_position", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index where excess fees are added. If not set, excess value from changeless transactions is added to fees."},
                 },
                 FundTxDoc()),
                 RPCArgOptions{.oneline_description="options"}},
@@ -1725,6 +1736,7 @@ RPCHelpMan walletcreatefundedpsbt()
                                     {"algo", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "One of: \"bnb\", \"cg\", \"knapsack\" or \"srd\"."},
                                 }
                             },
+                            {"add_excess_to_recipient_position", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index where excess fees are added. If not set, excess value from changeless transactions is added to fees."},
                         },
                         FundTxDoc()),
                         RPCArgOptions{.oneline_description="options"}},

src/wallet/spend.cpp

@@ -697,7 +697,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
     if (coin_selection_params.m_enable_algos.test(size_t(SelectionAlgorithm::BNB))) {
         // SFFO frequently causes issues in the context of changeless input sets: skip BnB when SFFO is active
         if (!coin_selection_params.m_subtract_fee_outputs) {
-            if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) {
+            if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight, coin_selection_params.m_add_excess_to_recipient_position.has_value())}) {
                 results.push_back(*bnb_result);
             } else append_error(std::move(bnb_result));
         }
@@ -1102,6 +1102,9 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     // All algorithms, or a subset of algorithms enabled by the user
     coin_selection_params.m_enable_algos = coin_control.m_enable_algos;
 
+    // If set, do not add any excess from a changeless transaction to fees 
+    coin_selection_params.m_add_excess_to_recipient_position = coin_control.m_add_excess_to_recipient_position;
+
     // vouts to the payees
     for (const auto& recipient : vecSend)
     {
@@ -1156,7 +1159,13 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
            result.GetWaste(),
            result.GetSelectedValue());
 
-    const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee);
+    if (result.GetTarget() != selection_target && coin_selection_params.m_add_excess_to_recipient_position) {
+        auto excess = result.GetTarget() - selection_target;
+        txNew.vout[coin_selection_params.m_add_excess_to_recipient_position.value()].nValue += excess;
+        recipients_sum += excess;
+    }
+
+    CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee);
     if (change_amount > 0) {
         CTxOut newTxOut(change_amount, scriptChange);
         if (!change_pos) {

src/wallet/test/coinselector_tests.cpp

@@ -100,7 +100,7 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
 
 std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
 {
-    auto res{SelectCoinsBnB(utxo_pool, selection_target, cost_of_change, MAX_STANDARD_TX_WEIGHT)};
+    auto res{SelectCoinsBnB(utxo_pool, selection_target, cost_of_change, MAX_STANDARD_TX_WEIGHT, /*add_excess_to_target=*/false)};
     return res ? std::optional<SelectionResult>(*res) : std::nullopt;
 }
 
@@ -328,7 +328,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
 
         add_coin(available_coins, *wallet, 1, coin_selection_params_bnb.m_effective_feerate);
         available_coins.All().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
-        BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change));
+        BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, /*add_excess_to_target=*/false));
 
         // Test fees subtracted from output:
         available_coins.Clear();
@@ -439,7 +439,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
 
         CAmount selection_target = 16 * CENT;
         const auto& no_res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs*/true),
-                                            selection_target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT);
+                                            selection_target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT, /*add_excess_to_target=*/false);
         BOOST_REQUIRE(!no_res);
         BOOST_CHECK(util::ErrorString(no_res).original.find("The inputs size exceeds the maximum weight") != std::string::npos);
 

src/wallet/test/fuzz/coinselection.cpp

@@ -258,7 +258,7 @@ FUZZ_TARGET(coinselection)
 
     // Run coinselection algorithms
     auto result_bnb = coin_params.m_subtract_fee_outputs ? util::Error{Untranslated("BnB disabled when SFFO is enabled")} :
-                      SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, MAX_STANDARD_TX_WEIGHT);
+                      SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, MAX_STANDARD_TX_WEIGHT, /*add_excess_to_target=*/false);
     if (result_bnb) {
         assert(result_bnb->GetChange(coin_params.min_viable_change, coin_params.m_change_fee) == 0);
         assert(result_bnb->GetSelectedValue() >= target);