refactor: simplify CreateRecipients

Move validation logic out of `CreateRecipients` and instead take the
already validated outputs from `ParseOutputs` as an input.

Move SFFO parsing out of `CreateRecipients` into a new function,
`InterpretSubtractFeeFromOutputsInstructions`. This takes the SFFO instructions
from `sendmany` and `sendtoaddress` and turns them into a set of integers.
In a later commit, we will also move the SFFO parsing logic from
`FundTransaction` into this function.

Worth noting: a user can pass duplicate addresses and addresses that dont exist
in the transaction outputs as SFFO args to `sendmany` and `sendtoaddress`
without triggering a warning. This behavior is preserved in to keep this commit
strictly a refactor.
This commit is contained in:
josibake 2023-10-02 14:36:53 +02:00
parent 47353a608d
commit 5ad19668db
No known key found for this signature in database
GPG key ID: 8ADCB558C4F33D65

View file

@ -24,33 +24,12 @@
namespace wallet {
std::vector<CRecipient> CreateRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs)
std::vector<CRecipient> CreateRecipients(const std::vector<std::pair<CTxDestination, CAmount>>& outputs, const std::set<int>& subtract_fee_outputs)
{
std::vector<CRecipient> recipients;
std::set<CTxDestination> destinations;
int i = 0;
for (const std::string& address: address_amounts.getKeys()) {
CTxDestination dest = DecodeDestination(address);
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + address);
}
if (destinations.count(dest)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address);
}
destinations.insert(dest);
CAmount amount = AmountFromValue(address_amounts[i++]);
bool subtract_fee = false;
for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) {
const UniValue& addr = subtract_fee_outputs[idx];
if (addr.get_str() == address) {
subtract_fee = true;
}
}
CRecipient recipient = {dest, amount, subtract_fee};
for (size_t i = 0; i < outputs.size(); ++i) {
const auto& [destination, amount] = outputs.at(i);
CRecipient recipient{destination, amount, subtract_fee_outputs.contains(i)};
recipients.push_back(recipient);
}
return recipients;
@ -78,6 +57,27 @@ static void InterpretFeeEstimationInstructions(const UniValue& conf_target, cons
}
}
std::set<int> InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_instructions, const std::vector<std::string>& destinations)
{
std::set<int> sffo_set;
if (sffo_instructions.isNull()) return sffo_set;
if (sffo_instructions.isBool()) {
if (sffo_instructions.get_bool()) sffo_set.insert(0);
return sffo_set;
}
for (const auto& sffo : sffo_instructions.getValues()) {
if (sffo.isStr()) {
for (size_t i = 0; i < destinations.size(); ++i) {
if (sffo.get_str() == destinations.at(i)) {
sffo_set.insert(i);
break;
}
}
}
}
return sffo_set;
}
static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const UniValue& options, const CMutableTransaction& rawTx)
{
// Make a blank psbt
@ -277,11 +277,6 @@ RPCHelpMan sendtoaddress()
if (!request.params[3].isNull() && !request.params[3].get_str().empty())
mapValue["to"] = request.params[3].get_str();
bool fSubtractFeeFromAmount = false;
if (!request.params[4].isNull()) {
fSubtractFeeFromAmount = request.params[4].get_bool();
}
CCoinControl coin_control;
if (!request.params[5].isNull()) {
coin_control.m_signal_bip125_rbf = request.params[5].get_bool();
@ -298,12 +293,10 @@ RPCHelpMan sendtoaddress()
UniValue address_amounts(UniValue::VOBJ);
const std::string address = request.params[0].get_str();
address_amounts.pushKV(address, request.params[1]);
UniValue subtractFeeFromAmount(UniValue::VARR);
if (fSubtractFeeFromAmount) {
subtractFeeFromAmount.push_back(address);
}
std::vector<CRecipient> recipients = CreateRecipients(address_amounts, subtractFeeFromAmount);
std::vector<CRecipient> recipients = CreateRecipients(
ParseOutputs(address_amounts),
InterpretSubtractFeeFromOutputInstructions(request.params[4], address_amounts.getKeys())
);
const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()};
return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose);
@ -387,10 +380,6 @@ RPCHelpMan sendmany()
if (!request.params[3].isNull() && !request.params[3].get_str().empty())
mapValue["comment"] = request.params[3].get_str();
UniValue subtractFeeFromAmount(UniValue::VARR);
if (!request.params[4].isNull())
subtractFeeFromAmount = request.params[4].get_array();
CCoinControl coin_control;
if (!request.params[5].isNull()) {
coin_control.m_signal_bip125_rbf = request.params[5].get_bool();
@ -398,7 +387,10 @@ RPCHelpMan sendmany()
SetFeeEstimateMode(*pwallet, coin_control, /*conf_target=*/request.params[6], /*estimate_mode=*/request.params[7], /*fee_rate=*/request.params[8], /*override_min_fee=*/false);
std::vector<CRecipient> recipients = CreateRecipients(sendTo, subtractFeeFromAmount);
std::vector<CRecipient> recipients = CreateRecipients(
ParseOutputs(sendTo),
InterpretSubtractFeeFromOutputInstructions(request.params[4], sendTo.getKeys())
);
const bool verbose{request.params[9].isNull() ? false : request.params[9].get_bool()};
return SendMoney(*pwallet, coin_control, recipients, std::move(mapValue), verbose);