From cc8f239663a0874c307efa30815b40d9ef40badb Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Tue, 22 Apr 2025 15:17:36 +0200
Subject: [PATCH] net: change FindNode() to not return a node and rename it

All callers of `CConnman::FindNode()` use its return value `CNode*` only
as a boolean null/notnull. So change that method to return `bool`.

This removes the dangerous pattern of handling a `CNode` object (the
return value of `FindNode()`) without holding `CConnman::m_nodes_mutex`
and without having that object's reference count incremented for the
duration of the usage.

Also rename the method to better describe what it does.
---
 src/net.cpp | 31 +++++++++----------------------
 src/net.h   | 19 +++++++++++++++++--
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 2870fd4d5a2..c3162089e27 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -332,26 +332,16 @@ bool IsLocal(const CService& addr)
     return mapLocalHost.count(addr) > 0;
 }
 
-CNode* CConnman::FindNode(const std::string& addrName)
+bool CConnman::OutboundConnectedToStr(const std::string& str_addr)
 {
     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, [&str_addr](CNode* node) { return node->m_addr_name == str_addr; });
 }
 
-CNode* CConnman::FindNode(const CService& addr)
+bool CConnman::OutboundConnectedToService(const CService& addr)
 {
     LOCK(m_nodes_mutex);
-    for (CNode* pnode : m_nodes) {
-        if (static_cast<CService>(pnode->addr) == addr) {
-            return pnode;
-        }
-    }
-    return nullptr;
+    return std::ranges::any_of(m_nodes, [&addr](CNode* node) { return node->addr == addr; });
 }
 
 bool CConnman::AlreadyConnectedToAddress(const CAddress& addr)
@@ -399,10 +389,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
             return nullptr;
 
         // Look for an existing connection
-        CNode* pnode = FindNode(static_cast<CService>(addrConnect));
-        if (pnode)
-        {
-            LogPrintf("Failed to open new connection, already connected\n");
+        if (OutboundConnectedToService(addrConnect)) {
+            LogPrintf("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
             return nullptr;
         }
     }
@@ -432,9 +420,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<CService>(addrConnect));
-                if (pnode) {
+                if (OutboundConnectedToService(addrConnect)) {
                     LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
                     return nullptr;
                 }
@@ -2999,8 +2985,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 (OutboundConnectedToStr(pszDest)) {
         return;
+    }
 
     CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type, use_v2transport);
 
diff --git a/src/net.h b/src/net.h
index 19b35e8fd2b..dc0b6661af1 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1352,8 +1352,23 @@ private:
 
     uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const;
 
-    CNode* FindNode(const std::string& addrName);
-    CNode* FindNode(const CService& addr);
+    /**
+     * 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] str_addr String of the form "address:port", e.g. "1.2.3.4:8333".
+     * @return true if connected to addrName.
+     */
+    bool OutboundConnectedToStr(const std::string& str_addr);
+
+    /**
+     * 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 Address and port to check.
+     * @return true if connected to addr.
+     */
+    bool OutboundConnectedToService(const CService& addr);
 
     /**
      * Determine whether we're already connected to a given address, in order to