Merge bitcoin/bitcoin#26251: refactor: add kernel/cs_main.h

282019c refactor: add kernel/cs_main.* (fanquake)

Pull request description:

  One place to find / include `cs_main`.
  No more:
  > // Actually declared in validation.cpp; can't include because of circular dependency.
  > extern RecursiveMutex cs_main;

  Ultimately, no more need to include `validation.h` (which also includes (heavy/boost filled) `txmempool.h`) everywhere for `cs_main`. See #26087 for another example of why that is useful.

ACKs for top commit:
  ajtowns:
    ACK 282019c

Tree-SHA512: 142835b794873e7a09c3246d6101843ae81ec0c6295e6873130c98a2abfa5f7282748d0f1a37237a779cc71c3bc0a75d03b20313ef5398c83d4814215cbc8287

Author: MarcoFalke

src/Makefile.am

@@ -177,6 +177,7 @@ BITCOIN_CORE_H = \
   kernel/checks.h \
   kernel/coinstats.h \
   kernel/context.h \
+  kernel/cs_main.h \
   kernel/mempool_entry.h \
   kernel/mempool_limits.h \
   kernel/mempool_options.h \
@@ -375,6 +376,7 @@ libbitcoin_node_a_SOURCES = \
   kernel/checks.cpp \
   kernel/coinstats.cpp \
   kernel/context.cpp \
+  kernel/cs_main.cpp \
   kernel/mempool_persist.cpp \
   mapport.cpp \
   net.cpp \
@@ -906,6 +908,7 @@ libbitcoinkernel_la_SOURCES = \
   kernel/checks.cpp \
   kernel/coinstats.cpp \
   kernel/context.cpp \
+  kernel/cs_main.cpp \
   kernel/mempool_persist.cpp \
   key.cpp \
   logging.cpp \

src/bench/rpc_mempool.cpp

@@ -4,6 +4,7 @@
 
 #include <bench/bench.h>
 #include <chainparamsbase.h>
+#include <kernel/cs_main.h>
 #include <kernel/mempool_entry.h>
 #include <rpc/mempool.h>
 #include <test/util/setup_common.h>

src/chain.h

@@ -9,6 +9,7 @@
 #include <arith_uint256.h>
 #include <consensus/params.h>
 #include <flatfile.h>
+#include <kernel/cs_main.h>
 #include <primitives/block.h>
 #include <sync.h>
 #include <uint256.h>
@@ -38,8 +39,6 @@ static constexpr int64_t TIMESTAMP_WINDOW = MAX_FUTURE_BLOCK_TIME;
  */
 static constexpr int64_t MAX_BLOCK_TIME_GAP = 90 * 60;
 
-extern RecursiveMutex cs_main;
-
 class CBlockFileInfo
 {
 public:

src/kernel/cs_main.cpp

@@ -0,0 +1,7 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <sync.h>
+
+RecursiveMutex cs_main;

src/kernel/cs_main.h

@@ -0,0 +1,22 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_KERNEL_CS_MAIN_H
+#define BITCOIN_KERNEL_CS_MAIN_H
+
+#include <sync.h>
+
+/**
+ * Mutex to guard access to validation specific variables, such as reading
+ * or changing the chainstate.
+ *
+ * This may also need to be locked when updating the transaction pool, e.g. on
+ * AcceptToMemoryPool. See CTxMemPool::cs comment for details.
+ *
+ * The transaction pool has a separate lock to allow reading from it and the
+ * chainstate at the same time.
+ */
+extern RecursiveMutex cs_main;
+
+#endif // BITCOIN_KERNEL_CS_MAIN_H

src/node/blockstorage.h

@@ -8,6 +8,7 @@
 #include <attributes.h>
 #include <chain.h>
 #include <fs.h>
+#include <kernel/cs_main.h>
 #include <protocol.h>
 #include <sync.h>
 #include <txdb.h>
@@ -17,8 +18,6 @@
 #include <unordered_map>
 #include <vector>
 
-extern RecursiveMutex cs_main;
-
 class ArgsManager;
 class BlockValidationState;
 class CBlock;

src/rpc/blockchain.h

@@ -16,8 +16,6 @@
 #include <stdint.h>
 #include <vector>
 
-extern RecursiveMutex cs_main;
-
 class CBlock;
 class CBlockIndex;
 class Chainstate;

src/rpc/node.cpp

@@ -12,6 +12,7 @@
 #include <interfaces/echo.h>
 #include <interfaces/init.h>
 #include <interfaces/ipc.h>
+#include <kernel/cs_main.h>
 #include <node/context.h>
 #include <rpc/server.h>
 #include <rpc/server_util.h>

src/test/mempool_tests.cpp

@@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
 
 
     CTxMemPool& testPool = *Assert(m_node.mempool);
-    LOCK2(cs_main, testPool.cs);
+    LOCK2(::cs_main, testPool.cs);
 
     // Nothing in pool, remove should do nothing:
     unsigned int poolSize = testPool.size();

src/test/txvalidationcache_tests.cpp

@@ -117,7 +117,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
 // should fail.
 // Capture this interaction with the upgraded_nop argument: set it when evaluating
 // any script flag that is implemented as an upgraded NOP code.
-static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
     PrecomputedTransactionData txdata;
 

src/test/validation_chainstatemanager_tests.cpp

@@ -116,7 +116,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
 
     // Create a legacy (IBD) chainstate.
     //
-    Chainstate& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool));
+    Chainstate& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(&mempool));
     chainstates.push_back(&c1);
     c1.InitCoinsDB(
         /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false);

src/txdb.h

@@ -8,6 +8,7 @@
 
 #include <coins.h>
 #include <dbwrapper.h>
+#include <kernel/cs_main.h>
 #include <sync.h>
 #include <fs.h>
 
@@ -44,9 +45,6 @@ static const int64_t max_filter_index_cache = 1024;
 //! Max memory allocated to coin DB specific cache (MiB)
 static const int64_t nMaxCoinsDBCache = 8;
 
-// Actually declared in validation.cpp; can't include because of circular dependency.
-extern RecursiveMutex cs_main;
-
 /** CCoinsView backed by the coin database (chainstate/) */
 class CCoinsViewDB final : public CCoinsView
 {

src/txmempool.h

@@ -21,6 +21,7 @@
 #include <coins.h>
 #include <consensus/amount.h>
 #include <indirectmap.h>
+#include <kernel/cs_main.h>
 #include <kernel/mempool_entry.h>
 #include <policy/feerate.h>
 #include <policy/packages.h>
@@ -39,7 +40,6 @@
 class CBlockIndex;
 class CChain;
 class Chainstate;
-extern RecursiveMutex cs_main;
 
 /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */
 static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF;

src/validation.cpp

@@ -107,18 +107,6 @@ const std::vector<std::string> CHECKLEVEL_DOC {
  * */
 static constexpr int PRUNE_LOCK_BUFFER{10};
 
-/**
- * Mutex to guard access to validation specific variables, such as reading
- * or changing the chainstate.
- *
- * This may also need to be locked when updating the transaction pool, e.g. on
- * AcceptToMemoryPool. See CTxMemPool::cs comment for details.
- *
- * The transaction pool has a separate lock to allow reading from it and the
- * chainstate at the same time.
- */
-RecursiveMutex cs_main;
-
 GlobalMutex g_best_block_mutex;
 std::condition_variable g_best_block_cv;
 uint256 g_best_block;

src/validation.h

@@ -18,6 +18,7 @@
 #include <consensus/amount.h>
 #include <deploymentstatus.h>
 #include <fs.h>
+#include <kernel/cs_main.h> // IWYU pragma: export
 #include <node/blockstorage.h>
 #include <policy/feerate.h>
 #include <policy/packages.h>
@@ -86,7 +87,6 @@ enum class SynchronizationState {
     POST_INIT
 };
 
-extern RecursiveMutex cs_main;
 extern GlobalMutex g_best_block_mutex;
 extern std::condition_variable g_best_block_cv;
 /** Used to notify getblocktemplate RPC of new tips. */

src/validationinterface.h

@@ -6,13 +6,13 @@
 #ifndef BITCOIN_VALIDATIONINTERFACE_H
 #define BITCOIN_VALIDATIONINTERFACE_H
 
+#include <kernel/cs_main.h>
 #include <primitives/transaction.h> // CTransaction(Ref)
 #include <sync.h>
 
 #include <functional>
 #include <memory>
 
-extern RecursiveMutex cs_main;
 class BlockValidationState;
 class CBlock;
 class CBlockIndex;

src/zmq/zmqpublishnotifier.cpp

@@ -7,6 +7,7 @@
 #include <chain.h>
 #include <chainparams.h>
 #include <crypto/common.h>
+#include <kernel/cs_main.h>
 #include <logging.h>
 #include <netaddress.h>
 #include <netbase.h>