From 0b47c1621524a96b79cbdc3c45ee5ad36e7ba541 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 26 Jul 2023 11:36:45 +0200 Subject: [PATCH 1/2] doc: Correct release-notes for sighashtype exceptions --- doc/release-notes-28113.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/release-notes-28113.md b/doc/release-notes-28113.md index c1a3a07b17..c9f418dbbb 100644 --- a/doc/release-notes-28113.md +++ b/doc/release-notes-28113.md @@ -2,6 +2,6 @@ RPC Wallet ---------- - The `signrawtransactionwithkey`, `signrawtransactionwithwallet`, - `walletprocesspsbt` and `descriptorprocesspsbt` calls now return more - specific RPC_INVALID_PARAMETER instead of RPC_PARSE_ERROR if their - sighashtype argument is malformed or not a string. + `walletprocesspsbt` and `descriptorprocesspsbt` calls now return the more + specific RPC_INVALID_PARAMETER error instead of RPC_MISC_ERROR if their + sighashtype argument is malformed. From 06199a995f20c55583f6948cfe99e608679fcdf1 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 26 Jul 2023 11:39:54 +0200 Subject: [PATCH 2/2] refactor: Revert addition of univalue sighash string check This check is already done by the rpc parser. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. To make the fuzz test more accurate and not swallow all runtime errors, add a check that the passed in UniValue sighash argument is either a string or null. Co-authored-by: stickies-v --- src/rpc/util.cpp | 8 +++++--- src/test/fuzz/parse_univalue.cpp | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 377181dd81..faae840d40 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -313,14 +313,16 @@ UniValue DescribeAddress(const CTxDestination& dest) return std::visit(DescribeAddressVisitor(), dest); } +/** + * Returns a sighash value corresponding to the passed in argument. + * + * @pre The sighash argument should be string or null. +*/ int ParseSighashString(const UniValue& sighash) { if (sighash.isNull()) { return SIGHASH_DEFAULT; } - if (!sighash.isStr()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "sighash needs to be null or string"); - } const auto result{SighashFromStr(sighash.get_str())}; if (!result) { throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original); diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp index c9096d0386..a3d6ab6375 100644 --- a/src/test/fuzz/parse_univalue.cpp +++ b/src/test/fuzz/parse_univalue.cpp @@ -67,7 +67,7 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue) } catch (const std::runtime_error&) { } try { - (void)ParseSighashString(univalue); + if (univalue.isNull() || univalue.isStr()) (void)ParseSighashString(univalue); } catch (const UniValue&) { } try {