From fa52eb55c9fe3f7f8517a5804a55edb7f7b3a625 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 8 May 2019 10:03:06 -0400 Subject: test: Remove True argument to CBlock::serialize Unnamed arguments are confusing as to what they mean without looking up the function signature. Since segwit is active by default in regtest, and all blocks are serialized with witness (#15664, c459c5f), remove the argument `with_witness=True` from all calls to `CBlock::serialize` and `BlockTransactions::serialize`. This diff has been created with a script, but is better reviewed without a scripted diff. sed -i --regexp-extended -e 's/block(_?[2a-z]*)\.serialize\([a-z_]*=?True/block\1.serialize(/g' $(git grep -l serialize ./test) --- test/functional/feature_bip68_sequence.py | 2 +- test/functional/feature_nulldummy.py | 2 +- test/functional/p2p_segwit.py | 24 ++++++++++++------------ test/functional/test_framework/messages.py | 4 ++-- test/functional/wallet_bumpfee.py | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) (limited to 'test') diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index fdb60fb0e8..d38eca6cbe 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -378,7 +378,7 @@ class BIP68Test(BitcoinTestFramework): add_witness_commitment(block) block.solve() - self.nodes[0].submitblock(block.serialize(True).hex()) + self.nodes[0].submitblock(block.serialize().hex()) assert_equal(self.nodes[0].getbestblockhash(), block.hash) def activateCSV(self): diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index a56c983ccc..60a703c48f 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -108,7 +108,7 @@ class NULLDUMMYTest(BitcoinTestFramework): witness and add_witness_commitment(block) block.rehash() block.solve() - node.submitblock(block.serialize(True).hex()) + node.submitblock(block.serialize().hex()) if (accept): assert_equal(node.getbestblockhash(), block.hash) self.tip = block.sha256 diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 000c30646a..512659f1d4 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -111,7 +111,7 @@ def get_virtual_size(witness_block): Virtual size is base + witness/4.""" base_size = len(witness_block.serialize(with_witness=False)) - total_size = len(witness_block.serialize(with_witness=True)) + total_size = len(witness_block.serialize()) # the "+3" is so we round up vsize = int((3 * base_size + total_size + 3) / 4) return vsize @@ -403,7 +403,7 @@ class SegWitTest(BitcoinTestFramework): block_hash = int(block_hash, 16) block = self.test_node.request_block(block_hash, 2) wit_block = self.test_node.request_block(block_hash, 2 | MSG_WITNESS_FLAG) - assert_equal(block.serialize(True), wit_block.serialize(True)) + assert_equal(block.serialize(), wit_block.serialize()) assert_equal(block.serialize(), hex_str_to_bytes(rpc_block)) else: # After activation, witness blocks and non-witness blocks should @@ -419,15 +419,15 @@ class SegWitTest(BitcoinTestFramework): rpc_block = self.nodes[0].getblock(block.hash, False) non_wit_block = self.test_node.request_block(block.sha256, 2) wit_block = self.test_node.request_block(block.sha256, 2 | MSG_WITNESS_FLAG) - assert_equal(wit_block.serialize(True), hex_str_to_bytes(rpc_block)) + assert_equal(wit_block.serialize(), hex_str_to_bytes(rpc_block)) assert_equal(wit_block.serialize(False), non_wit_block.serialize()) - assert_equal(wit_block.serialize(True), block.serialize(True)) + assert_equal(wit_block.serialize(), block.serialize()) # Test size, vsize, weight rpc_details = self.nodes[0].getblock(block.hash, True) - assert_equal(rpc_details["size"], len(block.serialize(True))) + assert_equal(rpc_details["size"], len(block.serialize())) assert_equal(rpc_details["strippedsize"], len(block.serialize(False))) - weight = 3 * len(block.serialize(False)) + len(block.serialize(True)) + weight = 3 * len(block.serialize(False)) + len(block.serialize()) assert_equal(rpc_details["weight"], weight) # Upgraded node should not ask for blocks from unupgraded @@ -884,13 +884,13 @@ class SegWitTest(BitcoinTestFramework): # We can't send over the p2p network, because this is too big to relay # TODO: repeat this test with a block that can be relayed - self.nodes[0].submitblock(block.serialize(True).hex()) + self.nodes[0].submitblock(block.serialize().hex()) assert self.nodes[0].getbestblockhash() != block.hash block.vtx[0].wit.vtxinwit[0].scriptWitness.stack.pop() assert get_virtual_size(block) < MAX_BLOCK_BASE_SIZE - self.nodes[0].submitblock(block.serialize(True).hex()) + self.nodes[0].submitblock(block.serialize().hex()) assert self.nodes[0].getbestblockhash() == block.hash @@ -969,7 +969,7 @@ class SegWitTest(BitcoinTestFramework): assert_equal(vsize, MAX_BLOCK_BASE_SIZE + 1) # Make sure that our test case would exceed the old max-network-message # limit - assert len(block.serialize(True)) > 2 * 1024 * 1024 + assert len(block.serialize()) > 2 * 1024 * 1024 test_witness_block(self.nodes[0], self.test_node, block, accepted=False) @@ -997,14 +997,14 @@ class SegWitTest(BitcoinTestFramework): add_witness_commitment(block, nonce=1) block.vtx[0].wit = CTxWitness() # drop the nonce block.solve() - self.nodes[0].submitblock(block.serialize(True).hex()) + self.nodes[0].submitblock(block.serialize().hex()) assert self.nodes[0].getbestblockhash() != block.hash # Now redo commitment with the standard nonce, but let bitcoind fill it in. add_witness_commitment(block, nonce=0) block.vtx[0].wit = CTxWitness() block.solve() - self.nodes[0].submitblock(block.serialize(True).hex()) + self.nodes[0].submitblock(block.serialize().hex()) assert_equal(self.nodes[0].getbestblockhash(), block.hash) # This time, add a tx with non-empty witness, but don't supply @@ -1019,7 +1019,7 @@ class SegWitTest(BitcoinTestFramework): block_2.vtx[0].vout.pop() block_2.vtx[0].wit = CTxWitness() - self.nodes[0].submitblock(block_2.serialize(True).hex()) + self.nodes[0].submitblock(block_2.serialize().hex()) # Tip should not advance! assert self.nodes[0].getbestblockhash() != block_2.hash diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 954ae3c4df..00190e4cbd 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1155,7 +1155,7 @@ class msg_generic: class msg_witness_block(msg_block): __slots__ = () def serialize(self): - r = self.block.serialize(with_witness=True) + r = self.block.serialize() return r @@ -1454,5 +1454,5 @@ class msg_witness_blocktxn(msg_blocktxn): def serialize(self): r = b"" - r += self.block_transactions.serialize(with_witness=True) + r += self.block_transactions.serialize() return r diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 568b1f28d8..4d9bacf299 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -360,7 +360,7 @@ def submit_block_with_tx(node, tx): block.hashMerkleRoot = block.calc_merkle_root() add_witness_commitment(block) block.solve() - node.submitblock(block.serialize(True).hex()) + node.submitblock(block.serialize().hex()) return block def test_no_more_inputs_fails(rbf_node, dest_address): -- cgit v1.2.3 From fa1d7667173eeae363d3729e3fc654057335cb44 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 8 May 2019 10:21:25 -0400 Subject: tests: Make msg_block a witness block This diff has been generated with the following script, but is better reviewed without looking at the script. # -BEGIN VERIFY SCRIPT- echo "Use msg_witness_block everywhere, except for tests that require msg_block" # This could be a separate commit, but it is combined with the # following scripts to reduce the overall diff sed -i -e 's/msg_block/msg_witness_block/g' ./test/functional/{feature_assumevalid,feature_cltv,feature_dersig,feature_versionbits_warning,p2p_fingerprint,p2p_sendheaders,p2p_unrequested_blocks,example_test,rpc_blockchain}.py echo "Rename msg_block to msg_no_witness_block" # Rename msg_block to msg_no_witness_block in all tests (not the # framework) sed -i -e 's/msg_block/msg_no_witness_block/g' $(git grep -l msg_block ./test/functional/*.py) # Derive msg_no_witness_block from msg_block # Make msg_block a witness block in messages.py patch -p1 --fuzz 0 << EOF diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 00190e4cbd..e454ed5987 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1133 +1133 @@ class msg_block: - return self.block.serialize(with_witness=False) + return self.block.serialize() @@ -1155 +1155 @@ class msg_generic: -class msg_witness_block(msg_block): +class msg_no_witness_block(msg_block): @@ -1158,2 +1158 @@ class msg_witness_block(msg_block): - r = self.block.serialize() - return r + return self.block.serialize(with_witness=False) @@ -1445 +1444 @@ class msg_blocktxn: - r += self.block_transactions.serialize(with_witness=False) + r += self.block_transactions.serialize() @@ -1452 +1451 @@ class msg_blocktxn: -class msg_witness_blocktxn(msg_blocktxn): +class msg_no_witness_blocktxn(msg_blocktxn): @@ -1456,3 +1455 @@ class msg_witness_blocktxn(msg_blocktxn): - r = b"" - r += self.block_transactions.serialize() - return r + return self.block_transactions.serialize(with_witness=False) EOF # Conclude rename of msg_block to msg_no_witness_block sed -i -e 's/msg_witness_block/msg_block/g' $(git grep -l msg_witness_block) # -END VERIFY SCRIPT- --- test/functional/p2p_compactblocks.py | 24 ++++++++++++------------ test/functional/p2p_segwit.py | 12 ++++++------ test/functional/test_framework/messages.py | 15 ++++++--------- 3 files changed, 24 insertions(+), 27 deletions(-) (limited to 'test') diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 3ca6bec254..0994857912 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -10,7 +10,7 @@ Version 2 compact blocks are post-segwit (wtxids) import random from test_framework.blocktools import create_block, create_coinbase, add_witness_commitment -from test_framework.messages import BlockTransactions, BlockTransactionsRequest, calculate_shortid, CBlock, CBlockHeader, CInv, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, FromHex, HeaderAndShortIDs, msg_block, msg_blocktxn, msg_cmpctblock, msg_getblocktxn, msg_getdata, msg_getheaders, msg_headers, msg_inv, msg_sendcmpct, msg_sendheaders, msg_tx, msg_witness_block, msg_witness_blocktxn, MSG_WITNESS_FLAG, NODE_NETWORK, P2PHeaderAndShortIDs, PrefilledTransaction, ser_uint256, ToHex +from test_framework.messages import BlockTransactions, BlockTransactionsRequest, calculate_shortid, CBlock, CBlockHeader, CInv, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, FromHex, HeaderAndShortIDs, msg_no_witness_block, msg_no_witness_blocktxn, msg_cmpctblock, msg_getblocktxn, msg_getdata, msg_getheaders, msg_headers, msg_inv, msg_sendcmpct, msg_sendheaders, msg_tx, msg_block, msg_blocktxn, MSG_WITNESS_FLAG, NODE_NETWORK, P2PHeaderAndShortIDs, PrefilledTransaction, ser_uint256, ToHex from test_framework.mininode import mininode_lock, P2PInterface from test_framework.script import CScript, OP_TRUE, OP_DROP from test_framework.test_framework import BitcoinTestFramework @@ -114,7 +114,7 @@ class CompactBlocksTest(BitcoinTestFramework): # Create 10 more anyone-can-spend utxo's for testing. def make_utxos(self): block = self.build_block_on_tip(self.nodes[0]) - self.segwit_node.send_and_ping(msg_block(block)) + self.segwit_node.send_and_ping(msg_no_witness_block(block)) assert int(self.nodes[0].getbestblockhash(), 16) == block.sha256 self.nodes[0].generatetoaddress(100, self.nodes[0].getnewaddress(address_type="bech32")) @@ -130,7 +130,7 @@ class CompactBlocksTest(BitcoinTestFramework): block2.vtx.append(tx) block2.hashMerkleRoot = block2.calc_merkle_root() block2.solve() - self.segwit_node.send_and_ping(msg_block(block2)) + self.segwit_node.send_and_ping(msg_no_witness_block(block2)) assert_equal(int(self.nodes[0].getbestblockhash(), 16), block2.sha256) self.utxos.extend([[tx.sha256, i, out_value] for i in range(10)]) @@ -408,9 +408,9 @@ class CompactBlocksTest(BitcoinTestFramework): # Send the coinbase, and verify that the tip advances. if version == 2: - msg = msg_witness_blocktxn() - else: msg = msg_blocktxn() + else: + msg = msg_no_witness_blocktxn() msg.block_transactions.blockhash = block.sha256 msg.block_transactions.transactions = [block.vtx[0]] test_node.send_and_ping(msg) @@ -463,9 +463,9 @@ class CompactBlocksTest(BitcoinTestFramework): test_getblocktxn_response(comp_block, test_node, [1, 2, 3, 4, 5]) - msg_bt = msg_blocktxn() + msg_bt = msg_no_witness_blocktxn() if with_witness: - msg_bt = msg_witness_blocktxn() # serialize with witnesses + msg_bt = msg_blocktxn() # serialize with witnesses msg_bt.block_transactions = BlockTransactions(block.sha256, block.vtx[1:]) test_tip_after_message(node, test_node, msg_bt, block.sha256) @@ -554,9 +554,9 @@ class CompactBlocksTest(BitcoinTestFramework): # different peer provide the block further down, so that we're still # verifying that the block isn't marked bad permanently. This is good # enough for now. - msg = msg_blocktxn() + msg = msg_no_witness_blocktxn() if version == 2: - msg = msg_witness_blocktxn() + msg = msg_blocktxn() msg.block_transactions = BlockTransactions(block.sha256, [block.vtx[5]] + block.vtx[7:]) test_node.send_and_ping(msg) @@ -571,9 +571,9 @@ class CompactBlocksTest(BitcoinTestFramework): # Deliver the block if version == 2: - test_node.send_and_ping(msg_witness_block(block)) - else: test_node.send_and_ping(msg_block(block)) + else: + test_node.send_and_ping(msg_no_witness_block(block)) assert_equal(int(node.getbestblockhash(), 16), block.sha256) def test_getblocktxn_handler(self, test_node): @@ -785,7 +785,7 @@ class CompactBlocksTest(BitcoinTestFramework): delivery_peer.send_and_ping(msg_cmpctblock(cmpct_block.to_p2p())) assert int(node.getbestblockhash(), 16) != block.sha256 - msg = msg_blocktxn() + msg = msg_no_witness_blocktxn() msg.block_transactions.blockhash = block.sha256 msg.block_transactions.transactions = block.vtx[1:] stalling_peer.send_and_ping(msg) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 512659f1d4..271c5bbb0e 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -25,12 +25,12 @@ from test_framework.messages import ( MSG_WITNESS_FLAG, NODE_NETWORK, NODE_WITNESS, - msg_block, + msg_no_witness_block, msg_getdata, msg_headers, msg_inv, msg_tx, - msg_witness_block, + msg_block, msg_witness_tx, ser_uint256, ser_vector, @@ -134,7 +134,7 @@ def test_witness_block(node, p2p, block, accepted, with_witness=True, reason=Non - use the getbestblockhash rpc to check for acceptance.""" reason = [reason] if reason else [] with node.assert_debug_log(expected_msgs=reason): - p2p.send_message(msg_witness_block(block) if with_witness else msg_block(block)) + p2p.send_message(msg_block(block) if with_witness else msg_no_witness_block(block)) p2p.sync_with_ping() assert_equal(node.getbestblockhash() == block.hash, accepted) @@ -298,7 +298,7 @@ class SegWitTest(BitcoinTestFramework): block = self.build_next_block(version=1) block.solve() - self.test_node.send_message(msg_block(block)) + self.test_node.send_message(msg_no_witness_block(block)) self.test_node.sync_with_ping() # make sure the block was processed txid = block.vtx[0].sha256 @@ -345,7 +345,7 @@ class SegWitTest(BitcoinTestFramework): # But it should not be permanently marked bad... # Resend without witness information. - self.test_node.send_message(msg_block(block)) + self.test_node.send_message(msg_no_witness_block(block)) self.test_node.sync_with_ping() assert_equal(self.nodes[0].getbestblockhash(), block.hash) @@ -791,7 +791,7 @@ class SegWitTest(BitcoinTestFramework): block.solve() # Test the test -- witness serialization should be different - assert msg_witness_block(block).serialize() != msg_block(block).serialize() + assert msg_block(block).serialize() != msg_no_witness_block(block).serialize() # This empty block should be valid. test_witness_block(self.nodes[0], self.test_node, block, accepted=True) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 00190e4cbd..e454ed5987 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1130,7 +1130,7 @@ class msg_block: self.block.deserialize(f) def serialize(self): - return self.block.serialize(with_witness=False) + return self.block.serialize() def __repr__(self): return "msg_block(block=%s)" % (repr(self.block)) @@ -1152,11 +1152,10 @@ class msg_generic: return "msg_generic()" -class msg_witness_block(msg_block): +class msg_no_witness_block(msg_block): __slots__ = () def serialize(self): - r = self.block.serialize() - return r + return self.block.serialize(with_witness=False) class msg_getaddr: @@ -1442,17 +1441,15 @@ class msg_blocktxn: def serialize(self): r = b"" - r += self.block_transactions.serialize(with_witness=False) + r += self.block_transactions.serialize() return r def __repr__(self): return "msg_blocktxn(block_transactions=%s)" % (repr(self.block_transactions)) -class msg_witness_blocktxn(msg_blocktxn): +class msg_no_witness_blocktxn(msg_blocktxn): __slots__ = () def serialize(self): - r = b"" - r += self.block_transactions.serialize() - return r + return self.block_transactions.serialize(with_witness=False) -- cgit v1.2.3