Skip to content

Commit 9c5af58

Browse files
EmpactEmpact
committed
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
1 parent 990fc0d commit 9c5af58

File tree

6 files changed

+87
-22
lines changed

6 files changed

+87
-22
lines changed

‎src/bitcoin-tx.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,10 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu
240240
throw std::runtime_error("TX input missing separator");
241241

242242
// extract and validate TXID
243-
std::string strTxid = vStrInputParts[0];
244-
if ((strTxid.size() != 64) || !IsHex(strTxid))
243+
uint256 txid;
244+
if (!ParseHashStr(vStrInputParts[0], txid)) {
245245
throw std::runtime_error("invalid TX input txid");
246-
uint256 txid(uint256S(strTxid));
246+
}
247247

248248
static const unsigned int minTxOutSz = 9;
249249
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)
590590
if (!prevOut.checkObject(types))
591591
throw std::runtime_error("prevtxs internal object typecheck fail");
592592

593-
uint256 txid = ParseHashStr(prevOut["txid"].get_str(), "txid");
593+
uint256 txid;
594+
if (!ParseHashStr(prevOut["txid"].get_str(), txid)) {
595+
throw std::runtime_error("txid must be hexadecimal string (not '" + prevOut["txid"].get_str() + "')");
596+
}
594597

595598
const int nOut = prevOut["vout"].get_int();
596599
if (nOut < 0)

‎src/core_io.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,16 @@ std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDeco
2525
bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true);
2626
bool DecodeHexBlk(CBlock&, const std::string& strHexBlk);
2727
bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
28-
uint256 ParseHashStr(const std::string&, const std::string& strName);
28+
29+
/**
30+
* Parse a hex string into 256 bits
31+
* @param[in] strHex a hex-formatted, 64-character string
32+
* @param[out] result the result of the parasing
33+
* @returns true if successful, false if not
34+
*
35+
* @see ParseHashV for an RPC-oriented version of this
36+
*/
37+
bool ParseHashStr(const std::string& strHex, uint256& result);
2938
std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName);
3039
bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error);
3140
int ParseSighashString(const UniValue& sighash);

‎src/core_read.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,13 @@ bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx,
193193
return true;
194194
}
195195

196-
uint256 ParseHashStr(const std::string& strHex, const std::string& strName)
196+
bool ParseHashStr(const std::string& strHex, uint256& result)
197197
{
198-
if (!IsHex(strHex)) // Note: IsHex("") is false
199-
throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')");
198+
if ((strHex.size() != 64) || !IsHex(strHex))
199+
return false;
200200

201-
uint256 result;
202201
result.SetHex(strHex);
203-
return result;
202+
return true;
204203
}
205204

206205
std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName)

‎src/rest.cpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,6 @@ static std::string AvailableDataFormatsString()
105105
return formats;
106106
}
107107

108-
static bool ParseHashStr(const std::string& strReq, uint256& v)
109-
{
110-
if (!IsHex(strReq) || (strReq.size() != 64))
111-
return false;
112-
113-
v.SetHex(strReq);
114-
return true;
115-
}
116-
117108
static bool CheckWarmup(HTTPRequest* req)
118109
{
119110
std::string statusmessage;

‎src/test/blockfilter_tests.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test)
114114

115115
unsigned int pos = 0;
116116
/*int block_height =*/ test[pos++].get_int();
117-
/*uint256 block_hash =*/ ParseHashStr(test[pos++].get_str(), "block_hash");
117+
uint256 block_hash;
118+
BOOST_CHECK(ParseHashStr(test[pos++].get_str(), block_hash));
118119

119120
CBlock block;
120121
BOOST_REQUIRE(DecodeHexBlk(block, test[pos++].get_str()));
@@ -129,9 +130,11 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test)
129130
tx_undo.vprevout.emplace_back(txout, 0, false);
130131
}
131132

132-
uint256 prev_filter_header_basic = ParseHashStr(test[pos++].get_str(), "prev_filter_header_basic");
133+
uint256 prev_filter_header_basic;
134+
BOOST_CHECK(ParseHashStr(test[pos++].get_str(), prev_filter_header_basic));
133135
std::vector<unsigned char> filter_basic = ParseHex(test[pos++].get_str());
134-
uint256 filter_header_basic = ParseHashStr(test[pos++].get_str(), "filter_header_basic");
136+
uint256 filter_header_basic;
137+
BOOST_CHECK(ParseHashStr(test[pos++].get_str(), filter_header_basic));
135138

136139
BlockFilter computed_filter_basic(BlockFilterType::BASIC, block, block_undo);
137140
BOOST_CHECK(computed_filter_basic.GetFilter().GetEncoded() == filter_basic);

‎test/util/data/bitcoin-util-test.json‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,30 @@
102102
"error_txt": "error: Invalid TX locktime requested",
103103
"description": "Tests the check for invalid locktime value"
104104
},
105+
{ "exec": "./bitcoin-tx",
106+
"args":
107+
["-create",
108+
"in=Z897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0"],
109+
"return_code": 1,
110+
"error_txt": "error: invalid TX input txid",
111+
"description": "Tests the check for an invalid txid invalid hex"
112+
},
113+
{ "exec": "./bitcoin-tx",
114+
"args":
115+
["-create",
116+
"in=5897de6bd6:0"],
117+
"return_code": 1,
118+
"error_txt": "error: invalid TX input txid",
119+
"description": "Tests the check for an invalid txid valid hex but too short"
120+
},
121+
{ "exec": "./bitcoin-tx",
122+
"args":
123+
["-create",
124+
"in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f12:0"],
125+
"return_code": 1,
126+
"error_txt": "error: invalid TX input txid",
127+
"description": "Tests the check for an invalid txid valid hex but too long"
128+
},
105129
{ "exec": "./bitcoin-tx",
106130
"args":
107131
["-create",
@@ -280,6 +304,42 @@
280304
"error_txt": "error: prevtxs internal object typecheck fail",
281305
"description": "Tests the check for invalid vout index in prevtxs for sign"
282306
},
307+
{ "exec": "./bitcoin-tx",
308+
"args":
309+
["-create",
310+
"in=4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485:0",
311+
"set=privatekeys:[\"5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf\"]",
312+
"set=prevtxs:[{\"txid\":\"Zd49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59412\",\"vout\":0,\"scriptPubKey\":\"76a91491b24bf9f5288532960ac687abb035127b1d28a588ac\"}]",
313+
"sign=ALL",
314+
"outaddr=0.001:193P6LtvS4nCnkDvM9uXn1gsSRqh4aDAz7"],
315+
"return_code": 1,
316+
"error_txt": "error: txid must be hexadecimal string (not 'Zd49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59412')",
317+
"description": "Tests the check for invalid txid due to invalid hex"
318+
},
319+
{ "exec": "./bitcoin-tx",
320+
"args":
321+
["-create",
322+
"in=4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485:0",
323+
"set=privatekeys:[\"5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf\"]",
324+
"set=prevtxs:[{\"txid\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc594\",\"vout\":0,\"scriptPubKey\":\"76a91491b24bf9f5288532960ac687abb035127b1d28a588ac\"}]",
325+
"sign=ALL",
326+
"outaddr=0.001:193P6LtvS4nCnkDvM9uXn1gsSRqh4aDAz7"],
327+
"return_code": 1,
328+
"error_txt": "error: txid must be hexadecimal string (not '4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc594')",
329+
"description": "Tests the check for invalid txid valid hex, but too short"
330+
},
331+
{ "exec": "./bitcoin-tx",
332+
"args":
333+
["-create",
334+
"in=4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485:0",
335+
"set=privatekeys:[\"5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf\"]",
336+
"set=prevtxs:[{\"txid\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc5948512\",\"vout\":0,\"scriptPubKey\":\"76a91491b24bf9f5288532960ac687abb035127b1d28a588ac\"}]",
337+
"sign=ALL",
338+
"outaddr=0.001:193P6LtvS4nCnkDvM9uXn1gsSRqh4aDAz7"],
339+
"return_code": 1,
340+
"error_txt": "error: txid must be hexadecimal string (not '4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc5948512')",
341+
"description": "Tests the check for invalid txid valid hex, but too long"
342+
},
283343
{ "exec": "./bitcoin-tx",
284344
"args":
285345
["-create", "outpubkey=0:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397", "nversion=1"],

0 commit comments

Comments
 (0)