Merge bitcoin/bitcoin#24937: test: Remove previous release check in feature_taproot.py

fafd67479aa770dac227622c893a9998fa78e99c test: Remove previous release check (MarcoFalke)

Pull request description:

  Now that the commit (7c08d81e119570792648fe95bbacddbb1d5f9ae2) which changes taproot to be enforced for all blocks is sufficiently buried by other commits, and thus less likely to be reverted,  it seems a good time to remove no longer needed test code.

  The `feature_taproot` functional test is cleaned up to no longer run against a previous release. Since previous releases are static and impossible to change, it is sufficient to run the test once against the release. Now that this is done, the check can be removed without decreasing test coverage.

ACKs for top commit:
  laanwj:
    Concept and code review ACK fafd67479aa770dac227622c893a9998fa78e99c
  vincenzopalazzo:
    ACK https://github.com/bitcoin/bitcoin/pull/24937/commits/fafd67479aa770dac227622c893a9998fa78e99c

Tree-SHA512: fcb1a93f3bf9deb5f5c7327a7cd23be10ba09c9f4cbfa73ee2764a93c6ce7d6fa98ca32f2cf4023c20ab624aee601beec949fd02a57a3a658fdbd4be1a9ff338

2 files changed, 22 insertions, 89 deletions

diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py
index c3925dbb00..0e44038196 100755
--- a/test/functional/feature_taproot.py
+++ b/test/functional/feature_taproot.py
@@ -10,7 +10,6 @@ from test_framework.blocktools import (
     create_block,
     add_witness_commitment,
     MAX_BLOCK_SIGOPS_WEIGHT,
-    NORMAL_GBT_REQUEST_PARAMS,
     WITNESS_SCALE_FACTOR,
 )
 from test_framework.messages import (
@@ -96,10 +95,9 @@ from test_framework.util import assert_raises_rpc_error, assert_equal
 from test_framework.key import generate_privkey, compute_xonly_pubkey, sign_schnorr, tweak_add_privkey, ECKey
 from test_framework.address import (
     hash160,
-    program_to_witness
+    program_to_witness,
 )
 from collections import OrderedDict, namedtuple
-from enum import Enum
 from io import BytesIO
 import json
 import hashlib
@@ -458,7 +456,7 @@ def spend(tx, idx, utxos, **kwargs):
 # Each spender is a tuple of:
 # - A scriptPubKey which is to be spent from (CScript)
 # - A comment describing the test (string)
-# - Whether the spending (on itself) is expected to be standard (Enum.Standard)
+# - Whether the spending (on itself) is expected to be standard (bool)
 # - A tx-signing lambda returning (scriptsig, witness_stack), taking as inputs:
 #   - A transaction to sign (CTransaction)
 #   - An input position (int)
@@ -470,14 +468,9 @@ def spend(tx, idx, utxos, **kwargs):
 # - Whether this test demands being placed in a txin with no corresponding txout (for testing SIGHASH_SINGLE behavior)
 
 Spender = namedtuple("Spender", "script,comment,is_standard,sat_function,err_msg,sigops_weight,no_fail,need_vin_vout_mismatch")
-# The full node versions that treat the tx standard.
-# ALL means any version
-# V23 means the major version 23.0 and any later version
-# NONE means no version
-Standard = Enum('Standard', 'ALL V23 NONE')
 
 
-def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh=False, spk_mutate_pre_p2sh=None, failure=None, standard=Standard.ALL, err_msg=None, sigops_weight=0, need_vin_vout_mismatch=False, **kwargs):
+def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh=False, spk_mutate_pre_p2sh=None, failure=None, standard=True, err_msg=None, sigops_weight=0, need_vin_vout_mismatch=False, **kwargs):
     """Helper for constructing Spender objects using the context signing framework.
 
     * tap: a TaprootInfo object (see taproot_construct), for Taproot spends (cannot be combined with pkh, witv0, or script)
@@ -487,18 +480,13 @@ def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh=
     * p2sh: whether the output is P2SH wrapper (this is supported even for Taproot, where it makes the output unencumbered)
     * spk_mutate_pre_psh: a callable to be applied to the script (before potentially P2SH-wrapping it)
     * failure: a dict of entries to override in the context when intentionally failing to spend (if None, no_fail will be set)
-    * standard: whether the (valid version of) spending is expected to be standard (True is mapped to Standard.ALL, False is mapped to Standard.NONE)
+    * standard: whether the (valid version of) spending is expected to be standard
     * err_msg: a string with an expected error message for failure (or None, if not cared about)
     * sigops_weight: the pre-taproot sigops weight consumed by a successful spend
     * need_vin_vout_mismatch: whether this test requires being tested in a transaction input that has no corresponding
                               transaction output.
     """
 
-    if standard == True:
-        standard = Standard.ALL
-    elif standard == False:
-        standard = Standard.NONE
-
     conf = dict()
 
     # Compute scriptPubKey and set useful defaults based on the inputs.
@@ -1168,24 +1156,20 @@ def spenders_taproot_active():
 
     return spenders
 
-def spenders_taproot_inactive():
-    """Spenders for testing that pre-activation Taproot rules don't apply."""
+
+def spenders_taproot_nonstandard():
+    """Spenders for testing that post-activation Taproot rules may be nonstandard."""
 
     spenders = []
 
     sec = generate_privkey()
     pub, _ = compute_xonly_pubkey(sec)
     scripts = [
-        ("pk", CScript([pub, OP_CHECKSIG])),
         ("future_leaf", CScript([pub, OP_CHECKSIG]), 0xc2),
         ("op_success", CScript([pub, OP_CHECKSIG, OP_0, OP_IF, CScriptOp(0x50), OP_ENDIF])),
     ]
     tap = taproot_construct(pub, scripts)
 
-    # Test that valid spending is standard.
-    add_spender(spenders, "inactive/keypath_valid", key=sec, tap=tap, standard=Standard.V23)
-    add_spender(spenders, "inactive/scriptpath_valid", key=sec, tap=tap, leaf="pk", standard=Standard.V23, inputs=[getter("sign")])
-
     # Test that features like annex, leaf versions, or OP_SUCCESS are valid but non-standard
     add_spender(spenders, "inactive/scriptpath_valid_unkleaf", key=sec, tap=tap, leaf="future_leaf", standard=False, inputs=[getter("sign")])
     add_spender(spenders, "inactive/scriptpath_invalid_unkleaf", key=sec, tap=tap, leaf="future_leaf", standard=False, inputs=[getter("sign")], sighash=bitflipper(default_sighash))
@@ -1214,7 +1198,7 @@ def dump_json_test(tx, input_utxos, idx, success, failure):
 
     # The "final" field indicates that a spend should be always valid, even with more validation flags enabled
     # than the listed ones. Use standardness as a proxy for this (which gives a conservative underestimate).
-    if spender.is_standard == Standard.ALL:
+    if spender.is_standard:
         fields.append(("final", True))
 
     def dump_witness(wit):
@@ -1241,31 +1225,14 @@ class TaprootTest(BitcoinTestFramework):
     def add_options(self, parser):
         parser.add_argument("--dumptests", dest="dump_tests", default=False, action="store_true",
                             help="Dump generated test cases to directory set by TEST_DUMP_DIR environment variable")
-        parser.add_argument("--previous_release", dest="previous_release", default=False, action="store_true",
-                            help="Use a previous release as taproot-inactive node")
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
-        if self.options.previous_release:
-            self.skip_if_no_previous_releases()
 
     def set_test_params(self):
-        self.num_nodes = 2
+        self.num_nodes = 1
         self.setup_clean_chain = True
-        # Node 0 has Taproot inactive, Node 1 active.
-        self.extra_args = [["-par=1"], ["-par=1"]]
-        if self.options.previous_release:
-            self.wallet_names = [None, self.default_wallet_name]
-        else:
-            self.extra_args[0].append("-vbparams=taproot:1:1")
-
-    def setup_nodes(self):
-        self.add_nodes(self.num_nodes, self.extra_args, versions=[
-            200100 if self.options.previous_release else None,
-            None,
-        ])
-        self.start_nodes()
-        self.import_deterministic_coinbase_privkeys()
+        self.extra_args = [["-par=1"]]
 
     def block_submit(self, node, txs, msg, err_msg, cb_pubkey=None, fees=0, sigops_weight=0, witness=False, accept=False):
 
@@ -1479,11 +1446,10 @@ class TaprootTest(BitcoinTestFramework):
                 for i in range(len(input_utxos)):
                     tx.vin[i].scriptSig = input_data[i][i != fail_input][0]
                     tx.wit.vtxinwit[i].scriptWitness.stack = input_data[i][i != fail_input][1]
-                taproot_spend_policy = Standard.V23 if node.version is None else Standard.ALL
                 # Submit to mempool to check standardness
                 is_standard_tx = (
                     fail_input is None  # Must be valid to be standard
-                    and (all(utxo.spender.is_standard == Standard.ALL or utxo.spender.is_standard == taproot_spend_policy for utxo in input_utxos))  # All inputs must be standard
+                    and (all(utxo.spender.is_standard for utxo in input_utxos))  # All inputs must be standard
                     and tx.nVersion >= 1  # The tx version must be standard
                     and tx.nVersion <= 2)
                 tx.rehash()
@@ -1510,7 +1476,7 @@ class TaprootTest(BitcoinTestFramework):
         self.log.info("Unit test scenario...")
 
         # Deterministically mine coins to OP_TRUE in block 1
-        assert self.nodes[1].getblockcount() == 0
+        assert_equal(self.nodes[0].getblockcount(), 0)
         coinbase = CTransaction()
         coinbase.nVersion = 1
         coinbase.vin = [CTxIn(COutPoint(0, 0xffffffff), CScript([OP_1, OP_1]), SEQUENCE_FINAL)]
@@ -1519,12 +1485,12 @@ class TaprootTest(BitcoinTestFramework):
         coinbase.rehash()
         assert coinbase.hash == "f60c73405d499a956d3162e3483c395526ef78286458a4cb17b125aa92e49b20"
         # Mine it
-        block = create_block(hashprev=int(self.nodes[1].getbestblockhash(), 16), coinbase=coinbase)
+        block = create_block(hashprev=int(self.nodes[0].getbestblockhash(), 16), coinbase=coinbase)
         block.rehash()
         block.solve()
-        self.nodes[1].submitblock(block.serialize().hex())
-        assert self.nodes[1].getblockcount() == 1
-        self.generate(self.nodes[1], COINBASE_MATURITY)
+        self.nodes[0].submitblock(block.serialize().hex())
+        assert_equal(self.nodes[0].getblockcount(), 1)
+        self.generate(self.nodes[0], COINBASE_MATURITY)
 
         SEED = 317
         VALID_LEAF_VERS = list(range(0xc0, 0x100, 2)) + [0x66, 0x7e, 0x80, 0x84, 0x96, 0x98, 0xba, 0xbc, 0xbe]
@@ -1613,8 +1579,8 @@ class TaprootTest(BitcoinTestFramework):
             spend_info[spk]['prevout'] = COutPoint(tx.sha256, i & 1)
             spend_info[spk]['utxo'] = CTxOut(val, spk)
         # Mine those transactions
-        self.init_blockinfo(self.nodes[1])
-        self.block_submit(self.nodes[1], txn, "Crediting txn", None, sigops_weight=10, accept=True)
+        self.init_blockinfo(self.nodes[0])
+        self.block_submit(self.nodes[0], txn, "Crediting txn", None, sigops_weight=10, accept=True)
 
         # scriptPubKey computation
         tests = {"version": 1}
@@ -1726,53 +1692,21 @@ class TaprootTest(BitcoinTestFramework):
         keypath_tests.append(tx_test)
         assert_equal(hashlib.sha256(tx.serialize()).hexdigest(), "24bab662cb55a7f3bae29b559f651674c62bcc1cd442d44715c0133939107b38")
         # Mine the spending transaction
-        self.block_submit(self.nodes[1], [tx], "Spending txn", None, sigops_weight=10000, accept=True, witness=True)
+        self.block_submit(self.nodes[0], [tx], "Spending txn", None, sigops_weight=10000, accept=True, witness=True)
 
         if GEN_TEST_VECTORS:
             print(json.dumps(tests, indent=4, sort_keys=False))
 
-
     def run_test(self):
         self.gen_test_vectors()
 
-        # Post-taproot activation tests go first (pre-taproot tests' blocks are invalid post-taproot).
         self.log.info("Post-activation tests...")
-        self.test_spenders(self.nodes[1], spenders_taproot_active(), input_counts=[1, 2, 2, 2, 2, 3])
-
-        # Re-connect nodes in case they have been disconnected
-        self.disconnect_nodes(0, 1)
-        self.connect_nodes(0, 1)
-
-        # Transfer value of the largest 500 coins to pre-taproot node.
-        addr = self.nodes[0].getnewaddress()
-
-        unsp = self.nodes[1].listunspent()
-        unsp = sorted(unsp, key=lambda i: i['amount'], reverse=True)
-        unsp = unsp[:500]
-
-        rawtx = self.nodes[1].createrawtransaction(
-            inputs=[{
-                'txid': i['txid'],
-                'vout': i['vout']
-            } for i in unsp],
-            outputs={addr: sum(i['amount'] for i in unsp)}
-        )
-        rawtx = self.nodes[1].signrawtransactionwithwallet(rawtx)['hex']
-
-        # Mine a block with the transaction
-        block = create_block(tmpl=self.nodes[1].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS), txlist=[rawtx])
-        add_witness_commitment(block)
-        block.solve()
-        assert_equal(None, self.nodes[1].submitblock(block.serialize().hex()))
-        self.sync_blocks()
-
-        # Pre-taproot activation tests.
-        self.log.info("Pre-activation tests...")
+        self.test_spenders(self.nodes[0], spenders_taproot_active(), input_counts=[1, 2, 2, 2, 2, 3])
         # Run each test twice; once in isolation, and once combined with others. Testing in isolation
         # means that the standardness is verified in every test (as combined transactions are only standard
         # when all their inputs are standard).
-        self.test_spenders(self.nodes[0], spenders_taproot_inactive(), input_counts=[1])
-        self.test_spenders(self.nodes[0], spenders_taproot_inactive(), input_counts=[2, 3])
+        self.test_spenders(self.nodes[0], spenders_taproot_nonstandard(), input_counts=[1])
+        self.test_spenders(self.nodes[0], spenders_taproot_nonstandard(), input_counts=[2, 3])
 
 
 if __name__ == '__main__':
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index d845e5e034..bb6f7d7e1a 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -112,7 +112,6 @@ BASE_SCRIPTS = [
     'p2p_tx_download.py',
     'mempool_updatefromblock.py',
     'wallet_dump.py --legacy-wallet',
-    'feature_taproot.py --previous_release',
     'feature_taproot.py',
     'rpc_signer.py',
     'wallet_signer.py --descriptors',