Skip to content

Commit 4833935

Browse files
luke-jrThomas Kerin
luke-jr
authored and
Thomas Kerin
committed
RPC: Use options object rather than adding a "sort" boolean for multisig methods
Also add accounts parameter to vRPCConvertParams (required by RPC mappings test)
1 parent 7f9e05c commit 4833935

File tree

5 files changed

+55
-19
lines changed

5 files changed

+55
-19
lines changed

‎src/rpc/client.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,11 @@ static const CRPCConvertParam vRPCConvertParams[] =
7676
{ "sendmany", 6 , "conf_target" },
7777
{ "addmultisigaddress", 0, "nrequired" },
7878
{ "addmultisigaddress", 1, "keys" },
79-
{ "addmultisigaddress", 3, "sort" },
79+
{ "addmultisigaddress", 2, "options" },
80+
{ "addmultisigaddress", 2, "account" },
8081
{ "createmultisig", 0, "nrequired" },
8182
{ "createmultisig", 1, "keys" },
82-
{ "createmultisig", 2, "sort" },
83+
{ "createmultisig", 2, "options" },
8384
{ "listunspent", 0, "minconf" },
8485
{ "listunspent", 1, "maxconf" },
8586
{ "listunspent", 2, "addresses" },

‎src/rpc/misc.cpp‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ UniValue createmultisig(const JSONRPCRequest& request)
282282

283283
if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
284284
{
285-
std::string msg = "createmultisig nrequired [\"key\",...] ( sort )\n"
285+
std::string msg = "createmultisig nrequired [\"key\",...] ( options )\n"
286286
"\nCreates a multi-signature address with n signature of m keys required.\n"
287287
"It returns a json object with the address and redeemScript.\n"
288288
"Public keys can be sorted according to BIP67 during the request if required.\n"
@@ -294,7 +294,10 @@ UniValue createmultisig(const JSONRPCRequest& request)
294294
" \"key\" (string) bitcoin address or hex-encoded public key\n"
295295
" ,...\n"
296296
" ]\n"
297-
"3. sort (bool, optional) Whether to sort public keys according to BIP67. Default setting is false.\n"
297+
"3. options (object, optional)\n"
298+
" {\n"
299+
" \"sort\" (bool, optional, default=false) Whether to sort public keys according to BIP67.\n"
300+
" }\n"
298301

299302
"\nResult:\n"
300303
"{\n"
@@ -311,7 +314,21 @@ UniValue createmultisig(const JSONRPCRequest& request)
311314
throw std::runtime_error(msg);
312315
}
313316

314-
bool fSorted = request.params.size() > 2 && request.params[2].get_bool();
317+
bool fSorted = false;
318+
319+
if (request.params.size() > 2) {
320+
const UniValue& options = request.params[2];
321+
RPCTypeCheckArgument(options, UniValue::VOBJ);
322+
RPCTypeCheckObj(options,
323+
{
324+
{"sort", UniValueType(UniValue::VBOOL)},
325+
},
326+
true, true);
327+
328+
if (options.exists("sort")) {
329+
fSorted = options["sort"].get_bool();
330+
}
331+
}
315332

316333
// Construct using pay-to-script-hash:
317334
CScript inner = _createmultisig_redeemScript(pwallet, request.params, fSorted);
@@ -613,7 +630,7 @@ static const CRPCCommand commands[] =
613630
// --------------------- ------------------------ ----------------------- ----------
614631
{ "control", "getmemoryinfo", &getmemoryinfo, {"mode"} },
615632
{ "util", "validateaddress", &validateaddress, {"address"} }, /* uses wallet if enabled */
616-
{ "util", "createmultisig", &createmultisig, {"nrequired","keys","sort"} },
633+
{ "util", "createmultisig", &createmultisig, {"nrequired","keys","options"} },
617634
{ "util", "verifymessage", &verifymessage, {"address","signature","message"} },
618635
{ "util", "signmessagewithprivkey", &signmessagewithprivkey, {"privkey","message"} },
619636

‎src/wallet/rpcwallet.cpp‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,9 +1107,9 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
11071107
return NullUniValue;
11081108
}
11091109

1110-
if (request.fHelp || request.params.size() < 2 || request.params.size() > 4)
1110+
if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
11111111
{
1112-
std::string msg = "addmultisigaddress nrequired [\"key\",...] ( \"account\" ) ( sort )\n"
1112+
std::string msg = "addmultisigaddress nrequired [\"key\",...] ( \"account\" or options )\n"
11131113
"\nAdd a nrequired-to-sign multisignature address to the wallet.\n"
11141114
"Each key is a Bitcoin address or hex-encoded public key.\n"
11151115
"If 'account' is specified (DEPRECATED), assign address to that account.\n"
@@ -1122,8 +1122,10 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
11221122
" \"address\" (string) bitcoin address or hex-encoded public key\n"
11231123
" ...,\n"
11241124
" ]\n"
1125-
"3. \"account\" (string, optional) DEPRECATED. An account to assign the addresses to.\n"
1126-
"4. sort (bool, optional) Whether to sort public keys according to BIP67. Default setting is false.\n"
1125+
"3. options (object, optional)\n"
1126+
" {\n"
1127+
" \"sort\" (bool, optional, default=false) Whether to sort public keys according to BIP67.\n"
1128+
" }\n"
11271129

11281130
"\nResult:\n"
11291131
"\"address\" (string) A bitcoin address associated with the keys.\n"
@@ -1140,10 +1142,26 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
11401142
LOCK2(cs_main, pwallet->cs_wallet);
11411143

11421144
std::string strAccount;
1143-
if (!request.params[2].isNull())
1144-
strAccount = AccountFromValue(request.params[2]);
1145+
bool fSorted = false;
1146+
1147+
if (request.params.size() > 2) {
1148+
if (request.params[2].type() == UniValue::VSTR) {
1149+
// Backward compatibility
1150+
strAccount = AccountFromValue(request.params[2]);
1151+
} else {
1152+
const UniValue& options = request.params[2];
1153+
RPCTypeCheckArgument(options, UniValue::VOBJ);
1154+
RPCTypeCheckObj(options,
1155+
{
1156+
{"sort", UniValueType(UniValue::VBOOL)},
1157+
},
1158+
true, true);
11451159

1146-
bool fSorted = request.params.size() > 3 && request.params[3].get_bool();
1160+
if (options.exists("sort")) {
1161+
fSorted = options["sort"].get_bool();
1162+
}
1163+
}
1164+
}
11471165

11481166
// Construct using pay-to-script-hash:
11491167
CScript inner = _createmultisig_redeemScript(pwallet, request.params, fSorted);
@@ -3234,7 +3252,7 @@ static const CRPCCommand commands[] =
32343252
{ "hidden", "resendwallettransactions", &resendwallettransactions, {} },
32353253
{ "wallet", "abandontransaction", &abandontransaction, {"txid"} },
32363254
{ "wallet", "abortrescan", &abortrescan, {} },
3237-
{ "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","account","sort"} },
3255+
{ "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","options|account"} },
32383256
{ "wallet", "addwitnessaddress", &addwitnessaddress, {"address","p2sh"} },
32393257
{ "wallet", "backupwallet", &backupwallet, {"destination"} },
32403258
{ "wallet", "bumpfee", &bumpfee, {"txid", "options"} },

‎test/functional/sort_multisig.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ def run_test(self):
2222
pubs = [pub1,pub2,pub3]
2323

2424
default = self.nodes[0].createmultisig(2, pubs)
25-
unsorted = self.nodes[0].createmultisig(2, pubs, False)
25+
unsorted = self.nodes[0].createmultisig(2, pubs, {"sort": False})
2626

2727
assert_equal("2N2BchzwfyuqJep7sKmFfBucfopHZQuPnpt", unsorted["address"])
2828
assert_equal("5221022df8750480ad5b26950b25c7ba79d3e37d75f640f8e5d9bcd5b150a0f85014da2103e3818b65bcc73a7d64064106a859cc1a5a728c4345ff0b641209fba0d90de6e921021f2f6e1e50cb6a953935c3601284925decd3fd21bc445712576873fb8c6ebc1853ae", unsorted["redeemScript"])
2929
assert_equal(default["address"], unsorted["address"])
3030
assert_equal(default["redeemScript"], unsorted["redeemScript"])
3131

32-
sorted = self.nodes[0].createmultisig(2, pubs, True)
32+
sorted = self.nodes[0].createmultisig(2, pubs, {"sort": True})
3333
assert_equal("2NFd5JqpwmQNz3gevZJ3rz9ofuHvqaP9Cye", sorted["address"])
3434
assert_equal("5221021f2f6e1e50cb6a953935c3601284925decd3fd21bc445712576873fb8c6ebc1821022df8750480ad5b26950b25c7ba79d3e37d75f640f8e5d9bcd5b150a0f85014da2103e3818b65bcc73a7d64064106a859cc1a5a728c4345ff0b641209fba0d90de6e953ae", sorted["redeemScript"])
3535

‎test/functional/wallet-accounts.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ def test_sort_multisig(self, node):
3333
"n2yWMtx8jVbo8wv9BK2eN1LdbaakgKL3Mt",
3434
]
3535

36-
sorted_default = node.addmultisigaddress(2, addresses, "sort-test")
37-
sorted_false = node.addmultisigaddress(2, addresses, "sort-test", False)
38-
sorted_true = node.addmultisigaddress(2, addresses, "sort-test", True)
36+
sorted_default = node.addmultisigaddress(2, addresses)
37+
sorted_false = node.addmultisigaddress(2, addresses, {"sort": False})
38+
sorted_true = node.addmultisigaddress(2, addresses, {"sort": True})
3939

4040
assert_equal(sorted_default, sorted_false)
4141
assert_equal("2N6dne8yzh13wsRJxCcMgCYNeN9fxKWNHt8", sorted_default)

0 commit comments

Comments
 (0)