From ee35250683ab9a395b70a0e90ebc68b1858387c7 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 30 May 2024 16:14:42 -0400 Subject: [PATCH] refactor: Delete ChainstateManager::IsSnapshotValidated() method IsSnapshotValidated() is only called one place outside of tests, and is use redundantly in some tests, asserting that a snapshot is not validated when a snapshot chainstate does not even exist. Simplify by dropping the method and checking Chainstate m_assumeutxo field directly. --- src/node/chainstate.cpp | 1 - src/test/validation_chainstatemanager_tests.cpp | 10 +++------- src/validation.cpp | 2 +- src/validation.h | 6 ------ 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index a301089811f..2c27ccf464f 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -210,7 +210,6 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // ChainstateManager::ResetChainstates(), reinitialize them here without // duplicating the blockindex work above. assert(chainman.GetAll().empty()); - assert(!chainman.IsSnapshotValidated()); chainman.InitializeChainstate(options.mempool); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 5d0bc907596..0dfc34f760c 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -50,7 +50,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup) chainstates.push_back(&c1); BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash)); - BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated())); auto all = manager.GetAll(); BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end()); @@ -85,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup) BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash); BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_from_snapshot_blockhash)); - BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated())); + BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED)); BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate()); BOOST_CHECK(&c1 != &manager.ActiveChainstate()); auto all2 = manager.GetAll(); @@ -185,7 +184,6 @@ struct SnapshotTestSetup : TestChain100Setup { { LOCK(::cs_main); BOOST_CHECK(!chainman.CurrentChainstate().m_from_snapshot_blockhash); - BOOST_CHECK(!chainman.IsSnapshotValidated()); BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir)); } @@ -611,7 +609,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup) BOOST_CHECK_EQUAL(chainstates[1]->m_chain.Height(), 210); BOOST_CHECK(chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash); - BOOST_CHECK(!chainman_restarted.IsSnapshotValidated()); + BOOST_CHECK(chainman_restarted.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED); BOOST_CHECK_EQUAL(chainman_restarted.ActiveTip()->GetBlockHash(), snapshot_tip_hash); BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210); @@ -656,7 +654,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup 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())); + BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::VALIDATED)); BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash)); // Cache should have been rebalanced and reallocated to the "only" remaining @@ -699,7 +697,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup LOCK(chainman_restarted.GetMutex()); BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1); BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash); - BOOST_CHECK(!chainman_restarted.IsSnapshotValidated()); BOOST_CHECK(active_cs2.m_coinstip_cache_size_bytes > tip_cache_before_complete); BOOST_CHECK(active_cs2.m_coinsdb_cache_size_bytes > db_cache_before_complete); @@ -771,7 +768,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna LOCK(::cs_main); BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1); BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash); - BOOST_CHECK(!chainman_restarted.IsSnapshotValidated()); BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210); } diff --git a/src/validation.cpp b/src/validation.cpp index ff868911da9..6ed044e2a41 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6367,7 +6367,7 @@ void ChainstateManager::RecalculateBestHeader() bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs) { AssertLockHeld(::cs_main); - if (!this->IsSnapshotValidated()) { + if (unvalidated_cs.m_assumeutxo != Assumeutxo::VALIDATED) { // No need to clean up. return false; } diff --git a/src/validation.h b/src/validation.h index 30d3c2aec37..336c1af66ef 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1191,12 +1191,6 @@ public: std::optional SnapshotBlockhash() const; - //! Is there a snapshot in use and has it been fully validated? - bool IsSnapshotValidated() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) - { - return m_snapshot_chainstate && m_snapshot_chainstate->m_assumeutxo == Assumeutxo::VALIDATED; - } - /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload() const;