net: merge AlreadyConnectedToAddress() and FindNode(CNetAddr)
What the developer wrote
Authored by Vasil Dimov
net: merge AlreadyConnectedToAddress() and FindNode(CNetAddr)
`CConnman::AlreadyConnectedToAddress()` is the only caller of
`CConnman::FindNode(CNetAddr)`, so merge the two in one function.
The unit test that checked whether `AlreadyConnectedToAddress()` ignores
the port is now unnecessary because now the function takes a `CNetAddr`
argument. It has no access to the port.
What changed, and why it matters
This is a routine code cleanup in Bitcoin Core's networking layer. It merges two internal functions that check whether the node is already connected to a given internet address, and removes a now-redundant unit test. There is no security-relevant change: the function still does the same basic check, just with slightly simpler code.
No action required. This is a benign refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors CConnman::AlreadyConnectedToAddress() by inlining the previously separate FindNode(const CNetAddr&) helper. The function now takes a CNetAddr directly and uses std::ranges::any_of under m_nodes_mutex to check for an existing connection. The change removes a unit test that verified port-agnostic matching, because CNetAddr has no port. The public test helper is renamed accordingly. No behavior affecting network security, peer selection, or connection logic is altered.
Changed components
src/net.cppsrc/net.hsrc/test/net_peer_connection_tests.cppsrc/test/util/net.hInspect captured patch +8 / −27
diff --git a/src/net.cpp b/src/net.cpp
index 78e5706b..e8d2819a 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -331,17 +331,6 @@ bool IsLocal(const CService& addr)
return mapLocalHost.count(addr) > 0;
}
-CNode* CConnman::FindNode(const CNetAddr& ip)
-{
- LOCK(m_nodes_mutex);
- for (CNode* pnode : m_nodes) {
- if (static_cast<CNetAddr>(pnode->addr) == ip) {
- return pnode;
- }
- }
- return nullptr;
-}
-
CNode* CConnman::FindNode(const std::string& addrName)
{
LOCK(m_nodes_mutex);
@@ -364,9 +353,10 @@ CNode* CConnman::FindNode(const CService& addr)
return nullptr;
}
-bool CConnman::AlreadyConnectedToAddress(const CAddress& addr)
+bool CConnman::AlreadyConnectedToAddress(const CNetAddr& addr) const
{
- return FindNode(static_cast<CNetAddr>(addr));
+ LOCK(m_nodes_mutex);
+ return std::ranges::any_of(m_nodes, [&addr](CNode* node) { return node->addr == addr; });
}
bool CConnman::CheckIncomingNonce(uint64_t nonce)
diff --git a/src/net.h b/src/net.h
index afbcc52b..52044fe1 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1365,15 +1365,13 @@ 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 address, in order to
- * avoid initiating duplicate connections.
+ * Determine whether we're already connected to a given address.
*/
- bool AlreadyConnectedToAddress(const CAddress& addr);
+ 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/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp
index ba3224bc..f00aabf5 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 a938ff18..f696cd53 100644
--- a/src/test/util/net.h
+++ b/src/test/util/net.h
@@ -89,7 +89,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);
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.