From 346a099fc1e282c8b53d740d65999d8866d17953 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 12 Mar 2025 20:16:04 +0100 Subject: [PATCH 1/2] test: avoid unneeded hash -> uint256 -> hash roundtrips In the functional test framework, we often treat hashes as uint256 integers, which seems to be confusing and for no good reason, as hashes are just sequences of bytes. This commit gets rid of obvious internal instances of that where individual functional tests are not affected. In the long-term, it might make sense to store other hashes (mostly txids) as actual bytes to avoid annoying conversions and improve code readability. --- test/functional/test_framework/blocktools.py | 5 ++--- test/functional/test_framework/script.py | 23 ++++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 38600bc005a..8e616285572 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -28,7 +28,6 @@ from .messages import ( ser_uint256, tx_from_hex, uint256_from_compact, - uint256_from_str, WITNESS_SCALE_FACTOR, ) from .script import ( @@ -111,8 +110,8 @@ def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl return block def get_witness_script(witness_root, witness_nonce): - witness_commitment = uint256_from_str(hash256(ser_uint256(witness_root) + ser_uint256(witness_nonce))) - output_data = WITNESS_COMMITMENT_HEADER + ser_uint256(witness_commitment) + witness_commitment = hash256(ser_uint256(witness_root) + ser_uint256(witness_nonce)) + output_data = WITNESS_COMMITMENT_HEADER + witness_commitment return CScript([OP_RETURN, output_data]) def add_witness_commitment(block, nonce=0): diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index d510cf9b1ca..12bfee7c776 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -17,9 +17,7 @@ from .messages import ( CTxOut, hash256, ser_string, - ser_uint256, sha256, - uint256_from_str, ) from .crypto.ripemd160 import ripemd160 @@ -711,41 +709,42 @@ def sign_input_segwitv0(tx, input_index, input_scriptpubkey, input_amount, privk # Note that this corresponds to sigversion == 1 in EvalScript, which is used # for version 0 witnesses. def SegwitV0SignatureMsg(script, txTo, inIdx, hashtype, amount): + ZERO_HASH = bytes([0]*32) - hashPrevouts = 0 - hashSequence = 0 - hashOutputs = 0 + hashPrevouts = ZERO_HASH + hashSequence = ZERO_HASH + hashOutputs = ZERO_HASH if not (hashtype & SIGHASH_ANYONECANPAY): serialize_prevouts = bytes() for i in txTo.vin: serialize_prevouts += i.prevout.serialize() - hashPrevouts = uint256_from_str(hash256(serialize_prevouts)) + hashPrevouts = hash256(serialize_prevouts) if (not (hashtype & SIGHASH_ANYONECANPAY) and (hashtype & 0x1f) != SIGHASH_SINGLE and (hashtype & 0x1f) != SIGHASH_NONE): serialize_sequence = bytes() for i in txTo.vin: serialize_sequence += i.nSequence.to_bytes(4, "little") - hashSequence = uint256_from_str(hash256(serialize_sequence)) + hashSequence = hash256(serialize_sequence) if ((hashtype & 0x1f) != SIGHASH_SINGLE and (hashtype & 0x1f) != SIGHASH_NONE): serialize_outputs = bytes() for o in txTo.vout: serialize_outputs += o.serialize() - hashOutputs = uint256_from_str(hash256(serialize_outputs)) + hashOutputs = hash256(serialize_outputs) elif ((hashtype & 0x1f) == SIGHASH_SINGLE and inIdx < len(txTo.vout)): serialize_outputs = txTo.vout[inIdx].serialize() - hashOutputs = uint256_from_str(hash256(serialize_outputs)) + hashOutputs = hash256(serialize_outputs) ss = bytes() ss += txTo.version.to_bytes(4, "little") - ss += ser_uint256(hashPrevouts) - ss += ser_uint256(hashSequence) + ss += hashPrevouts + ss += hashSequence ss += txTo.vin[inIdx].prevout.serialize() ss += ser_string(script) ss += amount.to_bytes(8, "little", signed=True) ss += txTo.vin[inIdx].nSequence.to_bytes(4, "little") - ss += ser_uint256(hashOutputs) + ss += hashOutputs ss += txTo.nLockTime.to_bytes(4, "little") ss += hashtype.to_bytes(4, "little") return ss From a82829f37e1ed298b6c2b6d2859d4bea65fe3dcc Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 12 Mar 2025 20:30:25 +0100 Subject: [PATCH 2/2] test: simplify (w)txid checks by avoiding .calc_sha256 calls Calls on the tx.calc_sha256 method can be confusing, as they return the result (either txid or wtxid, depending on the with_witness boolean parameter) as integer rather than as actual (w)txid. Use .rehash() and .getwtxid() instead to improve readability and in some cases avoid a conversion from string-txid to an integer. --- test/functional/feature_segwit.py | 6 +++--- test/functional/p2p_compactblocks.py | 11 ++++------- test/functional/p2p_segwit.py | 5 ++--- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index f98f326e8f4..257bc44d4ca 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -267,7 +267,7 @@ class SegWitTest(BitcoinTestFramework): tx1 = tx_from_hex(tx1_hex) # Check that wtxid is properly reported in mempool entry (txid1) - assert_equal(int(self.nodes[0].getmempoolentry(txid1)["wtxid"], 16), tx1.calc_sha256(True)) + assert_equal(self.nodes[0].getmempoolentry(txid1)["wtxid"], tx1.getwtxid()) # Check that weight and vsize are properly reported in mempool entry (txid1) assert_equal(self.nodes[0].getmempoolentry(txid1)["vsize"], tx1.get_vsize()) @@ -283,7 +283,7 @@ class SegWitTest(BitcoinTestFramework): assert not tx.wit.is_null() # Check that wtxid is properly reported in mempool entry (txid2) - assert_equal(int(self.nodes[0].getmempoolentry(txid2)["wtxid"], 16), tx.calc_sha256(True)) + assert_equal(self.nodes[0].getmempoolentry(txid2)["wtxid"], tx.getwtxid()) # Check that weight and vsize are properly reported in mempool entry (txid2) assert_equal(self.nodes[0].getmempoolentry(txid2)["vsize"], tx.get_vsize()) @@ -306,7 +306,7 @@ class SegWitTest(BitcoinTestFramework): assert txid3 in template_txids # Check that wtxid is properly reported in mempool entry (txid3) - assert_equal(int(self.nodes[0].getmempoolentry(txid3)["wtxid"], 16), tx.calc_sha256(True)) + assert_equal(self.nodes[0].getmempoolentry(txid3)["wtxid"], tx.getwtxid()) # Check that weight and vsize are properly reported in mempool entry (txid3) assert_equal(self.nodes[0].getmempoolentry(txid3)["vsize"], tx.get_vsize()) diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index ca36b2fbc06..e6f62881110 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -347,13 +347,11 @@ class CompactBlocksTest(BitcoinTestFramework): # Check that all prefilled_txn entries match what's in the block. for entry in header_and_shortids.prefilled_txn: - entry.tx.calc_sha256() # This checks the non-witness parts of the tx agree - assert_equal(entry.tx.sha256, block.vtx[entry.index].sha256) + assert_equal(entry.tx.rehash(), block.vtx[entry.index].rehash()) # And this checks the witness - wtxid = entry.tx.calc_sha256(True) - assert_equal(wtxid, block.vtx[entry.index].calc_sha256(True)) + assert_equal(entry.tx.getwtxid(), block.vtx[entry.index].getwtxid()) # Check that the cmpctblock message announced all the transactions. assert_equal(len(header_and_shortids.prefilled_txn) + len(header_and_shortids.shortids), len(block.vtx)) @@ -590,10 +588,9 @@ class CompactBlocksTest(BitcoinTestFramework): all_indices = msg.block_txn_request.to_absolute() for index in all_indices: tx = test_node.last_message["blocktxn"].block_transactions.transactions.pop(0) - tx.calc_sha256() - assert_equal(tx.sha256, block.vtx[index].sha256) + assert_equal(tx.rehash(), block.vtx[index].rehash()) # Check that the witness matches - assert_equal(tx.calc_sha256(True), block.vtx[index].calc_sha256(True)) + assert_equal(tx.getwtxid(), block.vtx[index].getwtxid()) test_node.last_message.pop("blocktxn", None) current_height -= 1 diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 9caf5a19aad..7df59fe0d54 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -336,8 +336,7 @@ class SegWitTest(BitcoinTestFramework): # Verify the hash with witness differs from the txid # (otherwise our testing framework must be broken!) - tx.rehash() - assert tx.sha256 != tx.calc_sha256(with_witness=True) + assert tx.rehash() != tx.getwtxid() # Construct a block that includes the transaction. block = self.build_next_block() @@ -1293,7 +1292,7 @@ class SegWitTest(BitcoinTestFramework): # Test that getrawtransaction returns correct witness information # hash, size, vsize raw_tx = self.nodes[0].getrawtransaction(tx3.hash, 1) - assert_equal(int(raw_tx["hash"], 16), tx3.calc_sha256(True)) + assert_equal(raw_tx["hash"], tx3.getwtxid()) assert_equal(raw_tx["size"], len(tx3.serialize_with_witness())) vsize = tx3.get_vsize() assert_equal(raw_tx["vsize"], vsize)