refactor: Add a bunch of GUARDED_BY(*) in llmq code (#4121)

src/llmq/quorums.h

@@ -205,8 +205,8 @@ private:
     CDKGSessionManager& dkgManager;
 
     mutable CCriticalSection quorumsCacheCs;
-    mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, CQuorumPtr, StaticSaltedHasher>> mapQuorumsCache;
-    mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, std::vector<CQuorumCPtr>, StaticSaltedHasher>> scanQuorumsCache;
+    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);
 
     mutable ctpl::thread_pool workerPool;
     mutable CThreadInterrupt quorumThreadInterrupt;

src/llmq/quorums_blockprocessor.h

@@ -29,10 +29,10 @@ private:
 
     // TODO cleanup
     CCriticalSection minableCommitmentsCs;
-    std::map<std::pair<Consensus::LLMQType, uint256>, uint256> minableCommitmentsByQuorum;
-    std::map<uint256, CFinalCommitment> minableCommitments;
+    std::map<std::pair<Consensus::LLMQType, uint256>, uint256> minableCommitmentsByQuorum GUARDED_BY(minableCommitmentsCs);
+    std::map<uint256, CFinalCommitment> minableCommitments GUARDED_BY(minableCommitmentsCs);
 
-    std::map<Consensus::LLMQType, unordered_lru_cache<uint256, bool, StaticSaltedHasher>> mapHasMinedCommitmentCache;
+    std::map<Consensus::LLMQType, unordered_lru_cache<uint256, bool, StaticSaltedHasher>> mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs);
 
 public:
     explicit CQuorumBlockProcessor(CEvoDB& _evoDb);

src/llmq/quorums_chainlocks.h

@@ -57,29 +57,29 @@ private:
     CScheduler* scheduler;
     boost::thread* scheduler_thread;
     CCriticalSection cs;
-    bool tryLockChainTipScheduled{false};
-    bool isEnabled{false};
-    bool isEnforced{false};
+    bool tryLockChainTipScheduled GUARDED_BY(cs) {false};
+    bool isEnabled GUARDED_BY(cs) {false};
+    bool isEnforced GUARDED_BY(cs) {false};
 
-    uint256 bestChainLockHash;
-    CChainLockSig bestChainLock;
+    uint256 bestChainLockHash GUARDED_BY(cs);
+    CChainLockSig bestChainLock GUARDED_BY(cs);
 
-    CChainLockSig bestChainLockWithKnownBlock;
-    const CBlockIndex* bestChainLockBlockIndex{nullptr};
-    const CBlockIndex* lastNotifyChainLockBlockIndex{nullptr};
+    CChainLockSig bestChainLockWithKnownBlock GUARDED_BY(cs);
+    const CBlockIndex* bestChainLockBlockIndex GUARDED_BY(cs) {nullptr};
+    const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs) {nullptr};
 
-    int32_t lastSignedHeight{-1};
-    uint256 lastSignedRequestId;
-    uint256 lastSignedMsgHash;
+    int32_t lastSignedHeight GUARDED_BY(cs) {-1};
+    uint256 lastSignedRequestId GUARDED_BY(cs);
+    uint256 lastSignedMsgHash GUARDED_BY(cs);
 
     // We keep track of txids from recently received blocks so that we can check if all TXs got islocked
     typedef std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>> BlockTxs;
-    BlockTxs blockTxs;
-    std::unordered_map<uint256, int64_t> txFirstSeenTime;
+    BlockTxs blockTxs GUARDED_BY(cs);
+    std::unordered_map<uint256, int64_t> txFirstSeenTime GUARDED_BY(cs);
 
-    std::map<uint256, int64_t> seenChainLocks;
+    std::map<uint256, int64_t> seenChainLocks GUARDED_BY(cs);
 
-    int64_t lastCleanupTime{0};
+    int64_t lastCleanupTime GUARDED_BY(cs) {0};
 
 public:
     explicit CChainLocksHandler();

src/llmq/quorums_debug.h

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

src/llmq/quorums_dkgsession.h

@@ -270,16 +270,16 @@ private:
     // conflicting messages as otherwise an attacker might be able to broadcast conflicting (valid+invalid) messages
     // and thus split the quorum. Such members are later removed from the quorum.
     mutable CCriticalSection invCs;
-    std::map<uint256, CDKGContribution> contributions;
-    std::map<uint256, CDKGComplaint> complaints;
-    std::map<uint256, CDKGJustification> justifications;
-    std::map<uint256, CDKGPrematureCommitment> prematureCommitments;
+    std::map<uint256, CDKGContribution> contributions GUARDED_BY(invCs);
+    std::map<uint256, CDKGComplaint> complaints GUARDED_BY(invCs);
+    std::map<uint256, CDKGJustification> justifications GUARDED_BY(invCs);
+    std::map<uint256, CDKGPrematureCommitment> prematureCommitments GUARDED_BY(invCs);
 
     mutable CCriticalSection cs_pending;
-    std::vector<size_t> pendingContributionVerifications;
+    std::vector<size_t> pendingContributionVerifications GUARDED_BY(cs_pending);
 
     // filled by ReceivePrematureCommitment and used by FinalizeCommitments
-    std::set<uint256> validCommitments;
+    std::set<uint256> validCommitments GUARDED_BY(invCs);
 
 public:
     CDKGSession(const Consensus::LLMQParams& _params, CBLSWorker& _blsWorker, CDKGSessionManager& _dkgManager) :

src/llmq/quorums_dkgsessionhandler.h

@@ -44,11 +44,11 @@ public:
 
 private:
     mutable CCriticalSection cs;
-    int invType;
-    size_t maxMessagesPerNode;
-    std::list<BinaryMessage> pendingMessages;
-    std::map<NodeId, size_t> messagesPerNode;
-    std::set<uint256> seenMessages;
+    const int invType;
+    size_t maxMessagesPerNode GUARDED_BY(cs);
+    std::list<BinaryMessage> pendingMessages GUARDED_BY(cs);
+    std::map<NodeId, size_t> messagesPerNode GUARDED_BY(cs);
+    std::set<uint256> seenMessages GUARDED_BY(cs);
 
 public:
     explicit CDKGPendingMessages(size_t _maxMessagesPerNode, int _invType);
@@ -110,17 +110,18 @@ private:
     CBLSWorker& blsWorker;
     CDKGSessionManager& dkgManager;
 
-    QuorumPhase phase{QuorumPhase_Idle};
-    int currentHeight{-1};
-    int quorumHeight{-1};
-    uint256 quorumHash;
+    QuorumPhase phase GUARDED_BY(cs) {QuorumPhase_Idle};
+    int currentHeight GUARDED_BY(cs) {-1};
+    int quorumHeight GUARDED_BY(cs) {-1};
+    uint256 quorumHash GUARDED_BY(cs);
+
     std::shared_ptr<CDKGSession> curSession;
     std::thread phaseHandlerThread;
 
-    CDKGPendingMessages pendingContributions;
-    CDKGPendingMessages pendingComplaints;
-    CDKGPendingMessages pendingJustifications;
-    CDKGPendingMessages pendingPrematureCommitments;
+    CDKGPendingMessages pendingContributions GUARDED_BY(cs);
+    CDKGPendingMessages pendingComplaints GUARDED_BY(cs);
+    CDKGPendingMessages pendingJustifications GUARDED_BY(cs);
+    CDKGPendingMessages pendingPrematureCommitments GUARDED_BY(cs);
 
 public:
     CDKGSessionHandler(const Consensus::LLMQParams& _params, CBLSWorker& blsWorker, CDKGSessionManager& _dkgManager);

src/llmq/quorums_dkgsessionmgr.h

@@ -43,7 +43,7 @@ private:
         BLSVerificationVectorPtr vvec;
         CBLSSecretKey skContribution;
     };
-    std::map<ContributionsCacheKey, ContributionsCacheEntry> contributionsCache;
+    std::map<ContributionsCacheKey, ContributionsCacheEntry> contributionsCache GUARDED_BY(contributionsCacheCs);
 
 public:
     CDKGSessionManager(CDBWrapper& _llmqDb, CBLSWorker& _blsWorker);

src/llmq/quorums_instantsend.h

@@ -90,19 +90,19 @@ private:
      * Request ids of inputs that we signed. Used to determine if a recovered signature belongs to an
      * in-progress input lock.
      */
-    std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds;
+    std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs);
 
     /**
      * These are the islocks that are currently in the middle of being created. Entries are created when we observed
      * recovered signatures for all inputs of a TX. At the same time, we initiate signing of our sigshare for the islock.
      * When the recovered sig for the islock later arrives, we can finish the islock and propagate it.
      */
-    std::unordered_map<uint256, CInstantSendLock, StaticSaltedHasher> creatingInstantSendLocks;
+    std::unordered_map<uint256, CInstantSendLock, StaticSaltedHasher> creatingInstantSendLocks GUARDED_BY(cs);
     // maps from txid to the in-progress islock
-    std::unordered_map<uint256, CInstantSendLock*, StaticSaltedHasher> txToCreatingInstantSendLocks;
+    std::unordered_map<uint256, CInstantSendLock*, StaticSaltedHasher> txToCreatingInstantSendLocks GUARDED_BY(cs);
 
     // Incoming and not verified yet
-    std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher> pendingInstantSendLocks;
+    std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher> pendingInstantSendLocks GUARDED_BY(cs);
 
     // TXs which are neither IS locked nor ChainLocked. We use this to determine for which TXs we need to retry IS locking
     // of child TXs
@@ -111,10 +111,10 @@ private:
         CTransactionRef tx;
         std::unordered_set<uint256, StaticSaltedHasher> children;
     };
-    std::unordered_map<uint256, NonLockedTxInfo, StaticSaltedHasher> nonLockedTxs;
-    std::unordered_map<COutPoint, uint256, SaltedOutpointHasher> nonLockedTxsByOutpoints;
+    std::unordered_map<uint256, NonLockedTxInfo, StaticSaltedHasher> nonLockedTxs GUARDED_BY(cs);
+    std::unordered_map<COutPoint, uint256, SaltedOutpointHasher> nonLockedTxsByOutpoints GUARDED_BY(cs);
 
-    std::unordered_set<uint256, StaticSaltedHasher> pendingRetryTxs;
+    std::unordered_set<uint256, StaticSaltedHasher> pendingRetryTxs GUARDED_BY(cs);
 
 public:
     explicit CInstantSendManager(CDBWrapper& _llmqDb);

src/llmq/quorums_signing.h

@@ -81,9 +81,9 @@ private:
     CDBWrapper& db;
 
     CCriticalSection cs;
-    unordered_lru_cache<std::pair<Consensus::LLMQType, uint256>, bool, StaticSaltedHasher, 30000> hasSigForIdCache;
-    unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForSessionCache;
-    unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForHashCache;
+    unordered_lru_cache<std::pair<Consensus::LLMQType, uint256>, bool, StaticSaltedHasher, 30000> hasSigForIdCache GUARDED_BY(cs);
+    unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForSessionCache GUARDED_BY(cs);
+    unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForHashCache GUARDED_BY(cs);
 
 public:
     explicit CRecoveredSigsDb(CDBWrapper& _db);
@@ -138,15 +138,14 @@ private:
     CRecoveredSigsDb db;
 
     // Incoming and not verified yet
-    std::unordered_map<NodeId, std::list<std::shared_ptr<const CRecoveredSig>>> pendingRecoveredSigs;
-    std::unordered_map<uint256, std::shared_ptr<const CRecoveredSig>, StaticSaltedHasher> pendingReconstructedRecoveredSigs;
+    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);
 
-    // must be protected by cs
-    FastRandomContext rnd;
+    FastRandomContext rnd GUARDED_BY(cs);
 
     int64_t lastCleanupTime{0};
 
-    std::vector<CRecoveredSigsListener*> recoveredSigsListeners;
+    std::vector<CRecoveredSigsListener*> recoveredSigsListeners GUARDED_BY(cs);
 
 public:
     CSigningManager(CDBWrapper& llmqDb, bool fMemory);

src/llmq/quorums_signing_shares.h

@@ -368,20 +368,19 @@ private:
     std::thread workThread;
     CThreadInterrupt workInterrupt;
 
-    SigShareMap<CSigShare> sigShares;
-    std::unordered_map<uint256, CSignedSession, StaticSaltedHasher> signedSessions;
+    SigShareMap<CSigShare> sigShares GUARDED_BY(cs);
+    std::unordered_map<uint256, CSignedSession, StaticSaltedHasher> signedSessions GUARDED_BY(cs);
 
     // stores time of last receivedSigShare. Used to detect timeouts
-    std::unordered_map<uint256, int64_t, StaticSaltedHasher> timeSeenForSessions;
+    std::unordered_map<uint256, int64_t, StaticSaltedHasher> timeSeenForSessions GUARDED_BY(cs);
 
-    std::unordered_map<NodeId, CSigSharesNodeState> nodeStates;
-    SigShareMap<std::pair<NodeId, int64_t>> sigSharesRequested;
-    SigShareMap<bool> sigSharesQueuedToAnnounce;
+    std::unordered_map<NodeId, CSigSharesNodeState> nodeStates GUARDED_BY(cs);
+    SigShareMap<std::pair<NodeId, int64_t>> sigSharesRequested GUARDED_BY(cs);
+    SigShareMap<bool> sigSharesQueuedToAnnounce GUARDED_BY(cs);
 
-    std::vector<std::tuple<const CQuorumCPtr, uint256, uint256>> pendingSigns;
+    std::vector<std::tuple<const CQuorumCPtr, uint256, uint256>> pendingSigns GUARDED_BY(cs);
 
-    // must be protected by cs
-    FastRandomContext rnd;
+    FastRandomContext rnd GUARDED_BY(cs);
 
     int64_t lastCleanupTime{0};
     std::atomic<uint32_t> recoveredSigsCounter{0};