From f8fec8f26dda1bcbc2d3832bcd5568d5176be099 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 11 Aug 2023 16:12:42 -0600 Subject: [PATCH 1/4] p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() Addnode (manual) peers connected to us via the cjdns network are currently not detected by CConnman::GetAddedNodeInfo(), i.e. fConnected is always false. This causes the following issues: - RPC `getaddednodeinfo` incorrectly shows them as not connected - CConnman::ThreadOpenAddedConnections() continually retries to connect them --- src/net.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index ad1e464667..7a64d3ce04 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2832,7 +2832,7 @@ std::vector CConnman::GetAddedNodeInfo(bool include_connected) co } for (const auto& addr : lAddresses) { - CService service(LookupNumeric(addr.m_added_node, GetDefaultPort(addr.m_added_node))); + CService service{MaybeFlipIPv6toCJDNS(LookupNumeric(addr.m_added_node, GetDefaultPort(addr.m_added_node)))}; AddedNodeInfo addedNode{addr, CService(), false, false}; if (service.IsValid()) { // strAddNode is an IP:port From ca1300fbc5ac61e0117851e58c06f6dcecb62d0c Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 14 Nov 2023 18:12:45 -0600 Subject: [PATCH 2/4] test: add GetAddedNodeInfo() CJDNS regression unit test --- src/test/net_peer_connection_tests.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index 58cbe9eb72..00bc1fdb6a 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -108,6 +108,12 @@ BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection) AddPeer(id, nodes, *peerman, *connman, ConnectionType::BLOCK_RELAY, /*onion_peer=*/true); AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND); + // Add a CJDNS peer connection. + AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND, /*onion_peer=*/false, + /*address=*/"[fc00:3344:5566:7788:9900:aabb:ccdd:eeff]:1234"); + BOOST_CHECK(nodes.back()->IsInboundConn()); + BOOST_CHECK_EQUAL(nodes.back()->ConnectedThroughNetwork(), Network::NET_CJDNS); + BOOST_TEST_MESSAGE("Call AddNode() for all the peers"); for (auto node : connman->TestNodes()) { BOOST_CHECK(connman->AddNode({/*m_added_node=*/node->addr.ToStringAddrPort(), /*m_use_v2transport=*/true})); From 28823f30dc1a865cd44a6b7598ea968f92b75c56 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 14 Nov 2023 21:12:34 -0600 Subject: [PATCH 3/4] p2p, bugfix: correctly detect CJDNS addnode entries in AddNode() --- src/net.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7a64d3ce04..a3ed1d9d10 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3483,12 +3483,17 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres bool CConnman::AddNode(const AddedNodeParams& add) { - const CService resolved(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_added_node))); - const bool resolved_is_valid{resolved.IsValid()}; + const CService resolved{MaybeFlipIPv6toCJDNS(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_added_node)))}; + const bool resolved_invalid{!resolved.IsValid()}; LOCK(m_added_nodes_mutex); for (const auto& it : m_added_node_params) { - if (add.m_added_node == it.m_added_node || (resolved_is_valid && resolved == LookupNumeric(it.m_added_node, GetDefaultPort(it.m_added_node)))) return false; + if (add.m_added_node == it.m_added_node) return false; + if (resolved_invalid) continue; + const CService service{MaybeFlipIPv6toCJDNS(LookupNumeric(it.m_added_node, GetDefaultPort(it.m_added_node)))}; + if (resolved == service) return false; + // Check if CJDNS address matches regardless of port to detect already-connected inbound peers. + if (resolved.IsCJDNS() && static_cast(resolved) == static_cast(service)) return false; } m_added_node_params.push_back(add); From be4541abe5969ff4b4a26c983d024006f2cec94b Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 15 Nov 2023 13:12:28 -0600 Subject: [PATCH 4/4] test: AddNode() CJDNS regression unit tests --- src/test/net_peer_connection_tests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index 00bc1fdb6a..d1eebe751b 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -127,6 +127,15 @@ BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection) BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true})); #endif + BOOST_TEST_MESSAGE("\nCall AddNode() with a CJDNS service equal to an existing addnode entry; it should not be added"); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"[fc00:3344:5566:7788:9900:aabb:ccdd:eeff]:1234", /*m_use_v2transport=*/false})); + + BOOST_TEST_MESSAGE("\nCall AddNode() with a CJDNS addr equal to an existing inbound one but with a different port specified; it should not be added"); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"[fc00:3344:5566:7788:9900:aabb:ccdd:eeff]:8333", /*m_use_v2transport=*/false})); + + BOOST_TEST_MESSAGE("\nCall AddNode() with a CJDNS addr equal to an existing inbound one but resolving to a different port; it should not be added"); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"fc00:3344:5566:7788:9900:aabb:ccdd:eeff", /*m_use_v2transport=*/false})); + BOOST_TEST_MESSAGE("\nExpect GetAddedNodeInfo to return expected number of peers with `include_connected` true/false"); BOOST_CHECK_EQUAL(connman->GetAddedNodeInfo(/*include_connected=*/true).size(), nodes.size()); BOOST_CHECK(connman->GetAddedNodeInfo(/*include_connected=*/false).empty());