Merge #19967: test: Replace (dis)?connect_nodes globals with TestFramework methods

3c7d9ab8c8 test: Move (dis)?connect_nodes globals into TestFramework as helpers (Elliott Jin)
4b16c61461 scripted-diff: test: Replace uses of (dis)?connect_nodes global (Prayank)
be386840d4 test: Replace use of (dis)?connect_nodes globals (Elliott Jin)

Pull request description:

  `util.py` defines global helper functions `connect_nodes` and `disconnect_nodes`; however, these functions are confusing because they take a node and an index (instead of two indexes).

  The `TestFramework` object has enough context to convert from `i` to `self.nodes[i]`, so we can replace all instances of `connect_nodes(self.nodes[a], b)` with `self.connect_nodes(a, b)`. Similarly, we can replace instances of `disconnect_nodes`.

  The approach taken in this PR builds on #19945 but uses a scripted-diff for the majority of the changes.

  Fixes: #19821

ACKs for top commit:
  MarcoFalke:
    ACK 3c7d9ab8c8
  guggero:
    ACK 3c7d9ab8

Tree-SHA512: e027092748602904abcd986d7299624c8754c3236314b6d8e392e306741c212f266c2207e385adfb194f67ae6559a585ee7b15d639b1d65c4651dbf503e5931a

test/functional/example_test.py

@@ -25,7 +25,6 @@ from test_framework.p2p import (
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
 )
 
 # P2PInterface is a class containing callbacks to be executed when a P2P
@@ -115,7 +114,7 @@ class ExampleTest(BitcoinTestFramework):
         # 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
@@ -183,7 +182,7 @@ class ExampleTest(BitcoinTestFramework):
         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 get_datadir_path, connect_nodes
+from test_framework.util import get_datadir_path
 import os
 
 
@@ -36,7 +36,7 @@ class AbortNodeTest(BitcoinTestFramework):
         # 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_fee_estimation.py

@@ -13,7 +13,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_greater_than_or_equal,
-    connect_nodes,
     satoshi_round,
 )
 
@@ -232,9 +231,9 @@ class EstimateFeeTest(BitcoinTestFramework):
         # 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)
+        self.connect_nodes(1, 0)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
-        connect_nodes(self.nodes[2], 1)
+        self.connect_nodes(2, 1)
 
         self.sync_all()
 

test/functional/feature_minchainwork.py

@@ -18,7 +18,7 @@ only succeeds past a given node once its nMinimumChainWork has been exceeded.
 import time
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import connect_nodes, assert_equal
+from test_framework.util import assert_equal
 
 # 2 hashes required per regtest block (with no difficulty adjustment)
 REGTEST_WORK_PER_BLOCK = 2
@@ -39,7 +39,7 @@ class MinimumChainWorkTest(BitcoinTestFramework):
         # block relay to inbound peers.
         self.setup_nodes()
         for i in range(self.num_nodes-1):
-            connect_nodes(self.nodes[i+1], i)
+            self.connect_nodes(i+1, i)
 
     def run_test(self):
         # Start building a chain on node0.  node2 shouldn't be able to sync until node1's

test/functional/feature_notifications.py

@@ -9,8 +9,6 @@ from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE, keyhash_to_p2pkh
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
     hex_str_to_bytes,
 )
 
@@ -75,7 +73,7 @@ class NotificationsTest(BitcoinTestFramework):
             self.log.info("test -walletnotify after rescan")
             # restart node to rescan to force wallet notifications
             self.start_node(1)
-            connect_nodes(self.nodes[0], 1)
+            self.connect_nodes(0, 1)
 
             self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10)
 
@@ -126,12 +124,12 @@ class NotificationsTest(BitcoinTestFramework):
             # Bump tx2 as bump2 and generate a block on node 0 while
             # disconnected, then reconnect and check for notifications on node 1
             # about newly confirmed bump2 and newly conflicted tx2.
-            disconnect_nodes(self.nodes[0], 1)
+            self.disconnect_nodes(0, 1)
             bump2 = self.nodes[0].bumpfee(tx2)["txid"]
             self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
             assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1)
             assert_equal(tx2 in self.nodes[1].getrawmempool(), True)
-            connect_nodes(self.nodes[0], 1)
+            self.connect_nodes(0, 1)
             self.sync_blocks()
             self.expect_wallet_notify([bump2, tx2])
             assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1)

test/functional/feature_pruning.py

@@ -18,8 +18,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
 )
 
 # Rescans start at the earliest block up to 2 hours before a key timestamp, so
@@ -102,11 +100,11 @@ class PruneTest(BitcoinTestFramework):
 
         self.prunedir = os.path.join(self.nodes[2].datadir, self.chain, 'blocks', '')
 
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
-        connect_nodes(self.nodes[0], 4)
+        self.connect_nodes(0, 4)
         self.sync_blocks(self.nodes[0:5])
 
     def setup_nodes(self):
@@ -148,8 +146,8 @@ class PruneTest(BitcoinTestFramework):
         for _ in range(12):
             # Disconnect node 0 so it can mine a longer reorg chain without knowing about node 1's soon-to-be-stale chain
             # Node 2 stays connected, so it hears about the stale blocks and then reorg's when node0 reconnects
-            disconnect_nodes(self.nodes[0], 1)
+            self.disconnect_nodes(0, 1)
-            disconnect_nodes(self.nodes[0], 2)
+            self.disconnect_nodes(0, 2)
             # Mine 24 blocks in node 1
             mine_large_blocks(self.nodes[1], 24)
 
@@ -157,8 +155,8 @@ class PruneTest(BitcoinTestFramework):
             mine_large_blocks(self.nodes[0], 25)
 
             # Create connections in the order so both nodes can see the reorg at the same time
-            connect_nodes(self.nodes[0], 1)
+            self.connect_nodes(0, 1)
-            connect_nodes(self.nodes[0], 2)
+            self.connect_nodes(0, 2)
             self.sync_blocks(self.nodes[0:3])
 
         self.log.info("Usage can be over target because of high stale rate: %d" % calc_usage(self.prunedir))
@@ -187,15 +185,15 @@ class PruneTest(BitcoinTestFramework):
         self.log.info("New best height: %d" % self.nodes[1].getblockcount())
 
         # Disconnect node1 and generate the new chain
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
-        disconnect_nodes(self.nodes[1], 2)
+        self.disconnect_nodes(1, 2)
 
         self.log.info("Generating new longer chain of 300 more blocks")
         self.nodes[1].generate(300)
 
         self.log.info("Reconnect nodes")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.sync_blocks(self.nodes[0:3], timeout=120)
 
         self.log.info("Verify height on node 2: %d" % self.nodes[2].getblockcount())
@@ -336,7 +334,7 @@ class PruneTest(BitcoinTestFramework):
         # check that wallet loads successfully when restarting a pruned node after IBD.
         # this was reported to fail in #7494.
         self.log.info("Syncing node 5 to test wallet")
-        connect_nodes(self.nodes[0], 5)
+        self.connect_nodes(0, 5)
         nds = [self.nodes[0], self.nodes[5]]
         self.sync_blocks(nds, wait=5, timeout=300)
         self.restart_node(5, extra_args=["-prune=550"]) # restart to trigger rescan

test/functional/feature_segwit.py

@@ -22,7 +22,6 @@ from test_framework.util import (
     assert_equal,
     assert_is_hex_string,
     assert_raises_rpc_error,
-    connect_nodes,
     hex_str_to_bytes,
     try_rpc,
 )
@@ -78,7 +77,7 @@ class SegWitTest(BitcoinTestFramework):
 
     def setup_network(self):
         super().setup_network()
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_all()
 
     def success_mine(self, node, txid, sign, redeem_script=""):

test/functional/interface_zmq.py

@@ -11,7 +11,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.messages import CTransaction, hash256, FromHex
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     assert_raises_rpc_error,
 )
 from io import BytesIO
@@ -102,7 +101,7 @@ class ZMQTest (BitcoinTestFramework):
         rawtx = subs[3]
 
         self.restart_node(0, ["-zmqpub%s=%s" % (sub.topic.decode(), address) for sub in [hashblock, hashtx, rawblock, rawtx]])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         for socket in sockets:
             socket.connect(address)
 
@@ -207,7 +206,7 @@ class ZMQTest (BitcoinTestFramework):
         connect_blocks = self.nodes[1].generatetoaddress(2, ADDRESS_BCRT1_P2WSH_OP_TRUE)
 
         # nodes[0] will reorg chain after connecting back nodes[1]
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks() # tx in mempool valid but not advertised
 
         # Should receive nodes[1] tip
@@ -264,7 +263,7 @@ class ZMQTest (BitcoinTestFramework):
         self.nodes[1].generatetoaddress(2, ADDRESS_BCRT1_P2WSH_OP_TRUE)
 
         # nodes[0] will reorg chain after connecting back nodes[1]
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         # Then we receive all block (dis)connect notifications for the 2 block reorg
         assert_equal((dc_block, "D", None), seq.receive_sequence())
@@ -406,7 +405,7 @@ class ZMQTest (BitcoinTestFramework):
         seq = ZMQSubscriber(socket, b'sequence')
 
         self.restart_node(0, ['-zmqpub%s=%s' % (seq.topic.decode(), address)])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         socket.connect(address)
         # Relax so that the subscriber is ready before publishing zmq messages
         sleep(0.2)

test/functional/mempool_persist.py

@@ -45,8 +45,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than_or_equal,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
 )
 
 
@@ -83,11 +81,11 @@ class MempoolPersistTest(BitcoinTestFramework):
         assert_greater_than_or_equal(tx_creation_time_higher, tx_creation_time)
 
         # disconnect nodes & make a txn that remains in the unbroadcast set.
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
         assert(len(self.nodes[0].getpeerinfo()) == 0)
         assert(len(self.nodes[0].p2ps) == 0)
         self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("12"))
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
         self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.")
         self.stop_nodes()

test/functional/mempool_unbroadcast.py

@@ -11,9 +11,7 @@ from test_framework.p2p import P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     create_confirmed_utxos,
-    disconnect_nodes,
 )
 
 MAX_INITIAL_BROADCAST_DELAY = 15 * 60 # 15 minutes in seconds
@@ -36,7 +34,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         min_relay_fee = node.getnetworkinfo()["relayfee"]
         utxos = create_confirmed_utxos(min_relay_fee, node, 10)
 
-        disconnect_nodes(node, 1)
+        self.disconnect_nodes(0, 1)
 
         self.log.info("Generate transactions that only node 0 knows about")
 
@@ -70,7 +68,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         self.restart_node(0)
 
         self.log.info("Reconnect nodes & check if they are sent to node 1")
-        connect_nodes(node, 1)
+        self.connect_nodes(0, 1)
 
         # fast forward into the future & ensure that the second node has the txns
         node.mockscheduler(MAX_INITIAL_BROADCAST_DELAY)
@@ -91,7 +89,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         time.sleep(2) # allow sufficient time for possibility of broadcast
         assert_equal(len(conn.get_invs()), 0)
 
-        disconnect_nodes(node, 1)
+        self.disconnect_nodes(0, 1)
         node.disconnect_p2ps()
 
     def test_txn_removal(self):

test/functional/mining_basic.py

@@ -25,7 +25,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 
@@ -56,7 +55,7 @@ class MiningTest(BitcoinTestFramework):
         assert_equal(mining_info['currentblocktx'], 0)
         assert_equal(mining_info['currentblockweight'], 4000)
         self.restart_node(0)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
     def run_test(self):
         self.mine_chain()

test/functional/p2p_blockfilters.py

@@ -22,8 +22,6 @@ from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
 )
 
 class CFiltersClient(P2PInterface):
@@ -61,7 +59,7 @@ class CompactFiltersTest(BitcoinTestFramework):
         self.sync_blocks(timeout=600)
 
         # Stale blocks by disconnecting nodes 0 & 1, mining, then reconnecting
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
 
         self.nodes[0].generate(1)
         self.wait_until(lambda: self.nodes[0].getblockcount() == 1000)
@@ -90,7 +88,7 @@ class CompactFiltersTest(BitcoinTestFramework):
         assert_equal(len(response.headers), 1)
 
         self.log.info("Reorg node 0 to a new chain.")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks(timeout=600)
 
         main_block_hash = self.nodes[0].getblockhash(1000)

test/functional/p2p_disconnect_ban.py

@@ -9,7 +9,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 class DisconnectBanTest(BitcoinTestFramework):
@@ -19,8 +18,8 @@ class DisconnectBanTest(BitcoinTestFramework):
 
     def run_test(self):
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(1, 0)
 
         self.log.info("Test setban and listbanned RPCs")
 
@@ -78,8 +77,8 @@ class DisconnectBanTest(BitcoinTestFramework):
         # Clear ban lists
         self.nodes[1].clearbanned()
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(1, 0)
 
         self.log.info("Test disconnectnode RPCs")
 
@@ -98,7 +97,7 @@ class DisconnectBanTest(BitcoinTestFramework):
         assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1]
 
         self.log.info("disconnectnode: successfully reconnect node")
-        connect_nodes(self.nodes[0], 1)  # reconnect the node
+        self.connect_nodes(0, 1)  # reconnect the node
         assert_equal(len(self.nodes[0].getpeerinfo()), 2)
         assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1]
 

test/functional/p2p_node_network_limited.py

@@ -13,8 +13,6 @@ from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    disconnect_nodes,
-    connect_nodes,
 )
 
 
@@ -40,9 +38,9 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         self.extra_args = [['-prune=550', '-addrmantest'], [], []]
 
     def disconnect_all(self):
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
-        disconnect_nodes(self.nodes[0], 2)
+        self.disconnect_nodes(0, 2)
-        disconnect_nodes(self.nodes[1], 2)
+        self.disconnect_nodes(1, 2)
 
     def setup_network(self):
         self.add_nodes(self.num_nodes, self.extra_args)
@@ -60,7 +58,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), expected_services)
 
         self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         blocks = self.nodes[1].generatetoaddress(292, self.nodes[1].get_deterministic_priv_key().address)
         self.sync_blocks([self.nodes[0], self.nodes[1]])
 
@@ -85,7 +83,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
 
         # connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer
         # because node 2 is in IBD and node 0 is a NODE_NETWORK_LIMITED peer, sync must not be possible
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         try:
             self.sync_blocks([self.nodes[0], self.nodes[2]], timeout=5)
         except:
@@ -94,7 +92,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         assert_equal(self.nodes[2].getblockheader(self.nodes[2].getbestblockhash())['height'], 0)
 
         # now connect also to node 1 (non pruned)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
 
         # sync must be possible
         self.sync_blocks()
@@ -106,7 +104,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         self.nodes[0].generatetoaddress(10, self.nodes[0].get_deterministic_priv_key().address)
 
         # connect node1 (non pruned) with node0 (pruned) and check if the can sync
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         # sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED)
         self.sync_blocks([self.nodes[0], self.nodes[1]])

test/functional/p2p_permissions.py

@@ -22,7 +22,6 @@ from test_framework.test_node import ErrorMatch
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     p2p_port,
 )
 
@@ -146,7 +145,7 @@ class P2PPermissionsTests(BitcoinTestFramework):
         p2p_rebroadcast_wallet.send_txs_and_test([tx], self.nodes[1])
 
         self.log.debug("Check that node[1] will send the tx to node[0] even though it is already in the mempool")
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(1, 0)
         with self.nodes[1].assert_debug_log(["Force relaying tx {} from peer=0".format(txid)]):
             p2p_rebroadcast_wallet.send_txs_and_test([tx], self.nodes[1])
             self.wait_until(lambda: txid in self.nodes[0].getrawmempool())
@@ -165,7 +164,7 @@ class P2PPermissionsTests(BitcoinTestFramework):
         if whitelisted is not None:
             args = [*args, '-deprecatedrpc=whitelisted']
         self.restart_node(1, args)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         peerinfo = self.nodes[1].getpeerinfo()[0]
         if whitelisted is None:
             assert 'whitelisted' not in peerinfo

test/functional/p2p_segwit.py

@@ -81,8 +81,6 @@ from test_framework.script import (
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
     softfork_active,
     hex_str_to_bytes,
     assert_raises_rpc_error,
@@ -233,8 +231,8 @@ class SegWitTest(BitcoinTestFramework):
 
     def setup_network(self):
         self.setup_nodes()
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_all()
 
     # Helper functions
@@ -498,7 +496,7 @@ class SegWitTest(BitcoinTestFramework):
         # node2 doesn't need to be connected for this test.
         # (If it's connected, node0 may propagate an invalid block to it over
         # compact blocks and the nodes would have inconsistent tips.)
-        disconnect_nodes(self.nodes[0], 2)
+        self.disconnect_nodes(0, 2)
 
         # Create two outputs, a p2wsh and p2sh-p2wsh
         witness_program = CScript([OP_TRUE])
@@ -560,7 +558,7 @@ class SegWitTest(BitcoinTestFramework):
             # TODO: support multiple acceptable reject reasons.
             test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=False)
 
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
         self.utxo.pop(0)
         self.utxo.append(UTXO(txid, 2, value))
@@ -1945,7 +1943,7 @@ class SegWitTest(BitcoinTestFramework):
         """Test the behavior of starting up a segwit-aware node after the softfork has activated."""
 
         self.restart_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)])
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
         # We reconnect more than 100 blocks, give it plenty of time
         self.sync_blocks(timeout=240)

test/functional/p2p_unrequested_blocks.py

@@ -60,7 +60,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 
@@ -284,7 +283,7 @@ class AcceptBlockTest(BitcoinTestFramework):
         test_node.wait_for_disconnect()
 
         # 9. Connect node1 to node0 and ensure it is able to sync
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks([self.nodes[0], self.nodes[1]])
         self.log.info("Successfully synced nodes 1 and 0")
 

test/functional/rpc_fundrawtransaction.py

@@ -12,7 +12,6 @@ from test_framework.util import (
     assert_greater_than,
     assert_greater_than_or_equal,
     assert_raises_rpc_error,
-    connect_nodes,
     count_bytes,
     find_vout_for_address,
 )
@@ -38,10 +37,10 @@ class RawTransactionsTest(BitcoinTestFramework):
     def setup_network(self):
         self.setup_nodes()
 
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
 
     def run_test(self):
         self.log.info("Connect nodes, set fees, generate blocks, and sync")

test/functional/rpc_getblockfilter.py

@@ -7,7 +7,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal, assert_is_hex_string, assert_raises_rpc_error,
-    connect_nodes, disconnect_nodes
     )
 
 FILTER_TYPES = ["basic"]
@@ -20,7 +19,7 @@ class GetBlockFilterTest(BitcoinTestFramework):
 
     def run_test(self):
         # Create two chains by disconnecting nodes 0 & 1, mining, then reconnecting
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
 
         self.nodes[0].generate(3)
         self.nodes[1].generate(4)
@@ -29,7 +28,7 @@ class GetBlockFilterTest(BitcoinTestFramework):
         chain0_hashes = [self.nodes[0].getblockhash(block_height) for block_height in range(4)]
 
         # Reorg node 0 to a new chain
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks()
 
         assert_equal(self.nodes[0].getblockcount(), 4)

test/functional/rpc_getpeerinfo_deprecation.py

@@ -5,7 +5,6 @@
 """Test deprecation of getpeerinfo RPC fields."""
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import connect_nodes
 
 
 class GetpeerinfoDeprecationTest(BitcoinTestFramework):
@@ -26,7 +25,7 @@ class GetpeerinfoDeprecationTest(BitcoinTestFramework):
 
     def test_addnode_deprecation(self):
         self.restart_node(1, ["-deprecatedrpc=getpeerinfo_addnode"])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         self.log.info("Test getpeerinfo by default no longer returns an addnode field")
         assert "addnode" not in self.nodes[0].getpeerinfo()[0].keys()

test/functional/rpc_invalidateblock.py

@@ -8,7 +8,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
 )
 
 
@@ -32,7 +31,7 @@ class InvalidateTest(BitcoinTestFramework):
         assert_equal(self.nodes[1].getblockcount(), 6)
 
         self.log.info("Connect nodes to force a reorg")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks(self.nodes[0:2])
         assert_equal(self.nodes[0].getblockcount(), 6)
         badhash = self.nodes[1].getblockhash(2)
@@ -43,7 +42,7 @@ class InvalidateTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].getbestblockhash(), besthash_n0)
 
         self.log.info("Make sure we won't reorg to a lower work chain:")
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.log.info("Sync node 2 to node 1 so both have 6 blocks")
         self.sync_blocks(self.nodes[1:3])
         assert_equal(self.nodes[2].getblockcount(), 6)

test/functional/rpc_net.py

@@ -24,7 +24,6 @@ from test_framework.util import (
     assert_greater_than_or_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    connect_nodes,
     p2p_port,
 )
 
@@ -53,8 +52,8 @@ class NetTest(BitcoinTestFramework):
         # Get out of IBD for the minfeefilter and getpeerinfo tests.
         self.nodes[0].generate(101)
         # Connect nodes both ways.
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(1, 0)
 
         self.test_connection_count()
         self.test_getpeerinfo()
@@ -117,8 +116,8 @@ class NetTest(BitcoinTestFramework):
         with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: true\n']):
             self.nodes[0].setnetworkactive(state=True)
         # Connect nodes both ways.
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(1, 0)
 
         info = self.nodes[0].getnetworkinfo()
         assert_equal(info['networkactive'], True)

test/functional/rpc_preciousblock.py

@@ -7,7 +7,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
 )
 
 def unidirectional_node_sync_via_rpc(node_src, node_dest):
@@ -61,7 +60,7 @@ class PreciousTest(BitcoinTestFramework):
         self.log.info("Connect nodes and check no reorg occurs")
         # Submit competing blocks via RPC so any reorg should occur before we proceed (no way to wait on inaction for p2p sync)
         node_sync_via_rpc(self.nodes[0:2])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         assert_equal(self.nodes[0].getbestblockhash(), hashC)
         assert_equal(self.nodes[1].getbestblockhash(), hashG)
         self.log.info("Make Node0 prefer block G")
@@ -98,8 +97,8 @@ class PreciousTest(BitcoinTestFramework):
         hashL = self.nodes[2].getbestblockhash()
         self.log.info("Connect nodes and check no reorg occurs")
         node_sync_via_rpc(self.nodes[1:3])
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         assert_equal(self.nodes[0].getbestblockhash(), hashH)
         assert_equal(self.nodes[1].getbestblockhash(), hashH)
         assert_equal(self.nodes[2].getbestblockhash(), hashL)

test/functional/rpc_psbt.py

@@ -12,8 +12,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
     find_output,
 )
 
@@ -46,7 +44,7 @@ class PSBTTest(BitcoinTestFramework):
 
         # Disconnect offline node from others
         # Topology of test network is linear, so this one call is enough
-        disconnect_nodes(offline_node, 1)
+        self.disconnect_nodes(0, 1)
 
         # Create watchonly on online_node
         online_node.createwallet(wallet_name='wonline', disable_private_keys=True)
@@ -80,8 +78,8 @@ class PSBTTest(BitcoinTestFramework):
         wonline.unloadwallet()
 
         # Reconnect
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
     def assert_change_type(self, psbtx, expected_type):
         """Assert that the given PSBT has a change output with the given type."""

test/functional/rpc_rawtransaction.py

@@ -20,7 +20,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
     hex_str_to_bytes,
 )
 
@@ -60,7 +59,7 @@ class RawTransactionsTest(BitcoinTestFramework):
 
     def setup_network(self):
         super().setup_network()
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
     def run_test(self):
         self.log.info('prepare some coins for multiple *rawtransaction commands')

test/functional/rpc_setban.py

@@ -6,7 +6,6 @@
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
-    connect_nodes,
     p2p_port
 )
 
@@ -18,7 +17,7 @@ class SetBanTests(BitcoinTestFramework):
 
     def run_test(self):
         # Node 0 connects to Node 1, check that the noban permission is not granted
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         peerinfo = self.nodes[1].getpeerinfo()[0]
         assert(not 'noban' in peerinfo['permissions'])
 
@@ -32,14 +31,14 @@ class SetBanTests(BitcoinTestFramework):
 
         # However, node 0 should be able to reconnect if it has noban permission
         self.restart_node(1, ['-whitelist=127.0.0.1'])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         peerinfo = self.nodes[1].getpeerinfo()[0]
         assert('noban' in peerinfo['permissions'])
 
         # If we remove the ban, Node 0 should be able to reconnect even without noban permission
         self.nodes[1].setban("127.0.0.1", "remove")
         self.restart_node(1, [])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         peerinfo = self.nodes[1].getpeerinfo()[0]
         assert(not 'noban' in peerinfo['permissions'])
 

test/functional/test_framework/test_framework.py

@@ -27,10 +27,9 @@ from .util import (
     PortSeed,
     assert_equal,
     check_json_precision,
-    connect_nodes,
-    disconnect_nodes,
     get_datadir_path,
     initialize_datadir,
+    p2p_port,
     wait_until_helper,
 )
 
@@ -529,10 +528,49 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         self.nodes[i].process.wait(timeout)
 
     def connect_nodes(self, a, b):
-        connect_nodes(self.nodes[a], b)
+        def connect_nodes_helper(from_connection, node_num):
+            ip_port = "127.0.0.1:" + str(p2p_port(node_num))
+            from_connection.addnode(ip_port, "onetry")
+            # poll until version handshake complete to avoid race conditions
+            # with transaction relaying
+            # See comments in net_processing:
+            # * Must have a version message before anything else
+            # * Must have a verack message before anything else
+            wait_until_helper(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
+            wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
+
+        connect_nodes_helper(self.nodes[a], b)
 
     def disconnect_nodes(self, a, b):
-        disconnect_nodes(self.nodes[a], b)
+        def disconnect_nodes_helper(from_connection, node_num):
+            def get_peer_ids():
+                result = []
+                for peer in from_connection.getpeerinfo():
+                    if "testnode{}".format(node_num) in peer['subver']:
+                        result.append(peer['id'])
+                return result
+
+            peer_ids = get_peer_ids()
+            if not peer_ids:
+                self.log.warning("disconnect_nodes: {} and {} were not connected".format(
+                    from_connection.index,
+                    node_num,
+                ))
+                return
+            for peer_id in peer_ids:
+                try:
+                    from_connection.disconnectnode(nodeid=peer_id)
+                except JSONRPCException as e:
+                    # If this node is disconnected between calculating the peer id
+                    # and issuing the disconnect, don't worry about it.
+                    # This avoids a race condition if we're mass-disconnecting peers.
+                    if e.error['code'] != -29:  # RPC_CLIENT_NODE_NOT_CONNECTED
+                        raise
+
+            # wait to disconnect
+            wait_until_helper(lambda: not get_peer_ids(), timeout=5)
+
+        disconnect_nodes_helper(self.nodes[a], b)
 
     def split_network(self):
         """

test/functional/test_framework/util.py

@@ -411,47 +411,6 @@ def set_node_times(nodes, t):
         node.setmocktime(t)
 
 
-def disconnect_nodes(from_connection, node_num):
-    def get_peer_ids():
-        result = []
-        for peer in from_connection.getpeerinfo():
-            if "testnode{}".format(node_num) in peer['subver']:
-                result.append(peer['id'])
-        return result
-
-    peer_ids = get_peer_ids()
-    if not peer_ids:
-        logger.warning("disconnect_nodes: {} and {} were not connected".format(
-            from_connection.index,
-            node_num,
-        ))
-        return
-    for peer_id in peer_ids:
-        try:
-            from_connection.disconnectnode(nodeid=peer_id)
-        except JSONRPCException as e:
-            # If this node is disconnected between calculating the peer id
-            # and issuing the disconnect, don't worry about it.
-            # This avoids a race condition if we're mass-disconnecting peers.
-            if e.error['code'] != -29:  # RPC_CLIENT_NODE_NOT_CONNECTED
-                raise
-
-    # wait to disconnect
-    wait_until_helper(lambda: not get_peer_ids(), timeout=5)
-
-
-def connect_nodes(from_connection, node_num):
-    ip_port = "127.0.0.1:" + str(p2p_port(node_num))
-    from_connection.addnode(ip_port, "onetry")
-    # poll until version handshake complete to avoid race conditions
-    # with transaction relaying
-    # See comments in net_processing:
-    # * Must have a version message before anything else
-    # * Must have a verack message before anything else
-    wait_until_helper(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
-    wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
-
-
 # Transaction/Block functions
 #############################
 

test/functional/wallet_abandonconflict.py

@@ -16,8 +16,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
 )
 
 
@@ -50,7 +48,7 @@ class AbandonConflictTest(BitcoinTestFramework):
         balance = newbalance
 
         # Disconnect nodes so node0's transactions don't get into node1's mempool
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
 
         # Identify the 10btc outputs
         nA = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txA)["details"] if tx_out["amount"] == Decimal("10"))
@@ -161,7 +159,7 @@ class AbandonConflictTest(BitcoinTestFramework):
         self.nodes[1].sendrawtransaction(signed["hex"])
         self.nodes[1].generate(1)
 
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks()
 
         # Verify that B and C's 10 BTC outputs are available for spending again because AB1 is now conflicted

test/functional/wallet_address_types.py

@@ -62,7 +62,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 class AddressTypeTest(BitcoinTestFramework):
@@ -90,7 +89,7 @@ class AddressTypeTest(BitcoinTestFramework):
         # Fully mesh-connect nodes for faster mempool sync
         for i, j in itertools.product(range(self.num_nodes), repeat=2):
             if i > j:
-                connect_nodes(self.nodes[i], j)
+                self.connect_nodes(i, j)
         self.sync_all()
 
     def get_balances(self, key='trusted'):

test/functional/wallet_avoidreuse.py

@@ -9,7 +9,6 @@ from test_framework.util import (
     assert_approx,
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 def reset_balance(node, discardaddr):
@@ -111,7 +110,7 @@ class AvoidReuseTest(BitcoinTestFramework):
         assert_equal(self.nodes[1].getwalletinfo()["avoid_reuse"], True)
 
         self.restart_node(1)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         # Flags should still be node1.avoid_reuse=false, node2.avoid_reuse=true
         assert_equal(self.nodes[0].getwalletinfo()["avoid_reuse"], False)

test/functional/wallet_backup.py

@@ -39,7 +39,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 
@@ -62,10 +61,10 @@ class WalletBackupTest(BitcoinTestFramework):
 
     def setup_network(self):
         self.setup_nodes()
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
-        connect_nodes(self.nodes[1], 3)
+        self.connect_nodes(1, 3)
-        connect_nodes(self.nodes[2], 3)
+        self.connect_nodes(2, 3)
-        connect_nodes(self.nodes[2], 0)
+        self.connect_nodes(2, 0)
         self.sync_all()
 
     def one_send(self, from_node, to_address):
@@ -96,10 +95,10 @@ class WalletBackupTest(BitcoinTestFramework):
         self.start_node(0)
         self.start_node(1)
         self.start_node(2)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
-        connect_nodes(self.nodes[1], 3)
+        self.connect_nodes(1, 3)
-        connect_nodes(self.nodes[2], 3)
+        self.connect_nodes(2, 3)
-        connect_nodes(self.nodes[2], 0)
+        self.connect_nodes(2, 0)
 
     def stop_three(self):
         self.stop_node(0)

test/functional/wallet_balance.py

@@ -11,7 +11,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 
@@ -262,7 +261,7 @@ class WalletTest(BitcoinTestFramework):
 
         # Now confirm tx_orig
         self.restart_node(1, ['-persistmempool=0'])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_blocks()
         self.nodes[1].sendrawtransaction(tx_orig)
         self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)

test/functional/wallet_basic.py

@@ -11,7 +11,6 @@ from test_framework.util import (
     assert_equal,
     assert_fee_amount,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 from test_framework.wallet_util import test_address
 
@@ -32,9 +31,9 @@ class WalletTest(BitcoinTestFramework):
         self.setup_nodes()
         # Only need nodes 0-2 running at start of test
         self.stop_node(3)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_all(self.nodes[0:3])
 
     def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size):
@@ -281,7 +280,7 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].getbalance(), node_0_bal)
 
         self.start_node(3, self.nodes[3].extra_args)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
         self.sync_all()
 
         # check if we can list zero value tx as available coins
@@ -316,9 +315,9 @@ class WalletTest(BitcoinTestFramework):
         self.start_node(0, ["-walletbroadcast=0"])
         self.start_node(1, ["-walletbroadcast=0"])
         self.start_node(2, ["-walletbroadcast=0"])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_all(self.nodes[0:3])
 
         txid_not_broadcast = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2)
@@ -343,9 +342,9 @@ class WalletTest(BitcoinTestFramework):
         self.start_node(0)
         self.start_node(1)
         self.start_node(2)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_blocks(self.nodes[0:3])
 
         self.nodes[0].generate(1)

test/functional/wallet_hd.py

@@ -10,7 +10,6 @@ import shutil
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     assert_raises_rpc_error,
 )
 
@@ -99,7 +98,7 @@ class WalletHDTest(BitcoinTestFramework):
                 assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/" + str(i) + "'")
             assert_equal(hd_info_2["hdmasterfingerprint"], hd_fingerprint)
         assert_equal(hd_add, hd_add_2)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
 
         # Needs rescan
@@ -115,7 +114,7 @@ class WalletHDTest(BitcoinTestFramework):
             os.path.join(self.nodes[1].datadir, self.chain, "wallets", self.default_wallet_name, self.wallet_data_filename),
         )
         self.start_node(1, extra_args=self.extra_args[1])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
         # Wallet automatically scans blocks older than key on startup
         assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1)
@@ -183,7 +182,7 @@ class WalletHDTest(BitcoinTestFramework):
             # Restart node 1 with keypool of 3 and a different wallet
             self.nodes[1].createwallet(wallet_name='origin', blank=True)
             self.restart_node(1, extra_args=['-keypool=3', '-wallet=origin'])
-            connect_nodes(self.nodes[0], 1)
+            self.connect_nodes(0, 1)
 
             # sethdseed restoring and seeing txs to addresses out of the keypool
             origin_rpc = self.nodes[1].get_wallet_rpc('origin')

test/functional/wallet_import_rescan.py

@@ -22,7 +22,6 @@ happened previously.
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.address import AddressType
 from test_framework.util import (
-    connect_nodes,
     assert_equal,
     set_node_times,
 )
@@ -165,7 +164,7 @@ class ImportRescanTest(BitcoinTestFramework):
 
         self.start_nodes()
         for i in range(1, self.num_nodes):
-            connect_nodes(self.nodes[i], 0)
+            self.connect_nodes(i, 0)
 
     def run_test(self):
         # Create one transaction on node 0 with a unique amount for

test/functional/wallet_keypool_topup.py

@@ -16,7 +16,6 @@ import shutil
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
 )
 
 
@@ -38,9 +37,9 @@ class KeypoolRestoreTest(BitcoinTestFramework):
         self.stop_node(1)
         shutil.copyfile(wallet_path, wallet_backup_path)
         self.start_node(1, self.extra_args[1])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
-        connect_nodes(self.nodes[0], 3)
+        self.connect_nodes(0, 3)
 
         for i, output_type in enumerate(["legacy", "p2sh-segwit", "bech32"]):
 
@@ -72,7 +71,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
             self.stop_node(idx)
             shutil.copyfile(wallet_backup_path, wallet_path)
             self.start_node(idx, self.extra_args[idx])
-            connect_nodes(self.nodes[0], idx)
+            self.connect_nodes(0, idx)
             self.sync_all()
 
             self.log.info("Verify keypool is restored and balance is correct")

test/functional/wallet_listsinceblock.py

@@ -10,7 +10,6 @@ from test_framework.util import (
     assert_array_result,
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 from decimal import Decimal
@@ -26,7 +25,7 @@ class ListSinceBlockTest(BitcoinTestFramework):
     def run_test(self):
         # All nodes are in IBD from genesis, so they'll need the miner (node2) to be an outbound connection, or have
         # only one connection. (See fPreferredDownload in net_processing)
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.nodes[2].generate(101)
         self.sync_all()
 

test/functional/wallet_reorgsrestore.py

@@ -20,8 +20,6 @@ import shutil
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
         assert_equal,
-        connect_nodes,
-        disconnect_nodes,
 )
 
 class ReorgsRestoreTest(BitcoinTestFramework):
@@ -38,9 +36,9 @@ class ReorgsRestoreTest(BitcoinTestFramework):
         self.sync_blocks()
 
         # Disconnect node1 from others to reorg its chain later
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
-        disconnect_nodes(self.nodes[1], 2)
+        self.disconnect_nodes(1, 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
         # Send a tx to be unconfirmed later
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10"))
@@ -50,7 +48,7 @@ class ReorgsRestoreTest(BitcoinTestFramework):
         assert_equal(tx_before_reorg["confirmations"], 4)
 
         # Disconnect node0 from node2 to broadcast a conflict on their respective chains
-        disconnect_nodes(self.nodes[0], 2)
+        self.disconnect_nodes(0, 2)
         nA = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txid_conflict_from)["details"] if tx_out["amount"] == Decimal("10"))
         inputs = []
         inputs.append({"txid": txid_conflict_from, "vout": nA})
@@ -69,7 +67,7 @@ class ReorgsRestoreTest(BitcoinTestFramework):
         self.nodes[2].generate(9)
 
         # Reconnect node0 and node2 and check that conflicted_txid is effectively conflicted
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_blocks([self.nodes[0], self.nodes[2]])
         conflicted = self.nodes[0].gettransaction(conflicted_txid)
         conflicting = self.nodes[0].gettransaction(conflicting_txid)

test/functional/wallet_txn_clone.py

@@ -8,8 +8,6 @@ import io
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
 )
 from test_framework.messages import CTransaction, COIN
 
@@ -30,7 +28,7 @@ class TxnMallTest(BitcoinTestFramework):
     def setup_network(self):
         # Start with split network:
         super().setup_network()
-        disconnect_nodes(self.nodes[1], 2)
+        self.disconnect_nodes(1, 2)
 
     def run_test(self):
         if self.options.segwit:
@@ -118,7 +116,7 @@ class TxnMallTest(BitcoinTestFramework):
         self.nodes[2].generate(1)
 
         # Reconnect the split network, and sync chain:
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.nodes[2].sendrawtransaction(node0_tx2["hex"])
         self.nodes[2].sendrawtransaction(tx2["hex"])
         self.nodes[2].generate(1)  # Mine another block to make sure we sync

test/functional/wallet_txn_doublespend.py

@@ -8,8 +8,6 @@ from decimal import Decimal
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
     find_output,
 )
 
@@ -28,7 +26,7 @@ class TxnMallTest(BitcoinTestFramework):
     def setup_network(self):
         # Start with split network:
         super().setup_network()
-        disconnect_nodes(self.nodes[1], 2)
+        self.disconnect_nodes(1, 2)
 
     def run_test(self):
         # All nodes should start with 1,250 BTC:
@@ -116,7 +114,7 @@ class TxnMallTest(BitcoinTestFramework):
         self.nodes[2].generate(1)
 
         # Reconnect the split network, and sync chain:
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.nodes[2].generate(1)  # Mine another block to make sure we sync
         self.sync_blocks()
         assert_equal(self.nodes[0].gettransaction(doublespend_txid)["confirmations"], 2)