Merge bitcoin/bitcoin#29996: Assumeutxo: bugfix on loadtxoutset with a divergent chain + test

5b7f70ba2661a194a3c476b81e03785feddb4e1c test: loadtxoutset in divergent chain with less work (Alfonso Roman Zubeldia)
d35efe1efc0dbeae0667baade2a40be08511c13e p2p: Start downloading historical blocks from common ancestor (Martin Zumsande)

Pull request description:

  This PR adds a test to cover the scenario of loading an assumeutxo snapshot when the current chain tip is not an ancestor of the snapshot block but has less work.

  During the review process, a bug was discovered where blocks between the last common ancestor and the background tip were not being requested if the background tip was not an ancestor of the snapshot block. mzumsande suggested a fix (65343ec49a6b73c4197dfc38e1c2f433b0a3838a) to start downloading historical blocks from the last common ancestor to address this issue. This fix has been incorporated into the PR with a slight modification.

  Related to https://github.com/bitcoin/bitcoin/issues/28648

ACKs for top commit:
  fjahr:
    tACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c
  achow101:
    ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c
  mzumsande:
    Code Review ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c

Tree-SHA512: f8957349686a6a1292165ea9e0fd8c912d21466072632a10f8ef9d852a5f430bc6b2a531e6884a4dbf2e3adb28b3d512b25919e78f5804a67320ef54c3b1aaf6

1 files changed, 31 insertions, 4 deletions

diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index 8bbb0b157c..688e2866b2 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
@@ -52,18 +51,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):
@@ -218,6 +218,29 @@ class AssumeutxoTest(BitcoinTestFramework):
             assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path)
             node.reconsiderblock(block_hash)
 
+    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,
@@ -229,6 +252,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)
 
@@ -279,6 +303,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:
@@ -484,7 +509,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)
@@ -521,6 +546,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