Skip to content

Commit 0692826

Browse files
PastaPastaPastaPastaPastaPasta
committed
refactor: make CActiveMasternodeManager::cs SharedMutex and private
1 parent 663774c commit 0692826

File tree

12 files changed

+82
-92
lines changed

12 files changed

+82
-92
lines changed

‎src/coinjoin/server.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
5858
LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */
5959

6060
auto mnList = m_dmnman.GetListAtChainTip();
61-
auto dmn = WITH_LOCK(m_mn_activeman->cs, return mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint()));
61+
auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint());
6262
if (!dmn) {
6363
PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
6464
return;
@@ -69,7 +69,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
6969
TRY_LOCK(cs_vecqueue, lockRecv);
7070
if (!lockRecv) return;
7171

72-
auto mnOutpoint = WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint());
72+
auto mnOutpoint = m_mn_activeman->GetOutPoint();
7373

7474
if (ranges::any_of(vecCoinJoinQueue,
7575
[&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
@@ -335,8 +335,8 @@ void CCoinJoinServer::CommitFinalTransaction()
335335
// create and sign masternode dstx transaction
336336
if (!m_dstxman.GetDSTX(hashTx)) {
337337
CCoinJoinBroadcastTx dstxNew(finalTransaction,
338-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
339-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
338+
m_mn_activeman->GetOutPoint(),
339+
m_mn_activeman->GetProTxHash(),
340340
GetAdjustedTime());
341341
dstxNew.Sign(*m_mn_activeman);
342342
m_dstxman.AddDSTX(dstxNew);
@@ -505,8 +505,8 @@ void CCoinJoinServer::CheckForCompleteQueue()
505505
SetState(POOL_STATE_ACCEPTING_ENTRIES);
506506

507507
CCoinJoinQueue dsq(nSessionDenom,
508-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
509-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
508+
m_mn_activeman->GetOutPoint(),
509+
m_mn_activeman->GetProTxHash(),
510510
GetAdjustedTime(), true);
511511
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
512512
"with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
@@ -721,8 +721,8 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
721721
if (!fUnitTest) {
722722
//broadcast that I'm accepting entries, only if it's the first entry through
723723
CCoinJoinQueue dsq(nSessionDenom,
724-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
725-
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
724+
m_mn_activeman->GetOutPoint(),
725+
m_mn_activeman->GetProTxHash(),
726726
GetAdjustedTime(), false);
727727
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
728728
dsq.Sign(*m_mn_activeman);

‎src/evo/mnauth.cpp‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,38 +24,36 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
2424
if (!fMasternodeMode) return;
2525

2626
CMNAuth mnauth;
27-
uint256 signHash;
28-
{
29-
LOCK(::activeMasternodeManager->cs);
30-
if (::activeMasternodeManager->GetProTxHash().IsNull()) {
31-
return;
32-
}
27+
if (::activeMasternodeManager->GetProTxHash().IsNull()) {
28+
return;
29+
}
3330

34-
const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
35-
if (receivedMNAuthChallenge.IsNull()) {
36-
return;
37-
}
38-
// We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
39-
// we protect ourselves against MITM in this form:
40-
// node1 <- Eve -> node2
41-
// It does not protect against:
42-
// node1 -> Eve -> node2
43-
// This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
44-
int nOurNodeVersion{PROTOCOL_VERSION};
45-
if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
46-
nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
47-
}
48-
const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
49-
auto pk = ::activeMasternodeManager->GetPubKey();
50-
const CBLSPublicKeyVersionWrapper pubKey(pk, !is_basic_scheme_active);
31+
const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
32+
if (receivedMNAuthChallenge.IsNull()) {
33+
return;
34+
}
35+
// We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
36+
// we protect ourselves against MITM in this form:
37+
// node1 <- Eve -> node2
38+
// It does not protect against:
39+
// node1 -> Eve -> node2
40+
// This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
41+
int nOurNodeVersion{PROTOCOL_VERSION};
42+
if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
43+
nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
44+
}
45+
const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
46+
auto pk = ::activeMasternodeManager->GetPubKey();
47+
const CBLSPublicKeyVersionWrapper pubKey(pk, !is_basic_scheme_active);
48+
uint256 signHash = [&]() {
5149
if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
52-
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
50+
return ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
5351
} else {
54-
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
52+
return ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
5553
}
54+
}();
5655

57-
mnauth.proRegTxHash = ::activeMasternodeManager->GetProTxHash();
58-
} // ::activeMasternodeManager->cs
56+
mnauth.proRegTxHash = ::activeMasternodeManager->GetProTxHash();
5957

6058
mnauth.sig = ::activeMasternodeManager->Sign(signHash);
6159

@@ -133,7 +131,7 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterm
133131
}
134132

135133
const uint256 myProTxHash = fMasternodeMode ?
136-
WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
134+
::activeMasternodeManager->GetProTxHash() :
137135
uint256();
138136

139137
connman.ForEachNode([&](CNode* pnode2) {

‎src/governance/governance.cpp‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -691,14 +691,11 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
691691
return std::nullopt;
692692
}
693693

694-
{
695-
LOCK(::activeMasternodeManager->cs);
696-
if (mn_payees.front()->proTxHash != ::activeMasternodeManager->GetProTxHash()) {
697-
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__);
698-
return std::nullopt;
699-
}
700-
gov_sb.SetMasternodeOutpoint(::activeMasternodeManager->GetOutPoint());
701-
} // ::activeMasternodeManager->cs
694+
if (mn_payees.front()->proTxHash != ::activeMasternodeManager->GetProTxHash()) {
695+
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__);
696+
return std::nullopt;
697+
}
698+
gov_sb.SetMasternodeOutpoint(::activeMasternodeManager->GetOutPoint());
702699
gov_sb.Sign(*::activeMasternodeManager);
703700

704701
if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), strError, true)) {
@@ -720,7 +717,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover
720717
{
721718
// only active masternodes can vote on triggers
722719
if (!fMasternodeMode) return;
723-
if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return;
720+
if (::activeMasternodeManager->GetProTxHash().IsNull()) return;
724721

725722
LOCK2(cs_main, cs);
726723

@@ -763,7 +760,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover
763760

764761
bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman)
765762
{
766-
CGovernanceVote vote(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()), nHash, VOTE_SIGNAL_FUNDING, outcome);
763+
CGovernanceVote vote(::activeMasternodeManager->GetOutPoint(), nHash, VOTE_SIGNAL_FUNDING, outcome);
767764
vote.SetTime(GetAdjustedTime());
768765
vote.Sign(*::activeMasternodeManager);
769766

‎src/llmq/dkgsessionhandler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ bool CDKGSessionHandler::InitNewQuorum(const CBlockIndex* pQuorumBaseBlockIndex)
194194
}
195195

196196
auto mns = utils::GetAllQuorumMembers(params.type, m_dmnman, pQuorumBaseBlockIndex);
197-
if (!curSession->Init(pQuorumBaseBlockIndex, mns, WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()), quorumIndex)) {
197+
if (!curSession->Init(pQuorumBaseBlockIndex, mns, m_mn_activeman->GetProTxHash(), quorumIndex)) {
198198
LogPrintf("CDKGSessionManager::%s -- height[%d] quorum initialization failed for %s qi[%d] mns[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, curSession->params.name, quorumIndex, mns.size());
199199
return false;
200200
}

‎src/llmq/quorums.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ bool CQuorum::SetVerificationVector(const std::vector<CBLSPublicKey>& quorumVecI
103103

104104
bool CQuorum::SetSecretKeyShare(const CBLSSecretKey& secretKeyShare, const CActiveMasternodeManager& mn_activeman)
105105
{
106-
if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(WITH_LOCK(mn_activeman.cs, return GetMemberIndex(mn_activeman.GetProTxHash()))))) {
106+
if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(mn_activeman.GetProTxHash())))) {
107107
return false;
108108
}
109109
LOCK(cs);
@@ -256,7 +256,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)
256256
const uint256 proTxHash = [this]() {
257257
if (!fMasternodeMode) return uint256();
258258
assert(m_mn_activeman);
259-
return WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash());
259+
return m_mn_activeman->GetProTxHash();
260260
}();
261261

262262
bool fWeAreQuorumTypeMember = ranges::any_of(vecQuorums, [&proTxHash](const auto& pQuorum) {
@@ -352,7 +352,7 @@ void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqPar
352352
const uint256 myProTxHash = [this]() {
353353
if (!fMasternodeMode) return uint256();
354354
assert(m_mn_activeman);
355-
return WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash());
355+
return m_mn_activeman->GetProTxHash();
356356
}();
357357

358358
bool isISType = llmqParams.type == Params().GetConsensus().llmqTypeDIP0024InstantSend;
@@ -664,10 +664,10 @@ size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, c
664664
std::sort(vecProTxHashes.begin(), vecProTxHashes.end());
665665
size_t nIndex{0};
666666
{
667-
LOCK(m_mn_activeman->cs);
667+
auto my_protx_hash = m_mn_activeman->GetProTxHash();
668668
for (const auto i : irange::range(vecProTxHashes.size())) {
669669
// cppcheck-suppress useStlAlgorithm
670-
if (m_mn_activeman->GetProTxHash() == vecProTxHashes[i]) {
670+
if (my_protx_hash == vecProTxHashes[i]) {
671671
nIndex = i;
672672
break;
673673
}
@@ -928,7 +928,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
928928

929929
vecMemberHashes.reserve(pQuorum->qc->validMembers.size());
930930
for (auto& member : pQuorum->members) {
931-
if (pQuorum->IsValidMember(member->proTxHash) && member->proTxHash != WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash())) {
931+
if (pQuorum->IsValidMember(member->proTxHash) && member->proTxHash != m_mn_activeman->GetProTxHash()) {
932932
vecMemberHashes.push_back(member->proTxHash);
933933
}
934934
}
@@ -977,7 +977,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
977977
printLog("Connect");
978978
}
979979

980-
auto proTxHash = WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash());
980+
auto proTxHash = m_mn_activeman->GetProTxHash();
981981
connman.ForEachNode([&](CNode* pNode) {
982982
auto verifiedProRegTxHash = pNode->GetVerifiedProRegTxHash();
983983
if (pCurrentMemberHash == nullptr || verifiedProRegTxHash != *pCurrentMemberHash) {

‎src/llmq/signing.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
898898
if (!fMasternodeMode) return false;
899899

900900
assert(m_mn_activeman);
901-
if (WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash().IsNull())) return false;
901+
if (m_mn_activeman->GetProTxHash().IsNull()) return false;
902902

903903
const CQuorumCPtr quorum = [&]() {
904904
if (quorumHash.IsNull()) {
@@ -920,7 +920,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
920920
return false;
921921
}
922922

923-
if (!WITH_LOCK(m_mn_activeman->cs, return quorum->IsValidMember(m_mn_activeman->GetProTxHash()))) {
923+
if (!quorum->IsValidMember(m_mn_activeman->GetProTxHash())) {
924924
return false;
925925
}
926926

‎src/llmq/signing_shares.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const CSporkManager&
221221
if (!fMasternodeMode) return;
222222

223223
assert(m_mn_activeman);
224-
if (WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash().IsNull())) return;
224+
if (m_mn_activeman->GetProTxHash().IsNull()) return;
225225

226226
if (sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
227227
std::vector<CSigShare> receivedSigShares;
@@ -468,7 +468,7 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s
468468
// quorum is too old
469469
return;
470470
}
471-
if (!quorum->IsMember(WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()))) {
471+
if (!quorum->IsMember(m_mn_activeman->GetProTxHash())) {
472472
// we're not a member so we can't verify it (we actually shouldn't have received it)
473473
return;
474474
}
@@ -518,7 +518,7 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(const CActiveMasternodeManager
518518
// quorum is too old
519519
return false;
520520
}
521-
if (!session.quorum->IsMember(WITH_LOCK(mn_activeman.cs, return mn_activeman.GetProTxHash()))) {
521+
if (!session.quorum->IsMember(mn_activeman.GetProTxHash())) {
522522
// we're not a member so we can't verify it (we actually shouldn't have received it)
523523
return false;
524524
}
@@ -703,7 +703,7 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CConnma
703703

704704
// prepare node set for direct-push in case this is our sig share
705705
std::set<NodeId> quorumNodes;
706-
if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman) && sigShare.getQuorumMember() == quorum->GetMemberIndex(WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()))) {
706+
if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman) && sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman->GetProTxHash())) {
707707
quorumNodes = connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash());
708708
}
709709

@@ -1496,7 +1496,7 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
14961496
assert(m_mn_activeman);
14971497

14981498
cxxtimer::Timer t(true);
1499-
auto activeMasterNodeProTxHash = WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash());
1499+
auto activeMasterNodeProTxHash = m_mn_activeman->GetProTxHash();
15001500

15011501
if (!quorum->IsValidMember(activeMasterNodeProTxHash)) {
15021502
return std::nullopt;

0 commit comments

Comments
 (0)