Skip to content

Commit

Permalink
refactor: Revert addition of univalue sighash string check
Browse files Browse the repository at this point in the history
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 <stickies-v@protonmail.com>
  • Loading branch information
TheCharlatan and stickies-v committed Jul 27, 2023
1 parent 0b47c16 commit 06199a9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/parse_univalue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 06199a9

Please sign in to comment.