[refactor] Make signals optional in mempool and chainman

This is done in preparation for the next two commits, where the
CMainSignals are de-globalized.

This avoids adding new constructor arguments to the ChainstateManager
and CTxMemPool classes over the next two commits.

This could also allow future tests that are only interested in the
internal behaviour of the classes to forgo instantiating the signals.

Author: TheCharlatan

contrib/devtools/test_deterministic_coverage.sh

@@ -17,21 +17,21 @@ NON_DETERMINISTIC_TESTS=(
     "blockfilter_index_tests/blockfilter_index_initial_sync"  # src/checkqueue.h: In CCheckQueue::Loop(): while (queue.empty()) { ... }
     "coinselector_tests/knapsack_solver_test"                 # coinselector_tests.cpp: if (equal_sets(setCoinsRet, setCoinsRet2))
     "fs_tests/fsbridge_fstream"                               # deterministic test failure?
-    "miner_tests/CreateNewBlock_validity"                     # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
+    "miner_tests/CreateNewBlock_validity"                     # validation.cpp: if (signals.CallbacksPending() > 10)
     "scheduler_tests/manythreads"                             # scheduler.cpp: CScheduler::serviceQueue()
     "scheduler_tests/singlethreadedscheduler_ordered"         # scheduler.cpp: CScheduler::serviceQueue()
-    "txvalidationcache_tests/checkinputs_test"                # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "txvalidationcache_tests/tx_mempool_block_doublespend"    # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "txindex_tests/txindex_initial_sync"                      # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "txvalidation_tests/tx_mempool_reject_coinbase"           # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "validation_block_tests/processnewblock_signals_ordering" # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/coin_mark_dirty_immature_credit"            # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/dummy_input_size_test"                      # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/importmulti_rescan"                         # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/importwallet_rescan"                        # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/ListCoins"                                  # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/scan_for_wallet_transactions"               # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
-    "wallet_tests/wallet_disableprivkeys"                     # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
+    "txvalidationcache_tests/checkinputs_test"                # validation.cpp: if (signals.CallbacksPending() > 10)
+    "txvalidationcache_tests/tx_mempool_block_doublespend"    # validation.cpp: if (signals.CallbacksPending() > 10)
+    "txindex_tests/txindex_initial_sync"                      # validation.cpp: if (signals.CallbacksPending() > 10)
+    "txvalidation_tests/tx_mempool_reject_coinbase"           # validation.cpp: if (signals.CallbacksPending() > 10)
+    "validation_block_tests/processnewblock_signals_ordering" # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/coin_mark_dirty_immature_credit"            # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/dummy_input_size_test"                      # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/importmulti_rescan"                         # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/importwallet_rescan"                        # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/ListCoins"                                  # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/scan_for_wallet_transactions"               # validation.cpp: if (signals.CallbacksPending() > 10)
+    "wallet_tests/wallet_disableprivkeys"                     # validation.cpp: if (signals.CallbacksPending() > 10)
 )
 
 TEST_BITCOIN_BINARY="src/test/test_bitcoin"

src/bitcoin-chainstate.cpp

@@ -118,6 +118,7 @@ int main(int argc, char* argv[])
         .chainparams = *chainparams,
         .datadir = abs_datadir,
         .notifications = *notifications,
+        .signals = &GetMainSignals(),
     };
     const node::BlockManager::Options blockman_opts{
         .chainparams = chainman_opts.chainparams,

src/init.cpp

@@ -1449,6 +1449,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         .chainparams = chainparams,
         .datadir = args.GetDataDirNet(),
         .notifications = *node.notifications,
+        .signals = &GetMainSignals(),
     };
     Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction
 
@@ -1478,6 +1479,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     CTxMemPool::Options mempool_opts{
         .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
+        .signals = &GetMainSignals(),
     };
     auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)};
     if (!result) {

src/kernel/chainstatemanager_opts.h

@@ -18,6 +18,7 @@
 #include <optional>
 
 class CChainParams;
+class CMainSignals;
 
 static constexpr bool DEFAULT_CHECKPOINTS_ENABLED{true};
 static constexpr auto DEFAULT_MAX_TIP_AGE{24h};
@@ -44,6 +45,7 @@ struct ChainstateManagerOpts {
     DBOptions coins_db{};
     CoinsViewOptions coins_view{};
     Notifications& notifications;
+    CMainSignals* signals{nullptr};
     //! Number of script check worker threads. Zero means no parallel verification.
     int worker_threads_num{0};
 };

src/kernel/mempool_options.h

@@ -13,6 +13,8 @@
 #include <cstdint>
 #include <optional>
 
+class CMainSignals;
+
 /** Default for -maxmempool, maximum megabytes of mempool memory usage */
 static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300};
 /** Default for -maxmempool when blocksonly is set */
@@ -56,6 +58,8 @@ struct MemPoolOptions {
     bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF};
     bool persist_v1_dat{DEFAULT_PERSIST_V1_DAT};
     MemPoolLimits limits{};
+
+    CMainSignals* signals{nullptr};
 };
 } // namespace kernel
 

src/test/util/setup_common.cpp

@@ -185,6 +185,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
         .datadir = m_args.GetDataDirNet(),
         .check_block_index = true,
         .notifications = *m_node.notifications,
+        .signals = &GetMainSignals(),
         .worker_threads_num = 2,
     };
     const BlockManager::Options blockman_opts{

src/test/util/txmempool.cpp

@@ -13,6 +13,7 @@
 #include <util/time.h>
 #include <util/translation.h>
 #include <validation.h>
+#include <validationinterface.h>
 
 using node::NodeContext;
 
@@ -22,6 +23,7 @@ CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node)
         // Default to always checking mempool regardless of
         // chainparams.DefaultConsistencyChecks for tests
         .check_ratio = 1,
+        .signals = &GetMainSignals(),
     };
     const auto result{ApplyArgsManOptions(*node.args, ::Params(), mempool_opts)};
     Assert(result);

src/test/validation_chainstatemanager_tests.cpp

@@ -383,6 +383,7 @@ struct SnapshotTestSetup : TestChain100Setup {
                 .chainparams = ::Params(),
                 .datadir = chainman.m_options.datadir,
                 .notifications = *m_node.notifications,
+                .signals = &GetMainSignals(),
             };
             const BlockManager::Options blockman_opts{
                 .chainparams = chainman_opts.chainparams,

src/txmempool.cpp

@@ -406,7 +406,8 @@ CTxMemPool::CTxMemPool(const Options& opts)
       m_require_standard{opts.require_standard},
       m_full_rbf{opts.full_rbf},
       m_persist_v1_dat{opts.persist_v1_dat},
-      m_limits{opts.limits}
+      m_limits{opts.limits},
+      m_signals{opts.signals}
 {
 }
 
@@ -487,12 +488,12 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     // even if not directly reported below.
     uint64_t mempool_sequence = GetAndIncrementSequence();
 
-    if (reason != MemPoolRemovalReason::BLOCK) {
+    if (reason != MemPoolRemovalReason::BLOCK && m_signals) {
         // Notify clients that a transaction has been removed from the mempool
         // for any reason except being included in a block. Clients interested
         // in transactions included in blocks can subscribe to the BlockConnected
         // notification.
-        GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
+        m_signals->TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
     }
     TRACE5(mempool, removed,
         it->GetTx().GetHash().data(),
@@ -643,7 +644,9 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
-    GetMainSignals().MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
+    if (m_signals) {
+        m_signals->MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
+    }
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
 }

src/txmempool.h

@@ -40,6 +40,7 @@
 #include <vector>
 
 class CChain;
+class CMainSignals;
 
 /** 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;
@@ -447,6 +448,8 @@ class CTxMemPool
 
     const Limits m_limits;
 
+    CMainSignals* const m_signals;
+
     /** Create a new CTxMemPool.
      * Sanity checks will be off by default for performance, because otherwise
      * accepting transactions becomes O(N^2) where N is the number of transactions