I see what is going on with this template type, but why not just use a boolean argument and return a PaymentDestination? I think this makes it more explicit as to what this function is doing. I found the two templated functions hard to follow.
<details>
<summary>Suggested diff</summary>
diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp
index 7123d5fde7..6b8ed82898 100644
--- a/src/rpc/rawtransaction_util.cpp
+++ b/src/rpc/rawtransaction_util.cpp
@@ -112,13 +112,11 @@ UniValue NormalizeOutputs(const UniValue& outputs_in)
return outputs;
}
-//! Parse normalized outputs, decoding each address with decode, which returns std::nullopt if the address is invalid
-template <typename Destination, typename Decode>
-static std::vector<std::pair<Destination, CAmount>> ParseOutputsWith(const UniValue& outputs, Decode decode)
+std::vector<std::pair<PaymentDestination, CAmount>> ParseOutputs(const UniValue& outputs, bool allow_silent_payments)
{
// Duplicate checking
- std::set<Destination> destinations;
- std::vector<std::pair<Destination, CAmount>> parsed_outputs;
+ std::set<PaymentDestination> destinations;
+ std::vector<std::pair<PaymentDestination, CAmount>> parsed_outputs;
bool has_data{false};
const auto& keys{outputs.getKeys()};
const auto& values{outputs.getValues()};
@@ -131,15 +129,18 @@ static std::vector<std::pair<Destination, CAmount>> ParseOutputsWith(const UniVa
}
has_data = true;
std::vector<unsigned char> data = ParseHexV(value.getValStr(), "Data");
- Destination destination{CNoDestination{CScript() << OP_RETURN << data}};
+ PaymentDestination destination{CNoDestination{CScript() << OP_RETURN << data}};
CAmount amount{0};
parsed_outputs.emplace_back(destination, amount);
} else {
- std::optional<Destination> destination{decode(name_)};
+ auto destination{PaymentDestination::FromString(name_)};
CAmount amount{AmountFromValue(value)};
if (!destination) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + name_);
}
+ if (!allow_silent_payments && destination->IsSilentPayment()) {
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Silent payments are not supported by this RPC");
+ }
if (!destinations.insert(*destination).second) {
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_);
@@ -150,32 +151,14 @@ static std::vector<std::pair<Destination, CAmount>> ParseOutputsWith(const UniVa
return parsed_outputs;
}
-std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs)
-{
- return ParseOutputsWith<CTxDestination>(outputs, [](const std::string& address) -> std::optional<CTxDestination> {
- CTxDestination destination{DecodeDestination(address)};
- if (!IsValidDestination(destination)) return std::nullopt;
- return destination;
- });
-}
-
-std::vector<std::pair<PaymentDestination, CAmount>> ParsePaymentOutputs(const UniValue& outputs)
-{
- return ParseOutputsWith<PaymentDestination>(outputs, [](const std::string& address) -> std::optional<PaymentDestination> {
- auto destination{PaymentDestination::FromString(address)};
- if (!destination) return std::nullopt;
- return std::move(*destination);
- });
-}
-
void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in)
{
UniValue outputs(UniValue::VOBJ);
outputs = NormalizeOutputs(outputs_in);
- std::vector<std::pair<CTxDestination, CAmount>> parsed_outputs = ParseOutputs(outputs);
+ std::vector<std::pair<PaymentDestination, CAmount>> parsed_outputs = ParseOutputs(outputs, /*allow_silent_payments=*/false);
for (const auto& [destination, nAmount] : parsed_outputs) {
- CScript scriptPubKey = GetScriptForDestination(destination);
+ CScript scriptPubKey = *CHECK_NONFATAL(destination.GetStaticScript());
CTxOut out(nAmount, scriptPubKey);
rawTx.vout.push_back(out);
diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h
index 3850285cc4..449a5e2216 100644
--- a/src/rpc/rawtransaction_util.h
+++ b/src/rpc/rawtransaction_util.h
@@ -52,11 +52,8 @@ void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, bool rbf);
/** Normalize univalue-represented outputs */
UniValue NormalizeOutputs(const UniValue& outputs_in);
-/** Parse normalized outputs into destination, amount tuples. Silent payments addresses are rejected. */
-std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs);
-
-/** Parse normalized outputs into destination, amount tuples, accepting silent payments addresses */
-std::vector<std::pair<PaymentDestination, CAmount>> ParsePaymentOutputs(const UniValue& outputs);
+/** Parse normalized outputs into destination, amount tuples. Silent payments addresses are rejected unless allow_silent_payments i
s true. */
+std::vector<std::pair<PaymentDestination, CAmount>> ParseOutputs(const UniValue& outputs, bool allow_silent_payments);
/** Normalize, parse, and add outputs to the transaction */
void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in);
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 776ac12a35..2684b4ce70 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -33,13 +33,12 @@ using common::TransactionErrorString;
using node::TransactionError;
namespace wallet {
-template <typename Destination>
-static std::vector<CRecipient> CreateRecipients(const std::vector<std::pair<Destination, CAmount>>& outputs, const std::set<int>& s
ubtract_fee_outputs)
+std::vector<CRecipient> CreateRecipients(const std::vector<std::pair<PaymentDestination, CAmount>>& outputs, const std::set<int>& s
ubtract_fee_outputs)
{
std::vector<CRecipient> recipients;
for (size_t i = 0; i < outputs.size(); ++i) {
const auto& [destination, amount] = outputs.at(i);
- CRecipient recipient{PaymentDestination{destination}, amount, subtract_fee_outputs.contains(i)};
+ CRecipient recipient{destination, amount, subtract_fee_outputs.contains(i)};
recipients.push_back(recipient);
}
return recipients;
@@ -371,7 +370,7 @@ RPCMethod sendtoaddress()
sffo_set.insert(0);
}
- std::vector<CRecipient> recipients{CreateRecipients(ParsePaymentOutputs(address_amounts), sffo_set)};
+ std::vector<CRecipient> recipients{CreateRecipients(ParseOutputs(address_amounts, /*allow_silent_payments=*/true), sffo_set)};
const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()};
return SendMoney(*pwallet, coin_control, recipients, comment, comment_to, verbose);
@@ -465,7 +464,7 @@ RPCMethod 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(
- ParsePaymentOutputs(sendTo),
+ ParseOutputs(sendTo, /*allow_silent_payments=*/true),
InterpretSubtractFeeFromOutputInstructions(request.params[4], sendTo.getKeys())
);
const bool verbose{request.params[9].isNull() ? false : request.params[9].get_bool()};
@@ -856,11 +855,11 @@ RPCMethod fundrawtransaction()
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
}
UniValue options = request.params[1];
- std::vector<std::pair<CTxDestination, CAmount>> destinations;
+ std::vector<std::pair<PaymentDestination, CAmount>> destinations;
for (const auto& tx_out : tx.vout) {
CTxDestination dest;
ExtractDestination(tx_out.scriptPubKey, dest);
- destinations.emplace_back(dest, tx_out.nValue);
+ destinations.emplace_back(PaymentDestination{dest}, tx_out.nValue);
}
std::vector<std::string> dummy(destinations.size(), "dummy");
std::vector<CRecipient> recipients = CreateRecipients(
@@ -1131,14 +1130,7 @@ static RPCMethod bumpfee_helper(std::string method_name)
// inputs, and the transaction must be added to the wallet when it is created, see
// EnsureSilentPaymentsTxIsAddedToWallet. A PSBT may be signed elsewhere and its inputs
// may change before signing, so only allow silent payments addresses for bumpfee RPC.
- const UniValue normalized_outputs{NormalizeOutputs(options["outputs"])};
- if (want_psbt) {
- for (auto& [dest, amount] : ParseOutputs(normalized_outputs)) {
- outputs.emplace_back(PaymentDestination{std::move(dest)}, amount);
- }
- } else {
- outputs = ParsePaymentOutputs(normalized_outputs);
- }
+ outputs = ParseOutputs(NormalizeOutputs(options["outputs"]), /*allow_silent_payments=*/!want_psbt);
}
if (options.exists("original_change_index")) {
@@ -1336,7 +1328,7 @@ RPCMethod send()
UniValue outputs(UniValue::VOBJ);
outputs = NormalizeOutputs(request.params[0]);
std::vector<CRecipient> recipients = CreateRecipients(
- ParsePaymentOutputs(outputs),
+ ParseOutputs(outputs, /*allow_silent_payments=*/true),
InterpretSubtractFeeFromOutputInstructions(options["subtract_fee_from_outputs"], outputs.getKeys())
);
if (std::ranges::any_of(recipients, [](const auto& r) { return r.dest.IsSilentPayment(); })) {
@@ -1525,7 +1517,7 @@ RPCMethod sendall()
CMutableTransaction rawTx{ConstructTransaction(options["inputs"], /*outputs_in=*/std::nullopt, options["locktime"], rbf
, coin_control.m_version)};
// Output i pays to the address in key i of the normalized outputs
const UniValue outputs{NormalizeOutputs(recipient_key_value_pairs)};
- const auto parsed_outputs{ParsePaymentOutputs(outputs)};
+ const auto parsed_outputs{ParseOutputs(outputs, /*allow_silent_payments=*/true)};
std::map<size_t, SilentPaymentsDestination> sp_destinations;
for (size_t i = 0; i < parsed_outputs.size(); ++i) {
if (const auto* sp = parsed_outputs[i].first.GetSilentPaymentsDestination()) {
@@ -1895,7 +1887,7 @@ RPCMethod walletcreatefundedpsbt()
UniValue outputs(UniValue::VOBJ);
outputs = NormalizeOutputs(request.params[1]);
std::vector<CRecipient> recipients = CreateRecipients(
- ParseOutputs(outputs),
+ ParseOutputs(outputs, /*allow_silent_payments=*/false),
InterpretSubtractFeeFromOutputInstructions(options["subtractFeeFromOutputs"], outputs.getKeys())
);
// Automatically select coins, unless at least one is manually selected. Can
</details>
