test: Prefer send_and_ping over send_message+sync_with_ping

Also, add an explanation for the preference in the docs.

Author: MarcoFalke

test/functional/p2p_compactblocks_blocksonly.py

@@ -93,19 +93,16 @@ def run_test(self):
 
         block1 = self.build_block_on_tip()
 
-        p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)]))
-        p2p_conn_blocksonly.sync_with_ping()
+        p2p_conn_blocksonly.send_and_ping(msg_headers(headers=[CBlockHeader(block1)]))
         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_with_ping()
+        p2p_conn_high_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)]))
         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"
                       " when no -blocksonly nodes are involved")
 
         p2p_conn_low_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)]))
-        p2p_conn_low_bw.sync_with_ping()
         assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)])
 
         self.log.info("Test that -blocksonly nodes still serve compact blocks")

test/functional/p2p_tx_download.py

@@ -229,8 +229,7 @@ def test_preferred_inv(self, connection_type: ConnectionType):
         else:
             peer = self.nodes[0].add_p2p_connection(TestP2PConn())
 
-        peer.send_message(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
-        peer.sync_with_ping()
+        peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
         if connection_type != ConnectionType.INBOUND:
             peer.wait_until(lambda: peer.tx_getdata_count >= 1, timeout=1)
         else:
@@ -250,17 +249,15 @@ def test_preferred_tiebreaker_inv(self):
         # of which is preferred and one which is not
         unresponsive_peer = self.nodes[0].add_outbound_p2p_connection(
            TestP2PConn(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay")
-        unresponsive_peer.send_message(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
-        unresponsive_peer.sync_with_ping()
+        unresponsive_peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
         unresponsive_peer.wait_until(lambda: unresponsive_peer.tx_getdata_count >= 1, timeout=1)
 
         # A bunch of incoming (non-preferred) connections that advertise the same tx
         non_pref_peers = []
         NUM_INBOUND = 10
         for _ in range(NUM_INBOUND):
             non_pref_peers.append(self.nodes[0].add_p2p_connection(TestP2PConn()))
-            non_pref_peers[-1].send_message(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
-            non_pref_peers[-1].sync_with_ping()
+            non_pref_peers[-1].send_and_ping(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
 
         # Check that no request made due to in-flight
         self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
@@ -272,8 +269,7 @@ def test_preferred_tiebreaker_inv(self):
         # upon advertisement
         pref_peer = self.nodes[0].add_outbound_p2p_connection(
            TestP2PConn(), wait_for_verack=True, p2p_idx=1, connection_type="outbound-full-relay")
-        pref_peer.send_message(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
-        pref_peer.sync_with_ping()
+        pref_peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=0xff00ff00)]))
 
         assert_equal(len(self.nodes[0].getpeerinfo()), NUM_INBOUND + 2)
 
@@ -302,8 +298,7 @@ def test_txid_inv_delay(self, glob_wtxid=False):
             # Add a second wtxid-relay connection otherwise TXID_RELAY_DELAY is waived in
             # lack of wtxid-relay peers
             self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=True))
-        peer.send_message(msg_inv([CInv(t=MSG_TX, h=0xff11ff11)]))
-        peer.sync_with_ping()
+        peer.send_and_ping(msg_inv([CInv(t=MSG_TX, h=0xff11ff11)]))
         with p2p_lock:
             assert_equal(peer.tx_getdata_count, 0 if glob_wtxid else 1)
         self.nodes[0].setmocktime(mock_time + TXID_RELAY_DELAY)

test/functional/test_framework/p2p.py

@@ -382,7 +382,13 @@ def send_message(self, message, is_decoy=False):
         """Send a P2P message over the socket.
 
         This method takes a P2P payload, builds the P2P header and adds
-        the message to the send buffer to be sent over the socket."""
+        the message to the send buffer to be sent over the socket.
+
+        When a message does not lead to a disconnect, send_and_ping is usually
+        preferred to send a message. This can help to reduce intermittent test
+        failures due to a missing sync. Also, it includes a call to
+        sync_with_ping, allowing for concise test code.
+        """
         with self._send_lock:
             tmsg = self.build_message(message, is_decoy)
             self._log_message("send", message)