From d14bebf100aaaa25c7558eeed8b5c536da99885f Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Tue, 1 Feb 2022 16:22:00 -0500 Subject: [PATCH 01/13] db: add StoragePath to CDBWrapper/CCoinsViewDB This is used in subsequent commits. It allows us to clean up UTXO snapshot chainstate after background validation completes. --- src/dbwrapper.cpp | 2 +- src/dbwrapper.h | 18 ++++++++++++++++++ src/txdb.h | 4 ++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 4dbc83994163a..7f45e35aef85e 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -128,7 +128,7 @@ static leveldb::Options GetOptions(size_t nCacheSize) } CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bool fWipe, bool obfuscate) - : m_name{fs::PathToString(path.stem())} + : m_name{fs::PathToString(path.stem())}, m_path{path}, m_is_memory{fMemory} { penv = nullptr; readoptions.verify_checksums = true; diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 665eaa0e98673..1052da01d54a1 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -39,6 +39,10 @@ class dbwrapper_error : public std::runtime_error class CDBWrapper; +namespace dbwrapper { + using leveldb::DestroyDB; +} + /** These should be considered an implementation detail of the specific database. */ namespace dbwrapper_private { @@ -219,6 +223,12 @@ class CDBWrapper std::vector CreateObfuscateKey() const; + //! path to filesystem storage + const fs::path m_path; + + //! whether or not the database resides in memory + bool m_is_memory; + public: /** * @param[in] path Location in the filesystem where leveldb data will be stored. @@ -268,6 +278,14 @@ class CDBWrapper return 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/txdb.h b/src/txdb.h index a04596f7bbed3..8c41e26f6a17d 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -72,6 +73,9 @@ class CCoinsViewDB final : public CCoinsView //! 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(); } }; /** Access to the block database (blocks/index/) */ From f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 20 Apr 2022 14:59:02 -0400 Subject: [PATCH 02/13] validation: rename snapshot chainstate dir This changes the snapshot's leveldb chainstate dir name from `chainstate_[blockhash]` to `chainstate_snapshot`. This simplifies later logic that loads snapshot data, and enforces the limitation of a single snapshot at any given time. Since we still need to persis the blockhash of the base block, we write that out to a file (`chainstate_snapshot/base_blockhash`) for later use during initialization, so that we can reinitialize the snapshot chainstate. Co-authored-by: Russell Yanofsky --- src/Makefile.am | 2 + src/node/utxo_snapshot.cpp | 79 +++++++++++++++++++ src/node/utxo_snapshot.h | 27 +++++++ src/streams.h | 6 +- .../validation_chainstatemanager_tests.cpp | 13 ++- src/validation.cpp | 12 ++- test/lint/lint-circular-dependencies.py | 1 + 7 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 src/node/utxo_snapshot.cpp diff --git a/src/Makefile.am b/src/Makefile.am index bf26cc9674329..cc5824c81e267 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -395,6 +395,7 @@ libbitcoin_node_a_SOURCES = \ node/minisketchwrapper.cpp \ node/psbt.cpp \ node/transaction.cpp \ + node/utxo_snapshot.cpp \ node/validation_cache_args.cpp \ noui.cpp \ policy/fees.cpp \ @@ -900,6 +901,7 @@ libbitcoinkernel_la_SOURCES = \ node/blockstorage.cpp \ node/chainstate.cpp \ node/interface_ui.cpp \ + node/utxo_snapshot.cpp \ policy/feerate.cpp \ policy/fees.cpp \ policy/packages.cpp \ diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp new file mode 100644 index 0000000000000..29ff83a03bc12 --- /dev/null +++ b/src/node/utxo_snapshot.cpp @@ -0,0 +1,79 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include + +#include +#include + +namespace node { + +bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) +{ + AssertLockHeld(::cs_main); + assert(snapshot_chainstate.m_from_snapshot_blockhash); + + const std::optional chaindir = snapshot_chainstate.CoinsDB().StoragePath(); + assert(chaindir); // Sanity check that chainstate isn't in-memory. + const fs::path write_to = *chaindir / node::SNAPSHOT_BLOCKHASH_FILENAME; + + FILE* file{fsbridge::fopen(write_to, "wb")}; + AutoFile afile{file}; + if (afile.IsNull()) { + LogPrintf("[snapshot] failed to open base blockhash file for writing: %s\n", + fs::PathToString(write_to)); + return false; + } + afile << *snapshot_chainstate.m_from_snapshot_blockhash; + + if (afile.fclose() != 0) { + LogPrintf("[snapshot] failed to close base blockhash file %s after writing\n", + fs::PathToString(write_to)); + return false; + } + return true; +} + +std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) +{ + if (!fs::exists(chaindir)) { + LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " /* Continued */ + "exists at path %s\n", fs::PathToString(chaindir)); + return std::nullopt; + } + const fs::path read_from = chaindir / node::SNAPSHOT_BLOCKHASH_FILENAME; + const std::string read_from_str = fs::PathToString(read_from); + + if (!fs::exists(read_from)) { + LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " /* Continued */ + "exists at path %s. Try deleting %s and calling loadtxoutset again?\n", + fs::PathToString(chaindir), read_from_str); + return std::nullopt; + } + + uint256 base_blockhash; + FILE* file{fsbridge::fopen(read_from, "rb")}; + AutoFile afile{file}; + if (afile.IsNull()) { + LogPrintf("[snapshot] failed to open base blockhash file for reading: %s\n", + read_from_str); + return std::nullopt; + } + afile >> base_blockhash; + + if (std::fgetc(afile.Get()) != EOF) { + LogPrintf("[snapshot] warning: unexpected trailing data in %s\n", read_from_str); + } else if (std::ferror(afile.Get())) { + LogPrintf("[snapshot] warning: i/o error reading %s\n", read_from_str); + } + return base_blockhash; +} + +} // namespace node diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index 9dd6f06997349..b82a791092fc4 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -6,8 +6,14 @@ #ifndef BITCOIN_NODE_UTXO_SNAPSHOT_H #define BITCOIN_NODE_UTXO_SNAPSHOT_H +#include #include #include +#include + +#include + +extern RecursiveMutex cs_main; namespace node { //! Metadata describing a serialized version of a UTXO set from which an @@ -33,6 +39,27 @@ class SnapshotMetadata SERIALIZE_METHODS(SnapshotMetadata, obj) { READWRITE(obj.m_base_blockhash, obj.m_coins_count); } }; + +//! The file in the snapshot chainstate dir which stores the base blockhash. This is +//! needed to reconstruct snapshot chainstates on init. +//! +//! Because we only allow loading a single snapshot at a time, there will only be one +//! chainstate directory with this filename present within it. +const fs::path SNAPSHOT_BLOCKHASH_FILENAME{"base_blockhash"}; + +//! Write out the blockhash of the snapshot base block that was used to construct +//! this chainstate. This value is read in during subsequent initializations and +//! used to reconstruct snapshot-based chainstates. +bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + +//! Read the blockhash of the snapshot base block that was used to construct the +//! chainstate. +std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + +constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot"; + } // namespace node #endif // BITCOIN_NODE_UTXO_SNAPSHOT_H diff --git a/src/streams.h b/src/streams.h index f14d347380ac6..1bd98f81647e7 100644 --- a/src/streams.h +++ b/src/streams.h @@ -488,12 +488,14 @@ class AutoFile AutoFile(const AutoFile&) = delete; AutoFile& operator=(const AutoFile&) = delete; - void fclose() + int fclose() { + int retval{0}; if (file) { - ::fclose(file); + retval = ::fclose(file); file = nullptr; } + return retval; } /** Get wrapped FILE* with transfer of ownership. diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 24ad9458c969d..5aa38fc4bb7f2 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -232,8 +232,17 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) *chainman.ActiveChainstate().m_from_snapshot_blockhash, *chainman.SnapshotBlockhash()); - // Ensure that the genesis block was not marked assumed-valid. - BOOST_CHECK(WITH_LOCK(::cs_main, return !chainman.ActiveChain().Genesis()->IsAssumedValid())); + { + LOCK(::cs_main); + + // Note: WriteSnapshotBaseBlockhash() is implicitly tested above. + BOOST_CHECK_EQUAL( + *node::ReadSnapshotBaseBlockhash(m_args.GetDataDirNet() / "chainstate_snapshot"), + *chainman.SnapshotBlockhash()); + + // Ensure that the genesis block was not marked assumed-valid. + BOOST_CHECK(!chainman.ActiveChain().Genesis()->IsAssumedValid()); + } const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()); diff --git a/src/validation.cpp b/src/validation.cpp index 402a962a04099..0bfa17bb2bf4b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1515,7 +1515,7 @@ void Chainstate::InitCoinsDB( fs::path leveldb_name) { if (m_from_snapshot_blockhash) { - leveldb_name += "_" + m_from_snapshot_blockhash->ToString(); + leveldb_name += node::SNAPSHOT_CHAINSTATE_SUFFIX; } m_coins_views = std::make_unique( @@ -4837,9 +4837,17 @@ bool ChainstateManager::ActivateSnapshot( static_cast(current_coinstip_cache_size * SNAPSHOT_CACHE_PERC)); } - const bool snapshot_ok = this->PopulateAndValidateSnapshot( + bool snapshot_ok = this->PopulateAndValidateSnapshot( *snapshot_chainstate, coins_file, metadata); + // If not in-memory, persist the base blockhash for use during subsequent + // initialization. + if (!in_memory) { + LOCK(::cs_main); + if (!node::WriteSnapshotBaseBlockhash(*snapshot_chainstate)) { + snapshot_ok = false; + } + } if (!snapshot_ok) { WITH_LOCK(::cs_main, this->MaybeRebalanceCaches()); return false; diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index a0f17ac119bec..abf38ca79b69e 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -14,6 +14,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES = ( "chainparamsbase -> util/system -> chainparamsbase", "node/blockstorage -> validation -> node/blockstorage", + "node/utxo_snapshot -> validation -> node/utxo_snapshot", "policy/fees -> txmempool -> policy/fees", "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel", "qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel", From 252abd1e8bc5cdf4368ad55e827a873240535b28 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 20 Apr 2022 17:40:01 -0400 Subject: [PATCH 03/13] init: add utxo snapshot detection Add functionality for activating a snapshot-based chainstate if one is detected on-disk. Also cautiously initialize chainstate cache usages so that we don't somehow blow past our cache allowances during initialization, then rebalance at the end of init. Co-authored-by: Russell Yanofsky --- src/node/chainstate.cpp | 24 +++++++-- src/node/utxo_snapshot.cpp | 12 +++++ src/node/utxo_snapshot.h | 6 +++ .../validation_chainstatemanager_tests.cpp | 6 +-- src/validation.cpp | 53 ++++++++++++------- src/validation.h | 19 ++++--- test/functional/feature_init.py | 1 - 7 files changed, 87 insertions(+), 34 deletions(-) diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 3f1d6dd743299..26af5234916ac 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -48,10 +48,15 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize } LOCK(cs_main); - chainman.InitializeChainstate(options.mempool); chainman.m_total_coinstip_cache = cache_sizes.coins; chainman.m_total_coinsdb_cache = cache_sizes.coins_db; + // Load the fully validated chainstate. + chainman.InitializeChainstate(options.mempool); + + // Load a chain created from a UTXO snapshot, if any exist. + chainman.DetectSnapshotChainstate(options.mempool); + auto& pblocktree{chainman.m_blockman.m_block_tree_db}; // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: @@ -98,12 +103,20 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")}; } + // Conservative value which is arbitrarily chosen, as it will ultimately be changed + // by a call to `chainman.MaybeRebalanceCaches()`. We just need to make sure + // that the sum of the two caches (40%) does not exceed the allowable amount + // during this temporary initialization state. + double init_cache_fraction = 0.2; + // At this point we're either in reindex or we've loaded a useful // block tree into BlockIndex()! for (Chainstate* chainstate : chainman.GetAll()) { + LogPrintf("Initializing chainstate %s\n", chainstate->ToString()); + chainstate->InitCoinsDB( - /*cache_size_bytes=*/cache_sizes.coins_db, + /*cache_size_bytes=*/chainman.m_total_coinsdb_cache * init_cache_fraction, /*in_memory=*/options.coins_db_in_memory, /*should_wipe=*/options.reindex || options.reindex_chainstate); @@ -125,7 +138,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize } // The on-disk coinsdb is now in a good state, create the cache - chainstate->InitCoinsCache(cache_sizes.coins); + chainstate->InitCoinsCache(chainman.m_total_coinstip_cache * init_cache_fraction); assert(chainstate->CanFlushToDisk()); if (!is_coinsview_empty(chainstate)) { @@ -146,6 +159,11 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize }; } + // Now that chainstates are loaded and we're able to flush to + // disk, rebalance the coins caches to desired levels based + // on the condition of each chainstate. + chainman.MaybeRebalanceCaches(); + return {ChainstateLoadStatus::SUCCESS, {}}; } diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index 29ff83a03bc12..bab1b7521167f 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -76,4 +77,15 @@ std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) return base_blockhash; } +std::optional FindSnapshotChainstateDir() +{ + fs::path possible_dir = + gArgs.GetDataDirNet() / fs::u8path(strprintf("chainstate%s", SNAPSHOT_CHAINSTATE_SUFFIX)); + + if (fs::exists(possible_dir)) { + return possible_dir; + } + return std::nullopt; +} + } // namespace node diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index b82a791092fc4..c94521792f367 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -58,8 +58,14 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); +//! Suffix appended to the chainstate (leveldb) dir when created based upon +//! a snapshot. constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot"; + +//! Return a path to the snapshot-based chainstate dir, if one exists. +std::optional FindSnapshotChainstateDir(); + } // namespace node #endif // BITCOIN_NODE_UTXO_SNAPSHOT_H diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 5aa38fc4bb7f2..2b99f770f717c 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // const uint256 snapshot_blockhash = GetRandHash(); - Chainstate& c2 = WITH_LOCK(::cs_main, return manager.InitializeChainstate( + Chainstate& c2 = WITH_LOCK(::cs_main, return manager.ActivateExistingSnapshot( &mempool, snapshot_blockhash)); chainstates.push_back(&c2); @@ -133,7 +133,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - Chainstate& c2 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool, GetRandHash())); + Chainstate& c2 = WITH_LOCK(cs_main, return manager.ActivateExistingSnapshot(&mempool, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); @@ -383,7 +383,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) BOOST_CHECK_EQUAL(expected_assumed_valid, num_assumed_valid); Chainstate& cs2 = WITH_LOCK(::cs_main, - return chainman.InitializeChainstate(&mempool, GetRandHash())); + return chainman.ActivateExistingSnapshot(&mempool, GetRandHash())); reload_all_block_indexes(); diff --git a/src/validation.cpp b/src/validation.cpp index 0bfa17bb2bf4b..d1abda5830101 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4744,28 +4744,15 @@ std::vector ChainstateManager::GetAll() return out; } -Chainstate& ChainstateManager::InitializeChainstate( - CTxMemPool* mempool, const std::optional& snapshot_blockhash) +Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool) { AssertLockHeld(::cs_main); - bool is_snapshot = snapshot_blockhash.has_value(); - std::unique_ptr& to_modify = - is_snapshot ? m_snapshot_chainstate : m_ibd_chainstate; + assert(!m_ibd_chainstate); + assert(!m_active_chainstate); - if (to_modify) { - throw std::logic_error("should not be overwriting a chainstate"); - } - to_modify.reset(new Chainstate(mempool, m_blockman, *this, snapshot_blockhash)); - - // Snapshot chainstates and initial IBD chaintates always become active. - if (is_snapshot || (!is_snapshot && !m_active_chainstate)) { - LogPrintf("Switching active chainstate to %s\n", to_modify->ToString()); - m_active_chainstate = to_modify.get(); - } else { - throw std::logic_error("unexpected chainstate activation"); - } - - return *to_modify; + m_ibd_chainstate = std::make_unique(mempool, m_blockman, *this); + m_active_chainstate = m_ibd_chainstate.get(); + return *m_active_chainstate; } const AssumeutxoData* ExpectedAssumeutxo( @@ -5134,3 +5121,31 @@ ChainstateManager::~ChainstateManager() i.clear(); } } + +bool ChainstateManager::DetectSnapshotChainstate(CTxMemPool* mempool) +{ + assert(!m_snapshot_chainstate); + std::optional path = node::FindSnapshotChainstateDir(); + if (!path) { + return false; + } + std::optional base_blockhash = node::ReadSnapshotBaseBlockhash(*path); + if (!base_blockhash) { + return false; + } + LogPrintf("[snapshot] detected active snapshot chainstate (%s) - loading\n", + fs::PathToString(*path)); + + this->ActivateExistingSnapshot(mempool, *base_blockhash); + return true; +} + +Chainstate& ChainstateManager::ActivateExistingSnapshot(CTxMemPool* mempool, uint256 base_blockhash) +{ + assert(!m_snapshot_chainstate); + m_snapshot_chainstate = + std::make_unique(mempool, m_blockman, *this, base_blockhash); + LogPrintf("[snapshot] switching active chainstate to %s\n", m_snapshot_chainstate->ToString()); + m_active_chainstate = m_snapshot_chainstate.get(); + return *m_snapshot_chainstate; +} diff --git a/src/validation.h b/src/validation.h index 9ba206855f0cc..8a977f21c2ebe 100644 --- a/src/validation.h +++ b/src/validation.h @@ -929,17 +929,11 @@ class ChainstateManager //! coins databases. This will be split somehow across chainstates. int64_t m_total_coinsdb_cache{0}; - //! Instantiate a new chainstate and assign it based upon whether it is - //! from a snapshot. + //! Instantiate a new chainstate. //! //! @param[in] mempool The mempool to pass to the chainstate // constructor - //! @param[in] snapshot_blockhash If given, signify that this chainstate - //! is based on a snapshot. - Chainstate& InitializeChainstate( - CTxMemPool* mempool, - const std::optional& snapshot_blockhash = std::nullopt) - LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + Chainstate& InitializeChainstate(CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Get all chainstates currently being used. std::vector GetAll(); @@ -1053,6 +1047,15 @@ class ChainstateManager * information. */ void ReportHeadersPresync(const arith_uint256& work, int64_t height, int64_t timestamp); + //! When starting up, search the datadir for a chainstate based on a UTXO + //! snapshot that is in the process of being validated. + bool DetectSnapshotChainstate(CTxMemPool* mempool) 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) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + ~ChainstateManager(); }; diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 13c7326519907..56d093c396bfb 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -55,7 +55,6 @@ def check_clean_start(): b'Loading P2P addresses', b'Loading banlist', b'Loading block index', - b'Switching active chainstate', b'Checking all blk files are present', b'Loaded best chain:', b'init message: Verifying blocks', From 34d159033106cc595cfa852695610bfe419c989c Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Fri, 29 Apr 2022 09:02:02 -0400 Subject: [PATCH 04/13] add utilities for deleting on-disk leveldb data Used in later commits to remove leveldb directories for - invalid snapshot chainstates, and - background-vaildation chainstates that have finished serving their purpose. --- src/validation.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index d1abda5830101..7e9c6b891ec19 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4767,6 +4767,46 @@ const AssumeutxoData* ExpectedAssumeutxo( return nullptr; } +static bool DeleteCoinsDBFromDisk(const fs::path db_path, bool is_snapshot) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +{ + AssertLockHeld(::cs_main); + + if (is_snapshot) { + fs::path base_blockhash_path = db_path / node::SNAPSHOT_BLOCKHASH_FILENAME; + + if (fs::exists(base_blockhash_path)) { + bool removed = fs::remove(base_blockhash_path); + if (!removed) { + LogPrintf("[snapshot] failed to remove file %s\n", + fs::PathToString(base_blockhash_path)); + } + } else { + LogPrintf("[snapshot] snapshot chainstate dir being removed lacks %s file\n", + fs::PathToString(node::SNAPSHOT_BLOCKHASH_FILENAME)); + } + } + + std::string path_str = fs::PathToString(db_path); + LogPrintf("Removing leveldb dir at %s\n", path_str); + + // We have to destruct before this call leveldb::DB in order to release the db + // lock, otherwise `DestroyDB` will fail. See `leveldb::~DBImpl()`. + const bool destroyed = dbwrapper::DestroyDB(path_str, {}).ok(); + + if (!destroyed) { + LogPrintf("error: leveldb DestroyDB call failed on %s\n", path_str); + } + + // Datadir should be removed from filesystem; otherwise initialization may detect + // it on subsequent statups and get confused. + // + // If the base_blockhash_path removal above fails in the case of snapshot + // chainstates, this will return false since leveldb won't remove a non-empty + // directory. + return destroyed && !fs::exists(db_path); +} + bool ChainstateManager::ActivateSnapshot( AutoFile& coins_file, const SnapshotMetadata& metadata, From ad67ff377c2b271cb4683da2fb25fd295557f731 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Tue, 17 Aug 2021 16:23:02 -0400 Subject: [PATCH 05/13] validation: remove snapshot datadirs upon validation failure If a UTXO snapshot fails to validate, don't leave the resulting datadir on disk as this will confuse initialization on next startup and we'll get an assertion error. --- src/validation.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 7e9c6b891ec19..b94950d1b4fb6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4876,7 +4876,22 @@ bool ChainstateManager::ActivateSnapshot( } } if (!snapshot_ok) { - WITH_LOCK(::cs_main, this->MaybeRebalanceCaches()); + LOCK(::cs_main); + this->MaybeRebalanceCaches(); + + // PopulateAndValidateSnapshot can return (in error) before the leveldb datadir + // has been created, so only attempt removal if we got that far. + if (auto snapshot_datadir = node::FindSnapshotChainstateDir()) { + // We have to destruct leveldb::DB in order to release the db lock, otherwise + // DestroyDB() (in DeleteCoinsDBFromDisk()) will fail. See `leveldb::~DBImpl()`. + // Destructing the chainstate (and so resetting the coinsviews object) does this. + snapshot_chainstate.reset(); + bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); + if (!removed) { + AbortNode(strprintf("Failed to remove snapshot chainstate dir (%s). " + "Manually remove it before restarting.\n", fs::PathToString(*snapshot_datadir))); + } + } return false; } From 8153bd9247dad3982d54488bcdb3960470315290 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 2 Feb 2022 09:47:13 -0500 Subject: [PATCH 06/13] blockmanager: avoid undefined behavior during FlushBlockFile If we call FlushBlockFile() without having intitialized the block index with LoadBlockIndexDB(), we may be indexing into an empty vector. Specifically this is an issue when we call MaybeRebalanceCaches() during chainstate init before the block index has been loaded, which calls FlushBlockFile(). Also add an assert to avoid undefined behavior. Co-authored-by: Russell Yanofsky --- src/node/blockstorage.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 57f81e6bb6a69..9fea644f05ce4 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -524,6 +524,16 @@ void BlockManager::FlushUndoFile(int block_file, bool finalize) void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) { LOCK(cs_LastBlockFile); + + if (m_blockfile_info.size() < 1) { + // Return if we haven't loaded any blockfiles yet. This happens during + // chainstate init, when we call ChainstateManager::MaybeRebalanceCaches() (which + // then calls FlushStateToDisk()), resulting in a call to this function before we + // have populated `m_blockfile_info` via LoadBlockIndexDB(). + return; + } + assert(static_cast(m_blockfile_info.size()) > m_last_blockfile); + FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize); if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) { AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error."); From 3a29dfbfb2c16a50d854f6f81428a68aa9180509 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Mon, 7 Feb 2022 19:29:52 -0500 Subject: [PATCH 07/13] move-only: test: make snapshot chainstate setup reusable For use in next commit. Most easily reviewed with `--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change`. --- .../validation_chainstatemanager_tests.cpp | 287 +++++++++--------- 1 file changed, 151 insertions(+), 136 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 2b99f770f717c..70e4e28c5f1ac 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -154,171 +154,186 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1); } -//! Test basic snapshot activation. -BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) -{ - ChainstateManager& chainman = *Assert(m_node.chainman); +struct SnapshotTestSetup : TestChain100Setup { + std::tuple SetupSnapshot() + { + ChainstateManager& chainman = *Assert(m_node.chainman); - size_t initial_size; - size_t initial_total_coins{100}; + BOOST_CHECK(!chainman.IsSnapshotActive()); + WITH_LOCK(::cs_main, BOOST_CHECK(!chainman.IsSnapshotValidated())); - // Make some initial assertions about the contents of the chainstate. - { - LOCK(::cs_main); - CCoinsViewCache& ibd_coinscache = chainman.ActiveChainstate().CoinsTip(); - initial_size = ibd_coinscache.GetCacheSize(); - size_t total_coins{0}; - - for (CTransactionRef& txn : m_coinbase_txns) { - COutPoint op{txn->GetHash(), 0}; - BOOST_CHECK(ibd_coinscache.HaveCoin(op)); - total_coins++; - } + size_t initial_size; + size_t initial_total_coins{100}; - BOOST_CHECK_EQUAL(total_coins, initial_total_coins); - BOOST_CHECK_EQUAL(initial_size, initial_total_coins); - } + // Make some initial assertions about the contents of the chainstate. + { + LOCK(::cs_main); + CCoinsViewCache& ibd_coinscache = chainman.ActiveChainstate().CoinsTip(); + initial_size = ibd_coinscache.GetCacheSize(); + size_t total_coins{0}; - // Snapshot should refuse to load at this height. - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); - BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash); - BOOST_CHECK(!chainman.SnapshotBlockhash()); - - // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can - // be found. - constexpr int snapshot_height = 110; - mineBlocks(10); - initial_size += 10; - initial_total_coins += 10; - - // Should not load malleated snapshots - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { - // A UTXO is missing but count is correct - metadata.m_coins_count -= 1; - - COutPoint outpoint; - Coin coin; - - auto_infile >> outpoint; - auto_infile >> coin; - })); - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { - // Coins count is larger than coins in file - metadata.m_coins_count += 1; - })); - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { - // Coins count is smaller than coins in file - metadata.m_coins_count -= 1; - })); - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { - // Wrong hash - metadata.m_base_blockhash = uint256::ZERO; - })); - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { - // Wrong hash - metadata.m_base_blockhash = uint256::ONE; - })); - - BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); - - // Ensure our active chain is the snapshot chainstate. - BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); - BOOST_CHECK_EQUAL( - *chainman.ActiveChainstate().m_from_snapshot_blockhash, - *chainman.SnapshotBlockhash()); + for (CTransactionRef& txn : m_coinbase_txns) { + COutPoint op{txn->GetHash(), 0}; + BOOST_CHECK(ibd_coinscache.HaveCoin(op)); + total_coins++; + } - { - LOCK(::cs_main); + BOOST_CHECK_EQUAL(total_coins, initial_total_coins); + BOOST_CHECK_EQUAL(initial_size, initial_total_coins); + } - // Note: WriteSnapshotBaseBlockhash() is implicitly tested above. + Chainstate& validation_chainstate = chainman.ActiveChainstate(); + + // Snapshot should refuse to load at this height. + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash); + BOOST_CHECK(!chainman.SnapshotBlockhash()); + + // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can + // be found. + constexpr int snapshot_height = 110; + mineBlocks(10); + initial_size += 10; + initial_total_coins += 10; + + // Should not load malleated snapshots + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + // A UTXO is missing but count is correct + metadata.m_coins_count -= 1; + + COutPoint outpoint; + Coin coin; + + auto_infile >> outpoint; + auto_infile >> coin; + })); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + // Coins count is larger than coins in file + metadata.m_coins_count += 1; + })); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + // Coins count is smaller than coins in file + metadata.m_coins_count -= 1; + })); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + // Wrong hash + metadata.m_base_blockhash = uint256::ZERO; + })); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + // Wrong hash + metadata.m_base_blockhash = uint256::ONE; + })); + + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + + // Ensure our active chain is the snapshot chainstate. + BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); BOOST_CHECK_EQUAL( - *node::ReadSnapshotBaseBlockhash(m_args.GetDataDirNet() / "chainstate_snapshot"), + *chainman.ActiveChainstate().m_from_snapshot_blockhash, *chainman.SnapshotBlockhash()); - // Ensure that the genesis block was not marked assumed-valid. - BOOST_CHECK(!chainman.ActiveChain().Genesis()->IsAssumedValid()); - } + Chainstate& snapshot_chainstate = chainman.ActiveChainstate(); - const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); - const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()); + { + LOCK(::cs_main); - BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); + // Note: WriteSnapshotBaseBlockhash() is implicitly tested above. + BOOST_CHECK_EQUAL( + *node::ReadSnapshotBaseBlockhash(m_args.GetDataDirNet() / "chainstate_snapshot"), + *chainman.SnapshotBlockhash()); - // To be checked against later when we try loading a subsequent snapshot. - uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()}; + // Ensure that the genesis block was not marked assumed-valid. + BOOST_CHECK(!chainman.ActiveChain().Genesis()->IsAssumedValid()); + } - // Make some assertions about the both chainstates. These checks ensure the - // legacy chainstate hasn't changed and that the newly created chainstate - // reflects the expected content. - { - LOCK(::cs_main); - int chains_tested{0}; + const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); + const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()); - for (Chainstate* chainstate : chainman.GetAll()) { - BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString()); - CCoinsViewCache& coinscache = chainstate->CoinsTip(); + BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); - // Both caches will be empty initially. - BOOST_CHECK_EQUAL((unsigned int)0, coinscache.GetCacheSize()); + // To be checked against later when we try loading a subsequent snapshot. + uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()}; - size_t total_coins{0}; + // Make some assertions about the both chainstates. These checks ensure the + // legacy chainstate hasn't changed and that the newly created chainstate + // reflects the expected content. + { + LOCK(::cs_main); + int chains_tested{0}; - for (CTransactionRef& txn : m_coinbase_txns) { - COutPoint op{txn->GetHash(), 0}; - BOOST_CHECK(coinscache.HaveCoin(op)); - total_coins++; - } + for (Chainstate* chainstate : chainman.GetAll()) { + BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString()); + CCoinsViewCache& coinscache = chainstate->CoinsTip(); - BOOST_CHECK_EQUAL(initial_size , coinscache.GetCacheSize()); - BOOST_CHECK_EQUAL(total_coins, initial_total_coins); - chains_tested++; - } + // Both caches will be empty initially. + BOOST_CHECK_EQUAL((unsigned int)0, coinscache.GetCacheSize()); - BOOST_CHECK_EQUAL(chains_tested, 2); - } + size_t total_coins{0}; - // Mine some new blocks on top of the activated snapshot chainstate. - constexpr size_t new_coins{100}; - mineBlocks(new_coins); // Defined in TestChain100Setup. + for (CTransactionRef& txn : m_coinbase_txns) { + COutPoint op{txn->GetHash(), 0}; + BOOST_CHECK(coinscache.HaveCoin(op)); + total_coins++; + } - { - LOCK(::cs_main); - size_t coins_in_active{0}; - size_t coins_in_background{0}; - size_t coins_missing_from_background{0}; + BOOST_CHECK_EQUAL(initial_size , coinscache.GetCacheSize()); + BOOST_CHECK_EQUAL(total_coins, initial_total_coins); + chains_tested++; + } - for (Chainstate* chainstate : chainman.GetAll()) { - BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString()); - CCoinsViewCache& coinscache = chainstate->CoinsTip(); - bool is_background = chainstate != &chainman.ActiveChainstate(); + BOOST_CHECK_EQUAL(chains_tested, 2); + } - for (CTransactionRef& txn : m_coinbase_txns) { - COutPoint op{txn->GetHash(), 0}; - if (coinscache.HaveCoin(op)) { - (is_background ? coins_in_background : coins_in_active)++; - } else if (is_background) { - coins_missing_from_background++; + // Mine some new blocks on top of the activated snapshot chainstate. + constexpr size_t new_coins{100}; + mineBlocks(new_coins); // Defined in TestChain100Setup. + + { + LOCK(::cs_main); + size_t coins_in_active{0}; + size_t coins_in_background{0}; + size_t coins_missing_from_background{0}; + + for (Chainstate* chainstate : chainman.GetAll()) { + BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString()); + CCoinsViewCache& coinscache = chainstate->CoinsTip(); + bool is_background = chainstate != &chainman.ActiveChainstate(); + + for (CTransactionRef& txn : m_coinbase_txns) { + COutPoint op{txn->GetHash(), 0}; + if (coinscache.HaveCoin(op)) { + (is_background ? coins_in_background : coins_in_active)++; + } else if (is_background) { + coins_missing_from_background++; + } } } + + BOOST_CHECK_EQUAL(coins_in_active, initial_total_coins + new_coins); + BOOST_CHECK_EQUAL(coins_in_background, initial_total_coins); + BOOST_CHECK_EQUAL(coins_missing_from_background, new_coins); } - BOOST_CHECK_EQUAL(coins_in_active, initial_total_coins + new_coins); - BOOST_CHECK_EQUAL(coins_in_background, initial_total_coins); - BOOST_CHECK_EQUAL(coins_missing_from_background, new_coins); - } + // Snapshot should refuse to load after one has already loaded. + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); - // Snapshot should refuse to load after one has already loaded. - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + // Snapshot blockhash should be unchanged. + BOOST_CHECK_EQUAL( + *chainman.ActiveChainstate().m_from_snapshot_blockhash, + loaded_snapshot_blockhash); + return std::make_tuple(&validation_chainstate, &snapshot_chainstate); + } +}; - // Snapshot blockhash should be unchanged. - BOOST_CHECK_EQUAL( - *chainman.ActiveChainstate().m_from_snapshot_blockhash, - loaded_snapshot_blockhash); +//! Test basic snapshot activation. +BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, SnapshotTestSetup) +{ + this->SetupSnapshot(); } //! Test LoadBlockIndex behavior when multiple chainstates are in use. From 00b357c215ed900145bd770525a341ba0ed9c027 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Thu, 21 Jul 2022 12:56:47 -0400 Subject: [PATCH 08/13] validation: add ResetChainstates() Necessary for the following test commit. --- src/validation.cpp | 7 +++++++ src/validation.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index b94950d1b4fb6..f46ee1145eba7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5165,6 +5165,13 @@ void ChainstateManager::MaybeRebalanceCaches() } } +void ChainstateManager::ResetChainstates() +{ + m_ibd_chainstate.reset(); + m_snapshot_chainstate.reset(); + m_active_chainstate = nullptr; +} + ChainstateManager::~ChainstateManager() { LOCK(::cs_main); diff --git a/src/validation.h b/src/validation.h index 8a977f21c2ebe..d95841f3e1c1b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1051,6 +1051,8 @@ class ChainstateManager //! snapshot that is in the process of being validated. bool DetectSnapshotChainstate(CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void ResetChainstates() 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) From 3c361391b8f5971eb3c7b620aa7ad9b437cc515e Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Tue, 8 Feb 2022 15:56:16 -0500 Subject: [PATCH 09/13] test: add reset_chainstate parameter for snapshot unittests This CreateAndActivateUTXOSnapshot parameter is necessary once we perform snapshot completion within ABC, since the existing UpdateTip test will fail because the IBD chain that has generated the snapshot will exceed the base of the snapshot. Being able to test snapshots being loaded into a mostly-uninitialized datadir allows for more realistic unittest scenarios. --- src/test/util/chainstate.h | 43 ++++++++++++++++++- src/test/validation_chainstate_tests.cpp | 3 +- .../validation_chainstatemanager_tests.cpp | 16 +++---- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index 2f0021b114480..79bc88221589e 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -20,11 +21,20 @@ const auto NoMalleation = [](AutoFile& file, node::SnapshotMetadata& meta){}; /** * Create and activate a UTXO snapshot, optionally providing a function to * malleate the snapshot. + * + * If `reset_chainstate` is true, reset the original chainstate back to the genesis + * block. This allows us to simulate more realistic conditions in which a snapshot is + * loaded into an otherwise mostly-uninitialized datadir. It also allows us to test + * conditions that would otherwise cause shutdowns based on the IBD chainstate going + * past the snapshot it generated. */ template static bool -CreateAndActivateUTXOSnapshot(node::NodeContext& node, const fs::path root, F malleation = NoMalleation) +CreateAndActivateUTXOSnapshot(TestingSetup* fixture, F malleation = NoMalleation, bool reset_chainstate = false) { + node::NodeContext& node = fixture->m_node; + fs::path root = fixture->m_path_root; + // Write out a snapshot to the test's tempdir. // int height; @@ -47,6 +57,37 @@ CreateAndActivateUTXOSnapshot(node::NodeContext& node, const fs::path root, F ma malleation(auto_infile, metadata); + if (reset_chainstate) { + { + // What follows is code to selectively reset chainstate data without + // disturbing the existing BlockManager instance, which is needed to + // recognize the headers chain previously generated by the chainstate we're + // removing. Without those headers, we can't activate the snapshot below. + // + // This is a stripped-down version of node::LoadChainstate which + // preserves the block index. + LOCK(::cs_main); + uint256 gen_hash = node.chainman->ActiveChainstate().m_chain[0]->GetBlockHash(); + node.chainman->ResetChainstates(); + node.chainman->InitializeChainstate(node.mempool.get()); + Chainstate& chain = node.chainman->ActiveChainstate(); + Assert(chain.LoadGenesisBlock()); + // These cache values will be corrected shortly in `MaybeRebalanceCaches`. + chain.InitCoinsDB(1 << 20, true, false, ""); + chain.InitCoinsCache(1 << 20); + chain.CoinsTip().SetBestBlock(gen_hash); + chain.setBlockIndexCandidates.insert(node.chainman->m_blockman.LookupBlockIndex(gen_hash)); + chain.LoadChainTip(); + node.chainman->MaybeRebalanceCaches(); + } + BlockValidationState state; + if (!node.chainman->ActiveChainstate().ActivateBestChain(state)) { + throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); + } + Assert( + 0 == WITH_LOCK(node.chainman->GetMutex(), return node.chainman->ActiveHeight())); + } + return node.chainman->ActivateSnapshot(auto_infile, metadata, /*in_memory=*/true); } diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 347a967b33e74..f868c0d4e6367 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -89,7 +89,8 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // After adding some blocks to the tip, best block should have changed. BOOST_CHECK(::g_best_block != curr_tip); - BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot( + this, NoMalleation, /*reset_chainstate=*/ true)); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive())); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 70e4e28c5f1ac..04195e7a45839 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -185,7 +185,7 @@ struct SnapshotTestSetup : TestChain100Setup { Chainstate& validation_chainstate = chainman.ActiveChainstate(); // Snapshot should refuse to load at this height. - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(this)); BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash); BOOST_CHECK(!chainman.SnapshotBlockhash()); @@ -198,7 +198,7 @@ struct SnapshotTestSetup : TestChain100Setup { // Should not load malleated snapshots BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // A UTXO is missing but count is correct metadata.m_coins_count -= 1; @@ -209,27 +209,27 @@ struct SnapshotTestSetup : TestChain100Setup { auto_infile >> coin; })); BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // Coins count is larger than coins in file metadata.m_coins_count += 1; })); BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // Coins count is smaller than coins in file metadata.m_coins_count -= 1; })); BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // Wrong hash metadata.m_base_blockhash = uint256::ZERO; })); BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { + this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // Wrong hash metadata.m_base_blockhash = uint256::ONE; })); - BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(this)); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); @@ -320,7 +320,7 @@ struct SnapshotTestSetup : TestChain100Setup { } // Snapshot should refuse to load after one has already loaded. - BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(this)); // Snapshot blockhash should be unchanged. BOOST_CHECK_EQUAL( From 51fc9241c08a00f1f407f1534853a5cddbbc0a23 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 13 Apr 2022 15:16:26 -0400 Subject: [PATCH 10/13] test: allow on-disk coins and block tree dbs in tests Used when testing cleanup of on-disk chainstate data for snapshot testcases. Also necessary for simulating node restart in .cpp tests. --- src/test/util/chainstate.h | 8 ++++++-- src/test/util/setup_common.cpp | 22 ++++++++++++++++------ src/test/util/setup_common.h | 16 +++++++++++++--- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index 79bc88221589e..0ca63810f333f 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -30,7 +30,11 @@ const auto NoMalleation = [](AutoFile& file, node::SnapshotMetadata& meta){}; */ template static bool -CreateAndActivateUTXOSnapshot(TestingSetup* fixture, F malleation = NoMalleation, bool reset_chainstate = false) +CreateAndActivateUTXOSnapshot( + TestingSetup* fixture, + F malleation = NoMalleation, + bool reset_chainstate = false, + bool in_memory_chainstate = false) { node::NodeContext& node = fixture->m_node; fs::path root = fixture->m_path_root; @@ -88,7 +92,7 @@ CreateAndActivateUTXOSnapshot(TestingSetup* fixture, F malleation = NoMalleation 0 == WITH_LOCK(node.chainman->GetMutex(), return node.chainman->ActiveHeight())); } - return node.chainman->ActivateSnapshot(auto_infile, metadata, /*in_memory=*/true); + return node.chainman->ActivateSnapshot(auto_infile, metadata, in_memory_chainstate); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 74b055ee453da..852c04e2d9496 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -220,8 +220,14 @@ ChainTestingSetup::~ChainTestingSetup() m_node.chainman.reset(); } -TestingSetup::TestingSetup(const std::string& chainName, const std::vector& extra_args) - : ChainTestingSetup(chainName, extra_args) +TestingSetup::TestingSetup( + const std::string& chainName, + const std::vector& extra_args, + const bool coins_db_in_memory, + const bool block_tree_db_in_memory) + : ChainTestingSetup(chainName, extra_args), + m_coins_db_in_memory(coins_db_in_memory), + m_block_tree_db_in_memory(block_tree_db_in_memory) { // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. @@ -229,8 +235,8 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector& extra_args) - : TestingSetup{chain_name, extra_args} +TestChain100Setup::TestChain100Setup( + const std::string& chain_name, + const std::vector& extra_args, + const bool coins_db_in_memory, + const bool block_tree_db_in_memory) + : TestingSetup{CBaseChainParams::REGTEST, extra_args, coins_db_in_memory, block_tree_db_in_memory} { SetMockTime(1598887952); constexpr std::array vchKey = { diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 136ee1fd62419..9fe2692c266d5 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -107,7 +107,14 @@ struct ChainTestingSetup : public BasicTestingSetup { /** Testing setup that configures a complete environment. */ struct TestingSetup : public ChainTestingSetup { - explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector& extra_args = {}); + bool m_coins_db_in_memory{true}; + bool m_block_tree_db_in_memory{true}; + + explicit TestingSetup( + const std::string& chainName = CBaseChainParams::MAIN, + const std::vector& extra_args = {}, + const bool coins_db_in_memory = true, + const bool block_tree_db_in_memory = true); }; /** Identical to TestingSetup, but chain set to regtest */ @@ -124,8 +131,11 @@ class CScript; * Testing fixture that pre-creates a 100-block REGTEST-mode block chain */ struct TestChain100Setup : public TestingSetup { - TestChain100Setup(const std::string& chain_name = CBaseChainParams::REGTEST, - const std::vector& extra_args = {}); + TestChain100Setup( + const std::string& chain_name = CBaseChainParams::REGTEST, + const std::vector& extra_args = {}, + const bool coins_db_in_memory = true, + const bool block_tree_db_in_memory = true); /** * Create a new block with just given transactions, coinbase paying to From cced4e7336d93a2dc88e4a61c49941887766bd72 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Thu, 28 Apr 2022 10:57:25 -0400 Subject: [PATCH 11/13] test: move-only-ish: factor out LoadVerifyActivateChainstate() in TestingSetup(). This is used in the following commit to test reinitializing chainstates after snapshot validation and cleanup. Best reviewed with `git diff --color-moved=dimmed-zebra`. --- src/test/util/setup_common.cpp | 29 +++++++++++++++++------------ src/test/util/setup_common.h | 2 ++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 852c04e2d9496..9ac6c468e20a9 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -220,19 +220,8 @@ ChainTestingSetup::~ChainTestingSetup() m_node.chainman.reset(); } -TestingSetup::TestingSetup( - const std::string& chainName, - const std::vector& extra_args, - const bool coins_db_in_memory, - const bool block_tree_db_in_memory) - : ChainTestingSetup(chainName, extra_args), - m_coins_db_in_memory(coins_db_in_memory), - m_block_tree_db_in_memory(block_tree_db_in_memory) +void TestingSetup::LoadVerifyActivateChainstate() { - // Ideally we'd move all the RPC tests to the functional testing framework - // instead of unit tests, but for now we need these here. - RegisterAllCoreRPCCommands(tableRPC); - node::ChainstateLoadOptions options; options.mempool = Assert(m_node.mempool.get()); options.block_tree_db_in_memory = m_block_tree_db_in_memory; @@ -252,6 +241,22 @@ TestingSetup::TestingSetup( if (!m_node.chainman->ActiveChainstate().ActivateBestChain(state)) { throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); } +} + +TestingSetup::TestingSetup( + const std::string& chainName, + const std::vector& extra_args, + const bool coins_db_in_memory, + const bool block_tree_db_in_memory) + : ChainTestingSetup(chainName, extra_args), + m_coins_db_in_memory(coins_db_in_memory), + m_block_tree_db_in_memory(block_tree_db_in_memory) +{ + // Ideally we'd move all the RPC tests to the functional testing framework + // instead of unit tests, but for now we need these here. + RegisterAllCoreRPCCommands(tableRPC); + + LoadVerifyActivateChainstate(); m_node.netgroupman = std::make_unique(/*asmap=*/std::vector()); m_node.addrman = std::make_unique(*m_node.netgroupman, diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 9fe2692c266d5..3a7d1b54b390d 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -110,6 +110,8 @@ struct TestingSetup : public ChainTestingSetup { bool m_coins_db_in_memory{true}; bool m_block_tree_db_in_memory{true}; + void LoadVerifyActivateChainstate(); + explicit TestingSetup( const std::string& chainName = CBaseChainParams::MAIN, const std::vector& extra_args = {}, From e4d799528696c5ede38c257afaffd367917e0de8 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Mon, 7 Feb 2022 19:56:31 -0500 Subject: [PATCH 12/13] test: add testcases for snapshot initialization --- .../validation_chainstatemanager_tests.cpp | 114 +++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 04195e7a45839..37de9565a3057 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -155,12 +156,32 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) } struct SnapshotTestSetup : TestChain100Setup { + // Run with coinsdb on the filesystem to support, e.g., moving invalidated + // chainstate dirs to "*_invalid". + // + // Note that this means the tests run considerably slower than in-memory DB + // tests, but we can't otherwise test this functionality since it relies on + // destructive filesystem operations. + SnapshotTestSetup() : TestChain100Setup{ + {}, + {}, + /*coins_db_in_memory=*/false, + /*block_tree_db_in_memory=*/false, + } + { + } + std::tuple SetupSnapshot() { ChainstateManager& chainman = *Assert(m_node.chainman); BOOST_CHECK(!chainman.IsSnapshotActive()); - WITH_LOCK(::cs_main, BOOST_CHECK(!chainman.IsSnapshotValidated())); + + { + LOCK(::cs_main); + BOOST_CHECK(!chainman.IsSnapshotValidated()); + BOOST_CHECK(!node::FindSnapshotChainstateDir()); + } size_t initial_size; size_t initial_total_coins{100}; @@ -208,6 +229,9 @@ struct SnapshotTestSetup : TestChain100Setup { auto_infile >> outpoint; auto_infile >> coin; })); + + BOOST_CHECK(!node::FindSnapshotChainstateDir()); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { // Coins count is larger than coins in file @@ -230,6 +254,7 @@ struct SnapshotTestSetup : TestChain100Setup { })); BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(this)); + BOOST_CHECK(fs::exists(*node::FindSnapshotChainstateDir())); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); @@ -242,9 +267,11 @@ struct SnapshotTestSetup : TestChain100Setup { { LOCK(::cs_main); + fs::path found = *node::FindSnapshotChainstateDir(); + // Note: WriteSnapshotBaseBlockhash() is implicitly tested above. BOOST_CHECK_EQUAL( - *node::ReadSnapshotBaseBlockhash(m_args.GetDataDirNet() / "chainstate_snapshot"), + *node::ReadSnapshotBaseBlockhash(found), *chainman.SnapshotBlockhash()); // Ensure that the genesis block was not marked assumed-valid. @@ -328,6 +355,34 @@ struct SnapshotTestSetup : TestChain100Setup { loaded_snapshot_blockhash); return std::make_tuple(&validation_chainstate, &snapshot_chainstate); } + + // Simulate a restart of the node by flushing all state to disk, clearing the + // existing ChainstateManager, and unloading the block index. + // + // @returns a reference to the "restarted" ChainstateManager + ChainstateManager& SimulateNodeRestart() + { + ChainstateManager& chainman = *Assert(m_node.chainman); + + BOOST_TEST_MESSAGE("Simulating node restart"); + { + LOCK(::cs_main); + for (Chainstate* cs : chainman.GetAll()) { + cs->ForceFlushStateToDisk(); + } + chainman.ResetChainstates(); + BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); + const ChainstateManager::Options chainman_opts{ + .chainparams = ::Params(), + .adjusted_time_callback = GetAdjustedTime, + }; + // For robustness, ensure the old manager is destroyed before creating a + // new one. + m_node.chainman.reset(); + m_node.chainman.reset(new ChainstateManager(chainman_opts)); + } + return *Assert(m_node.chainman); + } }; //! Test basic snapshot activation. @@ -414,4 +469,59 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes); } +//! Ensure that snapshot chainstates initialize properly when found on disk. +BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup) +{ + this->SetupSnapshot(); + + ChainstateManager& chainman = *Assert(m_node.chainman); + + fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(); + BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); + BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot"); + + BOOST_CHECK(chainman.IsSnapshotActive()); + const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(), + return chainman.ActiveTip()->GetBlockHash()); + + auto all_chainstates = chainman.GetAll(); + BOOST_CHECK_EQUAL(all_chainstates.size(), 2); + + // Test that simulating a shutdown (resetting ChainstateManager) and then performing + // chainstate reinitializing successfully cleans up the background-validation + // chainstate data, and we end up with a single chainstate that is at tip. + ChainstateManager& chainman_restarted = this->SimulateNodeRestart(); + + BOOST_TEST_MESSAGE("Performing Load/Verify/Activate of chainstate"); + + // This call reinitializes the chainstates. + this->LoadVerifyActivateChainstate(); + + { + LOCK(chainman_restarted.GetMutex()); + BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 2); + BOOST_CHECK(chainman_restarted.IsSnapshotActive()); + BOOST_CHECK(!chainman_restarted.IsSnapshotValidated()); + + BOOST_CHECK_EQUAL(chainman_restarted.ActiveTip()->GetBlockHash(), snapshot_tip_hash); + BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210); + } + + BOOST_TEST_MESSAGE( + "Ensure we can mine blocks on top of the initialized snapshot chainstate"); + mineBlocks(10); + { + LOCK(chainman_restarted.GetMutex()); + BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 220); + + // Background chainstate should be unaware of new blocks on the snapshot + // chainstate. + for (Chainstate* cs : chainman_restarted.GetAll()) { + if (cs != &chainman_restarted.ActiveChainstate()) { + BOOST_CHECK_EQUAL(cs->m_chain.Height(), 110); + } + } + } +} + BOOST_AUTO_TEST_SUITE_END() From bf9597606166323158bbf631137b82d41f39334f Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Tue, 6 Sep 2022 14:51:38 -0400 Subject: [PATCH 13/13] doc: add note about snapshot chainstate init --- doc/design/assumeutxo.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/design/assumeutxo.md b/doc/design/assumeutxo.md index c353c78ff872b..ea51b1b87f6b5 100644 --- a/doc/design/assumeutxo.md +++ b/doc/design/assumeutxo.md @@ -76,8 +76,9 @@ original chainstate remains in use as active. Once the snapshot chainstate is loaded and validated, it is promoted to active chainstate and a sync to tip begins. A new chainstate directory is created in the -datadir for the snapshot chainstate called -`chainstate_[SHA256 blockhash of snapshot base block]`. +datadir for the snapshot chainstate called `chainstate_snapshot`. When this directory +is present in the datadir, the snapshot chainstate will be detected and loaded as +active on node startup (via `DetectSnapshotChainstate()`). | | | | ---------- | ----------- |