diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 2c27ccf464f..59d6de63e5a 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -175,7 +175,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // 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()) { + if (!chainman.DeleteChainstate(*assumeutxo_cs)) { return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")}; } assumeutxo_cs = nullptr; diff --git a/src/validation.cpp b/src/validation.cpp index 83a37d07079..0d16dc209a6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3180,7 +3180,7 @@ bool Chainstate::ConnectTip( // 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)}; + Chainstate& current_cs{m_chainman.CurrentChainstate()}; m_chainman.MaybeValidateSnapshot(*this, current_cs); connectTrace.BlockConnected(pindexNew, std::move(block_to_connect)); @@ -3843,7 +3843,7 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd // m_chain_tx_count value is not zero, assert that value is actually correct. auto prev_tx_sum = [](CBlockIndex& block) { return block.nTx + (block.pprev ? block.pprev->m_chain_tx_count : 0); }; if (!Assume(pindexNew->m_chain_tx_count == 0 || pindexNew->m_chain_tx_count == prev_tx_sum(*pindexNew) || - pindexNew == CurrentChainstate().SnapshotBase())) { + std::ranges::any_of(m_chainstates, [&](const auto& cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->SnapshotBase() == pindexNew; }))) { LogWarning("Internal bug detected: block %d has unexpected m_chain_tx_count %i that should be %i (%s %s). Please report this issue here: %s\n", pindexNew->nHeight, pindexNew->m_chain_tx_count, prev_tx_sum(*pindexNew), CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT); pindexNew->m_chain_tx_count = 0; @@ -5307,8 +5307,8 @@ void ChainstateManager::CheckBlockIndex() const if (pindex->pprev == nullptr) { // Genesis block checks. assert(pindex->GetBlockHash() == GetConsensus().hashGenesisBlock); // Genesis block's hash must match. - for (const Chainstate* c : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) { - if (c && c->m_chain.Genesis() != nullptr) { + for (const auto& c : m_chainstates) { + if (c->m_chain.Genesis() != nullptr) { assert(pindex == c->m_chain.Genesis()); // The chain's genesis block must be this block. } } @@ -5367,8 +5367,8 @@ void ChainstateManager::CheckBlockIndex() const assert((pindex->nStatus & BLOCK_FAILED_MASK) || pindex->nChainWork <= m_best_header->nChainWork); // Chainstate-specific checks on setBlockIndexCandidates - for (const Chainstate* c : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) { - if (!c || c->m_chain.Tip() == nullptr) continue; + for (const auto& c : m_chainstates) { + if (c->m_chain.Tip() == nullptr) continue; // Two main factors determine whether pindex is a candidate in // setBlockIndexCandidates: // @@ -5406,12 +5406,12 @@ void ChainstateManager::CheckBlockIndex() const // is the base of the snapshot, pindex is also a potential // candidate. if (pindexFirstMissing == nullptr || pindex == c->m_chain.Tip() || pindex == c->SnapshotBase()) { - // If this chainstate is the active chainstate, pindex - // must be in setBlockIndexCandidates. Otherwise, this - // chainstate is a background validation chainstate, and - // pindex only needs to be added if it is an ancestor of - // the snapshot that is being validated. - if (c == &ActiveChainstate() || snap_base->GetAncestor(pindex->nHeight) == pindex) { + // If this chainstate is not a historical chainstate + // targeting a specific block, pindex must be in + // setBlockIndexCandidates. Otherwise, pindex only + // needs to be added if it is an ancestor of the target + // block. + if (!c->TargetBlock() || c->TargetBlock()->GetAncestor(pindex->nHeight) == pindex) { assert(c->setBlockIndexCandidates.contains(const_cast(pindex))); } } @@ -5451,12 +5451,10 @@ void ChainstateManager::CheckBlockIndex() const // tip. // So if this block is itself better than any m_chain.Tip() and it wasn't in // setBlockIndexCandidates, then it must be in m_blocks_unlinked. - for (const Chainstate* c : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) { - if (!c) continue; - const bool is_active = c == &ActiveChainstate(); + for (const auto& c : m_chainstates) { if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && !c->setBlockIndexCandidates.contains(const_cast(pindex))) { if (pindexFirstInvalid == nullptr) { - if (is_active || snap_base->GetAncestor(pindex->nHeight) == pindex) { + if (!c->TargetBlock() || c->TargetBlock()->GetAncestor(pindex->nHeight) == pindex) { assert(foundInUnlinked); } } @@ -5610,8 +5608,8 @@ std::vector ChainstateManager::GetAll() LOCK(::cs_main); std::vector out; - for (Chainstate* cs : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) { - if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && !cs->m_target_utxohash) out.push_back(cs); + for (const auto& cs : m_chainstates) { + if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && !cs->m_target_utxohash) out.push_back(cs.get()); } return out; @@ -5620,12 +5618,9 @@ std::vector ChainstateManager::GetAll() Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool) { AssertLockHeld(::cs_main); - assert(!m_ibd_chainstate); - assert(!m_active_chainstate); - - m_ibd_chainstate = std::make_unique(mempool, m_blockman, *this); - m_active_chainstate = m_ibd_chainstate.get(); - return *m_active_chainstate; + assert(m_chainstates.empty()); + m_chainstates.emplace_back(std::make_unique(mempool, m_blockman, *this)); + return *m_chainstates.back(); } [[nodiscard]] static bool DeleteCoinsDBFromDisk(const fs::path db_path, bool is_snapshot) @@ -5706,7 +5701,7 @@ util::Result ChainstateManager::ActivateSnapshot( return util::Error{Untranslated("A forked headers-chain with more work than the chain with the snapshot base block header exists. Please proceed to sync without AssumeUtxo.")}; } - auto mempool{m_active_chainstate->GetMempool()}; + auto mempool{CurrentChainstate().GetMempool()}; if (mempool && mempool->size() > 0) { return util::Error{Untranslated("Can't activate a snapshot when mempool not empty")}; } @@ -6091,7 +6086,6 @@ SnapshotCompletionResult ChainstateManager::MaybeValidateSnapshot(Chainstate& va // Reset chainstate target to network tip instead of snapshot block. validated_cs.SetTargetBlock(nullptr); - m_active_chainstate = &validated_cs; unvalidated_cs.m_assumeutxo = Assumeutxo::INVALID; auto rename_result = unvalidated_cs.InvalidateCoinsDBOnDisk(); @@ -6164,14 +6158,13 @@ SnapshotCompletionResult ChainstateManager::MaybeValidateSnapshot(Chainstate& va Chainstate& ChainstateManager::ActiveChainstate() const { LOCK(::cs_main); - assert(m_active_chainstate); - return *m_active_chainstate; + return CurrentChainstate(); } void ChainstateManager::MaybeRebalanceCaches() { AssertLockHeld(::cs_main); - Chainstate& current_cs{*Assert(m_active_chainstate)}; + Chainstate& current_cs{CurrentChainstate()}; Chainstate* historical_cs{HistoricalChainstate()}; if (!historical_cs && !current_cs.m_from_snapshot_blockhash) { // Allocate everything to the IBD chainstate. This will always happen @@ -6202,9 +6195,7 @@ void ChainstateManager::MaybeRebalanceCaches() void ChainstateManager::ResetChainstates() { - m_ibd_chainstate.reset(); - m_snapshot_chainstate.reset(); - m_active_chainstate = nullptr; + m_chainstates.clear(); } /** @@ -6238,7 +6229,7 @@ ChainstateManager::~ChainstateManager() Chainstate* ChainstateManager::LoadAssumeutxoChainstate() { - assert(!m_snapshot_chainstate); + assert(!CurrentChainstate().m_from_snapshot_blockhash); std::optional path = node::FindAssumeutxoChainstateDir(m_options.datadir); if (!path) { return nullptr; @@ -6257,22 +6248,21 @@ Chainstate* ChainstateManager::LoadAssumeutxoChainstate() Chainstate& ChainstateManager::AddChainstate(std::unique_ptr chainstate) { - assert(!m_snapshot_chainstate); - m_snapshot_chainstate = std::move(chainstate); - + Chainstate& prev_chainstate{CurrentChainstate()}; + assert(prev_chainstate.m_assumeutxo == Assumeutxo::VALIDATED); // Set target block for historical chainstate to snapshot block. - assert(m_ibd_chainstate.get()); - assert(!m_ibd_chainstate->m_target_blockhash); - m_ibd_chainstate->SetTargetBlockHash(*Assert(m_snapshot_chainstate->m_from_snapshot_blockhash)); + assert(!prev_chainstate.m_target_blockhash); + prev_chainstate.m_target_blockhash = chainstate->m_from_snapshot_blockhash; + m_chainstates.push_back(std::move(chainstate)); + Chainstate& curr_chainstate{CurrentChainstate()}; + assert(&curr_chainstate == m_chainstates.back().get()); // Transfer possession of the mempool to the chainstate. // Mempool is empty at this point because we're still in IBD. - Assert(m_active_chainstate->m_mempool->size() == 0); - Assert(!m_snapshot_chainstate->m_mempool); - m_snapshot_chainstate->m_mempool = m_active_chainstate->m_mempool; - m_active_chainstate->m_mempool = nullptr; - m_active_chainstate = m_snapshot_chainstate.get(); - return *m_snapshot_chainstate; + assert(prev_chainstate.m_mempool->size() == 0); + assert(!curr_chainstate.m_mempool); + std::swap(curr_chainstate.m_mempool, prev_chainstate.m_mempool); + return curr_chainstate; } bool IsBIP30Repeat(const CBlockIndex& block_index) @@ -6319,21 +6309,21 @@ util::Result Chainstate::InvalidateCoinsDBOnDisk() return {}; } -bool ChainstateManager::DeleteSnapshotChainstate() +bool ChainstateManager::DeleteChainstate(Chainstate& chainstate) { AssertLockHeld(::cs_main); - Assert(m_snapshot_chainstate); - Assert(m_ibd_chainstate); - - fs::path snapshot_datadir = Assert(node::FindAssumeutxoChainstateDir(m_options.datadir)).value(); - if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) { + assert(!chainstate.m_coins_views); + const fs::path db_path{chainstate.StoragePath()}; + if (!DeleteCoinsDBFromDisk(db_path, /*is_snapshot=*/bool{chainstate.m_from_snapshot_blockhash})) { LogError("Deletion of %s failed. Please remove it manually to continue reindexing.", - fs::PathToString(snapshot_datadir)); + fs::PathToString(db_path)); return false; } - m_active_chainstate = m_ibd_chainstate.get(); - m_active_chainstate->m_mempool = m_snapshot_chainstate->m_mempool; - m_snapshot_chainstate.reset(); + std::unique_ptr prev_chainstate{Assert(RemoveChainstate(chainstate))}; + Chainstate& curr_chainstate{CurrentChainstate()}; + assert(prev_chainstate->m_mempool->size() == 0); + assert(!curr_chainstate.m_mempool); + std::swap(curr_chainstate.m_mempool, prev_chainstate->m_mempool); return true; } diff --git a/src/validation.h b/src/validation.h index 450c7ca887e..f330d90af4c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -931,40 +931,6 @@ enum class SnapshotCompletionResult { class ChainstateManager { private: - //! The chainstate used under normal operation (i.e. "regular" IBD) or, if - //! a snapshot is in use, for background validation. - //! - //! Its contents (including on-disk data) will be deleted *upon shutdown* - //! after background validation of the snapshot has completed. We do not - //! free the chainstate contents immediately after it finishes validation - //! to cautiously avoid a case where some other part of the system is still - //! using this pointer (e.g. net_processing). - //! - //! Once this pointer is set to a corresponding chainstate, it will not - //! be reset until init.cpp:Shutdown(). - //! - //! It is important for the pointer to not be deleted until shutdown, - //! because cs_main is not always held when the pointer is accessed, for - //! example when calling ActivateBestChain, so there's no way you could - //! prevent code from using the pointer while deleting it. - std::unique_ptr m_ibd_chainstate GUARDED_BY(::cs_main); - - //! A chainstate initialized on the basis of a UTXO snapshot. If this is - //! non-null, it is always our active chainstate. - //! - //! Once this pointer is set to a corresponding chainstate, it will not - //! be reset until init.cpp:Shutdown(). - //! - //! It is important for the pointer to not be deleted until shutdown, - //! because cs_main is not always held when the pointer is accessed, for - //! example when calling ActivateBestChain, so there's no way you could - //! prevent code from using the pointer while deleting it. - std::unique_ptr m_snapshot_chainstate GUARDED_BY(::cs_main); - - //! Points to either the ibd or snapshot chainstate; indicates our - //! most-work chain. - Chainstate* m_active_chainstate GUARDED_BY(::cs_main) {nullptr}; - CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr}; /** The last header for which a headerTip notification was issued. */ @@ -1128,8 +1094,7 @@ public: //! per assumeutxo chain parameters. //! - Wait for our headers chain to include the base block of the snapshot. //! - "Fast forward" the tip of the new chainstate to the base of the snapshot. - //! - Move the new chainstate to `m_snapshot_chainstate` and make it our - //! ChainstateActive(). + //! - Construct the new Chainstate and add it to m_chainstates. [[nodiscard]] util::Result ActivateSnapshot( AutoFile& coins_file, const node::SnapshotMetadata& metadata, bool in_memory); @@ -1146,14 +1111,19 @@ public: //! Return current chainstate targeting the most-work, network tip. Chainstate& CurrentChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { - return *Assert(m_active_chainstate); + for (auto& cs : m_chainstates) { + if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && !cs->m_target_blockhash) return *cs; + } + abort(); } //! Return historical chainstate targeting a specific block, if any. Chainstate* HistoricalChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { - auto* cs{m_ibd_chainstate.get()}; - return cs && cs->m_target_blockhash && !cs->m_target_utxohash ? cs : nullptr; + for (auto& cs : m_chainstates) { + if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && cs->m_target_blockhash && !cs->m_target_utxohash) return cs.get(); + } + return nullptr; } //! Return fully validated chainstate that should be used for indexing, to @@ -1167,6 +1137,18 @@ public: abort(); } + //! Remove a chainstate. + std::unique_ptr RemoveChainstate(Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) + { + auto it{std::find_if(m_chainstates.begin(), m_chainstates.end(), [&](auto& cs) { return cs.get() == &chainstate; })}; + if (it != m_chainstates.end()) { + auto ret{std::move(*it)}; + m_chainstates.erase(it); + return ret; + } + return nullptr; + } + //! Alternatives to CurrentChainstate() used by older code to query latest //! chainstate information without locking cs_main. Newer code should avoid //! querying ChainstateManager and use Chainstate objects directly, or @@ -1331,9 +1313,9 @@ public: void ResetChainstates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - //! Remove the snapshot-based chainstate and all on-disk artifacts. + //! Remove the chainstate and all on-disk artifacts. //! Used when reindex{-chainstate} is called during snapshot use. - [[nodiscard]] bool DeleteSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + [[nodiscard]] bool DeleteChainstate(Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! If we have validated a snapshot chain during this runtime, copy its //! chainstate directory over to the main `chainstate` location, completing @@ -1363,6 +1345,15 @@ public: CCheckQueue& GetCheckQueue() { return m_script_check_queue; } ~ChainstateManager(); + + //! List of chainstates. Note: in general, it is not safe to delete + //! Chainstate objects once they are added to this list because there is no + //! mutex that can be locked to prevent Chainstate pointers from being used + //! while they are deleted. (cs_main doesn't work because it is too narrow + //! and is released in the middle of Chainstate::ActivateBestChain to let + //! notifications be processed. m_chainstate_mutex doesn't work because it + //! is not locked at other times when the chainstate is in use.) + std::vector> m_chainstates GUARDED_BY(::cs_main); }; /** Deployment* info via ChainstateManager */