Merge bitcoin/bitcoin#22564: refactor: Move mutable globals cleared in ::UnloadBlockIndex to BlockManager

7ab07e0332 validation: Prune UnloadBlockIndex and callees (Carl Dong)
7d99d725cd validation: No mempool clearing in UnloadBlockIndex (Carl Dong)
572d831927 Clear {versionbits,warning}cache in ~Chainstatemanager (Carl Dong)
eca4ca4d60 style-only: Use std::clamp for check_ratio, rename (Carl Dong)
fe96a2e4bd style-only: Use for instead of when loading Chainstate (Carl Dong)
5921b863e3 init: Reset mempool and chainman via reconstruction (Carl Dong)
6e747e80e7 validation: default initialize and guard chainman members (Anthony Towns)
98f4bdae81 refactor: Convert warningcache to std::array (Carl Dong)

Pull request description:

  Fixes #22964

  -----

  This is a small part of the work to accomplish what I described in 972c5166ee:
  ```
  Over time, we should probably move these mutable global state variables
  into ChainstateManager or CChainState so it's easier to reason about
  their lifecycles.
  ```

  `::UnloadBlockIndex` manually resets a subset of our mutable globals in addition to unloading the `ChainstateManager` and clearing the mempool. The need for this manual reset (AFAICT) arises out of the fact that many of these globals are closely related to the block index (hence `::UnloadBlockIndex`), and need to be reset with it.

  I've shot this "manual reset" gun at my foot several times while doing the de-globalize chainman work.

  Thankfully, now that we have a `BlockManager` class that owns the block index, these globals should be moved under that class so that they can live and die with the block index. These moves, along with making the block index non-heap-based, eliminates:
  1. 3585b52139 The need to reason about when we need to manually call `::UnloadBlockIndex` (this decision can at times seem almost arbitrary)
  2. f741623c25 The need to have an `::UnloadBlockIndex` or explicit `~ChainstateManager` at all

ACKs for top commit:
  MarcoFalke:
    ACK 7ab07e0332 👘
  ajtowns:
    ACK 7ab07e0332
  ryanofsky:
    Code review ACK 7ab07e0332. This all looks good and simplifies things nicely. I left some minor suggestions below but feel free to ignore.

Tree-SHA512: a36ee3fc122ce0b4e8d1c432662d7009df06264b724b793252978a1e409dde7a7ef1f78b9ade3f8bfb5388213f10ae2d058d57a7a46ae563e9034d7d33a52b69

src/test/util/setup_common.cpp

@@ -182,7 +182,6 @@ ChainTestingSetup::~ChainTestingSetup()
     m_node.addrman.reset();
     m_node.netgroupman.reset();
     m_node.args = nullptr;
-    WITH_LOCK(::cs_main, UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman));
     m_node.mempool.reset();
     m_node.scheduler.reset();
     m_node.chainman.reset();

src/test/validation_chainstate_tests.cpp

@@ -93,7 +93,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
     BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root));
 
     // Ensure our active chain is the snapshot chainstate.
-    BOOST_CHECK(chainman.IsSnapshotActive());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive()));
 
     curr_tip = ::g_best_block;
 

src/test/validation_chainstatemanager_tests.cpp

@@ -45,7 +45,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));
 
     BOOST_CHECK(!manager.IsSnapshotActive());
-    BOOST_CHECK(!manager.IsSnapshotValidated());
+    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());
 
@@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     BOOST_CHECK(c2.ActivateBestChain(_, nullptr));
 
     BOOST_CHECK(manager.IsSnapshotActive());
-    BOOST_CHECK(!manager.IsSnapshotValidated());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated()));
     BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate());
     BOOST_CHECK(&c1 != &manager.ActiveChainstate());
     auto all2 = manager.GetAll();