From 04caf09977109152a1b605bc54b2a09edebdf312 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Aug 2026 11:17:47 -0500 Subject: [PATCH 1/3] refactor: share snapshot lifecycle dir suffixes between writers and recovery RecoverSnapshotCleanup re-spelled the snapshot, todelete and INVALID directory names as string literals while the rename sites derived them from SNAPSHOT_CHAINSTATE_SUFFIX or inlined them. A rename of any suffix would compile cleanly while crash recovery silently stopped matching the on-disk layout. Define the INVALID and todelete suffixes next to SNAPSHOT_CHAINSTATE_SUFFIX and consume them at every producer and consumer. Co-Authored-By: Claude Fable 5 --- src/node/chainstate.cpp | 10 +++++++--- src/node/utxo_snapshot.h | 8 ++++++++ src/validation.cpp | 6 ++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index e3b59835c418..87c91b2dcfa6 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -39,9 +40,12 @@ namespace node { static bool RecoverSnapshotCleanup(CEvoDB& evodb, const fs::path& data_dir, bilingual_str& error) { const fs::path normal{data_dir / "chainstate"}; - const fs::path snapshot{data_dir / "chainstate_snapshot"}; - const fs::path to_delete{data_dir / "chainstate_todelete"}; - const fs::path invalid{data_dir / "chainstate_snapshot_INVALID"}; + fs::path snapshot{normal}; + snapshot += SNAPSHOT_CHAINSTATE_SUFFIX; + fs::path to_delete{normal}; + to_delete += SNAPSHOT_TODELETE_SUFFIX; + fs::path invalid{snapshot}; + invalid += SNAPSHOT_INVALID_SUFFIX; uint256 snapshot_tip; const bool has_snapshot_tip{evodb.ReadBestBlock(EvoDbIdentity::SNAPSHOT, snapshot_tip)}; diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index e900d4ec2b39..57e307c3c12b 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -60,6 +60,14 @@ std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) //! a snapshot. constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot"; +//! Suffix appended to the snapshot chainstate dir when the snapshot fails +//! validation and the directory is set aside for later inspection. +constexpr std::string_view SNAPSHOT_INVALID_SUFFIX = "_INVALID"; + +//! Suffix appended to the background chainstate dir while a fully validated +//! snapshot chainstate is moved into its place. +constexpr std::string_view SNAPSHOT_TODELETE_SUFFIX = "_todelete"; + //! Return a path to the snapshot-based chainstate dir, if one exists. std::optional FindSnapshotChainstateDir(); diff --git a/src/validation.cpp b/src/validation.cpp index 6200bcca314e..5c0bb0ec05f4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6404,7 +6404,8 @@ util::Result Chainstate::InvalidateCoinsDBOnDisk() // Coins views no longer usable. m_coins_views.reset(); - auto invalid_path = snapshot_datadir + "_INVALID"; + fs::path invalid_path{snapshot_datadir}; + invalid_path += node::SNAPSHOT_INVALID_SUFFIX; std::string dbpath = fs::PathToString(snapshot_datadir); std::string target = fs::PathToString(invalid_path); LogPrintf("[snapshot] renaming snapshot datadir %s to %s\n", dbpath, target); @@ -6486,7 +6487,8 @@ bool ChainstateManager::ValidatedSnapshotCleanup() LogPrintf("[snapshot] deleting background chainstate directory (now unnecessary) (%s)\n", fs::PathToString(ibd_chainstate_path)); - fs::path tmp_old{ibd_chainstate_path + "_todelete"}; + fs::path tmp_old{ibd_chainstate_path}; + tmp_old += node::SNAPSHOT_TODELETE_SUFFIX; auto rename_failed_abort = []( fs::path p_old, From 8cd7f27932a4fc42e884ae5c11eb2d2bc47ea71a Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Aug 2026 11:18:20 -0500 Subject: [PATCH 2/3] refactor: hoist chainstate enumeration out of the flag-reset admission loop The candidate-admission pass in ResetBlockFailureFlags called ChainstateManager::GetAll() for every reconsidered block, re-acquiring cs_main recursively and heap-allocating a vector per iteration even though the chainstate set cannot change while cs_main is held for the whole function. Fetch the set once before the loop. Co-Authored-By: Claude Fable 5 --- src/validation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 5c0bb0ec05f4..6e469d5a8518 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3848,9 +3848,10 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex, bool ignore_chainlo // Failure flags and m_best_invalid are shared by all chainstates, so // candidate admission must be recomputed for all of them as well. + const auto chainstates{m_chainman.GetAll()}; for (CBlockIndex* reconsidered : reconsidered_blocks) { if (!reconsidered->IsValid(BLOCK_VALID_TRANSACTIONS) || !reconsidered->HaveTxsDownloaded()) continue; - for (Chainstate* chainstate : m_chainman.GetAll()) { + for (Chainstate* chainstate : chainstates) { chainstate->TryAddBlockIndexCandidate(reconsidered); } } From d4851d8fbb8390d7e1ac4628add0d122dd61946f Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Aug 2026 11:18:39 -0500 Subject: [PATCH 3/3] chore: drop duplicated snapshot base-height assert assert(snapshot_base_height == index_new.nHeight) restated assert(index_new.nHeight == snapshot_base_height) from four lines earlier; upstream carries only the first assert plus the curr_height check. Co-Authored-By: Claude Fable 5 --- src/validation.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 6e469d5a8518..5b1e8924d8c4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6097,7 +6097,6 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( int curr_height = m_ibd_chainstate->m_chain.Height(); assert(snapshot_base_height == curr_height); - assert(snapshot_base_height == index_new.nHeight); assert(this->IsUsable(m_snapshot_chainstate.get())); assert(this->GetAll().size() == 2);