diff --git a/src/net.cpp b/src/net.cpp index c06db298672..584b4280905 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -331,42 +331,22 @@ bool IsLocal(const CService& addr) return mapLocalHost.count(addr) > 0; } -CNode* CConnman::FindNode(const CNetAddr& ip) +bool CConnman::AlreadyConnectedToHost(const std::string& host) const { LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (static_cast(pnode->addr) == ip) { - return pnode; - } - } - return nullptr; + return std::ranges::any_of(m_nodes, [&host](CNode* node) { return node->m_addr_name == host; }); } -CNode* CConnman::FindNode(const std::string& addrName) +bool CConnman::AlreadyConnectedToAddressPort(const CService& addr_port) const { LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (pnode->m_addr_name == addrName) { - return pnode; - } - } - return nullptr; + return std::ranges::any_of(m_nodes, [&addr_port](CNode* node) { return node->addr == addr_port; }); } -CNode* CConnman::FindNode(const CService& addr) +bool CConnman::AlreadyConnectedToAddress(const CNetAddr& addr) const { LOCK(m_nodes_mutex); - for (CNode* pnode : m_nodes) { - if (static_cast(pnode->addr) == addr) { - return pnode; - } - } - return nullptr; -} - -bool CConnman::AlreadyConnectedToAddress(const CAddress& addr) -{ - return FindNode(static_cast(addr)); + return std::ranges::any_of(m_nodes, [&addr](CNode* node) { return node->addr == addr; }); } bool CConnman::CheckIncomingNonce(uint64_t nonce) @@ -403,10 +383,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return nullptr; // Look for an existing connection - CNode* pnode = FindNode(static_cast(addrConnect)); - if (pnode) - { - LogPrintf("Failed to open new connection, already connected\n"); + if (AlreadyConnectedToAddressPort(addrConnect)) { + LogInfo("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort()); return nullptr; } } @@ -436,9 +414,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo } // It is possible that we already have a connection to the IP/port pszDest resolved to. // In that case, drop the connection that was just created. - LOCK(m_nodes_mutex); - CNode* pnode = FindNode(static_cast(addrConnect)); - if (pnode) { + if (AlreadyConnectedToAddressPort(addrConnect)) { LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); return nullptr; } @@ -3010,8 +2986,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (IsLocal(addrConnect) || banned_or_discouraged || AlreadyConnectedToAddress(addrConnect)) { return; } - } else if (FindNode(std::string(pszDest))) + } else if (AlreadyConnectedToHost(pszDest)) { return; + } CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport); @@ -3651,9 +3628,11 @@ void CConnman::GetNodeStats(std::vector& vstats) const bool CConnman::DisconnectNode(const std::string& strNode) { LOCK(m_nodes_mutex); - if (CNode* pnode = FindNode(strNode)) { - LogDebug(BCLog::NET, "disconnect by address%s match, %s", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->DisconnectMsg(fLogIPs)); - pnode->fDisconnect = true; + auto it = std::ranges::find_if(m_nodes, [&strNode](CNode* node) { return node->m_addr_name == strNode; }); + if (it != m_nodes.end()) { + CNode* node{*it}; + LogDebug(BCLog::NET, "disconnect by address%s match, %s", (fLogIPs ? strprintf("=%s", strNode) : ""), node->DisconnectMsg(fLogIPs)); + node->fDisconnect = true; return true; } return false; diff --git a/src/net.h b/src/net.h index 89a5fca1953..92aabae6404 100644 --- a/src/net.h +++ b/src/net.h @@ -1370,15 +1370,28 @@ private: uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const; - CNode* FindNode(const CNetAddr& ip); - CNode* FindNode(const std::string& addrName); - CNode* FindNode(const CService& addr); + /** + * Determine whether we're already connected to a given "host:port". + * Note that for inbound connections, the peer is likely using a random outbound + * port on their side, so this will likely not match any inbound connections. + * @param[in] host String of the form "host[:port]", e.g. "localhost" or "localhost:8333" or "1.2.3.4:8333". + * @return true if connected to `host`. + */ + bool AlreadyConnectedToHost(const std::string& host) const; /** - * Determine whether we're already connected to a given address, in order to - * avoid initiating duplicate connections. + * Determine whether we're already connected to a given address:port. + * Note that for inbound connections, the peer is likely using a random outbound + * port on their side, so this will likely not match any inbound connections. + * @param[in] addr_port Address and port to check. + * @return true if connected to addr_port. */ - bool AlreadyConnectedToAddress(const CAddress& addr); + bool AlreadyConnectedToAddressPort(const CService& addr_port) const; + + /** + * Determine whether we're already connected to a given address. + */ + bool AlreadyConnectedToAddress(const CNetAddr& addr) const; bool AttemptToEvictConnection(); CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 3a0432d7a9e..da01a60b9ec 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -130,7 +130,7 @@ static RPCHelpMan getpeerinfo() { { {RPCResult::Type::NUM, "id", "Peer index"}, - {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, + {RPCResult::Type::STR, "addr", "(host:port) The IP address/hostname optionally followed by :port of the peer"}, {RPCResult::Type::STR, "addrbind", /*optional=*/true, "(ip:port) Bind address of the connection to the peer"}, {RPCResult::Type::STR, "addrlocal", /*optional=*/true, "(ip:port) Local address as reported by the peer"}, {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/*append_unroutable=*/true), ", ") + ")"}, @@ -322,7 +322,7 @@ static RPCHelpMan addnode() strprintf("Addnode connections are limited to %u at a time", MAX_ADDNODE_CONNECTIONS) + " and are counted separately from the -maxconnections limit.\n", { - {"node", RPCArg::Type::STR, RPCArg::Optional::NO, "The address of the peer to connect to"}, + {"node", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address/hostname optionally followed by :port of the peer to connect to"}, {"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)"}, }, diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index ba3224bca9d..f00aabf5bef 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -151,15 +151,8 @@ BOOST_FIXTURE_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection, } BOOST_TEST_MESSAGE("\nCheck that all connected peers are correctly detected as connected"); - for (auto node : connman->TestNodes()) { - BOOST_CHECK(connman->AlreadyConnectedPublic(node->addr)); - } - - BOOST_TEST_MESSAGE("\nCheck that peers with the same addresses as connected peers but different ports are detected as connected."); - for (auto node : connman->TestNodes()) { - uint16_t changed_port = node->addr.GetPort() + 1; - CService address_with_changed_port{node->addr, changed_port}; - BOOST_CHECK(connman->AlreadyConnectedPublic(CAddress{address_with_changed_port, NODE_NONE})); + for (const auto& node : connman->TestNodes()) { + BOOST_CHECK(connman->AlreadyConnectedToAddressPublic(node->addr)); } // Clean up diff --git a/src/test/util/net.h b/src/test/util/net.h index 4ff23ac792c..77954d92a48 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -107,7 +107,7 @@ struct ConnmanTestMsg : public CConnman { bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg&& ser_msg) const; void FlushSendBuffer(CNode& node) const; - bool AlreadyConnectedPublic(const CAddress& addr) { return AlreadyConnectedToAddress(addr); }; + bool AlreadyConnectedToAddressPublic(const CNetAddr& addr) { return AlreadyConnectedToAddress(addr); }; CNode* ConnectNodePublic(PeerManager& peerman, const char* pszDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);