Merge #19604: Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState

fae8c28dae747f9c4c6481049742346d18202fc8 Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke)
fac674db200e6b2d5b32069335fb24e713d7b69f Pass mempool pointer to UnloadBlockIndex (MarcoFalke)
faec851b6eb7e65e28cdcae50d6dc86fafa0f91c test: Simplify cs_main locks (MarcoFalke)

Pull request description:

  Split out from #19556

  Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556.

ACKs for top commit:
  jnewbery:
    code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
  fjahr:
    Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
  darosior:
    Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8
  jamesob:
    ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))

Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728

4 files changed, 16 insertions, 25 deletions

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 0cd991f453..14f65dcb7c 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -182,9 +182,9 @@ TestingSetup::~TestingSetup()
     m_node.connman.reset();
     m_node.banman.reset();
     m_node.args = nullptr;
+    UnloadBlockIndex(m_node.mempool);
     m_node.mempool = nullptr;
     m_node.scheduler.reset();
-    UnloadBlockIndex();
     m_node.chainman->Reset();
     m_node.chainman = nullptr;
     pblocktree.reset();
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index f531b393b8..2076a1096a 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -34,9 +34,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
         return outp;
     };
 
-    ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c1 = manager.InitializeChainstate();
-    LEAVE_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate());
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
     WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index f99191b08f..887a48124f 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -54,10 +54,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
 
     // Create a snapshot-based chainstate.
     //
-    ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c2 = *WITH_LOCK(::cs_main,
-        return &manager.InitializeChainstate(GetRandHash()));
-    LEAVE_CRITICAL_SECTION(cs_main);
+    CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(GetRandHash()));
     chainstates.push_back(&c2);
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -115,9 +112,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
 
     // Create a legacy (IBD) chainstate.
     //
-    ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c1 = manager.InitializeChainstate();
-    LEAVE_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate());
     chainstates.push_back(&c1);
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -134,9 +129,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
 
     // Create a snapshot-based chainstate.
     //
-    ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c2 = manager.InitializeChainstate(GetRandHash());
-    LEAVE_CRITICAL_SECTION(cs_main);
+    CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(GetRandHash()));
     chainstates.push_back(&c2);
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
index 7111fc3380..8bac914f05 100644
--- a/src/test/validation_flush_tests.cpp
+++ b/src/test/validation_flush_tests.cpp
@@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
 
     // Without any coins in the cache, we shouldn't need to flush.
     BOOST_CHECK_EQUAL(
-        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+        chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
         CoinsCacheSizeState::OK);
 
     // If the initial memory allocations of cacheCoins don't match these common
@@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
         }
 
         BOOST_CHECK_EQUAL(
-            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+            chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
             CoinsCacheSizeState::CRITICAL);
 
         BOOST_TEST_MESSAGE("Exiting cache flush tests early due to unsupported arch");
@@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
         print_view_mem_usage(view);
         BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE);
         BOOST_CHECK_EQUAL(
-            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+            chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
             CoinsCacheSizeState::OK);
     }
 
@@ -100,26 +100,26 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     for (int i{0}; i < 4; ++i) {
         add_coin(view);
         print_view_mem_usage(view);
-        if (chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) ==
+        if (chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) ==
             CoinsCacheSizeState::CRITICAL) {
             break;
         }
     }
 
     BOOST_CHECK_EQUAL(
-        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+        chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
         CoinsCacheSizeState::CRITICAL);
 
     // Passing non-zero max mempool usage should allow us more headroom.
     BOOST_CHECK_EQUAL(
-        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+        chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
         CoinsCacheSizeState::OK);
 
     for (int i{0}; i < 3; ++i) {
         add_coin(view);
         print_view_mem_usage(view);
         BOOST_CHECK_EQUAL(
-            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+            chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
             CoinsCacheSizeState::OK);
     }
 
@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
         BOOST_CHECK(usage_percentage >= 0.9);
         BOOST_CHECK(usage_percentage < 1);
         BOOST_CHECK_EQUAL(
-            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10),
+            chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10),
             CoinsCacheSizeState::LARGE);
     }
 
@@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     for (int i{0}; i < 1000; ++i) {
         add_coin(view);
         BOOST_CHECK_EQUAL(
-            chainstate.GetCoinsCacheSizeState(tx_pool),
+            chainstate.GetCoinsCacheSizeState(&tx_pool),
             CoinsCacheSizeState::OK);
     }
 
@@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     // preallocated memory that doesn't get reclaimed even after flush.
 
     BOOST_CHECK_EQUAL(
-        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+        chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0),
         CoinsCacheSizeState::CRITICAL);
 
     view.SetBestBlock(InsecureRand256());
@@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     print_view_mem_usage(view);
 
     BOOST_CHECK_EQUAL(
-        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+        chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0),
         CoinsCacheSizeState::CRITICAL);
 }