From e1d4a128e8c71a741c2435f949c1427929e151b7 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 12 Nov 2021 23:32:01 +0100 Subject: [PATCH 1/3] test: simplify and document NULLDUMMY-invalidation helper The function `trueDummy` in feature_nulldummy.py is currently more complicated than it needs to be. Rather than converting the scriptSig to a CScript and looping through it to build a new scriptSig with the modified push, simply directly replace the push of null (OP_0) with a push of one (OP_TRUE/OP_1). Note that on master, actually an element with the value of 0x51 is pushed (0x0151...) -- this was very likely not intended, as 0x51 is the script op-code for OP_TRUE, and also the function's name suggests that the "true" value shall be pushed. --- test/functional/feature_nulldummy.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 217a38050d..34491ffc1a 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -22,7 +22,10 @@ from test_framework.blocktools import ( create_transaction, ) from test_framework.messages import CTransaction -from test_framework.script import CScript +from test_framework.script import ( + OP_0, + OP_TRUE, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -33,15 +36,10 @@ NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argumen def trueDummy(tx): - scriptSig = CScript(tx.vin[0].scriptSig) - newscript = [] - for i in scriptSig: - if len(newscript) == 0: - assert len(i) == 0 - newscript.append(b'\x51') - else: - newscript.append(i) - tx.vin[0].scriptSig = CScript(newscript) + """Transform a NULLDUMMY compliant tx (i.e. scriptSig starts with OP_0) + to be non-NULLDUMMY compliant by replacing the dummy with OP_TRUE""" + assert_equal(tx.vin[0].scriptSig[0], OP_0) + tx.vin[0].scriptSig = bytes([OP_TRUE]) + tx.vin[0].scriptSig[1:] tx.rehash() From 5ba9f1ff598e30a70407f331fabcbe9b4e17315a Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 12 Nov 2021 23:56:05 +0100 Subject: [PATCH 2/3] test: refactor: rename NULLDUMMY-invalidation helper The name is changed to match the coding guidelines (snake case) and to be more descriptive. --- test/functional/feature_nulldummy.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 34491ffc1a..2055e04a96 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -35,7 +35,7 @@ from test_framework.util import ( NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)" -def trueDummy(tx): +def invalidate_nulldummy_tx(tx): """Transform a NULLDUMMY compliant tx (i.e. scriptSig starts with OP_0) to be non-NULLDUMMY compliant by replacing the dummy with OP_TRUE""" assert_equal(tx.vin[0].scriptSig[0], OP_0) @@ -92,7 +92,7 @@ class NULLDUMMYTest(BitcoinTestFramework): self.log.info("Test 2: Non-NULLDUMMY base multisig transaction should not be accepted to mempool before activation") test2tx = create_transaction(self.nodes[0], txid2, self.ms_address, amount=47) - trueDummy(test2tx) + invalidate_nulldummy_tx(test2tx) assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test2tx.serialize_with_witness().hex(), 0) self.log.info(f"Test 3: Non-NULLDUMMY base transactions should be accepted in a block before activation [{COINBASE_MATURITY + 4}]") @@ -101,7 +101,7 @@ class NULLDUMMYTest(BitcoinTestFramework): self.log.info("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation") test4tx = create_transaction(self.nodes[0], test2tx.hash, self.address, amount=46) test6txs = [CTransaction(test4tx)] - trueDummy(test4tx) + invalidate_nulldummy_tx(test4tx) assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test4tx.serialize_with_witness().hex(), 0) self.block_submit(self.nodes[0], [test4tx], accept=False) From f1ed30451f04af4b45b6368305722f715dcbf4fb Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 13 Nov 2021 00:11:25 +0100 Subject: [PATCH 3/3] test: refactor: simplify `block_submit` in feature_nulldummy.py The `create_block` helper accepts a list of txs that it includes in the created block, hence we don't have to do that manually. Also, rehashing before solving the block is not needed and can be removed. --- test/functional/feature_nulldummy.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 2055e04a96..9fc19395db 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -121,14 +121,9 @@ class NULLDUMMYTest(BitcoinTestFramework): tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert_equal(tmpl['previousblockhash'], self.lastblockhash) assert_equal(tmpl['height'], self.lastblockheight + 1) - block = create_block(tmpl=tmpl, ntime=self.lastblocktime + 1) - for tx in txs: - tx.rehash() - block.vtx.append(tx) - block.hashMerkleRoot = block.calc_merkle_root() + block = create_block(tmpl=tmpl, ntime=self.lastblocktime + 1, txlist=txs) if with_witness: add_witness_commitment(block) - block.rehash() block.solve() assert_equal(None if accept else NULLDUMMY_ERROR, node.submitblock(block.serialize().hex())) if accept: