From 94e85a82a753a0aa5ad688fc46330e83c9a697fe Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 24 Apr 2025 16:19:47 +0200 Subject: [PATCH 1/2] net: remove unnecessary check from AlreadyConnectedToAddress() `CConnman::AlreadyConnectedToAddress()` searches the existent nodes by address or by address-and-port: ```cpp FindNode(static_cast(addr)) || FindNode(addr.ToStringAddrPort()) ``` but: * if there is a match by just the address, then the address-and-port search will not be evaluated and the whole condition will be `true` * if the there is no node with the same address, then the second search by address-and-port will not find a match either. The search by address-and-port is comparing against `CNode::m_addr_name` which could be a hostname, e.g. `"node.foobar.com:8333"`, but `addr.ToStringAddrPort()` is always going to be numeric. --- src/net.cpp | 2 +- src/test/fuzz/netaddress.cpp | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index fc0edc1a5c1..66ded1041cd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -367,7 +367,7 @@ CNode* CConnman::FindNode(const CService& addr) bool CConnman::AlreadyConnectedToAddress(const CAddress& addr) { - return FindNode(static_cast(addr)) || FindNode(addr.ToStringAddrPort()); + return FindNode(static_cast(addr)); } bool CConnman::CheckIncomingNonce(uint64_t nonce) diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 28a2026a358..71990e1298e 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -101,10 +101,14 @@ FUZZ_TARGET(netaddress) (void)net_addr.GetReachabilityFrom(other_net_addr); (void)sub_net.Match(other_net_addr); - const CService other_service{net_addr, fuzzed_data_provider.ConsumeIntegral()}; + const CService other_service{fuzzed_data_provider.ConsumeBool() ? net_addr : other_net_addr, fuzzed_data_provider.ConsumeIntegral()}; assert((service == other_service) != (service != other_service)); (void)(service < other_service); + if (service.ToStringAddrPort() == other_service.ToStringAddrPort()) { + assert(static_cast(service) == static_cast(other_service)); + } + const CSubNet sub_net_copy_1{net_addr, other_net_addr}; const CSubNet sub_net_copy_2{net_addr}; From f1b142856a4ecd0a0d90bc3d73ef5137997b14ff Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 24 Apr 2025 16:32:42 -0700 Subject: [PATCH 2/2] test: Same addr, diff port is already connected --- src/test/net_peer_connection_tests.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index e60ce8b99d3..ba3224bca9d 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -155,6 +155,13 @@ BOOST_FIXTURE_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection, 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})); + } + // Clean up for (auto node : connman->TestNodes()) { peerman->FinalizeNode(*node);