From 350a5ca47c74a54c1db4fb3b63029ad6c43e4e61 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <knstqq@gmail.com>
Date: Sun, 6 Oct 2024 21:13:26 +0700
Subject: [PATCH] refactor: drop CSuperblock::GetGovernanceObject to simplify
 thread safety analysis over FindGovernanceObject

---
 src/governance/classes.cpp    | 21 +++++++--------------
 src/governance/classes.h      |  4 ++--
 src/governance/governance.cpp |  2 +-
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp
index 70fac9b93c..3e15bd280e 100644
--- a/src/governance/classes.cpp
+++ b/src/governance/classes.cpp
@@ -237,8 +237,7 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m
             continue;
         }
 
-        CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this);
-
+        CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash());
         if (!pObj) {
             LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n");
             continue;
@@ -288,8 +287,7 @@ bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_li
             continue;
         }
 
-        const CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this);
-
+        const CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash());
         if (!pObj) {
             continue;
         }
@@ -396,7 +394,7 @@ CSuperblock::
     nStatus(SeenObjectStatus::Unknown),
     vecPayments()
 {
-    const CGovernanceObject* pGovObj = GetGovernanceObject(govman);
+    const CGovernanceObject* pGovObj = govman.FindGovernanceObject(nGovObjHash);
 
     if (!pGovObj) {
         throw std::runtime_error("CSuperblock: Failed to find Governance Object");
@@ -434,14 +432,6 @@ CSuperblock::CSuperblock(int nBlockHeight, std::vector<CGovernancePayment> vecPa
     nGovObjHash = GetHash();
 }
 
-CGovernanceObject* CSuperblock::GetGovernanceObject(CGovernanceManager& govman)
-{
-    AssertLockHeld(govman.cs);
-    CGovernanceObject* pObj = govman.FindGovernanceObject(nGovObjHash);
-    return pObj;
-}
-
-
 /**
  *   Is Valid Superblock Height
  *
@@ -592,6 +582,8 @@ CAmount CSuperblock::GetPaymentsTotalAmount()
 
 bool CSuperblock::IsValid(CGovernanceManager& govman, const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
 {
+    AssertLockHeld(govman.cs);
+
     // TODO : LOCK(cs);
     // No reason for a lock here now since this method only accesses data
     // internal to *this and since CSuperblock's are accessed only through
@@ -608,8 +600,9 @@ bool CSuperblock::IsValid(CGovernanceManager& govman, const CChain& active_chain
     int nPayments = CountPayments();
     int nMinerAndMasternodePayments = nOutputs - nPayments;
 
+    const CGovernanceObject* obj = govman.FindGovernanceObject(nGovObjHash);
     LogPrint(BCLog::GOBJECT, "CSuperblock::IsValid -- nOutputs = %d, nPayments = %d, GetDataAsHexString = %s\n",
-        nOutputs, nPayments, GetGovernanceObject(govman)->GetDataAsHexString());
+             nOutputs, nPayments, obj ? obj->GetDataAsHexString() : "");
 
     // We require an exact match (including order) between the expected
     // superblock payments and the payments actually in the block.
diff --git a/src/governance/classes.h b/src/governance/classes.h
index 1d5e610c5d..1e393f08c0 100644
--- a/src/governance/classes.h
+++ b/src/governance/classes.h
@@ -94,13 +94,13 @@ public:
     // TELL THE ENGINE WE EXECUTED THIS EVENT
     void SetExecuted() { nStatus = SeenObjectStatus::Executed; }
 
-    CGovernanceObject* GetGovernanceObject(CGovernanceManager& govman);
-
     int GetBlockHeight() const
     {
         return nBlockHeight;
     }
 
+    const uint256 GetGovernanceObjHash() const { return nGovObjHash; }
+
     int CountPayments() const { return (int)vecPayments.size(); }
     bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet);
     CAmount GetPaymentsTotalAmount();
diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp
index 932f5b3726..39a0ca21ac 100644
--- a/src/governance/governance.cpp
+++ b/src/governance/governance.cpp
@@ -795,7 +795,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover
     // Vote NO-FUNDING for the rest of the active triggers
     const auto activeTriggers = GetActiveTriggers();
     for (const auto& trigger : activeTriggers) {
-        const auto govobj = trigger->GetGovernanceObject(*this);
+        const auto govobj = FindGovernanceObject(trigger->GetGovernanceObjHash());
         const uint256 trigger_hash = govobj->GetHash();
         if (trigger->GetBlockHeight() <= nCachedBlockHeight) {
             // ignore triggers from the past