From fa5c5cd141f0265a5693234690ac757b811157d8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Apr 2019 09:09:50 -0400 Subject: [PATCH 1/2] rpc: Switch touched RPCs to IsValidNumArgs --- src/rpc/rawtransaction.cpp | 20 ++++++++++++-------- src/wallet/rpcwallet.cpp | 10 ++++++---- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 1a7216cee..cebdc5da9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -431,9 +431,7 @@ static UniValue createrawtransaction(const JSONRPCRequest& request) static UniValue decoderawtransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) - throw std::runtime_error( - RPCHelpMan{"decoderawtransaction", + const RPCHelpMan help{"decoderawtransaction", "\nReturn a JSON object representing the serialized, hex-encoded transaction.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction hex string"}, @@ -485,7 +483,11 @@ static UniValue decoderawtransaction(const JSONRPCRequest& request) HelpExampleCli("decoderawtransaction", "\"hexstring\"") + HelpExampleRpc("decoderawtransaction", "\"hexstring\"") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL}); @@ -1415,9 +1417,7 @@ UniValue createpsbt(const JSONRPCRequest& request) UniValue converttopsbt(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) - throw std::runtime_error( - RPCHelpMan{"converttopsbt", + const RPCHelpMan help{"converttopsbt", "\nConverts a network serialized transaction to a PSBT. This should be used only with createrawtransaction and fundrawtransaction\n" "createpsbt and walletcreatefundedpsbt should be used for new applications.\n", { @@ -1438,7 +1438,11 @@ UniValue converttopsbt(const JSONRPCRequest& request) "\nConvert the transaction to a PSBT\n" + HelpExampleCli("converttopsbt", "\"rawtransaction\"") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL, UniValue::VBOOL}, true); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7de74bac4..6cb3deb59 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2991,9 +2991,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) - throw std::runtime_error( - RPCHelpMan{"fundrawtransaction", + const RPCHelpMan help{"fundrawtransaction", "\nAdd inputs to a transaction until it has enough in value to meet its out value.\n" "This will not modify existing inputs, and will add at most one change output to the outputs.\n" "No existing outputs will be modified unless \"subtractFeeFromOutputs\" is specified.\n" @@ -3052,7 +3050,11 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) "\nSend the transaction\n" + HelpExampleCli("sendrawtransaction", "\"signedtransactionhex\"") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType(), UniValue::VBOOL}); From fa499b5f027f77c0bf13699852c8c06f78e27bef Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Apr 2019 09:04:08 -0400 Subject: [PATCH 2/2] rpc: bugfix: Properly use iswitness in converttopsbt Also explain the param in all RPCs --- src/rpc/rawtransaction.cpp | 25 ++++++++++++++++--------- src/wallet/rpcwallet.cpp | 9 +++++++-- test/functional/rpc_psbt.py | 5 +++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index cebdc5da9..4b1398d6a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -435,8 +435,13 @@ static UniValue decoderawtransaction(const JSONRPCRequest& request) "\nReturn a JSON object representing the serialized, hex-encoded transaction.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction hex string"}, - {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction\n" - " If iswitness is not present, heuristic tests will be used in decoding"}, + {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction.\n" + "If iswitness is not present, heuristic tests will be used in decoding.\n" + "If true, only witness deserialization will be tried.\n" + "If false, only non-witness deserialization will be tried.\n" + "This boolean should reflect whether the transaction has inputs\n" + "(e.g. fully valid, or on-chain transactions), if known by the caller." + }, }, RPCResult{ "{\n" @@ -1422,12 +1427,15 @@ UniValue converttopsbt(const JSONRPCRequest& request) "createpsbt and walletcreatefundedpsbt should be used for new applications.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of a raw transaction"}, - {"permitsigdata", RPCArg::Type::BOOL, /* default */ "false", "If true, any signatures in the input will be discarded and conversion.\n" + {"permitsigdata", RPCArg::Type::BOOL, /* default */ "false", "If true, any signatures in the input will be discarded and conversion\n" " will continue. If false, RPC will fail if any signatures are present."}, {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction.\n" - " If iswitness is not present, heuristic tests will be used in decoding. If true, only witness deserializaion\n" - " will be tried. If false, only non-witness deserialization will be tried. Only has an effect if\n" - " permitsigdata is true."}, + "If iswitness is not present, heuristic tests will be used in decoding.\n" + "If true, only witness deserialization will be tried.\n" + "If false, only non-witness deserialization will be tried.\n" + "This boolean should reflect whether the transaction has inputs\n" + "(e.g. fully valid, or on-chain transactions), if known by the caller." + }, }, RPCResult{ " \"psbt\" (string) The resulting raw transaction (base64-encoded string)\n" @@ -1444,7 +1452,6 @@ UniValue converttopsbt(const JSONRPCRequest& request) throw std::runtime_error(help.ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL, UniValue::VBOOL}, true); // parse hex string from parameter @@ -1452,8 +1459,8 @@ UniValue converttopsbt(const JSONRPCRequest& request) bool permitsigdata = request.params[1].isNull() ? false : request.params[1].get_bool(); bool witness_specified = !request.params[2].isNull(); bool iswitness = witness_specified ? request.params[2].get_bool() : false; - bool try_witness = permitsigdata ? (witness_specified ? iswitness : true) : false; - bool try_no_witness = permitsigdata ? (witness_specified ? !iswitness : true) : true; + const bool try_witness = witness_specified ? iswitness : true; + const bool try_no_witness = witness_specified ? !iswitness : true; if (!DecodeHexTx(tx, request.params[0].get_str(), try_no_witness, try_witness)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6cb3deb59..96f1051f1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3030,8 +3030,13 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) " \"CONSERVATIVE\""}, }, "options"}, - {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction \n" - " If iswitness is not present, heuristic tests will be used in decoding"}, + {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction.\n" + "If iswitness is not present, heuristic tests will be used in decoding.\n" + "If true, only witness deserialization will be tried.\n" + "If false, only non-witness deserialization will be tried.\n" + "This boolean should reflect whether the transaction has inputs\n" + "(e.g. fully valid, or on-chain transactions), if known by the caller." + }, }, RPCResult{ "{\n" diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 8a7ea7aa5..430be6dd3 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -152,9 +152,10 @@ class PSBTTest(BitcoinTestFramework): # Make sure that a non-psbt with signatures cannot be converted # Error could be either "TX decode failed" (segwit inputs causes parsing to fail) or "Inputs must not have scriptSigs and scriptWitnesses" + # We must set iswitness=True because the serialized transaction has inputs and is therefore a witness transaction signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex']) - assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, signedtx['hex']) - assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, signedtx['hex'], False) + assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, hexstring=signedtx['hex'], iswitness=True) + assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, hexstring=signedtx['hex'], permitsigdata=False, iswitness=True) # Unless we allow it to convert and strip signatures self.nodes[0].converttopsbt(signedtx['hex'], True)