diff --git a/src/chain.h b/src/chain.h index 466276c01a7d..5f1f1fbdab51 100644 --- a/src/chain.h +++ b/src/chain.h @@ -420,6 +420,15 @@ class CChain return int(vChain.size()) - 1; } + /** Check whether this chain's tip exists, has enough work, and is recent. */ + bool IsTipRecent(const arith_uint256& min_chain_work, std::chrono::seconds max_tip_age) const + { + const auto tip{Tip()}; + return tip && + tip->nChainWork >= min_chain_work && + tip->Time() >= Now() - max_tip_age; + } + /** Set/initialize a chain with a given tip. */ void SetTip(CBlockIndex& block); diff --git a/src/test/fuzz/p2p_handshake.cpp b/src/test/fuzz/p2p_handshake.cpp index aec7387eb2fd..8a6937088780 100644 --- a/src/test/fuzz/p2p_handshake.cpp +++ b/src/test/fuzz/p2p_handshake.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -43,9 +44,9 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); auto& connman = static_cast(*g_setup->m_node.connman); - auto& chainman = static_cast(*g_setup->m_node.chainman); + auto& chainman = *g_setup->m_node.chainman; SetMockTime(1610000000); // any time to successfully reset ibd - chainman.ResetIbd(); + Assert(chainman.IsInitialBlockDownload()); node::Warnings warnings{}; NetGroupManager netgroupman{{}}; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 809831a6efea..7b68be045169 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -35,7 +35,7 @@ std::string_view LIMIT_TO_MESSAGE_TYPE{}; void ResetChainman(TestingSetup& setup) { - SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time()); + SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time() + std::chrono::seconds{48 * 60 * 60}); setup.m_node.chainman.reset(); setup.m_make_chainman(); setup.LoadVerifyActivateChainstate(); @@ -71,9 +71,8 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) connman.ResetAddrCache(); connman.ResetMaxOutboundCycle(); auto& chainman = static_cast(*g_setup->m_node.chainman); + Assert(chainman.IsInitialBlockDownload()); const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; - SetMockTime(1610000000); // any time to successfully reset ibd - chainman.ResetIbd(); chainman.DisableNextWrite(); node::Warnings warnings{}; diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index baaeffe3dbf0..c108dd85093d 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -30,7 +31,7 @@ TestingSetup* g_setup; void ResetChainman(TestingSetup& setup) { - SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time()); + SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time() + std::chrono::seconds{48 * 60 * 60}); setup.m_node.chainman.reset(); setup.m_make_chainman(); setup.LoadVerifyActivateChainstate(); @@ -61,9 +62,8 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) connman.ResetAddrCache(); connman.ResetMaxOutboundCycle(); auto& chainman = static_cast(*g_setup->m_node.chainman); + Assert(chainman.IsInitialBlockDownload()); const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; - SetMockTime(1610000000); // any time to successfully reset ibd - chainman.ResetIbd(); chainman.DisableNextWrite(); node::Warnings warnings{}; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index aea29169bb32..467c7c754bea 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -801,8 +802,6 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle) BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) { - LOCK(NetEventsInterface::g_msgproc_mutex); - // Tests the following scenario: // * -bind=3.4.5.6:20001 is specified // * we make an outbound connection to a peer @@ -838,10 +837,15 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) const uint64_t services{NODE_NETWORK | NODE_WITNESS}; const int64_t time{0}; - // Force ChainstateManager::IsInitialBlockDownload() to return false. - // Otherwise PushAddress() isn't called by PeerManager::ProcessMessage(). - auto& chainman = static_cast(*m_node.chainman); - chainman.JumpOutOfIbd(); + // Ensure ChainstateManager::IsInitialBlockDownload() is false, otherwise + // PushAddress() isn't called by PeerManager::ProcessMessage(). + BOOST_CHECK(m_node.chainman->IsInitialBlockDownload()); + SetMockTime(WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1)); + MineBlock(m_node, {}); + BOOST_CHECK(!m_node.chainman->IsInitialBlockDownload()); + BOOST_CHECK(m_node.chainman->m_cached_finished_ibd.load(std::memory_order_relaxed)); + + LOCK(NetEventsInterface::g_msgproc_mutex); m_node.peerman->InitializeNode(peer, NODE_NETWORK); @@ -891,7 +895,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) BOOST_CHECK(sent); CaptureMessage = CaptureMessageOrig; - chainman.ResetIbd(); m_node.connman->SetCaptureMessages(false); m_node.args->ForceSetArg("-bind", ""); } diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp index bb9c6db22551..960f307a8bb6 100644 --- a/src/test/util/validation.cpp +++ b/src/test/util/validation.cpp @@ -30,19 +30,6 @@ void TestChainstateManager::DisableNextWrite() } } -void TestChainstateManager::ResetIbd() -{ - m_cached_finished_ibd = false; - assert(IsInitialBlockDownload()); -} - -void TestChainstateManager::JumpOutOfIbd() -{ - Assert(IsInitialBlockDownload()); - m_cached_finished_ibd = true; - Assert(!IsInitialBlockDownload()); -} - void ValidationInterfaceTest::BlockConnected( const ChainstateRole& role, CValidationInterface& obj, diff --git a/src/test/util/validation.h b/src/test/util/validation.h index 1be3f6a64db6..090c5e2385c5 100644 --- a/src/test/util/validation.h +++ b/src/test/util/validation.h @@ -20,10 +20,6 @@ struct TestBlockManager : public node::BlockManager { struct TestChainstateManager : public ChainstateManager { /** Disable the next write of all chainstates */ void DisableNextWrite(); - /** Reset the ibd cache to its initial state */ - void ResetIbd(); - /** Toggle IsInitialBlockDownload from true to false */ - void JumpOutOfIbd(); /** Wrappers that avoid making chainstatemanager internals public for tests */ void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 24122bd31579..e455ebc8a455 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -111,7 +111,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup) } //! Test rebalancing the caches associated with each chainstate. -BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) +BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, RegTestingSetup) { ChainstateManager& manager = *m_node.chainman; @@ -142,15 +142,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); - // Reset IBD state so IsInitialBlockDownload() returns true and causes - // MaybeRebalancesCaches() to prioritize the snapshot chainstate, giving it - // more cache space than the snapshot chainstate. Calling ResetIbd() is - // necessary because m_cached_finished_ibd is already latched to true before - // the test starts due to the test setup. After ResetIbd() is called. - // IsInitialBlockDownload will return true because at this point the active - // chainstate has a null chain tip. - static_cast(manager).ResetIbd(); - + BOOST_CHECK(manager.IsInitialBlockDownload()); { LOCK(::cs_main); c2.InitCoinsCache(1 << 23); diff --git a/src/validation.cpp b/src/validation.cpp index b6da6d2d8d6f..8169eca96ec3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1946,28 +1946,18 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) // bool ChainstateManager::IsInitialBlockDownload() const { - // Optimization: pre-test latch before taking the lock. - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) + if (m_cached_finished_ibd.load(std::memory_order_relaxed)) { return false; - - LOCK(cs_main); - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) - return false; - if (m_blockman.LoadingBlocks()) { - return true; } - CChain& chain{ActiveChain()}; - if (chain.Tip() == nullptr) { + if (m_blockman.LoadingBlocks()) { return true; } - if (chain.Tip()->nChainWork < MinimumChainWork()) { + if (!m_cached_tip_recent.load(std::memory_order_relaxed)) { return true; } - if (chain.Tip()->Time() < Now() - m_options.max_tip_age) { - return true; + if (!m_cached_finished_ibd.exchange(true, std::memory_order_relaxed)) { + LogInfo("Leaving InitialBlockDownload (latching to false)"); } - LogInfo("Leaving InitialBlockDownload (latching to false)"); - m_cached_finished_ibd.store(true, std::memory_order_relaxed); return false; } @@ -3011,7 +3001,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra } } - m_chain.SetTip(*pindexDelete->pprev); + m_chainman.SetTip(m_chain, *pindexDelete->pprev); UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to @@ -3139,8 +3129,8 @@ bool Chainstate::ConnectTip( m_mempool->removeForBlock(block_to_connect->vtx, pindexNew->nHeight); disconnectpool.removeForBlock(block_to_connect->vtx); } - // Update m_chain & related variables. - m_chain.SetTip(*pindexNew); + // Update m_chain and (for the active chainstate) m_cached_tip_recent and related variables. + m_chainman.SetTip(m_chain, *pindexNew); UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; @@ -3344,6 +3334,17 @@ static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_i return SynchronizationState::INIT_DOWNLOAD; } +void ChainstateManager::SetTip(CChain& chain, CBlockIndex& block) +{ + AssertLockHeld(cs_main); + chain.SetTip(block); + if (m_cached_tip_recent.load(std::memory_order_relaxed)) return; + if (&chain != &CurrentChainstate().m_chain) return; + if (chain.IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) { + m_cached_tip_recent.store(true, std::memory_order_relaxed); + } +} + bool ChainstateManager::NotifyHeaderTip() { bool fNotify = false; @@ -4626,7 +4627,7 @@ bool Chainstate::LoadChainTip() if (!pindex) { return false; } - m_chain.SetTip(*pindex); + m_chainman.SetTip(m_chain, *pindex); tip = m_chain.Tip(); // Make sure our chain tip before shutting down scores better than any other candidate diff --git a/src/validation.h b/src/validation.h index e4b1e555bdde..816b4628248b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1036,7 +1036,10 @@ class ChainstateManager * Mutable because we need to be able to mark IsInitialBlockDownload() * const, which latches this for caching purposes. */ - mutable std::atomic m_cached_finished_ibd{false}; + mutable std::atomic_bool m_cached_finished_ibd{false}; + + /** Latch that becomes true once the active tip has met IsTipRecent(). */ + mutable std::atomic_bool m_cached_tip_recent{false}; /** * Every received block is assigned a unique and increasing identifier, so we @@ -1157,6 +1160,9 @@ class ChainstateManager CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); } //! @} + /** Set a chain tip and latch m_cached_tip_recent if this is the active chain and the new tip is recent. */ + void SetTip(CChain& chain, CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main);