refactor: remove CDeterministicMNManager global, move to NodeContext

Author: kwvg

src/evo/deterministicmns.cpp

@@ -27,8 +27,6 @@
 static const std::string DB_LIST_SNAPSHOT = "dmn_S3";
 static const std::string DB_LIST_DIFF = "dmn_D3";
 
-std::unique_ptr<CDeterministicMNManager> deterministicMNManager;
-
 uint64_t CDeterministicMN::GetInternalId() const
 {
     // can't get it if it wasn't set yet

src/evo/deterministicmns.h

@@ -637,6 +637,4 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
 bool CheckProUpRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs);
 bool CheckProUpRevTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, bool check_sigs);
 
-extern std::unique_ptr<CDeterministicMNManager> deterministicMNManager;
-
 #endif // BITCOIN_EVO_DETERMINISTICMNS_H

src/init.cpp

@@ -343,8 +343,8 @@ void PrepareShutdown(NodeContext& node)
             node.llmq_ctx.reset();
         }
         llmq::quorumSnapshotManager.reset();
-        node.dmnman = nullptr;
-        deterministicMNManager.reset();
+        node.mempool->DisconnectManagers();
+        node.dmnman.reset();
         node.cpoolman.reset();
         node.mnhf_manager.reset();
         node.evodb.reset();
@@ -1683,7 +1683,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
     node.netfulfilledman = std::make_unique<CNetFulfilledRequestManager>();
 
     assert(!node.govman);
-    node.govman = std::make_unique<CGovernanceManager>(*node.mn_metaman, *node.netfulfilledman, ::deterministicMNManager, node.mn_sync);
+    node.govman = std::make_unique<CGovernanceManager>(*node.mn_metaman, *node.netfulfilledman, node.dmnman, node.mn_sync);
 
     assert(!node.sporkman);
     node.sporkman = std::make_unique<CSporkManager>();
@@ -1725,15 +1725,15 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
         fMasternodeMode = true;
         {
             // Create and register mn_activeman, will init later in ThreadImport
-            node.mn_activeman = std::make_unique<CActiveMasternodeManager>(keyOperator, *node.connman, ::deterministicMNManager);
+            node.mn_activeman = std::make_unique<CActiveMasternodeManager>(keyOperator, *node.connman, node.dmnman);
             RegisterValidationInterface(node.mn_activeman.get());
         }
     }
 
     assert(!node.peerman);
     node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(),
                                      *node.scheduler, chainman, *node.mempool, *node.mn_metaman, *node.mn_sync,
-                                     *node.govman, *node.sporkman, node.mn_activeman.get(), ::deterministicMNManager,
+                                     *node.govman, *node.sporkman, node.mn_activeman.get(), node.dmnman,
                                      node.cj_ctx, node.llmq_ctx, ignores_incoming_txs);
     RegisterValidationInterface(node.peerman.get());
 
@@ -1858,7 +1858,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
 #endif
 
     pdsNotificationInterface = new CDSNotificationInterface(
-        *node.connman, *node.mn_sync, *node.govman, node.mn_activeman.get(), ::deterministicMNManager, node.llmq_ctx, node.cj_ctx
+        *node.connman, *node.mn_sync, *node.govman, node.mn_activeman.get(), node.dmnman, node.llmq_ctx, node.cj_ctx
     );
     RegisterValidationInterface(pdsNotificationInterface);
 
@@ -1943,11 +1943,13 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
                 pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset));
 
                 // Same logic as above with pblocktree
-                deterministicMNManager.reset();
-                deterministicMNManager = std::make_unique<CDeterministicMNManager>(chainman.ActiveChainstate(), *node.connman, *node.evodb);
-                node.dmnman = deterministicMNManager.get();
+                node.dmnman.reset();
+                node.dmnman = std::make_unique<CDeterministicMNManager>(chainman.ActiveChainstate(), *node.connman, *node.evodb);
+                node.mempool->ConnectManagers(node.dmnman.get());
+
                 node.cpoolman.reset();
                 node.cpoolman = std::make_unique<CCreditPoolManager>(*node.evodb);
+
                 llmq::quorumSnapshotManager.reset();
                 llmq::quorumSnapshotManager.reset(new llmq::CQuorumSnapshotManager(*node.evodb));
 

src/node/context.cpp

@@ -7,8 +7,9 @@
 #include <addrman.h>
 #include <banman.h>
 #include <coinjoin/context.h>
-#include <evo/creditpool.h>
 #include <evo/chainhelper.h>
+#include <evo/creditpool.h>
+#include <evo/deterministicmns.h>
 #include <evo/evodb.h>
 #include <evo/mnhftx.h>
 #include <governance/governance.h>

src/node/context.h

@@ -75,6 +75,7 @@ struct NodeContext {
     std::unique_ptr<CCreditPoolManager> cpoolman;
     std::unique_ptr<CEvoDB> evodb;
     std::unique_ptr<CChainstateHelper> chain_helper;
+    std::unique_ptr<CDeterministicMNManager> dmnman;
     std::unique_ptr<CGovernanceManager> govman;
     std::unique_ptr<CJContext> cj_ctx;
     std::unique_ptr<CMasternodeMetaMan> mn_metaman;
@@ -83,7 +84,6 @@ struct NodeContext {
     std::unique_ptr<CNetFulfilledRequestManager> netfulfilledman;
     std::unique_ptr<CSporkManager> sporkman;
     std::unique_ptr<LLMQContext> llmq_ctx;
-    CDeterministicMNManager* dmnman{nullptr};
 
     //! Declare default constructor and destructor that are not inline, so code
     //! instantiating the NodeContext struct doesn't need to #include class

src/test/denialofservice_tests.cpp

@@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync,
-                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, ::deterministicMNManager,
+                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman,
                                        m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false);
 
     // Mock an outbound peer
@@ -138,7 +138,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
     auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync,
-                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, ::deterministicMNManager,
+                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman,
                                        m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false);
 
     constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS;
@@ -213,7 +213,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync,
-                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, ::deterministicMNManager,
+                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman,
                                        m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false);
 
     banman->ClearBanned();
@@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync,
-                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, ::deterministicMNManager,
+                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman,
                                        m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false);
 
     banman->ClearBanned();

src/test/evo_deterministicmns_tests.cpp

@@ -640,6 +640,9 @@ void FuncTestMempoolReorg(TestChainSetup& setup)
     SignTransaction(*(setup.m_node.mempool), tx_reg, setup.coinbaseKey);
 
     CTxMemPool testPool;
+    if (setup.m_node.dmnman) {
+        testPool.ConnectManagers(setup.m_node.dmnman.get());
+    }
     TestMemPoolEntryHelper entry;
     LOCK2(cs_main, testPool.cs);
 
@@ -709,6 +712,9 @@ void FuncTestMempoolDualProregtx(TestChainSetup& setup)
     SignTransaction(*(setup.m_node.mempool), tx_reg2, setup.coinbaseKey);
 
     CTxMemPool testPool;
+    if (setup.m_node.dmnman) {
+        testPool.ConnectManagers(setup.m_node.dmnman.get());
+    }
     TestMemPoolEntryHelper entry;
     LOCK2(cs_main, testPool.cs);
 

src/test/util/setup_common.cpp

@@ -109,8 +109,9 @@ void DashTestSetup(NodeContext& node, const CChainParams& chainparams)
 {
     CChainState& chainstate = Assert(node.chainman)->ActiveChainstate();
 
-    ::deterministicMNManager = std::make_unique<CDeterministicMNManager>(chainstate, *node.connman, *node.evodb);
-    node.dmnman = ::deterministicMNManager.get();
+    node.dmnman = std::make_unique<CDeterministicMNManager>(chainstate, *node.connman, *node.evodb);
+    node.mempool->ConnectManagers(node.dmnman.get());
+
     node.cj_ctx = std::make_unique<CJContext>(chainstate, *node.connman, *node.dmnman, *node.mn_metaman, *node.mempool,
                                               /* mn_activeman = */ nullptr, *node.mn_sync, /* relay_txes = */ true);
 #ifdef ENABLE_WALLET
@@ -131,8 +132,8 @@ void DashTestSetupClose(NodeContext& node)
 #ifdef ENABLE_WALLET
     node.coinjoin_loader.reset();
 #endif // ENABLE_WALLET
-    node.dmnman = nullptr;
-    ::deterministicMNManager.reset();
+    node.mempool->DisconnectManagers();
+    node.dmnman.reset();
     node.cj_ctx.reset();
 }
 
@@ -231,7 +232,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     m_node.mn_metaman = std::make_unique<CMasternodeMetaMan>();
     m_node.netfulfilledman = std::make_unique<CNetFulfilledRequestManager>();
     m_node.sporkman = std::make_unique<CSporkManager>();
-    m_node.govman = std::make_unique<CGovernanceManager>(*m_node.mn_metaman, *m_node.netfulfilledman, ::deterministicMNManager, m_node.mn_sync);
+    m_node.govman = std::make_unique<CGovernanceManager>(*m_node.mn_metaman, *m_node.netfulfilledman, m_node.dmnman, m_node.mn_sync);
     m_node.mn_sync = std::make_unique<CMasternodeSync>(*m_node.connman, *m_node.netfulfilledman, *m_node.govman);
 
     // Start script-checking threads. Set g_parallel_script_checks to true so they are used.
@@ -283,7 +284,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     m_node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(),
                                        *m_node.scheduler, *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync,
-                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, ::deterministicMNManager,
+                                       *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman,
                                        m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false);
     {
         CConnman::Options options;

src/txmempool.cpp

@@ -347,6 +347,13 @@ CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator, int check_ratio)
     _clear(); //lock free clear
 }
 
+void CTxMemPool::ConnectManagers(CDeterministicMNManager* dmnman)
+{
+    // Do not allow double-initialization
+    assert(m_dmnman == nullptr);
+    m_dmnman = Assert(dmnman);
+}
+
 bool CTxMemPool::isSpent(const COutPoint& outpoint) const
 {
     LOCK(cs);
@@ -415,7 +422,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
     // Invalid ProTxes should never get this far because transactions should be
     // fully checked by AcceptToMemoryPool() at this point, so we just assume that
     // everything is fine here.
-    if (::deterministicMNManager) {
+    if (m_dmnman) {
         addUncheckedProTx(newit, tx);
     }
 }
@@ -554,7 +561,7 @@ bool CTxMemPool::removeSpentIndex(const uint256 txhash)
 
 void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, const CTransaction& tx)
 {
-    assert(::deterministicMNManager);
+    assert(m_dmnman);
 
     if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
         auto proTx = *Assert(GetTxPayload<CProRegTx>(tx));
@@ -577,15 +584,15 @@ void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, con
         auto proTx = *Assert(GetTxPayload<CProUpRegTx>(tx));
         mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
         mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash());
-        auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash));
+        auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash));
         newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
         if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) {
             newit->isKeyChangeProTx = true;
         }
     } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
         auto proTx = *Assert(GetTxPayload<CProUpRevTx>(tx));
         mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
-        auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash));
+        auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash));
         newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
         if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) {
             newit->isKeyChangeProTx = true;
@@ -623,7 +630,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     } else
         vTxHashes.clear();
 
-    if (::deterministicMNManager) {
+    if (m_dmnman) {
         removeUncheckedProTx(it->GetTx());
     }
 
@@ -844,7 +851,7 @@ void CTxMemPool::removeProTxCollateralConflicts(const CTransaction &tx, const CO
 
 void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx)
 {
-    assert(::deterministicMNManager);
+    assert(m_dmnman);
 
     // Remove TXs that refer to a MN for which the collateral was spent
     auto removeSpentCollateralConflict = [&](const uint256& proTxHash) {
@@ -866,7 +873,7 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx)
             }
         }
     };
-    auto mnList = deterministicMNManager->GetListAtChainTip();
+    auto mnList = m_dmnman->GetListAtChainTip();
     for (const auto& in : tx.vin) {
         auto collateralIt = mapProTxCollaterals.find(in.prevout);
         if (collateralIt != mapProTxCollaterals.end()) {
@@ -983,7 +990,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
             RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
         }
         removeConflicts(*tx);
-        if (::deterministicMNManager) {
+        if (m_dmnman) {
             removeProTxConflicts(*tx);
         }
         ClearPrioritisation(tx->GetHash());
@@ -1259,7 +1266,7 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
 }
 
 bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
-    assert(::deterministicMNManager);
+    assert(m_dmnman);
 
     LOCK(cs);
 
@@ -1314,7 +1321,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
         auto& proTx = *opt_proTx;
 
         // this method should only be called with validated ProTxs
-        auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash);
+        auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
         if (!dmn) {
             LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString());
             return true; // i.e. failed to find validated ProTx == conflict
@@ -1336,7 +1343,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
         }
         auto& proTx = *opt_proTx;
         // this method should only be called with validated ProTxs
-        auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash);
+        auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash);
         if (!dmn) {
             LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString());
             return true; // i.e. failed to find validated ProTx == conflict

src/txmempool.h

@@ -361,6 +361,7 @@ struct entry_time {};
 struct ancestor_score {};
 
 class CBlockPolicyEstimator;
+class CDeterministicMNManager;
 
 /**
  * Information about a mempool transaction.
@@ -472,6 +473,7 @@ class CTxMemPool
     const int m_check_ratio; //!< Value n means that 1 times in n we check.
     std::atomic<unsigned int> nTransactionsUpdated{0}; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation
     CBlockPolicyEstimator* minerPolicyEstimator;
+    CDeterministicMNManager* m_dmnman{nullptr};
 
     uint64_t totalTxSize GUARDED_BY(cs);      //!< sum of all mempool tx' byte sizes
     CAmount m_total_fee GUARDED_BY(cs);       //!< sum of all mempool tx's fees (NOT modified fee)
@@ -599,6 +601,21 @@ class CTxMemPool
      */
     explicit CTxMemPool(CBlockPolicyEstimator* estimator = nullptr, int check_ratio = 0);
 
+    /**
+     * Set CDeterministicMNManager pointer.
+     *
+     * Separated from constructor as it's initialized after CTxMemPool
+     * is created. Required for ProTx processing.
+     */
+    void ConnectManagers(CDeterministicMNManager* dmnman);
+
+    /**
+     * Reset CDeterministicMNManager pointer.
+     *
+     * @pre Must be called before CDeterministicMNManager is destroyed.
+     */
+    void DisconnectManagers() { m_dmnman = nullptr; }
+
     /**
      * If sanity-checking is turned on, check makes sure the pool is
      * consistent (does not contain two transactions that spend the same inputs,
@@ -759,7 +776,10 @@ class CTxMemPool
     TxMempoolInfo info(const uint256& hash) const;
     std::vector<TxMempoolInfo> infoAll() const;
 
-    /** @pre Caller must ensure that CDeterministicMNManager exists */
+    /**
+     * @pre Caller must ensure that CDeterministicMNManager exists and has been
+     *      set using ConnectManagers() for the CTxMemPool instance.
+     */
     bool existsProviderTxConflict(const CTransaction &tx) const;
 
     size_t DynamicMemoryUsage() const;