refactor: consolidate activeMasternodeInfo{Cs} into CActiveMasternodeManager, create NodeContext alias, reduce globals usage

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 = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, 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 = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false));
+    CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
     if (!sig.IsValid()) {
         return false;
     }

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; }

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() {}

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;

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(activeMasternodeInfoCs, return mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint));
+    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(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint);
+            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(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
-                                    WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
+                                    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(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
-                            WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
+                            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(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
-                            WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
+                            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);

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)

src/evo/mnauth.cpp

@@ -21,40 +21,45 @@
 
 void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
 {
-    LOCK(activeMasternodeInfoCs);
-    if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) {
-        return;
-    }
+    if (!fMasternodeMode) return;
 
+    CMNAuth mnauth;
     uint256 signHash;
-    const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
-    if (receivedMNAuthChallenge.IsNull()) {
-        return;
-    }
-    // We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
-    // we protect ourselves against MITM in this form:
-    //   node1 <- Eve -> node2
-    // It does not protect against:
-    //   node1 -> Eve -> node2
-    // This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
-    int nOurNodeVersion{PROTOCOL_VERSION};
-    if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
-        nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
-    }
-    const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
-    const CBLSPublicKeyVersionWrapper pubKey(*activeMasternodeInfo.blsPubKeyOperator, !is_basic_scheme_active);
-    if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
-        signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
-    } else {
-        signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
-    }
+    {
+        LOCK(::activeMasternodeManager->cs);
+        if (::activeMasternodeManager->GetProTxHash().IsNull()) {
+            return;
+        }
 
-    CMNAuth mnauth;
-    mnauth.proRegTxHash = activeMasternodeInfo.proTxHash;
-    mnauth.sig = activeMasternodeInfo.blsKeyOperator->Sign(signHash);
+        const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
+        if (receivedMNAuthChallenge.IsNull()) {
+            return;
+        }
+        // We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
+        // we protect ourselves against MITM in this form:
+        //   node1 <- Eve -> node2
+        // It does not protect against:
+        //   node1 -> Eve -> node2
+        // This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
+        int nOurNodeVersion{PROTOCOL_VERSION};
+        if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
+            nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
+        }
+        const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
+        auto pk = ::activeMasternodeManager->GetPubKey();
+        const CBLSPublicKeyVersionWrapper pubKey(pk, !is_basic_scheme_active);
+        if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
+            signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
+        } else {
+            signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
+        }
 
-    LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
+        mnauth.proRegTxHash = ::activeMasternodeManager->GetProTxHash();
+    } // ::activeMasternodeManager->cs
 
+    mnauth.sig = ::activeMasternodeManager->Sign(signHash);
+
+    LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
     connman.PushMessage(&peer, CNetMsgMaker(peer.GetCommonVersion()).Make(NetMsgType::MNAUTH, mnauth));
 }
 
@@ -127,7 +132,9 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterm
         }
     }
 
-    const uint256 myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
+    const uint256 myProTxHash = fMasternodeMode ?
+                                WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
+                                uint256();
 
     connman.ForEachNode([&](CNode* pnode2) {
         if (peer.fDisconnect) {

src/governance/governance.cpp

@@ -692,14 +692,14 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
     }
 
     {
-        LOCK(activeMasternodeInfoCs);
-        if (mn_payees.front()->proTxHash != activeMasternodeInfo.proTxHash) {
+        LOCK(::activeMasternodeManager->cs);
+        if (mn_payees.front()->proTxHash != ::activeMasternodeManager->GetProTxHash()) {
             LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__);
             return std::nullopt;
         }
-        gov_sb.SetMasternodeOutpoint(activeMasternodeInfo.outpoint);
-        gov_sb.Sign( *activeMasternodeInfo.blsKeyOperator);
-    } // activeMasternodeInfoCs
+        gov_sb.SetMasternodeOutpoint(::activeMasternodeManager->GetOutPoint());
+    } // ::activeMasternodeManager->cs
+    gov_sb.Sign(*::activeMasternodeManager);
 
     if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), strError, true)) {
         LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError);
@@ -719,7 +719,8 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
 void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGovernanceObject>& trigger_opt, CConnman& connman)
 {
     // only active masternodes can vote on triggers
-    if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return;
+    if (!fMasternodeMode) return;
+    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return;
 
     LOCK2(cs_main, cs);
 
@@ -762,9 +763,9 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover
 
 bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman)
 {
-    CGovernanceVote vote(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), nHash, VOTE_SIGNAL_FUNDING, outcome);
+    CGovernanceVote vote(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()), nHash, VOTE_SIGNAL_FUNDING, outcome);
     vote.SetTime(GetAdjustedTime());
-    vote.Sign(WITH_LOCK(activeMasternodeInfoCs, return *activeMasternodeInfo.blsKeyOperator));
+    vote.Sign(*::activeMasternodeManager);
 
     CGovernanceException exception;
     if (!ProcessVoteAndRelay(vote, exception, connman)) {

src/governance/object.cpp

@@ -11,6 +11,7 @@
 #include <governance/governance.h>
 #include <governance/validators.h>
 #include <masternode/meta.h>
+#include <masternode/node.h>
 #include <masternode/sync.h>
 #include <messagesigner.h>
 #include <net.h>
@@ -273,9 +274,9 @@ void CGovernanceObject::SetMasternodeOutpoint(const COutPoint& outpoint)
     m_obj.masternodeOutpoint = outpoint;
 }
 
-bool CGovernanceObject::Sign(const CBLSSecretKey& key)
+bool CGovernanceObject::Sign(const CActiveMasternodeManager& mn_activeman)
 {
-    CBLSSignature sig = key.Sign(GetSignatureHash(), false);
+    CBLSSignature sig = mn_activeman.Sign(GetSignatureHash(), false);
     if (!sig.IsValid()) {
         return false;
     }

src/governance/object.h

@@ -13,7 +13,7 @@
 
 #include <univalue.h>
 
-class CBLSSecretKey;
+class CActiveMasternodeManager;
 class CBLSPublicKey;
 class CDeterministicMNList;
 class CGovernanceManager;
@@ -217,7 +217,7 @@ class CGovernanceObject
     // Signature related functions
 
     void SetMasternodeOutpoint(const COutPoint& outpoint);
-    bool Sign(const CBLSSecretKey& key);
+    bool Sign(const CActiveMasternodeManager& mn_activeman);
     bool CheckSignature(const CBLSPublicKey& pubKey) const;
 
     uint256 GetSignatureHash() const;