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 May 17, 2024
1 parent 6f594a7 commit fc9c153
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 8 deletions.
3 changes: 3 additions & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "fundrawtransaction", 1, "change_target"},
{ "fundrawtransaction", 1, "enable_algos"},
{ "fundrawtransaction", 1, "add_excess_to_recipient_position"},
{ "fundrawtransaction", 1, "max_excess"},
{ "fundrawtransaction", 2, "iswitness" },
{ "walletcreatefundedpsbt", 0, "inputs" },
{ "walletcreatefundedpsbt", 1, "outputs" },
Expand All @@ -170,6 +171,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "walletcreatefundedpsbt", 3, "change_target"},
{ "walletcreatefundedpsbt", 3, "enable_algos"},
{ "walletcreatefundedpsbt", 3, "add_excess_to_recipient_position"},
{ "walletcreatefundedpsbt", 3, "max_excess"},
{ "walletcreatefundedpsbt", 4, "bip32derivs" },
{ "walletprocesspsbt", 1, "sign" },
{ "walletprocesspsbt", 3, "bip32derivs" },
Expand Down Expand Up @@ -217,6 +219,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "send", 4, "change_target"},
{ "send", 4, "enable_algos"},
{ "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 @@ -123,6 +123,8 @@ class CCoinControl
std::bitset<size_t(SelectionAlgorithm::NUM_ELEMENTS)> m_enable_algos{std::numeric_limits<size_t>::max()};
//! 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_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_weight) { // Exceeding weight for standard tx, cannot find more solutions by adding more inputs
Expand Down Expand Up @@ -200,7 +200,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
auto excess = result.ResetTargetToSelectedValue();
best_waste -= excess;
}
result.ComputeAndSetWaste(cost_of_change, cost_of_change, CAmount{0});
result.ComputeAndSetWaste(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 @@ -195,6 +195,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, size_t change_output_size, size_t change_spend_size,
CAmount min_change_target, CFeeRate effective_feerate,
Expand Down Expand Up @@ -461,7 +465,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_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);
Expand Down
15 changes: 12 additions & 3 deletions src/wallet/rpc/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
{"change_target", UniValueType()}, // will be checked by AmountFromValue() below
{"enable_algos", UniValueType(UniValue::VARR)},
{"add_excess_to_recipient_position", UniValue::VNUM},
{"max_excess", UniValueType()}, // will be checked by AmountFromValue() below
},
true, true);

Expand Down Expand Up @@ -621,6 +622,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
}
SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee);

if (options.exists("change_target")) {
coinControl.m_change_target = CAmount(AmountFromValue(options["change_target"]));
}

if (options.exists("enable_algos")) {
coinControl.m_enable_algos.reset();
for (const UniValue& algo_name : options["enable_algos"].get_array().getValues()) {
Expand All @@ -640,6 +645,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
}
}

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 @@ -724,9 +733,6 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
coinControl.SetInputWeight(COutPoint(txid, vout), weight);
}
}
if (options.exists("change_target")) {
coinControl.m_change_target = CAmount(AmountFromValue(options["change_target"]));
}

if (recipients.empty())
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");
Expand Down Expand Up @@ -820,6 +826,7 @@ RPCHelpMan fundrawtransaction()
}
},
{"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 @@ -1281,6 +1288,7 @@ RPCHelpMan send()
}
},
{"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 @@ -1737,6 +1745,7 @@ RPCHelpMan walletcreatefundedpsbt()
}
},
{"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 @@ -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, 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_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));
}
Expand Down Expand Up @@ -1089,6 +1089,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

0 comments on commit fc9c153

Please sign in to comment.