Merge bitcoin/bitcoin#21945: test: add P2PK support to MiniWallet

4bea30169218e2f21e0c93a059966b41c8edd205 test: use P2PK-MiniWallet for feature_csv_activation.py (Sebastian Falbesoner)
dc7eb64e83f5b8e63f12729d5f77b1c920b136e4 test: MiniWallet: add P2PK support (Sebastian Falbesoner)

Pull request description:

  This PR adds support for creating and spending transactions with raw pubkey (P2PK) outputs to MiniWallet, [as suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/21900#discussion_r629524841). Using that  mode in the test `feature_csv_activation.py`, all txs submitted to the mempool follow the standard policy, i.e. `-acceptnonstdtxn=1` can be removed.

  Possible follow-ups:
  * Improve MiniWallet constructor Interface; an enum-like parameter instead of two booleans would probably be better
  * Look at other tests that could benefit from P2PK (e.g. feature_cltv.py?)
  * Check vsize also for P2PK txs (vsize varies due to signature, i.e. a range has to be asserted)

ACKs for top commit:
  laanwj:
    Code review ACK 4bea30169218e2f21e0c93a059966b41c8edd205

Tree-SHA512: 9b428e6b7cfde59a8c7955d5096cea88af1384a5f49723f00052e9884d819d952d20a5ab39bb02f9d8b6073769c44462aa265d84a33e33da33c2d21670c488a6

test/functional/feature_csv_activation.py

@@ -93,7 +93,6 @@ class BIP68_112_113Test(BitcoinTestFramework):
         self.extra_args = [[
             '-whitelist=noban@127.0.0.1',
             '-maxtipage=600100', '-dip3params=2000:2000',
-            '-acceptnonstdtxn=1',
             '-par=1',  # Use only one script thread to get the exact reject reason for testing
         ]]
         self.supports_cli = False
@@ -109,12 +108,14 @@ class BIP68_112_113Test(BitcoinTestFramework):
     def create_bip112special(self, input, txversion):
         tx = self.create_self_transfer_from_utxo(input)
         tx.nVersion = txversion
+        self.miniwallet.sign_tx(tx)
         tx.vin[0].scriptSig = CScript([-1, OP_CHECKSEQUENCEVERIFY, OP_DROP] + list(CScript(tx.vin[0].scriptSig)))
         return tx
 
     def create_bip112emptystack(self, input, txversion):
         tx = self.create_self_transfer_from_utxo(input)
         tx.nVersion = txversion
+        self.miniwallet.sign_tx(tx)
         tx.vin[0].scriptSig = CScript([OP_CHECKSEQUENCEVERIFY] + list(CScript(tx.vin[0].scriptSig)))
         return tx
 
@@ -132,6 +133,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
             tx = self.create_self_transfer_from_utxo(bip68inputs[i])
             tx.nVersion = txversion
             tx.vin[0].nSequence = locktime + locktime_delta
+            self.miniwallet.sign_tx(tx)
             tx.rehash()
             txs.append({'tx': tx, 'sdf': sdf, 'stf': stf})
 
@@ -149,6 +151,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
             else:  # vary nSequence instead, OP_CSV is fixed
                 tx.vin[0].nSequence = locktime + locktime_delta
             tx.nVersion = txversion
+            self.miniwallet.sign_tx(tx)
             if (varyOP_CSV):
                 tx.vin[0].scriptSig = CScript([locktime, OP_CHECKSEQUENCEVERIFY, OP_DROP] + list(CScript(tx.vin[0].scriptSig)))
             else:
@@ -184,7 +187,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
 
     def run_test(self):
         self.helper_peer = self.nodes[0].add_p2p_connection(P2PDataStore())
-        self.miniwallet = MiniWallet(self.nodes[0], raw_script=True)
+        self.miniwallet = MiniWallet(self.nodes[0], use_p2pk=True)
 
         self.log.info("Generate blocks in the past for coinbase outputs.")
         self.coinbase_blocks = self.miniwallet.generate(COINBASE_BLOCK_COUNT)  # blocks generated for inputs
@@ -291,7 +294,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
         success_txs = []
         # BIP113 tx, -1 CSV tx and empty stack CSV tx should succeed
         bip113tx_v1.nLockTime = self.last_block_time - 600 * 5  # = MTP of prior block (not <) but < time put on current block
-        bip113tx_v1.rehash()
+        self.miniwallet.sign_tx(bip113tx_v1)
         success_txs.append(bip113tx_v1)
         success_txs.append(bip112tx_special_v1)
         success_txs.append(bip112tx_emptystack_v1)
@@ -311,7 +314,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
         success_txs = []
         # BIP113 tx, -1 CSV tx and empty stack CSV tx should succeed
         bip113tx_v2.nLockTime = self.last_block_time - 600 * 5  # = MTP of prior block (not <) but < time put on current block
-        bip113tx_v2.rehash()
+        self.miniwallet.sign_tx(bip113tx_v2)
         success_txs.append(bip113tx_v2)
         success_txs.append(bip112tx_special_v2)
         success_txs.append(bip112tx_emptystack_v2)
@@ -337,16 +340,20 @@ class BIP68_112_113Test(BitcoinTestFramework):
         self.log.info("BIP 113 tests")
         # BIP 113 tests should now fail regardless of version number if nLockTime isn't satisfied by new rules
         bip113tx_v1.nLockTime = self.last_block_time - 600 * 5  # = MTP of prior block (not <) but < time put on current block
+        self.miniwallet.sign_tx(bip113tx_v1)
         bip113tx_v1.rehash()
         bip113tx_v2.nLockTime = self.last_block_time - 600 * 5  # = MTP of prior block (not <) but < time put on current block
+        self.miniwallet.sign_tx(bip113tx_v2)
         bip113tx_v2.rehash()
         for bip113tx in [bip113tx_v1, bip113tx_v2]:
             self.send_blocks([self.create_test_block([bip113tx])], success=False, reject_reason='bad-txns-nonfinal')
 
         # BIP 113 tests should now pass if the locktime is < MTP
         bip113tx_v1.nLockTime = self.last_block_time - 600 * 5 - 1  # < MTP of prior block
+        self.miniwallet.sign_tx(bip113tx_v1)
         bip113tx_v1.rehash()
         bip113tx_v2.nLockTime = self.last_block_time - 600 * 5 - 1  # < MTP of prior block
+        self.miniwallet.sign_tx(bip113tx_v2)
         bip113tx_v2.rehash()
         for bip113tx in [bip113tx_v1, bip113tx_v2]:
             self.send_blocks([self.create_test_block([bip113tx])])
@@ -471,6 +478,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
         time_txs = []
         for tx in [tx['tx'] for tx in bip112txs_vary_OP_CSV_v2 if not tx['sdf'] and tx['stf']]:
             tx.vin[0].nSequence = BASE_RELATIVE_LOCKTIME | SEQ_TYPE_FLAG
+            self.miniwallet.sign_tx(tx)
             tx.rehash()
             time_txs.append(tx)
 

test/functional/test_framework/wallet.py

@@ -6,6 +6,7 @@
 
 from decimal import Decimal
 from test_framework.address import ADDRESS_BCRT1_P2SH_OP_TRUE
+from test_framework.key import ECKey
 from typing import Optional
 from test_framework.messages import (
     COIN,
@@ -16,8 +17,11 @@ from test_framework.messages import (
 )
 from test_framework.script import (
     CScript,
+    SignatureHash,
+    OP_CHECKSIG,
     OP_TRUE,
     OP_NOP,
+    SIGHASH_ALL,
 )
 from test_framework.util import (
     assert_equal,
@@ -27,12 +31,20 @@ from test_framework.util import (
 
 
 class MiniWallet:
-    def __init__(self, test_node, *, raw_script=False):
+    def __init__(self, test_node, *, raw_script=False, use_p2pk=False):
         self._test_node = test_node
         self._utxos = []
-        if raw_script:
+        self._priv_key = None
         self._address = None
+
+        if raw_script:
             self._scriptPubKey = bytes(CScript([OP_TRUE]))
+        elif use_p2pk:
+            # use simple deterministic private key (k=1)
+            self._priv_key = ECKey()
+            self._priv_key.set((1).to_bytes(32, 'big'), True)
+            pub_key = self._priv_key.get_pubkey()
+            self._scriptPubKey = bytes(CScript([pub_key.get_bytes(), OP_CHECKSIG]))
         else:
             self._address = ADDRESS_BCRT1_P2SH_OP_TRUE
             self._scriptPubKey = hex_str_to_bytes(self._test_node.validateaddress(self._address)['scriptPubKey'])
@@ -50,6 +62,13 @@ class MiniWallet:
             if out['scriptPubKey']['hex'] == self._scriptPubKey.hex():
                 self._utxos.append({'txid': tx['txid'], 'vout': out['n'], 'value': out['value']})
 
+    def sign_tx(self, tx):
+        """Sign tx that has been created by MiniWallet in P2PK mode"""
+        assert self._priv_key is not None
+        (sighash, err) = SignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL)
+        assert err is None
+        tx.vin[0].scriptSig = CScript([self._priv_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))])
+
     def generate(self, num_blocks):
         """Generate blocks with coinbase outputs to the internal address, and append the outputs to the internal list"""
         blocks = self._test_node.generatetodescriptor(num_blocks, f'raw({self._scriptPubKey.hex()})')
@@ -99,6 +118,11 @@ class MiniWallet:
         tx.vout = [CTxOut(int(send_value * COIN), self._scriptPubKey)]
         if not self._address:
             # raw script
+            if self._priv_key is not None:
+                # P2PK, need to sign
+                self.sign_tx(tx)
+            else:
+                # anyone-can-spend
                 tx.vin[0].scriptSig = CScript([OP_NOP] * 24)  # pad to identical size
         else:
             tx.vin[0].scriptSig = CScript([CScript([OP_TRUE])])
@@ -107,6 +131,9 @@ class MiniWallet:
         tx_info = from_node.testmempoolaccept([tx_hex])[0]
         assert_equal(mempool_valid, tx_info['allowed'])
         if mempool_valid:
+            # TODO: for P2PK, vsize is not constant due to varying scriptSig length,
+            # so only check this for anyone-can-spend outputs right now
+            if self._priv_key is None:
                 assert_equal(len(tx_hex) // 2, vsize) # 1 byte = 2 character
             assert_equal(tx_info['fees']['base'], fee)
         return {'txid': tx_info['txid'], 'hex': tx_hex, 'tx': tx}