Skip to content

Commit

Permalink
wallet: Add coin control option "max_excess"
Browse files Browse the repository at this point in the history
If set, excess from changeless spends can not exceed this amount, otherwise use cost_of_change by default
  • Loading branch information
remyers committed Aug 23, 2024
1 parent 3d334f4 commit 96a395f
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 6 deletions.
3 changes: 3 additions & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand All @@ -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" },
Expand Down Expand Up @@ -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"},
Expand Down
2 changes: 2 additions & 0 deletions src/wallet/coincontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ class CCoinControl
std::optional<int> 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<uint32_t> 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<CAmount> m_max_excess;

CCoinControl();

Expand Down
6 changes: 3 additions & 3 deletions src/wallet/coinselection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ struct {

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,
util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& 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);
Expand Down Expand Up @@ -126,7 +126,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& 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
Expand Down Expand Up @@ -199,7 +199,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& 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;
Expand Down
6 changes: 5 additions & 1 deletion src/wallet/coinselection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t> 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,
Expand Down Expand Up @@ -452,7 +456,7 @@ struct SelectionResult
int GetWeight() const { return m_weight; }
};

util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& max_excess,
int max_selection_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_selection_weight);
Expand Down
9 changes: 9 additions & 0 deletions src/wallet/rpc/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"}},
Expand Down Expand Up @@ -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"}},
Expand Down
8 changes: 7 additions & 1 deletion src/wallet/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ util::Result<SelectionResult> 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));
}
Expand Down Expand Up @@ -1119,6 +1119,12 @@ static util::Result<CreatedTransactionResult> 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 = coin_control.m_max_excess.value();
} 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
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/coinselector_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 96a395f

Please sign in to comment.