From e7fd70f4b6b163f4ad5b25b4da7fa79899245235 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Wed, 31 Jan 2024 14:34:34 +0530
Subject: [PATCH] [test] make v2transport arg in addconnection mandatory and
 few cleanups

`TestNode::add_outbound_p2p_connection()` is the only place where
addconnection test-only RPC is used. here, we always pass the
appropriate v2transport option to addconnection RPC.

currently the v2transport option for addconnection RPC is optional.
so simply make the v2transport option mandatory instead.
---
 src/rpc/net.cpp                          | 4 ++--
 test/functional/feature_anchors.py       | 2 +-
 test/functional/test_framework/p2p.py    | 8 +++-----
 test/functional/test_framework/v2_p2p.py | 1 +
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index ff43edba3e..5e6f42b596 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -371,7 +371,7 @@ static RPCHelpMan addconnection()
         {
             {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."},
             {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."},
-            {"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol"},
+            {"v2transport", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Attempt to connect using BIP324 v2 transport protocol"},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -403,7 +403,7 @@ static RPCHelpMan addconnection()
     } else {
         throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString());
     }
-    bool use_v2transport = !request.params[2].isNull() && request.params[2].get_bool();
+    bool use_v2transport = self.Arg<bool>(2);
 
     NodeContext& node = EnsureAnyNodeContext(request.context);
     CConnman& connman = EnsureConnman(node);
diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py
index 3b75a06d9e..5d68f50f58 100755
--- a/test/functional/feature_anchors.py
+++ b/test/functional/feature_anchors.py
@@ -99,7 +99,7 @@ class AnchorsTest(BitcoinTestFramework):
         self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])
 
         self.log.info("Add 256-bit-address block-relay-only connections to node")
-        self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only')
+        self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only', v2transport=False)
 
         self.log.debug("Stop node")
         with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]):
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index c113a4c8d8..b3206eb7f5 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -243,7 +243,7 @@ class P2PConnection(asyncio.Protocol):
         self.on_close()
 
     # v2 handshake method
-    def v2_handshake(self):
+    def _on_data_v2_handshake(self):
         """v2 handshake performed before P2P messages are exchanged (see BIP324). P2PConnection is the initiator
         (in inbound connections to TestNode) and the responder (in outbound connections from TestNode).
         Performed by:
@@ -295,7 +295,7 @@ class P2PConnection(asyncio.Protocol):
         if len(t) > 0:
             self.recvbuf += t
             if self.supports_v2_p2p and not self.v2_state.tried_v2_handshake:
-                self.v2_handshake()
+                self._on_data_v2_handshake()
             else:
                 self._on_data()
 
@@ -593,9 +593,7 @@ class P2PInterface(P2PConnection):
 
     def wait_for_reconnect(self, timeout=60):
         def test_function():
-            if not (self.is_connected and self.last_message.get('version') and self.v2_state is None):
-                return False
-            return True
+            return self.is_connected and self.last_message.get('version') and not self.supports_v2_p2p
         self.wait_until(test_function, timeout=timeout, check_connected=False)
 
     # Message receiving helper methods
diff --git a/test/functional/test_framework/v2_p2p.py b/test/functional/test_framework/v2_p2p.py
index 0b3979fba2..8f79623bd8 100644
--- a/test/functional/test_framework/v2_p2p.py
+++ b/test/functional/test_framework/v2_p2p.py
@@ -220,6 +220,7 @@ class EncryptedP2PState:
             # decoy packets have contents = None. v2 handshake is complete only when version packet
             # (can be empty with contents = b"") with contents != None is received.
             if contents is not None:
+                assert contents == b""  # currently TestNode sends an empty version packet
                 self.tried_v2_handshake = True
                 return processed_length, True
             response = response[length:]