ParseHashV validates the length and encoding of the string and throws an informative RPC error on failure, which is as good or better than these alternative calls.
Note I switched ParseHashV to check string length first, because IsHex tests that the length is even, and an error like: “must be of length 64 (not 63, for X)” is much more informative than “must be hexadecimal string (not X)” in that case.
Split from #13420
246@@ -247,7 +247,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request)
247
248 LOCK(cs_main);
249
250- uint256 hash = ParseHashStr(request.params[0].get_str(), "txid");
251+ uint256 hash = ParseHashV(request.params[0], "txid");
ParseHashStr’s std::runtime_error-based encoding-only test for ParseHashV’s JSONRPCError-based length and encoding tests.
How about getchaintxstats?https://github.com/bitcoin/bitcoin/blob/56f69360dc98bd68704f19646a84d045788d199e/src/rpc/blockchain.cpp#L1575
And there may be other examples.
Concept ACK.
get_str call guaranteed, which means non-string values fail with -1, "JSON value is not a string as expected", rather than a length failure.
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
utACK
Small nit: you’re mixing uint256 hash = ParseHashV(...) and uint256 hash(ParseHashV(...)). Would be cool to do only one (latter one looks better IMO but either works).
ParseHashV validates the length and encoding of the string and throws
an informative RPC error on failure, which is as good or better than
these alternative calls.
Note I switched ParseHashV to check string length first, because
IsHex tests that the length is even, and an error like:
"must be of length 64 (not 63, for X)" is much more informative than
"must be hexadecimal string (not X)"
