From 9c5af58d51cea7d0cf2a598a9979eeba103b23ff Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Mon, 24 Sep 2018 10:59:17 -0400 Subject: Consolidate redundant implementations of ParseHashStr This change: * adds a length check to ParseHashStr, appropriate given its use to populate a 256-bit number from a hex str. * allows the caller to handle the failure, which allows for the more appropriate JSONRPCError on failure in prioritisetransaction rpc --- src/bitcoin-tx.cpp | 11 +++++++---- src/core_io.h | 11 ++++++++++- src/core_read.cpp | 9 ++++----- src/rest.cpp | 9 --------- src/test/blockfilter_tests.cpp | 9 ++++++--- 5 files changed, 27 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 9ee3e8eee7..a3fcb87675 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -240,10 +240,10 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu throw std::runtime_error("TX input missing separator"); // extract and validate TXID - std::string strTxid = vStrInputParts[0]; - if ((strTxid.size() != 64) || !IsHex(strTxid)) + uint256 txid; + if (!ParseHashStr(vStrInputParts[0], txid)) { throw std::runtime_error("invalid TX input txid"); - uint256 txid(uint256S(strTxid)); + } static const unsigned int minTxOutSz = 9; static const unsigned int maxVout = MAX_BLOCK_WEIGHT / (WITNESS_SCALE_FACTOR * minTxOutSz); @@ -590,7 +590,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) if (!prevOut.checkObject(types)) throw std::runtime_error("prevtxs internal object typecheck fail"); - uint256 txid = ParseHashStr(prevOut["txid"].get_str(), "txid"); + uint256 txid; + if (!ParseHashStr(prevOut["txid"].get_str(), txid)) { + throw std::runtime_error("txid must be hexadecimal string (not '" + prevOut["txid"].get_str() + "')"); + } const int nOut = prevOut["vout"].get_int(); if (nOut < 0) diff --git a/src/core_io.h b/src/core_io.h index d53a45c0cb..2c3b64d81e 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -25,7 +25,16 @@ std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDeco bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); -uint256 ParseHashStr(const std::string&, const std::string& strName); + +/** + * Parse a hex string into 256 bits + * @param[in] strHex a hex-formatted, 64-character string + * @param[out] result the result of the parasing + * @returns true if successful, false if not + * + * @see ParseHashV for an RPC-oriented version of this + */ +bool ParseHashStr(const std::string& strHex, uint256& result); std::vector ParseHexUV(const UniValue& v, const std::string& strName); bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error); int ParseSighashString(const UniValue& sighash); diff --git a/src/core_read.cpp b/src/core_read.cpp index b02016c014..301f99bc1c 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -193,14 +193,13 @@ bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, return true; } -uint256 ParseHashStr(const std::string& strHex, const std::string& strName) +bool ParseHashStr(const std::string& strHex, uint256& result) { - if (!IsHex(strHex)) // Note: IsHex("") is false - throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')"); + if ((strHex.size() != 64) || !IsHex(strHex)) + return false; - uint256 result; result.SetHex(strHex); - return result; + return true; } std::vector ParseHexUV(const UniValue& v, const std::string& strName) diff --git a/src/rest.cpp b/src/rest.cpp index 7792844992..1850c0b7a6 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -105,15 +105,6 @@ static std::string AvailableDataFormatsString() return formats; } -static bool ParseHashStr(const std::string& strReq, uint256& v) -{ - if (!IsHex(strReq) || (strReq.size() != 64)) - return false; - - v.SetHex(strReq); - return true; -} - static bool CheckWarmup(HTTPRequest* req) { std::string statusmessage; diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp index 773de343ea..4941ebd483 100644 --- a/src/test/blockfilter_tests.cpp +++ b/src/test/blockfilter_tests.cpp @@ -114,7 +114,8 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test) unsigned int pos = 0; /*int block_height =*/ test[pos++].get_int(); - /*uint256 block_hash =*/ ParseHashStr(test[pos++].get_str(), "block_hash"); + uint256 block_hash; + BOOST_CHECK(ParseHashStr(test[pos++].get_str(), block_hash)); CBlock block; BOOST_REQUIRE(DecodeHexBlk(block, test[pos++].get_str())); @@ -129,9 +130,11 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test) tx_undo.vprevout.emplace_back(txout, 0, false); } - uint256 prev_filter_header_basic = ParseHashStr(test[pos++].get_str(), "prev_filter_header_basic"); + uint256 prev_filter_header_basic; + BOOST_CHECK(ParseHashStr(test[pos++].get_str(), prev_filter_header_basic)); std::vector filter_basic = ParseHex(test[pos++].get_str()); - uint256 filter_header_basic = ParseHashStr(test[pos++].get_str(), "filter_header_basic"); + uint256 filter_header_basic; + BOOST_CHECK(ParseHashStr(test[pos++].get_str(), filter_header_basic)); BlockFilter computed_filter_basic(BlockFilterType::BASIC, block, block_undo); BOOST_CHECK(computed_filter_basic.GetFilter().GetEncoded() == filter_basic); -- cgit v1.2.3