mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-01-31 02:31:07 +00:00
Merge bitcoin/bitcoin#33604: p2p: Allow block downloads from peers without snapshot block after assumeutxo validation
7d9e1a810239a65a153c35f0f94490560441db49 test: Verify peer usage after assumeutxo validation completes (stringintech)
0067abe153298ce9f14262a15533033e6e907f2b p2p: Allow block downloads from peers without snapshot block after assumeutxo validation (stringintech)
Pull request description:
Currently, after assumeutxo background validation finishes, the node continues to skip peers that don't have the snapshot block in their best chain until restart. This unnecessarily excludes peers from block downloads even though the background sync has completed and undo data is available.
The restriction persists because `m_chainman.CurrentChainstate().SnapshotBase()` continues to return the snapshot base block until restart, even after validation completes. Added `m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED` check to only apply the peer restriction while background validation is ongoing.
Also added test coverage in `feature_assumeutxo.py` that verifies peers without the snapshot block can be used for block downloads after background validation completes. The test fails without this fix.
ACKs for top commit:
fjahr:
Re-ACK 7d9e1a810239a65a153c35f0f94490560441db49
achow101:
ACK 7d9e1a810239a65a153c35f0f94490560441db49
sedited:
Re-ACK 7d9e1a810239a65a153c35f0f94490560441db49
Tree-SHA512: 5515971da7bf7efc55eecdf03686f44c20c9e52dd168e7cfa119032d6a8ebccee69df7143075e4e9d0a01426cd9ae7202dce5c00919a82478ebf49a15dc0fe19
This commit is contained in:
commit
5401e673d5
@ -1406,11 +1406,13 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
|
||||
return;
|
||||
}
|
||||
|
||||
// When we sync with AssumeUtxo and discover the snapshot is not in the peer's best chain, abort:
|
||||
// We can't reorg to this chain due to missing undo data until the background sync has finished,
|
||||
// When syncing with AssumeUtxo and the snapshot has not yet been validated,
|
||||
// abort downloading blocks from peers that don't have the snapshot block in their best chain.
|
||||
// We can't reorg to this chain due to missing undo data until validation completes,
|
||||
// so downloading blocks from it would be futile.
|
||||
const CBlockIndex* snap_base{m_chainman.CurrentChainstate().SnapshotBase()};
|
||||
if (snap_base && state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
|
||||
if (snap_base && m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED &&
|
||||
state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
|
||||
LogDebug(BCLog::NET, "Not downloading blocks from peer=%d, which doesn't have the snapshot block in its best chain.\n", peer.m_id);
|
||||
return;
|
||||
}
|
||||
|
||||
@ -347,6 +347,29 @@ class AssumeutxoTest(BitcoinTestFramework):
|
||||
assert 'NETWORK' in ibd_node.getpeerinfo()[0]['servicesnames']
|
||||
self.sync_blocks(nodes=(ibd_node, snapshot_node))
|
||||
|
||||
def test_sync_to_most_work_chain_after_background_validation(self):
|
||||
"""
|
||||
After background validation completes, node should be able
|
||||
to download and process blocks from peers without the snapshot block in their chain.
|
||||
"""
|
||||
self.log.info("Testing sync to the most-work chain without the snapshot block after background validation")
|
||||
|
||||
forking_node = self.nodes[0]
|
||||
snapshot_node = self.nodes[2] # Has already completed background validation
|
||||
|
||||
self.log.info("Forking node switches to an alternative chain that forks one block before the snapshot block")
|
||||
fork_point = SNAPSHOT_BASE_HEIGHT - 1
|
||||
forking_node_old_height = forking_node.getblockcount()
|
||||
forking_node_old_chainwork = int(forking_node.getblockchaininfo()['chainwork'], 16)
|
||||
forking_node.invalidateblock(forking_node.getblockhash(fork_point + 1))
|
||||
|
||||
self.log.info("Mine one more block than original chain to make the new chain have most work")
|
||||
self.generate(forking_node, nblocks=(forking_node_old_height - fork_point) + 1, sync_fun=self.no_op)
|
||||
assert int(forking_node.getblockchaininfo()['chainwork'], 16) > forking_node_old_chainwork
|
||||
|
||||
self.log.info("Snapshot node should reorg to the most-work chain without the snapshot block")
|
||||
self.sync_blocks(nodes=(snapshot_node, forking_node))
|
||||
|
||||
def assert_only_network_limited_service(self, node):
|
||||
node_services = node.getnetworkinfo()['localservicesnames']
|
||||
assert 'NETWORK' not in node_services
|
||||
@ -775,6 +798,8 @@ class AssumeutxoTest(BitcoinTestFramework):
|
||||
# The following test cleans node2 and node3 chain directories.
|
||||
self.test_sync_from_assumeutxo_node(snapshot=dump_output)
|
||||
|
||||
self.test_sync_to_most_work_chain_after_background_validation()
|
||||
|
||||
@dataclass
|
||||
class Block:
|
||||
hash: str
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user