diff --git a/ci/matrix.sh b/ci/matrix.sh index be6b92516f40..db43046a25e0 100755 --- a/ci/matrix.sh +++ b/ci/matrix.sh @@ -40,7 +40,7 @@ if [ "$BUILD_TARGET" = "arm-linux" ]; then export CHECK_DOC=1 # -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1" # This could be removed once the ABI change warning does not show up by default - export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CXXFLAGS=-Wno-psabi" + export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports --enable-werror CXXFLAGS=-Wno-psabi" export RUN_UNITTESTS=false export RUN_INTEGRATIONTESTS=false elif [ "$BUILD_TARGET" = "win32" ]; then @@ -67,7 +67,7 @@ elif [ "$BUILD_TARGET" = "linux64" ]; then elif [ "$BUILD_TARGET" = "linux64_cxx17" ]; then export HOST=x86_64-unknown-linux-gnu export DEP_OPTS="NO_UPNP=1 DEBUG=1" - export BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports --enable-crash-hooks --enable-c++17 --with-sanitizers=undefined" + export BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports --enable-crash-hooks --enable-c++17 --enable-werror --with-sanitizers=undefined" export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG" export PYZMQ=true export RUN_INTEGRATIONTESTS=false diff --git a/src/bench/bench_dash.cpp b/src/bench/bench_dash.cpp index bcaf85d5d1ac..8fe2ddb0e2ac 100644 --- a/src/bench/bench_dash.cpp +++ b/src/bench/bench_dash.cpp @@ -20,7 +20,6 @@ const std::function G_TRANSLATION_FUN = nullptr; -static const int64_t DEFAULT_BENCH_EVALUATIONS = 5; static const char* DEFAULT_BENCH_FILTER = ".*"; void InitBLSTests(); diff --git a/src/bench/bls_dkg.cpp b/src/bench/bls_dkg.cpp index a10145297970..07860b9516db 100644 --- a/src/bench/bls_dkg.cpp +++ b/src/bench/bls_dkg.cpp @@ -86,8 +86,6 @@ class DKG ReceiveVvecs(); size_t memberIdx = 0; bench.minEpochIterations(epoch_iters).run([&] { - auto& m = members[memberIdx]; - ReceiveShares(memberIdx); std::set invalidIndexes; diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 262cab96ebdb..97f799fa9ddf 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -11,7 +11,6 @@ #include -static const int MIN_CORES = 2; static const size_t BATCHES = 101; static const size_t BATCH_SIZE = 30; static const int PREVECTOR_SIZE = 28; diff --git a/src/coinjoin/coinjoin-server.cpp b/src/coinjoin/coinjoin-server.cpp index a1f03a56ae0f..ad66126db40c 100644 --- a/src/coinjoin/coinjoin-server.cpp +++ b/src/coinjoin/coinjoin-server.cpp @@ -56,7 +56,7 @@ void CCoinJoinServer::ProcessMessage(CNode* pfrom, const std::string& strCommand LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CCoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */ auto mnList = deterministicMNManager->GetListAtChainTip(); - auto dmn = mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint); + auto dmn = WITH_LOCK(activeMasternodeInfoCs, return mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint)); if (!dmn) { PushStatus(pfrom, STATUS_REJECTED, ERR_MN_LIST, connman); return; @@ -68,7 +68,7 @@ void CCoinJoinServer::ProcessMessage(CNode* pfrom, const std::string& strCommand if (!lockRecv) return; for (const auto& q : vecCoinJoinQueue) { - if (q.masternodeOutpoint == activeMasternodeInfo.outpoint) { + if (WITH_LOCK(activeMasternodeInfoCs, return q.masternodeOutpoint == activeMasternodeInfo.outpoint)) { // refuse to create another queue this often LogPrint(BCLog::COINJOIN, "DSACCEPT -- last dsq is still in queue, refuse to mix\n"); PushStatus(pfrom, STATUS_REJECTED, ERR_RECENT, connman); @@ -334,7 +334,7 @@ void CCoinJoinServer::CommitFinalTransaction(CConnman& connman) // create and sign masternode dstx transaction if (!CCoinJoin::GetDSTX(hashTx)) { - CCoinJoinBroadcastTx dstxNew(finalTransaction, activeMasternodeInfo.outpoint, GetAdjustedTime()); + CCoinJoinBroadcastTx dstxNew(finalTransaction, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), GetAdjustedTime()); dstxNew.Sign(); CCoinJoin::AddDSTX(dstxNew); } @@ -501,7 +501,7 @@ void CCoinJoinServer::CheckForCompleteQueue(CConnman& connman) if (nState == POOL_STATE_QUEUE && IsSessionReady()) { SetState(POOL_STATE_ACCEPTING_ENTRIES); - CCoinJoinQueue dsq(nSessionDenom, activeMasternodeInfo.outpoint, GetAdjustedTime(), true); + CCoinJoinQueue dsq(nSessionDenom, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), 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(); @@ -708,7 +708,7 @@ 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, activeMasternodeInfo.outpoint, GetAdjustedTime(), false); + CCoinJoinQueue dsq(nSessionDenom, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), GetAdjustedTime(), false); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString()); dsq.Sign(); dsq.Relay(connman); diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 6c4f06f201d5..981661172251 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -50,7 +50,7 @@ bool CCoinJoinQueue::Sign() uint256 hash = GetSignatureHash(); - CBLSSignature sig = activeMasternodeInfo.blsKeyOperator->Sign(hash); + CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash)); if (!sig.IsValid()) { return false; } @@ -96,7 +96,7 @@ bool CCoinJoinBroadcastTx::Sign() uint256 hash = GetSignatureHash(); - CBLSSignature sig = activeMasternodeInfo.blsKeyOperator->Sign(hash); + CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash)); if (!sig.IsValid()) { return false; } diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp index deb8a60a79fd..d3f9689c6326 100644 --- a/src/evo/mnauth.cpp +++ b/src/evo/mnauth.cpp @@ -18,31 +18,30 @@ void CMNAuth::PushMNAUTH(CNode* pnode, CConnman& connman) { + LOCK(activeMasternodeInfoCs); if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) { return; } uint256 signHash; - { - LOCK(pnode->cs_mnauth); - if (pnode->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); - } - if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) { - signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, pnode->receivedMNAuthChallenge, pnode->fInbound)); - } else { - signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, pnode->receivedMNAuthChallenge, pnode->fInbound, nOurNodeVersion)); - } + auto receivedMNAuthChallenge = pnode->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); + } + if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) { + signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, receivedMNAuthChallenge, pnode->fInbound)); + } else { + signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, receivedMNAuthChallenge, pnode->fInbound, nOurNodeVersion)); } CMNAuth mnauth; @@ -66,11 +65,7 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS vRecv >> mnauth; // only one MNAUTH allowed - bool fAlreadyHaveMNAUTH = false; - { - LOCK(pnode->cs_mnauth); - fAlreadyHaveMNAUTH = !pnode->verifiedProRegTxHash.IsNull(); - } + bool fAlreadyHaveMNAUTH = !pnode->GetVerifiedProRegTxHash().IsNull(); if (fAlreadyHaveMNAUTH) { LOCK(cs_main); Misbehaving(pnode->GetId(), 100, "duplicate mnauth"); @@ -108,20 +103,17 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS } uint256 signHash; - { - LOCK(pnode->cs_mnauth); - int nOurNodeVersion{PROTOCOL_VERSION}; - if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) { - nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION); - } - // See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection) - if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) { - signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->sentMNAuthChallenge, !pnode->fInbound)); - } else { - signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->sentMNAuthChallenge, !pnode->fInbound, pnode->nVersion.load())); - } - LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, pnode->nVersion, pnode->GetId()); + int nOurNodeVersion{PROTOCOL_VERSION}; + if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) { + nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION); } + // See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection) + if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) { + signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->GetSentMNAuthChallenge(), !pnode->fInbound)); + } else { + signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->GetSentMNAuthChallenge(), !pnode->fInbound, pnode->nVersion.load())); + } + LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, pnode->nVersion, pnode->GetId()); if (!mnauth.sig.VerifyInsecure(dmn->pdmnState->pubKeyOperator.Get(), signHash)) { LOCK(cs_main); @@ -147,12 +139,12 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS return; } - if (pnode2->verifiedProRegTxHash == mnauth.proRegTxHash) { + if (pnode2->GetVerifiedProRegTxHash() == mnauth.proRegTxHash) { if (fMasternodeMode) { - auto deterministicOutbound = llmq::CLLMQUtils::DeterministicOutboundConnection(activeMasternodeInfo.proTxHash, mnauth.proRegTxHash); + auto deterministicOutbound = WITH_LOCK(activeMasternodeInfoCs, return llmq::CLLMQUtils::DeterministicOutboundConnection(activeMasternodeInfo.proTxHash, mnauth.proRegTxHash)); LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- Masternode %s has already verified as peer %d, deterministicOutbound=%s. peer=%d\n", mnauth.proRegTxHash.ToString(), pnode2->GetId(), deterministicOutbound.ToString(), pnode->GetId()); - if (deterministicOutbound == activeMasternodeInfo.proTxHash) { + if (WITH_LOCK(activeMasternodeInfoCs, return deterministicOutbound == activeMasternodeInfo.proTxHash)) { if (pnode2->fInbound) { LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- dropping old inbound, peer=%d\n", pnode2->GetId()); pnode2->fDisconnect = true; @@ -181,13 +173,10 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS return; } - { - LOCK(pnode->cs_mnauth); - pnode->verifiedProRegTxHash = mnauth.proRegTxHash; - pnode->verifiedPubKeyHash = dmn->pdmnState->pubKeyOperator.GetHash(); - } + pnode->SetVerifiedProRegTxHash(mnauth.proRegTxHash); + pnode->SetVerifiedPubKeyHash(dmn->pdmnState->pubKeyOperator.GetHash()); - if (!pnode->m_masternode_iqr_connection && connman.IsMasternodeQuorumRelayMember(pnode->verifiedProRegTxHash)) { + if (!pnode->m_masternode_iqr_connection && connman.IsMasternodeQuorumRelayMember(pnode->GetVerifiedProRegTxHash())) { // Tell our peer that we're interested in plain LLMQ recovered signatures. // Otherwise the peer would only announce/send messages resulting from QRECSIG, // e.g. InstantSend locks or ChainLocks. SPV and regular full nodes should not send @@ -209,11 +198,11 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& } g_connman->ForEachNode([&](CNode* pnode) { - LOCK(pnode->cs_mnauth); - if (pnode->verifiedProRegTxHash.IsNull()) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (verifiedProRegTxHash.IsNull()) { return; } - auto verifiedDmn = oldMNList.GetMN(pnode->verifiedProRegTxHash); + auto verifiedDmn = oldMNList.GetMN(verifiedProRegTxHash); if (!verifiedDmn) { return; } @@ -223,7 +212,7 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& } else { auto it = diff.updatedMNs.find(verifiedDmn->GetInternalId()); if (it != diff.updatedMNs.end()) { - if ((it->second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && it->second.state.pubKeyOperator.GetHash() != pnode->verifiedPubKeyHash) { + if ((it->second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && it->second.state.pubKeyOperator.GetHash() != pnode->GetVerifiedPubKeyHash()) { doRemove = true; } } @@ -231,7 +220,7 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& if (doRemove) { LogPrint(BCLog::NET_NETCONN, "CMNAuth::NotifyMasternodeListChanged -- Disconnecting MN %s due to key changed/removed, peer=%d\n", - pnode->verifiedProRegTxHash.ToString(), pnode->GetId()); + pnode->GetVerifiedProRegTxHash().ToString(), pnode->GetId()); pnode->fDisconnect = true; } }); diff --git a/src/hdchain.h b/src/hdchain.h index d1ee44515180..bb027b025779 100644 --- a/src/hdchain.h +++ b/src/hdchain.h @@ -74,6 +74,7 @@ class CHDChain // by swapping the members of two classes, // the two classes are effectively swapped + LOCK2(first.cs, second.cs); swap(first.nVersion, second.nVersion); swap(first.id, second.id); swap(first.fCrypted, second.fCrypted); diff --git a/src/init.cpp b/src/init.cpp index 8d193e92a14e..0acbd5f11c8a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -363,9 +363,12 @@ void PrepareShutdown() UnregisterValidationInterface(activeMasternodeManager); } - // make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool) - activeMasternodeInfo.blsKeyOperator.reset(); - activeMasternodeInfo.blsPubKeyOperator.reset(); + { + LOCK(activeMasternodeInfoCs); + // make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool) + activeMasternodeInfo.blsKeyOperator.reset(); + activeMasternodeInfo.blsPubKeyOperator.reset(); + } #ifndef WIN32 try { @@ -2061,7 +2064,6 @@ bool AppInitMain() LogPrintf("* Using %.1fMiB for in-memory UTXO set (plus up to %.1fMiB of unused mempool space)\n", nCoinCacheUsage * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024)); bool fLoaded = false; - int64_t nStart = GetTimeMillis(); while (!fLoaded && !ShutdownRequested()) { bool fReset = fReindex; @@ -2324,8 +2326,12 @@ bool AppInitMain() return InitError(_("Invalid masternodeblsprivkey. Please see documentation.")); } fMasternodeMode = true; - activeMasternodeInfo.blsKeyOperator = std::make_unique(keyOperator); - activeMasternodeInfo.blsPubKeyOperator = std::make_unique(activeMasternodeInfo.blsKeyOperator->GetPublicKey()); + { + LOCK(activeMasternodeInfoCs); + activeMasternodeInfo.blsKeyOperator = std::make_unique(keyOperator); + activeMasternodeInfo.blsPubKeyOperator = std::make_unique( + activeMasternodeInfo.blsKeyOperator->GetPublicKey()); + } LogPrintf("MASTERNODE:\n"); LogPrintf(" blsPubKeyOperator: %s\n", keyOperator.GetPublicKey().ToString()); } @@ -2336,11 +2342,14 @@ bool AppInitMain() RegisterValidationInterface(activeMasternodeManager); } - if (activeMasternodeInfo.blsKeyOperator == nullptr) { - activeMasternodeInfo.blsKeyOperator = std::make_unique(); - } - if (activeMasternodeInfo.blsPubKeyOperator == nullptr) { - activeMasternodeInfo.blsPubKeyOperator = std::make_unique(); + { + LOCK(activeMasternodeInfoCs); + if (activeMasternodeInfo.blsKeyOperator == nullptr) { + activeMasternodeInfo.blsKeyOperator = std::make_unique(); + } + if (activeMasternodeInfo.blsPubKeyOperator == nullptr) { + activeMasternodeInfo.blsPubKeyOperator = std::make_unique(); + } } // ********************************************************* Step 10b: setup CoinJoin diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index dd3a3fac7259..2555efed7d73 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -61,7 +61,10 @@ void CQuorum::Init(const CFinalCommitmentPtr& _qc, const CBlockIndex* _pindexQuo bool CQuorum::SetVerificationVector(const BLSVerificationVector& quorumVecIn) { - if (::SerializeHash(quorumVecIn) != qc->quorumVvecHash) { + const auto quorumVecInSerialized = ::SerializeHash(quorumVecIn); + + LOCK(cs); + if (quorumVecInSerialized != qc->quorumVvecHash) { return false; } quorumVvec = std::make_shared(quorumVecIn); @@ -70,9 +73,10 @@ bool CQuorum::SetVerificationVector(const BLSVerificationVector& quorumVecIn) bool CQuorum::SetSecretKeyShare(const CBLSSecretKey& secretKeyShare) { - if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(activeMasternodeInfo.proTxHash)))) { + if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(WITH_LOCK(activeMasternodeInfoCs, return GetMemberIndex(activeMasternodeInfo.proTxHash))))) { return false; } + LOCK(cs); skShare = secretKeyShare; return true; } @@ -99,15 +103,22 @@ bool CQuorum::IsValidMember(const uint256& proTxHash) const CBLSPublicKey CQuorum::GetPubKeyShare(size_t memberIdx) const { - if (quorumVvec == nullptr || memberIdx >= members.size() || !qc->validMembers[memberIdx]) { + LOCK(cs); + if (!HasVerificationVector() || memberIdx >= members.size() || !qc->validMembers[memberIdx]) { return CBLSPublicKey(); } auto& m = members[memberIdx]; return blsCache.BuildPubKeyShare(m->proTxHash, quorumVvec, CBLSId(m->proTxHash)); } -const CBLSSecretKey& CQuorum::GetSkShare() const +bool CQuorum::HasVerificationVector() const { + LOCK(cs); + return quorumVvec != nullptr; +} + +CBLSSecretKey CQuorum::GetSkShare() const { + LOCK(cs); return skShare; } @@ -125,7 +136,8 @@ void CQuorum::WriteContributions(CEvoDB& evoDb) const { uint256 dbKey = MakeQuorumKey(*this); - if (quorumVvec != nullptr) { + LOCK(cs); + if (HasVerificationVector()) { evoDb.GetRawDB().Write(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), *quorumVvec); } if (skShare.IsValid()) { @@ -139,14 +151,14 @@ bool CQuorum::ReadContributions(CEvoDB& evoDb) BLSVerificationVector qv; if (evoDb.Read(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), qv)) { - quorumVvec = std::make_shared(std::move(qv)); + WITH_LOCK(cs, quorumVvec = std::make_shared(std::move(qv))); } else { return false; } // We ignore the return value here as it is ok if this fails. If it fails, it usually means that we are not a // member of the quorum but observed the whole DKG process to have the quorum verification vector. - evoDb.Read(std::make_pair(DB_QUORUM_SK_SHARE, dbKey), skShare); + WITH_LOCK(cs, evoDb.Read(std::make_pair(DB_QUORUM_SK_SHARE, dbKey), skShare)); return true; } @@ -197,8 +209,10 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex) // First check if we are member of any quorum of this type bool fWeAreQuorumTypeMember{false}; + + auto proTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash); for (const auto& pQuorum : vecQuorums) { - if (pQuorum->IsValidMember(activeMasternodeInfo.proTxHash)) { + if (pQuorum->IsValidMember(proTxHash)) { fWeAreQuorumTypeMember = true; break; } @@ -211,16 +225,16 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex) } uint16_t nDataMask{0}; - const bool fWeAreQuorumMember = pQuorum->IsValidMember(activeMasternodeInfo.proTxHash); + const bool fWeAreQuorumMember = pQuorum->IsValidMember(proTxHash); const bool fSyncForTypeEnabled = mapQuorumVvecSync.count(pQuorum->qc->llmqType) > 0; const QvvecSyncMode syncMode = fSyncForTypeEnabled ? mapQuorumVvecSync.at(pQuorum->qc->llmqType) : QvvecSyncMode::Invalid; const bool fSyncCurrent = syncMode == QvvecSyncMode::Always || (syncMode == QvvecSyncMode::OnlyIfTypeMember && fWeAreQuorumTypeMember); - if ((fWeAreQuorumMember || (fSyncForTypeEnabled && fSyncCurrent)) && pQuorum->quorumVvec == nullptr) { + if ((fWeAreQuorumMember || (fSyncForTypeEnabled && fSyncCurrent)) && !pQuorum->HasVerificationVector()) { nDataMask |= llmq::CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR; } - if (fWeAreQuorumMember && !pQuorum->skShare.IsValid()) { + if (fWeAreQuorumMember && !pQuorum->GetSkShare().IsValid()) { nDataMask |= llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS; } @@ -266,7 +280,6 @@ void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const { const auto& llmq_params = GetLLMQParams(llmqType); - const auto& myProTxHash = activeMasternodeInfo.proTxHash; auto lastQuorums = ScanQuorums(llmqType, pindexNew, (size_t)llmq_params.keepOldConnections); auto connmanQuorumsToDelete = g_connman->GetMasternodeQuorums(llmqType); @@ -277,7 +290,7 @@ void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const connmanQuorumsToDelete.erase(curDkgBlock); for (const auto& quorum : lastQuorums) { - if (CLLMQUtils::EnsureQuorumConnections(llmqType, quorum->pindexQuorum, myProTxHash)) { + if (CLLMQUtils::EnsureQuorumConnections(llmqType, quorum->pindexQuorum, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { continue; } if (connmanQuorumsToDelete.count(quorum->qc->quorumHash) > 0) { @@ -339,8 +352,9 @@ bool CQuorumManager::BuildQuorumContributions(const CFinalCommitmentPtr& fqc, co } cxxtimer::Timer t2(true); + LOCK(quorum->cs); quorum->quorumVvec = blsWorker.BuildQuorumVerificationVector(vvecs); - if (quorum->quorumVvec == nullptr) { + if (!quorum->HasVerificationVector()) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- failed to build quorumVvec\n", __func__); // without the quorum vvec, there can't be a skShare, so we fail here. Failure is not fatal here, as it still // allows to use the quorum as a non-member (verification through the quorum pub key) @@ -371,7 +385,7 @@ bool CQuorumManager::RequestQuorumData(CNode* pFrom, Consensus::LLMQType llmqTyp LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Version must be %d or greater.\n", __func__, LLMQ_DATA_MESSAGES_VERSION); return false; } - if (pFrom == nullptr || (pFrom->verifiedProRegTxHash.IsNull() && !pFrom->qwatch)) { + if (pFrom == nullptr || (pFrom->GetVerifiedProRegTxHash().IsNull() && !pFrom->qwatch)) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pFrom is neither a verified masternode nor a qwatch connection\n", __func__); return false; } @@ -389,7 +403,7 @@ bool CQuorumManager::RequestQuorumData(CNode* pFrom, Consensus::LLMQType llmqTyp } LOCK(cs_data_requests); - auto key = std::make_pair(pFrom->verifiedProRegTxHash, true); + auto key = std::make_pair(pFrom->GetVerifiedProRegTxHash(), true); auto it = mapQuorumDataRequests.emplace(key, CQuorumDataRequest(llmqType, pQuorumIndex->GetBlockHash(), nDataMask, proTxHash)); if (!it.second && !it.first->second.IsExpired()) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Already requested\n", __func__); @@ -517,10 +531,13 @@ size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, c }); std::sort(vecProTxHashes.begin(), vecProTxHashes.end()); size_t nIndex{0}; - for (size_t i = 0; i < vecProTxHashes.size(); ++i) { - if (activeMasternodeInfo.proTxHash == vecProTxHashes[i]) { - nIndex = i; - break; + { + LOCK(activeMasternodeInfoCs); + for (size_t i = 0; i < vecProTxHashes.size(); ++i) { + if (activeMasternodeInfo.proTxHash == vecProTxHashes[i]) { + nIndex = i; + break; + } } } return nIndex % pQuorum->qc->validMembers.size(); @@ -539,7 +556,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, if (strCommand == NetMsgType::QGETDATA) { - if (!fMasternodeMode || pFrom == nullptr || (pFrom->verifiedProRegTxHash.IsNull() && !pFrom->qwatch)) { + if (!fMasternodeMode || pFrom == nullptr || (pFrom->GetVerifiedProRegTxHash().IsNull() && !pFrom->qwatch)) { errorHandler("Not a verified masternode or a qwatch connection"); return; } @@ -556,7 +573,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, { LOCK2(cs_main, cs_data_requests); - auto key = std::make_pair(pFrom->verifiedProRegTxHash, false); + auto key = std::make_pair(pFrom->GetVerifiedProRegTxHash(), false); auto it = mapQuorumDataRequests.find(key); if (it == mapQuorumDataRequests.end()) { it = mapQuorumDataRequests.emplace(key, request).first; @@ -592,13 +609,12 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, // Check if request wants QUORUM_VERIFICATION_VECTOR data if (request.GetDataMask() & CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR) { - - if (!pQuorum->quorumVvec) { + if (!pQuorum->HasVerificationVector()) { sendQDATA(CQuorumDataRequest::Errors::QUORUM_VERIFICATION_VECTOR_MISSING); return; } - ssResponseData << *pQuorum->quorumVvec; + WITH_LOCK(pQuorum->cs, ssResponseData << *pQuorum->quorumVvec); } // Check if request wants ENCRYPTED_CONTRIBUTIONS data @@ -624,8 +640,8 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, } if (strCommand == NetMsgType::QDATA) { - - if ((!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) || pFrom == nullptr || (pFrom->verifiedProRegTxHash.IsNull() && !pFrom->qwatch)) { + auto verifiedProRegTxHash = pFrom->GetVerifiedProRegTxHash(); + if ((!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) || pFrom == nullptr || (verifiedProRegTxHash.IsNull() && !pFrom->qwatch)) { errorHandler("Not a verified masternode or a qwatch connection"); return; } @@ -635,7 +651,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, { LOCK2(cs_main, cs_data_requests); - auto it = mapQuorumDataRequests.find(std::make_pair(pFrom->verifiedProRegTxHash, true)); + auto it = mapQuorumDataRequests.find(std::make_pair(verifiedProRegTxHash, true)); if (it == mapQuorumDataRequests.end()) { errorHandler("Not requested"); return; @@ -682,7 +698,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, // Check if request has ENCRYPTED_CONTRIBUTIONS data if (request.GetDataMask() & CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS) { - if (pQuorum->quorumVvec->size() != pQuorum->params.threshold) { + if (WITH_LOCK(pQuorum->cs, return pQuorum->quorumVvec->size() != pQuorum->params.threshold)) { errorHandler("No valid quorum verification vector available", 0); // Don't bump score because we asked for it return; } @@ -698,8 +714,9 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, BLSSecretKeyVector vecSecretKeys; vecSecretKeys.resize(vecEncrypted.size()); + auto secret = WITH_LOCK(activeMasternodeInfoCs, return *activeMasternodeInfo.blsKeyOperator); for (size_t i = 0; i < vecEncrypted.size(); ++i) { - if (!vecEncrypted[i].Decrypt(memberIdx, *activeMasternodeInfo.blsKeyOperator, vecSecretKeys[i], PROTOCOL_VERSION)) { + if (!vecEncrypted[i].Decrypt(memberIdx, secret, vecSecretKeys[i], PROTOCOL_VERSION)) { errorHandler("Failed to decrypt"); return; } @@ -718,7 +735,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const { - if (pQuorum->quorumVvec == nullptr) { + if (!pQuorum->HasVerificationVector()) { return; } @@ -771,7 +788,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 != activeMasternodeInfo.proTxHash) { + if (pQuorum->IsValidMember(member->proTxHash) && member->proTxHash != WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash)) { vecMemberHashes.push_back(member->proTxHash); } } @@ -781,12 +798,13 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co while (nDataMask > 0 && !quorumThreadInterrupt) { - if (nDataMask & llmq::CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR && pQuorum->quorumVvec != nullptr) { + if (nDataMask & llmq::CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR && + pQuorum->HasVerificationVector()) { nDataMask &= ~llmq::CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR; printLog("Received quorumVvec"); } - if (nDataMask & llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS && pQuorum->skShare.IsValid()) { + if (nDataMask & llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS && pQuorum->GetSkShare().IsValid()) { nDataMask &= ~llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS; printLog("Received skShare"); } @@ -818,18 +836,19 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co printLog("Connect"); } + auto proTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash); g_connman->ForEachNode([&](CNode* pNode) { - - if (pCurrentMemberHash == nullptr || pNode->verifiedProRegTxHash != *pCurrentMemberHash) { + auto verifiedProRegTxHash = pNode->GetVerifiedProRegTxHash(); + if (pCurrentMemberHash == nullptr || verifiedProRegTxHash != *pCurrentMemberHash) { return; } - if (quorumManager->RequestQuorumData(pNode, pQuorum->qc->llmqType, pQuorum->pindexQuorum, nDataMask, activeMasternodeInfo.proTxHash)) { + if (quorumManager->RequestQuorumData(pNode, pQuorum->qc->llmqType, pQuorum->pindexQuorum, nDataMask, proTxHash)) { nTimeLastSuccess = GetAdjustedTime(); printLog("Requested"); } else { LOCK(cs_data_requests); - auto it = mapQuorumDataRequests.find(std::make_pair(pNode->verifiedProRegTxHash, true)); + auto it = mapQuorumDataRequests.find(std::make_pair(verifiedProRegTxHash, true)); if (it == mapQuorumDataRequests.end()) { printLog("Failed"); pNode->fDisconnect = true; diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index f9f1c007da92..6faebf079ed5 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -157,16 +157,17 @@ class CQuorum uint256 minedBlockHash; std::vector members; - // These are only valid when we either participated in the DKG or fully watched it - BLSVerificationVectorPtr quorumVvec; - CBLSSecretKey skShare; - private: // Recovery of public key shares is very slow, so we start a background thread that pre-populates a cache so that // the public key shares are ready when needed later mutable CBLSWorkerCache blsCache; mutable std::atomic fQuorumDataRecoveryThreadRunning{false}; + mutable CCriticalSection cs; + // These are only valid when we either participated in the DKG or fully watched it + BLSVerificationVectorPtr quorumVvec GUARDED_BY(cs); + CBLSSecretKey skShare GUARDED_BY(cs); + public: CQuorum(const Consensus::LLMQParams& _params, CBLSWorker& _blsWorker); ~CQuorum(); @@ -175,12 +176,13 @@ class CQuorum bool SetVerificationVector(const BLSVerificationVector& quorumVecIn); bool SetSecretKeyShare(const CBLSSecretKey& secretKeyShare); + bool HasVerificationVector() const; bool IsMember(const uint256& proTxHash) const; bool IsValidMember(const uint256& proTxHash) const; int GetMemberIndex(const uint256& proTxHash) const; CBLSPublicKey GetPubKeyShare(size_t memberIdx) const; - const CBLSSecretKey& GetSkShare() const; + CBLSSecretKey GetSkShare() const; private: void WriteContributions(CEvoDB& evoDb) const; diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 1e9b192a94ad..fffaef4d91d3 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -555,23 +555,24 @@ void CChainLocksHandler::EnforceBestChainLock() activateNeeded = chainActive.Tip()->GetAncestor(currentBestChainLockBlockIndex->nHeight) != currentBestChainLockBlockIndex; } - if (activateNeeded && !ActivateBestChain(state, params)) { - LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); - } - - const CBlockIndex* pindexNotify = nullptr; - { + if (activateNeeded) { + if(!ActivateBestChain(state, params)) { + LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); + return; + } LOCK(cs_main); - if (lastNotifyChainLockBlockIndex != currentBestChainLockBlockIndex && - chainActive.Tip()->GetAncestor(currentBestChainLockBlockIndex->nHeight) == currentBestChainLockBlockIndex) { - lastNotifyChainLockBlockIndex = currentBestChainLockBlockIndex; - pindexNotify = currentBestChainLockBlockIndex; + if (chainActive.Tip()->GetAncestor(currentBestChainLockBlockIndex->nHeight) != currentBestChainLockBlockIndex) { + return; } } - if (pindexNotify) { - GetMainSignals().NotifyChainLock(pindexNotify, clsig); + { + LOCK(cs); + if (lastNotifyChainLockBlockIndex == currentBestChainLockBlockIndex) return; + lastNotifyChainLockBlockIndex = currentBestChainLockBlockIndex; } + + GetMainSignals().NotifyChainLock(currentBestChainLockBlockIndex, clsig); } void CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) diff --git a/src/llmq/quorums_dkgsession.cpp b/src/llmq/quorums_dkgsession.cpp index 360522f6b7c3..5bac24e423c5 100644 --- a/src/llmq/quorums_dkgsession.cpp +++ b/src/llmq/quorums_dkgsession.cpp @@ -199,7 +199,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages) logger.Batch("encrypted contributions. time=%d", t1.count()); - qc.sig = activeMasternodeInfo.blsKeyOperator->Sign(qc.GetSignHash()); + qc.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(qc.GetSignHash())); logger.Flush(); @@ -324,7 +324,7 @@ void CDKGSession::ReceiveMessage(const CDKGContribution& qc, bool& retBan) bool complain = false; CBLSSecretKey skContribution; - if (!qc.contributions->Decrypt(myIdx, *activeMasternodeInfo.blsKeyOperator, skContribution, PROTOCOL_VERSION)) { + if (!qc.contributions->Decrypt(myIdx, WITH_LOCK(activeMasternodeInfoCs, return *activeMasternodeInfo.blsKeyOperator), skContribution, PROTOCOL_VERSION)) { logger.Batch("contribution from %s could not be decrypted", member->dmn->proTxHash.ToString()); complain = true; } else if (member->idx != myIdx && ShouldSimulateError("complain-lie")) { @@ -466,10 +466,11 @@ void CDKGSession::VerifyConnectionAndMinProtoVersions() const std::unordered_map protoMap; g_connman->ForEachNode([&](const CNode* pnode) { - if (pnode->verifiedProRegTxHash.IsNull()) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (verifiedProRegTxHash.IsNull()) { return; } - protoMap.emplace(pnode->verifiedProRegTxHash, pnode->nVersion); + protoMap.emplace(verifiedProRegTxHash, pnode->nVersion); }); bool fShouldAllMembersBeConnected = CLLMQUtils::IsAllMembersConnectedEnabled(params.type); @@ -525,7 +526,7 @@ void CDKGSession::SendComplaint(CDKGPendingMessages& pendingMessages) logger.Batch("sending complaint. badCount=%d, complaintCount=%d", badCount, complaintCount); - qc.sig = activeMasternodeInfo.blsKeyOperator->Sign(qc.GetSignHash()); + qc.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(qc.GetSignHash())); logger.Flush(); @@ -657,6 +658,8 @@ void CDKGSession::VerifyAndJustify(CDKGPendingMessages& pendingMessages) std::set justifyFor; + LOCK(invCs); + for (const auto& m : members) { if (m->bad) { continue; @@ -723,7 +726,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const return; } - qj.sig = activeMasternodeInfo.blsKeyOperator->Sign(qj.GetSignHash()); + qj.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(qj.GetSignHash())); logger.Flush(); @@ -1025,7 +1028,7 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) (*commitmentHash.begin())++; } - qc.sig = activeMasternodeInfo.blsKeyOperator->Sign(commitmentHash); + qc.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(commitmentHash)); qc.quorumSig = skShare.Sign(commitmentHash); if (lieType == 3) { @@ -1212,21 +1215,25 @@ std::vector CDKGSession::FinalizeCommitments() typedef std::vector Key; std::map> commitmentsMap; - for (const auto& p : prematureCommitments) { - auto& qc = p.second; - if (!validCommitments.count(p.first)) { - continue; - } + { + LOCK(invCs); - // should have been verified before - assert(qc.CountValidMembers() >= params.minSize); + for (const auto& p : prematureCommitments) { + auto& qc = p.second; + if (!validCommitments.count(p.first)) { + continue; + } - auto it = commitmentsMap.find(qc.validMembers); - if (it == commitmentsMap.end()) { - it = commitmentsMap.emplace(qc.validMembers, std::vector()).first; - } + // should have been verified before + assert(qc.CountValidMembers() >= params.minSize); - it->second.emplace_back(qc); + auto it = commitmentsMap.find(qc.validMembers); + if (it == commitmentsMap.end()) { + it = commitmentsMap.emplace(qc.validMembers, std::vector()).first; + } + + it->second.emplace_back(qc); + } } std::vector finalCommitments; @@ -1328,9 +1335,10 @@ void CDKGSession::RelayInvToParticipants(const CInv& inv) const LOCK(invCs); g_connman->ForEachNode([&](CNode* pnode) { bool relay = false; + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); if (pnode->qwatch) { relay = true; - } else if (!pnode->verifiedProRegTxHash.IsNull() && relayMembers.count(pnode->verifiedProRegTxHash)) { + } else if (!verifiedProRegTxHash.IsNull() && relayMembers.count(verifiedProRegTxHash)) { relay = true; } if (relay) { diff --git a/src/llmq/quorums_dkgsessionhandler.cpp b/src/llmq/quorums_dkgsessionhandler.cpp index 05dcc1889785..2b409fabac3e 100644 --- a/src/llmq/quorums_dkgsessionhandler.cpp +++ b/src/llmq/quorums_dkgsessionhandler.cpp @@ -166,7 +166,7 @@ bool CDKGSessionHandler::InitNewQuorum(const CBlockIndex* pindexQuorum) auto mns = CLLMQUtils::GetAllQuorumMembers(params.type, pindexQuorum); - if (!curSession->Init(pindexQuorum, mns, activeMasternodeInfo.proTxHash)) { + if (!curSession->Init(pindexQuorum, mns, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { LogPrintf("CDKGSessionManager::%s -- quorum initialization failed for %s\n", __func__, curSession->params.name); return false; } @@ -386,7 +386,7 @@ std::set BatchVerifyMessageSigs(CDKGSession& session, const std::vector< } // are all messages from the same node? - NodeId firstNodeId; + NodeId firstNodeId{-1}; first = true; bool nodeIdsAllSame = true; for (auto it = messages.begin(); it != messages.end(); ++it) { diff --git a/src/llmq/quorums_dkgsessionhandler.h b/src/llmq/quorums_dkgsessionhandler.h index abbefae5d638..121b7dd13f68 100644 --- a/src/llmq/quorums_dkgsessionhandler.h +++ b/src/llmq/quorums_dkgsessionhandler.h @@ -117,10 +117,11 @@ class CDKGSessionHandler std::shared_ptr curSession; std::thread phaseHandlerThread; - CDKGPendingMessages pendingContributions GUARDED_BY(cs); - CDKGPendingMessages pendingComplaints GUARDED_BY(cs); - CDKGPendingMessages pendingJustifications GUARDED_BY(cs); - CDKGPendingMessages pendingPrematureCommitments GUARDED_BY(cs); + // Do not guard these, they protect their internals themselves + CDKGPendingMessages pendingContributions; + CDKGPendingMessages pendingComplaints; + CDKGPendingMessages pendingJustifications; + CDKGPendingMessages pendingPrematureCommitments; public: CDKGSessionHandler(const Consensus::LLMQParams& _params, CBLSWorker& blsWorker, CDKGSessionManager& _dkgManager); diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 7573398c0b1a..87da7e73c23e 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1418,7 +1418,7 @@ void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) { std::vector nodesToAskFor; g_connman->ForEachNode([&](CNode* pnode) { - LOCK(pnode->cs_filter); + LOCK(pnode->cs_inventory); if (pnode->filterInventoryKnown.contains(txid)) { pnode->AddRef(); nodesToAskFor.emplace_back(pnode); diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 70c82783fce1..2c029421475a 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -784,7 +784,7 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l) bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, const uint256& quorumHash, bool allowReSign) { - if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) { + if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) { return false; } @@ -805,7 +805,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint return false; } - if (!quorum->IsValidMember(activeMasternodeInfo.proTxHash)) { + if (!WITH_LOCK(activeMasternodeInfoCs, return quorum->IsValidMember(activeMasternodeInfo.proTxHash))) { return false; } diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 5b2e0f3ea990..fb241b00598c 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -234,7 +234,7 @@ void CSigSharesManager::InterruptWorkerThread() void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv) { // non-masternodes are not interested in sigshares - if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) { + if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) { return; } @@ -372,7 +372,7 @@ bool CSigSharesManager::ProcessMessageSigSharesInv(CNode* pfrom, const CSigShare LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, sessionInfo.signHash.ToString(), inv.ToString(), pfrom->GetId()); - if (sessionInfo.quorum->quorumVvec == nullptr) { + if (!sessionInfo.quorum->HasVerificationVector()) { // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, sessionInfo.quorumHash.ToString(), pfrom->GetId()); @@ -485,11 +485,11 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s // quorum is too old return; } - if (!quorum->IsMember(activeMasternodeInfo.proTxHash)) { + if (!quorum->IsMember(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { // we're not a member so we can't verify it (we actually shouldn't have received it) return; } - if (quorum->quorumVvec == nullptr) { + if (!quorum->HasVerificationVector()) { // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible. node=%d\n", __func__, quorum->qc->quorumHash.ToString(), fromId); @@ -534,11 +534,11 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(const CSigSharesNodeState::Ses // quorum is too old return false; } - if (!session.quorum->IsMember(activeMasternodeInfo.proTxHash)) { + if (!session.quorum->IsMember(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { // we're not a member so we can't verify it (we actually shouldn't have received it) return false; } - if (session.quorum->quorumVvec == nullptr) { + if (!session.quorum->HasVerificationVector()) { // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible.\n", __func__, session.quorumHash.ToString()); @@ -594,6 +594,7 @@ void CSigSharesManager::CollectPendingSigSharesToVerify( } auto& sigShare = *ns.pendingIncomingSigShares.GetFirst(); + AssertLockHeld(cs); bool alreadyHave = this->sigShares.Has(sigShare.GetKey()); if (!alreadyHave) { uniqueSignHashes.emplace(nodeId, sigShare.GetSignHash()); @@ -729,7 +730,7 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, CConnman& con // prepare node set for direct-push in case this is our sig share std::set quorumNodes; - if (!CLLMQUtils::IsAllMembersConnectedEnabled(llmqType) && sigShare.quorumMember == quorum->GetMemberIndex(activeMasternodeInfo.proTxHash)) { + if (!CLLMQUtils::IsAllMembersConnectedEnabled(llmqType) && sigShare.quorumMember == quorum->GetMemberIndex(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { quorumNodes = connman.GetMasternodeQuorumNodes(sigShare.llmqType, sigShare.quorumHash); } @@ -1017,10 +1018,11 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map proTxToNode; for (const auto& pnode : vNodes) { - if (pnode->verifiedProRegTxHash.IsNull()) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (verifiedProRegTxHash.IsNull()) { continue; } - proTxToNode.emplace(pnode->verifiedProRegTxHash, pnode); + proTxToNode.emplace(verifiedProRegTxHash, pnode); } auto curTime = GetTime().count(); @@ -1062,6 +1064,7 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map, std::unordered_set, StaticSaltedHasher> quorumNodesMap; sigSharesQueuedToAnnounce.ForEach([&](const SigShareKey& sigShareKey, bool) { + AssertLockHeld(cs); auto& signHash = sigShareKey.first; auto quorumMember = sigShareKey.second; const CSigShare* sigShare = sigShares.Get(sigShareKey); @@ -1421,6 +1424,7 @@ void CSigSharesManager::Cleanup() } // remove global requested state to force a re-request from another node it->second.requestedSigShares.ForEach([&](const SigShareKey& k, bool) { + AssertLockHeld(cs); sigSharesRequested.Erase(k); }); nodeStates.erase(nodeId); @@ -1455,6 +1459,7 @@ void CSigSharesManager::RemoveBannedNodeStates() if (IsBanned(it->first)) { // re-request sigshares from other nodes it->second.requestedSigShares.ForEach([&](const SigShareKey& k, int64_t) { + AssertLockHeld(cs); sigSharesRequested.Erase(k); }); it = nodeStates.erase(it); @@ -1484,6 +1489,7 @@ void CSigSharesManager::BanNode(NodeId nodeId) // Whatever we requested from him, let's request it from someone else now nodeState.requestedSigShares.ForEach([&](const SigShareKey& k, int64_t) { + AssertLockHeld(cs); sigSharesRequested.Erase(k); }); nodeState.requestedSigShares.Clear(); @@ -1562,8 +1568,9 @@ void CSigSharesManager::SignPendingSigShares() CSigShare CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const { cxxtimer::Timer t(true); + auto activeMasterNodeProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash); - if (!quorum->IsValidMember(activeMasternodeInfo.proTxHash)) { + if (!quorum->IsValidMember(activeMasterNodeProTxHash)) { return {}; } @@ -1573,7 +1580,7 @@ CSigShare CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uin return {}; } - int memberIdx = quorum->GetMemberIndex(activeMasternodeInfo.proTxHash); + int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash); if (memberIdx == -1) { // this should really not happen (IsValidMember gave true) return {}; diff --git a/src/masternode/activemasternode.cpp b/src/masternode/activemasternode.cpp index 61f0b828d531..43d477a29e6f 100644 --- a/src/masternode/activemasternode.cpp +++ b/src/masternode/activemasternode.cpp @@ -15,7 +15,8 @@ #include // Keep track of the active Masternode -CActiveMasternodeInfo activeMasternodeInfo; +CCriticalSection activeMasternodeInfoCs; +CActiveMasternodeInfo activeMasternodeInfo GUARDED_BY(activeMasternodeInfoCs); CActiveMasternodeManager* activeMasternodeManager; std::string CActiveMasternodeManager::GetStateString() const @@ -64,7 +65,7 @@ std::string CActiveMasternodeManager::GetStatus() const void CActiveMasternodeManager::Init(const CBlockIndex* pindex) { - LOCK(cs_main); + LOCK2(cs_main, activeMasternodeInfoCs); if (!fMasternodeMode) return; @@ -136,7 +137,7 @@ void CActiveMasternodeManager::Init(const CBlockIndex* pindex) void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) { - LOCK(cs_main); + LOCK2(cs_main, activeMasternodeInfoCs); if (!fMasternodeMode) return; @@ -201,10 +202,11 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) if (!fFoundLocal) { bool empty = true; // If we have some peers, let's try to find our local address from one of them - g_connman->ForEachNodeContinueIf(CConnman::AllNodes, [&fFoundLocal, &empty](CNode* pnode) { + auto service = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.service); + g_connman->ForEachNodeContinueIf(CConnman::AllNodes, [&](CNode* pnode) { empty = false; if (pnode->addr.IsIPv4()) - fFoundLocal = GetLocal(activeMasternodeInfo.service, &pnode->addr) && IsValidNetAddr(activeMasternodeInfo.service); + fFoundLocal = GetLocal(service, &pnode->addr) && IsValidNetAddr(service); return !fFoundLocal; }); // nothing and no live connections, can't do anything for now diff --git a/src/masternode/activemasternode.h b/src/masternode/activemasternode.h index 5eee35955b28..5991e6db8fbc 100644 --- a/src/masternode/activemasternode.h +++ b/src/masternode/activemasternode.h @@ -16,6 +16,7 @@ struct CActiveMasternodeInfo; class CActiveMasternodeManager; extern CActiveMasternodeInfo activeMasternodeInfo; +extern CCriticalSection activeMasternodeInfoCs; extern CActiveMasternodeManager* activeMasternodeManager; struct CActiveMasternodeInfo { diff --git a/src/masternode/masternode-sync.cpp b/src/masternode/masternode-sync.cpp index d236376782c0..bb962b646116 100644 --- a/src/masternode/masternode-sync.cpp +++ b/src/masternode/masternode-sync.cpp @@ -23,20 +23,24 @@ CMasternodeSync::CMasternodeSync() void CMasternodeSync::Reset(bool fForce, bool fNotifyReset) { // Avoid resetting the sync process if we just "recently" received a new block - if (fForce || (GetTime() - nTimeLastUpdateBlockTip > MASTERNODE_SYNC_RESET_SECONDS)) { - { - LOCK(cs); - nCurrentAsset = MASTERNODE_SYNC_BLOCKCHAIN; - nTriedPeerCount = 0; - nTimeAssetSyncStarted = GetTime(); - nTimeLastBumped = GetTime(); - nTimeLastUpdateBlockTip = 0; - fReachedBestHeader = false; - } - if (fNotifyReset) { - uiInterface.NotifyAdditionalDataSyncProgressChanged(-1); + if (!fForce) { + LOCK(cs); + if (GetTime() - nTimeLastUpdateBlockTip < MASTERNODE_SYNC_RESET_SECONDS) { + return; } } + { + LOCK(cs); + nCurrentAsset = MASTERNODE_SYNC_BLOCKCHAIN; + nTriedPeerCount = 0; + nTimeAssetSyncStarted = GetTime(); + nTimeLastBumped = GetTime(); + nTimeLastUpdateBlockTip = 0; + fReachedBestHeader = false; + } + if (fNotifyReset) { + uiInterface.NotifyAdditionalDataSyncProgressChanged(-1); + } } void CMasternodeSync::BumpAssetLastTime(const std::string& strFuncName) @@ -88,6 +92,7 @@ void CMasternodeSync::SwitchToNextAsset(CConnman& connman) std::string CMasternodeSync::GetSyncStatus() const { + LOCK(cs); switch (nCurrentAsset) { case MASTERNODE_SYNC_BLOCKCHAIN: return _("Synchronizing blockchain..."); case MASTERNODE_SYNC_GOVERNANCE: return _("Synchronizing governance objects..."); @@ -143,6 +148,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman) return; } + LOCK(cs); // Calculate "progress" for LOG reporting / GUI notification double nSyncProgress = double(nTriedPeerCount + (nCurrentAsset - 1) * 8) / (8*4); LogPrint(BCLog::MNSYNC, "CMasternodeSync::ProcessTick -- nTick %d nCurrentAsset %d nTriedPeerCount %d nSyncProgress %f\n", nTick, nCurrentAsset, nTriedPeerCount, nSyncProgress); @@ -371,6 +377,7 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitia // Note: since we sync headers first, it should be ok to use this bool fReachedBestHeaderNew = pindexNew->GetBlockHash() == pindexTip->GetBlockHash(); + LOCK(cs); if (fReachedBestHeader && !fReachedBestHeaderNew) { // Switching from true to false means that we previously stuck syncing headers for some reason, // probably initial timeout was not enough, @@ -378,10 +385,7 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitia Reset(true); } - { - LOCK(cs); - fReachedBestHeader = fReachedBestHeaderNew; - } + fReachedBestHeader = fReachedBestHeaderNew; LogPrint(BCLog::MNSYNC, "CMasternodeSync::UpdatedBlockTip -- pindexNew->nHeight: %d pindexTip->nHeight: %d fInitialDownload=%d fReachedBestHeader=%d\n", pindexNew->nHeight, pindexTip->nHeight, fInitialDownload, fReachedBestHeader); } diff --git a/src/net.cpp b/src/net.cpp index 0be9499d5c28..7a3ca1f5e185 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1108,7 +1108,7 @@ bool CConnman::AttemptToEvictConnection() } // if MNAUTH was valid, the node is always protected (and at the same time not accounted when // checking incoming connection limits) - if (!node->verifiedProRegTxHash.IsNull()) { + if (!node->GetVerifiedProRegTxHash().IsNull()) { isProtected = true; } if (isProtected) { @@ -1198,7 +1198,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { for (const CNode* pnode : vNodes) { if (pnode->fInbound) { nInbound++; - if (!pnode->verifiedProRegTxHash.IsNull()) { + if (!pnode->GetVerifiedProRegTxHash().IsNull()) { nVerifiedInboundMasternodes++; } } @@ -1278,15 +1278,15 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { m_msgproc->InitializeNode(pnode); if (fLogIPs) { - LogPrint(BCLog::NET_NETCONN, "connection from %s accepted, sock=%d, peer=%d\n", addr.ToString(), pnode->hSocket, pnode->GetId()); + LogPrint(BCLog::NET_NETCONN, "connection from %s accepted, sock=%d, peer=%d\n", addr.ToString(), hSocket, pnode->GetId()); } else { - LogPrint(BCLog::NET_NETCONN, "connection accepted, sock=%d, peer=%d\n", pnode->hSocket, pnode->GetId()); + LogPrint(BCLog::NET_NETCONN, "connection accepted, sock=%d, peer=%d\n", hSocket, pnode->GetId()); } { LOCK(cs_vNodes); vNodes.push_back(pnode); - mapSocketToNode.emplace(pnode->hSocket, pnode); + mapSocketToNode.emplace(hSocket, pnode); RegisterEvents(pnode); WakeSelect(); } @@ -1443,8 +1443,9 @@ void CConnman::CalculateNumConnectionsChangedStats() } mapRecvBytesMsgStats[NET_MESSAGE_COMMAND_OTHER] = 0; mapSentBytesMsgStats[NET_MESSAGE_COMMAND_OTHER] = 0; - LOCK(cs_vNodes); - for (const CNode* pnode : vNodes) { + auto vNodesCopy = CopyNodeVector(CConnman::FullyConnectedOnly); + for (auto pnode : vNodesCopy) { + LOCK(pnode->cs_vRecv); for (const mapMsgCmdSize::value_type &i : pnode->mapRecvBytesPerMsgCmd) mapRecvBytesMsgStats[i.first] += i.second; for (const mapMsgCmdSize::value_type &i : pnode->mapSendBytesPerMsgCmd) @@ -1466,6 +1467,7 @@ void CConnman::CalculateNumConnectionsChangedStats() if(pnode->nPingUsecTime > 0) statsClient.timing("peers.ping_us", pnode->nPingUsecTime, 1.0f); } + ReleaseNodeVector(vNodesCopy); for (const std::string &msg : getAllNetMessageTypes()) { statsClient.gauge("bandwidth.message." + msg + ".totalBytesReceived", mapRecvBytesMsgStats[msg], 1.0f); statsClient.gauge("bandwidth.message." + msg + ".totalBytesSent", mapSentBytesMsgStats[msg], 1.0f); @@ -2406,8 +2408,9 @@ void CConnman::ThreadOpenConnections(const std::vector connect) { LOCK(cs_vNodes); for (CNode* pnode : vNodes) { - if (!pnode->verifiedProRegTxHash.IsNull()) { - setConnectedMasternodes.emplace(pnode->verifiedProRegTxHash); + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (!verifiedProRegTxHash.IsNull()) { + setConnectedMasternodes.emplace(verifiedProRegTxHash); } } } @@ -2630,9 +2633,10 @@ void CConnman::ThreadOpenMasternodeConnections() std::set connectedNodes; std::map connectedProRegTxHashes; ForEachNode([&](const CNode* pnode) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); connectedNodes.emplace(pnode->addr); - if (!pnode->verifiedProRegTxHash.IsNull()) { - connectedProRegTxHashes.emplace(pnode->verifiedProRegTxHash, pnode->fInbound); + if (!verifiedProRegTxHash.IsNull()) { + connectedProRegTxHashes.emplace(verifiedProRegTxHash, pnode->fInbound); } }); @@ -2787,7 +2791,12 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- ConnectNode failed for %s\n", __func__, getIpStr()); return; } - LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- succesfully connected to %s, sock=%d, peer=%d\n", __func__, getIpStr(), pnode->hSocket, pnode->GetId()); + + { + LOCK(pnode->cs_hSocket); + LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- succesfully connected to %s, sock=%d, peer=%d\n", __func__, getIpStr(), pnode->hSocket, pnode->GetId()); + } + if (grantOutbound) grantOutbound->MoveTo(pnode->grantOutbound); if (fOneShot) @@ -2802,7 +2811,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai pnode->m_masternode_probe_connection = true; { - LOCK(cs_vNodes); + LOCK2(cs_vNodes, pnode->cs_hSocket); mapSocketToNode.emplace(pnode->hSocket, pnode); } @@ -3374,7 +3383,10 @@ void CConnman::Stop() } vNodes.clear(); mapSocketToNode.clear(); - mapReceivableNodes.clear(); + { + LOCK(cs_vNodes); + mapReceivableNodes.clear(); + } { LOCK(cs_mapNodesWithDataToSend); mapNodesWithDataToSend.clear(); @@ -3509,7 +3521,8 @@ void CConnman::SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, con // Update existing connections ForEachNode([&](CNode* pnode) { - if (!pnode->verifiedProRegTxHash.IsNull() && !pnode->m_masternode_iqr_connection && IsMasternodeQuorumRelayMember(pnode->verifiedProRegTxHash)) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (!verifiedProRegTxHash.IsNull() && !pnode->m_masternode_iqr_connection && IsMasternodeQuorumRelayMember(verifiedProRegTxHash)) { // Tell our peer that we're interested in plain LLMQ recovered signatures. // Otherwise the peer would only announce/send messages resulting from QRECSIG, // e.g. InstantSend locks or ChainLocks. SPV and regular full nodes should not send @@ -3554,7 +3567,8 @@ std::set CConnman::GetMasternodeQuorumNodes(Consensus::LLMQType llmqType if (pnode->fDisconnect) { continue; } - if (!pnode->qwatch && (pnode->verifiedProRegTxHash.IsNull() || !proRegTxHashes.count(pnode->verifiedProRegTxHash))) { + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (!pnode->qwatch && (verifiedProRegTxHash.IsNull() || !proRegTxHashes.count(verifiedProRegTxHash))) { continue; } nodes.emplace(pnode->GetId()); @@ -3574,7 +3588,7 @@ bool CConnman::IsMasternodeQuorumNode(const CNode* pnode) // Let's see if this is an outgoing connection to an address that is known to be a masternode // We however only need to know this if the node did not authenticate itself as a MN yet uint256 assumedProTxHash; - if (pnode->verifiedProRegTxHash.IsNull() && !pnode->fInbound) { + if (pnode->GetVerifiedProRegTxHash().IsNull() && !pnode->fInbound) { auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetMNByService(pnode->addr); if (dmn == nullptr) { @@ -3586,8 +3600,8 @@ bool CConnman::IsMasternodeQuorumNode(const CNode* pnode) LOCK(cs_vPendingMasternodes); for (const auto& p : masternodeQuorumNodes) { - if (!pnode->verifiedProRegTxHash.IsNull()) { - if (p.second.count(pnode->verifiedProRegTxHash)) { + if (!pnode->GetVerifiedProRegTxHash().IsNull()) { + if (p.second.count(pnode->GetVerifiedProRegTxHash())) { return true; } } else if (!assumedProTxHash.IsNull()) { diff --git a/src/net.h b/src/net.h index b86e87086695..9b1e6841f073 100644 --- a/src/net.h +++ b/src/net.h @@ -904,11 +904,11 @@ class CNode bool fRelayTxes; //protected by cs_filter bool fSentAddr; // If 'true' this node will be disconnected on CMasternodeMan::ProcessMasternodeConnections() - bool m_masternode_connection; + std::atomic m_masternode_connection; // If 'true' this node will be disconnected after MNAUTH - bool m_masternode_probe_connection; + std::atomic m_masternode_probe_connection; // If 'true', we identified it as an intra-quorum relay connection - bool m_masternode_iqr_connection{false}; + std::atomic m_masternode_iqr_connection{false}; CSemaphoreGrant grantOutbound; CCriticalSection cs_filter; std::unique_ptr pfilter PT_GUARDED_BY(cs_filter){nullptr}; @@ -979,13 +979,6 @@ class CNode // If true, we will send him CoinJoin queue messages std::atomic fSendDSQueue{false}; - // Challenge sent in VERSION to be answered with MNAUTH (only happens between MNs) - mutable CCriticalSection cs_mnauth; - uint256 sentMNAuthChallenge; - uint256 receivedMNAuthChallenge; - uint256 verifiedProRegTxHash; - uint256 verifiedPubKeyHash; - // If true, we will announce/send him plain recovered sigs (usually true for full nodes) std::atomic fSendRecSigs{false}; // If true, we will send him all quorum related messages, even if he is not a member of our quorums @@ -1013,6 +1006,14 @@ class CNode // Our address, as reported by the peer CService addrLocal GUARDED_BY(cs_addrLocal); mutable CCriticalSection cs_addrLocal; + + // Challenge sent in VERSION to be answered with MNAUTH (only happens between MNs) + mutable CCriticalSection cs_mnauth; + uint256 sentMNAuthChallenge GUARDED_BY(cs_mnauth); + uint256 receivedMNAuthChallenge GUARDED_BY(cs_mnauth); + uint256 verifiedProRegTxHash GUARDED_BY(cs_mnauth); + uint256 verifiedPubKeyHash GUARDED_BY(cs_mnauth); + public: NodeId GetId() const { @@ -1146,6 +1147,46 @@ class CNode std::string GetLogString() const; bool CanRelay() const { return !m_masternode_connection || m_masternode_iqr_connection; } + + uint256 GetSentMNAuthChallenge() const { + LOCK(cs_mnauth); + return sentMNAuthChallenge; + } + + uint256 GetReceivedMNAuthChallenge() const { + LOCK(cs_mnauth); + return receivedMNAuthChallenge; + } + + uint256 GetVerifiedProRegTxHash() const { + LOCK(cs_mnauth); + return verifiedProRegTxHash; + } + + uint256 GetVerifiedPubKeyHash() const { + LOCK(cs_mnauth); + return verifiedPubKeyHash; + } + + void SetSentMNAuthChallenge(const uint256& newSentMNAuthChallenge) { + LOCK(cs_mnauth); + sentMNAuthChallenge = newSentMNAuthChallenge; + } + + void SetReceivedMNAuthChallenge(const uint256& newReceivedMNAuthChallenge) { + LOCK(cs_mnauth); + receivedMNAuthChallenge = newReceivedMNAuthChallenge; + } + + void SetVerifiedProRegTxHash(const uint256& newVerifiedProRegTxHash) { + LOCK(cs_mnauth); + verifiedProRegTxHash = newVerifiedProRegTxHash; + } + + void SetVerifiedPubKeyHash(const uint256& newVerifiedPubKeyHash) { + LOCK(cs_mnauth); + verifiedPubKeyHash = newVerifiedPubKeyHash; + } }; class CExplicitNetCleanup diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 16605ae3cfa7..7f37fa1b9191 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -434,10 +434,7 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime) uint256 mnauthChallenge; GetRandBytes(mnauthChallenge.begin(), mnauthChallenge.size()); - { - LOCK(pnode->cs_mnauth); - pnode->sentMNAuthChallenge = mnauthChallenge; - } + pnode->SetSentMNAuthChallenge(mnauthChallenge); int nProtocolVersion = PROTOCOL_VERSION; if (params.NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) { @@ -445,7 +442,7 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime) } connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, nProtocolVersion, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, - nonce, strSubVersion, nNodeStartingHeight, ::fRelayTxes, mnauthChallenge, pnode->m_masternode_connection)); + nonce, strSubVersion, nNodeStartingHeight, ::fRelayTxes, mnauthChallenge, pnode->m_masternode_connection.load())); if (fLogIPs) { LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%d\n", nProtocolVersion, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), nodeid); @@ -1077,7 +1074,6 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV } } -// Requires cs_main. bool IsBanned(NodeId pnode) { CNodeState *state = State(pnode); @@ -2209,8 +2205,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (!vRecv.empty()) vRecv >> fRelay; if (!vRecv.empty()) { - LOCK(pfrom->cs_mnauth); - vRecv >> pfrom->receivedMNAuthChallenge; + uint256 receivedMNAuthChallenge; + vRecv >> receivedMNAuthChallenge; + pfrom->SetReceivedMNAuthChallenge(receivedMNAuthChallenge); } if (!vRecv.empty()) { bool fOtherMasternode = false; @@ -4235,13 +4232,12 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // std::vector vInv; { + LOCK2(mempool.cs, pto->cs_inventory); size_t reserve = std::min(pto->setInventoryTxToSend.size(), INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000); reserve = std::max(reserve, pto->vInventoryBlockToSend.size()); reserve = std::min(reserve, MAX_INV_SZ); vInv.reserve(reserve); - LOCK2(mempool.cs, pto->cs_inventory); - // Add blocks for (const uint256& hash : pto->vInventoryBlockToSend) { vInv.push_back(CInv(MSG_BLOCK, hash)); @@ -4263,7 +4259,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } else { // Use half the delay for regular outbound peers, as there is less privacy concern for them. // and quarter the delay for Masternode outbound peers, as there is even less privacy concern in this case. - pto->nNextInvSend = PoissonNextSend(current_time, std::chrono::seconds{INVENTORY_BROADCAST_INTERVAL >> 1 >> !pto->verifiedProRegTxHash.IsNull()}); + pto->nNextInvSend = PoissonNextSend(current_time, std::chrono::seconds{INVENTORY_BROADCAST_INTERVAL >> 1 >> !pto->GetVerifiedProRegTxHash().IsNull()}); } } diff --git a/src/net_processing.h b/src/net_processing.h index 5e24d53ff98c..b59349b32a67 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -87,15 +87,15 @@ struct CNodeStateStats { /** Get statistics from node state */ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); -bool IsBanned(NodeId nodeid); +bool IsBanned(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Upstream moved this into net_processing.cpp (13417), however since we use Misbehaving in a number of dash specific // files such as mnauth.cpp and governance.cpp it makes sense to keep it in the header /** Increase a node's misbehavior score. */ void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main); -void EraseObjectRequest(NodeId nodeId, const CInv& inv); -void RequestObject(NodeId nodeId, const CInv& inv, std::chrono::microseconds current_time, bool fForce=false); -size_t GetRequestedObjectCount(NodeId nodeId); +void EraseObjectRequest(NodeId nodeId, const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +void RequestObject(NodeId nodeId, const CInv& inv, std::chrono::microseconds current_time, bool fForce=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +size_t GetRequestedObjectCount(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(cs_main); #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index aa0daa512240..1a3a42c0ead8 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -925,7 +925,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) try { LOCK(cs_feeEstimator); int nVersionRequired, nVersionThatWrote; - unsigned int nFileBestSeenHeight, nFileHistoricalFirst, nFileHistoricalBest; + unsigned int nFileBestSeenHeight; filein >> nVersionRequired >> nVersionThatWrote; if (nVersionRequired > CLIENT_VERSION) return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 7f621211aa94..cedf8dcf44ad 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -482,7 +482,6 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * unsigned int nBytes = 0; unsigned int nBytesInputs = 0; unsigned int nQuantity = 0; - int nQuantityUncompressed = 0; bool fUnselectedSpent{false}; bool fUnselectedNonMixed{false}; diff --git a/src/qt/networkstyle.cpp b/src/qt/networkstyle.cpp index 639edda518b7..7a9c277e2c8a 100644 --- a/src/qt/networkstyle.cpp +++ b/src/qt/networkstyle.cpp @@ -42,9 +42,8 @@ void NetworkStyle::rotateColor(QColor& col, const int iconColorHueShift, const i col.setHsl(h,s,l,a); } -void NetworkStyle::rotateColors(QImage& img, const int iconColorHueShift, const int iconColorSaturationReduction) { - int h,s,l,a; - +void NetworkStyle::rotateColors(QImage& img, const int iconColorHueShift, const int iconColorSaturationReduction) +{ // traverse though lines for(int y=0;y::const_iterator i; - for (auto i = mapWalletViews.constBegin(); i != mapWalletViews.constEnd(); ++i) { + for (i = mapWalletViews.constBegin(); i != mapWalletViews.constEnd(); ++i) { i.value()->gotoCoinJoinCoinsPage(addr); } } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 3107eefd103a..9485b4a17606 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -310,11 +310,11 @@ static UniValue gobject_submit(const JSONRPCRequest& request) } auto mnList = deterministicMNManager->GetListAtChainTip(); - bool fMnFound = mnList.HasValidMNByCollateral(activeMasternodeInfo.outpoint); + bool fMnFound = WITH_LOCK(activeMasternodeInfoCs, return mnList.HasValidMNByCollateral(activeMasternodeInfo.outpoint)); LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n", - (activeMasternodeInfo.blsPubKeyOperator ? activeMasternodeInfo.blsPubKeyOperator->ToString() : "N/A"), - activeMasternodeInfo.outpoint.ToStringShort(), request.params.size(), fMnFound); + (WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsPubKeyOperator ? activeMasternodeInfo.blsPubKeyOperator->ToString() : "N/A")), + WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint.ToStringShort()), request.params.size(), fMnFound); // ASSEMBLE NEW GOVERNANCE OBJECT FROM USER PARAMETERS @@ -351,6 +351,7 @@ static UniValue gobject_submit(const JSONRPCRequest& request) // Attempt to sign triggers if we are a MN if (govobj.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER) { if (fMnFound) { + LOCK(activeMasternodeInfoCs); govobj.SetMasternodeOutpoint(activeMasternodeInfo.outpoint); govobj.Sign(*activeMasternodeInfo.blsKeyOperator); } else { @@ -450,7 +451,7 @@ static UniValue gobject_vote_conf(const JSONRPCRequest& request) UniValue statusObj(UniValue::VOBJ); UniValue returnObj(UniValue::VOBJ); - auto dmn = deterministicMNManager->GetListAtChainTip().GetValidMNByCollateral(activeMasternodeInfo.outpoint); + auto dmn = WITH_LOCK(activeMasternodeInfoCs, return deterministicMNManager->GetListAtChainTip().GetValidMNByCollateral(activeMasternodeInfo.outpoint)); if (!dmn) { nFailed++; @@ -468,8 +469,12 @@ static UniValue gobject_vote_conf(const JSONRPCRequest& request) if (govObjType == GOVERNANCE_OBJECT_PROPOSAL && eVoteSignal == VOTE_SIGNAL_FUNDING) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Can't use vote-conf for proposals"); } - if (activeMasternodeInfo.blsKeyOperator) { - signSuccess = vote.Sign(*activeMasternodeInfo.blsKeyOperator); + + { + LOCK(activeMasternodeInfoCs); + if (activeMasternodeInfo.blsKeyOperator) { + signSuccess = vote.Sign(*activeMasternodeInfo.blsKeyOperator); + } } if (!signSuccess) { diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 006f92309db9..bf3c6882b612 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -233,11 +233,15 @@ static UniValue masternode_status(const JSONRPCRequest& request) UniValue mnObj(UniValue::VOBJ); - // keep compatibility with legacy status for now (might get deprecated/removed later) - mnObj.pushKV("outpoint", activeMasternodeInfo.outpoint.ToStringShort()); - mnObj.pushKV("service", activeMasternodeInfo.service.ToString()); + CDeterministicMNCPtr dmn; + { + LOCK(activeMasternodeInfoCs); - auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(activeMasternodeInfo.proTxHash); + // keep compatibility with legacy status for now (might get deprecated/removed later) + mnObj.pushKV("outpoint", activeMasternodeInfo.outpoint.ToStringShort()); + mnObj.pushKV("service", activeMasternodeInfo.service.ToString()); + dmn = deterministicMNManager->GetListAtChainTip().GetMN(activeMasternodeInfo.proTxHash); + } if (dmn) { mnObj.pushKV("proTxHash", dmn->proTxHash.ToString()); mnObj.pushKV("collateralHash", dmn->collateralOutpoint.hash.ToString()); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 3ccd94a1ef97..5f6bf5a3787a 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -447,9 +447,8 @@ static UniValue mnauth(const JSONRPCRequest& request) } bool fSuccess = g_connman->ForNode(nodeId, CConnman::AllNodes, [&](CNode* pNode){ - LOCK(pNode->cs_mnauth); - pNode->verifiedProRegTxHash = proTxHash; - pNode->verifiedPubKeyHash = publicKey.GetHash(); + pNode->SetVerifiedProRegTxHash(proTxHash); + pNode->SetVerifiedPubKeyHash(publicKey.GetHash()); return true; }); diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index 1179075d1141..363ee23c9b9f 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -981,8 +981,6 @@ static UniValue protx_list(const JSONRPCRequest& request) g_txindex->BlockUntilSyncedToCurrentChain(); } - LOCK(cs_main); - if (type == "wallet") { if (!pwallet) { throw std::runtime_error("\"protx list wallet\" not supported when wallet is disabled"); diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index f7fc2f296ec1..a57d3031f3a6 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -191,6 +191,7 @@ static UniValue quorum_dkgstatus(const JSONRPCRequest& request) } int tipHeight = pindexTip->nHeight; + auto proTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash); UniValue minableCommitments(UniValue::VOBJ); UniValue quorumConnections(UniValue::VOBJ); for (const auto& type : llmq::CLLMQUtils::GetEnabledQuorumTypes(pindexTip)) { @@ -202,12 +203,13 @@ static UniValue quorum_dkgstatus(const JSONRPCRequest& request) LOCK(cs_main); pindexQuorum = chainActive[tipHeight - (tipHeight % llmq_params.dkgInterval)]; } - auto allConnections = llmq::CLLMQUtils::GetQuorumConnections(llmq_params.type, pindexQuorum, activeMasternodeInfo.proTxHash, false); - auto outboundConnections = llmq::CLLMQUtils::GetQuorumConnections(llmq_params.type, pindexQuorum, activeMasternodeInfo.proTxHash, true); + auto allConnections = llmq::CLLMQUtils::GetQuorumConnections(llmq_params.type, pindexQuorum, proTxHash, false); + auto outboundConnections = llmq::CLLMQUtils::GetQuorumConnections(llmq_params.type, pindexQuorum, proTxHash, true); std::map foundConnections; g_connman->ForEachNode([&](const CNode* pnode) { - if (!pnode->verifiedProRegTxHash.IsNull() && allConnections.count(pnode->verifiedProRegTxHash)) { - foundConnections.emplace(pnode->verifiedProRegTxHash, pnode->addr); + auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash(); + if (!verifiedProRegTxHash.IsNull() && allConnections.count(verifiedProRegTxHash)) { + foundConnections.emplace(verifiedProRegTxHash, pnode->addr); } }); UniValue arr(UniValue::VARR); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 7712037eab8c..88cfb1ff7f6d 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -25,7 +25,7 @@ #include static CCriticalSection cs_rpcWarmup; -static bool fRPCRunning = false; +static std::atomic fRPCRunning{false}; static bool fRPCInWarmup GUARDED_BY(cs_rpcWarmup) = true; static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server started"; /* Timer-creating functions */ diff --git a/src/support/allocators/mt_pooled_secure.h b/src/support/allocators/mt_pooled_secure.h index f6f891096c19..38bd003e2ce8 100644 --- a/src/support/allocators/mt_pooled_secure.h +++ b/src/support/allocators/mt_pooled_secure.h @@ -63,7 +63,6 @@ struct mt_pooled_secure_allocator : public std::allocator { private: size_t get_bucket() { - auto tid = std::this_thread::get_id(); size_t x = std::hash{}(std::this_thread::get_id()); return x % pools.size(); } diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index a96824db2a0d..4d89c295415c 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -517,7 +517,6 @@ BOOST_FIXTURE_TEST_CASE(dip3_test_mempool_reorg, TestChainDIP3Setup) BOOST_FIXTURE_TEST_CASE(dip3_test_mempool_dual_proregtx, TestChainDIP3Setup) { - int nHeight = chainActive.Height(); auto utxos = BuildSimpleUtxoMap(m_coinbase_txns); // Create a MN diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 214ae6d12605..86b4dc4ed3d6 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -531,6 +531,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) SetMockTime(0); mempool.clear(); + LOCK(::mempool.cs); TestPackageSelection(chainparams, scriptPubKey, txFirst); fCheckpointsEnabled = true; diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp index cfc240198752..3fcf2e4e20f5 100644 --- a/src/test/test_dash.cpp +++ b/src/test/test_dash.cpp @@ -31,7 +31,7 @@ const std::function G_TRANSLATION_FUN = nullptr; void CConnmanTest::AddNode(CNode& node) { - LOCK(g_connman->cs_vNodes); + LOCK2(g_connman->cs_vNodes, node.cs_hSocket); g_connman->vNodes.push_back(&node); g_connman->mapSocketToNode.emplace(node.hSocket, &node); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index d729d8106386..e438f3a668a2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -860,6 +860,7 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) // Remove TXs that refer to a MN for which the collateral was spent auto removeSpentCollateralConflict = [&](const uint256& proTxHash) { // Can't use equal_range here as every call to removeRecursive might invalidate iterators + AssertLockHeld(cs); while (true) { auto it = mapProTxRefs.find(proTxHash); if (it == mapProTxRefs.end()) { @@ -1248,6 +1249,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { LOCK(cs); auto hasKeyChangeInMempool = [&](const uint256& proTxHash) { + AssertLockHeld(cs); for (auto its = mapProTxRefs.equal_range(proTxHash); its.first != its.second; ++its.first) { auto txit = mapTx.find(its.first->second); if (txit == mapTx.end()) { diff --git a/src/txmempool.h b/src/txmempool.h index f3bda9f05321..107c91301d0d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -581,12 +581,12 @@ class CTxMemPool void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId); - void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSPublicKey &pubKey); - void removeProTxCollateralConflicts(const CTransaction &tx, const COutPoint &collateralOutpoint); - void removeProTxSpentCollateralConflicts(const CTransaction &tx); - void removeProTxKeyChangedConflicts(const CTransaction &tx, const uint256& proTxHash, const uint256& newKeyHash); - void removeProTxConflicts(const CTransaction &tx); + void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSPublicKey &pubKey) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeProTxCollateralConflicts(const CTransaction &tx, const COutPoint &collateralOutpoint) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeProTxSpentCollateralConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeProTxKeyChangedConflicts(const CTransaction &tx, const uint256& proTxHash, const uint256& newKeyHash) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeProTxConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight); void clear(); diff --git a/src/util/system.cpp b/src/util/system.cpp index 4ddc1ff77ad8..e8c2602514de 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -238,7 +238,7 @@ class ArgsManagerHelper { } /** Convert regular argument into the network-specific setting */ - static inline std::string NetworkArg(const ArgsManager& am, const std::string& arg) + static inline std::string NetworkArg(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) { assert(arg.length() > 1 && arg[0] == '-'); return "-" + am.m_network + "." + arg.substr(1); diff --git a/src/validation.cpp b/src/validation.cpp index 5a027ba8abc7..4249f9ed621f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -197,7 +197,7 @@ class CChainState { bool ReplayBlocks(const CChainParams& params, CCoinsView* view); bool LoadGenesisBlock(const CChainParams& chainparams); - bool AddGenesisBlock(const CChainParams& chainparams, const CBlock& block, CValidationState& state); + bool AddGenesisBlock(const CChainParams& chainparams, const CBlock& block, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void PruneBlockIndexCandidates(); @@ -693,8 +693,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool auto itConflicting = pool.mapNextTx.find(txin.prevout); if (itConflicting != pool.mapNextTx.end()) { - const CTransaction *ptxConflicting = itConflicting->second; - // Transaction conflicts with mempool and RBF doesn't exist in Dash return state.Invalid(false, REJECT_DUPLICATE, "txn-mempool-conflict"); } @@ -824,7 +822,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. PrecomputedTransactionData txdata(tx); - if (!CheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true, false, txdata)) + if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, txdata)) return false; // state filled in by CheckInputs // Check again against the current block tip's script verification @@ -1235,7 +1233,6 @@ bool IsInitialBlockDownload() return false; if (fImporting || fReindex) return true; - const CChainParams& chainParams = Params(); if (chainActive.Tip() == nullptr) return true; if (chainActive.Tip()->nChainWork < nMinimumChainWork) @@ -1366,7 +1363,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c CheckForkWarningConditions(); } -void static ConflictingChainFound(CBlockIndex* pindexNew) +void static ConflictingChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { statsClient.inc("warnings.ConflictingChainFound", 1.0f); @@ -2353,7 +2350,6 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl } else { // The node which relayed this should switch to correct chain. // TODO: relay instantsend data/proof. - LOCK(cs_main); return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "conflict-tx-lock"); } diff --git a/src/validation.h b/src/validation.h index f16f3c299037..fbdab0401606 100644 --- a/src/validation.h +++ b/src/validation.h @@ -306,7 +306,7 @@ uint64_t CalculateCurrentUsage(); /** * Mark one block file as pruned. */ -void PruneOneBlockFile(const int fileNumber); +void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Actually unlink the specified files diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index dffcb5cc0c6e..c07fc7062eb2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4367,7 +4367,6 @@ UniValue importwallet(const JSONRPCRequest& request); UniValue importprunedfunds(const JSONRPCRequest& request); UniValue removeprunedfunds(const JSONRPCRequest& request); UniValue importmulti(const JSONRPCRequest& request); -UniValue rescanblockchain(const JSONRPCRequest& request); UniValue dumphdinfo(const JSONRPCRequest& request); UniValue importelectrumwallet(const JSONRPCRequest& request); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 05095c4b9c80..c6ccd1d203db 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2321,6 +2321,7 @@ std::set CWalletTx::GetConflicts() const std::set result; if (pwallet != nullptr) { + AssertLockHeld(pwallet->cs_wallet); uint256 myHash = GetHash(); result = pwallet->GetConflicts(myHash); result.erase(myHash); @@ -3588,7 +3589,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac assert(txNew.nLockTime < LOCKTIME_THRESHOLD); FeeCalculation feeCalc; CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this, ::feeEstimator); - unsigned int nBytes; + unsigned int nBytes{0}; { std::vector vecCoins; LOCK2(cs_main, mempool.cs); @@ -4053,13 +4054,10 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) fFirstRunRet = mapKeys.empty() && mapHdPubKeys.empty() && mapCryptedKeys.empty() && mapWatchKeys.empty() && setWatchOnly.empty() && mapScripts.empty() && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } - { - LOCK2(cs_main, cs_wallet); - for (auto& pair : mapWallet) { - for(unsigned int i = 0; i < pair.second.tx->vout.size(); ++i) { - if (IsMine(pair.second.tx->vout[i]) && !IsSpent(pair.first, i)) { - setWalletUTXO.insert(COutPoint(pair.first, i)); - } + for (auto& pair : mapWallet) { + for(unsigned int i = 0; i < pair.second.tx->vout.size(); ++i) { + if (IsMine(pair.second.tx->vout[i]) && !IsSpent(pair.first, i)) { + setWalletUTXO.insert(COutPoint(pair.first, i)); } } } @@ -4379,11 +4377,14 @@ bool CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRe void CWallet::KeepKey(int64_t nIndex) { // Remove from key pool - WalletBatch batch(*database); - if (batch.ErasePool(nIndex)) - --nKeysLeftSinceAutoBackup; - if (!nWalletBackups) - nKeysLeftSinceAutoBackup = 0; + { + LOCK(cs_wallet); + WalletBatch batch(*database); + if (batch.ErasePool(nIndex)) + --nKeysLeftSinceAutoBackup; + if (!nWalletBackups) + nKeysLeftSinceAutoBackup = 0; + } WalletLogPrintf("keypool keep %d\n", nIndex); } @@ -5534,8 +5535,6 @@ void CMerkleTx::SetMerkleBranch(const CBlockIndex* pindex, int posInBlock) int CMerkleTx::GetDepthInMainChain() const { - int nResult; - if (hashUnset()) return 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 042d9f761017..600c1887ec27 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -487,8 +487,8 @@ class CWalletTx : public CMerkleTx CAmount GetImmatureWatchOnlyCredit(const bool fUseCache=true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); CAmount GetChange() const; - CAmount GetAnonymizedCredit(const CCoinControl* coinControl = nullptr) const; - CAmount GetDenominatedCredit(bool unconfirmed, bool fUseCache=true) const; + CAmount GetAnonymizedCredit(const CCoinControl* coinControl = nullptr) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CAmount GetDenominatedCredit(bool unconfirmed, bool fUseCache=true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Get the marginal bytes if spending the specified output from this transaction int GetSpendSize(unsigned int out, bool use_max_sig = false) const