test: Use connect_nodes when connecting nodes in the test_framework

backports bitcoin/bitcoin@faaee1e

test/functional/p2p_disconnect_ban.py

@@ -8,8 +8,9 @@
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    assert_raises_rpc_error,
+    connect_nodes,
     connect_nodes_bi,
+    assert_raises_rpc_error,
     wait_until,
 )
 
@@ -18,6 +19,10 @@ def set_test_params(self):
         self.num_nodes = 2
 
     def run_test(self):
+        self.log.info("Connect nodes both way")
+        connect_nodes(self.nodes[0], 1)
+        connect_nodes(self.nodes[1], 0)
+
         self.log.info("Test setban and listbanned RPCs")
 
         self.log.info("setban: successfully ban single IP address")
@@ -74,7 +79,9 @@ def run_test(self):
 
         # Clear ban lists
         self.nodes[1].clearbanned()
-        connect_nodes_bi(self.nodes, 0, 1)
+        self.log.info("Connect nodes both way")
+        connect_nodes(self.nodes[0], 1)
+        connect_nodes(self.nodes[1], 0)
 
         self.log.info("Test disconnectnode RPCs")
 

test/functional/rpc_net.py

@@ -12,7 +12,7 @@
     assert_equal,
     assert_greater_than_or_equal,
     assert_raises_rpc_error,
-    connect_nodes_bi,
+    connect_nodes,
     disconnect_nodes,
     p2p_port,
     wait_until,
@@ -24,6 +24,10 @@ def set_test_params(self):
         self.num_nodes = 2
 
     def run_test(self):
+        self.log.info("Connect nodes both way")
+        connect_nodes(self.nodes[0], 1)
+        connect_nodes(self.nodes[1], 0)
+
         self._test_connection_count()
         self._test_getnettotals()
         self._test_getnetworkinginfo()
@@ -68,7 +72,9 @@ def _test_getnetworkinginfo(self):
         # Wait a bit for all sockets to close
         wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3)
 
-        connect_nodes_bi(self.nodes, 0, 1)
+        self.log.info("Connect nodes both way")
+        connect_nodes(self.nodes[0], 1)
+        connect_nodes(self.nodes[1], 0)
         assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2)
 
     def _test_getaddednodeinfo(self):

test/functional/test_framework/test_framework.py

@@ -45,7 +45,7 @@
     assert_equal,
     assert_greater_than,
     check_json_precision,
-    connect_nodes_bi,
+    connect_nodes,
     connect_nodes_clique,
     disconnect_nodes,
     DEFAULT_FEE,
@@ -227,8 +227,18 @@ def setup_network(self):
         # Connect the nodes as a "chain".  This allows us
         # to split the network between nodes 1 and 2 to get
         # two halves that can work on competing chains.
+        #
+        # Topology looks like this:
+        # node0 <-- node1 <-- node2 <-- node3
+        #
+        # If all nodes are in IBD (clean chain from genesis), node0 is assumed to be the source of blocks (miner). To
+        # ensure block propagation, all nodes will establish outgoing connections toward node0.
+        # See fPreferredDownload in net_processing.
+        #
+        # 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_bi(self.nodes, i, i + 1)
+            connect_nodes(self.nodes[i + 1], i)
         self.sync_all()
 
     def setup_nodes(self):

test/functional/wallet_abandonconflict.py

@@ -5,8 +5,14 @@
 
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import *
-import urllib.parse
+from test_framework.util import (
+    assert_equal,
+    connect_nodes,
+    Decimal,
+    disconnect_nodes,
+    sync_blocks,
+    sync_mempools
+)
 
 class AbandonConflictTest(PivxTestFramework):
     def set_test_params(self):
@@ -31,8 +37,8 @@ def run_test(self):
         assert(balance - newbalance < Decimal("0.001")) #no more than fees lost
         balance = newbalance
 
-        url = urllib.parse.urlparse(self.nodes[1].url)
-        self.nodes[0].disconnectnode(url.hostname+":"+str(p2p_port(1)))
+        # Disconnect nodes so node0's transactions don't get into node1's mempool
+        disconnect_nodes(self.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)

test/functional/wallet_listsinceblock.py

@@ -5,14 +5,22 @@
 """Test the listsincelast RPC."""
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import assert_equal, assert_array_result, assert_raises_rpc_error
+from test_framework.util import (
+    assert_equal,
+    assert_array_result,
+    assert_raises_rpc_error,
+    connect_nodes,
+)
 
 class ListSinceBlockTest (PivxTestFramework):
     def set_test_params(self):
         self.num_nodes = 4
         self.setup_clean_chain = True
 
     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.nodes[2].generate(101)
         self.sync_all()