diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index ec0bf29c3b1..0cba830e2ab 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -68,29 +68,26 @@ std::set InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_in { std::set 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()) { + int pos{-1}; if (sffo.isStr()) { - for (size_t i = 0; i < destinations.size(); ++i) { - if (sffo.get_str() == destinations.at(i)) { - sffo_set.insert(i); - break; - } - } - } - if (sffo.isNum()) { - int pos = sffo.getInt(); - if (sffo_set.contains(pos)) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos)); - if (pos < 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos)); - if (pos >= int(destinations.size())) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos)); - sffo_set.insert(pos); + auto it = find(destinations.begin(), destinations.end(), sffo.get_str()); + if (it == destinations.end()) throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter 'subtract fee from output', destination %s not found in tx outputs", sffo.get_str())); + pos = it - destinations.begin(); + } else if (sffo.isNum()) { + pos = sffo.getInt(); + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter 'subtract fee from output', invalid value type: %s", uvTypeName(sffo.type()))); } + + if (sffo_set.contains(pos)) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter 'subtract fee from output', duplicated position: %d", pos)); + if (pos < 0) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter 'subtract fee from output', negative position: %d", pos)); + if (pos >= int(destinations.size())) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter 'subtract fee from output', position too large: %d", pos)); + sffo_set.insert(pos); } return sffo_set; } @@ -310,10 +307,13 @@ RPCHelpMan sendtoaddress() UniValue address_amounts(UniValue::VOBJ); const std::string address = request.params[0].get_str(); address_amounts.pushKV(address, request.params[1]); - std::vector recipients = CreateRecipients( - ParseOutputs(address_amounts), - InterpretSubtractFeeFromOutputInstructions(request.params[4], address_amounts.getKeys()) - ); + + std::set sffo_set; + if (!request.params[4].isNull() && request.params[4].get_bool()) { + sffo_set.insert(0); + } + + std::vector recipients{CreateRecipients(ParseOutputs(address_amounts), sffo_set)}; const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose); diff --git a/test/functional/wallet_sendmany.py b/test/functional/wallet_sendmany.py index ad99100590b..e338ba7b649 100755 --- a/test/functional/wallet_sendmany.py +++ b/test/functional/wallet_sendmany.py @@ -5,17 +5,17 @@ """Test the sendmany RPC command.""" from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error + class SendmanyTest(BitcoinTestFramework): # Setup and helpers def add_options(self, parser): self.add_wallet_options(parser) - def skip_test_if_missing_module(self): self.skip_if_no_wallet() - def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True @@ -26,9 +26,19 @@ class SendmanyTest(BitcoinTestFramework): addr_3 = self.wallet.getnewaddress() self.log.info("Test using duplicate address in SFFO argument") - self.def_wallet.sendmany(dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_1, addr_1, addr_1]) + assert_raises_rpc_error(-8, "Invalid parameter 'subtract fee from output', duplicated position: 0", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_1, addr_1, addr_1]) self.log.info("Test using address not present in tx.vout in SFFO argument") - self.def_wallet.sendmany(dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_3]) + assert_raises_rpc_error(-8, f"Invalid parameter 'subtract fee from output', destination {addr_3} not found in tx outputs", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_3]) + self.log.info("Test using negative index in SFFO argument") + assert_raises_rpc_error(-8, "Invalid parameter 'subtract fee from output', negative position: -5", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[-5]) + self.log.info("Test using an out of bounds index in SFFO argument") + assert_raises_rpc_error(-8, "Invalid parameter 'subtract fee from output', position too large: 5", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[5]) + self.log.info("Test using an unexpected type in SFFO argument") + assert_raises_rpc_error(-8, "Invalid parameter 'subtract fee from output', invalid value type: bool", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[False]) + self.log.info("Test duplicates in SFFO argument, mix string destinations with numeric indexes") + assert_raises_rpc_error(-8, "Invalid parameter 'subtract fee from output', duplicated position: 0", self.def_wallet.sendmany, dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[0, addr_1]) + self.log.info("Test valid mixing of string destinations with numeric indexes in SFFO argument") + self.def_wallet.sendmany(dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[0, addr_2]) def run_test(self): self.nodes[0].createwallet("activewallet")