lot's of mutex changes

src/llmq/blockprocessor.h

@@ -35,7 +35,7 @@ class CQuorumBlockProcessor
  CEvoDB& evoDb;
 
  // TODO cleanup
- mutable CCriticalSection minableCommitmentsCs;
+ mutable Mutex minableCommitmentsCs;
  std::map<std::pair<Consensus::LLMQType, uint256>, uint256> minableCommitmentsByQuorum GUARDED_BY(minableCommitmentsCs);
  std::map<uint256, CFinalCommitment> minableCommitments GUARDED_BY(minableCommitmentsCs);
 

src/llmq/debug.h

@@ -89,7 +89,7 @@ class CDKGDebugStatus
 class CDKGDebugManager
 {
 private:
- mutable CCriticalSection cs;
+ mutable Mutex cs;
  CDKGDebugStatus localStatus GUARDED_BY(cs);
 
 public:

src/llmq/dkgsession.cpp

@@ -33,8 +33,8 @@ namespace llmq
 // - commit-omit
 // - commit-lie
 
-static CCriticalSection cs_simDkgError;
-static std::map<std::string, double> simDkgErrorMap;
+static Mutex cs_simDkgError;
+static std::map<std::string, double> simDkgErrorMap GUARDED_BY(cs_simDkgError);
 
 void SetSimulatedDKGErrorRate(const std::string& type, double rate)
 {
@@ -65,7 +65,6 @@ CDKGMember::CDKGMember(const CDeterministicMNCPtr& _dmn, size_t _idx) :
  idx(_idx),
  id(_dmn->proTxHash)
 {
-
 }
 
 bool CDKGSession::Init(const CBlockIndex* _pQuorumBaseBlockIndex, const std::vector<CDeterministicMNCPtr>& mns, const uint256& _myProTxHash)

src/llmq/dkgsessionhandler.h

@@ -43,7 +43,7 @@ class CDKGPendingMessages
  using BinaryMessage = std::pair<NodeId, std::shared_ptr<CDataStream>>;
 
 private:
- mutable CCriticalSection cs;
+ mutable Mutex cs;
  const int invType;
  size_t maxMessagesPerNode GUARDED_BY(cs);
  std::list<BinaryMessage> pendingMessages GUARDED_BY(cs);
@@ -54,10 +54,10 @@ class CDKGPendingMessages
  explicit CDKGPendingMessages(size_t _maxMessagesPerNode, int _invType) :
  invType(_invType), maxMessagesPerNode(_maxMessagesPerNode) {};
 
- void PushPendingMessage(NodeId from, CDataStream& vRecv);
- std::list<BinaryMessage> PopPendingMessages(size_t maxCount);
- bool HasSeen(const uint256& hash) const;
- void Clear();
+ void PushPendingMessage(NodeId from, CDataStream& vRecv) LOCKS_EXCLUDED(cs);
+ std::list<BinaryMessage> PopPendingMessages(size_t maxCount) LOCKS_EXCLUDED(cs);
+ bool HasSeen(const uint256& hash) const LOCKS_EXCLUDED(cs);
+ void Clear() LOCKS_EXCLUDED(cs);
 
  template<typename Message>
  void PushPendingMessage(NodeId from, Message& msg)

src/llmq/dkgsessionmgr.cpp

@@ -306,7 +306,6 @@ void CDKGSessionManager::WriteEncryptedContributions(Consensus::LLMQType llmqTyp
 
 bool CDKGSessionManager::GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector<bool>& validMembers, std::vector<uint16_t>& memberIndexesRet, std::vector<BLSVerificationVectorPtr>& vvecsRet, BLSSecretKeyVector& skContributionsRet) const
 {
- LOCK(contributionsCacheCs);
  auto members = CLLMQUtils::GetAllQuorumMembers(GetLLMQParams(llmqType), pQuorumBaseBlockIndex);
 
  memberIndexesRet.clear();
@@ -315,6 +314,8 @@ bool CDKGSessionManager::GetVerifiedContributions(Consensus::LLMQType llmqType,
  memberIndexesRet.reserve(members.size());
  vvecsRet.reserve(members.size());
  skContributionsRet.reserve(members.size());
+
+ LOCK(contributionsCacheCs);
  for (size_t i = 0; i < members.size(); i++) {
  if (validMembers[i]) {
  const uint256& proTxHash = members[i]->proTxHash;
@@ -371,8 +372,8 @@ bool CDKGSessionManager::GetEncryptedContributions(Consensus::LLMQType llmqType,
 
 void CDKGSessionManager::CleanupCache() const
 {
- LOCK(contributionsCacheCs);
  auto curTime = GetTimeMillis();
+ LOCK(contributionsCacheCs);
  for (auto it = contributionsCache.begin(); it != contributionsCache.end(); ) {
  if (curTime - it->second.entryTime > MAX_CONTRIBUTION_CACHE_TIME) {
  it = contributionsCache.erase(it);

src/llmq/dkgsessionmgr.h

@@ -26,7 +26,6 @@ class CDKGSessionManager
 
  std::map<Consensus::LLMQType, CDKGSessionHandler> dkgSessionHandlers;
 
- mutable CCriticalSection contributionsCacheCs;
  struct ContributionsCacheKey {
  Consensus::LLMQType llmqType;
  uint256 quorumHash;
@@ -43,6 +42,7 @@ class CDKGSessionManager
  BLSVerificationVectorPtr vvec;
  CBLSSecretKey skContribution;
  };
+ mutable Mutex contributionsCacheCs;
  mutable std::map<ContributionsCacheKey, ContributionsCacheEntry> contributionsCache GUARDED_BY(contributionsCacheCs);
 
 public:
@@ -64,15 +64,18 @@ class CDKGSessionManager
  // Contributions are written while in the DKG
  void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec);
  void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution);
- bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector<bool>& validMembers, std::vector<uint16_t>& memberIndexesRet, std::vector<BLSVerificationVectorPtr>& vvecsRet, BLSSecretKeyVector& skContributionsRet) const;
+ bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex,
+ const std::vector<bool>& validMembers, std::vector<uint16_t>& memberIndexesRet,
+ std::vector<BLSVerificationVectorPtr>& vvecsRet,
+ BLSSecretKeyVector& skContributionsRet) const LOCKS_EXCLUDED(contributionsCacheCs);
  /// Write encrypted (unverified) DKG contributions for the member with the given proTxHash to the llmqDb
  void WriteEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSIESMultiRecipientObjects<CBLSSecretKey>& contributions);
  /// Read encrypted (unverified) DKG contributions for the member with the given proTxHash from the llmqDb
  bool GetEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector<bool>& validMembers, const uint256& proTxHash, std::vector<CBLSIESEncryptedObject<CBLSSecretKey>>& vecRet) const;
 
 private:
  void MigrateDKG();
- void CleanupCache() const;
+ void CleanupCache() const LOCKS_EXCLUDED(contributionsCacheCs);
 };
 
 bool IsQuorumDKGEnabled();

src/llmq/quorums.cpp

@@ -31,7 +31,7 @@ static const std::string DB_QUORUM_QUORUM_VVEC = "q_Qqvvec";
 
 CQuorumManager* quorumManager;
 
-CCriticalSection cs_data_requests;
+Mutex cs_data_requests;
 static std::unordered_map<std::pair<uint256, bool>, CQuorumDataRequest, StaticSaltedHasher> mapQuorumDataRequests GUARDED_BY(cs_data_requests);
 
 static uint256 MakeQuorumKey(const CQuorum& q)
@@ -285,7 +285,6 @@ void CQuorumManager::EnsureQuorumConnections(const Consensus::LLMQParams& llmqPa
 
 CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const
 {
- AssertLockHeld(quorumsCacheCs);
  assert(pQuorumBaseBlockIndex);
 
  const uint256& quorumHash{pQuorumBaseBlockIndex->GetBlockHash()};
@@ -321,7 +320,7 @@ CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType l
  StartCachePopulatorThread(quorum);
  }
 
- mapQuorumsCache[llmqType].insert(quorumHash, quorum);
+ WITH_LOCK(quorumsCacheCs, mapQuorumsCache[llmqType].insert(quorumHash, quorum));
 
  return quorum;
 }
@@ -336,20 +335,22 @@ bool CQuorumManager::BuildQuorumContributions(const CFinalCommitmentPtr& fqc, co
  }
 
  cxxtimer::Timer t2(true);
- LOCK(quorum->cs);
- quorum->quorumVvec = blsWorker.BuildQuorumVerificationVector(vvecs);
- 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)
- return false;
- }
- quorum->skShare = blsWorker.AggregateSecretKeys(skContributions);
- if (!quorum->skShare.IsValid()) {
- quorum->skShare.Reset();
- LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- failed to build skShare\n", __func__);
- // We don't bail out here as this is not a fatal error and still allows us to recover public key shares (as we
- // have a valid quorum vvec at this point)
+ {
+ LOCK(quorum->cs);
+ quorum->quorumVvec = blsWorker.BuildQuorumVerificationVector(vvecs);
+ 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)
+ return false;
+ }
+ quorum->skShare = blsWorker.AggregateSecretKeys(skContributions);
+ if (!quorum->skShare.IsValid()) {
+ quorum->skShare.Reset();
+ LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- failed to build skShare\n", __func__);
+ // We don't bail out here as this is not a fatal error and still allows us to recover public key shares (as we
+ // have a valid quorum vvec at this point)
+ }
  }
  t2.stop();
 
@@ -493,9 +494,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const CBlock
  return nullptr;
  }
 
- LOCK(quorumsCacheCs);
  CQuorumPtr pQuorum;
- if (mapQuorumsCache[llmqType].get(quorumHash, pQuorum)) {
+ if (WITH_LOCK(quorumsCacheCs, return mapQuorumsCache[llmqType].get(quorumHash, pQuorum))) {
  return pQuorum;
  }
 
@@ -649,12 +649,9 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand,
  }
 
  CQuorumPtr pQuorum;
- {
- LOCK(quorumsCacheCs);
- if (!mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash(), pQuorum)) {
- errorHandler("Quorum not found", 0); // Don't bump score because we asked for it
- return;
- }
+ if (LOCK(quorumsCacheCs); !mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash(), pQuorum)) {
+ errorHandler("Quorum not found", 0); // Don't bump score because we asked for it
+ return;
  }
 
  // Check if request has QUORUM_VERIFICATION_VECTOR data

src/llmq/quorums.h

@@ -191,7 +191,7 @@ class CQuorumManager
  CBLSWorker& blsWorker;
  CDKGSessionManager& dkgManager;
 
- mutable CCriticalSection quorumsCacheCs;
+ mutable Mutex quorumsCacheCs;
  mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, CQuorumPtr, StaticSaltedHasher>> mapQuorumsCache GUARDED_BY(quorumsCacheCs);
  mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, std::vector<CQuorumCPtr>, StaticSaltedHasher>> scanQuorumsCache GUARDED_BY(quorumsCacheCs);
 
@@ -226,7 +226,7 @@ class CQuorumManager
  // all private methods here are cs_main-free
  void EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const;
 
- CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const EXCLUSIVE_LOCKS_REQUIRED(quorumsCacheCs);
+ CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const;
  bool BuildQuorumContributions(const CFinalCommitmentPtr& fqc, const std::shared_ptr<CQuorum>& quorum) const;
 
  CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindex) const;

src/llmq/signing.cpp

@@ -358,8 +358,6 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
 
 void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey)
 {
- AssertLockHeld(cs);
-
  CRecoveredSig recSig;
  if (!ReadRecoveredSig(llmqType, id, recSig)) {
  return;
@@ -389,6 +387,7 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
  }
  }
 
+ LOCK(cs);
  hasSigForIdCache.erase(std::make_pair(recSig.llmqType, recSig.id));
  hasSigForSessionCache.erase(signHash);
  if (deleteHashKey) {
@@ -399,7 +398,6 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
 // Completely remove any traces of the recovered sig
 void CRecoveredSigsDb::RemoveRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
 {
- LOCK(cs);
  CDBBatch batch(*db);
  RemoveRecoveredSig(batch, llmqType, id, true, true);
  db->WriteBatch(batch);
@@ -409,7 +407,6 @@ void CRecoveredSigsDb::RemoveRecoveredSig(Consensus::LLMQType llmqType, const ui
 // This will leave the byHash key in-place so that HasRecoveredSigForHash still returns true
 void CRecoveredSigsDb::TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
 {
- LOCK(cs);
  CDBBatch batch(*db);
  RemoveRecoveredSig(batch, llmqType, id, false, false);
  db->WriteBatch(batch);
@@ -448,15 +445,12 @@ void CRecoveredSigsDb::CleanupOldRecoveredSigs(int64_t maxAge)
  }
 
  CDBBatch batch(*db);
- {
- LOCK(cs);
- for (const auto& e : toDelete) {
- RemoveRecoveredSig(batch, e.first, e.second, true, false);
+ for (const auto& e : toDelete) {
+ RemoveRecoveredSig(batch, e.first, e.second, true, false);
 
- if (batch.SizeEstimate() >= (1 << 24)) {
- db->WriteBatch(batch);
- batch.Clear();
- }
+ if (batch.SizeEstimate() >= (1 << 24)) {
+ db->WriteBatch(batch);
+ batch.Clear();
  }
  }
 

src/llmq/signing.h

@@ -78,7 +78,7 @@ class CRecoveredSigsDb
 private:
  std::unique_ptr<CDBWrapper> db{nullptr};
 
- mutable CCriticalSection cs;
+ mutable Mutex cs;
  mutable unordered_lru_cache<std::pair<Consensus::LLMQType, uint256>, bool, StaticSaltedHasher, 30000> hasSigForIdCache GUARDED_BY(cs);
  mutable unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForSessionCache GUARDED_BY(cs);
  mutable unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForHashCache GUARDED_BY(cs);
@@ -113,7 +113,7 @@ class CRecoveredSigsDb
  void MigrateRecoveredSigs();
 
  bool ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const;
- void RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey) EXCLUSIVE_LOCKS_REQUIRED(cs);
+ void RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey);
 };
 
 class CRecoveredSigsListener
@@ -134,10 +134,10 @@ class CSigningManager
  static constexpr int SIGN_HEIGHT_OFFSET{8};
 
 private:
- mutable CCriticalSection cs;
-
  CRecoveredSigsDb db;
 
+ mutable CCriticalSection cs;
+
  // Incoming and not verified yet
  std::unordered_map<NodeId, std::list<std::shared_ptr<const CRecoveredSig>>> pendingRecoveredSigs GUARDED_BY(cs);
  std::unordered_map<uint256, std::shared_ptr<const CRecoveredSig>, StaticSaltedHasher> pendingReconstructedRecoveredSigs GUARDED_BY(cs);