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 ff43edba3e9f9..5e6f42b596517 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 3b75a06d9e8f5..5d68f50f582be 100755
--- a/test/functional/feature_anchors.py
+++ b/test/functional/feature_anchors.py
@@ -99,7 +99,7 @@ def run_test(self):
         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 c113a4c8d81ea..b3206eb7f585d 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -243,7 +243,7 @@ def connection_lost(self, exc):
         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 @@ def data_received(self, t):
         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 @@ def wait_for_disconnect(self, timeout=60):
 
     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 0b3979fba2b6b..8f79623bd8ac2 100644
--- a/test/functional/test_framework/v2_p2p.py
+++ b/test/functional/test_framework/v2_p2p.py
@@ -220,6 +220,7 @@ def authenticate_handshake(self, response):
             # 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:]