From c711ca186f8d8a28810be0beedcb615ddcf93163 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@pm.me>
Date: Wed, 3 May 2023 15:39:51 -0400
Subject: [PATCH] assumeutxo: remove snapshot during -reindex{-chainstate}

Removing a snapshot chainstate from disk (and memory) is consistent with
existing reindex operations.
---
 src/node/chainstate.cpp |  9 ++++++++-
 src/validation.cpp      | 32 +++++++++++++++++++++++++++-----
 src/validation.h        |  7 ++++---
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index ae1457a87e..16ca1d9156 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -185,7 +185,14 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     chainman.InitializeChainstate(options.mempool);
 
     // Load a chain created from a UTXO snapshot, if any exist.
-    chainman.DetectSnapshotChainstate(options.mempool);
+    bool has_snapshot = chainman.DetectSnapshotChainstate(options.mempool);
+
+    if (has_snapshot && (options.reindex || options.reindex_chainstate)) {
+        LogPrintf("[snapshot] deleting snapshot chainstate due to reindexing\n");
+        if (!chainman.DeleteSnapshotChainstate()) {
+            return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")};
+        }
+    }
 
     auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options);
     if (init_status != ChainstateLoadStatus::SUCCESS) {
diff --git a/src/validation.cpp b/src/validation.cpp
index a57a664786..240543e6eb 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5111,7 +5111,7 @@ const AssumeutxoData* ExpectedAssumeutxo(
     return nullptr;
 }
 
-static bool DeleteCoinsDBFromDisk(const fs::path db_path, bool is_snapshot)
+[[nodiscard]] static bool DeleteCoinsDBFromDisk(const fs::path db_path, bool is_snapshot)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
     AssertLockHeld(::cs_main);
@@ -5750,15 +5750,20 @@ bool IsBIP30Unspendable(const CBlockIndex& block_index)
            (block_index.nHeight==91812 && block_index.GetBlockHash() == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"));
 }
 
-util::Result<void> Chainstate::InvalidateCoinsDBOnDisk()
+static fs::path GetSnapshotCoinsDBPath(Chainstate& cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
     AssertLockHeld(::cs_main);
     // Should never be called on a non-snapshot chainstate.
-    assert(m_from_snapshot_blockhash);
-    auto storage_path_maybe = this->CoinsDB().StoragePath();
+    assert(cs.m_from_snapshot_blockhash);
+    auto storage_path_maybe = cs.CoinsDB().StoragePath();
     // Should never be called with a non-existent storage path.
     assert(storage_path_maybe);
-    fs::path snapshot_datadir = *storage_path_maybe;
+    return *storage_path_maybe;
+}
+
+util::Result<void> Chainstate::InvalidateCoinsDBOnDisk()
+{
+    fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*this);
 
     // Coins views no longer usable.
     m_coins_views.reset();
@@ -5789,6 +5794,23 @@ util::Result<void> Chainstate::InvalidateCoinsDBOnDisk()
     return {};
 }
 
+bool ChainstateManager::DeleteSnapshotChainstate()
+{
+    AssertLockHeld(::cs_main);
+    Assert(m_snapshot_chainstate);
+    Assert(m_ibd_chainstate);
+
+    fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*m_snapshot_chainstate);
+    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_snapshot_chainstate.reset();
+    return true;
+}
+
 const CBlockIndex* ChainstateManager::GetSnapshotBaseBlock() const
 {
     return m_active_chainstate ? m_active_chainstate->SnapshotBase() : nullptr;
diff --git a/src/validation.h b/src/validation.h
index 319e40447b..4e9e91c299 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -848,9 +848,6 @@ private:
     //! Points to either the ibd or snapshot chainstate; indicates our
     //! most-work chain.
     //!
-    //! Once this pointer is set to a corresponding chainstate, it will not
-    //! be reset until init.cpp:Shutdown().
-    //!
     //! This is especially important when, e.g., calling ActivateBestChain()
     //! on all chainstates because we are not able to hold ::cs_main going into
     //! that call.
@@ -1203,6 +1200,10 @@ public:
 
     void ResetChainstates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    //! Remove the snapshot-based chainstate and all on-disk artifacts.
+    //! Used when reindex{-chainstate} is called during snapshot use.
+    [[nodiscard]] bool DeleteSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
     //! Switch the active chainstate to one based on a UTXO snapshot that was loaded
     //! previously.
     Chainstate& ActivateExistingSnapshot(CTxMemPool* mempool, uint256 base_blockhash)