Merge #14307: Consolidate redundant implementations of ParseHashStr

9c5af58d51 Consolidate redundant implementations of ParseHashStr (Ben Woosley)

Pull request description:

  This change:
  * adds a length check to all calls 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

  Relative to #14288

Tree-SHA512: baa791147e5ceb3c30c70df3981aaf807bf7d4a90a0be3625540b59aa4b9a9d303a452bfef18bf167cbb833ef9591b4ef5948bf4a1ce67b421d804ae8d20ea53

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)

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<unsigned char> 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);

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<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName)

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;

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<unsigned char> 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);