From ac71fe936da290adf5a3155fe8db5f78b485f1f1 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Fri, 4 Sep 2020 09:34:26 -0400 Subject: [PATCH 1/2] [doc] Clarify scope of eviction protection of outbound block-relay peers Block-relay-only peers were introduced by #15759. According to its author, it was intented to make them only immune to outbound peer rotation-based eviction and not from all eviction as modified comment leans to think of. Clearly indicate that outbound block-relay peers aren't protected from eviction by the bad/lagging chain logic. --- src/net_processing.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ce4ac3cd7..6762a94a6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2018,11 +2018,12 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan } } + // If this is an outbound full-relay peer, check to see if we should protect + // it from the bad/lagging chain logic. + // Note that outbound block-relay peers are excluded from this protection, and + // thus always subject to eviction under the bad/lagging chain logic. + // See ChainSyncTimeoutState. if (!pfrom.fDisconnect && pfrom.IsFullOutboundConn() && nodestate->pindexBestKnownBlock != nullptr) { - // If this is an outbound full-relay peer, check to see if we should protect - // it from the bad/lagging chain logic. - // Note that block-relay-only peers are already implicitly protected, so we - // only consider setting m_protect for the full-relay peers. if (g_outbound_peers_with_protect_from_disconnect < MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT && nodestate->pindexBestKnownBlock->nChainWork >= ::ChainActive().Tip()->nChainWork && !nodestate->m_chain_sync.m_protect) { LogPrint(BCLog::NET, "Protecting outbound peer=%d from eviction\n", pfrom.GetId()); nodestate->m_chain_sync.m_protect = true; From d76925478efd35e6fd835370639f2139b28381e4 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Fri, 4 Sep 2020 14:33:00 -0400 Subject: [PATCH 2/2] [doc] Clarify semantic of peer's m_protect w.r.t to outbound eviction logics The field m_protect is used to protect from eviction both by bad/lagging chain and extra outbound peers logics. Outbound block-relay peers are always excluded from this protection. --- src/net_processing.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6762a94a6..37eed95b6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -321,10 +321,17 @@ struct CNodeState { */ bool fSupportsDesiredCmpctVersion; - /** State used to enforce CHAIN_SYNC_TIMEOUT - * Only in effect for outbound, non-manual, full-relay connections, with - * m_protect == false - * Algorithm: if a peer's best known block has less work than our tip, + /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. + * + * Both are only in effect for outbound, non-manual, non-protected connections. + * Any peer protected (m_protect = true) is not chosen for eviction. A peer is + * marked as protected if all of these are true: + * - its connection type is IsBlockOnlyConn() == false + * - it gave us a valid connecting header + * - we haven't reached MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT yet + * - it has a better chain than we have + * + * CHAIN_SYNC_TIMEOUT: if a peer's best known block has less work than our tip, * set a timeout CHAIN_SYNC_TIMEOUT seconds in the future: * - If at timeout their best known block now has more work than our tip * when the timeout was set, then either reset the timeout or clear it @@ -334,6 +341,9 @@ struct CNodeState { * and set a shorter timeout, HEADERS_RESPONSE_TIME seconds in future. * If their best known block is still behind when that new timeout is * reached, disconnect. + * + * EXTRA_PEER_CHECK_INTERVAL: after each interval, if we have too many outbound peers, + * drop the outbound one that least recently announced us a new block. */ struct ChainSyncTimeoutState { //! A timeout used for checking whether our peer has sufficiently synced