refactor: pass CActiveMasternodeManager as pointer arg to CJContext

We could use std::optional<std::reference_wrapper<const CActiveMasternodeManager>> but then we'd also have to contend with accessing the value with mn_activeman. value().get(). We assert m_mn_activeman is present instead of fast-failing when it isn't because of the fMasternodeMode fast-fail check. If we are in masternode mode, m_mn_activeman should point to a valid target. If it doesn't, something's gone wrong.
vijaydasmp · Mar 24, 2024 · 5e0f777 · 5e0f777
1 parent f171c24
commit 5e0f777
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 27 deletions.
diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp
@@ -47,12 +47,10 @@ uint256 CCoinJoinQueue::GetSignatureHash() const
     return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
 }
 
-bool CCoinJoinQueue::Sign()
+bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman)
 {
-    if (!fMasternodeMode) return false;
-
     uint256 hash = GetSignatureHash();
-    CBLSSignature sig = ::activeMasternodeManager->Sign(hash, /*is_legacy=*/ false);
+    CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
     if (!sig.IsValid()) {
         return false;
     }
@@ -99,12 +97,10 @@ uint256 CCoinJoinBroadcastTx::GetSignatureHash() const
     return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
 }
 
-bool CCoinJoinBroadcastTx::Sign()
+bool CCoinJoinBroadcastTx::Sign(const CActiveMasternodeManager& mn_activeman)
 {
-    if (!fMasternodeMode) return false;
-
     uint256 hash = GetSignatureHash();
-    CBLSSignature sig = ::activeMasternodeManager->Sign(hash, /*is_legacy=*/ false);
+    CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
     if (!sig.IsValid()) {
         return false;
     }

diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h
@@ -22,6 +22,7 @@
 #include <optional>
 #include <utility>
 
+class CActiveMasternodeManager;
 class CChainState;
 class CConnman;
 class CBLSPublicKey;
@@ -213,7 +214,7 @@ class CCoinJoinQueue
      *     3) we signed the message successfully, and
      *     4) we verified the message successfully
      */
-    bool Sign();
+    bool Sign(const CActiveMasternodeManager& mn_activeman);
     /// Check if we have a valid Masternode address
     [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;
 
@@ -284,7 +285,7 @@ class CCoinJoinBroadcastTx
 
     [[nodiscard]] uint256 GetSignatureHash() const;
 
-    bool Sign();
+    bool Sign(const CActiveMasternodeManager& mn_activeman);
     [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;
 
     void SetConfirmedHeight(std::optional<int> nConfirmedHeightIn) { assert(nConfirmedHeightIn == std::nullopt || *nConfirmedHeightIn > 0); nConfirmedHeight = nConfirmedHeightIn; }

diff --git a/src/coinjoin/context.cpp b/src/coinjoin/context.cpp
@@ -14,13 +14,13 @@
 #include <coinjoin/server.h>
 
 CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
-                     const CMasternodeSync& mn_sync, bool relay_txes) :
+                     const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) :
     dstxman{std::make_unique<CDSTXManager>()},
 #ifdef ENABLE_WALLET
     walletman{std::make_unique<CoinJoinWalletManager>(connman, dmnman, mempool, mn_sync, queueman)},
     queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *walletman, dmnman, mn_sync) : nullptr},
 #endif // ENABLE_WALLET
-    server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mempool, mn_sync)}
+    server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mempool, mn_activeman, mn_sync)}
 {}
 
 CJContext::~CJContext() {}
diff --git a/src/coinjoin/context.h b/src/coinjoin/context.h
@@ -11,6 +11,7 @@
 
 #include <memory>
 
+class CActiveMasternodeManager;
 class CBlockPolicyEstimator;
 class CChainState;
 class CCoinJoinServer;
@@ -29,7 +30,7 @@ struct CJContext {
     CJContext() = delete;
     CJContext(const CJContext&) = delete;
     CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
-              const CMasternodeSync& mn_sync, bool relay_txes);
+              const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes);
     ~CJContext();
 
     const std::unique_ptr<CDSTXManager> dstxman;

diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp
@@ -43,6 +43,8 @@ PeerMsgRet CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_typ
 
 void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
 {
+    assert(m_mn_activeman);
+
     if (IsSessionReady()) {
         // too many users in this session already, reject new ones
         LogPrint(BCLog::COINJOIN, "DSACCEPT -- queue is already full!\n");
@@ -56,7 +58,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
     LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s)  txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */
 
     auto mnList = m_dmnman.GetListAtChainTip();
-    auto dmn = WITH_LOCK(::activeMasternodeManager->cs, return mnList.GetValidMNByCollateral(::activeMasternodeManager->GetOutPoint()));
+    auto dmn = WITH_LOCK(m_mn_activeman->cs, return mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint()));
     if (!dmn) {
         PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
         return;
@@ -67,7 +69,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
             TRY_LOCK(cs_vecqueue, lockRecv);
             if (!lockRecv) return;
 
-            auto mnOutpoint = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint());
+            auto mnOutpoint = WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint());
 
             if (ranges::any_of(vecCoinJoinQueue,
                                [&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
@@ -308,6 +310,8 @@ void CCoinJoinServer::CommitFinalTransaction()
     AssertLockNotHeld(cs_coinjoin);
     if (!fMasternodeMode) return; // check and relay final tx only on masternode
 
+    assert(m_mn_activeman);
+
     CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction));
     uint256 hashTx = finalTransaction->GetHash();
 
@@ -331,10 +335,10 @@ void CCoinJoinServer::CommitFinalTransaction()
     // create and sign masternode dstx transaction
     if (!m_dstxman.GetDSTX(hashTx)) {
         CCoinJoinBroadcastTx dstxNew(finalTransaction,
-                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
+                                    WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
+                                    WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
                                     GetAdjustedTime());
-        dstxNew.Sign();
+        dstxNew.Sign(*m_mn_activeman);
         m_dstxman.AddDSTX(dstxNew);
     }
 
@@ -495,16 +499,18 @@ void CCoinJoinServer::CheckForCompleteQueue()
 {
     if (!fMasternodeMode) return;
 
+    assert(m_mn_activeman);
+
     if (nState == POOL_STATE_QUEUE && IsSessionReady()) {
         SetState(POOL_STATE_ACCEPTING_ENTRIES);
 
         CCoinJoinQueue dsq(nSessionDenom,
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
+                            WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
+                            WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
                             GetAdjustedTime(), true);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
                                      "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
-        dsq.Sign();
+        dsq.Sign(*m_mn_activeman);
         dsq.Relay(connman);
     }
 }
@@ -692,6 +698,8 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
 {
     if (!fMasternodeMode || nSessionID != 0) return false;
 
+    assert(m_mn_activeman);
+
     // new session can only be started in idle mode
     if (nState != POOL_STATE_IDLE) {
         nMessageIDRet = ERR_MODE;
@@ -713,11 +721,11 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
     if (!fUnitTest) {
         //broadcast that I'm accepting entries, only if it's the first entry through
         CCoinJoinQueue dsq(nSessionDenom,
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
+                            WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
+                            WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
                             GetAdjustedTime(), false);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
-        dsq.Sign();
+        dsq.Sign(*m_mn_activeman);
         dsq.Relay(connman);
         LOCK(cs_vecqueue);
         vecCoinJoinQueue.push_back(dsq);

diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h
@@ -9,6 +9,7 @@
 
 #include <net_types.h>
 
+class CActiveMasternodeManager;
 class CChainState;
 class CCoinJoinServer;
 class CDataStream;
@@ -29,6 +30,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
     CDeterministicMNManager& m_dmnman;
     CDSTXManager& m_dstxman;
     CTxMemPool& mempool;
+    const CActiveMasternodeManager* m_mn_activeman;
     const CMasternodeSync& m_mn_sync;
 
     // Mixing uses collateral transactions to trust parties entering the pool
@@ -85,12 +87,13 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
 
 public:
     explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman,
-                             CTxMemPool& mempool, const CMasternodeSync& mn_sync) :
+                             CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync) :
         m_chainstate(chainstate),
         connman(_connman),
         m_dmnman(dmnman),
         m_dstxman(dstxman),
         mempool(mempool),
+        m_mn_activeman(mn_activeman),
         m_mn_sync(mn_sync),
         vecSessionCollaterals(),
         fUnitTest(false)

diff --git a/src/init.cpp b/src/init.cpp
@@ -2199,7 +2199,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
 
     // ********************************************************* Step 7c: Setup CoinJoin
 
-    node.cj_ctx = std::make_unique<CJContext>(chainman.ActiveChainstate(), *node.connman, *node.dmnman, *node.mempool, *node.mn_sync, !ignores_incoming_txs);
+    node.cj_ctx = std::make_unique<CJContext>(chainman.ActiveChainstate(), *node.connman, *node.dmnman, *node.mempool, node.mn_activeman,
+                                              *node.mn_sync, !ignores_incoming_txs);
 
 #ifdef ENABLE_WALLET
     node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*node.cj_ctx->walletman);

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -111,7 +111,7 @@ void DashTestSetup(NodeContext& node, const CChainParams& chainparams)
 
     ::deterministicMNManager = std::make_unique<CDeterministicMNManager>(chainstate, *node.connman, *node.evodb);
     node.dmnman = ::deterministicMNManager.get();
-    node.cj_ctx = std::make_unique<CJContext>(chainstate, *node.connman, *node.dmnman, *node.mempool, *node.mn_sync, /* relay_txes */ true);
+    node.cj_ctx = std::make_unique<CJContext>(chainstate, *node.connman, *node.dmnman, *node.mempool, /* mn_activeman = */ nullptr, *node.mn_sync, /* relay_txes = */ true);
 #ifdef ENABLE_WALLET
     node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*node.cj_ctx->walletman);
 #endif // ENABLE_WALLET