Merge bitcoin#29726: assumeutxo: Fix -reindex before snapshot was validated

b7ba60f test: add coverage for -reindex and assumeutxo (Martin Zumsande)
e57f951 init, validation: Fix -reindex option with an existing snapshot (Martin Zumsande)

Pull request description:

  In c711ca1 logic was introduced that `-reindex` and `-reindex-chainstate` will delete the snapshot chainstate.
  This doesn't work currently, instead of deleting the snapshot chainstate the node crashes with an assert (this can be triggered by applying the added test commit on master).
  Fix this, and another bug that would prevent the new active chainstate from having a mempool after `-reindex` has deleted the snapshot (also covered by the test).

ACKs for top commit:
  fjahr:
    re-ACK b7ba60f
  hernanmarino:
    crACK b7ba60f . Good fix
  BrandonOdiwuor:
    re-ACK b7ba60f
  byaye:
    Tested ACK b7ba60f

Tree-SHA512: c168f36997d7677d590af37b10427870f5d30123abf1c76032a16661e486735373bfa7e049e6aca439526fbcb6d619f970bf9d042196c851bf058a75a32fafdc

Author: ryanofsky

src/validation.cpp

@@ -6142,13 +6142,14 @@ bool ChainstateManager::DeleteSnapshotChainstate()
     Assert(m_snapshot_chainstate);
     Assert(m_ibd_chainstate);
 
-    fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*m_snapshot_chainstate);
+    fs::path snapshot_datadir = Assert(node::FindSnapshotChainstateDir(m_options.datadir)).value();
     if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) {
         LogPrintf("Deletion of %s failed. Please remove it manually to continue reindexing.\n",
                   fs::PathToString(snapshot_datadir));
         return false;
     }
     m_active_chainstate = m_ibd_chainstate.get();
+    m_active_chainstate->m_mempool = m_snapshot_chainstate->m_mempool;
     m_snapshot_chainstate.reset();
     return true;
 }

test/functional/feature_assumeutxo.py

@@ -11,9 +11,6 @@
 
 ## Possible test improvements
 
-- TODO: test what happens with -reindex and -reindex-chainstate before the
-      snapshot is validated, and make sure it's deleted successfully.
-
 Interesting test cases could be loading an assumeutxo snapshot file with:
 
 - TODO: Valid hash but invalid snapshot file (bad coin height or
@@ -379,6 +376,17 @@ def check_tx_counts(final: bool) -> None:
         assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT)
         assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT)
 
+        for reindex_arg in ['-reindex=1', '-reindex-chainstate=1']:
+            self.log.info(f"Check that restarting with {reindex_arg} will delete the snapshot chainstate")
+            self.restart_node(2, extra_args=[reindex_arg, *self.extra_args[2]])
+            assert_equal(1, len(n2.getchainstates()["chainstates"]))
+            for i in range(1, 300):
+                block = n0.getblock(n0.getblockhash(i), 0)
+                n2.submitheader(block)
+            loaded = n2.loadtxoutset(dump_output['path'])
+            assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT)
+            assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT)
+
         normal, snapshot = n2.getchainstates()['chainstates']
         assert_equal(normal['blocks'], START_HEIGHT)
         assert_equal(normal.get('snapshot_blockhash'), None)