refactor: subsume CoinJoin objects under CJContext, deglobalize coinJ…

…oin{ClientQueueManager,Server} (dashpay#5337)

## Motivation

CoinJoin's subsystems are initialized by variables and managers that
occupy the global context. The _extent_ to which these subsystems
entrench themselves into the codebase is difficult to assess and moving
them out of the global context forces us to enumerate the subsystems in
the codebase that rely on CoinJoin logic and enumerate the order in
which components are initialized and destroyed.

Keeping this in mind, the scope of this pull request aims to:

* Reduce the amount of CoinJoin-specific entities present in the global
scope
* Make the remaining usage of these entities in the global scope
explicit and easily searchable

## Additional Information

* The initialization of `CCoinJoinClientQueueManager` is dependent on
blocks-only mode being disabled (which can be alternatively interpreted
as enabling the relay of transactions). The same applies to
`CBlockPolicyEstimator`, which `CCoinJoinClientQueueManager` depends.

Therefore, `CCoinJoinClientQueueManager` is only initialized if
transaction relaying is enabled and so is its scheduled maintenance
task. This can be found by looking at `init.cpp`
[here](https://github.com/dashpay/dash/blob/93f8df1c31fdce6a14f149acfdff22678c3f22ca/src/init.cpp#L1681-L1683),
[here](https://github.com/dashpay/dash/blob/93f8df1c31fdce6a14f149acfdff22678c3f22ca/src/init.cpp#L2253-L2255)
and
[here](https://github.com/dashpay/dash/blob/93f8df1c31fdce6a14f149acfdff22678c3f22ca/src/init.cpp#L2326-L2327).
  
For this reason, `CBlockPolicyEstimator` is not a member of `CJContext`
and its usage is fulfilled by passing it as a reference when
initializing the scheduling task.

* `CJClientManager` has not used `CConnman` or `CTxMemPool` as `const`
as existing code that is outside the scope of this PR would cast away
constness, which would be unacceptable. Furthermore, some logical paths
are taken that will grind to a halt if they are stored as `const`.

  Examples of such a call chains would be:

* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoinClientSession::DoAutomaticDenominating >
CCoinJoinClientSession::StartNewQueue > CConnman::AddPendingMasternode`
which modifies `CConnman::vPendingMasternodes`, which is non-const
behaviour

* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoin::IsCollateralValid > AcceptToMemoryPool` which adds a
transaction to the memory pool, which is non-const behaviour

* There were cppcheck [linter
failures](dashpay#5337 (comment))
that seemed to be caused by the usage of `Assert` in
`coinjoin/client.h`. This seems to be resolved by backporting
[bitcoin#24714](bitcoin#24714). (Thanks
@knst!)
    * Depends on dashpay#5546

---------

Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

src/Makefile.am

@@ -149,6 +149,7 @@ BITCOIN_CORE_H = \
   clientversion.h \
   coinjoin/coinjoin.h \
   coinjoin/client.h \
+  coinjoin/context.h \
   coinjoin/options.h \
   coinjoin/server.h \
   coinjoin/util.h \
@@ -391,6 +392,7 @@ libbitcoin_server_a_SOURCES = \
   blockfilter.cpp \
   chain.cpp \
   coinjoin/coinjoin.cpp \
+  coinjoin/context.cpp \
   coinjoin/options.cpp \
   coinjoin/server.cpp \
   consensus/tx_verify.cpp \

src/coinjoin/client.h

@@ -7,33 +7,31 @@
 
 #include <coinjoin/util.h>
 #include <coinjoin/coinjoin.h>
+#include <util/check.h>
 #include <util/translation.h>
 
 #include <atomic>
 #include <deque>
+#include <memory>
 #include <utility>
 
-class CDeterministicMN;
-using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
-
+class CBlockPolicyEstimator;
 class CCoinJoinClientManager;
 class CCoinJoinClientQueueManager;
-
-class CBlockPolicyEstimator;
 class CConnman;
+class CDeterministicMN;
+class CJClientManager;
 class CNode;
+class CMasternodeSync;
 class CTxMemPool;
 class PeerManager;
 
 class UniValue;
-class CMasternodeSync;
 
+using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
 
 // The main object for accessing mixing
-extern std::map<const std::string, std::shared_ptr<CCoinJoinClientManager>> coinJoinClientManagers;
-
-// The object to track mixing queues
-extern std::unique_ptr<CCoinJoinClientQueueManager> coinJoinClientQueueManager;
+extern std::unique_ptr<CJClientManager> coinJoinClientManagers;
 
 class CPendingDsaRequest
 {
@@ -72,10 +70,53 @@ class CPendingDsaRequest
     }
 };
 
+class CJClientManager {
+public:
+    using wallet_name_cjman_map = std::map<const std::string, std::unique_ptr<CCoinJoinClientManager>>;
+
+public:
+    CJClientManager(CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync,
+                    const std::unique_ptr<CCoinJoinClientQueueManager>& queueman)
+        : m_connman(connman), m_mempool(mempool), m_mn_sync(mn_sync), m_queueman(queueman) {}
+    ~CJClientManager() {
+        for (auto& [wallet_name, cj_man] : m_wallet_manager_map) {
+            cj_man.reset();
+        }
+    }
+
+    void Add(CWallet& wallet);
+    void DoMaintenance(CBlockPolicyEstimator& fee_estimator);
+
+    void Remove(const std::string& name) {
+        m_wallet_manager_map.erase(name);
+    }
+
+    CCoinJoinClientManager* Get(const CWallet& wallet) const {
+        auto it = m_wallet_manager_map.find(wallet.GetName());
+        return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr;
+    }
+
+    const wallet_name_cjman_map& raw() const { return m_wallet_manager_map; }
+
+private:
+    CConnman& m_connman;
+    CTxMemPool& m_mempool;
+
+    const CMasternodeSync& m_mn_sync;
+    const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
+
+    wallet_name_cjman_map m_wallet_manager_map;
+};
+
 class CCoinJoinClientSession : public CCoinJoinBaseSession
 {
 private:
+    CWallet& m_wallet;
+    CJClientManager& m_clientman;
+    CCoinJoinClientManager& m_manager;
+
     const CMasternodeSync& m_mn_sync;
+    const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
 
     std::vector<COutPoint> vecOutPointLocked;
 
@@ -88,8 +129,6 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
 
     CKeyHolderStorage keyHolderStorage; // storage for keys used in PrepareDenominate
 
-    CWallet& mixingWallet;
-
     /// Create denominations
     bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate);
     bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals);
@@ -125,10 +164,9 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     void SetNull() EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
 public:
-    explicit CCoinJoinClientSession(CWallet& pwallet, const CMasternodeSync& mn_sync) :
-        m_mn_sync(mn_sync), mixingWallet(pwallet)
-    {
-    }
+    explicit CCoinJoinClientSession(CWallet& pwallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
+                                    const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
+        m_wallet(pwallet), m_clientman(clientman), m_manager(*Assert(clientman.Get(pwallet))), m_mn_sync(mn_sync), m_queueman(queueman) {}
 
     void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);
 
@@ -159,12 +197,13 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
 {
 private:
     CConnman& connman;
+    CJClientManager& m_clientman;
     const CMasternodeSync& m_mn_sync;
     mutable Mutex cs_ProcessDSQueue;
 
 public:
-    explicit CCoinJoinClientQueueManager(CConnman& _connman, const CMasternodeSync& mn_sync) :
-        connman(_connman), m_mn_sync(mn_sync) {};
+    explicit CCoinJoinClientQueueManager(CConnman& _connman, CJClientManager& clientman, const CMasternodeSync& mn_sync) :
+        connman(_connman), m_clientman(clientman), m_mn_sync(mn_sync) {};
 
     void ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
     void ProcessDSQueue(const CNode& peer, PeerManager& peerman, CDataStream& vRecv);
@@ -176,10 +215,14 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
 class CCoinJoinClientManager
 {
 private:
-    // Keep track of the used Masternodes
-    std::vector<COutPoint> vecMasternodesUsed;
+    CWallet& m_wallet;
+    CJClientManager& m_clientman;
 
     const CMasternodeSync& m_mn_sync;
+    const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
+
+    // Keep track of the used Masternodes
+    std::vector<COutPoint> vecMasternodesUsed;
 
     mutable Mutex cs_deqsessions;
     // TODO: or map<denom, CCoinJoinClientSession> ??
@@ -191,8 +234,6 @@ class CCoinJoinClientManager
     int nMinBlocksToWait{1}; // how many blocks to wait for after one successful mixing tx in non-multisession mode
     bilingual_str strAutoDenomResult;
 
-    CWallet& mixingWallet;
-
     // Keep track of current block height
     int nCachedBlockHeight{0};
 
@@ -209,8 +250,9 @@ class CCoinJoinClientManager
     CCoinJoinClientManager(CCoinJoinClientManager const&) = delete;
     CCoinJoinClientManager& operator=(CCoinJoinClientManager const&) = delete;
 
-    explicit CCoinJoinClientManager(CWallet& wallet, const CMasternodeSync& mn_sync) :
-        m_mn_sync(mn_sync), mixingWallet(wallet) {}
+    explicit CCoinJoinClientManager(CWallet& wallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
+                                    const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
+        m_wallet(wallet), m_clientman(clientman), m_mn_sync(mn_sync), m_queueman(queueman) {}
 
     void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_deqsessions);
 
@@ -246,7 +288,4 @@ class CCoinJoinClientManager
     void GetJsonInfo(UniValue& obj) const LOCKS_EXCLUDED(cs_deqsessions);
 };
 
-
-void DoCoinJoinMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool);
-
 #endif // BITCOIN_COINJOIN_CLIENT_H

src/coinjoin/context.cpp

@@ -0,0 +1,34 @@
+// Copyright (c) 2023 The Dash Core developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <coinjoin/context.h>
+
+#include <net.h>
+#include <policy/fees.h>
+#include <txmempool.h>
+
+#ifdef ENABLE_WALLET
+#include <coinjoin/client.h>
+#endif // ENABLE_WALLET
+#include <coinjoin/server.h>
+
+CJContext::CJContext(CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, bool relay_txes) :
+#ifdef ENABLE_WALLET
+    clientman {
+        [&]() -> CJClientManager* const {
+            assert(::coinJoinClientManagers == nullptr);
+            ::coinJoinClientManagers = std::make_unique<CJClientManager>(connman, mempool, mn_sync, queueman);
+            return ::coinJoinClientManagers.get();
+        }()
+    },
+    queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *clientman, mn_sync) : nullptr},
+#endif // ENABLE_WALLET
+    server{std::make_unique<CCoinJoinServer>(chainstate, connman, mempool, mn_sync)}
+{}
+
+CJContext::~CJContext() {
+#ifdef ENABLE_WALLET
+    ::coinJoinClientManagers.reset();
+#endif // ENABLE_WALLET
+}

src/coinjoin/context.h

@@ -0,0 +1,39 @@
+// Copyright (c) 2023 The Dash Core developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_COINJOIN_CONTEXT_H
+#define BITCOIN_COINJOIN_CONTEXT_H
+
+#if defined(HAVE_CONFIG_H)
+#include <config/bitcoin-config.h>
+#endif
+
+#include <memory>
+
+class CBlockPolicyEstimator;
+class CChainState;
+class CCoinJoinServer;
+class CConnman;
+class CMasternodeSync;
+class CTxMemPool;
+
+#ifdef ENABLE_WALLET
+class CCoinJoinClientQueueManager;
+class CJClientManager;
+#endif // ENABLE_WALLET
+
+struct CJContext {
+    CJContext() = delete;
+    CJContext(const CJContext&) = delete;
+    CJContext(CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, bool relay_txes);
+    ~CJContext();
+
+#ifdef ENABLE_WALLET
+    CJClientManager* const clientman;
+    const std::unique_ptr<CCoinJoinClientQueueManager> queueman;
+#endif // ENABLE_WALLET
+    const std::unique_ptr<CCoinJoinServer> server;
+};
+
+#endif // BITCOIN_COINJOIN_CONTEXT_H

src/coinjoin/server.cpp

@@ -23,7 +23,6 @@
 
 #include <univalue.h>
 
-std::unique_ptr<CCoinJoinServer> coinJoinServer;
 constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10};
 
 void CCoinJoinServer::ProcessMessage(CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
@@ -886,17 +885,15 @@ void CCoinJoinServer::SetState(PoolState nStateNew)
     nState = nStateNew;
 }
 
-void CCoinJoinServer::DoMaintenance() const
+void CCoinJoinServer::DoMaintenance()
 {
     if (!fMasternodeMode) return; // only run on masternodes
     if (!m_mn_sync.IsBlockchainSynced()) return;
     if (ShutdownRequested()) return;
 
-    if (!coinJoinServer) return;
-
-    coinJoinServer->CheckForCompleteQueue();
-    coinJoinServer->CheckPool();
-    coinJoinServer->CheckTimeout();
+    CheckForCompleteQueue();
+    CheckPool();
+    CheckTimeout();
 }
 
 void CCoinJoinServer::GetJsonInfo(UniValue& obj) const

src/coinjoin/server.h

@@ -15,9 +15,6 @@ class PeerManager;
 
 class UniValue;
 
-// The main object for accessing mixing
-extern std::unique_ptr<CCoinJoinServer> coinJoinServer;
-
 /** Used to keep track of current status of mixing pool
  */
 class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
@@ -96,7 +93,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
     void CheckTimeout();
     void CheckForCompleteQueue();
 
-    void DoMaintenance() const;
+    void DoMaintenance();
 
     void GetJsonInfo(UniValue& obj) const;
 };

src/dsnotificationinterface.cpp

@@ -7,6 +7,7 @@
 #ifdef ENABLE_WALLET
 #include <coinjoin/client.h>
 #endif // ENABLE_WALLET
+#include <coinjoin/context.h>
 #include <dsnotificationinterface.h>
 #include <governance/governance.h>
 #include <masternode/sync.h>
@@ -23,8 +24,9 @@
 
 CDSNotificationInterface::CDSNotificationInterface(CConnman& _connman,
                                                    CMasternodeSync& _mn_sync, const std::unique_ptr<CDeterministicMNManager>& _dmnman,
-                                                   CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx
-) : connman(_connman), m_mn_sync(_mn_sync), dmnman(_dmnman), govman(_govman), llmq_ctx(_llmq_ctx) {}
+                                                   CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx,
+                                                   const std::unique_ptr<CJContext>& _cj_ctx
+) : connman(_connman), m_mn_sync(_mn_sync), dmnman(_dmnman), govman(_govman), llmq_ctx(_llmq_ctx), cj_ctx(_cj_ctx) {}
 
 void CDSNotificationInterface::InitializeCurrentBlockTip()
 {
@@ -66,7 +68,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con
 
     CCoinJoin::UpdatedBlockTip(pindexNew, *llmq_ctx->clhandler, m_mn_sync);
 #ifdef ENABLE_WALLET
-    for (auto& pair : coinJoinClientManagers) {
+    for (auto& pair : cj_ctx->clientman->raw()) {
         pair.second->UpdatedBlockTip(pindexNew);
     }
 #endif // ENABLE_WALLET

src/dsnotificationinterface.h

@@ -11,14 +11,16 @@ class CConnman;
 class CDeterministicMNManager;
 class CGovernanceManager;
 class CMasternodeSync;
+struct CJContext;
 struct LLMQContext;
 
 class CDSNotificationInterface : public CValidationInterface
 {
 public:
     explicit CDSNotificationInterface(CConnman& _connman,
                                       CMasternodeSync& _mn_sync, const std::unique_ptr<CDeterministicMNManager>& _dmnman,
-                                      CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx);
+                                      CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx,
+                                      const std::unique_ptr<CJContext>& _cj_ctx);
     virtual ~CDSNotificationInterface() = default;
 
     // a small helper to initialize current block height in sub-modules on startup
@@ -45,6 +47,7 @@ class CDSNotificationInterface : public CValidationInterface
     CGovernanceManager& govman;
 
     const std::unique_ptr<LLMQContext>& llmq_ctx;
+    const std::unique_ptr<CJContext>& cj_ctx;
 };
 
 #endif // BITCOIN_DSNOTIFICATIONINTERFACE_H

src/dummywallet.cpp

@@ -24,7 +24,7 @@ class DummyWalletInit : public WalletInitInterface {
 
     // Dash Specific WalletInitInterface InitCoinJoinSettings
     void AutoLockMasternodeCollaterals() const override {}
-    void InitCoinJoinSettings() const override {}
+    void InitCoinJoinSettings(const CJClientManager& clientman) const override {}
     bool InitAutoBackup() const override {return true;}
 };
 
@@ -74,7 +74,7 @@ const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();
 
 namespace interfaces {
 
-std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
+std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet, const CJClientManager& clientman)
 {
     throw std::logic_error("Wallet function called in non-wallet build.");
 }