This commit is contained in:
Martin Zumsande 2024-04-29 04:29:07 +02:00 committed by GitHub
commit 03d8b4ad36
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 73 additions and 20 deletions

View File

@ -2347,12 +2347,13 @@ void CConnman::StartExtraBlockRelayPeers()
}
// Return the number of peers we have over our outbound connection limit
// Returns 0 if we are at the limit, and a negative number if below.
// Exclude peers that are marked for disconnect, or are going to be
// disconnected soon (eg ADDR_FETCH and FEELER)
// Also exclude peers that haven't finished initial connection handshake yet
// (so that we don't decide we're over our desired connection limit, and then
// evict some peer that has finished the handshake)
int CConnman::GetExtraFullOutboundCount() const
int CConnman::GetFullOutboundDelta() const
{
int full_outbound_peers = 0;
{
@ -2363,7 +2364,7 @@ int CConnman::GetExtraFullOutboundCount() const
}
}
}
return std::max(full_outbound_peers - m_max_outbound_full_relay, 0);
return full_outbound_peers - m_max_outbound_full_relay;
}
int CConnman::GetExtraBlockRelayCount() const

View File

@ -1132,7 +1132,7 @@ public:
void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
using NodeFn = std::function<void(CNode*)>;
void ForEachNode(const NodeFn& func)
void ForEachFullyConnectedNode(const NodeFn& func)
{
LOCK(m_nodes_mutex);
for (auto&& node : m_nodes) {
@ -1141,7 +1141,7 @@ public:
}
};
void ForEachNode(const NodeFn& func) const
void ForEachFullyConnectedNode(const NodeFn& func) const
{
LOCK(m_nodes_mutex);
for (auto&& node : m_nodes) {
@ -1181,7 +1181,8 @@ public:
// return a value less than (num_outbound_connections - num_outbound_slots)
// in cases where some outbound connections are not yet fully connected, or
// not yet fully disconnected.
int GetExtraFullOutboundCount() const;
// Returns 0 if we are at the target, and a negative number if below.
int GetFullOutboundDelta() const;
// Count the number of block-relay-only peers we have over our limit.
int GetExtraBlockRelayCount() const;

View File

@ -2094,10 +2094,10 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha
m_most_recent_block_txs = std::move(most_recent_block_txs);
}
m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
m_connman.ForEachFullyConnectedNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
AssertLockHeld(::cs_main);
if (pnode->GetCommonVersion() < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect)
if (pnode->GetCommonVersion() < INVALID_CB_NO_BAN_VERSION)
return;
ProcessBlockAvailability(pnode->GetId());
CNodeState &state = *State(pnode->GetId());
@ -5219,8 +5219,8 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now)
if (m_connman.GetExtraBlockRelayCount() > 0) {
std::pair<NodeId, std::chrono::seconds> youngest_peer{-1, 0}, next_youngest_peer{-1, 0};
m_connman.ForEachNode([&](CNode* pnode) {
if (!pnode->IsBlockOnlyConn() || pnode->fDisconnect) return;
m_connman.ForEachFullyConnectedNode([&](CNode* pnode) {
if (!pnode->IsBlockOnlyConn()) return;
if (pnode->GetId() > youngest_peer.first) {
next_youngest_peer = youngest_peer;
youngest_peer.first = pnode->GetId();
@ -5256,7 +5256,8 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now)
}
// Check whether we have too many outbound-full-relay peers
if (m_connman.GetExtraFullOutboundCount() > 0) {
int full_outbound_delta{m_connman.GetFullOutboundDelta()};
if (full_outbound_delta > 0) {
// If we have more outbound-full-relay peers than we target, disconnect one.
// Pick the outbound-full-relay peer that least recently announced
// us a new block, with ties broken by choosing the more recent
@ -5266,12 +5267,11 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now)
NodeId worst_peer = -1;
int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max();
m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) {
m_connman.ForEachFullyConnectedNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) {
AssertLockHeld(::cs_main);
// Only consider outbound-full-relay peers that are not already
// marked for disconnection
if (!pnode->IsFullOutboundConn() || pnode->fDisconnect) return;
// Only consider outbound-full-relay peers
if (!pnode->IsFullOutboundConn()) return;
CNodeState *state = State(pnode->GetId());
if (state == nullptr) return; // shouldn't be possible, but just in case
// Don't evict our protected peers
@ -5314,6 +5314,27 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now)
}
}
}
// If we are at the max full outbound limit but not above, check if any peers
// don't support tx relay (they might be in -blocksonly mode) - if so,
// evict the newest of them (highest Node Id) so we can fill the slot with a tx-relaying outbound peer
// This is not done if we are in -blocksonly mode ourselves or still in IBD,
// because then we don't care if our peer supports tx relay.
else if (full_outbound_delta == 0 && !m_opts.ignore_incoming_txs && !m_chainman.IsInitialBlockDownload()) {
std::optional<NodeId> node_to_evict;
m_connman.ForEachFullyConnectedNode([&node_to_evict](CNode* node) {
if (!node->IsFullOutboundConn() || node->m_relays_txs) return;
if (!node_to_evict.has_value() || *node_to_evict < node->GetId()) {
node_to_evict = node->GetId();
}
});
if (node_to_evict.has_value()) {
LogPrint(BCLog::NET, "disconnecting full outbound peer not participating in tx relay: peer=%d\n", *node_to_evict);
m_connman.ForNode(*node_to_evict, [](CNode* node) {
node->fDisconnect = true;
return true;
});
}
}
}
void PeerManagerImpl::CheckForStaleTipAndEvictPeers()

View File

@ -85,7 +85,7 @@ FUZZ_TARGET(connman, .init = initialize_connman)
connman.DisconnectNode(random_subnet);
},
[&] {
connman.ForEachNode([](auto) {});
connman.ForEachFullyConnectedNode([](auto) {});
},
[&] {
(void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral<NodeId>(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); });
@ -129,7 +129,7 @@ FUZZ_TARGET(connman, .init = initialize_connman)
});
}
(void)connman.GetAddedNodeInfo(fuzzed_data_provider.ConsumeBool());
(void)connman.GetExtraFullOutboundCount();
(void)connman.GetFullOutboundDelta();
(void)connman.GetLocalServices();
assert(connman.GetMaxOutboundTarget() == max_outbound_limit);
(void)connman.GetMaxOutboundTimeframe();

View File

@ -22,6 +22,7 @@ class P2PBlocksOnly(BitcoinTestFramework):
self.miniwallet = MiniWallet(self.nodes[0])
self.blocksonly_mode_tests()
self.blocksonly_peer_tests()
self.blocks_relay_conn_tests()
def blocksonly_mode_tests(self):
@ -78,6 +79,34 @@ class P2PBlocksOnly(BitcoinTestFramework):
self.nodes[0].disconnect_p2ps()
self.generate(self.nodes[0], 1)
# Tests with an outbound peer that sets txrelay to false (which they do if they are in blocksonly mode)
def blocksonly_peer_tests(self):
self.restart_node(0, ["-noblocksonly"]) # disables blocks only mode
self.log.info("Check that we sustain full-relay outbound connections to blocksonly peers when not at the full outbound limit")
blocksonly_peer = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, txrelay=False)
# Trigger CheckForStaleTipAndEvictPeers, which is scheduled every 45s (EXTRA_PEER_CHECK_INTERVAL)
self.nodes[0].mockscheduler(46)
blocksonly_peer.sync_with_ping()
blocksonly_peer.peer_disconnect()
self.log.info("Check that we evict full-relay outbound connections to blocksonly peers at the full outbound limit")
# Simulate the situation of being at the max limit of full-outbound connections by setting -maxconnections to a low value
self.restart_node(0, ["-noblocksonly", "-maxconnections=2"])
# Have one blocksonly peer that is not being disconnected
blocksonly_peer1 = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, txrelay=False)
blocksonly_peer2 = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=2, txrelay=False)
self.nodes[0].mockscheduler(46)
blocksonly_peer2.wait_for_disconnect()
blocksonly_peer1.sync_with_ping()
blocksonly_peer1.peer_disconnect()
self.log.info("Check that we don't evict full-relay outbound connections to blocksonly peers if we are blocksonly ourselves")
self.restart_node(0, ["-blocksonly", "-maxconnections=1"])
blocksonly_peer = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, txrelay=False)
self.nodes[0].mockscheduler(46)
blocksonly_peer.sync_with_ping()
blocksonly_peer.peer_disconnect()
def blocks_relay_conn_tests(self):
self.log.info('Tests with node in normal mode with block-relay-only connections')
self.restart_node(0, ["-noblocksonly"]) # disables blocks only mode

View File

@ -462,12 +462,12 @@ class P2PInterface(P2PConnection):
# If the peer supports wtxid-relay
self.wtxidrelay = wtxidrelay
def peer_connect_send_version(self, services):
def peer_connect_send_version(self, services, txrelay):
# Send a version msg
vt = msg_version()
vt.nVersion = P2P_VERSION
vt.strSubVer = P2P_SUBVERSION
vt.relay = P2P_VERSION_RELAY
vt.relay = P2P_VERSION_RELAY if txrelay else 0
vt.nServices = services
vt.addrTo.ip = self.dstaddr
vt.addrTo.port = self.dstport
@ -479,13 +479,14 @@ class P2PInterface(P2PConnection):
create_conn = super().peer_connect(**kwargs)
if send_version:
self.peer_connect_send_version(services)
self.peer_connect_send_version(services, txrelay=True)
return create_conn
def peer_accept_connection(self, *args, services=P2P_SERVICES, **kwargs):
txrelay = kwargs.pop('txrelay', True)
create_conn = super().peer_accept_connection(*args, **kwargs)
self.peer_connect_send_version(services)
self.peer_connect_send_version(services, txrelay)
return create_conn