mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-19 14:45:08 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28120: p2p: make block download logic aware of limited peers threshold

Browse Source 
c5b5843d8f test: avoid requesting blocks beyond limited peer threshold (furszy)
2f6a05512f p2p: sync from limited peer, only request blocks below threshold (furszy)
73127722a2 refactor: Make FindNextBlocks friendlier (furszy)

Pull request description:

  Even when the node believes it has IBD completed, need to avoid
  requesting historical blocks from network-limited peers.
  Otherwise, the limited peer will disconnect right away.

  The simplest scenario could be a node that gets synced, drops
  connections, and stays inactive for a while. Then, once it re-connects
  (IBD stays completed), the node tries to fetch all the missing blocks
  from any peer, getting disconnected by the limited ones.

  Note:
  Can verify the behavior by cherry-picking the test commit alone on
  master. It will fail there.

ACKs for top commit:
  achow101:
    ACK c5b5843d8f
  vasild:
    ACK c5b5843d8f
  mzumsande:
    Code Review ACK c5b5843d8f
  pinheadmz:
    ACK c5b5843d8f

Tree-SHA512: 9e550698bc6e63cc587b2b988a87d0ab555a8fa188c91c3f33287f8201d77c28b373331845356ad86f17bb21c15950b6466bc1cafd0ce8139d70364cb71c2ad2
This commit is contained in:
Ava Chow 2024-03-11 08:09:23 -04:00
commit 4a903741b0
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
2 changed files with 96 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
src/net_processing.cpp
View File

@ -1451,6 +1451,7 @@ void PeerManagerImpl::FindNextBlocks(std::vector<const CBlockIndex*>& vBlocks, c
{
std::vector<const CBlockIndex*> vToFetch;
int nMaxHeight = std::min<int>(state->pindexBestKnownBlock->nHeight, nWindowEnd + 1);
bool is_limited_peer = IsLimitedPeer(peer);
NodeId waitingfor = -1;
while (pindexWalk->nHeight < nMaxHeight) {
// Read up to 128 (or more, if more blocks than that are needed) successors of pindexWalk (towards
@ -1473,30 +1474,46 @@ void PeerManagerImpl::FindNextBlocks(std::vector<const CBlockIndex*>& vBlocks, c
// We consider the chain that this peer is on invalid.
return;
}
if (!CanServeWitnesses(peer) && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) {
// We wouldn't download this block or its descendants from this peer.
return;
}
if (pindex->nStatus & BLOCK_HAVE_DATA || (activeChain && activeChain->Contains(pindex))) {
if (activeChain && pindex->HaveNumChainTxs())
if (activeChain && pindex->HaveNumChainTxs()) {
state->pindexLastCommonBlock = pindex;
} else if (!IsBlockRequested(pindex->GetBlockHash())) {
// The block is not already downloaded, and not yet in flight.
if (pindex->nHeight > nWindowEnd) {
// We reached the end of the window.
if (vBlocks.size() == 0 && waitingfor != peer.m_id) {
// We aren't able to fetch anything, but we would be if the download window was one larger.
if (nodeStaller) *nodeStaller = waitingfor;
}
return;
}
vBlocks.push_back(pindex);
if (vBlocks.size() == count) {
return;
continue;
}
// Is block in-flight?
if (IsBlockRequested(pindex->GetBlockHash())) {
if (waitingfor == -1) {
// This is the first already-in-flight block.
waitingfor = mapBlocksInFlight.lower_bound(pindex->GetBlockHash())->second.first;
}
} else if (waitingfor == -1) {
// This is the first already-in-flight block.
waitingfor = mapBlocksInFlight.lower_bound(pindex->GetBlockHash())->second.first;
continue;
}
// The block is not already downloaded, and not yet in flight.
if (pindex->nHeight > nWindowEnd) {
// We reached the end of the window.
if (vBlocks.size() == 0 && waitingfor != peer.m_id) {
// We aren't able to fetch anything, but we would be if the download window was one larger.
if (nodeStaller) *nodeStaller = waitingfor;
}
return;
}
// Don't request blocks that go further than what limited peers can provide
if (is_limited_peer && (state->pindexBestKnownBlock->nHeight - pindex->nHeight >= static_cast<int>(NODE_NETWORK_LIMITED_MIN_BLOCKS) - 2 /* two blocks buffer for possible races */)) {
continue;
}
vBlocks.push_back(pindex);
if (vBlocks.size() == count) {
return;
}
}
}

63
test/functional/p2p_node_network_limited.py
View File

@ -21,8 +21,12 @@ from test_framework.p2p import P2PInterface
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
try_rpc
)
# Minimum blocks required to signal NODE_NETWORK_LIMITED #
NODE_NETWORK_LIMITED_MIN_BLOCKS = 288
class P2PIgnoreInv(P2PInterface):
firstAddrnServices = 0
@ -54,6 +58,63 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
self.add_nodes(self.num_nodes, self.extra_args)
self.start_nodes()
def test_avoid_requesting_historical_blocks(self):
self.log.info("Test full node does not request blocks beyond the limited peer threshold")
pruned_node = self.nodes[0]
miner = self.nodes[1]
full_node = self.nodes[2]
# Connect and generate block to ensure IBD=false
self.connect_nodes(1, 0)
self.connect_nodes(1, 2)
self.generate(miner, 1)
# Verify peers are out of IBD
for node in self.nodes:
assert not node.getblockchaininfo()['initialblockdownload']
# Isolate full_node (the node will remain out of IBD)
full_node.setnetworkactive(False)
self.wait_until(lambda: len(full_node.getpeerinfo()) == 0)
# Mine blocks and sync the pruned node. Surpass the NETWORK_NODE_LIMITED threshold.
# Blocks deeper than the threshold are considered "historical blocks"
num_historial_blocks = 12
self.generate(miner, NODE_NETWORK_LIMITED_MIN_BLOCKS + num_historial_blocks, sync_fun=self.no_op)
self.sync_blocks([miner, pruned_node])
# Connect full_node to prune_node and check peers don't disconnect right away.
# (they will disconnect if full_node, which is chain-wise behind, request blocks
# older than NODE_NETWORK_LIMITED_MIN_BLOCKS)
start_height_full_node = full_node.getblockcount()
full_node.setnetworkactive(True)
self.connect_nodes(2, 0)
assert_equal(len(full_node.getpeerinfo()), 1)
# Wait until the full_node is headers-wise sync
best_block_hash = pruned_node.getbestblockhash()
self.wait_until(lambda: next(filter(lambda x: x['hash'] == best_block_hash, full_node.getchaintips()))['status'] == "headers-only")
# Now, since the node aims to download a window of 1024 blocks,
# ensure it requests the blocks below the threshold only (with a
# 2-block buffer). And also, ensure it does not request any
# historical block.
tip_height = pruned_node.getblockcount()
limit_buffer = 2
# Prevent races by waiting for the tip to arrive first
self.wait_until(lambda: not try_rpc(-1, "Block not found", full_node.getblock, pruned_node.getbestblockhash()))
for height in range(start_height_full_node + 1, tip_height + 1):
if height <= tip_height - (NODE_NETWORK_LIMITED_MIN_BLOCKS - limit_buffer):
assert_raises_rpc_error(-1, "Block not found on disk", full_node.getblock, pruned_node.getblockhash(height))
else:
full_node.getblock(pruned_node.getblockhash(height)) # just assert it does not throw an exception
# Lastly, ensure the full_node is not sync and verify it can get synced by
# establishing a connection with another full node capable of providing them.
assert_equal(full_node.getblockcount(), start_height_full_node)
self.connect_nodes(2, 1)
self.sync_blocks([miner, full_node])
def run_test(self):
node = self.nodes[0].add_p2p_connection(P2PIgnoreInv())
@ -118,5 +179,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
# sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED)
self.sync_blocks([self.nodes[0], self.nodes[1]])
self.test_avoid_requesting_historical_blocks()
if __name__ == '__main__':
NodeNetworkLimitedTest().main()