fix: resolve lock order issues

Author: kwvg

src/governance/governance.cpp

@@ -320,7 +320,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
             return ret;
         }
 
-        AddGovernanceObject(govobj, &peer);
+        InternalAddGovernanceObject(govobj, &peer);
         return ret;
     }
 
@@ -398,9 +398,12 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj)
     }
 }
 
-void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom)
+void CGovernanceManager::InternalAddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom)
 {
+    AssertLockHeld(::cs_main);
+    AssertLockHeld(cs_store);
     AssertLockNotHeld(cs_relay);
+
     uint256 nHash = govobj.GetHash();
     std::string strHash = nHash.ToString();
 
@@ -410,7 +413,6 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN
 
     govobj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object
 
-    LOCK2(::cs_main, cs_store);
     std::string strError;
 
     // MAKE SURE THIS OBJECT IS OK
@@ -459,6 +461,15 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN
     GetMainSignals().NotifyGovernanceObject(std::make_shared<const Governance::Object>(govobj.Object()), nHash.ToString());
 }
 
+void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom)
+{
+    AssertLockNotHeld(cs_store);
+    AssertLockNotHeld(cs_relay);
+
+    LOCK2(::cs_main, cs_store);
+    InternalAddGovernanceObject(govobj, pfrom);
+}
+
 void CGovernanceManager::CheckAndRemove()
 {
     AssertLockNotHeld(cs_store);
@@ -989,12 +1000,11 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
 
 void CGovernanceManager::CheckPostponedObjects()
 {
+    AssertLockHeld(::cs_main);
     AssertLockHeld(cs_store);
     AssertLockNotHeld(cs_relay);
     if (!m_mn_sync.IsSynced()) return;
 
-    LOCK(::cs_main);
-
     // Check postponed proposals
     for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) {
         const uint256& nHash = it->first;
@@ -1006,7 +1016,7 @@ void CGovernanceManager::CheckPostponedObjects()
         bool fMissingConfirmations;
         if (govobj.IsCollateralValid(m_chainman, strError, fMissingConfirmations)) {
             if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), m_chainman, strError, false)) {
-                AddGovernanceObject(govobj);
+                InternalAddGovernanceObject(govobj);
             } else {
                 LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString());
             }
@@ -1379,7 +1389,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex)
     nCachedBlockHeight = pindex->nHeight;
     LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight);
 
-    LOCK(cs_store);
+    LOCK2(::cs_main, cs_store);
     if (DeploymentDIP0003Enforced(pindex->nHeight, Params().GetConsensus())) {
         RemoveInvalidVotes();
     }

src/governance/governance.h

@@ -54,6 +54,8 @@ using vote_time_pair_t = std::pair<CGovernanceVote, int64_t>;
 static constexpr int RATE_BUFFER_SIZE = 5;
 static constexpr bool DEFAULT_GOVERNANCE_ENABLE{true};
 
+extern RecursiveMutex cs_main;
+
 class CRateCheckBuffer
 {
 private:
@@ -340,7 +342,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     std::vector<std::shared_ptr<const CGovernanceObject>> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
     void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay);
 
     // Superblocks
     bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight,
@@ -398,14 +400,19 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     const CGovernanceObject* InternalFindConstGovernanceObject(const uint256& nHash) const
         EXCLUSIVE_LOCKS_REQUIRED(cs_store);
 
+    // Internal counterpart to "Signer interface"
+    void InternalAddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr)
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main, cs_store, !cs_relay);
+
+    // ...
     void MasternodeRateUpdate(const CGovernanceObject& govobj)
         EXCLUSIVE_LOCKS_REQUIRED(cs_store);
 
     bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed)
         EXCLUSIVE_LOCKS_REQUIRED(cs_store);
 
     void CheckPostponedObjects()
-        EXCLUSIVE_LOCKS_REQUIRED(cs_store, !cs_relay);
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main, cs_store, !cs_relay);
 
     void InitOnLoad()
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);