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

bf5662c678455fb47c402f8520215726ddfe7a94 test: enable v2 for python p2p depending on global --v2transport flag (Martin Zumsande)
6e9e39da434f8dafacee4cba068daf499bdb4cc2 test: Don't use v2transport when it's too slow. (Martin Zumsande)
87549c8f89fe8c9f404b74c5a40b5ee54c5a966d test: enable p2p_invalid_messages.py with v2transport (Martin Zumsande)
5fc9db504b9ac784019e7e8c215c31abfccb62b6 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 bf5662c678455fb47c402f8520215726ddfe7a94
  vasild:
    ACK bf5662c678455fb47c402f8520215726ddfe7a94
  stratospher:
    reACK bf5662c6.
  theStack:
    Tested ACK bf5662c678455fb47c402f8520215726ddfe7a94

Tree-SHA512: 4f5a08248ba8a755f7d0f48deb2b79bef03292345cacb7deef01be955481093800e4e56ff218ea56734eef5de1fb3ab0f04657447ea27d393bb536539d7b289d

8 files changed, 81 insertions, 36 deletions

diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 58ef1e761d..8a95975184 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -1263,6 +1263,10 @@ class FullBlockTest(BitcoinTestFramework):
         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)
diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py
index 814eb21e6f..39cff7b738 100755
--- a/test/functional/feature_maxuploadtarget.py
+++ b/test/functional/feature_maxuploadtarget.py
@@ -81,7 +81,8 @@ class MaxUploadTest(BitcoinTestFramework):
         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 @@ class MaxUploadTest(BitcoinTestFramework):
         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())
diff --git a/test/functional/p2p_ibd_stalling.py b/test/functional/p2p_ibd_stalling.py
index 0eb37fa92f..830f374d63 100755
--- a/test/functional/p2p_ibd_stalling.py
+++ b/test/functional/p2p_ibd_stalling.py
@@ -80,7 +80,8 @@ class P2PIBDStallingTest(BitcoinTestFramework):
 
         # 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
diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py
index 4916d36ab7..40a69936bc 100755
--- a/test/functional/p2p_invalid_messages.py
+++ b/test/functional/p2p_invalid_messages.py
@@ -109,6 +109,9 @@ class InvalidMessagesTest(BitcoinTestFramework):
         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 @@ class InvalidMessagesTest(BitcoinTestFramework):
         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 @@ class InvalidMessagesTest(BitcoinTestFramework):
     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 @@ class InvalidMessagesTest(BitcoinTestFramework):
     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 @@ class InvalidMessagesTest(BitcoinTestFramework):
 
     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
 
diff --git a/test/functional/p2p_timeouts.py b/test/functional/p2p_timeouts.py
index b4fa5099d8..80d7b6e9ae 100755
--- a/test/functional/p2p_timeouts.py
+++ b/test/functional/p2p_timeouts.py
@@ -69,11 +69,8 @@ class TimeoutsTest(BitcoinTestFramework):
         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 @@ class TimeoutsTest(BitcoinTestFramework):
         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)
diff --git a/test/functional/p2p_v2_earlykeyresponse.py b/test/functional/p2p_v2_earlykeyresponse.py
index 1f570e6010..32d2e1148a 100755
--- a/test/functional/p2p_v2_earlykeyresponse.py
+++ b/test/functional/p2p_v2_earlykeyresponse.py
@@ -75,7 +75,7 @@ class P2PEarlyKey(BitcoinTestFramework):
         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')
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index afb75ab208..b4a58df5b2 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -117,6 +117,9 @@ class NetTest(BitcoinTestFramework):
         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 @@ class NetTest(BitcoinTestFramework):
                 "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,8 +137,8 @@ class NetTest(BitcoinTestFramework):
                 "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": [],
@@ -145,13 +146,13 @@ class NetTest(BitcoinTestFramework):
                 "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,
             },
         )
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 838dcba141..3baa78fd79 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -667,7 +667,7 @@ class TestNode():
                     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 @@ class TestNode():
             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 @@ class TestNode():
 
         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 @@ class TestNode():
 
         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 @@ class TestNode():
             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 @@ class TestNode():
             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()