refactor: Pass chainstate parameters to MaybeCompleteSnapshotValidation
What changed, and why it matters
This commit is a code cleanup (refactor) in Bitcoin Core's assumeutxo snapshot feature. It renames functions and changes how the snapshot validation code receives its inputs, so it no longer relies on hardcoded internal chainstate names. The change does not alter the actual snapshot validation logic or security checks; it is preparation for future work to reduce how long a global lock is held during snapshot validation. There is no direct security vulnerability in this patch.
No security action required. Treat as normal code-review/merge for a refactor. Monitor follow-up commits that actually move MaybeValidateSnapshot outside ActivateBestChain, since those will carry the real concurrency and lock-holding risks.
Security signals we found
Refactor only: no change to validation rules, hash comparisons, or snapshot invalidation behavior
Renames internal assumeutxo APIs for clarity and to remove hardcoded chainstate references
Adds defensive comment warning that ComputeUTXOStats can hold cs_main for minutes
Removes ChainstateManager::GetSnapshotBaseHeight() helper; callers now use ActiveChainstate().SnapshotBase()->nHeight
No new network inputs, file formats, RPCs, or consensus code introduced
Evidence from the diff
The patch refactors ChainstateManager::MaybeCompleteSnapshotValidation() into MaybeValidateSnapshot(Chainstate& validated_cs, Chainstate& unvalidated_cs), passing the two chainstates explicitly instead of reading m_ibd_chainstate and m_snapshot_chainstate. DetectSnapshotChainstate() is renamed LoadAssumeutxoChainstate() and now returns Chainstate*. FindSnapshotChainstateDir() is renamed FindAssumeutxoChainstateDir(). Callers in LoadChainstate, ConnectTip, tests, and snapshot activation are updated. The validation logic (checks for Assumeutxo state, target block, ReachedTarget, ComputeUTXOStats hash comparison, invalid snapshot handling) is preserved verbatim. A TODO comment is updated to note that holding cs_main is no longer strictly necessary for correctness but still required because the function is called inside ActivateBestChain.
Changed components
src/validation.cppsrc/validation.hsrc/node/chainstate.cppsrc/node/utxo_snapshot.cppsrc/node/utxo_snapshot.hsrc/test/fuzz/utxo_snapshot.cppsrc/test/validation_chainstatemanager_tests.cppdoc/design/assumeutxo.mdInspect captured patch +93 / −110
diff --git a/doc/design/assumeutxo.md b/doc/design/assumeutxo.md
index f1559cf4..9f04a52b 100644
--- a/doc/design/assumeutxo.md
+++ b/doc/design/assumeutxo.md
@@ -63,7 +63,7 @@ chainstate and a sync to tip begins. A new chainstate directory is created in th
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()`).
+and loaded as active on node startup (via `LoadAssumeutxoChainstate()`).
A special file is created within that directory, `base_blockhash`, which contains the
serialized `uint256` of the base block of the snapshot. This is used to reinitialize
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 0cebcad3..5a03e581 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -169,15 +169,16 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
Chainstate& validated_cs{chainman.InitializeChainstate(options.mempool)};
// Load a chain created from a UTXO snapshot, if any exist.
- bool has_snapshot = chainman.DetectSnapshotChainstate();
+ Chainstate* assumeutxo_cs{chainman.LoadAssumeutxoChainstate()};
- if (has_snapshot && options.wipe_chainstate_db) {
+ if (assumeutxo_cs && options.wipe_chainstate_db) {
// Reset chainstate target to network tip instead of snapshot block.
validated_cs.SetTargetBlock(nullptr);
LogInfo("[snapshot] deleting snapshot chainstate due to reindexing");
if (!chainman.DeleteSnapshotChainstate()) {
return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")};
}
+ assumeutxo_cs = nullptr;
}
auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options);
@@ -193,7 +194,9 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
// snapshot is actually validated? Because this entails unusual
// filesystem operations to move leveldb data directories around, and that seems
// too risky to do in the middle of normal runtime.
- auto snapshot_completion = chainman.MaybeCompleteSnapshotValidation();
+ auto snapshot_completion{assumeutxo_cs
+ ? chainman.MaybeValidateSnapshot(validated_cs, *assumeutxo_cs)
+ : SnapshotCompletionResult::SKIPPED};
if (snapshot_completion == SnapshotCompletionResult::SKIPPED) {
// do nothing; expected case
diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp
index d6e878ae..e159d9e0 100644
--- a/src/node/utxo_snapshot.cpp
+++ b/src/node/utxo_snapshot.cpp
@@ -81,7 +81,7 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir)
return base_blockhash;
}
-std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir)
+std::optional<fs::path> FindAssumeutxoChainstateDir(const fs::path& data_dir)
{
fs::path possible_dir =
data_dir / fs::u8path(strprintf("chainstate%s", SNAPSHOT_CHAINSTATE_SUFFIX));
diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h
index e4eb6d60..cc7caccc 100644
--- a/src/node/utxo_snapshot.h
+++ b/src/node/utxo_snapshot.h
@@ -125,7 +125,7 @@ constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot";
//! Return a path to the snapshot-based chainstate dir, if one exists.
-std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir);
+std::optional<fs::path> FindAssumeutxoChainstateDir(const fs::path& data_dir);
} // namespace node
diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
index 56d36155..5b088af4 100644
--- a/src/test/fuzz/utxo_snapshot.cpp
+++ b/src/test/fuzz/utxo_snapshot.cpp
@@ -191,7 +191,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
const auto* index{chainman.m_blockman.LookupBlockIndex(block->GetHash())};
Assert(index);
Assert(index->nTx == 0);
- if (index->nHeight == chainman.GetSnapshotBaseHeight()) {
+ if (index->nHeight == chainman.ActiveChainstate().SnapshotBase()->nHeight) {
auto params{chainman.GetParams().AssumeutxoForHeight(index->nHeight)};
Assert(params.has_value());
Assert(params.value().m_chain_tx_count == index->m_chain_tx_count);
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 03cec59c..036089c3 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -187,7 +187,7 @@ struct SnapshotTestSetup : TestChain100Setup {
{
LOCK(::cs_main);
BOOST_CHECK(!chainman.IsSnapshotValidated());
- BOOST_CHECK(!node::FindSnapshotChainstateDir(chainman.m_options.datadir));
+ BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
}
size_t initial_size;
@@ -240,7 +240,7 @@ struct SnapshotTestSetup : TestChain100Setup {
auto_infile >> coin;
}));
- BOOST_CHECK(!node::FindSnapshotChainstateDir(chainman.m_options.datadir));
+ BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(
this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) {
@@ -264,7 +264,7 @@ struct SnapshotTestSetup : TestChain100Setup {
}));
BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(this));
- BOOST_CHECK(fs::exists(*node::FindSnapshotChainstateDir(chainman.m_options.datadir)));
+ BOOST_CHECK(fs::exists(*node::FindAssumeutxoChainstateDir(chainman.m_options.datadir)));
// Ensure our active chain is the snapshot chainstate.
BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull());
@@ -277,7 +277,7 @@ struct SnapshotTestSetup : TestChain100Setup {
{
LOCK(::cs_main);
- fs::path found = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+ fs::path found = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
// Note: WriteSnapshotBaseBlockhash() is implicitly tested above.
BOOST_CHECK_EQUAL(
@@ -565,7 +565,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
this->SetupSnapshot();
- fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+ fs::path snapshot_chainstate_dir = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
@@ -639,13 +639,14 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
ChainstateManager& chainman = *Assert(m_node.chainman);
Chainstate& active_cs = chainman.ActiveChainstate();
+ Chainstate& validated_cs{*Assert(WITH_LOCK(cs_main, return chainman.HistoricalChainstate()))};
auto tip_cache_before_complete = active_cs.m_coinstip_cache_size_bytes;
auto db_cache_before_complete = active_cs.m_coinsdb_cache_size_bytes;
SnapshotCompletionResult res;
m_node.notifications->m_shutdown_on_fatal_error = false;
- fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+ fs::path snapshot_chainstate_dir = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
@@ -653,7 +654,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(),
return chainman.ActiveTip()->GetBlockHash());
- res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+ res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validated_cs, active_cs));
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS);
WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated()));
@@ -669,7 +670,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
BOOST_CHECK_EQUAL(all_chainstates[0], &active_cs);
// Trying completion again should return false.
- res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+ res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validated_cs, active_cs));
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SKIPPED);
// The invalid snapshot path should not have been used.
@@ -720,6 +721,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
{
auto chainstates = this->SetupSnapshot();
Chainstate& validation_chainstate = *std::get<0>(chainstates);
+ Chainstate& unvalidated_cs = *std::get<1>(chainstates);
ChainstateManager& chainman = *Assert(m_node.chainman);
SnapshotCompletionResult res;
m_node.notifications->m_shutdown_on_fatal_error = false;
@@ -740,7 +742,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
{
ASSERT_DEBUG_LOG("failed to validate the -assumeutxo snapshot state");
- res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+ res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validation_chainstate, unvalidated_cs));
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::HASH_MISMATCH);
}
diff --git a/src/validation.cpp b/src/validation.cpp
index 9e27da58..15981708 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3165,11 +3165,17 @@ bool Chainstate::ConnectTip(
Ticks<SecondsDouble>(m_chainman.time_total),
Ticks<MillisecondsDouble>(m_chainman.time_total) / m_chainman.num_blocks_total);
- // If we are the background validation chainstate, check to see if we are done
- // validating the snapshot (i.e. our tip has reached the snapshot's base block).
- if (this != &m_chainman.ActiveChainstate()) {
- m_chainman.MaybeCompleteSnapshotValidation();
- }
+ // See if this chainstate has reached a target block and can be used to
+ // validate an assumeutxo snapshot. If it can, hashing the UTXO database
+ // will be slow, and cs_main could remain locked here for several minutes.
+ // If the snapshot is validated, the UTXO hash will be saved to
+ // this->m_target_utxohash, causing HistoricalChainstate() to return null
+ // and this chainstate to no longer be used. ActivateBestChain() will also
+ // stop connecting blocks to this chainstate because this->ReachedTarget()
+ // will be true and this->setBlockIndexCandidates will not have additional
+ // blocks.
+ Chainstate& current_cs{*Assert(m_chainman.m_active_chainstate)};
+ m_chainman.MaybeValidateSnapshot(*this, current_cs);
connectTrace.BlockConnected(pindexNew, std::move(block_to_connect));
return true;
@@ -3527,7 +3533,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
// the target block.
//
// This cannot be done while holding cs_main (within
- // MaybeCompleteSnapshotValidation) or a cs_main deadlock will occur.
+ // MaybeValidateSnapshot) or a cs_main deadlock will occur.
if (m_chainman.snapshot_download_completed) {
m_chainman.snapshot_download_completed();
}
@@ -5761,7 +5767,7 @@ util::Result<CBlockIndex*> ChainstateManager::ActivateSnapshot(
// 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(m_options.datadir)) {
+ if (auto snapshot_datadir = node::FindAssumeutxoChainstateDir(m_options.datadir)) {
// 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.
@@ -6037,45 +6043,35 @@ util::Result<void> ChainstateManager::PopulateAndValidateSnapshot(
}
// Currently, this function holds cs_main for its duration, which could be for
-// multiple minutes due to the ComputeUTXOStats call. This hold is necessary
-// because we need to avoid advancing the background validation chainstate
-// farther than the snapshot base block - and this function is also invoked
-// from within ConnectTip, i.e. from within ActivateBestChain, so cs_main is
-// held anyway.
-//
-// Eventually (TODO), we could somehow separate this function's runtime from
-// maintenance of the active chain, but that will either require
+// multiple minutes due to the ComputeUTXOStats call. Holding cs_main used to be
+// necessary (before d43a1f1a2fa3) to avoid advancing validated_cs farther than
+// its target block. Now it should be possible to avoid this, but simply
+// releasing cs_main here would not be possible because this function is invoked
+// by ConnectTip within ActivateBestChain.
//
-// (i) setting `m_disabled` immediately and ensuring all chainstate accesses go
-// through IsUsable() checks, or
-//
-// (ii) giving each chainstate its own lock instead of using cs_main for everything.
-SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
+// Eventually (TODO) it would be better to call this function outside of
+// ActivateBestChain, on a separate thread that should not require cs_main to
+// hash, because the UTXO set is only hashed after the historical chainstate
+// reaches its target block and is no longer changing.
+SnapshotCompletionResult ChainstateManager::MaybeValidateSnapshot(Chainstate& validated_cs, Chainstate& unvalidated_cs)
{
AssertLockHeld(cs_main);
- // If a snapshot does not need to be validated...
- if (m_ibd_chainstate.get() == &this->ActiveChainstate() ||
+ // If the snapshot does not need to be validated...
+ if (unvalidated_cs.m_assumeutxo != Assumeutxo::UNVALIDATED ||
// Or if either chainstate is unusable...
- !m_snapshot_chainstate ||
- m_snapshot_chainstate->m_assumeutxo != Assumeutxo::UNVALIDATED ||
- !m_snapshot_chainstate->m_from_snapshot_blockhash ||
- !m_ibd_chainstate ||
- m_ibd_chainstate->m_assumeutxo != Assumeutxo::VALIDATED ||
- !m_ibd_chainstate->m_chain.Tip() ||
+ !unvalidated_cs.m_from_snapshot_blockhash ||
+ validated_cs.m_assumeutxo != Assumeutxo::VALIDATED ||
+ !validated_cs.m_chain.Tip() ||
// Or the validated chainstate is not targeting the snapshot block...
- !m_ibd_chainstate->m_target_blockhash ||
- *m_ibd_chainstate->m_target_blockhash != *m_snapshot_chainstate->m_from_snapshot_blockhash ||
+ !validated_cs.m_target_blockhash ||
+ *validated_cs.m_target_blockhash != *unvalidated_cs.m_from_snapshot_blockhash ||
// Or the validated chainstate has not reached the snapshot block yet...
- !m_ibd_chainstate->ReachedTarget()) {
- // Then a snapshot cannot be validated and there is nothing to do.
+ !validated_cs.ReachedTarget()) {
+ // Then the snapshot cannot be validated and there is nothing to do.
return SnapshotCompletionResult::SKIPPED;
}
- const int snapshot_tip_height = this->ActiveHeight();
- const int snapshot_base_height = *Assert(this->GetSnapshotBaseHeight());
- const CBlockIndex& index_new = *Assert(m_ibd_chainstate->m_chain.Tip());
- assert(SnapshotBlockhash());
- uint256 snapshot_blockhash = *Assert(SnapshotBlockhash());
+ assert(validated_cs.TargetBlock() == validated_cs.m_chain.Tip());
auto handle_invalid_snapshot = [&]() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
bilingual_str user_error = strprintf(_(
@@ -6090,19 +6086,20 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
"Please report this incident to %s, including how you obtained the snapshot. "
"The invalid snapshot chainstate will be left on disk in case it is "
"helpful in diagnosing the issue that caused this error."),
- CLIENT_NAME, snapshot_tip_height, snapshot_base_height, snapshot_base_height, CLIENT_BUGREPORT
- );
+ CLIENT_NAME, unvalidated_cs.m_chain.Height(),
+ validated_cs.m_chain.Height(),
+ validated_cs.m_chain.Height(), CLIENT_BUGREPORT);
LogError("[snapshot] !!! %s\n", user_error.original);
LogError("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n");
// Reset chainstate target to network tip instead of snapshot block.
- m_ibd_chainstate->SetTargetBlock(nullptr);
+ validated_cs.SetTargetBlock(nullptr);
- m_active_chainstate = m_ibd_chainstate.get();
- m_snapshot_chainstate->m_assumeutxo = Assumeutxo::INVALID;
+ m_active_chainstate = &validated_cs;
+ unvalidated_cs.m_assumeutxo = Assumeutxo::INVALID;
- auto rename_result = m_snapshot_chainstate->InvalidateCoinsDBOnDisk();
+ auto rename_result = unvalidated_cs.InvalidateCoinsDBOnDisk();
if (!rename_result) {
user_error += Untranslated("\n") + util::ErrorString(rename_result);
}
@@ -6110,34 +6107,25 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
GetNotifications().fatalError(user_error);
};
- assert(index_new.GetBlockHash() == snapshot_blockhash);
- assert(index_new.nHeight == snapshot_base_height);
-
- int curr_height = m_ibd_chainstate->m_chain.Height();
-
- assert(snapshot_base_height == curr_height);
- assert(snapshot_base_height == index_new.nHeight);
- assert(this->GetAll().size() == 2);
+ CCoinsViewDB& validated_coins_db = validated_cs.CoinsDB();
+ validated_cs.ForceFlushStateToDisk();
- CCoinsViewDB& ibd_coins_db = m_ibd_chainstate->CoinsDB();
- m_ibd_chainstate->ForceFlushStateToDisk();
-
- const auto& maybe_au_data = m_options.chainparams.AssumeutxoForHeight(curr_height);
+ const auto& maybe_au_data = m_options.chainparams.AssumeutxoForHeight(validated_cs.m_chain.Height());
if (!maybe_au_data) {
LogWarning("[snapshot] assumeutxo data not found for height "
- "(%d) - refusing to validate snapshot", curr_height);
+ "(%d) - refusing to validate snapshot", validated_cs.m_chain.Height());
handle_invalid_snapshot();
return SnapshotCompletionResult::MISSING_CHAINPARAMS;
}
const AssumeutxoData& au_data = *maybe_au_data;
- std::optional<CCoinsStats> maybe_ibd_stats;
+ std::optional<CCoinsStats> validated_cs_stats;
LogInfo("[snapshot] computing UTXO stats for background chainstate to validate "
"snapshot - this could take a few minutes");
try {
- maybe_ibd_stats = ComputeUTXOStats(
+ validated_cs_stats = ComputeUTXOStats(
CoinStatsHashType::HASH_SERIALIZED,
- &ibd_coins_db,
+ &validated_coins_db,
m_blockman,
[&interrupt = m_interrupt] { SnapshotUTXOHashBreakpoint(interrupt); });
} catch (StopHashingException const&) {
@@ -6145,7 +6133,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
}
// XXX note that this function is slow and will hold cs_main for potentially minutes.
- if (!maybe_ibd_stats) {
+ if (!validated_cs_stats) {
LogWarning("[snapshot] failed to generate stats for validation coins db");
// While this isn't a problem with the snapshot per se, this condition
// prevents us from validating the snapshot, so we should shut down and let the
@@ -6153,7 +6141,6 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
handle_invalid_snapshot();
return SnapshotCompletionResult::STATS_FAILED;
}
- const auto& ibd_stats = *maybe_ibd_stats;
// Compare the background validation chainstate's UTXO set hash against the hard-coded
// assumeutxo hash we expect.
@@ -6161,19 +6148,19 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
// TODO: For belt-and-suspenders, we could cache the UTXO set
// hash for the snapshot when it's loaded in its chainstate's leveldb. We could then
// reference that here for an additional check.
- if (AssumeutxoHash{ibd_stats.hashSerialized} != au_data.hash_serialized) {
+ if (AssumeutxoHash{validated_cs_stats->hashSerialized} != au_data.hash_serialized) {
LogWarning("[snapshot] hash mismatch: actual=%s, expected=%s",
- ibd_stats.hashSerialized.ToString(),
+ validated_cs_stats->hashSerialized.ToString(),
au_data.hash_serialized.ToString());
handle_invalid_snapshot();
return SnapshotCompletionResult::HASH_MISMATCH;
}
LogInfo("[snapshot] snapshot beginning at %s has been fully validated",
- snapshot_blockhash.ToString());
+ unvalidated_cs.m_from_snapshot_blockhash->ToString());
- m_snapshot_chainstate->m_assumeutxo = Assumeutxo::VALIDATED;
- m_ibd_chainstate->m_target_utxohash = AssumeutxoHash{ibd_stats.hashSerialized};
+ unvalidated_cs.m_assumeutxo = Assumeutxo::VALIDATED;
+ validated_cs.m_target_utxohash = AssumeutxoHash{validated_cs_stats->hashSerialized};
this->MaybeRebalanceCaches();
return SnapshotCompletionResult::SUCCESS;
@@ -6260,24 +6247,23 @@ ChainstateManager::~ChainstateManager()
m_versionbitscache.Clear();
}
-bool ChainstateManager::DetectSnapshotChainstate()
+Chainstate* ChainstateManager::LoadAssumeutxoChainstate()
{
assert(!m_snapshot_chainstate);
- std::optional<fs::path> path = node::FindSnapshotChainstateDir(m_options.datadir);
+ std::optional<fs::path> path = node::FindAssumeutxoChainstateDir(m_options.datadir);
if (!path) {
- return false;
+ return nullptr;
}
std::optional<uint256> base_blockhash = node::ReadSnapshotBaseBlockhash(*path);
if (!base_blockhash) {
- return false;
+ return nullptr;
}
LogInfo("[snapshot] detected active snapshot chainstate (%s) - loading",
fs::PathToString(*path));
auto snapshot_chainstate{std::make_unique<Chainstate>(nullptr, m_blockman, *this, base_blockhash)};
LogInfo("[snapshot] switching active chainstate to %s", snapshot_chainstate->ToString());
- this->AddChainstate(std::move(snapshot_chainstate));
- return true;
+ return &this->AddChainstate(std::move(snapshot_chainstate));
}
Chainstate& ChainstateManager::AddChainstate(std::unique_ptr<Chainstate> chainstate)
@@ -6337,7 +6323,7 @@ util::Result<void> Chainstate::InvalidateCoinsDBOnDisk()
// The invalid snapshot datadir is simply moved and not deleted because we may
// want to do forensics later during issue investigation. The user is instructed
- // accordingly in MaybeCompleteSnapshotValidation().
+ // accordingly in MaybeValidateSnapshot().
try {
fs::rename(snapshot_datadir, invalid_path);
} catch (const fs::filesystem_error& e) {
@@ -6362,7 +6348,7 @@ bool ChainstateManager::DeleteSnapshotChainstate()
Assert(m_snapshot_chainstate);
Assert(m_ibd_chainstate);
- fs::path snapshot_datadir = Assert(node::FindSnapshotChainstateDir(m_options.datadir)).value();
+ fs::path snapshot_datadir = Assert(node::FindAssumeutxoChainstateDir(m_options.datadir)).value();
if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) {
LogError("Deletion of %s failed. Please remove it manually to continue reindexing.",
fs::PathToString(snapshot_datadir));
@@ -6384,12 +6370,6 @@ const CBlockIndex* ChainstateManager::GetSnapshotBaseBlock() const
return m_active_chainstate ? m_active_chainstate->SnapshotBase() : nullptr;
}
-std::optional<int> ChainstateManager::GetSnapshotBaseHeight() const
-{
- const CBlockIndex* base = this->GetSnapshotBaseBlock();
- return base ? std::make_optional(base->nHeight) : std::nullopt;
-}
-
void ChainstateManager::RecalculateBestHeader()
{
AssertLockHeld(cs_main);
diff --git a/src/validation.h b/src/validation.h
index 5e6d11d1..6fe31117 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1131,14 +1131,15 @@ public:
[[nodiscard]] util::Result<CBlockIndex*> ActivateSnapshot(
AutoFile& coins_file, const node::SnapshotMetadata& metadata, bool in_memory);
- //! Once the background validation chainstate has reached the height which
- //! is the base of the UTXO snapshot in use, compare its coins to ensure
- //! they match those expected by the snapshot.
- //!
- //! If the coins match (expected), then mark the validation chainstate for
- //! deletion and continue using the snapshot chainstate as active.
- //! Otherwise, revert to using the ibd chainstate and shutdown.
- SnapshotCompletionResult MaybeCompleteSnapshotValidation() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+ //! Try to validate an assumeutxo snapshot by using a validated historical
+ //! chainstate targeted at the snapshot block. When the target block is
+ //! reached, the UTXO hash is computed and saved to
+ //! `validated_cs.m_target_utxohash`, and `unvalidated_cs.m_assumeutxo` will
+ //! be updated from UNVALIDATED to either VALIDATED or INVALID depending on
+ //! whether the hash matches. The INVALID case should not happen in practice
+ //! because the software should refuse to load unrecognized snapshots, but
+ //! if it does happen, it is a fatal error.
+ SnapshotCompletionResult MaybeValidateSnapshot(Chainstate& validated_cs, Chainstate& unvalidated_cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
//! Returns nullptr if no snapshot has been loaded.
const CBlockIndex* GetSnapshotBaseBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
@@ -1312,8 +1313,9 @@ public:
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() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+ //! snapshot that is in the process of being validated and load it if found.
+ //! Return pointer to the Chainstate if it is loaded.
+ Chainstate* LoadAssumeutxoChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
//! Add new chainstate.
Chainstate& AddChainstate(std::unique_ptr<Chainstate> chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
@@ -1351,10 +1353,6 @@ public:
std::pair<int, int> GetPruneRange(
const Chainstate& chainstate, int last_height_can_prune) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
- //! Return the height of the base block of the snapshot in use, if one exists, else
- //! nullopt.
- std::optional<int> GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-
//! Get range of historical blocks to download.
std::optional<std::pair<const CBlockIndex*, const CBlockIndex*>> GetHistoricalBlockRange() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.