Merge bitcoin/bitcoin#28409: test: Combine sync_send_with_ping and sync_with_ping

fae0b21 test: Combine sync_send_with_ping and sync_with_ping (MarcoFalke)

Pull request description:

  This reduces bloat, complexity, and makes tests less fragile to intermittent failures, see bitcoin/bitcoin#27675 (comment).

  This should not cause any noticeable slowdown, or may even be faster, because active polling will be done at most once.

ACKs for top commit:
  glozow:
    Concept ACK fae0b21
  theStack:
    ACK fae0b21 🏓

Tree-SHA512: 6c543241a7b85458dc7ff6a6203316b80a6227d83d38427e74f53f4c666a882647a8a221e5259071ee7bb5dfd63476fb03c9b558a1ea546734b14728c3c619ba

Author: fanquake

test/functional/p2p_addr_relay.py

@@ -133,7 +133,7 @@ def send_addr_msg(self, source, msg, receivers):
         self.mocktime += 10 * 60
         self.nodes[0].setmocktime(self.mocktime)
         for peer in receivers:
-            peer.sync_send_with_ping()
+            peer.sync_with_ping()
 
     def oversized_addr_test(self):
         self.log.info('Send an addr message that is too large')

test/functional/p2p_addrfetch.py

@@ -48,7 +48,7 @@ def run_test(self):
         self.assert_getpeerinfo(peer_ids=[peer_id])
 
         self.log.info("Check that we send getaddr but don't try to sync headers with the addr-fetch peer")
-        peer.sync_send_with_ping()
+        peer.sync_with_ping()
         with p2p_lock:
             assert peer.message_count['getaddr'] == 1
             assert peer.message_count['getheaders'] == 0

test/functional/p2p_blocksonly.py

@@ -101,7 +101,7 @@ def blocks_relay_conn_tests(self):
         # Bump time forward to ensure m_next_inv_send_time timer pops
         self.nodes[0].setmocktime(int(time.time()) + 60)
 
-        conn.sync_send_with_ping()
+        conn.sync_with_ping()
         assert int(txid, 16) not in conn.get_invs()
 
     def check_p2p_inv_violation(self, peer):

test/functional/p2p_compactblocks_blocksonly.py

@@ -94,11 +94,11 @@ def run_test(self):
         block1 = self.build_block_on_tip()
 
         p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)]))
-        p2p_conn_blocksonly.sync_send_with_ping()
+        p2p_conn_blocksonly.sync_with_ping()
         assert_equal(p2p_conn_blocksonly.last_message['getdata'].inv, [CInv(MSG_BLOCK | MSG_WITNESS_FLAG, block1.sha256)])
 
         p2p_conn_high_bw.send_message(msg_headers(headers=[CBlockHeader(block1)]))
-        p2p_conn_high_bw.sync_send_with_ping()
+        p2p_conn_high_bw.sync_with_ping()
         assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)])
 
         self.log.info("Test that getdata(CMPCT) is still sent on BIP152 low bandwidth connections"

test/functional/p2p_filter.py

@@ -177,7 +177,7 @@ def test_filter(self, filter_peer):
         filter_peer.merkleblock_received = False
         filter_peer.tx_received = False
         self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=getnewdestination()[1], amount=7 * COIN)
-        filter_peer.sync_send_with_ping()
+        filter_peer.sync_with_ping()
         assert not filter_peer.merkleblock_received
         assert not filter_peer.tx_received
 

test/functional/p2p_ibd_stalling.py

@@ -151,7 +151,7 @@ def total_bytes_recv_for_blocks(self):
     def all_sync_send_with_ping(self, peers):
         for p in peers:
             if p.is_connected:
-                p.sync_send_with_ping()
+                p.sync_with_ping()
 
     def is_block_requested(self, peers, hash):
         for p in peers:

test/functional/p2p_ibd_txrelay.py

@@ -53,7 +53,7 @@ def run_test(self):
         peer_inver.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=txid)]))
         # The node should not send a getdata, but if it did, it would first delay 2 seconds
         self.nodes[0].setmocktime(int(time.time() + NONPREF_PEER_TX_DELAY))
-        peer_inver.sync_send_with_ping()
+        peer_inver.sync_with_ping()
         with p2p_lock:
             assert txid not in peer_inver.getdata_requests
         self.nodes[0].disconnect_p2ps()

test/functional/test_framework/p2p.py

@@ -558,16 +558,12 @@ def send_and_ping(self, message, timeout=60):
         self.send_message(message)
         self.sync_with_ping(timeout=timeout)
 
-    def sync_send_with_ping(self, timeout=60):
-        """Ensure SendMessages is called on this connection"""
-        # Calling sync_with_ping twice requires that the node calls
+    def sync_with_ping(self, timeout=60):
+        """Ensure ProcessMessages and SendMessages is called on this connection"""
+        # Sending two pings back-to-back, requires that the node calls
         # `ProcessMessage` twice, and thus ensures `SendMessages` must have
         # been called at least once
-        self.sync_with_ping()
-        self.sync_with_ping()
-
-    def sync_with_ping(self, timeout=60):
-        """Ensure ProcessMessages is called on this connection"""
+        self.send_message(msg_ping(nonce=0))
         self.send_message(msg_ping(nonce=self.ping_counter))
 
         def test_function():