mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#32180: p2p: Advance pindexLastCommonBlock early after connecting blocks

Browse Source 
01cc20f330 test: improve coverage for a resolved stalling situation (Martin Zumsande)
9af6daf07e test: remove magic number when checking for blocks that have arrived (Martin Zumsande)
3069d66dca p2p: During block download, adjust pindexLastCommonBlock better (Martin Zumsande)

Pull request description:

  As described in #32179, `pindexLastCommonBlock` is updated later than necessary
  in master.
  In case of a linear chain with no forks, it can be moved forward at the beginning of
  `FindNextBlocksToDownload`, so that the updated value can be used to better estimate `nWindowEnd`.
  This helps the node to request all blocks from peers within the correct 1024-block-window and avoids peers being incorrectly marked as stallers.

  I also changed `p2p_ibd_stalling.py` to cover the situation after a resolved situation, making sure that no additional peers are marked for stalling.

  Fixes #32179

ACKs for top commit:
  Crypt-iQ:
    crACK 01cc20f330
  stringintech:
    re-ACK 01cc20f
  achow101:
    ACK 01cc20f330
  sipa:
    utACK 01cc20f330

Tree-SHA512: a97f7a7ef5ded538ee35576e04b3fbcdd46a6d0189c7ba3abacc6e0d81e800aac5b0c2d2565d0462ef6fd4acc751989f577fd6adfd450171a7d6ab26f437df32
This commit is contained in:
Ava Chow committed 2025-11-10 08:48:49 -08:00
2 parents 5f0303b93f 01cc20f330
commit 1fe851a478
2 files changed
+28 -34
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/net_processing.cpp
+8 -9
View File
@@ -1393,17 +1393,16 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
return;
}
// Bootstrap quickly by guessing a parent of our best tip is the forking point.
// Guessing wrong in either direction is not a problem.
// Also reset pindexLastCommonBlock after a snapshot was loaded, so that blocks after the snapshot will be prioritised for download.
// Determine the forking point between the peer's chain and our chain:
// pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock, and will be used as a starting point.
// It is being set to the fork point between the peer's best known block and the current tip, unless it is already set to
// an ancestor with more work than the fork point.
auto fork_point = LastCommonAncestor(state->pindexBestKnownBlock, m_chainman.ActiveTip());
if (state->pindexLastCommonBlock == nullptr ||
(snap_base && state->pindexLastCommonBlock->nHeight < snap_base->nHeight)) {
state->pindexLastCommonBlock = m_chainman.ActiveChain()[std::min(state->pindexBestKnownBlock->nHeight, m_chainman.ActiveChain().Height())];
fork_point->nChainWork > state->pindexLastCommonBlock->nChainWork ||
state->pindexBestKnownBlock->GetAncestor(state->pindexLastCommonBlock->nHeight) != state->pindexLastCommonBlock) {
state->pindexLastCommonBlock = fork_point;
}
// If the peer reorganized, our previous pindexLastCommonBlock may not be an ancestor
// of its current tip anymore. Go back enough to fix that.
state->pindexLastCommonBlock = LastCommonAncestor(state->pindexLastCommonBlock, state->pindexBestKnownBlock);
if (state->pindexLastCommonBlock == state->pindexBestKnownBlock)
return;
test/functional/p2p_ibd_stalling.py
+20 -25
View File
@@ -29,15 +29,15 @@ from test_framework.util import (
class P2PStaller(P2PDataStore):
def __init__(self, stall_block):
self.stall_block = stall_block
def __init__(self, stall_blocks):
self.stall_blocks = stall_blocks
super().__init__()
def on_getdata(self, message):
for inv in message.inv:
self.getdata_requests.append(inv.hash)
if (inv.type & MSG_TYPE_MASK) == MSG_BLOCK:
if (inv.hash != self.stall_block):
if (inv.hash not in self.stall_blocks):
self.send_without_ping(msg_block(self.block_store[inv.hash]))
def on_getheaders(self, message):
@@ -51,7 +51,7 @@ class P2PIBDStallingTest(BitcoinTestFramework):
def run_test(self):
NUM_BLOCKS = 1025
NUM_PEERS = 4
NUM_PEERS = 5
node = self.nodes[0]
tip = int(node.getbestblockhash(), 16)
blocks = []
@@ -66,7 +66,9 @@ class P2PIBDStallingTest(BitcoinTestFramework):
block_time += 1
height += 1
block_dict[blocks[-1].hash_int] = blocks[-1]
stall_block = blocks[0].hash_int
stall_index = 0
second_stall_index = 500
stall_blocks = [blocks[stall_index].hash_int, blocks[second_stall_index].hash_int]
headers_message = msg_headers()
headers_message.headers = [CBlockHeader(b) for b in blocks[:NUM_BLOCKS-1]]
@@ -76,14 +78,12 @@ class P2PIBDStallingTest(BitcoinTestFramework):
self.mocktime = int(time.time()) + 1
node.setmocktime(self.mocktime)
for id in range(NUM_PEERS):
peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_block), p2p_idx=id, connection_type="outbound-full-relay"))
peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_blocks), p2p_idx=id, connection_type="outbound-full-relay"))
peers[-1].block_store = block_dict
peers[-1].send_and_ping(headers_message)
# Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
# returning the number of downloaded (but not connected) blocks.
bytes_recv = 172761 if not self.options.v2transport else 169692
self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
# Wait until all blocks are received (except for the stall blocks), so that no other blocks are in flight.
self.wait_until(lambda: sum(len(peer['inflight']) for peer in node.getpeerinfo()) == len(stall_blocks))
self.all_sync_send_with_ping(peers)
# If there was a peer marked for stalling, it would get disconnected
@@ -104,7 +104,7 @@ class P2PIBDStallingTest(BitcoinTestFramework):
node.setmocktime(self.mocktime)
peers[0].wait_for_disconnect()
assert_equal(node.num_test_p2p_connections(), NUM_PEERS - 1)
self.wait_until(lambda: self.is_block_requested(peers, stall_block))
self.wait_until(lambda: self.is_block_requested(peers, stall_blocks[0]))
# Make sure that SendMessages() is invoked, which assigns the missing block
# to another peer and starts the stalling logic for them
self.all_sync_send_with_ping(peers)
@@ -119,7 +119,7 @@ class P2PIBDStallingTest(BitcoinTestFramework):
self.mocktime += 2
node.setmocktime(self.mocktime)
self.wait_until(lambda: sum(x.is_connected for x in node.p2ps) == NUM_PEERS - 2)
self.wait_until(lambda: self.is_block_requested(peers, stall_block))
self.wait_until(lambda: self.is_block_requested(peers, stall_blocks[0]))
self.all_sync_send_with_ping(peers)
self.log.info("Check that the stalling timeout gets doubled to 8 seconds for the next staller")
@@ -132,24 +132,19 @@ class P2PIBDStallingTest(BitcoinTestFramework):
self.mocktime += 2
node.setmocktime(self.mocktime)
self.wait_until(lambda: sum(x.is_connected for x in node.p2ps) == NUM_PEERS - 3)
self.wait_until(lambda: self.is_block_requested(peers, stall_block))
self.wait_until(lambda: self.is_block_requested(peers, stall_blocks[0]))
self.all_sync_send_with_ping(peers)
self.log.info("Provide the withheld block and check that stalling timeout gets reduced back to 2 seconds")
with node.assert_debug_log(expected_msgs=['Decreased stalling timeout to 2 seconds']):
self.log.info("Provide the first withheld block and check that stalling timeout gets reduced back to 2 seconds")
with node.assert_debug_log(expected_msgs=['Decreased stalling timeout to 2 seconds'], unexpected_msgs=['Stall started']):
for p in peers:
if p.is_connected and (stall_block in p.getdata_requests):
p.send_without_ping(msg_block(block_dict[stall_block]))
if p.is_connected and (stall_blocks[0] in p.getdata_requests):
p.send_without_ping(msg_block(block_dict[stall_blocks[0]]))
self.all_sync_send_with_ping(peers)
self.log.info("Check that all outstanding blocks get connected")
self.wait_until(lambda: node.getblockcount() == NUM_BLOCKS)
self.log.info("Check that all outstanding blocks up to the second stall block get connected")
self.wait_until(lambda: node.getblockcount() == second_stall_index)
def total_bytes_recv_for_blocks(self):
total = 0
for info in self.nodes[0].getpeerinfo():
if ("block" in info["bytesrecv_per_msg"].keys()):
total += info["bytesrecv_per_msg"]["block"]
return total
def all_sync_send_with_ping(self, peers):
for p in peers: