From ed871d2a079cc6cde18870a703faea07db3cf772 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 26 Jan 2022 15:59:09 -0500 Subject: [PATCH] merge bitcoin#24171: Sync chain more readily from inbound peers during IBD --- src/net_processing.cpp | 27 +++++++++++++++++++--- test/functional/p2p_block_sync.py | 37 +++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 62 insertions(+), 3 deletions(-) create mode 100755 test/functional/p2p_block_sync.py diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 81aa58a87f..bf883d4d86 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5462,10 +5462,31 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (m_chainman.m_best_header == nullptr) { m_chainman.m_best_header = m_chainman.ActiveChain().Tip(); } - bool fFetch = state.fPreferredDownload || (nPreferredDownload == 0 && !pto->fClient && !pto->IsAddrFetchConn()); // Download if this is a nice peer, or we have no nice peers and this one might do. + + // Determine whether we might try initial headers sync or parallel + // block download from this peer -- this mostly affects behavior while + // in IBD (once out of IBD, we sync from all peers). + bool sync_blocks_and_headers_from_peer = false; + if (state.fPreferredDownload) { + sync_blocks_and_headers_from_peer = true; + } else if (!pto->fClient && !pto->IsAddrFetchConn()) { + // Typically this is an inbound peer. If we don't have any outbound + // peers, or if we aren't downloading any blocks from such peers, + // then allow block downloads from this peer, too. + // We prefer downloading blocks from outbound peers to avoid + // putting undue load on (say) some home user who is just making + // outbound connections to the network, but if our only source of + // the latest blocks is from an inbound peer, we have to be sure to + // eventually download it (and not just wait indefinitely for an + // outbound peer to have it). + if (nPreferredDownload == 0 || mapBlocksInFlight.empty()) { + sync_blocks_and_headers_from_peer = true; + } + } + if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex && pto->CanRelay()) { // Only actively request headers from a single peer, unless we're close to end of initial download. - if ((nSyncStarted == 0 && fFetch) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - nMaxTipAge) { + if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - nMaxTipAge) { state.fSyncStarted = true; state.m_headers_sync_timeout = current_time + HEADERS_DOWNLOAD_TIMEOUT_BASE + ( @@ -5888,7 +5909,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Message: getdata (blocks) // std::vector vGetData; - if (!pto->fClient && pto->CanRelay() && ((fFetch && !pto->m_limited_node) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (!pto->fClient && pto->CanRelay() && ((sync_blocks_and_headers_from_peer && !pto->m_limited_node) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { std::vector vToDownload; NodeId staller = -1; FindNextBlocksToDownload(pto->GetId(), MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); diff --git a/test/functional/p2p_block_sync.py b/test/functional/p2p_block_sync.py new file mode 100755 index 0000000000..0506735971 --- /dev/null +++ b/test/functional/p2p_block_sync.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test block download + +Ensure that even in IBD, we'll eventually sync chain from inbound peers +(whether we have only inbound peers or both inbound and outbound peers). +""" + +from test_framework.test_framework import BitcoinTestFramework + +class BlockSyncTest(BitcoinTestFramework): + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 3 + + def setup_network(self): + self.setup_nodes() + # Construct a network: + # node0 -> node1 -> node2 + # So node1 has both an inbound and outbound peer. + # In our test, we will mine a block on node0, and ensure that it makes + # to to both node1 and node2. + self.connect_nodes(0, 1) + self.connect_nodes(1, 2) + + def run_test(self): + self.log.info("Setup network: node0->node1->node2") + self.log.info("Mining one block on node0 and verify all nodes sync") + self.nodes[0].generate(1) + self.log.info("Success!") + + +if __name__ == '__main__': + BlockSyncTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2a87e9c6af..3a8856d3c6 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -172,6 +172,7 @@ BASE_SCRIPTS = [ 'wallet_avoidreuse.py --descriptors', 'mempool_reorg.py', 'mempool_persist.py', + 'p2p_block_sync.py', 'wallet_multiwallet.py --legacy-wallet', 'wallet_multiwallet.py --descriptors', 'wallet_multiwallet.py --usecli',