Merge bitcoin/bitcoin#22257: test: refactor: various (de)serialization helpers cleanups/improvements

bdb8b9a test: doc: improve doc for `from_hex` helper (mention `to_hex` alternative) (Sebastian Falbesoner)
1914054 scripted-diff: test: rename `FromHex` to `from_hex` (Sebastian Falbesoner)
a79396f test: remove `ToHex` helper, use .serialize().hex() instead (Sebastian Falbesoner)
2ce7b47 test: introduce `tx_from_hex` helper for tx deserialization (Sebastian Falbesoner)

Pull request description:

  There are still many functional tests that perform conversions from a hex-string to a message object (deserialization) manually. This PR identifies all those instances and replaces them with a newly introduced helper `tx_from_hex`.

  Instances were found via
  * `git grep "deserialize.*BytesIO"`

  and some of them manually, when it were not one-liners.

  Further, the helper `ToHex` was removed and simply replaced by `.serialize().hex()`, since now both variants are in use (sometimes even within the same test) and using the helper doesn't really have an advantage in readability. (see discussion bitcoin/bitcoin#22257 (comment))

ACKs for top commit:
  MarcoFalke:
    review re-ACK bdb8b9a 😁

Tree-SHA512: e25d7dc85918de1d6755a5cea65471b07a743204c20ad1c2f71ff07ef48cc1b9ad3fe5f515c1efaba2b2e3d89384e7980380c5d81895f9826e2046808cd3266e

Author: MarcoFalke

contrib/signet/miner

@@ -23,7 +23,7 @@ PATH_BASE_TEST_FUNCTIONAL = os.path.abspath(os.path.join(PATH_BASE_CONTRIB_SIGNE
 sys.path.insert(0, PATH_BASE_TEST_FUNCTIONAL)
 
 from test_framework.blocktools import WITNESS_COMMITMENT_HEADER, script_BIP34_coinbase_height # noqa: E402
-from test_framework.messages import CBlock, CBlockHeader, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, FromHex, ToHex, deser_string, hash256, ser_compact_size, ser_string, ser_uint256, uint256_from_str # noqa: E402
+from test_framework.messages import CBlock, CBlockHeader, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, from_hex, deser_string, hash256, ser_compact_size, ser_string, ser_uint256, tx_from_hex, uint256_from_str # noqa: E402
 from test_framework.script import CScriptOp # noqa: E402
 
 logging.basicConfig(
@@ -37,7 +37,7 @@ RE_MULTIMINER = re.compile("^(\d+)(-(\d+))?/(\d+)$")
 
 # #### some helpers that could go into test_framework
 
-# like FromHex, but without the hex part
+# like from_hex, but without the hex part
 def FromBinary(cls, stream):
     """deserialize a binary stream (or bytes object) into an object"""
     # handle bytes object by turning it into a stream
@@ -195,7 +195,7 @@ def finish_block(block, signet_solution, grind_cmd):
         headhex = CBlockHeader.serialize(block).hex()
         cmd = grind_cmd.split(" ") + [headhex]
         newheadhex = subprocess.run(cmd, stdout=subprocess.PIPE, input=b"", check=True).stdout.strip()
-        newhead = FromHex(CBlockHeader(), newheadhex.decode('utf8'))
+        newhead = from_hex(CBlockHeader(), newheadhex.decode('utf8'))
         block.nNonce = newhead.nNonce
         block.rehash()
     return block
@@ -216,7 +216,7 @@ def generate_psbt(tmpl, reward_spk, *, blocktime=None):
         block.nTime = tmpl["mintime"]
     block.nBits = int(tmpl["bits"], 16)
     block.nNonce = 0
-    block.vtx = [cbtx] + [FromHex(CTransaction(), t["data"]) for t in tmpl["transactions"]]
+    block.vtx = [cbtx] + [tx_from_hex(t["data"]) for t in tmpl["transactions"]]
 
     witnonce = 0
     witroot = block.calc_witness_merkle_root()
@@ -274,7 +274,7 @@ def do_genpsbt(args):
 def do_solvepsbt(args):
     block, signet_solution = do_decode_psbt(sys.stdin.read())
     block = finish_block(block, signet_solution, args.grind_cmd)
-    print(ToHex(block))
+    print(block.serialize().hex())
 
 def nbits_to_target(nbits):
     shift = (nbits >> 24) & 0xff
@@ -503,7 +503,7 @@ def do_generate(args):
         block = finish_block(block, signet_solution, args.grind_cmd)
 
         # submit block
-        r = args.bcli("-stdin", "submitblock", input=ToHex(block).encode('utf8'))
+        r = args.bcli("-stdin", "submitblock", input=block.serialize().hex().encode('utf8'))
 
         # report
         bstr = "block" if is_mine else "backup block"

test/functional/feature_bip68_sequence.py

@@ -6,8 +6,19 @@
 
 import time
 
-from test_framework.blocktools import create_block, NORMAL_GBT_REQUEST_PARAMS, add_witness_commitment
-from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut, FromHex, ToHex
+from test_framework.blocktools import (
+    NORMAL_GBT_REQUEST_PARAMS,
+    add_witness_commitment,
+    create_block,
+)
+from test_framework.messages import (
+    COIN,
+    COutPoint,
+    CTransaction,
+    CTxIn,
+    CTxOut,
+    tx_from_hex,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -89,7 +100,7 @@ def test_disable_flag(self):
         tx1.vin = [CTxIn(COutPoint(int(utxo["txid"], 16), utxo["vout"]), nSequence=sequence_value)]
         tx1.vout = [CTxOut(value, DUMMY_P2WPKH_SCRIPT)]
 
-        tx1_signed = self.nodes[0].signrawtransactionwithwallet(ToHex(tx1))["hex"]
+        tx1_signed = self.nodes[0].signrawtransactionwithwallet(tx1.serialize().hex())["hex"]
         tx1_id = self.nodes[0].sendrawtransaction(tx1_signed)
         tx1_id = int(tx1_id, 16)
 
@@ -102,13 +113,13 @@ def test_disable_flag(self):
         tx2.vout = [CTxOut(int(value - self.relayfee * COIN), DUMMY_P2WPKH_SCRIPT)]
         tx2.rehash()
 
-        assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, ToHex(tx2))
+        assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, tx2.serialize().hex())
 
         # Setting the version back down to 1 should disable the sequence lock,
         # so this should be accepted.
         tx2.nVersion = 1
 
-        self.nodes[0].sendrawtransaction(ToHex(tx2))
+        self.nodes[0].sendrawtransaction(tx2.serialize().hex())
 
     # Calculate the median time past of a prior block ("confirmations" before
     # the current tip).
@@ -193,9 +204,9 @@ def test_sequence_lock_confirmed_inputs(self):
                 tx.vin.append(CTxIn(COutPoint(int(utxos[j]["txid"], 16), utxos[j]["vout"]), nSequence=sequence_value))
                 value += utxos[j]["amount"]*COIN
             # Overestimate the size of the tx - signatures should be less than 120 bytes, and leave 50 for the output
-            tx_size = len(ToHex(tx))//2 + 120*num_inputs + 50
+            tx_size = len(tx.serialize().hex())//2 + 120*num_inputs + 50
             tx.vout.append(CTxOut(int(value-self.relayfee*tx_size*COIN/1000), DUMMY_P2WPKH_SCRIPT))
-            rawtx = self.nodes[0].signrawtransactionwithwallet(ToHex(tx))["hex"]
+            rawtx = self.nodes[0].signrawtransactionwithwallet(tx.serialize().hex())["hex"]
 
             if (using_sequence_locks and not should_pass):
                 # This transaction should be rejected
@@ -215,7 +226,7 @@ def test_sequence_lock_unconfirmed_inputs(self):
 
         # Create a mempool tx.
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 2)
-        tx1 = FromHex(CTransaction(), self.nodes[0].getrawtransaction(txid))
+        tx1 = tx_from_hex(self.nodes[0].getrawtransaction(txid))
         tx1.rehash()
 
         # Anyone-can-spend mempool tx.
@@ -224,8 +235,8 @@ def test_sequence_lock_unconfirmed_inputs(self):
         tx2.nVersion = 2
         tx2.vin = [CTxIn(COutPoint(tx1.sha256, 0), nSequence=0)]
         tx2.vout = [CTxOut(int(tx1.vout[0].nValue - self.relayfee*COIN), DUMMY_P2WPKH_SCRIPT)]
-        tx2_raw = self.nodes[0].signrawtransactionwithwallet(ToHex(tx2))["hex"]
-        tx2 = FromHex(tx2, tx2_raw)
+        tx2_raw = self.nodes[0].signrawtransactionwithwallet(tx2.serialize().hex())["hex"]
+        tx2 = tx_from_hex(tx2_raw)
         tx2.rehash()
 
         self.nodes[0].sendrawtransaction(tx2_raw)
@@ -246,10 +257,10 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
 
             if (orig_tx.hash in node.getrawmempool()):
                 # sendrawtransaction should fail if the tx is in the mempool
-                assert_raises_rpc_error(-26, NOT_FINAL_ERROR, node.sendrawtransaction, ToHex(tx))
+                assert_raises_rpc_error(-26, NOT_FINAL_ERROR, node.sendrawtransaction, tx.serialize().hex())
             else:
                 # sendrawtransaction should succeed if the tx is not in the mempool
-                node.sendrawtransaction(ToHex(tx))
+                node.sendrawtransaction(tx.serialize().hex())
 
             return tx
 
@@ -299,7 +310,7 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
         utxos = self.nodes[0].listunspent()
         tx5.vin.append(CTxIn(COutPoint(int(utxos[0]["txid"], 16), utxos[0]["vout"]), nSequence=1))
         tx5.vout[0].nValue += int(utxos[0]["amount"]*COIN)
-        raw_tx5 = self.nodes[0].signrawtransactionwithwallet(ToHex(tx5))["hex"]
+        raw_tx5 = self.nodes[0].signrawtransactionwithwallet(tx5.serialize().hex())["hex"]
 
         assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, raw_tx5)
 
@@ -325,7 +336,7 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
             block.rehash()
             block.solve()
             tip = block.sha256
-            assert_equal(None if i == 1 else 'inconclusive', self.nodes[0].submitblock(ToHex(block)))
+            assert_equal(None if i == 1 else 'inconclusive', self.nodes[0].submitblock(block.serialize().hex()))
             tmpl = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
             tmpl['previousblockhash'] = '%x' % tip
             tmpl['transactions'] = []
@@ -348,7 +359,7 @@ def test_bip68_not_consensus(self):
         assert not softfork_active(self.nodes[0], 'csv')
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 2)
 
-        tx1 = FromHex(CTransaction(), self.nodes[0].getrawtransaction(txid))
+        tx1 = tx_from_hex(self.nodes[0].getrawtransaction(txid))
         tx1.rehash()
 
         # Make an anyone-can-spend transaction
@@ -358,11 +369,11 @@ def test_bip68_not_consensus(self):
         tx2.vout = [CTxOut(int(tx1.vout[0].nValue - self.relayfee*COIN), DUMMY_P2WPKH_SCRIPT)]
 
         # sign tx2
-        tx2_raw = self.nodes[0].signrawtransactionwithwallet(ToHex(tx2))["hex"]
-        tx2 = FromHex(tx2, tx2_raw)
+        tx2_raw = self.nodes[0].signrawtransactionwithwallet(tx2.serialize().hex())["hex"]
+        tx2 = tx_from_hex(tx2_raw)
         tx2.rehash()
 
-        self.nodes[0].sendrawtransaction(ToHex(tx2))
+        self.nodes[0].sendrawtransaction(tx2.serialize().hex())
 
         # Now make an invalid spend of tx2 according to BIP68
         sequence_value = 100 # 100 block relative locktime
@@ -373,7 +384,7 @@ def test_bip68_not_consensus(self):
         tx3.vout = [CTxOut(int(tx2.vout[0].nValue - self.relayfee * COIN), DUMMY_P2WPKH_SCRIPT)]
         tx3.rehash()
 
-        assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, ToHex(tx3))
+        assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, tx3.serialize().hex())
 
         # make a block that violates bip68; ensure that the tip updates
         block = create_block(tmpl=self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS))
@@ -404,9 +415,9 @@ def test_version2_relay(self):
         outputs = { self.nodes[1].getnewaddress() : 1.0 }
         rawtx = self.nodes[1].createrawtransaction(inputs, outputs)
         rawtxfund = self.nodes[1].fundrawtransaction(rawtx)['hex']
-        tx = FromHex(CTransaction(), rawtxfund)
+        tx = tx_from_hex(rawtxfund)
         tx.nVersion = 2
-        tx_signed = self.nodes[1].signrawtransactionwithwallet(ToHex(tx))["hex"]
+        tx_signed = self.nodes[1].signrawtransactionwithwallet(tx.serialize().hex())["hex"]
         self.nodes[1].sendrawtransaction(tx_signed)
 
 if __name__ == '__main__':

test/functional/feature_coinstatsindex.py

@@ -22,7 +22,6 @@
     CTransaction,
     CTxIn,
     CTxOut,
-    ToHex,
 )
 from test_framework.script import (
     CScript,
@@ -170,7 +169,7 @@ def _test_coin_stats_index(self):
         tx2 = CTransaction()
         tx2.vin.append(CTxIn(COutPoint(int(tx1_txid, 16), n), b''))
         tx2.vout.append(CTxOut(int(20.99 * COIN), CScript([OP_RETURN] + [OP_FALSE]*30)))
-        tx2_hex = self.nodes[0].signrawtransactionwithwallet(ToHex(tx2))['hex']
+        tx2_hex = self.nodes[0].signrawtransactionwithwallet(tx2.serialize().hex())['hex']
         self.nodes[0].sendrawtransaction(tx2_hex)
 
         # Include both txs in a block
@@ -207,7 +206,7 @@ def _test_coin_stats_index(self):
         block_time = self.nodes[0].getblock(tip)['time'] + 1
         block = create_block(int(tip, 16), cb, block_time)
         block.solve()
-        self.nodes[0].submitblock(ToHex(block))
+        self.nodes[0].submitblock(block.serialize().hex())
         self.sync_all()
 
         self.wait_until(lambda: not try_rpc(-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, 'muhash'))

test/functional/feature_dbcrash.py

@@ -36,7 +36,6 @@
     CTransaction,
     CTxIn,
     CTxOut,
-    ToHex,
 )
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -208,7 +207,7 @@ def generate_small_transactions(self, node, count, utxo_list):
                 tx.vout.append(CTxOut(output_amount, hex_str_to_bytes(utxo['scriptPubKey'])))
 
             # Sign and send the transaction to get into the mempool
-            tx_signed_hex = node.signrawtransactionwithwallet(ToHex(tx))['hex']
+            tx_signed_hex = node.signrawtransactionwithwallet(tx.serialize().hex())['hex']
             node.sendrawtransaction(tx_signed_hex)
             num_transactions += 1
 

test/functional/feature_fee_estimation.py

@@ -6,8 +6,23 @@
 from decimal import Decimal
 import random
 
-from test_framework.messages import CTransaction, CTxIn, CTxOut, COutPoint, ToHex, COIN
-from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
+from test_framework.messages import (
+    COIN,
+    COutPoint,
+    CTransaction,
+    CTxIn,
+    CTxOut,
+)
+from test_framework.script import (
+    CScript,
+    OP_1,
+    OP_2,
+    OP_DROP,
+    OP_EQUAL,
+    OP_HASH160,
+    OP_TRUE,
+    hash160,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -64,11 +79,11 @@ def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee
     # the ScriptSig that will satisfy the ScriptPubKey.
     for inp in tx.vin:
         inp.scriptSig = SCRIPT_SIG[inp.prevout.n]
-    txid = from_node.sendrawtransaction(hexstring=ToHex(tx), maxfeerate=0)
+    txid = from_node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0)
     unconflist.append({"txid": txid, "vout": 0, "amount": total_in - amount - fee})
     unconflist.append({"txid": txid, "vout": 1, "amount": amount})
 
-    return (ToHex(tx), fee)
+    return (tx.serialize().hex(), fee)
 
 
 def split_inputs(from_node, txins, txouts, initial_split=False):
@@ -91,10 +106,10 @@ def split_inputs(from_node, txins, txouts, initial_split=False):
     # If this is the initial split we actually need to sign the transaction
     # Otherwise we just need to insert the proper ScriptSig
     if (initial_split):
-        completetx = from_node.signrawtransactionwithwallet(ToHex(tx))["hex"]
+        completetx = from_node.signrawtransactionwithwallet(tx.serialize().hex())["hex"]
     else:
         tx.vin[0].scriptSig = SCRIPT_SIG[prevtxout["vout"]]
-        completetx = ToHex(tx)
+        completetx = tx.serialize().hex()
     txid = from_node.sendrawtransaction(hexstring=completetx, maxfeerate=0)
     txouts.append({"txid": txid, "vout": 0, "amount": half_change})
     txouts.append({"txid": txid, "vout": 1, "amount": rem_change})

test/functional/feature_pruning.py

@@ -11,8 +11,12 @@
 import os
 
 from test_framework.blocktools import create_coinbase
-from test_framework.messages import CBlock, ToHex
-from test_framework.script import CScript, OP_RETURN, OP_NOP
+from test_framework.messages import CBlock
+from test_framework.script import (
+    CScript,
+    OP_NOP,
+    OP_RETURN,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -62,7 +66,7 @@ def mine_large_blocks(node, n):
         block.solve()
 
         # Submit to the node
-        node.submitblock(ToHex(block))
+        node.submitblock(block.serialize().hex())
 
         previousblockhash = block.sha256
         height += 1