From b198b9c2ce28463f7c6a2b4cf08d64138c8509ee Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 30 Jan 2024 02:59:01 +0100 Subject: [PATCH 1/3] test: p2p: introduce helper for sending prepared VERSION message This deduplicates code for sending out the VERSION message (if available and not sent yet), currently used at three different places: 1) in the `connection_made` asyncio callback (for v1 connections that are not v2 reconnects) 2) at the end of `v2_handshake`, if the v2 handshake succeeded 3) in the `on_version` callback, if a reconnection with v1 happens --- test/functional/test_framework/p2p.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index c113a4c8d81..783baf480da 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -226,9 +226,8 @@ class P2PConnection(asyncio.Protocol): self.send_raw_message(send_handshake_bytes) # if v2 connection, send `on_connection_send_msg` after initial v2 handshake. # if reconnection situation, send `on_connection_send_msg` after version message is received in `on_version()`. - if self.on_connection_send_msg and not self.supports_v2_p2p and not self.reconnect: - self.send_message(self.on_connection_send_msg) - self.on_connection_send_msg = None # Never used again + if not self.supports_v2_p2p and not self.reconnect: + self.send_version() self.on_open() def connection_lost(self, exc): @@ -284,9 +283,8 @@ class P2PConnection(asyncio.Protocol): if not is_mac_auth: raise ValueError("invalid v2 mac tag in handshake authentication") self.recvbuf = self.recvbuf[length:] - if self.v2_state.tried_v2_handshake and self.on_connection_send_msg: - self.send_message(self.on_connection_send_msg) - self.on_connection_send_msg = None + if self.v2_state.tried_v2_handshake: + self.send_version() # Socket read methods @@ -559,9 +557,7 @@ class P2PInterface(P2PConnection): assert message.nVersion >= MIN_P2P_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_P2P_VERSION_SUPPORTED) # reconnection using v1 P2P has happened since version message can be processed, previously unsent version message is sent using v1 P2P here if self.reconnect: - if self.on_connection_send_msg: - self.send_message(self.on_connection_send_msg) - self.on_connection_send_msg = None + self.send_version() self.reconnect = False if message.nVersion >= 70016 and self.wtxidrelay: self.send_message(msg_wtxidrelay()) @@ -676,6 +672,11 @@ class P2PInterface(P2PConnection): # Message sending helper functions + def send_version(self): + if self.on_connection_send_msg: + self.send_message(self.on_connection_send_msg) + self.on_connection_send_msg = None # Never used again + def send_and_ping(self, message, timeout=60): self.send_message(message) self.sync_with_ping(timeout=timeout) From 7ddfc28309e6b70c273112a93f01e518409ceaa0 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 1 Feb 2024 13:23:50 +0100 Subject: [PATCH 2/3] test: p2p: process post-v2-handshake data immediately In the course of executing the asyncio data reception callback during a v2 handshake, it's possible that the receive buffer already contains data for after the handshake (usually a VERSION message for inbound connections). If we don't process that data immediately, we would do so after the next message is received, but with the adapted protocol flow introduced in the next commit, there is no next message, as the TestNode wouldn't continue until we send back our own version in `on_version`. Fix this by calling `self._on_data` immediately if there's data left in the receive buffer after a completed v2 handshake. --- test/functional/test_framework/p2p.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 783baf480da..d7668009028 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -285,6 +285,9 @@ class P2PConnection(asyncio.Protocol): self.recvbuf = self.recvbuf[length:] if self.v2_state.tried_v2_handshake: self.send_version() + # process post-v2-handshake data immediately, if available + if len(self.recvbuf) > 0: + self._on_data() # Socket read methods From c340503b67585b631909584f1acaa327f5af25d3 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 30 Jan 2024 04:34:43 +0100 Subject: [PATCH 3/3] test: p2p: adhere to typical VERSION message protocol flow The test framework's p2p implementation currently sends out it's VERSION message immediately after an inbound connection (i.e. TestNode outbound connection) is made. This doesn't follow the usual protocol flow where the initiator sends a version first, and the responders processes that and only then responds with its own version message. Change that accordingly by only sending immediate VERSION message for outbound connections (or after v2 handshake for v2 connections, respectively), and sending out VERSION messages as response for incoming VERSION messages (i.e. in the function `on_version`) for inbound connections. Note that some of the overruled `on_version` methods in functional tests needed to be changed to send the version explicitly. --- test/functional/p2p_add_connections.py | 2 +- test/functional/p2p_addr_relay.py | 1 + test/functional/p2p_sendtxrcncl.py | 4 +++- test/functional/test_framework/p2p.py | 15 +++++++++------ 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py index f4462673f20..bd766a279ef 100755 --- a/test/functional/p2p_add_connections.py +++ b/test/functional/p2p_add_connections.py @@ -17,7 +17,7 @@ class P2PFeelerReceiver(P2PInterface): # message is received from the test framework. Don't send any responses # to the node's version message since the connection will already be # closed. - pass + self.send_version() class P2PAddConnections(BitcoinTestFramework): def set_test_params(self): diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 2adcaf178c7..b23ec1028b6 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -75,6 +75,7 @@ class AddrReceiver(P2PInterface): return self.num_ipv4_received != 0 def on_version(self, message): + self.send_version() self.send_message(msg_verack()) if (self.send_getaddr): self.send_message(msg_getaddr()) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 0e349ef70cd..8f5e6c03873 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -29,6 +29,7 @@ class PeerNoVerack(P2PInterface): # Avoid sending verack in response to version. # When calling add_p2p_connection, wait_for_verack=False must be set (see # comment in add_p2p_connection). + self.send_version() if message.nVersion >= 70016 and self.wtxidrelay: self.send_message(msg_wtxidrelay()) @@ -43,7 +44,8 @@ class SendTxrcnclReceiver(P2PInterface): class P2PFeelerReceiver(SendTxrcnclReceiver): def on_version(self, message): - pass # feeler connections can not send any message other than their own version + # feeler connections can not send any message other than their own version + self.send_version() class PeerTrackMsgOrder(P2PInterface): diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index d7668009028..ddb68dd4e25 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -224,9 +224,9 @@ class P2PConnection(asyncio.Protocol): if self.supports_v2_p2p and self.v2_state.initiating and not self.v2_state.tried_v2_handshake: send_handshake_bytes = self.v2_state.initiate_v2_handshake() self.send_raw_message(send_handshake_bytes) - # if v2 connection, send `on_connection_send_msg` after initial v2 handshake. - # if reconnection situation, send `on_connection_send_msg` after version message is received in `on_version()`. - if not self.supports_v2_p2p and not self.reconnect: + # for v1 outbound connections, send version message immediately after opening + # (for v2 outbound connections, send it after the initial v2 handshake) + if self.p2p_connected_to_node and not self.supports_v2_p2p: self.send_version() self.on_open() @@ -284,7 +284,9 @@ class P2PConnection(asyncio.Protocol): raise ValueError("invalid v2 mac tag in handshake authentication") self.recvbuf = self.recvbuf[length:] if self.v2_state.tried_v2_handshake: - self.send_version() + # for v2 outbound connections, send version message immediately after v2 handshake + if self.p2p_connected_to_node: + self.send_version() # process post-v2-handshake data immediately, if available if len(self.recvbuf) > 0: self._on_data() @@ -558,8 +560,9 @@ class P2PInterface(P2PConnection): def on_version(self, message): assert message.nVersion >= MIN_P2P_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_P2P_VERSION_SUPPORTED) - # reconnection using v1 P2P has happened since version message can be processed, previously unsent version message is sent using v1 P2P here - if self.reconnect: + # for inbound connections, reply to version with own version message + # (could be due to v1 reconnect after a failed v2 handshake) + if not self.p2p_connected_to_node: self.send_version() self.reconnect = False if message.nVersion >= 70016 and self.wtxidrelay: