From d35efe1efc0dbeae0667baade2a40be08511c13e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 10 Jun 2024 18:20:15 -0400 Subject: [PATCH 1/2] p2p: Start downloading historical blocks from common ancestor Otherwise, if the background tip is not an ancestor of the snapshot, blocks in between that ancestor up to the height of the background tip will never be requested. Co-authored-by: Martin Zumsande Co-authored-by: Alfonso Roman Zubeldia <19962151+alfonsoromanz@users.noreply.github.com> --- src/net_processing.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6374cb52c1..babff0796f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -6293,10 +6293,13 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // before the background chainstate to prioritize getting to network tip. FindNextBlocksToDownload(*peer, get_inflight_budget(), vToDownload, staller); if (m_chainman.BackgroundSyncInProgress() && !IsLimitedPeer(*peer)) { + // If the background tip is not an ancestor of the snapshot block, + // we need to start requesting blocks from their last common ancestor. + const CBlockIndex *from_tip = LastCommonAncestor(m_chainman.GetBackgroundSyncTip(), m_chainman.GetSnapshotBaseBlock()); TryDownloadingHistoricalBlocks( *peer, get_inflight_budget(), - vToDownload, m_chainman.GetBackgroundSyncTip(), + vToDownload, from_tip, Assert(m_chainman.GetSnapshotBaseBlock())); } for (const CBlockIndex *pindex : vToDownload) { From 5b7f70ba2661a194a3c476b81e03785feddb4e1c Mon Sep 17 00:00:00 2001 From: Alfonso Roman Zubeldia <19962151+alfonsoromanz@users.noreply.github.com> Date: Wed, 19 Jun 2024 15:07:55 +0200 Subject: [PATCH 2/2] test: loadtxoutset in divergent chain with less work --- test/functional/feature_assumeutxo.py | 35 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 658eea0a0e..0edd9af680 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -21,7 +21,6 @@ Interesting test cases could be loading an assumeutxo snapshot file with: Interesting starting states could be loading a snapshot when the current chain tip is: - TODO: An ancestor of snapshot block -- TODO: Not an ancestor of the snapshot block but has less work - TODO: The snapshot block - TODO: A descendant of the snapshot block - TODO: Not an ancestor or a descendant of the snapshot block and has more work @@ -51,18 +50,19 @@ class AssumeutxoTest(BitcoinTestFramework): def set_test_params(self): """Use the pregenerated, deterministic chain up to height 199.""" - self.num_nodes = 3 + self.num_nodes = 4 self.rpc_timeout = 120 self.extra_args = [ [], ["-fastprune", "-prune=1", "-blockfilterindex=1", "-coinstatsindex=1"], ["-persistmempool=0","-txindex=1", "-blockfilterindex=1", "-coinstatsindex=1"], + [] ] def setup_network(self): """Start with the nodes disconnected so that one can generate a snapshot including blocks the other hasn't yet seen.""" - self.add_nodes(3) + self.add_nodes(4) self.start_nodes(extra_args=self.extra_args) def test_invalid_snapshot_scenarios(self, valid_snapshot_path): @@ -204,6 +204,29 @@ class AssumeutxoTest(BitcoinTestFramework): assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", node.loadtxoutset, dump_output_path) self.restart_node(0, extra_args=self.extra_args[0]) + def test_snapshot_in_a_divergent_chain(self, dump_output_path): + n0 = self.nodes[0] + n3 = self.nodes[3] + assert_equal(n0.getblockcount(), FINAL_HEIGHT) + assert_equal(n3.getblockcount(), START_HEIGHT) + + self.log.info("Check importing a snapshot where current chain-tip is not an ancestor of the snapshot block but has less work") + # Generate a divergent chain in n3 up to 298 + self.generate(n3, nblocks=99, sync_fun=self.no_op) + assert_equal(n3.getblockcount(), SNAPSHOT_BASE_HEIGHT - 1) + + # Try importing the snapshot and assert its success + loaded = n3.loadtxoutset(dump_output_path) + assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT) + normal, snapshot = n3.getchainstates()["chainstates"] + assert_equal(normal['blocks'], START_HEIGHT + 99) + assert_equal(snapshot['blocks'], SNAPSHOT_BASE_HEIGHT) + + # Now lets sync the nodes and wait for the background validation to finish + self.connect_nodes(0, 3) + self.sync_blocks(nodes=(n0, n3)) + self.wait_until(lambda: len(n3.getchainstates()['chainstates']) == 1) + def run_test(self): """ Bring up two (disconnected) nodes, mine some new blocks on the first, @@ -215,6 +238,7 @@ class AssumeutxoTest(BitcoinTestFramework): n0 = self.nodes[0] n1 = self.nodes[1] n2 = self.nodes[2] + n3 = self.nodes[3] self.mini_wallet = MiniWallet(n0) @@ -265,6 +289,7 @@ class AssumeutxoTest(BitcoinTestFramework): # block. n1.submitheader(block) n2.submitheader(block) + n3.submitheader(block) # Ensure everyone is seeing the same headers. for n in self.nodes: @@ -455,7 +480,7 @@ class AssumeutxoTest(BitcoinTestFramework): self.connect_nodes(0, 2) self.wait_until(lambda: n2.getchainstates()['chainstates'][-1]['blocks'] == FINAL_HEIGHT) - self.sync_blocks() + self.sync_blocks(nodes=(n0, n2)) self.log.info("Ensuring background validation completes") self.wait_until(lambda: len(n2.getchainstates()['chainstates']) == 1) @@ -492,6 +517,8 @@ class AssumeutxoTest(BitcoinTestFramework): self.connect_nodes(0, 2) self.wait_until(lambda: n2.getblockcount() == FINAL_HEIGHT) + self.test_snapshot_in_a_divergent_chain(dump_output['path']) + @dataclass class Block: hash: str