Merge 4c199714f9 into 85f96b01b7

src/net.cpp

@@ -1871,8 +1871,7 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ
     CSemaphoreGrant grant(*semOutbound, true);
     if (!grant) return false;
 
-    OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport);
-    return true;
+    return OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport);
 }
 
 void CConnman::DisconnectNodes()
@@ -2959,7 +2958,7 @@ void CConnman::ThreadOpenAddedConnections()
 }
 
 // if successful, this moves the passed grant to the constructed node
-void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char *pszDest, ConnectionType conn_type, bool use_v2transport)
+bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char *pszDest, ConnectionType conn_type, bool use_v2transport)
 {
     AssertLockNotHeld(m_unused_i2p_sessions_mutex);
     assert(conn_type != ConnectionType::INBOUND);
@@ -2968,23 +2967,23 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
     // Initiate outbound network connection
     //
     if (interruptNet) {
-        return;
+        return false;
     }
     if (!fNetworkActive) {
-        return;
+        return false;
     }
     if (!pszDest) {
         bool banned_or_discouraged = m_banman && (m_banman->IsDiscouraged(addrConnect) || m_banman->IsBanned(addrConnect));
         if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) {
-            return;
+            return false;
         }
     } else if (FindNode(std::string(pszDest)))
-        return;
+        return false;
 
     CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport);
 
     if (!pnode)
-        return;
+        return false;
     pnode->grantOutbound = std::move(grant_outbound);
 
     m_msgproc->InitializeNode(*pnode, m_local_services);
@@ -2995,6 +2994,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
         // update connection count by network
         if (pnode->IsManualOrFullOutboundConn()) ++m_network_conn_counts[pnode->addr.GetNetwork()];
     }
+    return true;
 }
 
 Mutex NetEventsInterface::g_msgproc_mutex;

src/net.h

@@ -1136,7 +1136,7 @@ public:
     bool GetNetworkActive() const { return fNetworkActive; };
     bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; };
     void SetNetworkActive(bool active);
-    void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
+    bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
     bool CheckIncomingNonce(uint64_t nonce);
     void ASMapHealthCheck();
 

src/rpc/net.cpp

@@ -316,7 +316,14 @@ static RPCHelpMan addnode()
                     {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'add' to add a node to the list, 'remove' to remove a node from the list, 'onetry' to try a connection to the node once"},
                     {"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport"}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"},
                 },
-                RPCResult{RPCResult::Type::NONE, "", ""},
+                RPCResult{
+                    RPCResult::Type::OBJ, "", "",
+                    {
+                        {RPCResult::Type::STR, "operation", "The operation called (onetry/add/remove)"},
+                        {RPCResult::Type::STR, "result", "The result of the operation (success/failed)"},
+                        {RPCResult::Type::STR, "address", "The address of the peer"},
+                    }
+                },
                 RPCExamples{
                     HelpExampleCli("addnode", "\"192.168.0.6:8333\" \"onetry\" true")
             + HelpExampleRpc("addnode", "\"192.168.0.6:8333\", \"onetry\" true")
@@ -325,42 +332,41 @@ static RPCHelpMan addnode()
 {
     const auto command{self.Arg<std::string>("command")};
     if (command != "onetry" && command != "add" && command != "remove") {
-        throw std::runtime_error(
-            self.ToString());
+        throw std::runtime_error(self.ToString());
     }
 
     NodeContext& node = EnsureAnyNodeContext(request.context);
     CConnman& connman = EnsureConnman(node);
-
     const auto node_arg{self.Arg<std::string>("node")};
     bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
     bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport);
 
-    if (use_v2transport && !node_v2transport) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)");
+    UniValue result(UniValue::VOBJ);
+    result.pushKV("address", node_arg);
+    result.pushKV("operation", command);
+
+    if (command != "remove" && use_v2transport && !node_v2transport) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Adding v2transport connections requires -v2transport init flag to be set.");
     }
 
-    if (command == "onetry")
-    {
-        CAddress addr;
-        connman.OpenNetworkConnection(addr, /*fCountFailure=*/false, /*grant_outbound=*/{}, node_arg.c_str(), ConnectionType::MANUAL, use_v2transport);
-        return UniValue::VNULL;
-    }
+    std::string error_message;
 
-    if (command == "add")
-    {
+    if (command == "onetry") {
+        if (!connman.OpenNetworkConnection(CAddress(), false, {}, node_arg.c_str(), ConnectionType::MANUAL, use_v2transport)) {
+            throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Unable to open connection");
+        }
+    } else if (command == "add") {
         if (!connman.AddNode({node_arg, use_v2transport})) {
-            throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Node already added");
+            throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Node already added");
         }
-    }
-    else if (command == "remove")
-    {
+    } else if (command == "remove") {
         if (!connman.RemoveAddedNode(node_arg)) {
-            throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node could not be removed. It has not been added previously.");
+            throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Node could not be removed. It has not been added previously.");
         }
     }
 
-    return UniValue::VNULL;
+    result.pushKV("result", "success");
+    return result;
 },
     };
 }
@@ -415,7 +421,7 @@ static RPCHelpMan addconnection()
 
     const bool success = connman.AddConnection(address, conn_type, use_v2transport);
     if (!success) {
-        throw JSONRPCError(RPC_CLIENT_NODE_CAPACITY_REACHED, "Error: Already at capacity for specified connection type.");
+        throw JSONRPCError(RPC_CLIENT_PEER_NOT_CONNECTED, "Error: Unable to open connection");
     }
 
     UniValue info(UniValue::VOBJ);

src/rpc/protocol.h

@@ -62,7 +62,7 @@ enum RPCErrorCode
     RPC_CLIENT_NODE_NOT_CONNECTED   = -29, //!< Node to disconnect not found in connected nodes
     RPC_CLIENT_INVALID_IP_OR_SUBNET = -30, //!< Invalid IP/Subnet
     RPC_CLIENT_P2P_DISABLED         = -31, //!< No valid connection manager instance found
-    RPC_CLIENT_NODE_CAPACITY_REACHED= -34, //!< Max number of outbound or block-relay connections already open
+    RPC_CLIENT_PEER_NOT_CONNECTED   = -34, //!< We were unable to open a new connection to this peer
 
     //! Chain errors
     RPC_CLIENT_MEMPOOL_DISABLED     = -33, //!< No mempool instance found

test/functional/p2p_i2p_ports.py

@@ -8,6 +8,7 @@ Test ports handling for I2P hosts
 
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_raises_rpc_error
 
 PROXY = "127.0.0.1:60000"
 
@@ -23,12 +24,12 @@ class I2PPorts(BitcoinTestFramework):
         self.log.info("Ensure we don't try to connect if port!=0")
         addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p:8333"
         with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}, connection refused due to arbitrary port 8333"]):
-            node.addnode(node=addr, command="onetry")
+            assert_raises_rpc_error(-24, "Unable to open connection", node.addnode, node=addr, command='onetry')
 
         self.log.info("Ensure we try to connect if port=0 and get an error due to missing I2P proxy")
         addr = "h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0"
         with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}: Cannot connect to {PROXY}"]):
-            node.addnode(node=addr, command="onetry")
+            assert_raises_rpc_error(-24, "Unable to open connection", node.addnode, node=addr, command='onetry')
 
 
 if __name__ == '__main__':

test/functional/p2p_i2p_sessions.py

@@ -7,6 +7,7 @@ Test whether persistent or transient I2P sessions are being used, based on `-i2p
 """
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_raises_rpc_error
 
 
 class I2PSessions(BitcoinTestFramework):
@@ -24,12 +25,12 @@ class I2PSessions(BitcoinTestFramework):
         self.log.info("Ensure we create a persistent session when -i2pacceptincoming=1")
         node0 = self.nodes[0]
         with node0.assert_debug_log(expected_msgs=["Creating persistent SAM session"]):
-            node0.addnode(node=addr, command="onetry")
+            assert_raises_rpc_error(-24, "Unable to open connection", node0.addnode, node=addr, command='onetry')
 
         self.log.info("Ensure we create a transient session when -i2pacceptincoming=0")
         node1 = self.nodes[1]
         with node1.assert_debug_log(expected_msgs=["Creating transient SAM session"]):
-            node1.addnode(node=addr, command="onetry")
+            assert_raises_rpc_error(-24, "Unable to open connection", node1.addnode, node=addr, command='onetry')
 
 
 if __name__ == '__main__':

test/functional/p2p_v2_transport.py

@@ -63,7 +63,7 @@ class V2TransportTest(BitcoinTestFramework):
 
         # addnode rpc error when v2transport requested but not enabled
         ip_port = "127.0.0.1:{}".format(p2p_port(3))
-        assert_raises_rpc_error(-8, "Error: v2transport requested but not enabled (see -v2transport)", self.nodes[2].addnode, node=ip_port, command='add', v2transport=True)
+        assert_raises_rpc_error(-8, "Error: Adding v2transport connections requires -v2transport init flag to be set.", self.nodes[2].addnode, node=ip_port, command='add', v2transport=True)
 
         with self.nodes[2].assert_debug_log(expected_msgs=[],
                                             unexpected_msgs=[sending_handshake, downgrading_to_v1]):

test/functional/rpc_net.py

@@ -242,7 +242,7 @@ class NetTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].getaddednodeinfo(), [])
         self.log.info("Add a node (node2) to node0")
         ip_port = "127.0.0.1:{}".format(p2p_port(2))
-        self.nodes[0].addnode(node=ip_port, command='add')
+        assert self.nodes[0].addnode(node=ip_port, command='add')["result"] == "success"
         self.log.info("Try to add an equivalent ip and check it fails")
         self.log.debug("(note that OpenBSD doesn't support the IPv4 shorthand notation with omitted zero-bytes)")
         if platform.system() != "OpenBSD":