mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-20 10:38:42 +01:00
Code Issues Projects Releases Wiki Activity

p2p: do not make automatic outbound connections to addnode peers

Browse Source 
to allocate our limited outbound slots correctly, and to ensure addnode
connections benefit from their intended protections.

Our addnode logic usually connects the addnode peers before the automatic
outbound logic does, but not always, as a connection race can occur.  If an
addnode peer disconnects us and if it was the only one from its network, there
can be a race between reconnecting to it with the addnode thread, and it being
picked as automatic network-specific outbound peer.  Or our internet connection
or router, or the addnode peer, could be temporarily offline, and then return
online during the automatic outbound thread.  Or we could add a new manual peer
using the addnode RPC at that time.

The race can be more apparent when our node doesn't know many peers, or with
networks like cjdns that currently have few bitcoin peers.

When an addnode peer is connected as an automatic outbound peer and is the only
connection we have to a network, it can be protected by our new outbound
eviction logic and persist in the "wrong role".

Examples on mainnet using logging added in the same pull request:

2023-08-12T14:51:05.681743Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to i2p peer selected for manual (addnode) connection: [geh...odq.b32.i2p]:0

2023-08-13T03:59:28.050853Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic block-relay-only connection to onion peer
selected for manual (addnode) connection: kpg...aid.onion:8333

2023-08-13T16:21:26.979052Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fcc...8ce]:8333

2023-08-14T20:43:53.401271Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fc7...59e]:8333

2023-08-15T00:10:01.894147Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic feeler connection to i2p peer selected for
manual (addnode) connection: geh...odq.b32.i2p:8333

Finally, there does not seem to be a reason to make block-relay or short-lived
feeler connections to addnode peers, as the addnode logic will ensure we connect
to them if they are up, within the addnode connection limit.

Fix these issues by checking if the address is an addnode peer in our automatic
outbound connection logic.
This commit is contained in:
Jon Atack 2023-11-16 09:56:03 -06:00
parent 22025d06e5
commit cc62716920
2 changed files with 23 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/net.cpp
View File

@ -2715,6 +2715,17 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
continue;
}
// Do not make automatic outbound connections to addnode peers, to
// not use our limited outbound slots for them and to ensure
// addnode connections benefit from their intended protections.
if (AddedNodesContain(addr)) {
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "Not making automatic %s%s connection to %s peer selected for manual (addnode) connection%s\n",
preferred_net.has_value() ? "network-specific " : "",
ConnectionTypeAsString(conn_type), GetNetworkName(addr.GetNetwork()),
fLogIPs ? strprintf(": %s", addr.ToStringAddrPort()) : "");
continue;
}
addrConnect = addr;
break;
}
@ -3454,6 +3465,17 @@ bool CConnman::RemoveAddedNode(const std::string& strNode)
return false;
}
bool CConnman::AddedNodesContain(const CAddress& addr) const
{
AssertLockNotHeld(m_added_nodes_mutex);
const std::string addr_str{addr.ToStringAddr()};
const std::string addr_port_str{addr.ToStringAddrPort()};
LOCK(m_added_nodes_mutex);
return (m_added_node_params.size() < 24 // bound the query to a reasonable limit
&& std::any_of(m_added_node_params.cbegin(), m_added_node_params.cend(),
[&](const auto& p) { return p.m_added_node == addr_str || p.m_added_node == addr_port_str; }));
}
size_t CConnman::GetNodeCount(ConnectionDirection flags) const
{
LOCK(m_nodes_mutex);

1
src/net.h
View File

@ -1184,6 +1184,7 @@ public:
bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool AddedNodesContain(const CAddress& addr) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
std::vector<AddedNodeInfo> GetAddedNodeInfo(bool include_connected) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
/**