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

fafd674 test: Remove previous release check (MarcoFalke)

Pull request description:

  Now that the commit (7c08d81) 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 fafd674
  vincenzopalazzo:
    ACK bitcoin/bitcoin@fafd674

Tree-SHA512: fcb1a93f3bf9deb5f5c7327a7cd23be10ba09c9f4cbfa73ee2764a93c6ce7d6fa98ca32f2cf4023c20ab624aee601beec949fd02a57a3a658fdbd4be1a9ff338

Author: laanwj

test/functional/feature_taproot.py

@@ -10,7 +10,6 @@
     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.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 to_script(elem):
 # 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 to_script(elem):
 # - 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 predict_sigops_ratio(n, dummy_size):
 
     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 @@ def test_spenders(self, node, spenders, input_counts):
                 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 @@ def gen_test_vectors(self):
         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 @@ def gen_test_vectors(self):
         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 @@ def gen_test_vectors(self):
             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 @@ def pr(node):
         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__':

test/functional/test_runner.py

@@ -112,7 +112,6 @@
     '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',