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

test/functional/feature_dersig.py

@@ -10,9 +10,8 @@ Test that the DERSIG soft-fork activates at (regtest) height 1251.
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 from test_framework.mininode import *
-from test_framework.blocktools import create_coinbase, create_block
+from test_framework.blocktools import create_coinbase, create_block, create_transaction
 from test_framework.script import CScript
-from io import BytesIO
 
 DERSIG_HEIGHT = 1251
 
@@ -37,15 +36,6 @@ def unDERify(tx):
             newscript.append(i)
     tx.vin[0].scriptSig = CScript(newscript)
 
-def create_transaction(node, coinbase, to_address, amount):
-    from_txid = node.getblock(coinbase)['tx'][0]
-    inputs = [{ "txid" : from_txid, "vout" : 0}]
-    outputs = { to_address : amount }
-    rawtx = node.createrawtransaction(inputs, outputs)
-    signresult = node.signrawtransactionwithwallet(rawtx)
-    tx = CTransaction()
-    tx.deserialize(BytesIO(hex_str_to_bytes(signresult['hex'])))
-    return tx
 
 
 class BIP66Test(BitcoinTestFramework):
@@ -58,12 +48,12 @@ class BIP66Test(BitcoinTestFramework):
         self.nodes[0].add_p2p_connection(P2PInterface())
 
         self.log.info("Mining %d blocks", DERSIG_HEIGHT - 2)
-        self.coinbase_blocks = self.nodes[0].generate(DERSIG_HEIGHT - 2)
+        self.coinbase_txids = [self.nodes[0].getblock(b)['tx'][0] for b in self.nodes[0].generate(DERSIG_HEIGHT - 2)]
         self.nodeaddress = self.nodes[0].getnewaddress()
 
         self.log.info("Test that a transaction with non-DER signature can still appear in a block")
 
-        spendtx = create_transaction(self.nodes[0], self.coinbase_blocks[0],
+        spendtx = create_transaction(self.nodes[0], self.coinbase_txids[0],
                 self.nodeaddress, 1.0)
         unDERify(spendtx)
         spendtx.rehash()
@@ -100,7 +90,7 @@ class BIP66Test(BitcoinTestFramework):
         self.log.info("Test that transactions with non-DER signatures cannot appear in a block")
         block.nVersion = 3
 
-        spendtx = create_transaction(self.nodes[0], self.coinbase_blocks[1],
+        spendtx = create_transaction(self.nodes[0], self.coinbase_txids[1],
                 self.nodeaddress, 1.0)
         unDERify(spendtx)
         spendtx.rehash()
@@ -138,7 +128,7 @@ class BIP66Test(BitcoinTestFramework):
 
         self.log.info("Test that a version 3 block with a DERSIG-compliant transaction is accepted")
         block.vtx[1] = create_transaction(self.nodes[0],
-                self.coinbase_blocks[1], self.nodeaddress, 1.0)
+                self.coinbase_txids[1], self.nodeaddress, 1.0)
         block.hashMerkleRoot = block.calc_merkle_root()
         block.rehash()
         block.solve()