Merge bitcoin#25443: test: Fail if connect_nodes fails

faee330 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 faee330

Tree-SHA512: 641ca8adcb9f5ff33239b143573bddc0dfde41dbd103751ee870f1572ca2469f6a0d4bab6693102454cd3e270ef8251d87fbfac48f6d8adac70d2d6bbffaae56

Author: laanwj

test/functional/feature_signet.py

@@ -39,6 +39,14 @@ def set_test_params(self):
             shared_args3, shared_args3,
         ]
 
+    def setup_network(self):
+        self.setup_nodes()
+
+        # Setup the three signets, which are incompatible with each other
+        self.connect_nodes(0, 1)
+        self.connect_nodes(2, 3)
+        self.connect_nodes(4, 5)
+
     def run_test(self):
         self.log.info("basic tests using OP_TRUE challenge")
 

test/functional/test_framework/test_framework.py

@@ -581,17 +581,19 @@ 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
         # 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['version'] != 0 for peer in to_connection.getpeerinfo()))
-        wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))
-        wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo()))
+        self.wait_until(lambda: sum(peer['version'] != 0 for peer in from_connection.getpeerinfo()) == from_num_peers)
+        self.wait_until(lambda: sum(peer['version'] != 0 for peer in to_connection.getpeerinfo()) == to_num_peers)
+        self.wait_until(lambda: sum(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()) == from_num_peers)
+        self.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):
@@ -620,7 +622,7 @@ def get_peer_ids():
                         raise
 
             # wait to disconnect
-            wait_until_helper(lambda: not get_peer_ids(), timeout=5)
+            self.wait_until(lambda: not get_peer_ids(), timeout=5)
 
         disconnect_nodes_helper(self.nodes[a], b)