merge bitcoin#19967: Replace (dis)?connect_nodes globals with TestFramework methods

Co-authored-by: UdjinM6 <[email protected]>

Author: kwvg

test/functional/example_test.py

@@ -25,7 +25,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     wait_until,
 )
 
@@ -116,7 +115,7 @@ def setup_network(self):
         # In this test, we're not connecting node2 to node0 or node1. Calls to
         # sync_all() should not include node2, since we're not expecting it to
         # sync.
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all(self.nodes[0:2])
 
     # Use setup_nodes() to customize the node start behaviour (for example if
@@ -184,7 +183,7 @@ def run_test(self):
         self.nodes[1].waitforblockheight(11)
 
         self.log.info("Connect node2 and node1")
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
 
         self.log.info("Wait for node2 to receive all the blocks from node1")
         self.sync_all()

test/functional/feature_abortnode.py

@@ -11,7 +11,7 @@
 """
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import wait_until, get_datadir_path, connect_nodes
+from test_framework.util import wait_until, get_datadir_path
 import os
 
 class AbortNodeTest(BitcoinTestFramework):
@@ -35,7 +35,7 @@ def run_test(self):
         # attempt.
         self.nodes[1].generate(3)
         with self.nodes[0].assert_debug_log(["Failed to disconnect block"]):
-            connect_nodes(self.nodes[0], 1)
+            self.connect_nodes(0, 1)
             self.nodes[1].generate(1)
 
             # Check that node0 aborted

test/functional/feature_addressindex.py

@@ -13,7 +13,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
 from test_framework.script import CScript, OP_CHECKSIG, OP_DUP, OP_EQUAL, OP_EQUALVERIFY, OP_HASH160
-from test_framework.util import assert_equal, connect_nodes
+from test_framework.util import assert_equal
 
 class AddressIndexTest(BitcoinTestFramework):
 
@@ -33,9 +33,9 @@ def setup_network(self):
         # Nodes 2/3 are used for testing
         self.start_node(2, ["-addressindex"])
         self.start_node(3, ["-addressindex"])
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[0], 2)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(0, 2)
+        self.connect_nodes(0, 3)
         self.sync_all()
         self.import_deterministic_coinbase_privkeys()
 
@@ -44,12 +44,12 @@ def run_test(self):
         self.stop_node(1)
         self.nodes[1].assert_start_raises_init_error(["-addressindex=0"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX)
         self.start_node(1, ["-addressindex=0", "-reindex"])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
         self.stop_node(1)
         self.nodes[1].assert_start_raises_init_error(["-addressindex"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX)
         self.start_node(1, ["-addressindex", "-reindex"])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
 
         self.log.info("Mining blocks...")

test/functional/feature_dip3_deterministicmns.py

@@ -12,7 +12,7 @@
 from test_framework.blocktools import create_block, create_coinbase, get_masternode_payment
 from test_framework.messages import CCbTx, COIN, CTransaction, FromHex, ToHex, uint256_to_string
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, connect_nodes, force_finish_mnsync, get_bip9_status, p2p_port
+from test_framework.util import assert_equal, force_finish_mnsync, get_bip9_status, p2p_port
 
 class Masternode(object):
     pass
@@ -43,7 +43,7 @@ def start_controller_node(self):
         self.start_node(0, extra_args=self.extra_args)
         for node in self.nodes[1:]:
             if node is not None and node.process is not None:
-                connect_nodes(node, 0)
+                self.connect_nodes(node.index, 0)
 
     def run_test(self):
         self.log.info("funding controller node")
@@ -269,7 +269,7 @@ def start_mn(self, mn):
         self.start_node(mn.idx, extra_args = self.extra_args + ['-masternodeblsprivkey=%s' % mn.blsMnkey])
         force_finish_mnsync(self.nodes[mn.idx])
         mn.node = self.nodes[mn.idx]
-        connect_nodes(mn.node, 0)
+        self.connect_nodes(mn.idx, 0)
         self.sync_all()
 
     def spend_mn_collateral(self, mn, with_dummy_input_output=False):

test/functional/feature_fee_estimation.py

@@ -13,7 +13,6 @@
     assert_equal,
     assert_greater_than,
     assert_greater_than_or_equal,
-    connect_nodes,
     satoshi_round,
 )
 
@@ -213,9 +212,9 @@ def run_test(self):
         # so the estimates would not be affected by the splitting transactions
         self.start_node(1)
         self.start_node(2)
-        connect_nodes(self.nodes[1], 0)
-        connect_nodes(self.nodes[0], 2)
-        connect_nodes(self.nodes[2], 1)
+        self.connect_nodes(1, 0)
+        self.connect_nodes(0, 2)
+        self.connect_nodes(2, 1)
 
         self.sync_all()
 

test/functional/feature_llmq_chainlocks.py

@@ -13,7 +13,7 @@
 import time
 
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import connect_nodes, force_finish_mnsync, isolate_node, reconnect_isolated_node
+from test_framework.util import force_finish_mnsync
 
 
 class LLMQChainLocksTest(DashTestFramework):
@@ -27,7 +27,7 @@ def run_test(self):
         # Usually node0 is the one that does this, but in this test we isolate it multiple times
         for i in range(len(self.nodes)):
             if i != 1:
-                connect_nodes(self.nodes[i], 1)
+                self.connect_nodes(i, 1)
 
         self.activate_dip8()
 
@@ -53,24 +53,24 @@ def run_test(self):
             assert block['chainlock']
 
         self.log.info("Isolate node, mine on another, and reconnect")
-        isolate_node(self.nodes[0])
+        self.isolate_node(0)
         node0_mining_addr = self.nodes[0].getnewaddress()
         node0_tip = self.nodes[0].getbestblockhash()
         self.nodes[1].generatetoaddress(5, node0_mining_addr)
         self.wait_for_chainlocked_block(self.nodes[1], self.nodes[1].getbestblockhash())
         assert self.nodes[0].getbestblockhash() == node0_tip
-        reconnect_isolated_node(self.nodes[0], 1)
+        self.reconnect_isolated_node(0, 1)
         self.nodes[1].generatetoaddress(1, node0_mining_addr)
         self.wait_for_chainlocked_block_all_nodes(self.nodes[1].getbestblockhash())
 
         self.log.info("Isolate node, mine on both parts of the network, and reconnect")
-        isolate_node(self.nodes[0])
+        self.isolate_node(0)
         bad_tip = self.nodes[0].generate(5)[-1]
         self.nodes[1].generatetoaddress(1, node0_mining_addr)
         good_tip = self.nodes[1].getbestblockhash()
         self.wait_for_chainlocked_block(self.nodes[1], good_tip)
         assert not self.nodes[0].getblock(self.nodes[0].getbestblockhash())["chainlock"]
-        reconnect_isolated_node(self.nodes[0], 1)
+        self.reconnect_isolated_node(0, 1)
         self.nodes[1].generatetoaddress(1, node0_mining_addr)
         self.wait_for_chainlocked_block_all_nodes(self.nodes[1].getbestblockhash())
         assert self.nodes[0].getblock(self.nodes[0].getbestblockhash())["previousblockhash"] == good_tip
@@ -90,7 +90,7 @@ def run_test(self):
         self.log.info("Restart it so that it forgets all the chainlock messages from the past")
         self.stop_node(0)
         self.start_node(0)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         assert self.nodes[0].getbestblockhash() == good_tip
         self.nodes[0].invalidateblock(good_tip)
         self.log.info("Now try to reorg the chain")
@@ -129,7 +129,7 @@ def run_test(self):
 
         self.log.info("Isolate a node and let it create some transactions which won't get IS locked")
         force_finish_mnsync(self.nodes[0])
-        isolate_node(self.nodes[0])
+        self.isolate_node(0)
         txs = []
         for i in range(3):
             txs.append(self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1))
@@ -155,7 +155,7 @@ def run_test(self):
         # Enable network on first node again, which will cause the blocks to propagate and IS locks to happen retroactively
         # for the mined TXs, which will then allow the network to create a CLSIG
         self.log.info("Re-enable network on first node and wait for chainlock")
-        reconnect_isolated_node(self.nodes[0], 1)
+        self.reconnect_isolated_node(0, 1)
         self.wait_for_chainlocked_block(self.nodes[0], self.nodes[0].getbestblockhash(), timeout=30)
 
     def create_chained_txs(self, node, amount):

test/functional/feature_llmq_connections.py

@@ -13,7 +13,7 @@
 import time
 
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import assert_greater_than_or_equal, connect_nodes, Options, wait_until
+from test_framework.util import assert_greater_than_or_equal, Options, wait_until
 
 # Probes should age after this many seconds.
 # NOTE: mine_quorum() can bump mocktime quite often internally so make sure this number is high enough.
@@ -119,7 +119,7 @@ def check_reconnects(self, expected_connection_count):
 
         # Also re-connect non-masternode connections
         for i in range(1, len(self.nodes)):
-            connect_nodes(self.nodes[i], 0)
+            self.connect_nodes(i, 0)
             self.nodes[i].ping()
         # wait for ping/pong so that we can be sure that spork propagation works
         time.sleep(1) # needed to make sure we don't check before the ping is actually sent (fPingQueued might be true but SendMessages still not called)

test/functional/feature_llmq_data_recovery.py

@@ -6,7 +6,7 @@
 import time
 from test_framework.mininode import logger
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import force_finish_mnsync, connect_nodes, wait_until
+from test_framework.util import force_finish_mnsync, wait_until
 
 '''
 feature_llmq_data_recovery.py
@@ -29,19 +29,19 @@ def set_test_params(self):
         self.set_dash_llmq_test_params(4, 3)
 
     def restart_mn(self, mn, reindex=False, qvvec_sync=[], qdata_recovery_enabled=True):
-        args = self.extra_args[mn.nodeIdx] + ['-masternodeblsprivkey=%s' % mn.keyOperator,
+        args = self.extra_args[mn.node.index] + ['-masternodeblsprivkey=%s' % mn.keyOperator,
                                               '-llmq-data-recovery=%d' % qdata_recovery_enabled]
         for llmq_sync in qvvec_sync:
             args.append('-llmq-qvvec-sync=%s:%d' % (llmq_type_strings[llmq_sync[0]], llmq_sync[1]))
         if reindex:
             args.append('-reindex')
             bb_hash = mn.node.getbestblockhash()
-            self.restart_node(mn.nodeIdx, args)
+            self.restart_node(mn.node.index, args)
             wait_until(lambda: mn.node.getbestblockhash() == bb_hash)
         else:
-            self.restart_node(mn.nodeIdx, args)
+            self.restart_node(mn.node.index, args)
         force_finish_mnsync(mn.node)
-        connect_nodes(mn.node, 0)
+        self.connect_nodes(mn.node.index, 0)
         if qdata_recovery_enabled:
             # trigger recovery threads and wait for them to start
             self.nodes[0].generate(1)

test/functional/feature_llmq_is_migration.py

@@ -6,7 +6,7 @@
 
 from test_framework.messages import CTransaction, FromHex, hash256, ser_compact_size, ser_string
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import wait_until, connect_nodes
+from test_framework.util import wait_until
 
 '''
 feature_llmq_is_migration.py
@@ -33,7 +33,7 @@ def run_test(self):
 
         for i in range(len(self.nodes)):
             if i != 1:
-                connect_nodes(self.nodes[i], 0)
+                self.connect_nodes(i, 0)
 
         self.activate_dip8()
 

test/functional/feature_llmq_is_retroactive.py

@@ -16,7 +16,7 @@
 import time
 
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import set_node_times, isolate_node, reconnect_isolated_node
+from test_framework.util import set_node_times
 
 
 class LLMQ_IS_RetroactiveSigning(DashTestFramework):
@@ -69,13 +69,13 @@ def run_test(self):
         self.wait_for_chainlocked_block_all_nodes(block)
 
         self.log.info("testing normal signing with partially known TX")
-        isolate_node(self.nodes[3])
+        self.isolate_node(3)
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
         # Make sure nodes 1 and 2 received the TX before we continue,
         # otherwise it might announce the TX to node 3 when reconnecting
         self.wait_for_tx(txid, self.nodes[1])
         self.wait_for_tx(txid, self.nodes[2])
-        reconnect_isolated_node(self.nodes[3], 0)
+        self.reconnect_isolated_node(3, 0)
         # Make sure nodes actually try re-connecting quorum connections
         self.bump_mocktime(30)
         self.wait_for_mnauth(self.nodes[3], 2)
@@ -88,26 +88,26 @@ def run_test(self):
         self.wait_for_instantlock(txid, self.nodes[0])
 
         self.log.info("testing retroactive signing with unknown TX")
-        isolate_node(self.nodes[3])
+        self.isolate_node(3)
         rawtx = self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1})
         rawtx = self.nodes[0].fundrawtransaction(rawtx)['hex']
         rawtx = self.nodes[0].signrawtransactionwithwallet(rawtx)['hex']
         txid = self.nodes[3].sendrawtransaction(rawtx)
         # Make node 3 consider the TX as safe
         self.bump_mocktime(10 * 60 + 1)
         block = self.nodes[3].generatetoaddress(1, self.nodes[0].getnewaddress())[0]
-        reconnect_isolated_node(self.nodes[3], 0)
+        self.reconnect_isolated_node(3, 0)
         self.wait_for_chainlocked_block_all_nodes(block)
         self.nodes[0].setmocktime(self.mocktime)
 
         self.log.info("testing retroactive signing with partially known TX")
-        isolate_node(self.nodes[3])
+        self.isolate_node(3)
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
         # Make sure nodes 1 and 2 received the TX before we continue,
         # otherwise it might announce the TX to node 3 when reconnecting
         self.wait_for_tx(txid, self.nodes[1])
         self.wait_for_tx(txid, self.nodes[2])
-        reconnect_isolated_node(self.nodes[3], 0)
+        self.reconnect_isolated_node(3, 0)
         # Make sure nodes actually try re-connecting quorum connections
         self.bump_mocktime(30)
         self.wait_for_mnauth(self.nodes[3], 2)
@@ -136,7 +136,7 @@ def cycle_llmqs(self):
 
     def test_all_nodes_session_timeout(self, do_cycle_llmqs):
         set_node_times(self.nodes, self.mocktime)
-        isolate_node(self.nodes[3])
+        self.isolate_node(3)
         rawtx = self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1})
         rawtx = self.nodes[0].fundrawtransaction(rawtx)['hex']
         rawtx = self.nodes[0].signrawtransactionwithwallet(rawtx)['hex']
@@ -150,7 +150,7 @@ def test_all_nodes_session_timeout(self, do_cycle_llmqs):
         # Make the signing session for the IS lock timeout on nodes 1-3
         self.bump_mocktime(61)
         time.sleep(2) # make sure Cleanup() is called
-        reconnect_isolated_node(self.nodes[3], 0)
+        self.reconnect_isolated_node(3, 0)
         # Make sure nodes actually try re-connecting quorum connections
         self.bump_mocktime(30)
         self.wait_for_mnauth(self.nodes[3], 2)
@@ -167,7 +167,7 @@ def test_all_nodes_session_timeout(self, do_cycle_llmqs):
 
     def test_single_node_session_timeout(self, do_cycle_llmqs):
         set_node_times(self.nodes, self.mocktime)
-        isolate_node(self.nodes[3])
+        self.isolate_node(3)
         rawtx = self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1})
         rawtx = self.nodes[0].fundrawtransaction(rawtx)['hex']
         rawtx = self.nodes[0].signrawtransactionwithwallet(rawtx)['hex']
@@ -176,7 +176,7 @@ def test_single_node_session_timeout(self, do_cycle_llmqs):
         # Make the signing session for the IS lock timeout on node 3
         self.bump_mocktime(61)
         time.sleep(2) # make sure Cleanup() is called
-        reconnect_isolated_node(self.nodes[3], 0)
+        self.reconnect_isolated_node(3, 0)
         # Make sure nodes actually try re-connecting quorum connections
         self.bump_mocktime(30)
         self.wait_for_mnauth(self.nodes[3], 2)

test/functional/feature_llmq_rotation.py

@@ -17,7 +17,6 @@
 from test_framework.util import (
     assert_equal,
     assert_greater_than_or_equal,
-    connect_nodes,
     wait_until,
 )
 
@@ -67,7 +66,7 @@ def run_test(self):
 
         for i in range(len(self.nodes)):
             if i != 1:
-                connect_nodes(self.nodes[i], 0)
+                self.connect_nodes(i, 0)
 
         self.activate_dip8()
 

test/functional/feature_llmq_signing.py

@@ -13,7 +13,7 @@
 from test_framework.messages import CSigShare, msg_qsigshare, uint256_to_string
 from test_framework.mininode import P2PInterface
 from test_framework.test_framework import DashTestFramework
-from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes, force_finish_mnsync, hex_str_to_bytes, wait_until
+from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, hex_str_to_bytes, wait_until
 
 
 class LLMQSigningTest(DashTestFramework):
@@ -184,7 +184,7 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout):
             assert_sigs_nochange(False, False, False, 3)
             # Need to re-connect so that it later gets the recovered sig
             mn.node.setnetworkactive(True)
-            connect_nodes(mn.node, 0)
+            self.connect_nodes(mn.node.index, 0)
             force_finish_mnsync(mn.node)
             # Make sure intra-quorum connections were also restored
             self.bump_mocktime(1)  # need this to bypass quorum connection retry timeout