Merge bitcoin/bitcoin#27884: test: Use TestNode datadir_path or chain_path where possible

aaaa3ae test: Use TestNode *_path properties where possible (MarcoFalke)
dddd899 test: Allow pathlib.Path as RPC argument via authproxy (MarcoFalke)
fa41614 scripted-diff: Use wallets_path and chain_path where possible (MarcoFalke)
fa493fa test: Use wallet_dir lambda in wallet_multiwallet test where possible (MarcoFalke)

Pull request description:

  It seems inconsistent, fragile and verbose to:

  * Call `get_datadir_path` to recreate the path that already exists as field in TestNode
  * Call `os.path.join` with the hardcoded chain name or `self.chain` to recreate the TestNode `chain_path` property
  * Sometimes even use the hardcoded node dir name (`"node0"`)

  Fix all issues by using the TestNode properties.

ACKs for top commit:
  willcl-ark:
    re-ACK aaaa3ae
  theStack:
    Code-review ACK aaaa3ae 🌊

Tree-SHA512: e4720278085beb8164e1fe6c1aa18f601558a9263494ce69a83764c1487007de63ebb51d1b1151862dc4d5b49ded6162a5c1553cd30ea1c28627d447db4d8e72

Author: fanquake

test/functional/feature_abortnode.py

@@ -9,10 +9,7 @@
 - Mine a fork that requires disconnecting the tip.
 - Verify that bitcoind AbortNode's.
 """
-
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import get_datadir_path
-import os
 
 
 class AbortNodeTest(BitcoinTestFramework):
@@ -26,10 +23,9 @@ def setup_network(self):
 
     def run_test(self):
         self.generate(self.nodes[0], 3, sync_fun=self.no_op)
-        datadir = get_datadir_path(self.options.tmpdir, 0)
 
         # Deleting the undo file will result in reorg failure
-        os.unlink(os.path.join(datadir, self.chain, 'blocks', 'rev00000.dat'))
+        (self.nodes[0].chain_path / "blocks" / "rev00000.dat").unlink()
 
         # Connecting to a node with a more work chain will trigger a reorg
         # attempt.

test/functional/feature_addrman.py

@@ -53,7 +53,7 @@ def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
-        peers_dat = os.path.join(self.nodes[0].datadir, self.chain, "peers.dat")
+        peers_dat = os.path.join(self.nodes[0].chain_path, "peers.dat")
         init_error = lambda reason: (
             f"Error: Invalid or corrupt peers.dat \\({reason}\\). If you believe this "
             f"is a bug, please report it to {self.config['environment']['PACKAGE_BUGREPORT']}. "

test/functional/feature_anchors.py

@@ -20,9 +20,7 @@ def set_test_params(self):
         self.disable_autoconnect = False
 
     def run_test(self):
-        node_anchors_path = os.path.join(
-            self.nodes[0].datadir, "regtest", "anchors.dat"
-        )
+        node_anchors_path = self.nodes[0].chain_path / "anchors.dat"
 
         self.log.info("When node starts, check if anchors.dat doesn't exist")
         assert not os.path.exists(node_anchors_path)

test/functional/feature_asmap.py

@@ -113,7 +113,7 @@ def test_empty_asmap(self):
 
     def run_test(self):
         self.node = self.nodes[0]
-        self.datadir = os.path.join(self.node.datadir, self.chain)
+        self.datadir = self.node.chain_path
         self.default_asmap = os.path.join(self.datadir, DEFAULT_ASMAP_FILENAME)
         self.asmap_raw = os.path.join(os.path.dirname(os.path.realpath(__file__)), ASMAP)
 

test/functional/feature_blocksdir.py

@@ -18,7 +18,7 @@ def set_test_params(self):
 
     def run_test(self):
         self.stop_node(0)
-        assert os.path.isdir(os.path.join(self.nodes[0].datadir, self.chain, "blocks"))
+        assert os.path.isdir(os.path.join(self.nodes[0].chain_path, "blocks"))
         assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "blocks"))
         shutil.rmtree(self.nodes[0].datadir)
         initialize_datadir(self.options.tmpdir, 0, self.chain)
@@ -31,7 +31,7 @@ def run_test(self):
         self.log.info("mining blocks..")
         self.generatetoaddress(self.nodes[0], 10, self.nodes[0].get_deterministic_priv_key().address)
         assert os.path.isfile(os.path.join(blocksdir_path, self.chain, "blocks", "blk00000.dat"))
-        assert os.path.isdir(os.path.join(self.nodes[0].datadir, self.chain, "blocks", "index"))
+        assert os.path.isdir(os.path.join(self.nodes[0].chain_path, "blocks", "index"))
 
 
 if __name__ == '__main__':

test/functional/feature_config_args.py

@@ -32,7 +32,7 @@ def test_config_file_parser(self):
         self.stop_node(0)
 
         # Check that startup fails if conf= is set in bitcoin.conf or in an included conf file
-        bad_conf_file_path = os.path.join(self.options.tmpdir, 'node0', 'bitcoin_bad.conf')
+        bad_conf_file_path = self.nodes[0].datadir_path / "bitcoin_bad.conf"
         util.write_config(bad_conf_file_path, n=0, chain='', extra_config=f'conf=some.conf\n')
         conf_in_config_file_err = 'Error: Error reading configuration file: conf cannot be set in the configuration file; use includeconf= if you want to include additional config files'
         self.nodes[0].assert_start_raises_init_error(
@@ -75,7 +75,7 @@ def test_config_file_parser(self):
                 conf.write("wallet=foo\n")
             self.nodes[0].assert_start_raises_init_error(expected_msg=f'Error: Config setting for -wallet only applied on {self.chain} network when in [{self.chain}] section.')
 
-        main_conf_file_path = os.path.join(self.options.tmpdir, 'node0', 'bitcoin_main.conf')
+        main_conf_file_path = self.nodes[0].datadir_path / "bitcoin_main.conf"
         util.write_config(main_conf_file_path, n=0, chain='', extra_config=f'includeconf={inc_conf_file_path}\n')
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('acceptnonstdtxn=1\n')

test/functional/feature_fee_estimation.py

@@ -421,7 +421,7 @@ def run_test(self):
 
         self.log.info("Restarting node with fresh estimation")
         self.stop_node(0)
-        fee_dat = os.path.join(self.nodes[0].datadir, self.chain, "fee_estimates.dat")
+        fee_dat = os.path.join(self.nodes[0].chain_path, "fee_estimates.dat")
         os.remove(fee_dat)
         self.start_node(0)
         self.connect_nodes(0, 1)

test/functional/feature_filelock.py

@@ -3,7 +3,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Check that it's not possible to start a second bitcoind instance using the same datadir or wallet."""
-import os
 import random
 import string
 
@@ -24,7 +23,7 @@ def setup_network(self):
         self.nodes[0].wait_for_rpc_connection()
 
     def run_test(self):
-        datadir = os.path.join(self.nodes[0].datadir, self.chain)
+        datadir = self.nodes[0].chain_path
         self.log.info(f"Using datadir {datadir}")
 
         self.log.info("Check that we can't start a second bitcoind instance using the same datadir")
@@ -35,7 +34,7 @@ def run_test(self):
             def check_wallet_filelock(descriptors):
                 wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])
                 self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=descriptors)
-                wallet_dir = os.path.join(datadir, 'wallets')
+                wallet_dir = self.nodes[0].wallets_path
                 self.log.info("Check that we can't start a second bitcoind instance using the same wallet")
                 if descriptors:
                     expected_msg = f"Error: SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of {self.config['environment']['PACKAGE_NAME']}?"

test/functional/feature_includeconf.py

@@ -14,27 +14,25 @@
 4. multiple includeconf arguments can be specified in the main config
    file.
 """
-import os
-
 from test_framework.test_framework import BitcoinTestFramework
 
+
 class IncludeConfTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
 
-    def setup_chain(self):
-        super().setup_chain()
+    def run_test(self):
         # Create additional config files
         # - tmpdir/node0/relative.conf
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
             f.write("uacomment=relative\n")
         # - tmpdir/node0/relative2.conf
-        with open(os.path.join(self.options.tmpdir, "node0", "relative2.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative2.conf", "w", encoding="utf8") as f:
             f.write("uacomment=relative2\n")
-        with open(os.path.join(self.options.tmpdir, "node0", "bitcoin.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "bitcoin.conf", "a", encoding="utf8") as f:
             f.write("uacomment=main\nincludeconf=relative.conf\n")
+        self.restart_node(0)
 
-    def run_test(self):
         self.log.info("-includeconf works from config file. subversion should end with 'main; relative)/'")
 
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
@@ -52,7 +50,7 @@ def run_test(self):
         )
 
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
         self.start_node(0)
 
@@ -63,20 +61,20 @@ def run_test(self):
         self.log.info("-includeconf cannot contain invalid arg")
 
         # Commented out as long as we ignore invalid arguments in configuration files
-        #with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        #with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
         #    f.write("foo=bar\n")
         #self.nodes[0].assert_start_raises_init_error(expected_msg="Error: Error reading configuration file: Invalid configuration value foo")
 
         self.log.info("-includeconf cannot be invalid path")
-        os.remove(os.path.join(self.options.tmpdir, "node0", "relative.conf"))
+        (self.nodes[0].datadir_path / "relative.conf").unlink()
         self.nodes[0].assert_start_raises_init_error(expected_msg="Error: Error reading configuration file: Failed to include configuration file relative.conf")
 
         self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
             # Restore initial file contents
             f.write("uacomment=relative\n")
 
-        with open(os.path.join(self.options.tmpdir, "node0", "bitcoin.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "bitcoin.conf", "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
 
         self.start_node(0)

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 = os.path.join(data_dir, self.chain, "blocks")
+        blocks_dir = self.nodes[0].chain_path / "blocks"
         hash_list = tempfile.NamedTemporaryFile(dir=data_dir,
                                                 mode='w',
                                                 delete=False,