From e19d72f1c6558c682b64dfad781a5a9cd06f5149 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 8 May 2020 19:31:13 -0400
Subject: [PATCH 01/10] Merge #18866: test: Fix verack race to avoid
 intermittent test failures

fae153b40968bfd974a4709bcd841a59447abf18 test: Fix verack race to avoid intermittent test failures (MarcoFalke)

Pull request description:

  Fixes #18832

ACKs for top commit:
  laanwj:
    ACK fae153b40968bfd974a4709bcd841a59447abf18

Tree-SHA512: 071de8c8e2b2787c9433c7460e18b9a54beaf471a52ce848c5ac7263fc2a40f5b976d4f558ecc494fd0fa07284b7c98d29267cade58f80ab74fe9a7d18d94298
---
 test/functional/test_framework/util.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 14bad1bd66672..937257f050467 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -382,7 +382,11 @@ def connect_nodes(from_connection, node_num):
     from_connection.addnode(ip_port, "onetry")
     # poll until version handshake complete to avoid race conditions
     # with transaction relaying
-    wait_until(lambda:  all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
+    # See comments in net_processing:
+    # * Must have a version message before anything else
+    # * Must have a verack message before anything else
+    wait_until(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
+    wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
 
 def connect_nodes_clique(nodes):
     # max_workers should be the maximum number of nodes that we have in the same functional test,

From d73ac82f6c45bb17e1691389863f8d4b3a284443 Mon Sep 17 00:00:00 2001
From: Alessandro Rezzi <alessandrorezzi2000@gmail.com>
Date: Mon, 11 Nov 2024 09:24:34 +0100
Subject: [PATCH 02/10] test: refactor connect_nodes and disconnect_nodes to
 take two indices instead of index + node

---
 test/functional/example_test.py               |  5 +-
 test/functional/feature_abortnode.py          |  4 +-
 test/functional/feature_fee_estimation.py     |  8 +--
 test/functional/feature_minchainwork.py       |  4 +-
 test/functional/feature_notifications.py      |  5 +-
 test/functional/interface_rest.py             |  4 +-
 test/functional/mining_pos_reorg.py           | 11 ++--
 test/functional/p2p_disconnect_ban.py         | 11 ++--
 test/functional/p2p_quorum_connect.py         |  3 +-
 test/functional/p2p_sendheaders.py            |  4 +-
 test/functional/p2p_time_offset.py            | 23 ++++---
 test/functional/p2p_unrequested_blocks.py     |  4 +-
 test/functional/rpc_invalidateblock.py        |  6 +-
 test/functional/rpc_net.py                    | 12 ++--
 test/functional/rpc_rawtransaction.py         |  3 +-
 test/functional/sapling_wallet.py             |  8 +--
 test/functional/sapling_wallet_anchorfork.py  | 10 +--
 .../test_framework/test_framework.py          | 65 ++++++++++++++++---
 test/functional/test_framework/util.py        | 44 -------------
 test/functional/tiertwo_chainlocks.py         | 11 ++--
 test/functional/tiertwo_deterministicmns.py   |  3 +-
 test/functional/tiertwo_governance_reorg.py   | 12 ++--
 .../tiertwo_governance_sync_basic.py          |  5 --
 .../tiertwo_masternode_activation.py          |  6 +-
 test/functional/tiertwo_mn_compatibility.py   | 11 ++--
 test/functional/tiertwo_reorg_mempool.py      | 10 ++-
 test/functional/wallet_abandonconflict.py     |  8 +--
 test/functional/wallet_backup.py              | 17 +++--
 test/functional/wallet_basic.py               |  7 +-
 test/functional/wallet_hd.py                  | 11 ++--
 test/functional/wallet_import_rescan.py       |  3 +-
 test/functional/wallet_keypool_topup.py       |  9 +--
 test/functional/wallet_listsinceblock.py      |  3 +-
 test/functional/wallet_reorgsrestore.py       | 16 ++---
 test/functional/wallet_txn_clone.py           | 15 ++---
 test/functional/wallet_txn_doublespend.py     | 14 ++--
 36 files changed, 175 insertions(+), 220 deletions(-)

diff --git a/test/functional/example_test.py b/test/functional/example_test.py
index 5562b55b04dd3..d1a519ff330f3 100755
--- a/test/functional/example_test.py
+++ b/test/functional/example_test.py
@@ -26,7 +26,6 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     wait_until,
 )
 
@@ -111,7 +110,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:1])
 
     # Use setup_nodes() to customize the node start behaviour (for example if
@@ -182,7 +181,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("Add P2P connection to node2")
         self.nodes[0].disconnect_p2ps()
diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py
index 9fbd831f866bb..e18b1356bf183 100755
--- a/test/functional/feature_abortnode.py
+++ b/test/functional/feature_abortnode.py
@@ -13,7 +13,7 @@
 import os
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import wait_until, get_datadir_path, connect_nodes
+from test_framework.util import wait_until, get_datadir_path
 
 
 class AbortNodeTest(PivxTestFramework):
@@ -37,7 +37,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
diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py
index 219c4f2f24742..28c151dd938ba 100755
--- a/test/functional/feature_fee_estimation.py
+++ b/test/functional/feature_fee_estimation.py
@@ -10,7 +10,7 @@
 from test_framework.messages import CTransaction, CTxIn, CTxOut, COutPoint, ToHex, COIN
 from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import connect_nodes, satoshi_round
+from test_framework.util import satoshi_round
 
 
 # Use as minTxFee
@@ -247,9 +247,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()
 
diff --git a/test/functional/feature_minchainwork.py b/test/functional/feature_minchainwork.py
index 7d915b5baa4fe..29ffff524c8bc 100755
--- a/test/functional/feature_minchainwork.py
+++ b/test/functional/feature_minchainwork.py
@@ -18,7 +18,7 @@
 import time
 
 from test_framework.test_framework import PivxTestFramework
-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)
@@ -40,7 +40,7 @@ def setup_network(self):
         # 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
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
index ecb48b08d4284..bdf95f962d4f2 100755
--- a/test/functional/feature_notifications.py
+++ b/test/functional/feature_notifications.py
@@ -9,8 +9,7 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    wait_until,
-    connect_nodes,
+    wait_until
 )
 
 
@@ -66,7 +65,7 @@ def run_test(self):
             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)
 
             wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10)
 
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index e6b7e92eb51cc..dafd92dedde5a 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -13,7 +13,7 @@
 import urllib.parse
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, assert_greater_than, connect_nodes, hex_str_to_bytes
+from test_framework.util import assert_equal, assert_greater_than, hex_str_to_bytes
 
 
 def deser_uint256(f):
@@ -52,7 +52,7 @@ def set_test_params(self):
 
     def setup_network(self, split=False):
         super().setup_network()
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
     def run_test(self):
         url = urllib.parse.urlparse(self.nodes[0].url)
diff --git a/test/functional/mining_pos_reorg.py b/test/functional/mining_pos_reorg.py
index 29a01b454a02f..6370584b618f8 100755
--- a/test/functional/mining_pos_reorg.py
+++ b/test/functional/mining_pos_reorg.py
@@ -7,9 +7,6 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
-    connect_nodes_clique,
-    disconnect_nodes,
     set_node_times,
     DecimalAmt,
 )
@@ -28,7 +25,7 @@ def setup_chain(self):
     def setup_network(self):
         # connect all nodes between each other
         self.setup_nodes()
-        connect_nodes_clique(self.nodes)
+        self.connect_nodes_clique(self.nodes)
         self.sync_all()
 
     def log_title(self):
@@ -42,7 +39,7 @@ def disconnect_all(self):
         for i in range(self.num_nodes):
             for j in range(self.num_nodes):
                 if j != i:
-                    disconnect_nodes(self.nodes[i], j)
+                    self.disconnect_nodes(i, j)
         self.log.info("Nodes disconnected")
 
     def get_tot_balance(self, nodeid):
@@ -109,7 +106,7 @@ def findUtxoInList(txid, vout, utxo_list):
 
         # Connect with node 2 and sync
         self.log.info("Reconnecting node 0 and node 2")
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
         self.sync_blocks([self.nodes[i] for i in [0, 2]])
 
         # verify that the stakeinput can't be spent
@@ -140,7 +137,7 @@ def findUtxoInList(txid, vout, utxo_list):
         new_best_hash = self.nodes[1].getbestblockhash()
         self.log.info("Connecting and syncing nodes...")
         set_node_times(self.nodes, self.mocktime)
-        connect_nodes_clique(self.nodes)
+        self.connect_nodes_clique(self.nodes)
         self.sync_blocks()
         for i in [0, 2]:
             assert_equal(self.nodes[i].getbestblockhash(), new_best_hash)
diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py
index bb2231d07e627..3d7475404cbfc 100755
--- a/test/functional/p2p_disconnect_ban.py
+++ b/test/functional/p2p_disconnect_ban.py
@@ -9,7 +9,6 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     assert_raises_rpc_error,
     wait_until,
 )
@@ -21,8 +20,8 @@ def set_test_params(self):
 
     def run_test(self):
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
 
         self.log.info("Test setban and listbanned RPCs")
 
@@ -81,8 +80,8 @@ def run_test(self):
         # Clear ban lists
         self.nodes[1].clearbanned()
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
 
         self.log.info("Test disconnectnode RPCs")
 
@@ -101,7 +100,7 @@ def run_test(self):
         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]
 
diff --git a/test/functional/p2p_quorum_connect.py b/test/functional/p2p_quorum_connect.py
index bda5ad6faa3fe..ccccae641dbf4 100755
--- a/test/functional/p2p_quorum_connect.py
+++ b/test/functional/p2p_quorum_connect.py
@@ -13,7 +13,6 @@
 from test_framework.messages import msg_version
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     hash256,
     wait_until,
 )
@@ -174,7 +173,7 @@ def run_test(self):
         self.clean_conns_and_disconnect(mn6_node)
 
         # Create the regular connection
-        connect_nodes(mn5_node, mn6.idx)
+        self.connect_nodes(mn5.idx, mn6.idx)
         self.wait_for_peers_count([mn5_node], 1)
         assert self.has_single_regular_connection(mn5_node)
         assert self.has_single_regular_connection(mn6_node)
diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py
index 04e8e114c52a7..0f19179f2524c 100755
--- a/test/functional/p2p_sendheaders.py
+++ b/test/functional/p2p_sendheaders.py
@@ -88,7 +88,7 @@
 )
 from test_framework.mininode import mininode_lock, NetworkThread, P2PInterface
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, wait_until, connect_nodes, p2p_port
+from test_framework.util import assert_equal, wait_until, p2p_port
 
 
 direct_fetch_response_time = 0.05
@@ -242,7 +242,7 @@ def __init__(self):
     def setup_network(self):
         self.nodes = []
         self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, [["-debug", "-logtimemicros=1"]]*2)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
     # mine count blocks and return the new tip
     def mine_blocks(self, count):
diff --git a/test/functional/p2p_time_offset.py b/test/functional/p2p_time_offset.py
index 7911c05c2e358..2ace694ec7e48 100755
--- a/test/functional/p2p_time_offset.py
+++ b/test/functional/p2p_time_offset.py
@@ -8,15 +8,10 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     set_node_times,
 )
 
 
-def connect_nodes_bi(nodes, a, b):
-    connect_nodes(nodes[a], b)
-    connect_nodes(nodes[b], a)
-
 class TimeOffsetTest(PivxTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
@@ -27,6 +22,10 @@ def setup_network(self):
         # don't connect nodes yet
         self.setup_nodes()
 
+    def connect_nodes_bi(self, a, b):
+        self.connect_nodes(a, b)
+        self.connect_nodes(b, a)
+
     def check_connected_nodes(self):
         ni = [node.getnetworkinfo() for node in self.connected_nodes]
         assert_equal([x['connections'] for x in ni], [2] * len(ni))
@@ -52,8 +51,8 @@ def run_test(self):
 
         # connect nodes 1 and 2
         self.log.info("Connecting with node-1 (+10 s) and node-2 (+15 s)...")
-        connect_nodes_bi(self.nodes, 0, 1)
-        connect_nodes_bi(self.nodes, 0, 2)
+        self.connect_nodes_bi(0, 1)
+        self.connect_nodes_bi(0, 2)
         self.log.info("--> samples = [+0, +10, (+10), +15, +15]")
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 4)
@@ -64,7 +63,7 @@ def run_test(self):
 
         # connect node 3
         self.log.info("Connecting with node-3 (+20 s). This will print the warning...")
-        connect_nodes_bi(self.nodes, 0, 3)
+        self.connect_nodes_bi(0, 3)
         self.log.info("--> samples = [+0, +10, +10, (+15), +15, +20, +20]")
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 6)
@@ -75,7 +74,7 @@ def run_test(self):
 
         # connect node 6
         self.log.info("Connecting with node-6 (-5 s)...")
-        connect_nodes_bi(self.nodes, 0, 6)
+        self.connect_nodes_bi(0, 6)
         self.log.info("--> samples = [-5, -5, +0, +10, (+10), +15, +15, +20, +20]")
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 8)
@@ -86,7 +85,7 @@ def run_test(self):
 
         # connect node 4
         self.log.info("Connecting with node-4 (+25 s). This will print the warning...")
-        connect_nodes_bi(self.nodes, 0, 4)
+        self.connect_nodes_bi(0, 4)
         self.log.info("--> samples = [-5, -5, +0, +10, +10, (+15), +15, +20, +20, +25, +25]")
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 10)
@@ -97,7 +96,7 @@ def run_test(self):
 
         # try to connect node 5 and check that it can't
         self.log.info("Trying to connect with node-5 (+30 s)...")
-        connect_nodes_bi(self.nodes, 0, 5)
+        self.connect_nodes_bi(0, 5)
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 10)
         assert_equal(ni['timeoffset'], 15)
@@ -106,7 +105,7 @@ def run_test(self):
 
         # connect node 7
         self.log.info("Connecting with node-7 (-10 s)...")
-        connect_nodes_bi(self.nodes, 0, 7)
+        self.connect_nodes_bi(0, 7)
         self.log.info("--> samples = [-10, -10, -5, -5, +0, +10, (+10), +15, +15, +20, +20, +25, +25]")
         ni = self.nodes[0].getnetworkinfo()
         assert_equal(ni['connections'], 12)
diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py
index 1bbc5b11f95d0..65a468c0e2113 100755
--- a/test/functional/p2p_unrequested_blocks.py
+++ b/test/functional/p2p_unrequested_blocks.py
@@ -58,7 +58,7 @@
 from test_framework.messages import CBlockHeader, CInv, msg_block, msg_headers, msg_inv
 from test_framework.mininode import mininode_lock, P2PInterface
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes
+from test_framework.util import assert_equal, assert_raises_rpc_error
 
 
 class AcceptBlockTest(PivxTestFramework):
@@ -311,7 +311,7 @@ def run_test(self):
         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")
 
diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py
index 873bf7de9e08f..ed1c62382d646 100755
--- a/test/functional/rpc_invalidateblock.py
+++ b/test/functional/rpc_invalidateblock.py
@@ -5,7 +5,7 @@
 """Test the invalidateblock RPC."""
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, connect_nodes, wait_until
+from test_framework.util import assert_equal, wait_until
 
 
 class InvalidateTest(PivxTestFramework):
@@ -29,7 +29,7 @@ def run_test(self):
         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)
@@ -40,7 +40,7 @@ def run_test(self):
         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)
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 7d715bee9413b..7399a8b2dcc29 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -15,8 +15,6 @@
     assert_greater_than_or_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
     p2p_port,
     wait_until,
 )
@@ -28,8 +26,8 @@ def set_test_params(self):
 
     def run_test(self):
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
 
         self._test_connection_count()
         self._test_getnettotals()
@@ -69,13 +67,13 @@ def _test_getnettotals(self):
     def _test_getnetworkinginfo(self):
         assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2)
 
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
         # Wait a bit for all sockets to close
         wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3)
 
         self.log.info("Connect nodes both way")
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
         assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2)
 
     def _test_getaddednodeinfo(self):
diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 9d0c82dfaae7e..f80cba1fe6905 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -18,7 +18,6 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes
 )
 
 
@@ -48,7 +47,7 @@ def set_test_params(self):
 
     def setup_network(self, split=False):
         super().setup_network()
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 2)
 
     def run_test(self):
 
diff --git a/test/functional/sapling_wallet.py b/test/functional/sapling_wallet.py
index 6e4a94daf0c1a..24133d0a776bc 100755
--- a/test/functional/sapling_wallet.py
+++ b/test/functional/sapling_wallet.py
@@ -11,8 +11,6 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
-    disconnect_nodes,
     satoshi_round,
     get_coinstake_address,
     wait_until,
@@ -77,7 +75,7 @@ def run_test(self):
         self.restart_node(0, extra_args=self.extra_args[0]+['-minrelaytxfee=0.0000001'])
         rawtx_hex = self.nodes[0].rawshieldsendmany("from_transparent", recipients, 1)
         self.restart_node(0, extra_args=self.extra_args[0])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         assert_raises_rpc_error(-26, "insufficient fee",
                                 self.nodes[0].sendrawtransaction, rawtx_hex)
         self.log.info("Good. Not accepted in the mempool.")
@@ -132,7 +130,7 @@ def run_test(self):
         self.log.info("Balances check out")
 
         # Now disconnect the block, activate SPORK_20, and try to reconnect it
-        disconnect_nodes(self.nodes[0], 1)
+        self.disconnect_nodes(0, 1)
         tip_hash = self.nodes[0].getbestblockhash()
         self.nodes[0].invalidateblock(tip_hash)
         assert tip_hash != self.nodes[0].getbestblockhash()
@@ -152,7 +150,7 @@ def run_test(self):
         assert_equal(tip_hash, self.nodes[0].getbestblockhash())    # Block connected
         assert_equal(self.nodes[0].getshieldbalance(saplingAddr0), Decimal('30'))
         self.log.info("Reconnected after deactivation of SPORK_20. Balance restored.")
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         # Node 0 sends some shield funds to node 1
         # Sapling -> Sapling
diff --git a/test/functional/sapling_wallet_anchorfork.py b/test/functional/sapling_wallet_anchorfork.py
index 53ad75c708cb5..73dcab4d2db6c 100755
--- a/test/functional/sapling_wallet_anchorfork.py
+++ b/test/functional/sapling_wallet_anchorfork.py
@@ -7,7 +7,7 @@
 from decimal import Decimal
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, connect_nodes, get_coinstake_address
+from test_framework.util import assert_equal, get_coinstake_address
 
 class WalletAnchorForkTest(PivxTestFramework):
 
@@ -61,7 +61,7 @@ def run_test (self):
         self.start_node(0, self.extra_args[0])
         self.start_node(1, self.extra_args[1])
         self.start_node(2, self.extra_args[2])
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
 
         # Partition B, node 1 mines an empty block
         self.nodes[1].generate(1)
@@ -95,9 +95,9 @@ def run_test (self):
 
         # Relaunch nodes and reconnect the entire network
         self.start_nodes()
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[2], 1)
-        connect_nodes(self.nodes[2], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(2, 1)
+        self.connect_nodes(2, 0)
 
         # Mine a new block and let it propagate
         self.nodes[1].generate(1)
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 9a41d209c54dc..a32c8de6d2708 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Base class for RPC testing."""
 
+from concurrent.futures import ThreadPoolExecutor
 from enum import Enum
 from io import BytesIO
 import logging
@@ -43,9 +44,6 @@
     assert_equal,
     assert_greater_than,
     check_json_precision,
-    connect_nodes,
-    connect_nodes_clique,
-    disconnect_nodes,
     get_collateral_vout,
     Decimal,
     DEFAULT_FEE,
@@ -251,7 +249,7 @@ def setup_network(self):
         # If further outbound connections are needed, they can be added at the beginning of the test with e.g.
         # connect_nodes(self.nodes[1], 2)
         for i in range(self.num_nodes - 1):
-            connect_nodes(self.nodes[i + 1], i)
+            self.connect_nodes(i + 1, i)
         self.sync_all()
 
     def setup_nodes(self):
@@ -343,12 +341,59 @@ def restart_node(self, i, extra_args=None):
     def wait_for_node_exit(self, i, timeout):
         self.nodes[i].process.wait(timeout)
 
+    def connect_nodes(self, 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(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
+            wait_until(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):
+        def disconnect_nodes_helper(from_connection, node_num):
+            for addr in [peer['addr'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']]:
+                try:
+                    from_connection.disconnectnode(addr)
+                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(lambda: [peer['addr'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']] == [], timeout=5)
+        disconnect_nodes_helper(self.nodes[a], b)
+
+    def connect_nodes_clique(self, nodes):
+        # max_workers should be the maximum number of nodes that we have in the same functional test,
+        # 15 seems to be a good upper bound
+        parallel_exec = ThreadPoolExecutor(max_workers=15)
+        l = len(nodes)
+
+        def connect_nodes_clique_internal(a):
+            for b in range(0, l):
+                self.connect_nodes(a, b)
+        jobs = []
+        for a in range(l):
+            jobs.append(parallel_exec.submit(connect_nodes_clique_internal, a))
+
+        for job in jobs:
+            job.result()
+        jobs.clear()
+        parallel_exec.shutdown()
+
     def split_network(self):
         """
         Split the network of four nodes into nodes 0/1 and 2/3.
         """
-        disconnect_nodes(self.nodes[1], 2)
-        disconnect_nodes(self.nodes[2], 1)
+        self.disconnect_nodes(1, 2)
+        self.disconnect_nodes(2, 1)
         self.sync_all(self.nodes[:2])
         self.sync_all(self.nodes[2:])
 
@@ -356,7 +401,7 @@ def join_network(self):
         """
         Join the (previously split) network halves together.
         """
-        connect_nodes(self.nodes[1], 2)
+        self.connect_nodes(1, 2)
         self.sync_all()
 
     def sync_blocks(self, nodes=None, wait=1, timeout=60):
@@ -538,7 +583,7 @@ def start_nodes_from_dir(ddir, num_nodes=MAX_NODES):
             for node in range(4):
                 self.nodes[node].wait_for_rpc_connection()
             self.log.info("Connecting nodes")
-            connect_nodes_clique(self.nodes)
+            self.connect_nodes_clique(self.nodes)
 
         def stop_and_clean_cache_dir(ddir):
             self.stop_nodes()
@@ -1176,7 +1221,7 @@ def setupMasternode(self,
     def connect_to_all(self, nodePos):
         for i in range(self.num_nodes):
             if i != nodePos and self.nodes[i] is not None:
-                connect_nodes(self.nodes[i], nodePos)
+                self.connect_nodes(i, nodePos)
 
     def assert_equal_for_all(self, expected, func_name, *args):
         def not_found():
@@ -1415,7 +1460,7 @@ def wait_until_mnsync_completed(self):
     def setup_test(self):
         self.mns = []
         self.disable_mocktime()
-        connect_nodes_clique(self.nodes)
+        self.connect_nodes_clique(self.nodes)
 
         # Enforce mn payments and reject legacy mns at block 131
         self.activate_spork(0, "SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT")
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 937257f050467..32010def457fc 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -13,7 +13,6 @@
 import os
 import random
 import re
-from concurrent.futures import ThreadPoolExecutor
 from subprocess import CalledProcessError
 import time
 
@@ -363,49 +362,6 @@ def set_node_times(nodes, t):
     for node in nodes:
         node.setmocktime(t)
 
-def disconnect_nodes(from_connection, node_num):
-    for addr in [peer['addr'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']]:
-        try:
-            from_connection.disconnectnode(addr)
-        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(lambda: [peer['addr'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']] == [], 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(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
-    wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
-
-def connect_nodes_clique(nodes):
-    # max_workers should be the maximum number of nodes that we have in the same functional test,
-    # 15 seems to be a good upper bound
-    parallel_exec = ThreadPoolExecutor(max_workers=15)
-    l = len(nodes)
-
-    def connect_nodes_clique_internal(a):
-        for b in range(0, l):
-            connect_nodes(nodes[a], b)
-    jobs = []
-    for a in range(l):
-        jobs.append(parallel_exec.submit(connect_nodes_clique_internal, a))
-
-    for job in jobs:
-        job.result()
-    jobs.clear()
-    parallel_exec.shutdown()
-
 # Transaction/Block functions
 #############################
 
diff --git a/test/functional/tiertwo_chainlocks.py b/test/functional/tiertwo_chainlocks.py
index 5633b485fd1e4..a46da934ce8ab 100755
--- a/test/functional/tiertwo_chainlocks.py
+++ b/test/functional/tiertwo_chainlocks.py
@@ -5,10 +5,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 from test_framework.test_framework import PivxDMNTestFramework
-from test_framework.util import (
-    assert_equal,
-    connect_nodes,
-)
+from test_framework.util import assert_equal
 import time
 
 '''
@@ -60,7 +57,7 @@ def run_test(self):
         self.wait_for_chainlock_tip(self.nodes[1])
         assert self.nodes[0].getbestblockhash() == node0_tip
         self.nodes[0].setnetworkactive(True)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.nodes[1].generate(1)
         self.wait_for_chainlock(self.nodes[0], self.nodes[1].getbestblockhash())
 
@@ -72,7 +69,7 @@ def run_test(self):
         self.wait_for_chainlock_tip(self.nodes[1])
         assert not self.nodes[0].getblock(self.nodes[0].getbestblockhash())["chainlock"]
         self.nodes[0].setnetworkactive(True)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.nodes[1].generate(1)
         self.wait_for_chainlock(self.nodes[0], self.nodes[1].getbestblockhash())
         assert self.nodes[0].getblock(self.nodes[0].getbestblockhash())["previousblockhash"] == good_tip
@@ -83,7 +80,7 @@ def run_test(self):
         # Restart it so that it forgets all the chainlocks from the past
         self.stop_node(0)
         self.start_node(0, extra_args=self.extra_args[0])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
         # Now try to reorg the chain
         self.nodes[0].generate(2)
diff --git a/test/functional/tiertwo_deterministicmns.py b/test/functional/tiertwo_deterministicmns.py
index 8e55980922490..f42a2b16306bf 100755
--- a/test/functional/tiertwo_deterministicmns.py
+++ b/test/functional/tiertwo_deterministicmns.py
@@ -16,7 +16,6 @@
     assert_equal,
     assert_raises_rpc_error,
     create_new_dmn,
-    connect_nodes,
     hex_str_to_bytes,
     is_coin_locked_by,
     spend_mn_collateral,
@@ -74,7 +73,7 @@ def create_block(self, mn_payee_script, prev_block):
     def restart_controller(self):
         self.restart_node(self.controllerPos, extra_args=self.extra_args[self.controllerPos])
         self.connect_to_all(self.controllerPos)
-        connect_nodes(self.nodes[self.controllerPos], self.minerPos)
+        self.connect_nodes(self.controllerPos, self.minerPos)
         self.sync_all()
 
     def wait_until_mnsync_completed(self):
diff --git a/test/functional/tiertwo_governance_reorg.py b/test/functional/tiertwo_governance_reorg.py
index 6d6cb05233f14..e408509870438 100755
--- a/test/functional/tiertwo_governance_reorg.py
+++ b/test/functional/tiertwo_governance_reorg.py
@@ -10,8 +10,6 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
-    disconnect_nodes,
     p2p_port,
     set_node_times,
 )
@@ -183,17 +181,17 @@ def send_3_pings(self, mn_list):
     def split_network(self):
         for i in range(self.num_nodes):
             if i != self.minerBPos:
-                disconnect_nodes(self.nodes[i], self.minerBPos)
-                disconnect_nodes(self.nodes[self.minerBPos], i)
+                self.disconnect_nodes(i, self.minerBPos)
+                self.disconnect_nodes(self.minerBPos, i)
         # by-pass ring connection
         assert self.minerBPos > 0
-        connect_nodes(self.nodes[self.minerBPos-1], self.minerBPos+1)
+        self.connect_nodes(self.minerBPos-1, self.minerBPos+1)
 
     def reconnect_nodes(self):
         for i in range(self.num_nodes):
             if i != self.minerBPos:
-                connect_nodes(self.nodes[i], self.minerBPos)
-                connect_nodes(self.nodes[self.minerBPos], i)
+                self.connect_nodes(i, self.minerBPos)
+                self.connect_nodes(self.minerBPos, i)
 
     def create_and_check_superblock(self, node, next_superblock, payee):
         self.stake_and_ping(self.nodes.index(node), 1, [])
diff --git a/test/functional/tiertwo_governance_sync_basic.py b/test/functional/tiertwo_governance_sync_basic.py
index 7121b9dc11c32..0ede54cd7916d 100755
--- a/test/functional/tiertwo_governance_sync_basic.py
+++ b/test/functional/tiertwo_governance_sync_basic.py
@@ -18,7 +18,6 @@
 from test_framework.util import (
     assert_equal,
     assert_true,
-    connect_nodes,
     get_datadir_path,
     satoshi_round
 )
@@ -130,10 +129,6 @@ def check_budgetprojection(self, expected):
             assert_equal(self.nodes[i].getbudgetprojection(), expected)
             self.log.info("Budget projection valid for node %d" % i)
 
-    def connect_nodes_bi(self, nodes, a, b):
-        connect_nodes(nodes[a], b)
-        connect_nodes(nodes[b], a)
-
     def create_proposals_tx(self, props):
         nextSuperBlockHeight = self.miner.getnextsuperblock()
         for entry in props:
diff --git a/test/functional/tiertwo_masternode_activation.py b/test/functional/tiertwo_masternode_activation.py
index 1ce26a98ca6d5..96ac694e67d05 100755
--- a/test/functional/tiertwo_masternode_activation.py
+++ b/test/functional/tiertwo_masternode_activation.py
@@ -17,8 +17,6 @@
 
 from test_framework.test_framework import PivxTier2TestFramework
 from test_framework.util import (
-    connect_nodes_clique,
-    disconnect_nodes,
     wait_until,
 )
 
@@ -29,10 +27,10 @@ def disconnect_remotes(self):
         for i in [self.remoteOnePos, self.remoteTwoPos]:
             for j in range(self.num_nodes):
                 if i != j:
-                    disconnect_nodes(self.nodes[i], j)
+                    self.disconnect_nodes(i, j)
 
     def reconnect_remotes(self):
-        connect_nodes_clique(self.nodes)
+        self.connect_nodes_clique(self.nodes)
         self.sync_all()
 
     def reconnect_and_restart_masternodes(self):
diff --git a/test/functional/tiertwo_mn_compatibility.py b/test/functional/tiertwo_mn_compatibility.py
index aeb7f30604e27..c9bdd4e911caf 100755
--- a/test/functional/tiertwo_mn_compatibility.py
+++ b/test/functional/tiertwo_mn_compatibility.py
@@ -9,10 +9,7 @@
 from decimal import Decimal
 
 from test_framework.test_framework import PivxTier2TestFramework
-from test_framework.util import (
-    assert_equal,
-    connect_nodes,
-)
+from test_framework.util import assert_equal
 
 
 class MasternodeCompatibilityTest(PivxTier2TestFramework):
@@ -105,9 +102,9 @@ def run_test(self):
         self.remoteDMN2 = self.nodes[self.remoteDMN2Pos]
         self.remoteDMN3 = self.nodes[self.remoteDMN3Pos]
         # add more direct connections to the miner
-        connect_nodes(self.miner, 2)
-        connect_nodes(self.remoteTwo, 0)
-        connect_nodes(self.remoteDMN2, 0)
+        self.connect_nodes(self.minerPos, 2)
+        self.connect_nodes(self.remoteTwoPos, 0)
+        self.connect_nodes(self.remoteDMN2Pos, 0)
         self.sync_all()
 
         # check mn list from miner
diff --git a/test/functional/tiertwo_reorg_mempool.py b/test/functional/tiertwo_reorg_mempool.py
index 12bfddcc09e1c..c3a0a0482e9ca 100755
--- a/test/functional/tiertwo_reorg_mempool.py
+++ b/test/functional/tiertwo_reorg_mempool.py
@@ -20,8 +20,6 @@
     assert_greater_than,
     assert_raises_rpc_error,
     create_new_dmn,
-    connect_nodes,
-    disconnect_nodes,
     get_collateral_vout,
 )
 
@@ -40,13 +38,13 @@ def setup_network(self):
         self.connect_all()
 
     def connect_all(self):
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
 
     def disconnect_all(self):
         self.log.info("Disconnecting nodes...")
-        disconnect_nodes(self.nodes[0], 1)
-        disconnect_nodes(self.nodes[1], 0)
+        self.disconnect_nodes(0, 1)
+        self.disconnect_nodes(1, 0)
         self.log.info("Nodes disconnected")
 
     def register_masternode(self, from_node, dmn, collateral_addr):
diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py
index b7f251b63928f..01e2799bb2137 100755
--- a/test/functional/wallet_abandonconflict.py
+++ b/test/functional/wallet_abandonconflict.py
@@ -7,9 +7,7 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
-    Decimal,
-    disconnect_nodes,
+    Decimal
 )
 
 class AbandonConflictTest(PivxTestFramework):
@@ -41,7 +39,7 @@ def run_test(self):
         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(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txA, 1)["vout"]) if vout["value"] == 10)
@@ -152,7 +150,7 @@ def run_test(self):
         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
diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py
index 4c839e8e329cc..c42802406481d 100755
--- a/test/functional/wallet_backup.py
+++ b/test/functional/wallet_backup.py
@@ -40,7 +40,6 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 
@@ -53,10 +52,10 @@ def set_test_params(self):
 
     def setup_network(self, split=False):
         self.setup_nodes()
-        connect_nodes(self.nodes[0], 3)
-        connect_nodes(self.nodes[1], 3)
-        connect_nodes(self.nodes[2], 3)
-        connect_nodes(self.nodes[2], 0)
+        self.connect_nodes(0, 3)
+        self.connect_nodes(1, 3)
+        self.connect_nodes(2, 3)
+        self.connect_nodes(2, 0)
         self.sync_all()
 
     def one_send(self, from_node, to_address):
@@ -87,10 +86,10 @@ def start_three(self):
         self.start_node(0)
         self.start_node(1)
         self.start_node(2)
-        connect_nodes(self.nodes[0], 3)
-        connect_nodes(self.nodes[1], 3)
-        connect_nodes(self.nodes[2], 3)
-        connect_nodes(self.nodes[2], 0)
+        self.connect_nodes(0, 3)
+        self.connect_nodes(1, 3)
+        self.connect_nodes(2, 3)
+        self.connect_nodes(2, 0)
 
     def stop_three(self):
         self.stop_node(0)
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
index a9dc3f790acc5..1ae69b7a19d13 100755
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@@ -10,7 +10,6 @@
     assert_equal,
     assert_fee_amount,
     assert_raises_rpc_error,
-    connect_nodes,
     Decimal,
     wait_until,
 )
@@ -25,9 +24,9 @@ def setup_network(self):
         self.start_node(0)
         self.start_node(1)
         self.start_node(2)
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 2)
-        connect_nodes(self.nodes[0], 2)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 2)
+        self.connect_nodes(0, 2)
         self.sync_all(self.nodes[0:3])
 
     def get_vsize(self, txn):
diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py
index e0d295b951c08..16435f03922ff 100755
--- a/test/functional/wallet_hd.py
+++ b/test/functional/wallet_hd.py
@@ -10,7 +10,6 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    connect_nodes,
     assert_raises_rpc_error
 )
 
@@ -52,8 +51,8 @@ def generate_shield_addr(self, seedid, count):
 
     def start_and_connect_node1(self):
         self.start_node(1)
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
         self.sync_all()
 
     def check_addressbook(self, old_book, new_book):
@@ -120,7 +119,7 @@ def run_test(self):
         self.stop_node(1)
         shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat"))
         self.start_node(1)
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
         self.check_addressbook(addrbook_old, self.nodes[1].getaddressesbylabel(""))
 
@@ -140,8 +139,8 @@ def run_test(self):
         z_add_2 = self.generate_shield_addr(masterkeyid, NUM_SHIELD_ADDS)
         assert_equal(z_add, z_add_2)
         # connect and sync
-        connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[1], 0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 0)
         self.sync_all()
         assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + NUM_SHIELD_ADDS + 1)
 
diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index d7c5a5b5d03dd..f595402b90fac 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -26,7 +26,6 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_raises_rpc_error,
-    connect_nodes,
     assert_equal,
     set_node_times
 )
@@ -126,7 +125,7 @@ def setup_network(self):
         self.add_nodes(self.num_nodes, extra_args=extra_args)
         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
diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py
index 02f854d1db052..5107f9fac1789 100755
--- a/test/functional/wallet_keypool_topup.py
+++ b/test/functional/wallet_keypool_topup.py
@@ -15,10 +15,7 @@
 import shutil
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import (
-    assert_equal,
-    connect_nodes,
-)
+from test_framework.util import assert_equal
 
 
 class KeypoolRestoreTest(PivxTestFramework):
@@ -38,7 +35,7 @@ def run_test(self):
 
         shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak")
         self.start_node(1, self.extra_args[1])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
 
         self.log.info("Generate keys for wallet")
 
@@ -64,7 +61,7 @@ def run_test(self):
         self.log.info("Verify keypool is restored and balance is correct")
 
         self.start_node(1, self.extra_args[1])
-        connect_nodes(self.nodes[0], 1)
+        self.connect_nodes(0, 1)
         self.sync_all()
 
         # wallet was not backed-up after emptying the key pool.
diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py
index a3b95f3be2e02..2333a43925874 100755
--- a/test/functional/wallet_listsinceblock.py
+++ b/test/functional/wallet_listsinceblock.py
@@ -9,7 +9,6 @@
     assert_equal,
     assert_array_result,
     assert_raises_rpc_error,
-    connect_nodes,
 )
 
 class ListSinceBlockTest (PivxTestFramework):
@@ -20,7 +19,7 @@ def set_test_params(self):
     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()
 
diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py
index 5edec1e065a6a..24aa9f4bcce62 100755
--- a/test/functional/wallet_reorgsrestore.py
+++ b/test/functional/wallet_reorgsrestore.py
@@ -18,11 +18,7 @@
 import shutil
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import (
-        assert_equal,
-        connect_nodes,
-        disconnect_nodes,
-)
+from test_framework.util import assert_equal
 
 
 class ReorgsRestoreTest(PivxTestFramework):
@@ -39,9 +35,9 @@ def run_test(self):
         self.sync_blocks()
 
         # Disconnect node1 from others to reorg its chain later
-        disconnect_nodes(self.nodes[0], 1)
-        disconnect_nodes(self.nodes[1], 2)
-        connect_nodes(self.nodes[0], 2)
+        self.disconnect_nodes(0, 1)
+        self.disconnect_nodes(1, 2)
+        self.connect_nodes(0, 2)
 
         # Send a tx to be unconfirmed later
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10"))
@@ -51,7 +47,7 @@ def run_test(self):
         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})
@@ -70,7 +66,7 @@ def run_test(self):
         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)
diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py
index ddf52f8eb70a4..1ec0c78ce2c1f 100755
--- a/test/functional/wallet_txn_clone.py
+++ b/test/functional/wallet_txn_clone.py
@@ -8,8 +8,7 @@
 
 from test_framework.messages import CTransaction, COIN
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, connect_nodes, disconnect_nodes
-
+from test_framework.util import assert_equal
 
 class TxnMallTest(PivxTestFramework):
     def set_test_params(self):
@@ -22,8 +21,8 @@ def add_options(self, parser):
     def setup_network(self):
         # Start with split network:
         super(TxnMallTest, self).setup_network()
-        disconnect_nodes(self.nodes[1], 2)
-        disconnect_nodes(self.nodes[2], 1)
+        self.disconnect_nodes(1, 2)
+        self.disconnect_nodes(2, 1)
 
     def run_test(self):
         # All nodes should start with 6,250 PIV:
@@ -108,10 +107,10 @@ def run_test(self):
         self.nodes[2].generate(1)
 
         # Reconnect the split network, and sync chain:
-        connect_nodes(self.nodes[1], 2)
-        connect_nodes(self.nodes[0], 2)
-        connect_nodes(self.nodes[2], 0)
-        connect_nodes(self.nodes[2], 1)
+        self.connect_nodes(1, 2)
+        self.connect_nodes(0, 2)
+        self.connect_nodes(2, 0)
+        self.connect_nodes(2, 1)
 
         self.nodes[2].sendrawtransaction(node0_tx2["hex"])
         self.nodes[2].sendrawtransaction(tx2["hex"])
diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py
index fe9cd4075f6e8..478e5fea36d20 100755
--- a/test/functional/wallet_txn_doublespend.py
+++ b/test/functional/wallet_txn_doublespend.py
@@ -7,7 +7,7 @@
 from decimal import Decimal
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, connect_nodes, disconnect_nodes, find_output
+from test_framework.util import assert_equal, find_output
 
 class TxnMallTest(PivxTestFramework):
     def set_test_params(self):
@@ -20,8 +20,8 @@ def add_options(self, parser):
     def setup_network(self):
         # Start with split network:
         super().setup_network()
-        disconnect_nodes(self.nodes[1], 2)
-        disconnect_nodes(self.nodes[2], 1)
+        self.disconnect_nodes(1, 2)
+        self.disconnect_nodes(2, 1)
 
     def run_test(self):
         # All nodes should start with 6,250 PIV:
@@ -102,10 +102,10 @@ def run_test(self):
         self.nodes[2].generate(1)
 
         # Reconnect the split network, and sync chain:
-        connect_nodes(self.nodes[1], 2)
-        connect_nodes(self.nodes[0], 2)
-        connect_nodes(self.nodes[2], 0)
-        connect_nodes(self.nodes[2], 1)
+        self.connect_nodes(1, 2)
+        self.connect_nodes(0, 2)
+        self.connect_nodes(2, 0)
+        self.connect_nodes(2, 1)
         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)

From 17c065da749689f9b28f9b05e39ed677765304d5 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 27 Aug 2021 10:24:37 +0200
Subject: [PATCH 03/10] test: Avoid race after connect_nodes

---
 .../test_framework/test_framework.py          | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index a32c8de6d2708..43a4146c0e61a 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -342,17 +342,19 @@ def wait_for_node_exit(self, i, timeout):
         self.nodes[i].process.wait(timeout)
 
     def connect_nodes(self, 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(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
-            wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
-        connect_nodes_helper(self.nodes[a], b)
+        from_connection = self.nodes[a]
+        to_connection = self.nodes[b]
+        ip_port = "127.0.0.1:" + str(p2p_port(b))
+        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(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
+        wait_until(lambda: all(peer['version'] != 0 for peer in to_connection.getpeerinfo()))
+        wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
+        wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()))
 
     def disconnect_nodes(self, a, b):
         def disconnect_nodes_helper(from_connection, node_num):

From 77697b8969c776553e5e4748e8e19b2eb240c06c Mon Sep 17 00:00:00 2001
From: Alessandro Rezzi <alessandrorezzi2000@gmail.com>
Date: Mon, 11 Nov 2024 10:02:30 +0100
Subject: [PATCH 04/10] test: Do not connect the nodes in parallel

---
 .../test_framework/test_framework.py           | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 43a4146c0e61a..4a88fce672d75 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Base class for RPC testing."""
 
-from concurrent.futures import ThreadPoolExecutor
 from enum import Enum
 from io import BytesIO
 import logging
@@ -373,22 +372,11 @@ def disconnect_nodes_helper(from_connection, node_num):
         disconnect_nodes_helper(self.nodes[a], b)
 
     def connect_nodes_clique(self, nodes):
-        # max_workers should be the maximum number of nodes that we have in the same functional test,
-        # 15 seems to be a good upper bound
-        parallel_exec = ThreadPoolExecutor(max_workers=15)
         l = len(nodes)
-
-        def connect_nodes_clique_internal(a):
-            for b in range(0, l):
-                self.connect_nodes(a, b)
-        jobs = []
         for a in range(l):
-            jobs.append(parallel_exec.submit(connect_nodes_clique_internal, a))
-
-        for job in jobs:
-            job.result()
-        jobs.clear()
-        parallel_exec.shutdown()
+            for b in range(a, l):
+                self.connect_nodes(a, b)
+                self.connect_nodes(b, a)
 
     def split_network(self):
         """

From 72709b9b4230e4af8fb6b1b51ec0bbcf9c08b6e6 Mon Sep 17 00:00:00 2001
From: Alessandro Rezzi <alessandrorezzi2000@gmail.com>
Date: Mon, 11 Nov 2024 11:03:09 +0100
Subject: [PATCH 05/10] test: Avoid connecting a peer to himself

---
 test/functional/test_framework/test_framework.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 4a88fce672d75..aa68289dd14e9 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -374,7 +374,7 @@ def disconnect_nodes_helper(from_connection, node_num):
     def connect_nodes_clique(self, nodes):
         l = len(nodes)
         for a in range(l):
-            for b in range(a, l):
+            for b in range(a + 1, l):
                 self.connect_nodes(a, b)
                 self.connect_nodes(b, a)
 

From b3a3d780fa5062e5620a656ec28a7e959c982c32 Mon Sep 17 00:00:00 2001
From: laanwj <126646+laanwj@users.noreply.github.com>
Date: Wed, 22 Jun 2022 12:19:57 +0200
Subject: [PATCH 06/10] Merge bitcoin/bitcoin#25443: test: Fail if
 connect_nodes fails

faee330c7bde4d52dc18cf55166b5975b26bdecd test: Fail if connect_nodes fails (MacroFake)

Pull request description:

  Currently, `connect_nodes` will return silently when the connection is disconnected while connecting. This is confusing, so fix it.

  Can be tested by reverting the signet test change and observing the failure when running the test.

ACKs for top commit:
  laanwj:
    Tested ACK faee330c7bde4d52dc18cf55166b5975b26bdecd

Tree-SHA512: 641ca8adcb9f5ff33239b143573bddc0dfde41dbd103751ee870f1572ca2469f6a0d4bab6693102454cd3e270ef8251d87fbfac48f6d8adac70d2d6bbffaae56
---
 test/functional/test_framework/test_framework.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index aa68289dd14e9..26b0e8f8969c1 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -343,6 +343,8 @@ def wait_for_node_exit(self, i, timeout):
     def connect_nodes(self, a, b):
         from_connection = self.nodes[a]
         to_connection = self.nodes[b]
+        from_num_peers = 1 + len(from_connection.getpeerinfo())
+        to_num_peers = 1 + len(to_connection.getpeerinfo())
         ip_port = "127.0.0.1:" + str(p2p_port(b))
         from_connection.addnode(ip_port, "onetry")
         # poll until version handshake complete to avoid race conditions
@@ -350,10 +352,10 @@ def connect_nodes(self, a, b):
         # See comments in net_processing:
         # * Must have a version message before anything else
         # * Must have a verack message before anything else
-        wait_until(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo()))
-        wait_until(lambda: all(peer['version'] != 0 for peer in to_connection.getpeerinfo()))
-        wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
-        wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()))
+        wait_until(lambda: sum(peer['version'] != 0 for peer in from_connection.getpeerinfo()) == from_num_peers)
+        wait_until(lambda: sum(peer['version'] != 0 for peer in to_connection.getpeerinfo()) == to_num_peers)
+        wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()) == from_num_peers)
+        wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()) == to_num_peers)
 
     def disconnect_nodes(self, a, b):
         def disconnect_nodes_helper(from_connection, node_num):

From 9beef8d47672b09d4f359474a8e26d5f3888671f Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 12 Jan 2023 12:51:29 +0100
Subject: [PATCH 07/10] Merge bitcoin/bitcoin#26854: test: Fix intermittent
 timeout in p2p_permissions.py

fa1bf4e7052e617dd0e5c8c54969d84314af9577 test: Fix intermittent timeout in p2p_permissions.py (MarcoFalke)

Pull request description:

  The sync is based on `bytesrecv_per_msg["verack"]`. However, the bytes are counted before processing the message, so they are not sufficient to ensure the connection is fully up.

ACKs for top commit:
  mzumsande:
    ACK fa1bf4e7052e617dd0e5c8c54969d84314af9577
  aureleoules:
    ACK fa1bf4e7052e617dd0e5c8c54969d84314af9577

Tree-SHA512: eb1ed537032c76a449b1ed5e42ff062e9b8b3c7e11fde2a5b8183ae0d6fbe31dba39e2c758836160cd8157d9ac5cc1f5d1916415861b8d711b7370c88f5e9790
---
 test/functional/test_framework/test_framework.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 26b0e8f8969c1..3e84de308abed 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -356,6 +356,10 @@ def connect_nodes(self, a, b):
         wait_until(lambda: sum(peer['version'] != 0 for peer in to_connection.getpeerinfo()) == to_num_peers)
         wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()) == from_num_peers)
         wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()) == to_num_peers)
+        # The message bytes are counted before processing the message, so make
+        # sure it was fully processed by waiting for a ping.
+        wait_until(lambda: sum(peer["bytesrecv_per_msg"].pop("pong", 0) >= 32 for peer in from_connection.getpeerinfo()) == from_num_peers)
+        wait_until(lambda: sum(peer["bytesrecv_per_msg"].pop("pong", 0) >= 32 for peer in to_connection.getpeerinfo()) == to_num_peers)
 
     def disconnect_nodes(self, a, b):
         def disconnect_nodes_helper(from_connection, node_num):

From e7c21c7e583280fae31dd562db6351030b4fd35b Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Thu, 23 May 2024 10:00:00 -0400
Subject: [PATCH 08/10] Merge bitcoin/bitcoin#30118: test: improve robustness
 of connect_nodes()

6629d1d0f8285d1bf2d87341a856abe903f26c13 test: improve robustness of connect_nodes() (furszy)

Pull request description:

  Decoupled from #27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200.

  The `connect_nodes` function in the test framework relies on a stable number of peer
  connections to verify that the new connection between the nodes is successfully established.
  This approach is fragile, as any of the peers involved in the process can drop, lose, or
  create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
  even when the peers in question were connected successfully.

  This commit improves the situation by using the nodes' subversion and the connection
  direction (inbound/outbound) to identify the exact peer connection and perform the
  checks exclusively on it.

ACKs for top commit:
  stratospher:
    reACK 6629d1d.
  achow101:
    ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
  maflcko:
    utACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
  AngusP:
    re-ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13

Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
---
 .../test_framework/test_framework.py          | 28 +++++++++++++------
 test/functional/test_framework/test_node.py   |  2 +-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 3e84de308abed..8e693b2097da2 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -343,23 +343,35 @@ def wait_for_node_exit(self, i, timeout):
     def connect_nodes(self, a, b):
         from_connection = self.nodes[a]
         to_connection = self.nodes[b]
-        from_num_peers = 1 + len(from_connection.getpeerinfo())
-        to_num_peers = 1 + len(to_connection.getpeerinfo())
         ip_port = "127.0.0.1:" + str(p2p_port(b))
         from_connection.addnode(ip_port, "onetry")
+
+        # Use subversion as peer id. Test nodes have their node number appended to the user agent string
+        from_connection_subver = from_connection.getnetworkinfo()['subversion']
+        to_connection_subver = to_connection.getnetworkinfo()['subversion']
+
+        def find_conn(node, peer_subversion, inbound):
+            return next(filter(lambda peer: peer['subver'] == peer_subversion and peer['inbound'] == inbound, node.getpeerinfo()), None)
+
         # 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(lambda: sum(peer['version'] != 0 for peer in from_connection.getpeerinfo()) == from_num_peers)
-        wait_until(lambda: sum(peer['version'] != 0 for peer in to_connection.getpeerinfo()) == to_num_peers)
-        wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()) == from_num_peers)
-        wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()) == to_num_peers)
+        wait_until(lambda: find_conn(from_connection, to_connection_subver, inbound=False) is not None)
+        wait_until(lambda: find_conn(to_connection, from_connection_subver, inbound=True) is not None)
+
+        def check_bytesrecv(peer, msg_type, min_bytes_recv):
+            assert peer is not None, "Error: peer disconnected"
+            return peer['bytesrecv_per_msg'].pop(msg_type, 0) >= min_bytes_recv
+
+        wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'verack', 21))
+        wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'verack', 21))
+
         # The message bytes are counted before processing the message, so make
         # sure it was fully processed by waiting for a ping.
-        wait_until(lambda: sum(peer["bytesrecv_per_msg"].pop("pong", 0) >= 32 for peer in from_connection.getpeerinfo()) == from_num_peers)
-        wait_until(lambda: sum(peer["bytesrecv_per_msg"].pop("pong", 0) >= 32 for peer in to_connection.getpeerinfo()) == to_num_peers)
+        wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'pong', 29))
+        wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'pong', 29))
 
     def disconnect_nodes(self, a, b):
         def disconnect_nodes_helper(from_connection, node_num):
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 644de9c740652..93cbaf1736714 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -79,7 +79,7 @@ def __init__(self, i, dirname, rpchost, timewait, binary, stderr, mocktime, cove
             "-debugexclude=libevent",
             "-debugexclude=leveldb",
             "-mocktime=" + str(mocktime),
-            "-uacomment=testnode%d" % i
+            "-uacomment=testnode%d" % i, # required for subversion uniqueness across peers
         ]
 
         self.cli = TestNodeCLI(os.getenv("BITCOINCLI", "pivx-cli"), self.datadir)

From 53742291e707960329cb3aad547b774e29c2f10a Mon Sep 17 00:00:00 2001
From: merge-script <fanquake@gmail.com>
Date: Tue, 11 Jun 2024 14:11:34 +0100
Subject: [PATCH 09/10] Merge bitcoin/bitcoin#30252: test: Remove redundant
 verack check

0000276b31cea5e443a59d94a98c569293ada951 test: Remove redundant verack check (MarcoFalke)

Pull request description:

  Currently the sync in `connect_nodes` mentions the `version` and `verack` message types, but only checks the `verack`. Neither check is required, as the `pong` check implies both. In case of failure, the debug log will have to be consulted anyway, so the redundant check doesn't add value.

  Also clarify in the comments that the goal is to check the flag `fSuccessfullyConnected` indirectly.

ACKs for top commit:
  furszy:
    utACK 0000276b31ce
  brunoerg:
    ACK 0000276b31cea5e443a59d94a98c569293ada951
  tdb3:
    ACK 0000276b31cea5e443a59d94a98c569293ada951

Tree-SHA512: f9ddcb1436d2f70da462a8dd470ecfc90a534dd6507c23877ef7626e7c02326c077001a42ad0171a87fba5c5275d1970d8c5e5d82c56c8412de944856fdfd6db
---
 test/functional/test_framework/test_framework.py | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 8e693b2097da2..dc812f174e1c5 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -353,11 +353,6 @@ def connect_nodes(self, a, b):
         def find_conn(node, peer_subversion, inbound):
             return next(filter(lambda peer: peer['subver'] == peer_subversion and peer['inbound'] == inbound, node.getpeerinfo()), None)
 
-        # 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(lambda: find_conn(from_connection, to_connection_subver, inbound=False) is not None)
         wait_until(lambda: find_conn(to_connection, from_connection_subver, inbound=True) is not None)
 
@@ -365,11 +360,12 @@ def check_bytesrecv(peer, msg_type, min_bytes_recv):
             assert peer is not None, "Error: peer disconnected"
             return peer['bytesrecv_per_msg'].pop(msg_type, 0) >= min_bytes_recv
 
-        wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'verack', 21))
-        wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'verack', 21))
-
-        # The message bytes are counted before processing the message, so make
-        # sure it was fully processed by waiting for a ping.
+        # Poll until version handshake (fSuccessfullyConnected) is complete to
+        # avoid race conditions, because some message types are blocked from
+        # being sent or received before fSuccessfullyConnected.
+        #
+        # As the flag fSuccessfullyConnected is not exposed, check it by
+        # waiting for a pong, which can only happen after the flag was set.
         wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'pong', 29))
         wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'pong', 29))
 

From 5f8c06ea1904856702bcc99a1a69cd57addc7214 Mon Sep 17 00:00:00 2001
From: Alessandro Rezzi <alessandrorezzi2000@gmail.com>
Date: Mon, 11 Nov 2024 13:02:22 +0100
Subject: [PATCH 10/10] test: Add possibility to skip awaiting for the
 connection

---
 test/functional/p2p_time_offset.py               | 13 +++++++------
 test/functional/test_framework/test_framework.py |  5 ++++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/test/functional/p2p_time_offset.py b/test/functional/p2p_time_offset.py
index 2ace694ec7e48..98ff17d91d97d 100755
--- a/test/functional/p2p_time_offset.py
+++ b/test/functional/p2p_time_offset.py
@@ -9,6 +9,7 @@
 from test_framework.util import (
     assert_equal,
     set_node_times,
+    wait_until,
 )
 
 
@@ -22,9 +23,9 @@ def setup_network(self):
         # don't connect nodes yet
         self.setup_nodes()
 
-    def connect_nodes_bi(self, a, b):
-        self.connect_nodes(a, b)
-        self.connect_nodes(b, a)
+    def connect_nodes_bi(self, a, b, wait_for_connect = True):
+        self.connect_nodes(a, b, wait_for_connect)
+        self.connect_nodes(b, a, wait_for_connect)
 
     def check_connected_nodes(self):
         ni = [node.getnetworkinfo() for node in self.connected_nodes]
@@ -96,9 +97,9 @@ def run_test(self):
 
         # try to connect node 5 and check that it can't
         self.log.info("Trying to connect with node-5 (+30 s)...")
-        self.connect_nodes_bi(0, 5)
-        ni = self.nodes[0].getnetworkinfo()
-        assert_equal(ni['connections'], 10)
+        # Don't wait for a connection that will never be established.
+        self.connect_nodes_bi(0, 5, False)
+        wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 10)
         assert_equal(ni['timeoffset'], 15)
         self.log.info("Not connected.")
         self.log.info("Node-0 nTimeOffset: +%d seconds" % ni['timeoffset'])
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index dc812f174e1c5..e737d80c982b3 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -340,12 +340,15 @@ def restart_node(self, i, extra_args=None):
     def wait_for_node_exit(self, i, timeout):
         self.nodes[i].process.wait(timeout)
 
-    def connect_nodes(self, a, b):
+    def connect_nodes(self, a, b, wait_for_connect = True):
         from_connection = self.nodes[a]
         to_connection = self.nodes[b]
         ip_port = "127.0.0.1:" + str(p2p_port(b))
         from_connection.addnode(ip_port, "onetry")
 
+        if not wait_for_connect:
+            return
+
         # Use subversion as peer id. Test nodes have their node number appended to the user agent string
         from_connection_subver = from_connection.getnetworkinfo()['subversion']
         to_connection_subver = to_connection.getnetworkinfo()['subversion']