test: persist -v2transport over restarts and respect -v2transport=0

mzumsande · mzumsande · commit 68a900175198 · 2023-11-08T17:30:20.000-05:00
Before, a global -v2transport provided to the test would be dropped
when restarting the node within a test and specifying any extra_args.

Fix this by adding "v2transport=1" to args (not extra_args) based
on the global parameter, and deciding for each (re)start of the node
based on this default and test-specific extra_args
(which take precedence over args) whether v2 should be used.
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
@@ -508,8 +508,6 @@ def get_bin_from_version(version, bin_name, bin_default):
         assert_equal(len(binary_cli), num_nodes)
         for i in range(num_nodes):
             args = list(extra_args[i])
-            if self.options.v2transport and ("-v2transport=0" not in args):
-                args.append("-v2transport=1")
             test_node_i = TestNode(
                 i,
                 get_datadir_path(self.options.tmpdir, i),
@@ -528,6 +526,7 @@ def get_bin_from_version(version, bin_name, bin_default):
                 start_perf=self.options.perf,
                 use_valgrind=self.options.valgrind,
                 descriptors=self.options.descriptors,
+                v2transport=self.options.v2transport,
             )
             self.nodes.append(test_node_i)
             if not test_node_i.version_is_at_least(170000):
@@ -602,7 +601,7 @@ def connect_nodes(self, a, b, *, peer_advertises_v2=None, wait_for_connect: bool
         ip_port = "127.0.0.1:" + str(p2p_port(b))
 
         if peer_advertises_v2 is None:
-            peer_advertises_v2 = self.options.v2transport
+            peer_advertises_v2 = from_connection.use_v2transport
 
         if peer_advertises_v2:
             from_connection.addnode(node=ip_port, command="onetry", v2transport=True)
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
@@ -67,7 +67,7 @@ class TestNode():
     To make things easier for the test writer, any unrecognised messages will
     be dispatched to the RPC connection."""
 
-    def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False):
+    def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False):
         """
         Kwargs:
             start_perf (bool): If True, begin profiling the node with `perf` as soon as
@@ -126,6 +126,12 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
         if self.version_is_at_least(239000):
             self.args.append("-loglevel=trace")
 
+        # Default behavior from global -v2transport flag is added to args to persist it over restarts.
+        # May be overwritten in individual tests, using extra_args.
+        self.default_to_v2 = v2transport
+        if self.default_to_v2:
+            self.args.append("-v2transport=1")
+
         self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path)
         self.use_cli = use_cli
         self.start_perf = start_perf
@@ -198,6 +204,8 @@ def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None
         if extra_args is None:
             extra_args = self.extra_args
 
+        self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)
+
         # Add a new stdout and stderr file each time bitcoind is started
         if stderr is None:
             stderr = tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False)
