diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 87c91b2dcfa6..054261c8567f 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -37,6 +37,35 @@ #include namespace node { +static bool RemoveSnapshotChainstateArtifacts(const fs::path& data_dir, bilingual_str& error) +{ + // Explicit reindexing discards both coins databases and EvoDB, so remove + // every snapshot lifecycle directory in the same stroke. A directory must + // not outlive the markers that describe it: a chainstate_snapshot dir whose + // EvoDB markers were just wiped can no longer be revived by + // ActivateExistingSnapshot(), and a reindex is also the user's request to + // discard the _INVALID forensics directory and any interrupted-swap + // remnant, neither of which the (skipped) recovery pass will see. + const fs::path normal{data_dir / "chainstate"}; + fs::path snapshot{normal}; + snapshot += SNAPSHOT_CHAINSTATE_SUFFIX; + fs::path invalid{snapshot}; + invalid += SNAPSHOT_INVALID_SUFFIX; + fs::path to_delete{normal}; + to_delete += SNAPSHOT_TODELETE_SUFFIX; + for (const auto& path : {snapshot, invalid, to_delete}) { + if (!fs::exists(path)) continue; + try { + RemoveAllDurably(path); + } catch (const fs::filesystem_error& e) { + error = strprintf(_("Failed to remove snapshot chainstate artifact %s for reindex: %s"), + fs::PathToString(path), e.what()); + return false; + } + } + return true; +} + static bool RecoverSnapshotCleanup(CEvoDB& evodb, const fs::path& data_dir, bilingual_str& error) { const fs::path normal{data_dir / "chainstate"}; @@ -326,6 +355,13 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize LOCK(cs_main); + if (options.reindex || options.reindex_chainstate) { + bilingual_str cleanup_error; + if (!RemoveSnapshotChainstateArtifacts(options.data_dir, cleanup_error)) { + return {ChainstateLoadStatus::FAILURE, cleanup_error}; + } + } + evodb.reset(); // TODO: pass DbWrapperParams as options instead multiple params evodb = std::make_unique(util::DbWrapperParams{ @@ -344,15 +380,6 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // Load the fully validated chainstate. chainman.InitializeChainstate(options.mempool, *evodb, chain_helper); - // Wiping the shared EvoDB above erased the SNAPSHOT best-block marker that - // ActivateExistingSnapshot() requires, so a persisted snapshot chainstate can - // no longer be revived. Discard it here rather than letting startup fail with - // advice ("reindex") the user has just followed, which would never recover. - if ((options.reindex || options.reindex_chainstate) && !DeleteSnapshotChainstateFromDisk()) { - return {ChainstateLoadStatus::FAILURE, - _("Failed to remove the snapshot chainstate directory. Remove it manually before restarting.")}; - } - // Load a chain created from a UTXO snapshot, if any exist. bilingual_str snapshot_error; if (!chainman.DetectSnapshotChainstate(options.mempool, snapshot_error)) { diff --git a/src/validation.cpp b/src/validation.cpp index 2d5f23772751..1e5142023b90 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5570,19 +5570,6 @@ static bool DeleteCoinsDBFromDisk(const fs::path db_path, bool is_snapshot) return destroyed && !fs::exists(db_path); } -bool DeleteSnapshotChainstateFromDisk() -{ - AssertLockHeld(::cs_main); - - auto snapshot_datadir = node::FindSnapshotChainstateDir(); - if (!snapshot_datadir) { - return true; - } - LogPrintf("[snapshot] discarding persisted snapshot chainstate at %s\n", - fs::PathToString(*snapshot_datadir)); - return DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); -} - bool ChainstateManager::ActivateSnapshot( AutoFile& coins_file, const SnapshotMetadata& metadata, diff --git a/src/validation.h b/src/validation.h index c325d8b637f2..2bab90895e41 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1294,16 +1294,6 @@ MnRewardEra GetMnRewardEraAfter(const CBlockIndex* pindexPrev, const ChainstateM */ const AssumeutxoData* ExpectedAssumeutxo(const int height, const CChainParams& params); -/** - * Remove a persisted snapshot chainstate's on-disk artifacts: its coins database - * and the base-blockhash file identifying it. Only valid while no snapshot - * Chainstate object exists, i.e. at startup before DetectSnapshotChainstate(). - * - * @returns false only if a snapshot chainstate was found but could not be fully - * removed; true when there was nothing to remove. - */ -bool DeleteSnapshotChainstateFromDisk() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** Identifies blocks that overwrote an existing coinbase output in the UTXO set (see BIP30) */ bool IsBIP30Repeat(const CBlockIndex& block_index); diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 0a12c216adbc..6b74a99e3595 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -11,6 +11,7 @@ """ import os +from pathlib import Path from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import MAGIC_BYTES from test_framework.util import assert_equal @@ -25,9 +26,19 @@ def reindex(self, justchainstate=False, txindex=0): self.generatetoaddress(self.nodes[0], 3, self.nodes[0].get_deterministic_priv_key().address) blockcount = self.nodes[0].getblockcount() self.stop_nodes() + chain_dir = Path(self.nodes[0].datadir) / self.nodes[0].chain + snapshot_artifacts = [ + chain_dir / "chainstate_snapshot", + chain_dir / "chainstate_snapshot_INVALID", + chain_dir / "chainstate_todelete", + ] + for artifact in snapshot_artifacts: + artifact.mkdir() + (artifact / "stale").touch() extra_args = [["-reindex-chainstate", "-txindex=0"]] if justchainstate else [["-reindex", f"-txindex={txindex}"]] self.start_nodes(extra_args) assert_equal(self.nodes[0].getblockcount(), blockcount) # start_node is blocking on reindex + assert all(not artifact.exists() for artifact in snapshot_artifacts) self.log.info("Success") # Check that blocks can be processed out of order