diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8ef996e93ea3d9..a2ac68fdf86875 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -148,6 +148,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "fundrawtransaction", 1, "solving_data"}, { "fundrawtransaction", 1, "max_tx_weight"}, { "fundrawtransaction", 1, "add_excess_to_recipient_position"}, + { "fundrawtransaction", 1, "max_excess"}, { "fundrawtransaction", 2, "iswitness" }, { "walletcreatefundedpsbt", 0, "inputs" }, { "walletcreatefundedpsbt", 1, "outputs" }, @@ -168,6 +169,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "walletcreatefundedpsbt", 3, "solving_data"}, { "walletcreatefundedpsbt", 3, "max_tx_weight"}, { "walletcreatefundedpsbt", 3, "add_excess_to_recipient_position"}, + { "walletcreatefundedpsbt", 3, "max_excess"}, { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletprocesspsbt", 1, "sign" }, { "walletprocesspsbt", 3, "bip32derivs" }, @@ -214,6 +216,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 4, "solving_data"}, { "send", 4, "max_tx_weight"}, { "send", 4, "add_excess_to_recipient_position"}, + { "send", 4, "max_excess"}, { "sendall", 0, "recipients" }, { "sendall", 1, "conf_target" }, { "sendall", 3, "fee_rate"}, diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 39cf5f797a9cd3..8b3e557c6ee5a2 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -119,6 +119,8 @@ class CCoinControl std::optional m_max_tx_weight{std::nullopt}; //! 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 m_add_excess_to_recipient_position; + //! If set, excess from changeless spends can not exceed this amount, otherwise use cost_of_change by default + std::optional m_max_excess; CCoinControl(); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index aa519ccc057aea..de2279a04d32f8 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -52,7 +52,7 @@ struct { /* * This is the Branch and Bound Coin Selection algorithm designed by Murch. It searches for an input * set that can pay for the spending target and does not exceed the spending target by more than the - * cost of creating and spending a change output. The algorithm uses a depth-first search on a binary + * the specified maximum excess value. The algorithm uses a depth-first search on a binary * tree. In the binary tree, each node corresponds to the inclusion or the omission of a UTXO. UTXOs * are sorted by their effective values and the tree is explored deterministically per the inclusion * branch first. At each node, the algorithm checks whether the selection is within the target range. @@ -82,7 +82,7 @@ struct { * values are their effective values. * @param const CAmount& selection_target This is the value that we want to select. It is the lower * bound of the range. - * @param const CAmount& cost_of_change This is the cost of creating and spending a change output. + * @param const CAmount& max_excess This is the amount of value over the selection target we can select. * This plus selection_target is the upper bound of the range. * @param int max_selection_weight The maximum allowed weight for a selection result to be valid. * @param bool add_excess_to_target When true do not count excess as waste and add to the result target @@ -91,7 +91,7 @@ struct { static const size_t TOTAL_TRIES = 100000; -util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& max_excess, int max_selection_weight, const bool add_excess_to_target) { SelectionResult result(selection_target, SelectionAlgorithm::BNB); @@ -126,7 +126,7 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool // Conditions for starting a backtrack bool backtrack = false; if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value. - curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch + curr_value > selection_target + max_excess || // Selected value is out of range, go back and try other branch (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing backtrack = true; } else if (curr_selection_weight > max_selection_weight) { // Selected UTXOs weight exceeds the maximum weight allowed, cannot find more solutions by adding more inputs @@ -199,7 +199,7 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool auto excess = result.ResetTargetToSelectedValue(); best_waste -= excess; } - result.RecalculateWaste(cost_of_change, cost_of_change, CAmount{0}); + result.RecalculateWaste(max_excess, max_excess, CAmount{0}); assert(best_waste == result.GetWaste()); return result; diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index b7a5b6f2ab3393..8235bb80416b40 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -181,6 +181,10 @@ struct CoinSelectionParams { * and not counted as waste. Otherwise excess value will be be applied to fees and counted as waste. */ std::optional m_add_excess_to_recipient_position; + /*** + * amount that changeless spends can exceed the target amount. + */ + CAmount m_max_excess{0}; CoinSelectionParams(FastRandomContext& rng_fast, int change_output_size, int change_spend_size, @@ -452,7 +456,7 @@ struct SelectionResult int GetWeight() const { return m_weight; } }; -util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& max_excess, int max_selection_weight, const bool add_excess_to_target); util::Result CoinGrinder(std::vector& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 268fd16a906ed5..46a3a230ce7af5 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -544,6 +544,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact {"input_weights", UniValueType(UniValue::VARR)}, {"max_tx_weight", UniValueType(UniValue::VNUM)}, {"add_excess_to_recipient_position", UniValue::VNUM}, + {"max_excess", UniValueType()}, // will be checked by AmountFromValue() below }, true, true); @@ -633,6 +634,11 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact 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())); } } + + if (options.exists("max_excess")) { + coinControl.m_max_excess = CAmount(AmountFromValue(options["max_excess"])); + } + } } else { // if options is null and not a bool @@ -803,6 +809,7 @@ RPCHelpMan fundrawtransaction() "Transaction building will fail if this can not be satisfied."}, {"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"}, + {"max_excess", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to default wallet behavior"}, "Specify the maximum excess amount for changeless transactions in " + CURRENCY_UNIT + "."}, }, FundTxDoc()), RPCArgOptions{ @@ -1261,6 +1268,7 @@ RPCHelpMan send() "Transaction building will fail if this can not be satisfied."}, {"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."}, + {"max_excess", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to default wallet behavior"}, "Specify the maximum excess amount for changeless transactions in " + CURRENCY_UNIT + "."}, }, FundTxDoc()), RPCArgOptions{.oneline_description="options"}}, @@ -1725,6 +1733,7 @@ RPCHelpMan walletcreatefundedpsbt() "Transaction building will fail if this can not be satisfied."}, {"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."}, + {"max_excess", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to default wallet behavior"}, "Specify the maximum excess amount for changeless transactions in " + CURRENCY_UNIT + "."}, }, FundTxDoc()), RPCArgOptions{.oneline_description="options"}}, diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4aa53117706f9c..240641612a2a96 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -705,7 +705,7 @@ util::Result ChooseSelectionResult(interfaces::Chain& chain, co // 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_selection_weight, coin_selection_params.m_add_excess_to_recipient_position.has_value())}) { + if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_max_excess, max_selection_weight, coin_selection_params.m_add_excess_to_recipient_position.has_value())}) { results.push_back(*bnb_result); } else append_error(std::move(bnb_result)); } @@ -1119,6 +1119,12 @@ static util::Result CreateTransactionInternal( coin_selection_params.m_min_change_target = GenerateChangeTarget(std::floor(recipients_sum / vecSend.size()), coin_selection_params.m_change_fee, rng_fast); + if (coin_control.m_max_excess) { + coin_selection_params.m_max_excess = std::max(coin_control.m_max_excess.value(), coin_selection_params.m_cost_of_change); + } else { + coin_selection_params.m_max_excess = coin_selection_params.m_cost_of_change; + } + // The smallest change amount should be: // 1. at least equal to dust threshold // 2. at least 1 sat greater than fees to spend it at m_discard_feerate diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index fff6dc7a3e76f0..7d3465d68ce89e 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -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, /*add_excess_to_target=*/false); + selection_target, /*max_excess=*/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);