refactor: make m_info private, get const refs (or copies) from Get*() functions

External logic should not be able to mutate the CActiveMasternodeManager
state (i.e. CActiveMasternodeInfo). Access is brokered through getter
functions.

src/coinjoin/server.cpp

@@ -56,7 +56,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->m_info.outpoint));
+    auto dmn = WITH_LOCK(::activeMasternodeManager->cs, return mnList.GetValidMNByCollateral(::activeMasternodeManager->GetOutPoint()));
     if (!dmn) {
         PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
         return;
@@ -67,7 +67,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
             TRY_LOCK(cs_vecqueue, lockRecv);
             if (!lockRecv) return;
 
-            auto mnOutpoint = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.outpoint);
+            auto mnOutpoint = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint());
 
             if (ranges::any_of(vecCoinJoinQueue,
                                [&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
@@ -331,8 +331,8 @@ void CCoinJoinServer::CommitFinalTransaction()
     // create and sign masternode dstx transaction
     if (!m_dstxman.GetDSTX(hashTx)) {
         CCoinJoinBroadcastTx dstxNew(finalTransaction,
-                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.outpoint),
+                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash),
+                                    WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
                                     GetAdjustedTime());
         dstxNew.Sign();
         m_dstxman.AddDSTX(dstxNew);
@@ -499,8 +499,8 @@ void CCoinJoinServer::CheckForCompleteQueue()
         SetState(POOL_STATE_ACCEPTING_ENTRIES);
 
         CCoinJoinQueue dsq(nSessionDenom,
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.outpoint),
+                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash),
+                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
                             GetAdjustedTime(), true);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
                                      "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
@@ -713,8 +713,8 @@ 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->m_info.outpoint),
+                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()),
-                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash),
+                            WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()),
                             GetAdjustedTime(), false);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
         dsq.Sign();

src/evo/mnauth.cpp

@@ -27,7 +27,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
     uint256 signHash;
     {
         LOCK(::activeMasternodeManager->cs);
-        if (::activeMasternodeManager->m_info.proTxHash.IsNull()) {
+        if (::activeMasternodeManager->GetProTxHash().IsNull()) {
             return;
         }
 
@@ -46,14 +46,15 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
             nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
         }
         const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
-        const CBLSPublicKeyVersionWrapper pubKey(*::activeMasternodeManager->m_info.blsPubKeyOperator, !is_basic_scheme_active);
+        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));
         }
 
-        mnauth.proRegTxHash = ::activeMasternodeManager->m_info.proTxHash;
+        mnauth.proRegTxHash = ::activeMasternodeManager->GetProTxHash();
     } // ::activeMasternodeManager->cs
 
     mnauth.sig = ::activeMasternodeManager->Sign(signHash);
@@ -132,7 +133,7 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterm
     }
 
     const uint256 myProTxHash = fMasternodeMode ?
-                                WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash) :
+                                WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
                                 uint256();
 
     connman.ForEachNode([&](CNode* pnode2) {

src/governance/governance.cpp

@@ -693,11 +693,11 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
 
     {
         LOCK(::activeMasternodeManager->cs);
-        if (mn_payees.front()->proTxHash != ::activeMasternodeManager->m_info.proTxHash) {
+        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(::activeMasternodeManager->m_info.outpoint);
+        gov_sb.SetMasternodeOutpoint(::activeMasternodeManager->GetOutPoint());
     } // ::activeMasternodeManager->cs
     gov_sb.Sign(*::activeMasternodeManager);
 
@@ -720,7 +720,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover
 {
     // only active masternodes can vote on triggers
     if (!fMasternodeMode) return;
-    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash.IsNull())) return;
+    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return;
 
     LOCK2(cs_main, cs);
 
@@ -763,7 +763,7 @@ 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(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.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(*::activeMasternodeManager);
 

src/llmq/dkgsessionhandler.cpp

@@ -192,7 +192,7 @@ bool CDKGSessionHandler::InitNewQuorum(const CBlockIndex* pQuorumBaseBlockIndex)
     }
 
     auto mns = utils::GetAllQuorumMembers(params.type, m_dmnman, pQuorumBaseBlockIndex);
-    if (!curSession->Init(pQuorumBaseBlockIndex, mns, WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash), quorumIndex)) {
+    if (!curSession->Init(pQuorumBaseBlockIndex, mns, WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()), quorumIndex)) {
         LogPrintf("CDKGSessionManager::%s -- height[%d] quorum initialization failed for %s qi[%d] mns[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, curSession->params.name, quorumIndex, mns.size());
         return false;
     }

src/llmq/quorums.cpp

@@ -103,7 +103,7 @@ bool CQuorum::SetVerificationVector(const std::vector<CBLSPublicKey>& quorumVecI
 
 bool CQuorum::SetSecretKeyShare(const CBLSSecretKey& secretKeyShare)
 {
-    if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(WITH_LOCK(::activeMasternodeManager->cs, return GetMemberIndex(::activeMasternodeManager->m_info.proTxHash))))) {
+    if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(WITH_LOCK(::activeMasternodeManager->cs, return GetMemberIndex(::activeMasternodeManager->GetProTxHash()))))) {
         return false;
     }
     LOCK(cs);
@@ -251,7 +251,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)
 
         // First check if we are member of any quorum of this type
         const uint256 proTxHash = fMasternodeMode ?
-                                  WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash) :
+                                  WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
                                   uint256();
 
         bool fWeAreQuorumTypeMember = ranges::any_of(vecQuorums, [&proTxHash](const auto& pQuorum) {
@@ -345,7 +345,7 @@ void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqPar
     }
 
     const uint256 myProTxHash = fMasternodeMode ?
-                                WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash) :
+                                WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
                                 uint256();
 
     bool isISType = llmqParams.type == Params().GetConsensus().llmqTypeDIP0024InstantSend;
@@ -658,7 +658,7 @@ size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, c
         LOCK(::activeMasternodeManager->cs);
         for (const auto i : irange::range(vecProTxHashes.size())) {
             // cppcheck-suppress useStlAlgorithm
-            if (::activeMasternodeManager->m_info.proTxHash == vecProTxHashes[i]) {
+            if (::activeMasternodeManager->GetProTxHash() == vecProTxHashes[i]) {
                 nIndex = i;
                 break;
             }
@@ -916,7 +916,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
 
         vecMemberHashes.reserve(pQuorum->qc->validMembers.size());
         for (auto& member : pQuorum->members) {
-            if (pQuorum->IsValidMember(member->proTxHash) && member->proTxHash != WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash)) {
+            if (pQuorum->IsValidMember(member->proTxHash) && member->proTxHash != WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash())) {
                 vecMemberHashes.push_back(member->proTxHash);
             }
         }
@@ -965,7 +965,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
                 printLog("Connect");
             }
 
-            auto proTxHash = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash);
+            auto proTxHash = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash());
             connman.ForEachNode([&](CNode* pNode) {
                 auto verifiedProRegTxHash = pNode->GetVerifiedProRegTxHash();
                 if (pCurrentMemberHash == nullptr || verifiedProRegTxHash != *pCurrentMemberHash) {

src/llmq/signing.cpp

@@ -896,7 +896,7 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l)
 bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id, const uint256& msgHash, const uint256& quorumHash, bool allowReSign)
 {
     if (!fMasternodeMode) return false;
-    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash.IsNull())) return false;
+    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return false;
 
     const CQuorumCPtr quorum = [&]() {
         if (quorumHash.IsNull()) {
@@ -918,7 +918,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
         return false;
     }
 
-    if (!WITH_LOCK(::activeMasternodeManager->cs, return quorum->IsValidMember(::activeMasternodeManager->m_info.proTxHash))) {
+    if (!WITH_LOCK(::activeMasternodeManager->cs, return quorum->IsValidMember(::activeMasternodeManager->GetProTxHash()))) {
         return false;
     }
 

src/llmq/signing_shares.cpp

@@ -219,7 +219,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const CSporkManager&
 {
     // non-masternodes are not interested in sigshares
     if (!fMasternodeMode) return;
-    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash.IsNull())) return;
+    if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return;
 
     if (sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
         std::vector<CSigShare> receivedSigShares;
@@ -464,7 +464,7 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s
         // quorum is too old
         return;
     }
-    if (!quorum->IsMember(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash))) {
+    if (!quorum->IsMember(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()))) {
         // we're not a member so we can't verify it (we actually shouldn't have received it)
         return;
     }
@@ -513,7 +513,7 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(const CQuorumManager& quorum_m
         // quorum is too old
         return false;
     }
-    if (!session.quorum->IsMember(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash))) {
+    if (!session.quorum->IsMember(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()))) {
         // we're not a member so we can't verify it (we actually shouldn't have received it)
         return false;
     }
@@ -697,7 +697,7 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CConnma
 
     // prepare node set for direct-push in case this is our sig share
     std::set<NodeId> quorumNodes;
-    if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman) && sigShare.getQuorumMember() == quorum->GetMemberIndex(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash))) {
+    if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman) && sigShare.getQuorumMember() == quorum->GetMemberIndex(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()))) {
         quorumNodes = connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash());
     }
 
@@ -1488,7 +1488,7 @@ void CSigSharesManager::SignPendingSigShares()
 std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const
 {
     cxxtimer::Timer t(true);
-    auto activeMasterNodeProTxHash = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash);
+    auto activeMasterNodeProTxHash = WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash());
 
     if (!quorum->IsValidMember(activeMasterNodeProTxHash)) {
         return std::nullopt;

src/masternode/node.cpp

@@ -11,6 +11,7 @@
 #include <net.h>
 #include <netbase.h>
 #include <protocol.h>
+#include <util/check.h>
 #include <validation.h>
 #include <warnings.h>
 
@@ -275,3 +276,10 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec
     AssertLockNotHeld(cs);
     return WITH_LOCK(cs, return Assert(m_info.blsKeyOperator)->Sign(hash, is_legacy));
 }
+
+// We need to pass a copy as opposed to a const ref because CBLSPublicKeyVersionWrapper
+// does not accept a const ref in its construction args
+[[nodiscard]] CBLSPublicKey CActiveMasternodeManager::GetPubKey() const
+{
+    return *Assert(m_info.blsPubKeyOperator);
+}

src/masternode/node.h

@@ -41,11 +41,12 @@ public:
     };
 
     mutable RecursiveMutex cs;
-    CActiveMasternodeInfo m_info GUARDED_BY(cs);
 
 private:
     masternode_state_t state{MASTERNODE_WAITING_FOR_PROTX};
+    CActiveMasternodeInfo m_info GUARDED_BY(cs);
     std::string strError;
+
     CConnman& connman;
     const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
 
@@ -70,6 +71,13 @@ public:
     [[nodiscard]] CBLSSignature Sign(const uint256& hash) const LOCKS_EXCLUDED(cs);
     [[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool is_legacy) const LOCKS_EXCLUDED(cs);
 
+    /* TODO: Reconsider external locking */
+    [[nodiscard]] const COutPoint& GetOutPoint() const EXCLUSIVE_LOCKS_REQUIRED(cs) { return m_info.outpoint; }
+    [[nodiscard]] const uint256& GetProTxHash() const EXCLUSIVE_LOCKS_REQUIRED(cs) { return m_info.proTxHash; }
+    [[nodiscard]] const CService& GetService() const EXCLUSIVE_LOCKS_REQUIRED(cs) { return m_info.service; }
+    [[nodiscard]] CBLSPublicKey GetPubKey() const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    [[nodiscard]] const bool IsLegacy() const EXCLUSIVE_LOCKS_REQUIRED(cs) { return m_info.legacy; }
+
 private:
     bool GetLocalAddress(CService& addrRet);
 };

src/rpc/governance.cpp

@@ -319,11 +319,11 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
     bool fMnFound{false};
     if (fMasternodeMode) {
         LOCK(::activeMasternodeManager->cs);
-        fMnFound = mnList.HasValidMNByCollateral(::activeMasternodeManager->m_info.outpoint);
+        fMnFound = mnList.HasValidMNByCollateral(::activeMasternodeManager->GetOutPoint());
 
         LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n",
-                (::activeMasternodeManager->m_info.blsPubKeyOperator ? ::activeMasternodeManager->m_info.blsPubKeyOperator->ToString(::activeMasternodeManager->m_info.legacy) : "N/A"),
+                (::activeMasternodeManager->GetPubKey().IsValid() ? ::activeMasternodeManager->GetPubKey().ToString(::activeMasternodeManager->IsLegacy()) : "N/A"),
-                ::activeMasternodeManager->m_info.outpoint.ToStringShort(), request.params.size(), fMnFound);
+                ::activeMasternodeManager->GetOutPoint().ToStringShort(), request.params.size(), fMnFound);
     } else {
         LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = N/A, outpoint = N/A, params.size() = %lld, fMnFound = %d\n",
                  request.params.size(), fMnFound);

src/rpc/masternode.cpp

@@ -269,9 +269,9 @@ static UniValue masternode_status(const JSONRPCRequest& request)
         LOCK(::activeMasternodeManager->cs);
 
         // keep compatibility with legacy status for now (might get deprecated/removed later)
-        mnObj.pushKV("outpoint", ::activeMasternodeManager->m_info.outpoint.ToStringShort());
+        mnObj.pushKV("outpoint", ::activeMasternodeManager->GetOutPoint().ToStringShort());
-        mnObj.pushKV("service", ::activeMasternodeManager->m_info.service.ToString());
+        mnObj.pushKV("service", ::activeMasternodeManager->GetService().ToString());
-        dmn = node.dmnman->GetListAtChainTip().GetMN(::activeMasternodeManager->m_info.proTxHash);
+        dmn = node.dmnman->GetListAtChainTip().GetMN(::activeMasternodeManager->GetProTxHash());
     }
     if (dmn) {
         mnObj.pushKV("proTxHash", dmn->proTxHash.ToString());

src/rpc/quorums.cpp

@@ -296,7 +296,7 @@ static UniValue quorum_dkgstatus(const JSONRPCRequest& request, CDeterministicMN
     int tipHeight = pindexTip->nHeight;
 
     const uint256 proTxHash = fMasternodeMode ?
-                              WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->m_info.proTxHash) :
+                              WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
                               uint256();
 
     UniValue minableCommitments(UniValue::VARR);