assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager

Summary:
This change makes IsInitialBlockDownload and NotifyHeaderTip functions no
longer tied to individual Chainstate objects. It makes them work with the
ChainstateManager object instead so code is simpler and it is no longer
possible to call them incorrectly with an inactive Chainstate.

This change also makes m_cached_finished_ibd caching easier to reason about,
because now there is only one cached value instead of two (for background and
snapshot chainstates) so the cached IBD state now no longer gets reset when a
snapshot is loaded.

There should be no change in behavior because these functions were always
called on the active ChainState objects.

These changes were discussed previously
bitcoin/bitcoin#27746 (comment) and
bitcoin/bitcoin#27746 (comment) as
possible followups for that PR.

This is a backport of [[bitcoin/bitcoin#28218 | core#28218]]

Depends on D17605

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D17635

Author: ryanofsky

src/avalanche/processor.cpp

@@ -777,7 +777,7 @@ bool Processor::stopEventLoop() {
 void Processor::avaproofsSent(NodeId nodeid) {
     AssertLockNotHeld(cs_main);
 
-    if (chainman.ActiveChainstate().IsInitialBlockDownload()) {
+    if (chainman.IsInitialBlockDownload()) {
         // Before IBD is complete there is no way to make sure a proof is valid
         // or not, e.g. it can be spent in a block we don't know yet. In order
         // to increase confidence that our proof set is similar to other nodes
@@ -822,7 +822,7 @@ bool Processor::isQuorumEstablished() {
     }
 
     // Don't do Avalanche while node is IBD'ing
-    if (chainman.ActiveChainstate().IsInitialBlockDownload()) {
+    if (chainman.IsInitialBlockDownload()) {
         return false;
     }
 

src/bitcoin-chainstate.cpp

@@ -175,8 +175,8 @@ int main(int argc, char *argv[]) {
                   << "Active Height: " << chainman.ActiveHeight() << std::endl
                   << "\t"
                   << "Active IBD: " << std::boolalpha
-                  << chainman.ActiveChainstate().IsInitialBlockDownload()
-                  << std::noboolalpha << std::endl;
+                  << chainman.IsInitialBlockDownload() << std::noboolalpha
+                  << std::endl;
         CBlockIndex *tip = chainman.ActiveTip();
         if (tip) {
             std::cout << "\t" << tip->ToString() << std::endl;

src/net_processing.cpp

@@ -2453,7 +2453,7 @@ void PeerManagerImpl::AvalanchePeriodicNetworking(CScheduler &scheduler) const {
         }
     }
 
-    if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+    if (m_chainman.IsInitialBlockDownload()) {
         // Don't request proofs while in IBD. We're likely to orphan them
         // because we don't have the UTXOs.
         goto scheduleLater;
@@ -3081,8 +3081,7 @@ void PeerManagerImpl::BlockChecked(const CBlock &block,
     // 3. This is currently the best block we're aware of. We haven't updated
     //    the tip yet so we have no way to check this directly here. Instead we
     //    just check that there are currently no other blocks in flight.
-    else if (state.IsValid() &&
-             !m_chainman.ActiveChainstate().IsInitialBlockDownload() &&
+    else if (state.IsValid() && !m_chainman.IsInitialBlockDownload() &&
              mapBlocksInFlight.count(hash) == mapBlocksInFlight.size()) {
         if (it != mapBlockSource.end()) {
             MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first);
@@ -4057,8 +4056,7 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(
 
     // If we're in IBD, we want outbound peers that will serve us a useful
     // chain. Disconnect peers that are on chains with insufficient work.
-    if (m_chainman.ActiveChainstate().IsInitialBlockDownload() &&
-        !may_have_more_headers) {
+    if (m_chainman.IsInitialBlockDownload() && !may_have_more_headers) {
         // When nCount < MAX_HEADERS_RESULTS, we know we have no more
         // headers to fetch from this peer.
         if (nodestate->pindexBestKnownBlock &&
@@ -5380,7 +5378,7 @@ void PeerManagerImpl::ProcessMessage(
                 AddKnownProof(*peer, proofid);
 
                 if (!fAlreadyHave && m_avalanche &&
-                    !m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+                    !m_chainman.IsInitialBlockDownload()) {
                     const bool preferred = isPreferredDownloadPeer(pfrom);
 
                     LOCK(cs_proofrequest);
@@ -5405,8 +5403,8 @@ void PeerManagerImpl::ProcessMessage(
                              txid.ToString(), pfrom.GetId());
                     pfrom.fDisconnect = true;
                     return;
-                } else if (!fAlreadyHave && !m_chainman.ActiveChainstate()
-                                                 .IsInitialBlockDownload()) {
+                } else if (!fAlreadyHave &&
+                           !m_chainman.IsInitialBlockDownload()) {
                     AddTxAnnouncement(pfrom, txid, current_time);
                 }
 
@@ -5747,7 +5745,7 @@ void PeerManagerImpl::ProcessMessage(
         // don't have enough information to validate it yet. Sending unsolicited
         // transactions is not considered a protocol violation, so don't punish
         // the peer.
-        if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+        if (m_chainman.IsInitialBlockDownload()) {
             return;
         }
 
@@ -6024,7 +6022,7 @@ void PeerManagerImpl::ProcessMessage(
             if (!prev_block) {
                 // Doesn't connect (or is genesis), instead of DoSing in
                 // AcceptBlockHeader, request deeper headers
-                if (!m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+                if (!m_chainman.IsInitialBlockDownload()) {
                     MaybeSendGetHeaders(
                         pfrom, GetLocator(m_chainman.m_best_header), *peer);
                 }
@@ -6516,9 +6514,8 @@ void PeerManagerImpl::ProcessMessage(
         // unless we're still syncing with the network. Such an unrequested
         // block may still be processed, subject to the conditions in
         // AcceptBlock().
-        bool forceProcessing =
-            pfrom.HasPermission(NetPermissionFlags::NoBan) &&
-            !m_chainman.ActiveChainstate().IsInitialBlockDownload();
+        bool forceProcessing = pfrom.HasPermission(NetPermissionFlags::NoBan) &&
+                               !m_chainman.IsInitialBlockDownload();
         const BlockHash hash = pblock->GetHash();
         bool min_pow_checked = false;
         {
@@ -6618,8 +6615,7 @@ void PeerManagerImpl::ProcessMessage(
             WITH_LOCK(peer->m_addr_token_bucket_mutex,
                       peer->m_addr_token_bucket += m_opts.max_addr_to_send);
 
-            if (peer->m_proof_relay &&
-                !m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+            if (peer->m_proof_relay && !m_chainman.IsInitialBlockDownload()) {
                 m_connman.PushMessage(&pfrom,
                                       msgMaker.Make(NetMsgType::GETAVAPROOFS));
                 peer->m_proof_relay->compactproofs_requested = true;
@@ -8180,7 +8176,7 @@ void PeerManagerImpl::MaybeSendAddr(CNode &node, Peer &peer,
     }
 
     LOCK(peer.m_addr_send_times_mutex);
-    if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload() &&
+    if (fListen && !m_chainman.IsInitialBlockDownload() &&
         peer.m_next_local_addr_send < current_time) {
         // If we've sent before, clear the bloom filter for the peer, so
         // that our self-announcement will actually go out. This might
@@ -8300,7 +8296,7 @@ void PeerManagerImpl::MaybeSendFeefilter(
 
     Amount currentFilter = m_mempool.GetMinFee().GetFeePerK();
 
-    if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+    if (m_chainman.IsInitialBlockDownload()) {
         // Received tx-inv messages are discarded when the active
         // chainstate is in IBD, so tell the peer to not send them.
         currentFilter = MAX_MONEY;
@@ -9003,7 +8999,7 @@ bool PeerManagerImpl::SendMessages(const Config &config, CNode *pto) {
 
         if (CanServeBlocks(*peer) &&
             ((sync_blocks_and_headers_from_peer && !IsLimitedPeer(*peer)) ||
-             !m_chainman.ActiveChainstate().IsInitialBlockDownload()) &&
+             !m_chainman.IsInitialBlockDownload()) &&
             state.vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
             std::vector<const CBlockIndex *> vToDownload;
             NodeId staller = -1;
@@ -9129,7 +9125,7 @@ bool PeerManagerImpl::ReceivedAvalancheProof(CNode &node, Peer &peer,
 
     AddKnownProof(peer, proofid);
 
-    if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+    if (m_chainman.IsInitialBlockDownload()) {
         // We cannot reliably verify proofs during IBD, so bail out early and
         // keep the inventory as pending so it can be requested when the node
         // has synced.

src/node/interfaces.cpp

@@ -267,7 +267,7 @@ namespace {
                                              tip);
         }
         bool isInitialBlockDownload() override {
-            return chainman().ActiveChainstate().IsInitialBlockDownload();
+            return chainman().IsInitialBlockDownload();
         }
         bool isLoadingBlocks() override {
             return chainman().m_blockman.LoadingBlocks();
@@ -697,7 +697,7 @@ namespace {
                    !isInitialBlockDownload();
         }
         bool isInitialBlockDownload() override {
-            return chainman().ActiveChainstate().IsInitialBlockDownload();
+            return chainman().IsInitialBlockDownload();
         }
         bool shutdownRequested() override { return ShutdownRequested(); }
         void initMessage(const std::string &message) override {

src/rpc/blockchain.cpp

@@ -1359,7 +1359,7 @@ RPCHelpMan getblockchaininfo() {
                 "verificationprogress",
                 GuessVerificationProgress(chainman.GetParams().TxData(), &tip));
             obj.pushKV("initialblockdownload",
-                       active_chainstate.IsInitialBlockDownload());
+                       chainman.IsInitialBlockDownload());
             obj.pushKV("chainwork", tip.nChainWork.GetHex());
             obj.pushKV("size_on_disk",
                        chainman.m_blockman.CalculateCurrentUsage());

src/rpc/mining.cpp

@@ -929,7 +929,7 @@ static RPCHelpMan getblocktemplate() {
                                    "Bitcoin is not connected!");
             }
 
-            if (active_chainstate.IsInitialBlockDownload()) {
+            if (chainman.IsInitialBlockDownload()) {
                 throw JSONRPCError(
                     RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME
                     " is in initial sync and waiting for blocks...");

src/test/net_tests.cpp

@@ -1293,11 +1293,10 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) {
     const int64_t time{0};
     const CNetMsgMaker msg_maker{PROTOCOL_VERSION};
 
-    // Force CChainState::IsInitialBlockDownload() to return false.
+    // Force ChainstateManager::IsInitialBlockDownload() to return false.
     // Otherwise PushAddress() isn't called by PeerManager::ProcessMessage().
-    TestChainState &chainstate =
-        *static_cast<TestChainState *>(&m_node.chainman->ActiveChainstate());
-    chainstate.JumpOutOfIbd();
+    auto &chainman = static_cast<TestChainstateManager &>(*m_node.chainman);
+    chainman.JumpOutOfIbd();
 
     const Config &config = m_node.chainman->GetConfig();
 
@@ -1354,7 +1353,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) {
     BOOST_CHECK(sent);
 
     CaptureMessage = CaptureMessageOrig;
-    chainstate.ResetIbd();
+    chainman.ResetIbd();
 
     m_node.peerman->FinalizeNode(config, peer);
 

src/test/util/validation.cpp

@@ -14,12 +14,12 @@ void ValidationInterfaceTest::BlockConnected(
     obj.BlockConnected(block, pindex);
 }
 
-void TestChainState::ResetIbd() {
+void TestChainstateManager::ResetIbd() {
     m_cached_finished_ibd = false;
     assert(IsInitialBlockDownload());
 }
 
-void TestChainState::JumpOutOfIbd() {
+void TestChainstateManager::JumpOutOfIbd() {
     Assert(IsInitialBlockDownload());
     m_cached_finished_ibd = true;
     Assert(!IsInitialBlockDownload());

src/test/util/validation.h

@@ -16,7 +16,7 @@ class ValidationInterfaceTest {
                                const CBlockIndex *pindex);
 };
 
-struct TestChainState : public Chainstate {
+struct TestChainstateManager : public ChainstateManager {
     /** Reset the ibd cache to its initial state */
     void ResetIbd();
     /** Toggle IsInitialBlockDownload from true to false */

src/test/validation_chainstatemanager_tests.cpp

@@ -15,6 +15,7 @@
 #include <test/util/chainstate.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
+#include <test/util/validation.h>
 #include <timedata.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -166,14 +167,21 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) {
         /* 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<TestChainstateManager &>(manager).ResetIbd();
+
     {
         LOCK(::cs_main);
         c2.InitCoinsCache(1 << 23);
         manager.MaybeRebalanceCaches();
     }
 
-    // Since both chainstates are considered to be in initial block download,
-    // the snapshot chainstate should take priority.
     BOOST_CHECK_CLOSE(c1.m_coinstip_cache_size_bytes, max_cache * 0.05, 1);
     BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1);
     BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1);