Merge #16743: refactor: move LoadChainTip/RelayBlocks under CChainState

3cf36736e5 refactoring: move ReplayBlocks under CChainState (James O'Beirne)
bcf73d3b84 refactoring: move LoadChainTip to CChainState method (James O'Beirne)
f5809d5b13 doc: fix CChainState::ActivateBestChain doc (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

  ---

  Move more chainstate-related functionality to methods on CChainState. Nothing too interesting here, but needed to work with multiple chainstates. And brief to review. :)

  Also fixes doc on ActivateBestChain.

ACKs for top commit:
  MarcoFalke:
    ACK 3cf36736e5
  ryanofsky:
    Can confirm. utACK 3cf36736e5. Removes wrapper functions and removes more  ::ChainActive() and ::ChainstateActive() calls than it adds, so seems good.

Tree-SHA512: 4bf8a1dd454ca9d61c85f6736910fa7354c57acc0002e3a8e5ce494035d8280e4c20e066f03478eeff7d44195e7912c282a486526da9be53854b478b961affaa

src/init.cpp

@@ -1545,7 +1545,7 @@ bool AppInitMain(InitInterfaces& interfaces)
                 }
 
                 // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
-                if (!ReplayBlocks(chainparams, &::ChainstateActive().CoinsDB())) {
+                if (!::ChainstateActive().ReplayBlocks(chainparams)) {
                     strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.").translated;
                     break;
                 }
@@ -1557,8 +1557,8 @@ bool AppInitMain(InitInterfaces& interfaces)
                 is_coinsview_empty = fReset || fReindexChainState ||
                     ::ChainstateActive().CoinsTip().GetBestBlock().IsNull();
                 if (!is_coinsview_empty) {
-                    // LoadChainTip sets ::ChainActive() based on CoinsTip()'s best block
+                    // LoadChainTip initializes the chain based on CoinsTip()'s best block
-                    if (!LoadChainTip(chainparams)) {
+                    if (!::ChainstateActive().LoadChainTip(chainparams)) {
                         strLoadError = _("Error initializing block database").translated;
                         break;
                     }

src/validation.cpp

@@ -4087,28 +4087,31 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_RE
     return true;
 }
 
-bool LoadChainTip(const CChainParams& chainparams)
+bool CChainState::LoadChainTip(const CChainParams& chainparams)
 {
     AssertLockHeld(cs_main);
-    const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip();
+    const CCoinsViewCache& coins_cache = CoinsTip();
     assert(!coins_cache.GetBestBlock().IsNull()); // Never called when the coins view is empty
+    const CBlockIndex* tip = m_chain.Tip();
 
-    if (::ChainActive().Tip() &&
+    if (tip && tip->GetBlockHash() == coins_cache.GetBestBlock()) {
-        ::ChainActive().Tip()->GetBlockHash() == coins_cache.GetBestBlock()) return true;
+        return true;
+    }
 
     // Load pointer to end of best chain
     CBlockIndex* pindex = LookupBlockIndex(coins_cache.GetBestBlock());
     if (!pindex) {
         return false;
     }
-    ::ChainActive().SetTip(pindex);
+    m_chain.SetTip(pindex);
-
+    PruneBlockIndexCandidates();
-    ::ChainstateActive().PruneBlockIndexCandidates();
 
+    tip = m_chain.Tip();
     LogPrintf("Loaded best chain: hashBestChain=%s height=%d date=%s progress=%f\n",
-        ::ChainActive().Tip()->GetBlockHash().ToString(), ::ChainActive().Height(),
+        tip->GetBlockHash().ToString(),
-        FormatISO8601DateTime(::ChainActive().Tip()->GetBlockTime()),
+        m_chain.Height(),
-        GuessVerificationProgress(chainparams.TxData(), ::ChainActive().Tip()));
+        FormatISO8601DateTime(tip->GetBlockTime()),
+        GuessVerificationProgress(chainparams.TxData(), tip));
     return true;
 }
 
@@ -4243,13 +4246,14 @@ bool CChainState::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& i
     return true;
 }
 
-bool CChainState::ReplayBlocks(const CChainParams& params, CCoinsView* view)
+bool CChainState::ReplayBlocks(const CChainParams& params)
 {
     LOCK(cs_main);
 
-    CCoinsViewCache cache(view);
+    CCoinsView& db = this->CoinsDB();
+    CCoinsViewCache cache(&db);
 
-    std::vector<uint256> hashHeads = view->GetHeadBlocks();
+    std::vector<uint256> hashHeads = db.GetHeadBlocks();
     if (hashHeads.empty()) return true; // We're already in a consistent state.
     if (hashHeads.size() != 2) return error("ReplayBlocks(): unknown inconsistent state");
 
@@ -4309,10 +4313,6 @@ bool CChainState::ReplayBlocks(const CChainParams& params, CCoinsView* view)
     return true;
 }
 
-bool ReplayBlocks(const CChainParams& params, CCoinsView* view) {
-    return ::ChainstateActive().ReplayBlocks(params, view);
-}
-
 //! Helper for CChainState::RewindBlockIndex
 void CChainState::EraseBlockData(CBlockIndex* index)
 {

src/validation.h

@@ -240,8 +240,6 @@ bool LoadGenesisBlock(const CChainParams& chainparams);
 /** Load the block tree and coins database from disk,
  * initializing state if we're running with -reindex. */
 bool LoadBlockIndex(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-/** Update the chain tip based on database information. */
-bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 /** Unload database information */
 void UnloadBlockIndex();
 /** Run an instance of the script checking thread */
@@ -400,9 +398,6 @@ public:
     bool VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, int nCheckLevel, int nCheckDepth);
 };
 
-/** Replay blocks that aren't fully applied to the database. */
-bool ReplayBlocks(const CChainParams& params, CCoinsView* view);
-
 CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 /** Find the last common block between the parameter chain and a locator. */
@@ -678,9 +673,13 @@ public:
      * we avoid holding cs_main for an extended period of time; the length of this
      * call may be quite long during reindexing or a substantial reorg.
      *
+     * May not be called with cs_main held. May not be called in a
+     * validationinterface callback.
+     *
      * @returns true unless a system error occurred
      */
-    bool ActivateBestChain(CValidationState& state,
+    bool ActivateBestChain(
+        CValidationState& state,
         const CChainParams& chainparams,
         std::shared_ptr<const CBlock> pblock) LOCKS_EXCLUDED(cs_main);
 
@@ -699,7 +698,8 @@ public:
     bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main);
     void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
-    bool ReplayBlocks(const CChainParams& params, CCoinsView* view);
+    /** Replay blocks that aren't fully applied to the database. */
+    bool ReplayBlocks(const CChainParams& params);
     bool RewindBlockIndex(const CChainParams& params) LOCKS_EXCLUDED(cs_main);
     bool LoadGenesisBlock(const CChainParams& chainparams);
 
@@ -717,6 +717,9 @@ public:
      */
     void CheckBlockIndex(const Consensus::Params& consensusParams);
 
+    /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */
+    bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
 private:
     bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);
     bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);