Merge bitcoin/bitcoin#28070: test: Drop 22.x node from TxindexCompatibilityTest

fafe43c scripted-diff: Use blocks_path where possible (MarcoFalke)
fa060c1 test: Add blocks_path property to TestNode (MarcoFalke)
faba4fc test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke)
fa7f65b test: Use clean chain in MempoolCompatibilityTest (MarcoFalke)

Pull request description:

  The node in this test was never really needed, because the compatibility tests shouldn't be used to test previous releases. (The test suite of the previous release itself should be used for that). So remove it.

  Also, other test changes. (See individual commits)

ACKs for top commit:
  theStack:
    Code-review ACK fafe43c

Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc

Author: fanquake

test/functional/feature_abortnode.py

@@ -25,7 +25,7 @@ def run_test(self):
         self.generate(self.nodes[0], 3, sync_fun=self.no_op)
 
         # Deleting the undo file will result in reorg failure
-        (self.nodes[0].chain_path / "blocks" / "rev00000.dat").unlink()
+        (self.nodes[0].blocks_path / "rev00000.dat").unlink()
 
         # Connecting to a node with a more work chain will trigger a reorg
         # attempt.

test/functional/feature_dirsymlinks.py

@@ -26,7 +26,7 @@ def run_test(self):
         self.stop_node(0)
 
         rename_and_link(
-            from_name=self.nodes[0].chain_path / "blocks",
+            from_name=self.nodes[0].blocks_path,
             to_name=dir_new_blocks,
         )
         rename_and_link(

test/functional/feature_loadblock.py

@@ -37,7 +37,7 @@ def run_test(self):
         cfg_file = os.path.join(data_dir, "linearize.cfg")
         bootstrap_file = os.path.join(self.options.tmpdir, "bootstrap.dat")
         genesis_block = self.nodes[0].getblockhash(0)
-        blocks_dir = self.nodes[0].chain_path / "blocks"
+        blocks_dir = self.nodes[0].blocks_path
         hash_list = tempfile.NamedTemporaryFile(dir=data_dir,
                                                 mode='w',
                                                 delete=False,

test/functional/feature_reindex.py

@@ -38,7 +38,7 @@ def out_of_order(self):
         # In this test environment, blocks will always be in order (since
         # we're generating them rather than getting them from peers), so to
         # test out-of-order handling, swap blocks 1 and 2 on disk.
-        blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat"
+        blk0 = self.nodes[0].blocks_path / "blk00000.dat"
         with open(blk0, 'r+b') as bf:
             # Read at least the first few blocks (including genesis)
             b = bf.read(2000)

test/functional/feature_remove_pruned_files_on_startup.py

@@ -20,10 +20,10 @@ def mine_batches(self, blocks):
         self.sync_blocks()
 
     def run_test(self):
-        blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat"
-        rev0 = self.nodes[0].chain_path / "blocks" / "rev00000.dat"
-        blk1 = self.nodes[0].chain_path / "blocks" / "blk00001.dat"
-        rev1 = self.nodes[0].chain_path / "blocks" / "rev00001.dat"
+        blk0 = self.nodes[0].blocks_path / "blk00000.dat"
+        rev0 = self.nodes[0].blocks_path / "rev00000.dat"
+        blk1 = self.nodes[0].blocks_path / "blk00001.dat"
+        rev1 = self.nodes[0].blocks_path / "rev00001.dat"
         self.mine_batches(800)
         fo1 = os.open(blk0, os.O_RDONLY)
         fo2 = os.open(rev1, os.O_RDONLY)

test/functional/feature_txindex_compatibility.py

@@ -11,16 +11,16 @@
 import shutil
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_raises_rpc_error
 from test_framework.wallet import MiniWallet
 
 
 class TxindexCompatibilityTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.num_nodes = 3
+        self.num_nodes = 2
         self.extra_args = [
             ["-reindex", "-txindex"],
             [],
-            [],
         ]
 
     def skip_test_if_missing_module(self):
@@ -33,12 +33,10 @@ def setup_network(self):
             versions=[
                 160300,  # Last release with legacy txindex
                 None,  # For MiniWallet, without migration code
-                220000,  # Last release with migration code (0.17.x - 22.x)
             ],
         )
         self.start_nodes()
         self.connect_nodes(0, 1)
-        self.connect_nodes(1, 2)
 
     def run_test(self):
         mini_wallet = MiniWallet(self.nodes[1])
@@ -47,22 +45,12 @@ def run_test(self):
         self.generate(self.nodes[1], 1)
 
         self.log.info("Check legacy txindex")
+        assert_raises_rpc_error(-5, "Use -txindex", lambda: self.nodes[1].getrawtransaction(txid=spend_utxo["txid"]))
         self.nodes[0].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
 
         self.stop_nodes()
         legacy_chain_dir = self.nodes[0].chain_path
 
-        self.log.info("Migrate legacy txindex")
-        migrate_chain_dir = self.nodes[2].chain_path
-        shutil.rmtree(migrate_chain_dir)
-        shutil.copytree(legacy_chain_dir, migrate_chain_dir)
-        with self.nodes[2].assert_debug_log([
-                "Upgrading txindex database...",
-                "txindex is enabled at height 200",
-        ]):
-            self.start_node(2, extra_args=["-txindex"])
-        self.nodes[2].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
-
         self.log.info("Drop legacy txindex")
         drop_index_chain_dir = self.nodes[1].chain_path
         shutil.rmtree(drop_index_chain_dir)
@@ -73,16 +61,14 @@ def run_test(self):
         )
         # Build txindex from scratch and check there is no error this time
         self.start_node(1, extra_args=["-txindex"])
-        self.nodes[2].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
+        self.wait_until(lambda: self.nodes[1].getindexinfo()["txindex"]["synced"] == True)
+        self.nodes[1].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
 
         self.stop_nodes()
 
         self.log.info("Check migrated txindex cannot be read by legacy node")
         err_msg = f": You need to rebuild the database using -reindex to change -txindex.{os.linesep}Please restart with -reindex or -reindex-chainstate to recover."
         shutil.rmtree(legacy_chain_dir)
-        shutil.copytree(migrate_chain_dir, legacy_chain_dir)
-        self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg)
-        shutil.rmtree(legacy_chain_dir)
         shutil.copytree(drop_index_chain_dir, legacy_chain_dir)
         self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg)
 

test/functional/feature_unsupported_utxo_db.py

@@ -40,9 +40,9 @@ def run_test(self):
 
         self.log.info("Check init error")
         legacy_utxos_dir = self.nodes[0].chain_path / "chainstate"
-        legacy_blocks_dir = self.nodes[0].chain_path / "blocks"
+        legacy_blocks_dir = self.nodes[0].blocks_path
         recent_utxos_dir = self.nodes[1].chain_path / "chainstate"
-        recent_blocks_dir = self.nodes[1].chain_path / "blocks"
+        recent_blocks_dir = self.nodes[1].blocks_path
         shutil.copytree(legacy_utxos_dir, recent_utxos_dir)
         shutil.copytree(legacy_blocks_dir, recent_blocks_dir)
         self.nodes[1].assert_start_raises_init_error(

test/functional/mempool_compatibility.py

@@ -10,8 +10,6 @@
 Previous releases are required by this test, see test/README.md.
 """
 
-import os
-
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.wallet import (
@@ -23,6 +21,7 @@
 class MempoolCompatibilityTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
+        self.setup_clean_chain = True
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_previous_releases()
@@ -55,9 +54,9 @@ def run_test(self):
         self.stop_node(1)
 
         self.log.info("Move mempool.dat from old to new node")
-        old_node_mempool = os.path.join(old_node.chain_path, 'mempool.dat')
-        new_node_mempool = os.path.join(new_node.chain_path, 'mempool.dat')
-        os.rename(old_node_mempool, new_node_mempool)
+        old_node_mempool = old_node.chain_path / "mempool.dat"
+        new_node_mempool = new_node.chain_path / "mempool.dat"
+        old_node_mempool.rename(new_node_mempool)
 
         self.log.info("Start new node and verify mempool contains the tx")
         self.start_node(1)
@@ -70,7 +69,7 @@ def run_test(self):
         self.stop_node(1)
 
         self.log.info("Move mempool.dat from new to old node")
-        os.rename(new_node_mempool, old_node_mempool)
+        new_node_mempool.rename(old_node_mempool)
 
         self.log.info("Start old node again and verify mempool contains both txs")
         self.start_node(0, ['-nowallet'])

test/functional/rpc_blockchain.py

@@ -577,8 +577,8 @@ def assert_vin_does_not_contain_prevout(verbosity):
         self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data")
 
         def move_block_file(old, new):
-            old_path = self.nodes[0].chain_path / "blocks" / old
-            new_path = self.nodes[0].chain_path / "blocks" / new
+            old_path = self.nodes[0].blocks_path / old
+            new_path = self.nodes[0].blocks_path / new
             old_path.rename(new_path)
 
         # Move instead of deleting so we can restore chain state afterwards

test/functional/test_framework/test_node.py

@@ -420,6 +420,10 @@ def chain_path(self) -> Path:
     def debug_log_path(self) -> Path:
         return self.chain_path / 'debug.log'
 
+    @property
+    def blocks_path(self) -> Path:
+        return self.chain_path / "blocks"
+
     @property
     def wallets_path(self) -> Path:
         return self.chain_path / "wallets"