From 2435b974ee49473726aaffb561929024c9a8c9ac Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Wed, 15 Feb 2023 23:46:14 -0700 Subject: [PATCH] Enrich zcash-cli arg conversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of storing the indices of args to convert from string, store two `vector` (per operation), the first containing an entry for each required parameter (`true` if we should convert it), and the second containing an entry for each optional parameter. This allows us to check a few more things on the client side: - does the operation exist - have enough arguments been passed - have too many arguments been passed This is ostensibly a fix for `zcash-cli` to be able to use `asOfHeight` where available, but it also caught a few bugs in the old implementation: - `submitblock` didn’t convert its optional (but ignored) second arg; - `z_getpaymentdisclosure` docs claimed all the args were strings, but two are actually ints;` - `listreceivedbyaddress` didn’t convert the optional `includeImmatureCodebase`; - `listsinceblock` didn’t convert the optional `includeRemoved` and `includeChange`;` - `gettransaction` didn’t convert `verbose`; - `listunspent` didn’t convert `includeUnsafe` or `queryOptions`; - `z_getbalanceforviewingkey` didn’t convert minconf; and - a minor non-bug – `z_getbalanceforaddress` had a handler even though the operation has been removed. `getblockdeltas` also incorrectly tries to convert its required string argument, but correcting that would be a breaking API change. Instead, it is deferred to Fixes #6429. --- src/bitcoin-cli.cpp | 11 +- src/rpc/client.cpp | 304 +++++++++++++++++++++++++---------- src/rpc/client.h | 48 +++++- src/test/rpc_tests.cpp | 30 ++-- src/test/test_util.cpp | 9 +- src/wallet/rpcdisclosure.cpp | 18 +-- 6 files changed, 300 insertions(+), 120 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 4ef57104f..8825d1c55 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -340,13 +340,20 @@ int CommandLineRPC(int argc, char *argv[]) if (args.size() < 1) throw std::runtime_error("too few parameters (need at least command)"); std::string strMethod = args[0]; - UniValue params = RPCConvertValues(strMethod, std::vector(args.begin()+1, args.end())); + auto params = + RPCConvertValues(strMethod, std::vector(args.begin()+1, args.end())); + + if (!params.has_value()) { + params.map_error([&](auto failure) { + throw std::runtime_error(FormatConversionFailure(strMethod, failure)); + }); + } // Execute and handle connection failures with -rpcwait const bool fWait = GetBoolArg("-rpcwait", false); do { try { - const UniValue reply = CallRPC(strMethod, params); + const UniValue reply = CallRPC(strMethod, params.value()); // Parse reply const UniValue& result = find_value(reply, "result"); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 7f1d6f1f3..3830c1c52 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -6,6 +6,7 @@ #include "rpc/client.h" #include "rpc/protocol.h" +#include "util/match.h" #include "util/system.h" #include @@ -13,118 +14,245 @@ #include -typedef std::map> CRPCConvertTable; +typedef std::map, std::vector>> CRPCConvertTable; + +/** A string parameter, should not be converted. */ +static const bool s = false; + +/** Something other than a string parameter, should be converted. */ +static const bool o = true; static const CRPCConvertTable rpcCvtTable = { - { "stop", {0} }, - { "setmocktime", {0} }, - { "getaddednodeinfo", {0} }, - { "setgenerate", {0, 1} }, - { "generate", {0} }, - { "getnetworkhashps", {0, 1} }, - { "getnetworksolps", {0, 1} }, - { "sendtoaddress", {1, 4} }, - { "settxfee", {0} }, - { "getreceivedbyaddress", {1, 2} }, - { "listreceivedbyaddress", {0, 1, 2} }, - { "getbalance", {1, 2, 3} }, - { "getblockhash", {0} }, - { "listtransactions", {1, 2, 3} }, - { "walletpassphrase", {1} }, - { "getblocktemplate", {0} }, - { "listsinceblock", {1, 2} }, - { "sendmany", {1, 2, 4} }, - { "addmultisigaddress", {0, 1} }, - { "createmultisig", {0, 1} }, - { "listunspent", {0, 1, 2} }, - { "getblock", {1} }, - { "getblockheader", {1} }, - { "gettransaction", {1} }, - { "getrawtransaction", {1} }, - { "createrawtransaction", {0, 1, 2, 3} }, - { "signrawtransaction", {1, 2} }, - { "sendrawtransaction", {1} }, - { "fundrawtransaction", {1} }, - { "gettxout", {1, 2} }, - { "gettxoutproof", {0} }, - { "lockunspent", {0, 1} }, - { "importprivkey", {2} }, - { "importaddress", {2, 3} }, - { "importpubkey", {2} }, - { "verifychain", {0, 1} }, - { "keypoolrefill", {0} }, - { "getrawmempool", {0} }, - { "estimatefee", {0} }, - { "estimatepriority", {0} }, - { "prioritisetransaction", {1, 2} }, - { "setban", {2, 3} }, - { "getspentinfo", {0} }, - { "getaddresstxids", {0} }, - { "getaddressbalance", {0} }, - { "getaddressdeltas", {0} }, - { "getaddressutxos", {0} }, - { "getaddressmempool", {0} }, - { "getblockhashes", {0, 1, 2} }, - { "getblockdeltas", {0} }, - { "zcbenchmark", {1, 2} }, - { "getblocksubsidy", {0} }, - { "z_listaddresses", {0} }, - { "z_listreceivedbyaddress", {1} }, - { "z_listunspent", {0, 1, 2, 3} }, - { "z_getaddressforaccount", {0, 1, 2} }, - { "z_getbalance", {1, 2} }, - { "z_getbalanceforaccount", {0, 1} }, - { "z_getbalanceforaddress", {1} }, - { "z_gettotalbalance", {0, 1, 2} }, - { "z_mergetoaddress", {0, 2, 3, 4} }, - { "z_sendmany", {1, 2, 3} }, - { "z_shieldcoinbase", {2, 3} }, - { "z_getoperationstatus", {0} }, - { "z_getoperationresult", {0} }, - { "z_importkey", {2} }, - { "z_importviewingkey", {2} }, - { "z_getpaymentdisclosure", {1, 2} }, - { "z_setmigration", {0} }, - { "z_getnotescount", {0} }, + // operation {required params, optional params} + // blockchain + { "getblockcount", {{}, {}} }, + { "getbestblockhash", {{}, {}} }, + { "getdifficulty", {{}, {}} }, + { "getrawmempool", {{}, {o}} }, + { "getblockdeltas", {{o}, {}} }, + { "getblockhashes", {{o, o}, {o}} }, + { "getblockhash", {{o}, {}} }, + { "getblockheader", {{s}, {o}} }, + { "getblock", {{s}, {o}} }, + { "gettxoutsetinfo", {{}, {}} }, + { "gettxout", {{s, o}, {o}} }, + { "verifychain", {{}, {o, o}} }, + { "getblockchaininfo", {{}, {}} }, + { "getchaintips", {{}, {}} }, + { "z_gettreestate", {{s}, {}} }, + { "getmempoolinfo", {{}, {}} }, + { "invalidateblock", {{s}, {}} }, + { "reconsiderblock", {{s}, {}} }, + // mining + { "getlocalsolps", {{}, {}} }, + { "getnetworksolps", {{}, {o, o}} }, + { "getnetworkhashps", {{}, {o, o}} }, + { "getgenerate", {{}, {}} }, + { "generate", {{o}, {}} }, + { "setgenerate", {{o}, {o}} }, + { "getmininginfo", {{}, {}} }, + { "prioritisetransaction", {{s, o, o}, {}} }, + { "getblocktemplate", {{}, {o}} }, + // NB: The second argument _should_ be an object, but upstream treats it as a string, so we + // preserve that here. + { "submitblock", {{s}, {s}} }, + { "estimatefee", {{o}, {}} }, + { "estimatepriority", {{o}, {}} }, + { "getblocksubsidy", {{o}, {}} }, + // misc + { "getinfo", {{}, {}} }, + { "validateaddress", {{s}, {}} }, + { "z_validateaddress", {{s}, {}} }, + { "createmultisig", {{o, o}, {}} }, + { "verifymessage", {{s, s ,s}, {}} }, + { "setmocktime", {{o}, {}} }, + { "getexperimentalfeatures", {{}, {}} }, + { "getaddressmempool", {{o}, {}} }, + { "getaddressutxos", {{o}, {}} }, + { "getaddressdeltas", {{o}, {}} }, + { "getaddressbalance", {{o}, {}} }, + { "getaddresstxids", {{o}, {}} }, + { "getspentinfo", {{o}, {}} }, + { "getmemoryinfo", {{}, {}} }, + // net + { "getconnectioncount", {{}, {}} }, + { "ping", {{}, {}} }, + { "getpeerinfo", {{}, {}} }, + { "addnode", {{s, s}, {}} }, + { "disconnectnode", {{s}, {}} }, + { "getaddednodeinfo", {{o}, {s}} }, + { "getnettotals", {{}, {}} }, + { "getdeprecationinfo", {{}, {}} }, + { "getnetworkinfo", {{}, {}} }, + { "setban", {{s, s}, {o, o}} }, + { "listbanned", {{}, {}} }, + { "clearbanned", {{}, {}} }, + // rawtransaction + { "getrawtransaction", {{s}, {o, s}} }, + { "gettxoutproof", {{o}, {s}} }, + { "verifytxoutproof", {{s}, {}} }, + { "createrawtransaction", {{o, o}, {o, o}} }, + { "decoderawtransaction", {{s}, {}} }, + { "decodescript", {{s}, {}} }, + { "signrawtransaction", {{s}, {o, o, s, s}} }, + { "sendrawtransaction", {{s}, {o}} }, + // rpcdisclosure + { "z_getpaymentdisclosure", {{s, o, o}, {s}} }, + { "z_validatepaymentdisclosure", {{s}, {}} }, + // rpcdump + { "importprivkey", {{s}, {s, o}} }, + { "importaddress", {{s}, {s, o, o}} }, + { "importpubkey", {{s}, {s, o}} }, + { "z_importwallet", {{s}, {}} }, + { "importwallet", {{s}, {}} }, + { "dumpprivkey", {{s}, {}} }, + { "z_exportwallet", {{s}, {}} }, + { "z_importkey", {{s}, {s, o}} }, + { "z_importviewingkey", {{s}, {s, o}} }, + { "z_exportkey", {{s}, {}} }, + { "z_exportviewingkey", {{s}, {}} }, + // rpcwallet + { "getnewaddress", {{}, {s}} }, + { "getrawchangeaddress", {{}, {}} }, + { "sendtoaddress", {{s, o}, {s, s, o}} }, + { "listaddresses", {{}, {}} }, + { "listaddressgroupings", {{}, {o}} }, + { "signmessage", {{s, s}, {}} }, + { "getreceivedbyaddress", {{s}, {o, o, o}} }, + { "getbalance", {{}, {s, o, o, o, o}} }, + { "sendmany", {{s, o}, {o, s, o}} }, + { "addmultisigaddress", {{o, o}, {s}} }, + { "listreceivedbyaddress", {{}, {o, o, o, s, o, o}} }, + { "listtransactions", {{}, {s, o, o, o, o}} }, + { "listsinceblock", {{}, {s, o, o, o, o, o}} }, + { "gettransaction", {{s}, {o, o, o}} }, + { "backupwallet", {{s}, {}} }, + { "keypoolrefill", {{}, {o}} }, + { "walletpassphrase", {{s, o}, {}} }, + { "walletpassphrasechange", {{s, s}, {}} }, + { "walletconfirmbackup", {{s}, {}} }, + { "walletlock", {{}, {}} }, + { "encryptwallet", {{s}, {}} }, + { "lockunspent", {{o, o}, {}} }, + { "listlockunspent", {{}, {}} }, + { "settxfee", {{o}, {}} }, + { "getwalletinfo", {{}, {o}} }, + { "resendwallettransactions", {{}, {}} }, + { "listunspent", {{}, {o, o, o, o, o, o}} }, + { "z_listunspent", {{}, {o, o, o, o, o}} }, + { "fundrawtransaction", {{s}, {o}} }, + { "zcsamplejoinsplit", {{}, {}} }, + { "zcbenchmark", {{s, o}, {o}} }, + { "z_getnewaddress", {{}, {s}} }, + { "z_getnewaccount", {{}, {}} }, + { "z_getaddressforaccount", {{o}, {o, o}} }, + { "z_listaccounts", {{}, {}} }, + { "z_listaddresses", {{}, {o}} }, + { "z_listunifiedreceivers", {{s}, {}} }, + { "z_listreceivedbyaddress", {{s}, {o, o}} }, + { "z_getbalance", {{s}, {o, o}} }, + { "z_getbalanceforviewingkey", {{s}, {o, o}} }, + { "z_getbalanceforaccount", {{o}, {o, o}} }, + { "z_gettotalbalance", {{}, {o, o}} }, + { "z_viewtransaction", {{s}, {}} }, + { "z_getoperationresult", {{}, {o}} }, + { "z_getoperationstatus", {{}, {o}} }, + { "z_sendmany", {{s, o}, {o, o, s}} }, + { "z_setmigration", {{o}, {}} }, + { "z_getmigrationstatus", {{}, {o}} }, + { "z_shieldcoinbase", {{s, s}, {o, o}} }, + { "z_mergetoaddress", {{o, s}, {o, o, o, s}} }, + { "z_listoperationids", {{}, {s}} }, + { "z_getnotescount", {{}, {o, o}} }, + // server + { "help", {{}, {s}} }, + { "setlogfilter", {{s}, {}} }, + { "stop", {{}, {o}} }, }; -std::set ParamsToConvertFor(const std::string& method) { +std::string FormatConversionFailure(const std::string& method, const ConversionFailure& failure) { + return std::visit(match { + [&](const UnknownRPCMethod&) { + return tinyformat::format("Unknown RPC method, %s", method); + }, + [&](const WrongNumberOfArguments& err) { + return tinyformat::format( + "%s for method, %s. Needed between %u and %u, but received %u", + err.providedArgs < err.requiredParams + ? "Not enough arguments" + : "Too many arguments", + method, + err.requiredParams, + err.requiredParams + err.optionalParams, + err.providedArgs); + }, + [](const UnparseableArgument& err) { + return std::string("Error parsing JSON:") + err.unparsedArg; + } + }, failure); +} + +std::optional, std::vector>> +ParamsToConvertFor(const std::string& method) { auto search = rpcCvtTable.find(method); if (search != rpcCvtTable.end()) { return search->second; } else { - return std::set(); + return std::nullopt; } } -/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null) - * as well as objects and arrays. - */ -UniValue ParseNonRFCJSONValue(const std::string& strVal) +std::optional ParseNonRFCJSONValue(const std::string& strVal) { UniValue jVal; - if (!jVal.read(std::string("[")+strVal+std::string("]")) || - !jVal.isArray() || jVal.size()!=1) - throw std::runtime_error(std::string("Error parsing JSON:")+strVal); - return jVal[0]; + if (jVal.read(std::string("[")+strVal+std::string("]")) && jVal.isArray() && jVal.size() == 1) { + return jVal[0]; + } else { + return std::nullopt; + } } -/** Convert strings to command-specific RPC representation */ -UniValue RPCConvertValues(const std::string &strMethod, const std::vector &strParams) +tl::expected +RPCConvertValues(const std::string &method, const std::vector &strArgs) { UniValue params(UniValue::VARR); + auto paramsToConvert = ParamsToConvertFor(method); + std::vector requiredParams{}, optionalParams{}; + if (paramsToConvert.has_value()) { + std::tie(requiredParams, optionalParams) = paramsToConvert.value(); + } else { + return tl::expected(tl::unexpect, UnknownRPCMethod()); + } - auto paramsToConvert = ParamsToConvertFor(strMethod); - for (std::vector::size_type idx = 0; idx < strParams.size(); idx++) { - const std::string& strVal = strParams[idx]; + if (strArgs.size() < requiredParams.size() + || requiredParams.size() + optionalParams.size() < strArgs.size()) { + return tl::expected( + tl::unexpect, + WrongNumberOfArguments(requiredParams.size(), optionalParams.size(), strArgs.size())); + } - if (paramsToConvert.count(idx) != 0) { + std::vector allParams(requiredParams.begin(), requiredParams.end()); + allParams.reserve(requiredParams.size() + optionalParams.size()); + allParams.insert(allParams.end(), optionalParams.begin(), optionalParams.end()); + + for (std::vector::size_type idx = 0; idx < strArgs.size(); idx++) { + const bool shouldConvert = allParams[idx]; + const std::string& strVal = strArgs[idx]; + + if (!shouldConvert) { // insert string value directly params.push_back(strVal); } else { // parse string as JSON, insert bool/number/object/etc. value - params.push_back(ParseNonRFCJSONValue(strVal)); + auto parsedArg = ParseNonRFCJSONValue(strVal); + if (parsedArg.has_value()) { + params.push_back(parsedArg.value()); + } else { + return tl::expected( + tl::unexpect, + UnparseableArgument(strVal)); + } } } diff --git a/src/rpc/client.h b/src/rpc/client.h index 11a4dac3d..0ef6a56e7 100644 --- a/src/rpc/client.h +++ b/src/rpc/client.h @@ -7,12 +7,54 @@ #ifndef BITCOIN_RPC_CLIENT_H #define BITCOIN_RPC_CLIENT_H +#include #include -UniValue RPCConvertValues(const std::string& strMethod, const std::vector& strParams); +class UnknownRPCMethod { + public: + UnknownRPCMethod() {}; +}; + +class WrongNumberOfArguments { + public: + std::vector::size_type requiredParams; + std::vector::size_type optionalParams; + std::vector::size_type providedArgs; + + WrongNumberOfArguments( + std::vector::size_type requiredParams, + std::vector::size_type optionalParams, + std::vector::size_type providedArgs) : + requiredParams(requiredParams), + optionalParams(optionalParams), + providedArgs(providedArgs) {} +}; + +class UnparseableArgument { + public: + std::string unparsedArg; + + UnparseableArgument(std::string unparsedArg) : + unparsedArg(unparsedArg) {} +}; + +typedef std::variant< + UnknownRPCMethod, + WrongNumberOfArguments, + UnparseableArgument + > ConversionFailure; + +// TODO: This should be closer to the leaves, but don’t have a good place for it, since it’s +// currently shared by bitcoin-cli and tests. +std::string FormatConversionFailure(const std::string& method, const ConversionFailure& failure); + +/** Convert strings to command-specific RPC representation */ +tl::expected + RPCConvertValues(const std::string& method, const std::vector& strArgs); + /** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null) - * as well as objects and arrays. + * as well as objects and arrays. Returns `std::nullopt` if `strVal` couldn’t be parsed. */ -UniValue ParseNonRFCJSONValue(const std::string& strVal); +std::optional ParseNonRFCJSONValue(const std::string& strVal); #endif // BITCOIN_RPC_CLIENT_H diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 81d9ccc0b..209a78af0 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -166,28 +166,28 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values) BOOST_AUTO_TEST_CASE(json_parse_errors) { // Valid - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0").get_real(), 1.0); + BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0").value().get_real(), 1.0); // Valid, with leading or trailing whitespace - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue(" 1.0").get_real(), 1.0); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0 ").get_real(), 1.0); + BOOST_CHECK_EQUAL(ParseNonRFCJSONValue(" 1.0").value().get_real(), 1.0); + BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0 ").value().get_real(), 1.0); - BOOST_CHECK_THROW(AmountFromValue(ParseNonRFCJSONValue(".19e-6")), std::runtime_error); //should fail, missing leading 0, therefore invalid JSON - BOOST_CHECK_EQUAL(AmountFromValue(ParseNonRFCJSONValue("0.00000000000000000000000000000000000001e+30 ")), 1); + BOOST_CHECK(!ParseNonRFCJSONValue(".19e-6").has_value()); //should fail, missing leading 0, therefore invalid JSON + BOOST_CHECK_EQUAL(AmountFromValue(ParseNonRFCJSONValue("0.00000000000000000000000000000000000001e+30 ").value()), 1); // Invalid, initial garbage - BOOST_CHECK_THROW(ParseNonRFCJSONValue("[1.0"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("a1.0"), std::runtime_error); + BOOST_CHECK(!ParseNonRFCJSONValue("[1.0").has_value()); + BOOST_CHECK(!ParseNonRFCJSONValue("a1.0").has_value()); // Invalid, trailing garbage - BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0sds"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0]"), std::runtime_error); + BOOST_CHECK(!ParseNonRFCJSONValue("1.0sds").has_value()); + BOOST_CHECK(!ParseNonRFCJSONValue("1.0]").has_value()); // BTC addresses should fail parsing - BOOST_CHECK_THROW(ParseNonRFCJSONValue("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"), std::runtime_error); + BOOST_CHECK(!ParseNonRFCJSONValue("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W").has_value()); + BOOST_CHECK(!ParseNonRFCJSONValue("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL").has_value()); } BOOST_AUTO_TEST_CASE(rpc_ban) { BOOST_CHECK_NO_THROW(CallRPC(string("clearbanned"))); - + UniValue r; BOOST_CHECK_NO_THROW(r = CallRPC(string("setban 127.0.0.0 add"))); BOOST_CHECK_THROW(r = CallRPC(string("setban 127.0.0.0:8334")), runtime_error); //portnumber for setban not allowed @@ -353,7 +353,7 @@ BOOST_AUTO_TEST_CASE(rpc_insightexplorer) const string addr = "t1T3G72ToPuCDTiCEytrU1VUBRHsNupEBut"; BOOST_CHECK_NO_THROW(CallRPC("getaddressmempool \"" + addr + "\"")); BOOST_CHECK_NO_THROW(CallRPC("getaddressmempool {\"addresses\":[\"" + addr + "\"]}")); - BOOST_CHECK_NO_THROW(CallRPC("getaddressmempool {\"addresses\":[\"" + addr + "\",\"" + addr + "\"]}")); + BOOST_CHECK_NO_THROW(CallRPC("getaddressmempool {\"addresses\":[\"" + addr + "\",\"" + addr + "\"]}")); BOOST_CHECK_NO_THROW(CallRPC("getaddressutxos {\"addresses\":[],\"chainInfo\":true}")); CheckRPCThrows("getaddressutxos {}", @@ -385,9 +385,9 @@ BOOST_AUTO_TEST_CASE(rpc_insightexplorer) // transaction does not exist: CheckRPCThrows("getspentinfo {\"txid\":\"b4cc287e58f87cdae59417329f710f3ecd75a4ee1d2872b7248f50977c8493f3\",\"index\":0}", - "Unable to get spent info"); + "Unable to get spent info"); CheckRPCThrows("getspentinfo {\"txid\":\"b4cc287e58f87cdae59417329f710f3ecd75a4ee1d2872b7248f50977c8493f3\"}", - "Invalid index, must be an integer"); + "Invalid index, must be an integer"); CheckRPCThrows("getspentinfo {\"txid\":\"hello\",\"index\":0}", "txid must be hexadecimal string (not 'hello')"); diff --git a/src/test/test_util.cpp b/src/test/test_util.cpp index 1636ab6b0..9c80a5a68 100644 --- a/src/test/test_util.cpp +++ b/src/test/test_util.cpp @@ -94,11 +94,15 @@ UniValue CallRPC(std::string args) vArgs[i] = ""; } } - UniValue params = RPCConvertValues(strMethod, vArgs); + auto params = RPCConvertValues(strMethod, vArgs); + + params.map_error([&](auto failure) { + throw std::runtime_error(FormatConversionFailure(strMethod, failure)); + }); rpcfn_type method = tableRPC[strMethod]->actor; try { - UniValue result = (*method)(params, false); + UniValue result = (*method)(params.value(), false); return result; } catch (const UniValue& objError) { @@ -117,4 +121,3 @@ void CheckRPCThrows(std::string rpcString, std::string expectedErrorMessage) { BOOST_FAIL(std::string("Unexpected exception: ") + typeid(e).name() + ", message=\"" + e.what() + "\""); } } - diff --git a/src/wallet/rpcdisclosure.cpp b/src/wallet/rpcdisclosure.cpp index 4bc0570d3..a388656bf 100644 --- a/src/wallet/rpcdisclosure.cpp +++ b/src/wallet/rpcdisclosure.cpp @@ -51,14 +51,14 @@ UniValue z_getpaymentdisclosure(const UniValue& params, bool fHelp) if (fHelp || params.size() < 3 || params.size() > 4 ) throw runtime_error( - "z_getpaymentdisclosure \"txid\" \"js_index\" \"output_index\" (\"message\") \n" + "z_getpaymentdisclosure \"txid\" js_index output_index (\"message\") \n" "\nGenerate a payment disclosure for a given joinsplit output.\n" "\nEXPERIMENTAL FEATURE\n" + disabledMsg + "\nArguments:\n" "1. \"txid\" (string, required) \n" - "2. \"js_index\" (string, required) \n" - "3. \"output_index\" (string, required) \n" + "2. js_index (numeric, required) \n" + "3. output_index (numeric, required) \n" "4. \"message\" (string, optional) \n" "\nResult:\n" "\"paymentdisclosure\" (string) Hex data string, with \"zpd:\" prefix.\n" @@ -101,7 +101,7 @@ UniValue z_getpaymentdisclosure(const UniValue& params, bool fHelp) // Check if shielded tx if (wtx.vJoinSplit.empty()) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction is not a shielded transaction"); + throw JSONRPCError(RPC_MISC_ERROR, "Transaction is not a shielded transaction"); } // Check js_index @@ -195,7 +195,7 @@ UniValue z_validatepaymentdisclosure(const UniValue& params, bool fHelp) // too much data is ignored, but if not enough data, exception of type ios_base::failure is thrown, // CBaseDataStream::read(): end of data: iostream error } catch (const std::exception &e) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, payment disclosure data is malformed."); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, payment disclosure data is malformed."); } if (pd.payload.marker != PAYMENT_DISCLOSURE_PAYLOAD_MAGIC_BYTES) { @@ -221,7 +221,7 @@ UniValue z_validatepaymentdisclosure(const UniValue& params, bool fHelp) // Check if shielded tx if (tx.vJoinSplit.empty()) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction is not a shielded transaction"); + throw JSONRPCError(RPC_MISC_ERROR, "Transaction is not a shielded transaction"); } UniValue errs(UniValue::VARR); @@ -259,9 +259,9 @@ UniValue z_validatepaymentdisclosure(const UniValue& params, bool fHelp) dataToBeSigned.begin(), 32); o.pushKV("signatureVerified", sigVerified); if (!sigVerified) { - errs.push_back("Payment disclosure signature does not match transaction signature"); + errs.push_back("Payment disclosure signature does not match transaction signature"); } - + KeyIO keyIO(Params()); // Check the payment address is valid @@ -289,7 +289,7 @@ UniValue z_validatepaymentdisclosure(const UniValue& params, bool fHelp) string memoHexString = HexStr(npt.memo().data(), npt.memo().data() + npt.memo().size()); o.pushKV("memo", memoHexString); o.pushKV("value", ValueFromAmount(npt.value())); - + // Check the blockchain commitment matches decrypted note commitment uint256 cm_blockchain = jsdesc.commitments[pd.payload.n]; SproutNote note = npt.note(zaddr);