diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 608f61f07c6..b3f08cb20d6 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -214,7 +214,7 @@ struct LevelDBContext { }; CDBWrapper::CDBWrapper(const DBParams& params) - : m_db_context{std::make_unique()}, m_name{fs::PathToString(params.path.stem())}, m_path{params.path}, m_is_memory{params.memory_only} + : m_db_context{std::make_unique()}, m_name{fs::PathToString(params.path.stem())} { DBContext().penv = nullptr; DBContext().readoptions.verify_checksums = true; diff --git a/src/dbwrapper.h b/src/dbwrapper.h index c770df8e206..b2ce67c7c2e 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -191,12 +191,6 @@ private: //! obfuscation key storage key, null-prefixed to avoid collisions inline static const std::string OBFUSCATION_KEY{"\000obfuscate_key", 14}; // explicit size to avoid truncation at leading \0 - //! path to filesystem storage - const fs::path m_path; - - //! whether or not the database resides in memory - bool m_is_memory; - std::optional ReadImpl(std::span key) const; bool ExistsImpl(std::span key) const; size_t EstimateSizeImpl(std::span key1, std::span key2) const; @@ -237,14 +231,6 @@ public: WriteBatch(batch, fSync); } - //! @returns filesystem path to the on-disk data. - std::optional StoragePath() { - if (m_is_memory) { - return {}; - } - return m_path; - } - template bool Exists(const K& key) const { diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 5a03e581626..bba518d1825 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -202,7 +202,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // do nothing; expected case } else if (snapshot_completion == SnapshotCompletionResult::SUCCESS) { LogInfo("[snapshot] cleaning up unneeded background chainstate, then reinitializing"); - if (!chainman.ValidatedSnapshotCleanup()) { + if (!chainman.ValidatedSnapshotCleanup(validated_cs, *assumeutxo_cs)) { return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Background chainstate cleanup failed unexpectedly.")}; } diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index e159d9e0213..abd7bcc19b6 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -25,7 +25,7 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) AssertLockHeld(::cs_main); assert(snapshot_chainstate.m_from_snapshot_blockhash); - const std::optional chaindir = snapshot_chainstate.CoinsDB().StoragePath(); + const std::optional chaindir = snapshot_chainstate.StoragePath(); assert(chaindir); // Sanity check that chainstate isn't in-memory. const fs::path write_to = *chaindir / node::SNAPSHOT_BLOCKHASH_FILENAME; diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index aac7816e0e3..2d5919c2a8f 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -81,7 +81,7 @@ CreateAndActivateUTXOSnapshot( Chainstate& chain = node.chainman->ActiveChainstate(); Assert(chain.LoadGenesisBlock()); // These cache values will be corrected shortly in `MaybeRebalanceCaches`. - chain.InitCoinsDB(1 << 20, true, false, ""); + chain.InitCoinsDB(1 << 20, /*in_memory=*/true, /*should_wipe=*/false); chain.InitCoinsCache(1 << 20); chain.CoinsTip().SetBestBlock(gen_hash); chain.setBlockIndexCandidates.insert(node.chainman->m_blockman.LookupBlockIndex(gen_hash)); diff --git a/src/txdb.h b/src/txdb.h index ea0cf9d77e5..89e94473333 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -53,9 +53,6 @@ public: //! Dynamically alter the underlying leveldb cache size. void ResizeCache(size_t new_cache_size) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - - //! @returns filesystem path to on-disk storage or std::nullopt if in memory. - std::optional StoragePath() { return m_db->StoragePath(); } }; #endif // BITCOIN_TXDB_H diff --git a/src/validation.cpp b/src/validation.cpp index 15981708f58..a9bf12b5495 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1885,6 +1885,15 @@ Chainstate::Chainstate( m_assumeutxo(from_snapshot_blockhash ? Assumeutxo::UNVALIDATED : Assumeutxo::VALIDATED), m_from_snapshot_blockhash(from_snapshot_blockhash) {} +fs::path Chainstate::StoragePath() const +{ + fs::path path{m_chainman.m_options.datadir / "chainstate"}; + if (m_from_snapshot_blockhash) { + path += node::SNAPSHOT_CHAINSTATE_SUFFIX; + } + return path; +} + const CBlockIndex* Chainstate::SnapshotBase() const { if (!m_from_snapshot_blockhash) return nullptr; @@ -1918,16 +1927,11 @@ void Chainstate::SetTargetBlockHash(uint256 block_hash) void Chainstate::InitCoinsDB( size_t cache_size_bytes, bool in_memory, - bool should_wipe, - fs::path leveldb_name) + bool should_wipe) { - if (m_from_snapshot_blockhash) { - leveldb_name += node::SNAPSHOT_CHAINSTATE_SUFFIX; - } - m_coins_views = std::make_unique( DBParams{ - .path = m_chainman.m_options.datadir / leveldb_name, + .path = StoragePath(), .cache_bytes = cache_size_bytes, .memory_only = in_memory, .wipe_data = should_wipe, @@ -5757,7 +5761,7 @@ util::Result ChainstateManager::ActivateSnapshot( LOCK(::cs_main); snapshot_chainstate->InitCoinsDB( static_cast(current_coinsdb_cache_size * SNAPSHOT_CACHE_PERC), - in_memory, false, "chainstate"); + in_memory, /*should_wipe=*/false); snapshot_chainstate->InitCoinsCache( static_cast(current_coinstip_cache_size * SNAPSHOT_CACHE_PERC)); } @@ -6298,46 +6302,34 @@ bool IsBIP30Unspendable(const uint256& block_hash, int block_height) (block_height==91812 && block_hash == uint256{"00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"}); } -static fs::path GetSnapshotCoinsDBPath(Chainstate& cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) -{ - AssertLockHeld(::cs_main); - // Should never be called on a non-snapshot chainstate. - 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); - return *storage_path_maybe; -} - util::Result Chainstate::InvalidateCoinsDBOnDisk() { - fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*this); + // Should never be called on a non-snapshot chainstate. + assert(m_from_snapshot_blockhash); // Coins views no longer usable. m_coins_views.reset(); - auto invalid_path = snapshot_datadir + "_INVALID"; - std::string dbpath = fs::PathToString(snapshot_datadir); - std::string target = fs::PathToString(invalid_path); - LogInfo("[snapshot] renaming snapshot datadir %s to %s", dbpath, target); + const fs::path db_path{StoragePath()}; + const fs::path invalid_path{db_path + "_INVALID"}; + const std::string db_path_str{fs::PathToString(db_path)}; + const std::string invalid_path_str{fs::PathToString(invalid_path)}; + LogInfo("[snapshot] renaming snapshot datadir %s to %s", db_path_str, invalid_path_str); - // The invalid snapshot datadir is simply moved and not deleted because we may + // The invalid storage directory is simply moved and not deleted because we may // want to do forensics later during issue investigation. The user is instructed // accordingly in MaybeValidateSnapshot(). try { - fs::rename(snapshot_datadir, invalid_path); + fs::rename(db_path, invalid_path); } catch (const fs::filesystem_error& e) { - auto src_str = fs::PathToString(snapshot_datadir); - auto dest_str = fs::PathToString(invalid_path); - LogError("While invalidating the coins db: Error renaming file '%s' -> '%s': %s", - src_str, dest_str, e.what()); + db_path_str, invalid_path_str, e.what()); return util::Error{strprintf(_( "Rename of '%s' -> '%s' failed. " "You should resolve this by manually moving or deleting the invalid " "snapshot directory %s, otherwise you will encounter the same error again " "on the next startup."), - src_str, dest_str, src_str)}; + db_path_str, invalid_path_str, db_path_str)}; } return {}; } @@ -6381,33 +6373,17 @@ void ChainstateManager::RecalculateBestHeader() } } -bool ChainstateManager::ValidatedSnapshotCleanup() +bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs) { AssertLockHeld(::cs_main); - auto get_storage_path = [](auto& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) -> std::optional { - if (!(chainstate && chainstate->HasCoinsViews())) { - return {}; - } - return chainstate->CoinsDB().StoragePath(); - }; - std::optional ibd_chainstate_path_maybe = get_storage_path(m_ibd_chainstate); - std::optional snapshot_chainstate_path_maybe = get_storage_path(m_snapshot_chainstate); - if (!this->IsSnapshotValidated()) { // No need to clean up. return false; } - // If either path doesn't exist, that means at least one of the chainstates - // is in-memory, in which case we can't do on-disk cleanup. You'd better be - // in a unittest! - if (!ibd_chainstate_path_maybe || !snapshot_chainstate_path_maybe) { - LogError("[snapshot] snapshot chainstate cleanup cannot happen with " - "in-memory chainstates. You are testing, right?"); - return false; - } - const auto& snapshot_chainstate_path = *snapshot_chainstate_path_maybe; - const auto& ibd_chainstate_path = *ibd_chainstate_path_maybe; + const fs::path validated_path{validated_cs.StoragePath()}; + const fs::path assumed_valid_path{unvalidated_cs.StoragePath()}; + const fs::path delete_path{validated_path + "_todelete"}; // Since we're going to be moving around the underlying leveldb filesystem content // for each chainstate, make sure that the chainstates (and their constituent @@ -6421,9 +6397,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() assert(this->GetAll().size() == 0); LogInfo("[snapshot] deleting background chainstate directory (now unnecessary) (%s)", - fs::PathToString(ibd_chainstate_path)); - - fs::path tmp_old{ibd_chainstate_path + "_todelete"}; + fs::PathToString(validated_path)); auto rename_failed_abort = [this]( fs::path p_old, @@ -6438,32 +6412,32 @@ bool ChainstateManager::ValidatedSnapshotCleanup() }; try { - fs::rename(ibd_chainstate_path, tmp_old); + fs::rename(validated_path, delete_path); } catch (const fs::filesystem_error& e) { - rename_failed_abort(ibd_chainstate_path, tmp_old, e); + rename_failed_abort(validated_path, delete_path, e); throw; } LogInfo("[snapshot] moving snapshot chainstate (%s) to " "default chainstate directory (%s)", - fs::PathToString(snapshot_chainstate_path), fs::PathToString(ibd_chainstate_path)); + fs::PathToString(assumed_valid_path), fs::PathToString(validated_path)); try { - fs::rename(snapshot_chainstate_path, ibd_chainstate_path); + fs::rename(assumed_valid_path, validated_path); } catch (const fs::filesystem_error& e) { - rename_failed_abort(snapshot_chainstate_path, ibd_chainstate_path, e); + rename_failed_abort(assumed_valid_path, validated_path, e); throw; } - if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { + if (!DeleteCoinsDBFromDisk(delete_path, /*is_snapshot=*/false)) { // No need to FatalError because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. LogWarning("Deletion of %s failed. Please remove it manually, as the " "directory is now unnecessary.", - fs::PathToString(tmp_old)); + fs::PathToString(delete_path)); } else { LogInfo("[snapshot] deleted background chainstate directory (%s)", - fs::PathToString(ibd_chainstate_path)); + fs::PathToString(validated_path)); } return true; } diff --git a/src/validation.h b/src/validation.h index 6fe31117034..37c6051e5b6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -579,6 +579,9 @@ public: ChainstateManager& chainman, std::optional from_snapshot_blockhash = std::nullopt); + //! Return path to chainstate leveldb directory. + fs::path StoragePath() const; + //! Return the current role of the chainstate. See `ChainstateManager` //! documentation for a description of the different types of chainstates. //! @@ -594,8 +597,7 @@ public: void InitCoinsDB( size_t cache_size_bytes, bool in_memory, - bool should_wipe, - fs::path leveldb_name = "chainstate"); + bool should_wipe); //! Initialize the in-memory coins cache (to be done after the health of the on-disk database //! is verified). @@ -703,9 +705,6 @@ public: //! Destructs all objects related to accessing the UTXO set. void ResetCoinsViews() { m_coins_views.reset(); } - //! Does this chainstate have a UTXO set attached? - bool HasCoinsViews() const { return (bool)m_coins_views; } - //! The cache size of the on-disk coins view. size_t m_coinsdb_cache_size_bytes{0}; @@ -1335,7 +1334,7 @@ public: //! directories are moved or deleted. //! //! @sa node/chainstate:LoadChainstate() - bool ValidatedSnapshotCleanup() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! @returns the chainstate that indexes should consult when ensuring that an //! index is synced with a chain where we can expect block index entries to have