From 1598a15aedb9fd9c4e4a671785ebebf56fc1e072 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 30 May 2024 09:42:03 -0400 Subject: [PATCH] refactor: Deduplicate Chainstate activation code Move duplicate code from ChainstateManager::ActivateSnapshot and ChainstateManager::ActivateExistingSnapshot methods to a new ChainstateManager::AddChainstate method. The "AddChainstate" method name doesn't mention snapshots even though it is only used to add snapshot chainstates now, because it becomes more generalized in a later commit in this PR ("refactor: Add ChainstateManager::m_chainstates member") --- .../validation_chainstatemanager_tests.cpp | 7 ++-- src/validation.cpp | 34 ++++++------------- src/validation.h | 7 ++-- 3 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index c6ae6088ec2..03cec59c3dc 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -69,7 +69,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup) // Create a snapshot-based chainstate. // const uint256 snapshot_blockhash = active_tip->GetBlockHash(); - Chainstate& c2 = WITH_LOCK(::cs_main, return manager.ActivateExistingSnapshot(snapshot_blockhash)); + Chainstate& c2{WITH_LOCK(::cs_main, return manager.AddChainstate(std::make_unique(nullptr, manager.m_blockman, manager, snapshot_blockhash)))}; chainstates.push_back(&c2); c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); @@ -135,7 +135,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) // Create a snapshot-based chainstate. // CBlockIndex* snapshot_base{WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()[manager.ActiveChain().Height() / 2])}; - Chainstate& c2 = WITH_LOCK(cs_main, return manager.ActivateExistingSnapshot(*snapshot_base->phashBlock)); + Chainstate& c2{WITH_LOCK(::cs_main, return manager.AddChainstate(std::make_unique(nullptr, manager.m_blockman, manager, *snapshot_base->phashBlock)))}; chainstates.push_back(&c2); c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); @@ -489,8 +489,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) } // Note: cs2's tip is not set when ActivateExistingSnapshot is called. - Chainstate& cs2 = WITH_LOCK(::cs_main, - return chainman.ActivateExistingSnapshot(*assumed_base->phashBlock)); + Chainstate& cs2{WITH_LOCK(::cs_main, return chainman.AddChainstate(std::make_unique(nullptr, chainman.m_blockman, chainman, *assumed_base->phashBlock)))}; // Set tip of the fully validated chain to be the validated tip cs1.m_chain.SetTip(*validated_tip); diff --git a/src/validation.cpp b/src/validation.cpp index 4c6432de84c..9e27da58dfd 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5796,28 +5796,14 @@ util::Result ChainstateManager::ActivateSnapshot( } } - assert(!m_snapshot_chainstate); - m_snapshot_chainstate.swap(snapshot_chainstate); - const bool chaintip_loaded = m_snapshot_chainstate->LoadChainTip(); + Chainstate& chainstate{AddChainstate(std::move(snapshot_chainstate))}; + const bool chaintip_loaded{chainstate.LoadChainTip()}; assert(chaintip_loaded); - - // Set snapshot block as the target block for the historical chainstate. - assert(m_ibd_chainstate.get()); - assert(!m_ibd_chainstate->m_target_blockhash); - m_ibd_chainstate->SetTargetBlockHash(base_blockhash); - - // Transfer possession of the mempool to the snapshot 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(); - m_blockman.m_snapshot_height = this->GetSnapshotBaseHeight(); + m_blockman.m_snapshot_height = Assert(chainstate.SnapshotBase())->nHeight; LogInfo("[snapshot] successfully activated snapshot %s", base_blockhash.ToString()); LogInfo("[snapshot] (%.2f MB)", - m_snapshot_chainstate->CoinsTip().DynamicMemoryUsage() / (1000 * 1000)); + chainstate.CoinsTip().DynamicMemoryUsage() / (1000 * 1000)); this->MaybeRebalanceCaches(); return snapshot_start_block; @@ -6288,21 +6274,21 @@ bool ChainstateManager::DetectSnapshotChainstate() LogInfo("[snapshot] detected active snapshot chainstate (%s) - loading", fs::PathToString(*path)); - this->ActivateExistingSnapshot(*base_blockhash); + auto snapshot_chainstate{std::make_unique(nullptr, m_blockman, *this, base_blockhash)}; + LogInfo("[snapshot] switching active chainstate to %s", snapshot_chainstate->ToString()); + this->AddChainstate(std::move(snapshot_chainstate)); return true; } -Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash) +Chainstate& ChainstateManager::AddChainstate(std::unique_ptr chainstate) { assert(!m_snapshot_chainstate); - m_snapshot_chainstate = - std::make_unique(nullptr, m_blockman, *this, base_blockhash); - LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString()); + m_snapshot_chainstate = std::move(chainstate); // 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(base_blockhash); + m_ibd_chainstate->SetTargetBlockHash(*Assert(m_snapshot_chainstate->m_from_snapshot_blockhash)); // Transfer possession of the mempool to the chainstate. // Mempool is empty at this point because we're still in IBD. diff --git a/src/validation.h b/src/validation.h index d2d9e4196d2..5e6d11d1c28 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1315,16 +1315,15 @@ public: //! snapshot that is in the process of being validated. bool DetectSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Add new chainstate. + Chainstate& AddChainstate(std::unique_ptr chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void ResetChainstates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Remove the snapshot-based chainstate and all on-disk artifacts. //! Used when reindex{-chainstate} is called during snapshot use. [[nodiscard]] bool DeleteSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - //! Switch the active chainstate to one based on a UTXO snapshot that was loaded - //! previously. - Chainstate& ActivateExistingSnapshot(uint256 base_blockhash) 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 //! validation of the snapshot.