Merge #13669: Tests: Cleanup create_transaction implementations

44bbceeef1 [Tests] Cleanup feature_block.py, remove unnecessary PreviousSpendableOutput object (Conor Scott)
736f941424 [Tests] Cleanup extra instances of create_transaction (Conor Scott)
157651855f [Tests] Rename create_tx and move to blocktools.py (Conor Scott)

Pull request description:

  There currently exist seven ([1](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_cltv.py#L52-L60), [2](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_csv_activation.py#L88-L95) [3](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_dersig.py#L40-L48), [4](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_nulldummy.py#L100-L108), [5](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/util.py#L529-L535), [6](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/blocktools.py#L120-L129), [7](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_block.py#L1218-L1220)) implementations of a function called something similar to `create_transaction` in the functional tests, some of which are exact copies of each other.

  This PR aims to clean this up into [three different cases implemented in blocktools.py](https://github.com/conscott/bitcoin/blob/create_tx_cleanup/test/functional/test_framework/blocktools.py#L121-L149)
  1. `create_tx_with_script`: Return transaction object spending generic tx output optionally specifying scriptSig and scriptPubKey
  2. `create_transaction`: Return transaction object spending coinbase tx
  2. `create_raw_transaction`: Return raw transaction (hex string) spending coinbase tx

  I am not committed to any of these function names, so I'll gladly take suggestions on there.

  Additionally there are some related cleanups to feature_block.py tests, specifically removing the [PreviousSpendableOutput](https://github.com/conscott/bitcoin/blob/master/test/functional/feature_block.py#L51-L54) object, which seems like an unnecessary layer given that every instance spends the 0 output.

Tree-SHA512: 63c6233b6f0942c81ba1ca67ea6770809b8c9409314c6d4cf8e5a3991cb9ee92b22bebe88c0dde45cd71e754eb351230c4c404b70ff118f5f43c034452ada65c

1 files changed, 7 insertions, 19 deletions

diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py
index bd41913e0d..21255c89c8 100755
--- a/test/functional/feature_nulldummy.py
+++ b/test/functional/feature_nulldummy.py
@@ -16,9 +16,8 @@ Generate 427 more blocks.
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 from test_framework.messages import CTransaction
-from test_framework.blocktools import create_coinbase, create_block, add_witness_commitment
+from test_framework.blocktools import create_coinbase, create_block, create_transaction, add_witness_commitment
 from test_framework.script import CScript
-from io import BytesIO
 import time
 
 NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero) (code 64)"
@@ -61,16 +60,16 @@ class NULLDUMMYTest(BitcoinTestFramework):
         self.lastblocktime = int(time.time()) + 429
 
         self.log.info("Test 1: NULLDUMMY compliant base transactions should be accepted to mempool and mined before activation [430]")
-        test1txs = [self.create_transaction(self.nodes[0], coinbase_txid[0], self.ms_address, 49)]
+        test1txs = [create_transaction(self.nodes[0], coinbase_txid[0], self.ms_address, 49)]
         txid1 = self.nodes[0].sendrawtransaction(bytes_to_hex_str(test1txs[0].serialize_with_witness()), True)
-        test1txs.append(self.create_transaction(self.nodes[0], txid1, self.ms_address, 48))
+        test1txs.append(create_transaction(self.nodes[0], txid1, self.ms_address, 48))
         txid2 = self.nodes[0].sendrawtransaction(bytes_to_hex_str(test1txs[1].serialize_with_witness()), True)
-        test1txs.append(self.create_transaction(self.nodes[0], coinbase_txid[1], self.wit_ms_address, 49))
+        test1txs.append(create_transaction(self.nodes[0], coinbase_txid[1], self.wit_ms_address, 49))
         txid3 = self.nodes[0].sendrawtransaction(bytes_to_hex_str(test1txs[2].serialize_with_witness()), True)
         self.block_submit(self.nodes[0], test1txs, False, True)
 
         self.log.info("Test 2: Non-NULLDUMMY base multisig transaction should not be accepted to mempool before activation")
-        test2tx = self.create_transaction(self.nodes[0], txid2, self.ms_address, 47)
+        test2tx = create_transaction(self.nodes[0], txid2, self.ms_address, 47)
         trueDummy(test2tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, bytes_to_hex_str(test2tx.serialize_with_witness()), True)
 
@@ -78,14 +77,14 @@ class NULLDUMMYTest(BitcoinTestFramework):
         self.block_submit(self.nodes[0], [test2tx], False, True)
 
         self.log.info("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation")
-        test4tx = self.create_transaction(self.nodes[0], test2tx.hash, self.address, 46)
+        test4tx = create_transaction(self.nodes[0], test2tx.hash, self.address, 46)
         test6txs=[CTransaction(test4tx)]
         trueDummy(test4tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, bytes_to_hex_str(test4tx.serialize_with_witness()), True)
         self.block_submit(self.nodes[0], [test4tx])
 
         self.log.info("Test 5: Non-NULLDUMMY P2WSH multisig transaction invalid after activation")
-        test5tx = self.create_transaction(self.nodes[0], txid3, self.wit_address, 48)
+        test5tx = create_transaction(self.nodes[0], txid3, self.wit_address, 48)
         test6txs.append(CTransaction(test5tx))
         test5tx.wit.vtxinwit[0].scriptWitness.stack[0] = b'\x01'
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, bytes_to_hex_str(test5tx.serialize_with_witness()), True)
@@ -97,17 +96,6 @@ class NULLDUMMYTest(BitcoinTestFramework):
         self.block_submit(self.nodes[0], test6txs, True, True)
 
 
-    def create_transaction(self, node, txid, to_address, amount):
-        inputs = [{ "txid" : txid, "vout" : 0}]
-        outputs = { to_address : amount }
-        rawtx = node.createrawtransaction(inputs, outputs)
-        signresult = node.signrawtransactionwithwallet(rawtx)
-        tx = CTransaction()
-        f = BytesIO(hex_str_to_bytes(signresult['hex']))
-        tx.deserialize(f)
-        return tx
-
-
     def block_submit(self, node, txs, witness = False, accept = False):
         block = create_block(self.tip, create_coinbase(self.lastblockheight + 1), self.lastblocktime + 1)
         block.nVersion = 4