Merge bitcoin/bitcoin#29358: test: use v2 everywhere for P2PConnection if --v2transport is enabled

bf5662c test: enable v2 for python p2p depending on global --v2transport flag (Martin Zumsande)
6e9e39d test: Don't use v2transport when it's too slow. (Martin Zumsande)
87549c8 test: enable p2p_invalid_messages.py with v2transport (Martin Zumsande)
5fc9db5 test: enable p2p_sendtxrcncl.py with v2transport (Martin Zumsande)

Pull request description:

  #24748 added v2 transport to the python `P2PConnection`, but so far each test that wants to make use of it needs to enable it on an individual basis.
  This PR changes it so that if the test suite is run with `--v2transport` option, v2 is used in each test by default, not only for connections between two bitcoind instances as before, but also wherever `P2PConnection` is used. Individual tests can override this global option.

  To do that, a few tests need to be adjusted.
  In addition, I added a commit to always use v1 in a few select subtests that send a large number of large messages (e.g. large reorgs). These tests don't have a fundamental problem with v2 but become very slow due to the unoptimised python ChaCha20 implementation (~30 minutes on my computer, so probably not suitable to be run in the CI).

  As a result, `python3 test_runner.py --v2transport` should succeed and use `v2` everywhere (unless v1 is chosen explicitly).

  [Edit]: To make the "test each commit" CI pass, several test fixes were squashed into the last commit, which actually enables v2 p2p for `P2PConnection`. I have an unsquashed version at https://github.com/mzumsande/bitcoin/tree/202401_bip324_alltests_unsquashed, in case that helps with review.

ACKs for top commit:
  fjahr:
    tACK bf5662c
  vasild:
    ACK bf5662c
  stratospher:
    reACK bf5662c.
  theStack:
    Tested ACK bf5662c

Tree-SHA512: 4f5a08248ba8a755f7d0f48deb2b79bef03292345cacb7deef01be955481093800e4e56ff218ea56734eef5de1fb3ab0f04657447ea27d393bb536539d7b289d

Author: fanquake

test/functional/feature_block.py

@@ -1263,6 +1263,10 @@ def run_test(self):
         b89a = self.update_block("89a", [tx])
         self.send_blocks([b89a], success=False, reject_reason='bad-txns-inputs-missingorspent', reconnect=True)
 
+        # Don't use v2transport for the large reorg, which is too slow with the unoptimized python ChaCha20 implementation
+        if self.options.v2transport:
+            self.nodes[0].disconnect_p2ps()
+            self.helper_peer = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
         self.log.info("Test a re-org of one week's worth of blocks (1088 blocks)")
 
         self.move_tip(88)

test/functional/feature_maxuploadtarget.py

@@ -81,7 +81,8 @@ def run_test(self):
         p2p_conns = []
 
         for _ in range(3):
-            p2p_conns.append(self.nodes[0].add_p2p_connection(TestP2PConn()))
+            # Don't use v2transport in this test (too slow with the unoptimized python ChaCha20 implementation)
+            p2p_conns.append(self.nodes[0].add_p2p_connection(TestP2PConn(), supports_v2_p2p=False))
 
         # Now mine a big block
         mine_large_block(self, self.wallet, self.nodes[0])
@@ -173,7 +174,7 @@ def run_test(self):
         self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=False)
 
         # Reconnect to self.nodes[0]
-        peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+        peer = self.nodes[0].add_p2p_connection(TestP2PConn(), supports_v2_p2p=False)
 
         # Sending mempool message shouldn't disconnect peer, as total limit isn't reached yet
         peer.send_and_ping(msg_mempool())

test/functional/p2p_ibd_stalling.py

@@ -80,7 +80,8 @@ def run_test(self):
 
         # Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
         # returning the number of downloaded (but not connected) blocks.
-        self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)
+        bytes_recv = 172761 if not self.options.v2transport else 169692
+        self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
 
         self.all_sync_send_with_ping(peers)
         # If there was a peer marked for stalling, it would get disconnected

test/functional/p2p_invalid_messages.py

@@ -109,6 +109,9 @@ def test_duplicate_version_msg(self):
         self.nodes[0].disconnect_p2ps()
 
     def test_magic_bytes(self):
+        # Skip with v2, magic bytes are v1-specific
+        if self.options.v2transport:
+            return
         self.log.info("Test message with invalid magic bytes disconnects peer")
         conn = self.nodes[0].add_p2p_connection(P2PDataStore())
         with self.nodes[0].assert_debug_log(['Header error: Wrong MessageStart ffffffff received']):
@@ -120,6 +123,9 @@ def test_magic_bytes(self):
         self.nodes[0].disconnect_p2ps()
 
     def test_checksum(self):
+        # Skip with v2, the checksum is v1-specific
+        if self.options.v2transport:
+            return
         self.log.info("Test message with invalid checksum logs an error")
         conn = self.nodes[0].add_p2p_connection(P2PDataStore())
         with self.nodes[0].assert_debug_log(['Header error: Wrong checksum (badmsg, 2 bytes), expected 78df0a04 was ffffffff']):
@@ -137,7 +143,11 @@ def test_checksum(self):
     def test_size(self):
         self.log.info("Test message with oversized payload disconnects peer")
         conn = self.nodes[0].add_p2p_connection(P2PDataStore())
-        with self.nodes[0].assert_debug_log(['Header error: Size too large (badmsg, 4000001 bytes)']):
+        error_msg = (
+            ['V2 transport error: packet too large (4000014 bytes)'] if self.options.v2transport
+            else ['Header error: Size too large (badmsg, 4000001 bytes)']
+        )
+        with self.nodes[0].assert_debug_log(error_msg):
             msg = msg_unrecognized(str_data="d" * (VALID_DATA_LIMIT + 1))
             msg = conn.build_message(msg)
             conn.send_raw_message(msg)
@@ -147,15 +157,26 @@ def test_size(self):
     def test_msgtype(self):
         self.log.info("Test message with invalid message type logs an error")
         conn = self.nodes[0].add_p2p_connection(P2PDataStore())
-        with self.nodes[0].assert_debug_log(['Header error: Invalid message type']):
+        if self.options.v2transport:
+            msgtype = 99 # not defined
             msg = msg_unrecognized(str_data="d")
-            msg = conn.build_message(msg)
-            # Modify msgtype
-            msg = msg[:7] + b'\x00' + msg[7 + 1:]
-            conn.send_raw_message(msg)
-            conn.sync_with_ping(timeout=1)
-        # Check that traffic is accounted for (24 bytes header + 2 bytes payload)
-        assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 26)
+            contents = msgtype.to_bytes(1, 'big') + msg.serialize()
+            tmsg = conn.v2_state.v2_enc_packet(contents, ignore=False)
+            with self.nodes[0].assert_debug_log(['V2 transport error: invalid message type']):
+                conn.send_raw_message(tmsg)
+                conn.sync_with_ping(timeout=1)
+            # Check that traffic is accounted for (20 bytes plus 3 bytes contents)
+            assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 23)
+        else:
+            with self.nodes[0].assert_debug_log(['Header error: Invalid message type']):
+                msg = msg_unrecognized(str_data="d")
+                msg = conn.build_message(msg)
+                # Modify msgtype
+                msg = msg[:7] + b'\x00' + msg[7 + 1:]
+                conn.send_raw_message(msg)
+                conn.sync_with_ping(timeout=1)
+                # Check that traffic is accounted for (24 bytes header + 2 bytes payload)
+                assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 26)
         self.nodes[0].disconnect_p2ps()
 
     def test_addrv2(self, label, required_log_messages, raw_addrv2):
@@ -306,8 +327,10 @@ def test_noncontinuous_headers_msg(self):
 
     def test_resource_exhaustion(self):
         self.log.info("Test node stays up despite many large junk messages")
-        conn = self.nodes[0].add_p2p_connection(P2PDataStore())
-        conn2 = self.nodes[0].add_p2p_connection(P2PDataStore())
+        # Don't use v2 here - the non-optimised encryption would take too long to encrypt
+        # the large messages
+        conn = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
+        conn2 = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
         msg_at_size = msg_unrecognized(str_data="b" * VALID_DATA_LIMIT)
         assert len(msg_at_size.serialize()) == MAX_PROTOCOL_MESSAGE_LENGTH
 

test/functional/p2p_timeouts.py

@@ -69,11 +69,8 @@ def run_test(self):
         with self.nodes[0].assert_debug_log(['Unsupported message "ping" prior to verack from peer=0']):
             no_verack_node.send_message(msg_ping())
 
-        # With v2, non-version messages before the handshake would be interpreted as part of the key exchange.
-        # Therefore, don't execute this part of the test if v2transport is chosen.
-        if not self.options.v2transport:
-            with self.nodes[0].assert_debug_log(['non-version message before version handshake. Message "ping" from peer=1']):
-                no_version_node.send_message(msg_ping())
+        with self.nodes[0].assert_debug_log(['non-version message before version handshake. Message "ping" from peer=1']):
+            no_version_node.send_message(msg_ping())
 
         self.mock_forward(1)
         assert "version" in no_verack_node.last_message
@@ -83,14 +80,20 @@ def run_test(self):
         assert no_send_node.is_connected
 
         no_verack_node.send_message(msg_ping())
-        if not self.options.v2transport:
-            no_version_node.send_message(msg_ping())
-
-        expected_timeout_logs = [
-            "version handshake timeout peer=0",
-            f"socket no message in first 3 seconds, {'0' if self.options.v2transport else '1'} 0 peer=1",
-            "socket no message in first 3 seconds, 0 0 peer=2",
-        ]
+        no_version_node.send_message(msg_ping())
+
+        if self.options.v2transport:
+            expected_timeout_logs = [
+                "version handshake timeout peer=0",
+                "version handshake timeout peer=1",
+                "version handshake timeout peer=2",
+            ]
+        else:
+            expected_timeout_logs = [
+                "version handshake timeout peer=0",
+                "socket no message in first 3 seconds, 1 0 peer=1",
+                "socket no message in first 3 seconds, 0 0 peer=2",
+            ]
 
         with self.nodes[0].assert_debug_log(expected_msgs=expected_timeout_logs):
             self.mock_forward(2)

test/functional/p2p_v2_earlykeyresponse.py

@@ -75,7 +75,7 @@ def run_test(self):
         self.log.info('Sending first 4 bytes of ellswift which match network magic')
         self.log.info('If a response is received, assertion failure would happen in our custom data_received() function')
         # send happens in `initiate_v2_handshake()` in `connection_made()`
-        peer1 = node0.add_p2p_connection(PeerEarlyKey(), wait_for_verack=False, send_version=False, supports_v2_p2p=True)
+        peer1 = node0.add_p2p_connection(PeerEarlyKey(), wait_for_verack=False, send_version=False, supports_v2_p2p=True, wait_for_v2_handshake=False)
         self.wait_until(lambda: peer1.connection_opened)
         self.log.info('Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is')
         self.log.info('expected now, our custom data_received() function wouldn\'t result in assertion failure')

test/functional/rpc_net.py

@@ -117,6 +117,9 @@ def test_getpeerinfo(self):
         peer_info = self.nodes[0].getpeerinfo()[no_version_peer_id]
         peer_info.pop("addr")
         peer_info.pop("addrbind")
+        # The next two fields will vary for v2 connections because we send a rng-based number of decoy messages
+        peer_info.pop("bytesrecv")
+        peer_info.pop("bytessent")
         assert_equal(
             peer_info,
             {
@@ -125,9 +128,7 @@ def test_getpeerinfo(self):
                 "addr_relay_enabled": False,
                 "bip152_hb_from": False,
                 "bip152_hb_to": False,
-                "bytesrecv": 0,
                 "bytesrecv_per_msg": {},
-                "bytessent": 0,
                 "bytessent_per_msg": {},
                 "connection_type": "inbound",
                 "conntime": no_version_peer_conntime,
@@ -136,22 +137,22 @@ def test_getpeerinfo(self):
                 "inflight": [],
                 "last_block": 0,
                 "last_transaction": 0,
-                "lastrecv": 0,
-                "lastsend": 0,
+                "lastrecv": 0 if not self.options.v2transport else no_version_peer_conntime,
+                "lastsend": 0 if not self.options.v2transport else no_version_peer_conntime,
                 "minfeefilter": Decimal("0E-8"),
                 "network": "not_publicly_routable",
                 "permissions": [],
                 "presynced_headers": -1,
                 "relaytxes": False,
                 "services": "0000000000000000",
                 "servicesnames": [],
-                "session_id": "",
+                "session_id": "" if not self.options.v2transport else no_version_peer.v2_state.peer['session_id'].hex(),
                 "startingheight": -1,
                 "subver": "",
                 "synced_blocks": -1,
                 "synced_headers": -1,
                 "timeoffset": 0,
-                "transport_protocol_type": "v1" if not self.options.v2transport else "detecting",
+                "transport_protocol_type": "v1" if not self.options.v2transport else "v2",
                 "version": 0,
             },
         )

test/functional/test_framework/test_node.py

@@ -667,7 +667,7 @@ def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, mat
                     assert_msg += "with expected error " + expected_msg
                 self._raise_assertion_error(assert_msg)
 
-    def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=False, **kwargs):
+    def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=None, wait_for_v2_handshake=True, **kwargs):
         """Add an inbound p2p connection to the node.
 
         This method adds the p2p connection to the self.p2ps list and also
@@ -684,6 +684,9 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru
             kwargs['dstport'] = p2p_port(self.index)
         if 'dstaddr' not in kwargs:
             kwargs['dstaddr'] = '127.0.0.1'
+        if supports_v2_p2p is None:
+            supports_v2_p2p = self.use_v2transport
+
 
         p2p_conn.p2p_connected_to_node = True
         if self.use_v2transport:
@@ -693,6 +696,8 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru
 
         self.p2ps.append(p2p_conn)
         p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False)
+        if supports_v2_p2p and wait_for_v2_handshake:
+            p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake)
         if send_version:
             p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg)
         if wait_for_verack:
@@ -721,7 +726,7 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru
 
         return p2p_conn
 
-    def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=False, advertise_v2_p2p=False, **kwargs):
+    def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=None, advertise_v2_p2p=None, **kwargs):
         """Add an outbound p2p connection from node. Must be an
         "outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection.
 
@@ -749,6 +754,11 @@ def addconnection_callback(address, port):
             self.addconnection('%s:%d' % (address, port), connection_type, advertise_v2_p2p)
 
         p2p_conn.p2p_connected_to_node = False
+        if supports_v2_p2p is None:
+            supports_v2_p2p = self.use_v2transport
+        if advertise_v2_p2p is None:
+            advertise_v2_p2p = self.use_v2transport
+
         if advertise_v2_p2p:
             kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2
             assert self.use_v2transport  # only a v2 TestNode could make a v2 outbound connection
@@ -771,6 +781,8 @@ def addconnection_callback(address, port):
             p2p_conn.wait_for_connect()
             self.p2ps.append(p2p_conn)
 
+            if supports_v2_p2p:
+                p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake)
             p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg)
             if wait_for_verack:
                 p2p_conn.wait_for_verack()