From 9d647e8e3d12c60fa1ac35e898309cda41272242 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 16:58:01 +0530 Subject: [PATCH 01/18] refactor: remove unused code `votedFundingYesTriggerHash` was moved to GovernanceSigner but the old definition still lingered around, get rid of it. --- src/governance/governance.cpp | 23 ----------------------- src/governance/governance.h | 2 -- 2 files changed, 25 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 9d193de7036c..e4e7b93d7c0b 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -27,8 +27,6 @@ #include #include -int nSubmittedFinalBudget; - const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16"; namespace { @@ -80,7 +78,6 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi nCachedBlockHeight(0), mapPostponedObjects(), fRateChecksEnabled(true), - votedFundingYesTriggerHash(std::nullopt), mapTrigger{} { } @@ -596,15 +593,6 @@ CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint return nullptr; } -void CGovernanceManager::DeleteGovernanceObject(const uint256& nHash) -{ - LOCK(cs); - - if (mapObjects.count(nHash)) { - mapObjects.erase(nHash); - } -} - std::vector CGovernanceManager::GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const { LOCK(cs); @@ -660,17 +648,6 @@ void CGovernanceManager::GetAllNewerThan(std::vector& objs, i } } -// -// Sort by votes, if there's a tie sort by their feeHash TX -// -struct sortProposalsByVotes { - bool operator()(const std::pair& left, const std::pair& right) const - { - if (left.second != right.second) return (left.second > right.second); - return (UintToArith256(left.first->GetCollateralHash()) > UintToArith256(right.first->GetCollateralHash())); - } -}; - bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) { // do not request objects until it's time to sync diff --git a/src/governance/governance.h b/src/governance/governance.h index 43d3979bdcd4..bf26e812188c 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -265,7 +265,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::set setAdditionalRelayObjects; std::map m_requested_hash_time; bool fRateChecksEnabled; - std::optional votedFundingYesTriggerHash; std::map> mapTrigger; mutable Mutex cs_relay; @@ -302,7 +301,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - void DeleteGovernanceObject(const uint256& nHash); // These commands are only used in RPC std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; From 249d021a8c328575715f85e4f9d888ee8e359810 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 23 Oct 2025 05:03:27 +0530 Subject: [PATCH 02/18] refactor: guard all `GovernanceStore` vars, move `cmapVoteToObject` `cmapVoteToObject` isn't (de)ser'ed so we move it to the manager and don't consider it for the mutex --- src/governance/governance.cpp | 27 ++++++++++++++++++++------- src/governance/governance.h | 16 +++++++++------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index e4e7b93d7c0b..eae314ee378e 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -57,7 +57,6 @@ GovernanceStore::GovernanceStore() : cs(), mapObjects(), mapErasedGovernanceObjects(), - cmapVoteToObject(MAX_CACHE_SIZE), cmapInvalidVotes(MAX_CACHE_SIZE), cmmapOrphanVotes(MAX_CACHE_SIZE), mapLastMasternodeObject(), @@ -76,6 +75,7 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi m_mn_sync{mn_sync}, nTimeLastDiff(0), nCachedBlockHeight(0), + cmapVoteToObject(MAX_CACHE_SIZE), mapPostponedObjects(), fRateChecksEnabled(true), mapTrigger{} @@ -352,6 +352,8 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) { + LOCK(cs); + AssertLockNotHeld(cs_relay); uint256 nHash = govobj.GetHash(); std::vector vecVotePairs; @@ -794,6 +796,8 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) { if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; + LOCK(cs); + const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint(); auto it = mapLastMasternodeObject.find(masternodeOutpoint); @@ -1047,6 +1051,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH void CGovernanceManager::AddInvalidVote(const CGovernanceVote& vote) { + LOCK(cs); cmapInvalidVotes.Insert(vote.GetHash(), vote); } @@ -1228,16 +1233,20 @@ void CGovernanceManager::InitOnLoad() void GovernanceStore::Clear() { LOCK(cs); - - LogPrint(BCLog::GOBJECT, "Governance object manager was cleared\n"); mapObjects.clear(); mapErasedGovernanceObjects.clear(); - cmapVoteToObject.Clear(); cmapInvalidVotes.Clear(); cmmapOrphanVotes.Clear(); mapLastMasternodeObject.clear(); } +void CGovernanceManager::Clear() +{ + LogPrint(BCLog::GOBJECT, "Governance object manager was cleared\n"); + GovernanceStore::Clear(); + cmapVoteToObject.Clear(); +} + std::string GovernanceStore::ToString() const { LOCK(cs); @@ -1260,10 +1269,14 @@ std::string GovernanceStore::ToString() const } } - return strprintf("Governance Objects: %d (Proposals: %d, Triggers: %d, Other: %d; Erased: %d), Votes: %d", + return strprintf("Governance Objects: %d (Proposals: %d, Triggers: %d, Other: %d; Erased: %d)", (int)mapObjects.size(), - nProposalCount, nTriggerCount, nOtherCount, (int)mapErasedGovernanceObjects.size(), - (int)cmapVoteToObject.GetSize()); + nProposalCount, nTriggerCount, nOtherCount, (int)mapErasedGovernanceObjects.size()); +} + +std::string CGovernanceManager::ToString() const +{ + return strprintf("%s, Votes: %d", GovernanceStore::ToString(), (int)cmapVoteToObject.GetSize()); } UniValue CGovernanceManager::ToJson() const diff --git a/src/governance/governance.h b/src/governance/governance.h index bf26e812188c..d564982e10f2 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -170,7 +170,6 @@ class GovernanceStore bool fStatusOK; }; - using object_ref_cm_t = CacheMap; using txout_m_t = std::map; using vote_cmm_t = CacheMultiMap; @@ -188,13 +187,12 @@ class GovernanceStore // mapErasedGovernanceObjects contains key-value pairs, where // key - governance object's hash // value - expiration time for deleted objects - std::map mapErasedGovernanceObjects; - object_ref_cm_t cmapVoteToObject; - CacheMap cmapInvalidVotes; - vote_cmm_t cmmapOrphanVotes; - txout_m_t mapLastMasternodeObject; + std::map mapErasedGovernanceObjects GUARDED_BY(cs); + CacheMap cmapInvalidVotes GUARDED_BY(cs); + vote_cmm_t cmmapOrphanVotes GUARDED_BY(cs); + txout_m_t mapLastMasternodeObject GUARDED_BY(cs); // used to check for changed voting keys - std::shared_ptr lastMNListForVotingKeys; + std::shared_ptr lastMNListForVotingKeys GUARDED_BY(cs); public: GovernanceStore(); @@ -247,6 +245,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent private: using db_type = CFlatDB; + using object_ref_cm_t = CacheMap; private: const std::unique_ptr m_db; @@ -261,6 +260,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent int64_t nTimeLastDiff; // keep track of current block height int nCachedBlockHeight; + object_ref_cm_t cmapVoteToObject; std::map mapPostponedObjects; std::set setAdditionalRelayObjects; std::map m_requested_hash_time; @@ -309,9 +309,11 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void Clear(); void CheckAndRemove(); UniValue ToJson() const; + std::string ToString() const; void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); int64_t GetLastDiffTime() const { return nTimeLastDiff; } From 4080ee992c5e17ae5d0be5454c9ac01a9915462d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 18:01:25 +0000 Subject: [PATCH 03/18] refactor: group together public accessor functions Review with `git log -p -n1 --color-moved=dimmed_zebra`. --- src/governance/governance.h | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/governance/governance.h b/src/governance/governance.h index d564982e10f2..4f42c3f30f27 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -299,7 +299,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); // These commands are only used in RPC @@ -321,19 +320,20 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent int GetCachedBlockHeight() const override { return nCachedBlockHeight; } - // Accessors for thread-safe access to maps + // Thread-safe accessors + bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HaveObjectForHash(const uint256& nHash) const; - bool HaveVoteForHash(const uint256& nHash) const; - - int GetVoteCount() const; - bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const; - bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const; - + CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetVoteCount() const; void AddPostponedObject(const CGovernanceObject& govobj); + // Thread-safe accessors for trigger management + std::vector> GetActiveTriggers() const override EXCLUSIVE_LOCKS_REQUIRED(!cs); + void MasternodeRateUpdate(const CGovernanceObject& govobj); bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override; @@ -362,7 +362,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - std::vector> GetActiveTriggers() const override EXCLUSIVE_LOCKS_REQUIRED(!cs); bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -386,18 +385,15 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); - bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs); private: - //! Internal functions that require locks to be held - CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); + // Internal counterparts to "Thread-safe accessors" bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); + CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); From 423c1eb8401f90a2ddeca41b1b48ee987261d1be Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 23 Oct 2025 00:07:20 +0530 Subject: [PATCH 04/18] refactor: add `cs` annotations for accessor functions --- src/governance/governance.cpp | 10 +++++++-- src/governance/governance.h | 38 ++++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index eae314ee378e..39c30016a505 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -203,7 +203,13 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) { LOCK(cs); - mapPostponedObjects.emplace(govobj.GetHash(), govobj); + AddPostponedObjectInternal(govobj); +} + +void CGovernanceManager::AddPostponedObjectInternal(const CGovernanceObject& govobj) +{ + AssertLockHeld(cs); + mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); } MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, @@ -289,7 +295,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman if (!fIsValid) { if (fMissingConfirmations) { - AddPostponedObject(govobj); + AddPostponedObjectInternal(govobj); LogPrintf("MNGOVERNANCEOBJECT -- Not enough fee confirmations for: %s, strError = %s\n", strHash, strError); } else { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Governance object is invalid - %s\n", strError); diff --git a/src/governance/governance.h b/src/governance/governance.h index 4f42c3f30f27..20c48fee9f23 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -322,17 +322,26 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Thread-safe accessors bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool HaveObjectForHash(const uint256& nHash) const; - bool HaveVoteForHash(const uint256& nHash) const; - bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const; - bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const; - CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - int GetVoteCount() const; - void AddPostponedObject(const CGovernanceObject& govobj); + int nBlockHeight) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool HaveObjectForHash(const uint256& nHash) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool HaveVoteForHash(const uint256& nHash) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + CGovernanceObject* FindGovernanceObject(const uint256& nHash) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetVoteCount() const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + void AddPostponedObject(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(!cs); // Thread-safe accessors for trigger management - std::vector> GetActiveTriggers() const override EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector> GetActiveTriggers() const override + EXCLUSIVE_LOCKS_REQUIRED(!cs); void MasternodeRateUpdate(const CGovernanceObject& govobj); @@ -390,10 +399,15 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent private: // Internal counterparts to "Thread-safe accessors" + void AddPostponedObjectInternal(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs); bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); + int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(cs); + CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) + EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetActiveTriggersInternal() const + EXCLUSIVE_LOCKS_REQUIRED(cs); void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); From 17312727c352ca72874fd82c1fdde09b22508edd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 15:55:19 +0530 Subject: [PATCH 05/18] refactor: protect `GovernanceStore::cs`, add accessor to remove ext. use --- src/governance/governance.cpp | 10 ++++++++-- src/governance/governance.h | 8 +++++--- src/rpc/governance.cpp | 7 ++----- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 39c30016a505..6b713e5acc49 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -568,6 +568,12 @@ void CGovernanceManager::CheckAndRemove() } const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const +{ + LOCK(cs); + return FindConstGovernanceObjectInternal(nHash); +} + +const CGovernanceObject* CGovernanceManager::FindConstGovernanceObjectInternal(const uint256& nHash) const { AssertLockHeld(cs); @@ -1039,7 +1045,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH size_t nVoteCount = 0; if (fUseFilter) { LOCK(cs); - const CGovernanceObject* pObj = FindConstGovernanceObject(nHash); + const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash); if (pObj) { filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRand(/*nMax=*/999999), BLOOM_UPDATE_ALL); @@ -1562,7 +1568,7 @@ std::vector CGovernanceManager::GetActiveTriggersInternal() co // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS for (const auto& pair : mapTrigger) { - const CGovernanceObject* pObj = FindConstGovernanceObject(pair.first); + const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(pair.first); if (pObj) { vecResults.push_back(pair.second); } diff --git a/src/governance/governance.h b/src/governance/governance.h index 20c48fee9f23..e3f11d848ad3 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -177,11 +177,10 @@ class GovernanceStore static constexpr int MAX_CACHE_SIZE = 1000000; static const std::string SERIALIZATION_VERSION_STRING; -public: +protected: // critical section to protect the inner data structures mutable RecursiveMutex cs; -protected: // keep track of the scanning errors std::map mapObjects GUARDED_BY(cs); // mapErasedGovernanceObjects contains key-value pairs, where @@ -298,7 +297,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); // These commands are only used in RPC @@ -339,6 +337,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent void AddPostponedObject(const CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs); + const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + // Thread-safe accessors for trigger management std::vector> GetActiveTriggers() const override EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -409,6 +409,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); + const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); + void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 861678e0216a..42bec108ed1c 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -414,7 +414,6 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); { - LOCK(node.govman->cs); const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hash); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); @@ -604,7 +603,7 @@ static UniValue ListObjects(CGovernanceManager& govman, const CDeterministicMNLi g_txindex->BlockUntilSyncedToCurrentChain(); } - LOCK2(cs_main, govman.cs); + LOCK(cs_main); std::vector objs; govman.GetAllNewerThan(objs, nStartTime); @@ -729,7 +728,7 @@ static RPCHelpMan gobject_get() const ChainstateManager& chainman = EnsureChainman(node); CHECK_NONFATAL(node.govman); - LOCK2(cs_main, node.govman->cs); + LOCK(cs_main); const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash); if (pGovObj == nullptr) { @@ -828,7 +827,6 @@ static RPCHelpMan gobject_getcurrentvotes() const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); - LOCK(node.govman->cs); const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash); if (pGovObj == nullptr) { @@ -924,7 +922,6 @@ static RPCHelpMan voteraw() const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); GovernanceObject govObjType = [&]() { - LOCK(node.govman->cs); const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hashGovObj); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); From 9f5cf4ac34e7aebdbb0ee8d35ec7832def083748 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 17:57:42 +0000 Subject: [PATCH 06/18] refactor: privatize internally used functions This will simplify our work in subsequent commits. Review with `git log -p -n1 --color-moved=dimmed_zebra`. --- src/governance/governance.h | 141 +++++++++++++++--------------------- 1 file changed, 58 insertions(+), 83 deletions(-) diff --git a/src/governance/governance.h b/src/governance/governance.h index e3f11d848ad3..809351e969c0 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -275,48 +275,60 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent const std::unique_ptr& dmnman, CMasternodeSync& mn_sync); ~CGovernanceManager(); - void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman); - + // Basic initialization and querying + bool IsValid() const override { return is_valid; } bool LoadCache(bool load_cache); + std::string ToString() const; + UniValue ToJson() const; + void Clear(); + void CheckAndRemove(); + void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman); - bool IsValid() const override { return is_valid; } + // CGovernanceObject + bool AreRateChecksEnabled() const { return fRateChecksEnabled; } + void AddInvalidVote(const CGovernanceVote& vote); - void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + // Getters/Setters + int GetCachedBlockHeight() const override { return nCachedBlockHeight; } + int64_t GetLastDiffTime() const { return nTimeLastDiff; } + std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; + void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; + void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } + // Networking /** * This is called by AlreadyHave in net_processing.cpp as part of the inventory * retrieval process. Returns true if we want to retrieve the object, otherwise * false. (Note logic is inverted in AlreadyHave). */ bool ConfirmInventoryRequest(const CInv& inv); - - [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); - [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; - + bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const; + int RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, + const PeerManager& peerman) const; [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - - // These commands are only used in RPC - std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; - void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; + // Notification interface trigger + void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + // Signer interface + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override; + CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - void Clear(); - void CheckAndRemove(); - - UniValue ToJson() const; - std::string ToString() const; - - void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - int64_t GetLastDiffTime() const { return nTimeLastDiff; } - void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } - - int GetCachedBlockHeight() const override { return nCachedBlockHeight; } + // Superblocks + bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, + std::vector& voutSuperblockRet); + bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, + const CTransaction& txNew, int nBlockHeight, CAmount blockReward); // Thread-safe accessors bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, @@ -334,38 +346,40 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent EXCLUSIVE_LOCKS_REQUIRED(!cs); int GetVoteCount() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector> GetActiveTriggers() const override + EXCLUSIVE_LOCKS_REQUIRED(!cs); void AddPostponedObject(const CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs); - const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - - // Thread-safe accessors for trigger management - std::vector> GetActiveTriggers() const override + const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - void MasternodeRateUpdate(const CGovernanceObject& govobj); +private: + // Branches of ProcessMessage + [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; + [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override; + // Internal counterparts to "Thread-safe accessors" + void AddPostponedObjectInternal(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs); + bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(cs); + CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) + EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetActiveTriggersInternal() const + EXCLUSIVE_LOCKS_REQUIRED(cs); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); + const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); - bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override - EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void MasternodeRateUpdate(const CGovernanceObject& govobj); - void CheckPostponedObjects() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); - bool AreRateChecksEnabled() const - { - LOCK(cs); - return fRateChecksEnabled; - } + void CheckPostponedObjects() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void InitOnLoad(); - int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const; - int RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, - const PeerManager& peerman) const; - /* * Trigger Management (formerly CGovernanceTriggerManager) * - Track governance objects which are triggers @@ -374,49 +388,10 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); - // Superblocks related: - - /** - * Is Superblock Triggered - * - * - Does this block have a non-executed and activated trigger? - */ - bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight); - - /** - * Get Superblock Payments - * - * - Returns payments for superblock - */ - bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, - std::vector& voutSuperblockRet); - - bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, - const CTransaction& txNew, int nBlockHeight, CAmount blockReward); - - std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); - -private: - // Internal counterparts to "Thread-safe accessors" - void AddPostponedObjectInternal(const CGovernanceObject& govobj) - EXCLUSIVE_LOCKS_REQUIRED(cs); - bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) - EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) - EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector> GetActiveTriggersInternal() const - EXCLUSIVE_LOCKS_REQUIRED(cs); - - const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); - void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const; - void AddInvalidVote(const CGovernanceVote& vote); - bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman); /// Called to indicate a requested object or vote has been received From e8a1e8be766f60c295f8efd5cbb13e7afbe66f30 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 23 Oct 2025 00:14:05 +0530 Subject: [PATCH 07/18] refactor: add `cs` annotations for private functions --- src/governance/governance.cpp | 46 +++++++++++++-------- src/governance/governance.h | 75 +++++++++++++++++++++++------------ 2 files changed, 79 insertions(+), 42 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 6b713e5acc49..4745d0bbcde0 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -119,6 +119,7 @@ void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, Peer bool CGovernanceManager::LoadCache(bool load_cache) { + AssertLockNotHeld(cs); assert(m_db != nullptr); is_valid = load_cache ? m_db->Load(*this) : m_db->Store(*this); if (is_valid && load_cache) { @@ -215,6 +216,7 @@ void CGovernanceManager::AddPostponedObjectInternal(const CGovernanceObject& gov MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) { + AssertLockNotHeld(cs); AssertLockNotHeld(cs_relay); if (!IsValid()) return {}; if (!m_mn_sync.IsBlockchainSynced()) return {}; @@ -233,6 +235,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman vRecv >> filter; LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing governance objects to our peer %s\n", peer.GetLogString()); + LOCK(cs); if (nProp == uint256()) { return SyncObjects(peer, connman); } else { @@ -288,7 +291,8 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman bool fMissingConfirmations = false; bool fIsValid = govobj.IsValidLocally(tip_mn_list, m_chainman, strError, fMissingConfirmations, true); - if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true)) { + bool unused_rcb; + if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true, true, unused_rcb)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight); return ret; } @@ -358,9 +362,9 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) { - LOCK(cs); - + AssertLockHeld(cs); AssertLockNotHeld(cs_relay); + uint256 nHash = govobj.GetHash(); std::vector vecVotePairs; cmmapOrphanVotes.GetAll(nHash, vecVotePairs); @@ -448,6 +452,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN void CGovernanceManager::CheckAndRemove() { + AssertLockNotHeld(cs); assert(m_mn_metaman.IsValid()); // Return on initial sync, spammed the debug.log and provided no use @@ -707,6 +712,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman) { + AssertLockHeld(cs); // do not provide any data until our node is synced if (!m_mn_sync.IsSynced()) return {}; @@ -714,8 +720,6 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing single object to peer=%d, nProp = %s\n", __func__, peer.GetId(), nProp.ToString()); - LOCK(cs); - // single valid object and its valid votes auto it = mapObjects.find(nProp); if (it == mapObjects.end()) { @@ -758,6 +762,7 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const { + AssertLockHeld(cs); assert(m_netfulfilledman.IsValid()); // do not provide any data until our node is synced @@ -774,8 +779,6 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing all objects to peer=%d\n", __func__, peer.GetId()); - LOCK(cs); - // all valid objects, no votes MessageProcessingResult ret{}; for (const auto& objPair : mapObjects) { @@ -806,9 +809,9 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) { - if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; + AssertLockHeld(cs); - LOCK(cs); + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint(); auto it = mapLastMasternodeObject.find(masternodeOutpoint); @@ -830,13 +833,14 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus) { + LOCK(cs); bool fRateCheckBypassed; return MasternodeRateCheck(govobj, fUpdateFailStatus, true, fRateCheckBypassed); } bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) { - LOCK(cs); + AssertLockHeld(cs); fRateCheckBypassed = false; @@ -900,6 +904,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { + AssertLockNotHeld(cs); AssertLockNotHeld(cs_relay); bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); if (fOK) { @@ -910,6 +915,7 @@ bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGover bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { + AssertLockNotHeld(cs); ENTER_CRITICAL_SECTION(cs); uint256 nHashVote = vote.GetHash(); uint256 nHashGovobj = vote.GetParentHash(); @@ -964,10 +970,11 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, void CGovernanceManager::CheckPostponedObjects() { + AssertLockHeld(cs); AssertLockNotHeld(cs_relay); if (!m_mn_sync.IsSynced()) return; - LOCK2(::cs_main, cs); + LOCK(::cs_main); // Check postponed proposals for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) { @@ -1032,6 +1039,7 @@ void CGovernanceManager::CheckPostponedObjects() void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter) const { + AssertLockNotHeld(cs); if (!pfrom) { return; } @@ -1186,6 +1194,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& bool CGovernanceManager::AcceptMessage(const uint256& nHash) { + AssertLockNotHeld(cs); LOCK(cs); auto it = m_requested_hash_time.find(nHash); if (it == m_requested_hash_time.end()) { @@ -1199,7 +1208,7 @@ bool CGovernanceManager::AcceptMessage(const uint256& nHash) void CGovernanceManager::RebuildIndexes() { - LOCK(cs); + AssertLockHeld(cs); cmapVoteToObject.Clear(); for (auto& objPair : mapObjects) { @@ -1213,7 +1222,7 @@ void CGovernanceManager::RebuildIndexes() void CGovernanceManager::AddCachedTriggers() { - LOCK(cs); + AssertLockHeld(cs); int64_t nNow = GetTime().count(); @@ -1325,6 +1334,7 @@ UniValue CGovernanceManager::ToJson() const void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) { + AssertLockNotHeld(cs); AssertLockNotHeld(cs_relay); // Note this gets called from ActivateBestChain without cs_main being held // so it should be safe to lock our mutex here without risking a deadlock @@ -1338,6 +1348,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) nCachedBlockHeight = pindex->nHeight; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight); + LOCK(cs); if (DeploymentDIP0003Enforced(pindex->nHeight, Params().GetConsensus())) { RemoveInvalidVotes(); } @@ -1349,12 +1360,13 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) void CGovernanceManager::RequestOrphanObjects(CConnman& connman) { + AssertLockNotHeld(cs); const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly}; std::vector vecHashesFiltered; { - std::vector vecHashes; LOCK(cs); + std::vector vecHashes; cmmapOrphanVotes.GetKeys(vecHashes); for (const uint256& nHash : vecHashes) { if (mapObjects.find(nHash) == mapObjects.end()) { @@ -1394,12 +1406,12 @@ void CGovernanceManager::CleanOrphanObjects() void CGovernanceManager::RemoveInvalidVotes() { + AssertLockHeld(cs); + if (!m_mn_sync.IsSynced()) { return; } - LOCK(cs); - const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); auto diff = lastMNListForVotingKeys->BuildDiff(tip_mn_list); @@ -1733,7 +1745,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { diff --git a/src/governance/governance.h b/src/governance/governance.h index 809351e969c0..445d42fbfa50 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -277,11 +277,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Basic initialization and querying bool IsValid() const override { return is_valid; } - bool LoadCache(bool load_cache); + bool LoadCache(bool load_cache) + EXCLUSIVE_LOCKS_REQUIRED(!cs); std::string ToString() const; UniValue ToJson() const; void Clear(); - void CheckAndRemove(); + void CheckAndRemove() + EXCLUSIVE_LOCKS_REQUIRED(!cs); void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman); // CGovernanceObject @@ -303,20 +305,23 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent */ bool ConfirmInventoryRequest(const CInv& inv); bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override - EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const; int RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, const PeerManager& peerman) const; [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, - CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); // Notification interface trigger - void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void UpdatedBlockTip(const CBlockIndex* pindex) + EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); // Signer interface - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override; + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -356,8 +361,11 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent private: // Branches of ProcessMessage - [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; - [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); + [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const + EXCLUSIVE_LOCKS_REQUIRED(cs); + [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, + CConnman& connman) + EXCLUSIVE_LOCKS_REQUIRED(cs); // Internal counterparts to "Thread-safe accessors" void AddPostponedObjectInternal(const CGovernanceObject& govobj) @@ -370,44 +378,61 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); - const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); + const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const + EXCLUSIVE_LOCKS_REQUIRED(cs); - void MasternodeRateUpdate(const CGovernanceObject& govobj); + void MasternodeRateUpdate(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) + EXCLUSIVE_LOCKS_REQUIRED(cs); - void CheckPostponedObjects() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void CheckPostponedObjects() + EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay); - void InitOnLoad(); + void InitOnLoad() + EXCLUSIVE_LOCKS_REQUIRED(!cs); /* * Trigger Management (formerly CGovernanceTriggerManager) * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); - void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); + bool AddNewTrigger(uint256 nHash) + EXCLUSIVE_LOCKS_REQUIRED(cs); + void CleanAndRemoveTriggers() + EXCLUSIVE_LOCKS_REQUIRED(cs); - void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(cs); - void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const; + void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman); + bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) + EXCLUSIVE_LOCKS_REQUIRED(!cs); /// Called to indicate a requested object or vote has been received - bool AcceptMessage(const uint256& nHash); + bool AcceptMessage(const uint256& nHash) + EXCLUSIVE_LOCKS_REQUIRED(!cs); - void CheckOrphanVotes(CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void CheckOrphanVotes(CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay); - void RebuildIndexes(); + void RebuildIndexes() + EXCLUSIVE_LOCKS_REQUIRED(cs); - void AddCachedTriggers(); + void AddCachedTriggers() + EXCLUSIVE_LOCKS_REQUIRED(cs); - void RequestOrphanObjects(CConnman& connman); + void RequestOrphanObjects(CConnman& connman) + EXCLUSIVE_LOCKS_REQUIRED(!cs); - void CleanOrphanObjects(); + void CleanOrphanObjects() + EXCLUSIVE_LOCKS_REQUIRED(!cs); - void RemoveInvalidVotes(); + void RemoveInvalidVotes() + EXCLUSIVE_LOCKS_REQUIRED(cs); }; bool AreSuperblocksEnabled(const CSporkManager& sporkman); From 7b9997ce65678a9be4727189b8bf47a8bc31459c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 18:38:54 +0000 Subject: [PATCH 08/18] refactor: add `cs` annotations for public functions --- src/governance/governance.cpp | 14 +++++++++ src/governance/governance.h | 58 +++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 4745d0bbcde0..cb881ab24fc2 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -90,6 +90,9 @@ CGovernanceManager::~CGovernanceManager() void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman) { + AssertLockNotHeld(cs); + AssertLockNotHeld(cs_relay); + assert(IsValid()); scheduler.scheduleEvery( @@ -464,6 +467,7 @@ void CGovernanceManager::CheckAndRemove() const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); + { LOCK2(::cs_main, cs); for (const uint256& nHash : vecDirtyHashes) { @@ -567,6 +571,7 @@ void CGovernanceManager::CheckAndRemove() ++r_it; } } + } LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, m_requested_hash_time size=%d\n", ToString(), m_requested_hash_time.size()); @@ -669,6 +674,8 @@ void CGovernanceManager::GetAllNewerThan(std::vector& objs, i bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) { + AssertLockNotHeld(cs); + // do not request objects until it's time to sync if (!m_mn_sync.IsBlockchainSynced()) return false; @@ -1084,6 +1091,8 @@ int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& conn int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, const PeerManager& peerman) const { + AssertLockNotHeld(cs); + static std::map > mapAskedRecently; // Maximum number of nodes to request votes from for the same object hash on real networks // (mainnet, testnet, devnets). Keep this low to avoid unnecessary bandwidth usage. @@ -1241,6 +1250,7 @@ void CGovernanceManager::AddCachedTriggers() void CGovernanceManager::InitOnLoad() { + { LOCK(cs); const auto start{SteadyClock::now()}; LogPrintf("Preparing masternode indexes and governance triggers...\n"); @@ -1248,6 +1258,7 @@ void CGovernanceManager::InitOnLoad() AddCachedTriggers(); LogPrintf("Masternode indexes and governance triggers prepared %dms\n", Ticks(SteadyClock::now() - start)); + } LogPrintf(" %s\n", ToString()); } @@ -1263,6 +1274,7 @@ void GovernanceStore::Clear() void CGovernanceManager::Clear() { + AssertLockNotHeld(cs); LogPrint(BCLog::GOBJECT, "Governance object manager was cleared\n"); GovernanceStore::Clear(); cmapVoteToObject.Clear(); @@ -1297,6 +1309,7 @@ std::string GovernanceStore::ToString() const std::string CGovernanceManager::ToString() const { + AssertLockNotHeld(cs); return strprintf("%s, Votes: %d", GovernanceStore::ToString(), (int)cmapVoteToObject.GetSize()); } @@ -1591,6 +1604,7 @@ std::vector CGovernanceManager::GetActiveTriggersInternal() co bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { + AssertLockNotHeld(cs); LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; diff --git a/src/governance/governance.h b/src/governance/governance.h index 445d42fbfa50..b9ca42ddfe42 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -198,7 +198,7 @@ class GovernanceStore ~GovernanceStore() = default; template - void Serialize(Stream &s) const + void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); s << SERIALIZATION_VERSION_STRING @@ -211,7 +211,7 @@ class GovernanceStore } template - void Unserialize(Stream &s) + void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs) { Clear(); @@ -230,9 +230,11 @@ class GovernanceStore >> *lastMNListForVotingKeys; } - void Clear(); + void Clear() + EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::string ToString() const; + std::string ToString() const + EXCLUSIVE_LOCKS_REQUIRED(!cs); }; // @@ -279,22 +281,29 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool IsValid() const override { return is_valid; } bool LoadCache(bool load_cache) EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::string ToString() const; - UniValue ToJson() const; - void Clear(); + std::string ToString() const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + UniValue ToJson() const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Clear() + EXCLUSIVE_LOCKS_REQUIRED(!cs); void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman); + void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman) + EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); // CGovernanceObject bool AreRateChecksEnabled() const { return fRateChecksEnabled; } - void AddInvalidVote(const CGovernanceVote& vote); + void AddInvalidVote(const CGovernanceVote& vote) + EXCLUSIVE_LOCKS_REQUIRED(!cs); // Getters/Setters int GetCachedBlockHeight() const override { return nCachedBlockHeight; } int64_t GetLastDiffTime() const { return nTimeLastDiff; } - std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; - void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; + std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } // Networking @@ -303,17 +312,22 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent * retrieval process. Returns true if we want to retrieve the object, otherwise * false. (Note logic is inverted in AlreadyHave). */ - bool ConfirmInventoryRequest(const CInv& inv); + bool ConfirmInventoryRequest(const CInv& inv) + EXCLUSIVE_LOCKS_REQUIRED(!cs); bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); - int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const; + int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); int RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, - const PeerManager& peerman) const; + const PeerManager& peerman) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); - void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void RelayObject(const CGovernanceObject& obj) + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void RelayVote(const CGovernanceVote& vote) + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); // Notification interface trigger void UpdatedBlockTip(const CBlockIndex* pindex) @@ -322,7 +336,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Signer interface bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs); void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override @@ -330,10 +345,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Superblocks bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, - std::vector& voutSuperblockRet); - bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + std::vector& voutSuperblockRet) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(!cs); bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, - const CTransaction& txNew, int nBlockHeight, CAmount blockReward); + const CTransaction& txNew, int nBlockHeight, CAmount blockReward) + EXCLUSIVE_LOCKS_REQUIRED(!cs); // Thread-safe accessors bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, From f077ca3f37935f58dd4823fc2460566ccd242c6a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 18:50:15 +0000 Subject: [PATCH 09/18] refactor: move from `RecursiveMutex` to `Mutex`, rename to `cs_store` --- src/governance/governance.cpp | 164 +++++++++++++++++----------------- src/governance/governance.h | 134 +++++++++++++-------------- 2 files changed, 149 insertions(+), 149 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index cb881ab24fc2..f3df5d636c2b 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -41,7 +41,7 @@ class ScopedLockBool bool fPrevValue; public: - ScopedLockBool(RecursiveMutex& _cs, bool& _ref, bool _value) : + ScopedLockBool(Mutex& _cs, bool& _ref, bool _value) : ref(_ref) { AssertLockHeld(_cs); @@ -54,7 +54,7 @@ class ScopedLockBool } // anonymous namespace GovernanceStore::GovernanceStore() : - cs(), + cs_store(), mapObjects(), mapErasedGovernanceObjects(), cmapInvalidVotes(MAX_CACHE_SIZE), @@ -90,7 +90,7 @@ CGovernanceManager::~CGovernanceManager() void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); AssertLockNotHeld(cs_relay); assert(IsValid()); @@ -122,7 +122,7 @@ void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, Peer bool CGovernanceManager::LoadCache(bool load_cache) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); assert(m_db != nullptr); is_valid = load_cache ? m_db->Load(*this) : m_db->Store(*this); if (is_valid && load_cache) { @@ -165,13 +165,13 @@ void CGovernanceManager::RelayVote(const CGovernanceVote& vote) // Accessors for thread-safe access to maps bool CGovernanceManager::HaveObjectForHash(const uint256& nHash) const { - LOCK(cs); + LOCK(cs_store); return (mapObjects.count(nHash) == 1 || mapPostponedObjects.count(nHash) == 1); } bool CGovernanceManager::SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const { - LOCK(cs); + LOCK(cs_store); auto it = mapObjects.find(nHash); if (it == mapObjects.end()) { it = mapPostponedObjects.find(nHash); @@ -184,7 +184,7 @@ bool CGovernanceManager::SerializeObjectForHash(const uint256& nHash, CDataStrea bool CGovernanceManager::HaveVoteForHash(const uint256& nHash) const { - LOCK(cs); + LOCK(cs_store); CGovernanceObject* pGovobj = nullptr; return cmapVoteToObject.Get(nHash, pGovobj) && pGovobj->GetVoteFile().HasVote(nHash); @@ -192,13 +192,13 @@ bool CGovernanceManager::HaveVoteForHash(const uint256& nHash) const int CGovernanceManager::GetVoteCount() const { - LOCK(cs); + LOCK(cs_store); return (int)cmapVoteToObject.GetSize(); } bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const { - LOCK(cs); + LOCK(cs_store); CGovernanceObject* pGovobj = nullptr; return cmapVoteToObject.Get(nHash, pGovobj) && pGovobj->GetVoteFile().SerializeVoteToStream(nHash, ss); @@ -206,20 +206,20 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) { - LOCK(cs); + LOCK(cs_store); AddPostponedObjectInternal(govobj); } void CGovernanceManager::AddPostponedObjectInternal(const CGovernanceObject& govobj) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); } MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); AssertLockNotHeld(cs_relay); if (!IsValid()) return {}; if (!m_mn_sync.IsBlockchainSynced()) return {}; @@ -238,7 +238,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman vRecv >> filter; LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing governance objects to our peer %s\n", peer.GetLogString()); - LOCK(cs); + LOCK(cs_store); if (nProp == uint256()) { return SyncObjects(peer, connman); } else { @@ -274,7 +274,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman return ret; } - LOCK2(::cs_main, cs); + LOCK2(::cs_main, cs_store); if (mapObjects.count(nHash) || mapPostponedObjects.count(nHash) || mapErasedGovernanceObjects.count(nHash)) { // TODO - print error code? what if it's GOVOBJ_ERROR_IMMATURE? @@ -365,14 +365,14 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); AssertLockNotHeld(cs_relay); uint256 nHash = govobj.GetHash(); std::vector vecVotePairs; cmmapOrphanVotes.GetAll(nHash, vecVotePairs); - ScopedLockBool guard(cs, fRateChecksEnabled, false); + ScopedLockBool guard(cs_store, fRateChecksEnabled, false); int64_t nNow = GetAdjustedTime(); const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); @@ -404,7 +404,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN govobj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object - LOCK2(::cs_main, cs); + LOCK2(::cs_main, cs_store); std::string strError; // MAKE SURE THIS OBJECT IS OK @@ -455,7 +455,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN void CGovernanceManager::CheckAndRemove() { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); assert(m_mn_metaman.IsValid()); // Return on initial sync, spammed the debug.log and provided no use @@ -468,7 +468,7 @@ void CGovernanceManager::CheckAndRemove() const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); { - LOCK2(::cs_main, cs); + LOCK2(::cs_main, cs_store); for (const uint256& nHash : vecDirtyHashes) { auto it = mapObjects.find(nHash); @@ -478,7 +478,7 @@ void CGovernanceManager::CheckAndRemove() it->second.ClearMasternodeVotes(tip_mn_list); } - ScopedLockBool guard(cs, fRateChecksEnabled, false); + ScopedLockBool guard(cs_store, fRateChecksEnabled, false); // Clean up any expired or invalid triggers CleanAndRemoveTriggers(); @@ -579,13 +579,13 @@ void CGovernanceManager::CheckAndRemove() const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const { - LOCK(cs); + LOCK(cs_store); return FindConstGovernanceObjectInternal(nHash); } const CGovernanceObject* CGovernanceManager::FindConstGovernanceObjectInternal(const uint256& nHash) const { - AssertLockHeld(cs); + AssertLockHeld(cs_store); auto it = mapObjects.find(nHash); if (it != mapObjects.end()) return &(it->second); @@ -595,22 +595,22 @@ const CGovernanceObject* CGovernanceManager::FindConstGovernanceObjectInternal(c CGovernanceObject* CGovernanceManager::FindGovernanceObject(const uint256& nHash) { - AssertLockNotHeld(cs); - LOCK(cs); + AssertLockNotHeld(cs_store); + LOCK(cs_store); return FindGovernanceObjectInternal(nHash); } CGovernanceObject* CGovernanceManager::FindGovernanceObjectInternal(const uint256& nHash) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); if (mapObjects.count(nHash)) return &mapObjects[nHash]; return nullptr; } CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint256 &nDataHash) { - AssertLockNotHeld(cs); - LOCK(cs); + AssertLockNotHeld(cs_store); + LOCK(cs_store); for (const auto& [nHash, object] : mapObjects) { if (object.GetDataHash() == nDataHash) return &mapObjects[nHash]; } @@ -619,7 +619,7 @@ CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint std::vector CGovernanceManager::GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const { - LOCK(cs); + LOCK(cs_store); std::vector vecResult; // Find the governance object or short-circuit. @@ -659,7 +659,7 @@ std::vector CGovernanceManager::GetCurrentVotes(const uint256& void CGovernanceManager::GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const { - LOCK(cs); + LOCK(cs_store); for (const auto& objPair : mapObjects) { // IF THIS OBJECT IS OLDER THAN TIME, CONTINUE @@ -674,12 +674,12 @@ void CGovernanceManager::GetAllNewerThan(std::vector& objs, i bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); // do not request objects until it's time to sync if (!m_mn_sync.IsBlockchainSynced()) return false; - LOCK(cs); + LOCK(cs_store); LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest inv = %s\n", inv.ToString()); @@ -719,7 +719,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); // do not provide any data until our node is synced if (!m_mn_sync.IsSynced()) return {}; @@ -769,7 +769,7 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const { - AssertLockHeld(cs); + AssertLockHeld(cs_store); assert(m_netfulfilledman.IsValid()); // do not provide any data until our node is synced @@ -816,7 +816,7 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; @@ -840,14 +840,14 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus) { - LOCK(cs); + LOCK(cs_store); bool fRateCheckBypassed; return MasternodeRateCheck(govobj, fUpdateFailStatus, true, fRateCheckBypassed); } bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); fRateCheckBypassed = false; @@ -911,7 +911,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); AssertLockNotHeld(cs_relay); bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); if (fOK) { @@ -922,15 +922,15 @@ bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGover bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { - AssertLockNotHeld(cs); - ENTER_CRITICAL_SECTION(cs); + AssertLockNotHeld(cs_store); + ENTER_CRITICAL_SECTION(cs_store); uint256 nHashVote = vote.GetHash(); uint256 nHashGovobj = vote.GetParentHash(); if (cmapVoteToObject.HasKey(nHashVote)) { LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- skipping known valid vote %s for object %s\n", __func__, nHashVote.ToString(), nHashGovobj.ToString()); - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -939,7 +939,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, __func__, vote.GetMasternodeOutpoint().ToStringShort(), nHashGovobj.ToString())}; LogPrint(BCLog::GOBJECT, "%s\n", msg); exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -950,14 +950,14 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING); if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, count_seconds(GetTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME)))) { - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); RequestGovernanceObject(pfrom, nHashGovobj, connman); LogPrint(BCLog::GOBJECT, "%s\n", msg); return false; } LogPrint(BCLog::GOBJECT, "%s\n", msg); - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -966,18 +966,18 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- ignoring vote for expired or deleted object, hash = %s\n", __func__, nHashGovobj.ToString()); - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); return false; } bool fOk = govobj.ProcessVote(m_mn_metaman, *this, Assert(m_dmnman)->GetListAtChainTip(), vote, exception) && cmapVoteToObject.Insert(nHashVote, &govobj); - LEAVE_CRITICAL_SECTION(cs); + LEAVE_CRITICAL_SECTION(cs_store); return fOk; } void CGovernanceManager::CheckPostponedObjects() { - AssertLockHeld(cs); + AssertLockHeld(cs_store); AssertLockNotHeld(cs_relay); if (!m_mn_sync.IsSynced()) return; @@ -1046,7 +1046,7 @@ void CGovernanceManager::CheckPostponedObjects() void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter) const { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); if (!pfrom) { return; } @@ -1059,7 +1059,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH size_t nVoteCount = 0; if (fUseFilter) { - LOCK(cs); + LOCK(cs_store); const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash); if (pObj) { @@ -1078,7 +1078,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH void CGovernanceManager::AddInvalidVote(const CGovernanceVote& vote) { - LOCK(cs); + LOCK(cs_store); cmapInvalidVotes.Insert(vote.GetHash(), vote); } @@ -1091,7 +1091,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& conn int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, const PeerManager& peerman) const { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); static std::map > mapAskedRecently; // Maximum number of nodes to request votes from for the same object hash on real networks @@ -1124,7 +1124,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& } { - LOCK(cs); + LOCK(cs_store); if (mapObjects.empty()) return -2; @@ -1203,8 +1203,8 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& bool CGovernanceManager::AcceptMessage(const uint256& nHash) { - AssertLockNotHeld(cs); - LOCK(cs); + AssertLockNotHeld(cs_store); + LOCK(cs_store); auto it = m_requested_hash_time.find(nHash); if (it == m_requested_hash_time.end()) { // We never requested this @@ -1217,7 +1217,7 @@ bool CGovernanceManager::AcceptMessage(const uint256& nHash) void CGovernanceManager::RebuildIndexes() { - AssertLockHeld(cs); + AssertLockHeld(cs_store); cmapVoteToObject.Clear(); for (auto& objPair : mapObjects) { @@ -1231,7 +1231,7 @@ void CGovernanceManager::RebuildIndexes() void CGovernanceManager::AddCachedTriggers() { - AssertLockHeld(cs); + AssertLockHeld(cs_store); int64_t nNow = GetTime().count(); @@ -1251,7 +1251,7 @@ void CGovernanceManager::AddCachedTriggers() void CGovernanceManager::InitOnLoad() { { - LOCK(cs); + LOCK(cs_store); const auto start{SteadyClock::now()}; LogPrintf("Preparing masternode indexes and governance triggers...\n"); RebuildIndexes(); @@ -1264,7 +1264,7 @@ void CGovernanceManager::InitOnLoad() void GovernanceStore::Clear() { - LOCK(cs); + LOCK(cs_store); mapObjects.clear(); mapErasedGovernanceObjects.clear(); cmapInvalidVotes.Clear(); @@ -1274,7 +1274,7 @@ void GovernanceStore::Clear() void CGovernanceManager::Clear() { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); LogPrint(BCLog::GOBJECT, "Governance object manager was cleared\n"); GovernanceStore::Clear(); cmapVoteToObject.Clear(); @@ -1282,7 +1282,7 @@ void CGovernanceManager::Clear() std::string GovernanceStore::ToString() const { - LOCK(cs); + LOCK(cs_store); int nProposalCount = 0; int nTriggerCount = 0; @@ -1309,13 +1309,13 @@ std::string GovernanceStore::ToString() const std::string CGovernanceManager::ToString() const { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); return strprintf("%s, Votes: %d", GovernanceStore::ToString(), (int)cmapVoteToObject.GetSize()); } UniValue CGovernanceManager::ToJson() const { - LOCK(cs); + LOCK(cs_store); int nProposalCount = 0; int nTriggerCount = 0; @@ -1347,7 +1347,7 @@ UniValue CGovernanceManager::ToJson() const void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); AssertLockNotHeld(cs_relay); // Note this gets called from ActivateBestChain without cs_main being held // so it should be safe to lock our mutex here without risking a deadlock @@ -1361,7 +1361,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) nCachedBlockHeight = pindex->nHeight; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight); - LOCK(cs); + LOCK(cs_store); if (DeploymentDIP0003Enforced(pindex->nHeight, Params().GetConsensus())) { RemoveInvalidVotes(); } @@ -1373,12 +1373,12 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) void CGovernanceManager::RequestOrphanObjects(CConnman& connman) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly}; std::vector vecHashesFiltered; { - LOCK(cs); + LOCK(cs_store); std::vector vecHashes; cmmapOrphanVotes.GetKeys(vecHashes); for (const uint256& nHash : vecHashes) { @@ -1401,7 +1401,7 @@ void CGovernanceManager::RequestOrphanObjects(CConnman& connman) void CGovernanceManager::CleanOrphanObjects() { - LOCK(cs); + LOCK(cs_store); const vote_cmm_t::list_t& items = cmmapOrphanVotes.GetItemList(); int64_t nNow = GetTime().count(); @@ -1419,7 +1419,7 @@ void CGovernanceManager::CleanOrphanObjects() void CGovernanceManager::RemoveInvalidVotes() { - AssertLockHeld(cs); + AssertLockHeld(cs_store); if (!m_mn_sync.IsSynced()) { return; @@ -1473,7 +1473,7 @@ void CGovernanceManager::RemoveInvalidVotes() bool CGovernanceManager::AddNewTrigger(uint256 nHash) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); // IF WE ALREADY HAVE THIS HASH, RETURN if (mapTrigger.count(nHash)) { @@ -1512,7 +1512,7 @@ bool CGovernanceManager::AddNewTrigger(uint256 nHash) void CGovernanceManager::CleanAndRemoveTriggers() { - AssertLockHeld(cs); + AssertLockHeld(cs_store); // Remove triggers that are invalid or expired LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- mapTrigger.size() = %d\n", __func__, mapTrigger.size()); @@ -1581,14 +1581,14 @@ void CGovernanceManager::CleanAndRemoveTriggers() */ std::vector CGovernanceManager::GetActiveTriggers() const { - AssertLockNotHeld(cs); - LOCK(cs); + AssertLockNotHeld(cs_store); + LOCK(cs_store); return GetActiveTriggersInternal(); } std::vector CGovernanceManager::GetActiveTriggersInternal() const { - AssertLockHeld(cs); + AssertLockHeld(cs_store); std::vector vecResults; // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS @@ -1604,13 +1604,13 @@ std::vector CGovernanceManager::GetActiveTriggersInternal() co bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; } - LOCK(cs); + LOCK(cs_store); // GET ALL ACTIVE TRIGGERS std::vector vecTriggers = GetActiveTriggersInternal(); @@ -1658,8 +1658,8 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) { - AssertLockNotHeld(cs); - LOCK(cs); + AssertLockNotHeld(cs_store); + LOCK(cs_store); return GetBestSuperblockInternal(tip_mn_list, pSuperblockRet, nBlockHeight); } @@ -1670,7 +1670,7 @@ bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& t return false; } - AssertLockHeld(cs); + AssertLockHeld(cs_store); std::vector vecTriggers = GetActiveTriggersInternal(); int nYesCount = 0; @@ -1699,7 +1699,7 @@ bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& t bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet) { - LOCK(cs); + LOCK(cs_store); // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT @@ -1747,7 +1747,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe const CTransaction& txNew, int nBlockHeight, CAmount blockReward) { // GET BEST SUPERBLOCK, SHOULD MATCH - LOCK(cs); + LOCK(cs_store); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { @@ -1759,7 +1759,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { - AssertLockHeld(cs); + AssertLockHeld(cs_store); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { @@ -1772,7 +1772,7 @@ void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_m std::vector> CGovernanceManager::GetApprovedProposals( const CDeterministicMNList& tip_mn_list) { - AssertLockNotHeld(cs); + AssertLockNotHeld(cs_store); // Use std::vector of std::shared_ptr because CGovernanceObject doesn't support move operations (needed for sorting the vector later) std::vector> ret{}; @@ -1782,7 +1782,7 @@ std::vector> CGovernanceManager::GetApp const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); - LOCK(cs); + LOCK(cs_store); for (const auto& [unused, object] : mapObjects) { // Skip all non-proposals objects if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; diff --git a/src/governance/governance.h b/src/governance/governance.h index b9ca42ddfe42..f63b10597124 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -179,28 +179,28 @@ class GovernanceStore protected: // critical section to protect the inner data structures - mutable RecursiveMutex cs; + mutable Mutex cs_store; // keep track of the scanning errors - std::map mapObjects GUARDED_BY(cs); + std::map mapObjects GUARDED_BY(cs_store); // mapErasedGovernanceObjects contains key-value pairs, where // key - governance object's hash // value - expiration time for deleted objects - std::map mapErasedGovernanceObjects GUARDED_BY(cs); - CacheMap cmapInvalidVotes GUARDED_BY(cs); - vote_cmm_t cmmapOrphanVotes GUARDED_BY(cs); - txout_m_t mapLastMasternodeObject GUARDED_BY(cs); + std::map mapErasedGovernanceObjects GUARDED_BY(cs_store); + CacheMap cmapInvalidVotes GUARDED_BY(cs_store); + vote_cmm_t cmmapOrphanVotes GUARDED_BY(cs_store); + txout_m_t mapLastMasternodeObject GUARDED_BY(cs_store); // used to check for changed voting keys - std::shared_ptr lastMNListForVotingKeys GUARDED_BY(cs); + std::shared_ptr lastMNListForVotingKeys GUARDED_BY(cs_store); public: GovernanceStore(); ~GovernanceStore() = default; template - void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs) + void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store) { - LOCK(cs); + LOCK(cs_store); s << SERIALIZATION_VERSION_STRING << mapErasedGovernanceObjects << cmapInvalidVotes @@ -211,11 +211,11 @@ class GovernanceStore } template - void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs) + void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs_store) { Clear(); - LOCK(cs); + LOCK(cs_store); std::string strVersion; s >> strVersion; if (strVersion != SERIALIZATION_VERSION_STRING) { @@ -231,10 +231,10 @@ class GovernanceStore } void Clear() - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::string ToString() const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); }; // @@ -280,30 +280,30 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Basic initialization and querying bool IsValid() const override { return is_valid; } bool LoadCache(bool load_cache) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::string ToString() const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); UniValue ToJson() const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void Clear() - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void CheckAndRemove() - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman) - EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); // CGovernanceObject bool AreRateChecksEnabled() const { return fRateChecksEnabled; } void AddInvalidVote(const CGovernanceVote& vote) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); // Getters/Setters int GetCachedBlockHeight() const override { return nCachedBlockHeight; } int64_t GetLastDiffTime() const { return nTimeLastDiff; } std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } // Networking @@ -313,17 +313,17 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent * false. (Note logic is inverted in AlreadyHave). */ bool ConfirmInventoryRequest(const CInv& inv) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override - EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); int RequestGovernanceObjectVotes(const std::vector& vNodesCopy, CConnman& connman, const PeerManager& peerman) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) - EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void RelayVote(const CGovernanceVote& vote) @@ -331,85 +331,85 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Notification interface trigger void UpdatedBlockTip(const CBlockIndex* pindex) - EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); // Signer interface bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); // Superblocks bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); // Thread-safe accessors bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool HaveObjectForHash(const uint256& nHash) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool HaveVoteForHash(const uint256& nHash) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); CGovernanceObject* FindGovernanceObject(const uint256& nHash) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); int GetVoteCount() const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::vector> GetActiveTriggers() const override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void AddPostponedObject(const CGovernanceObject& govobj) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); private: // Branches of ProcessMessage [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); // Internal counterparts to "Thread-safe accessors" void AddPostponedObjectInternal(const CGovernanceObject& govobj) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); std::vector> GetActiveTriggersInternal() const - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void MasternodeRateUpdate(const CGovernanceObject& govobj) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void CheckPostponedObjects() - EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(cs_store, !cs_relay); void InitOnLoad() - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); /* * Trigger Management (formerly CGovernanceTriggerManager) @@ -417,40 +417,40 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent * - After triggers are activated and executed, they can be removed */ bool AddNewTrigger(uint256 nHash) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void CleanAndRemoveTriggers() - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); /// Called to indicate a requested object or vote has been received bool AcceptMessage(const uint256& nHash) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void CheckOrphanVotes(CGovernanceObject& govobj) - EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay); + EXCLUSIVE_LOCKS_REQUIRED(cs_store, !cs_relay); void RebuildIndexes() - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void AddCachedTriggers() - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); void RequestOrphanObjects(CConnman& connman) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void CleanOrphanObjects() - EXCLUSIVE_LOCKS_REQUIRED(!cs); + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void RemoveInvalidVotes() - EXCLUSIVE_LOCKS_REQUIRED(cs); + EXCLUSIVE_LOCKS_REQUIRED(cs_store); }; bool AreSuperblocksEnabled(const CSporkManager& sporkman); From 2378a99bd236834f4247db73a3a41b6ffe162359 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 19:50:18 +0000 Subject: [PATCH 10/18] trivial: tidy-up `governance/object.h` Needed to make the next commit easier to follow. --- src/governance/object.h | 108 ++++++++++------------------------------ 1 file changed, 26 insertions(+), 82 deletions(-) diff --git a/src/governance/object.h b/src/governance/object.h index 5e0374520859..3f0cc0047bfd 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -97,7 +97,6 @@ class CGovernanceObject /// time this object was marked for deletion int64_t nDeletionTime{0}; - /// is valid by blockchain bool fCachedLocalValidity{false}; std::string strLocalValidityError; @@ -133,99 +132,45 @@ class CGovernanceObject public: CGovernanceObject(); - CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTime, const uint256& nCollateralHashIn, const std::string& strDataHexIn); - CGovernanceObject(const CGovernanceObject& other); - // Public Getter methods - - const Governance::Object& Object() const - { - return m_obj; - } - - int64_t GetCreationTime() const - { - return m_obj.time; - } - - int64_t GetDeletionTime() const - { - return nDeletionTime; - } - - GovernanceObject GetObjectType() const - { - return m_obj.type; - } - - const uint256& GetCollateralHash() const - { - return m_obj.collateralHash; - } - - const COutPoint& GetMasternodeOutpoint() const - { - return m_obj.masternodeOutpoint; - } - - bool IsSetCachedFunding() const - { - return fCachedFunding; - } - - bool IsSetCachedValid() const - { - return fCachedValid; - } - - bool IsSetCachedDelete() const - { - return fCachedDelete; - } - - bool IsSetCachedEndorsed() const - { - return fCachedEndorsed; - } - - bool IsSetDirtyCache() const - { - return fDirtyCache; - } - - bool IsSetExpired() const - { - return fExpired; - } - - void SetExpired() - { - fExpired = true; - } - - const CGovernanceObjectVoteFile& GetVoteFile() const - { - return fileVotes; - } - - // Signature related functions - + // Getters + bool IsSetCachedFunding() const { return fCachedFunding; } + bool IsSetCachedValid() const { return fCachedValid; } + bool IsSetCachedDelete() const { return fCachedDelete; } + bool IsSetCachedEndorsed() const { return fCachedEndorsed; } + bool IsSetDirtyCache() const { return fDirtyCache; } + bool IsSetExpired() const { return fExpired; } + GovernanceObject GetObjectType() const { return m_obj.type; } + int64_t GetCreationTime() const { return m_obj.time; } + int64_t GetDeletionTime() const { return nDeletionTime; } + + const CGovernanceObjectVoteFile& GetVoteFile() const { return fileVotes; } + const COutPoint& GetMasternodeOutpoint() const { return m_obj.masternodeOutpoint; } + const Governance::Object& Object() const { return m_obj; } + const uint256& GetCollateralHash() const { return m_obj.collateralHash; } + + // Setters + void SetExpired() { fExpired = true; } void SetMasternodeOutpoint(const COutPoint& outpoint); void SetSignature(Span sig); - bool CheckSignature(const CBLSPublicKey& pubKey) const; + // Signature related functions + bool CheckSignature(const CBLSPublicKey& pubKey) const; uint256 GetSignatureHash() const; // CORE OBJECT FUNCTIONS - bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /// Check the collateral transaction for the budget proposal/finalized budget - bool IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -297,5 +242,4 @@ class CGovernanceObject std::set RemoveInvalidVotes(const CDeterministicMNList& tip_mn_list, const COutPoint& mnOutpoint); }; - #endif // BITCOIN_GOVERNANCE_OBJECT_H From 2ee408fb38e810fba1318b64a87adfce891f4a73 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 23 Oct 2025 00:19:50 +0530 Subject: [PATCH 11/18] refactor: protect some `CGovernanceObject` variables Currently, the LOCKs aren't guarding anything. Variables protected were based on existing LOCK'ing patterns (i.e. (de)ser'ed variables except for m_obj). --- src/governance/governance.cpp | 15 ++++++---- src/governance/object.cpp | 1 + src/governance/object.h | 53 ++++++++++++++++++++++++++--------- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index f3df5d636c2b..9ad5c6942da5 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -187,7 +187,7 @@ bool CGovernanceManager::HaveVoteForHash(const uint256& nHash) const LOCK(cs_store); CGovernanceObject* pGovobj = nullptr; - return cmapVoteToObject.Get(nHash, pGovobj) && pGovobj->GetVoteFile().HasVote(nHash); + return cmapVoteToObject.Get(nHash, pGovobj) && WITH_LOCK(pGovobj->cs, return pGovobj->GetVoteFile().HasVote(nHash)); } int CGovernanceManager::GetVoteCount() const @@ -201,7 +201,7 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& LOCK(cs_store); CGovernanceObject* pGovobj = nullptr; - return cmapVoteToObject.Get(nHash, pGovobj) && pGovobj->GetVoteFile().SerializeVoteToStream(nHash, ss); + return cmapVoteToObject.Get(nHash, pGovobj) && WITH_LOCK(pGovobj->cs, return pGovobj->GetVoteFile().SerializeVoteToStream(nHash, ss)); } void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) @@ -744,10 +744,12 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons return {}; } - const auto& fileVotes = govobj.GetVoteFile(); + MessageProcessingResult ret{}; const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); - MessageProcessingResult ret{}; + { + LOCK(govobj.cs); + const auto& fileVotes = govobj.GetVoteFile(); for (const auto& vote : fileVotes.GetVotes()) { uint256 nVoteHash = vote.GetHash(); @@ -758,6 +760,7 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons } ret.m_inventory.emplace_back(MSG_GOVERNANCE_OBJECT_VOTE, nVoteHash); } + } CNetMsgMaker msgMaker(peer.GetCommonVersion()); connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ_VOTE, @@ -1064,7 +1067,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH if (pObj) { filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRand(/*nMax=*/999999), BLOOM_UPDATE_ALL); - std::vector vecVotes = pObj->GetVoteFile().GetVotes(); + std::vector vecVotes = WITH_LOCK(pObj->cs, return pObj->GetVoteFile().GetVotes()); nVoteCount = vecVotes.size(); for (const auto& vote : vecVotes) { filter.insert(vote.GetHash()); @@ -1222,7 +1225,7 @@ void CGovernanceManager::RebuildIndexes() cmapVoteToObject.Clear(); for (auto& objPair : mapObjects) { CGovernanceObject& govobj = objPair.second; - std::vector vecVotes = govobj.GetVoteFile().GetVotes(); + std::vector vecVotes = WITH_LOCK(govobj.cs, return govobj.GetVoteFile().GetVotes()); for (const auto& vecVote : vecVotes) { cmapVoteToObject.Insert(vecVote.GetHash(), &govobj); } diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 8bea754eea50..6ecfd6531433 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -614,6 +614,7 @@ void CGovernanceObject::UpdateSentinelVariables(const CDeterministicMNList& tip_ if (GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING) >= nAbsVoteReq) fCachedFunding = true; if ((GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_DELETE) >= nAbsDeleteReq) && !fCachedDelete) { fCachedDelete = true; + LOCK(cs); if (nDeletionTime == 0) { nDeletionTime = GetTime().count(); } diff --git a/src/governance/object.h b/src/governance/object.h index 3f0cc0047bfd..19c5ce5f447d 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -88,14 +88,15 @@ class CGovernanceObject public: // Types using vote_m_t = std::map; -private: +public: /// critical section to protect the inner data structures mutable RecursiveMutex cs; +private: Governance::Object m_obj; /// time this object was marked for deletion - int64_t nDeletionTime{0}; + int64_t nDeletionTime GUARDED_BY(cs){0}; /// is valid by blockchain bool fCachedLocalValidity{false}; @@ -121,14 +122,14 @@ class CGovernanceObject bool fDirtyCache{true}; /// Object is no longer of interest - bool fExpired{false}; + bool fExpired GUARDED_BY(cs){false}; /// Failed to parse object data bool fUnparsable{false}; - vote_m_t mapCurrentMNVotes; + vote_m_t mapCurrentMNVotes GUARDED_BY(cs); - CGovernanceObjectVoteFile fileVotes; + CGovernanceObjectVoteFile fileVotes GUARDED_BY(cs); public: CGovernanceObject(); @@ -141,18 +142,31 @@ class CGovernanceObject bool IsSetCachedDelete() const { return fCachedDelete; } bool IsSetCachedEndorsed() const { return fCachedEndorsed; } bool IsSetDirtyCache() const { return fDirtyCache; } - bool IsSetExpired() const { return fExpired; } + bool IsSetExpired() const EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return WITH_LOCK(cs, return fExpired); + } GovernanceObject GetObjectType() const { return m_obj.type; } int64_t GetCreationTime() const { return m_obj.time; } - int64_t GetDeletionTime() const { return nDeletionTime; } + int64_t GetDeletionTime() const EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + return WITH_LOCK(cs, return nDeletionTime); + } - const CGovernanceObjectVoteFile& GetVoteFile() const { return fileVotes; } + const CGovernanceObjectVoteFile& GetVoteFile() const EXCLUSIVE_LOCKS_REQUIRED(cs) + { + AssertLockHeld(cs); + return fileVotes; + } const COutPoint& GetMasternodeOutpoint() const { return m_obj.masternodeOutpoint; } const Governance::Object& Object() const { return m_obj; } const uint256& GetCollateralHash() const { return m_obj.collateralHash; } // Setters - void SetExpired() { fExpired = true; } + void SetExpired() EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + WITH_LOCK(cs, fExpired = true); + } void SetMasternodeOutpoint(const COutPoint& outpoint); void SetSignature(Span sig); @@ -177,9 +191,10 @@ class CGovernanceObject void UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list); - void PrepareDeletion(int64_t nDeletionTime_) + void PrepareDeletion(int64_t nDeletionTime_) EXCLUSIVE_LOCKS_REQUIRED(!cs) { fCachedDelete = true; + LOCK(cs); if (nDeletionTime == 0) { nDeletionTime = nDeletionTime_; } @@ -211,15 +226,27 @@ class CGovernanceObject // SERIALIZER - SERIALIZE_METHODS(CGovernanceObject, obj) + template + void Serialize(Stream& s) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { // SERIALIZE DATA FOR SAVING/LOADING OR NETWORK FUNCTIONS - READWRITE(obj.m_obj); + s << m_obj; if (s.GetType() & SER_DISK) { // Only include these for the disk file format - READWRITE(obj.nDeletionTime, obj.fExpired, obj.mapCurrentMNVotes, obj.fileVotes); + LOCK(cs); + s << nDeletionTime << fExpired << mapCurrentMNVotes << fileVotes; } + } + template + void Unserialize(Stream& s) EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + s >> m_obj; + if (s.GetType() & SER_DISK) { + // Only include these for the disk file format + LOCK(cs); + s >> nDeletionTime >> fExpired >> mapCurrentMNVotes >> fileVotes; + } // AFTER DESERIALIZATION OCCURS, CACHED VARIABLES MUST BE CALCULATED MANUALLY } From 009f5cb16a9e9f3967b0a1fa3a42aa38d038bc92 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 23 Oct 2025 00:20:33 +0530 Subject: [PATCH 12/18] refactor: add `cs` annotations for `CGovernanceObject` --- src/governance/object.cpp | 7 +++++++ src/governance/object.h | 33 ++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 6ecfd6531433..b46660600905 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -554,26 +554,31 @@ int CGovernanceObject::CountMatchingVotes(const CDeterministicMNList& tip_mn_lis int CGovernanceObject::GetAbsoluteYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const { + AssertLockNotHeld(cs); return GetYesCount(tip_mn_list, eVoteSignalIn) - GetNoCount(tip_mn_list, eVoteSignalIn); } int CGovernanceObject::GetAbsoluteNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const { + AssertLockNotHeld(cs); return GetNoCount(tip_mn_list, eVoteSignalIn) - GetYesCount(tip_mn_list, eVoteSignalIn); } int CGovernanceObject::GetYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const { + AssertLockNotHeld(cs); return CountMatchingVotes(tip_mn_list, eVoteSignalIn, VOTE_OUTCOME_YES); } int CGovernanceObject::GetNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const { + AssertLockNotHeld(cs); return CountMatchingVotes(tip_mn_list, eVoteSignalIn, VOTE_OUTCOME_NO); } int CGovernanceObject::GetAbstainCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const { + AssertLockNotHeld(cs); return CountMatchingVotes(tip_mn_list, eVoteSignalIn, VOTE_OUTCOME_ABSTAIN); } @@ -591,6 +596,8 @@ bool CGovernanceObject::GetCurrentMNVotes(const COutPoint& mnCollateralOutpoint, void CGovernanceObject::UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list) { + AssertLockNotHeld(cs); + // CALCULATE MINIMUM SUPPORT LEVELS REQUIRED int nWeightedMnCount = (int)tip_mn_list.GetValidWeightedMNsCount(); diff --git a/src/governance/object.h b/src/governance/object.h index 19c5ce5f447d..32d5a88e3737 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -189,7 +189,8 @@ class CGovernanceObject void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - void UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list); + void UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list) + EXCLUSIVE_LOCKS_REQUIRED(!cs); void PrepareDeletion(int64_t nDeletionTime_) EXCLUSIVE_LOCKS_REQUIRED(!cs) { @@ -209,15 +210,22 @@ class CGovernanceObject // GET VOTE COUNT FOR SIGNAL - int CountMatchingVotes(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn, vote_outcome_enum_t eVoteOutcomeIn) const; + int CountMatchingVotes(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn, vote_outcome_enum_t eVoteOutcomeIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); - int GetAbsoluteYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const; - int GetAbsoluteNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const; - int GetYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const; - int GetNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const; - int GetAbstainCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const; + int GetAbsoluteYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetAbsoluteNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetYesCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetNoCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + int GetAbstainCount(const CDeterministicMNList& tip_mn_list, vote_signal_enum_t eVoteSignalIn) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool GetCurrentMNVotes(const COutPoint& mnCollateralOutpoint, vote_rec_t& voteRecord) const; + bool GetCurrentMNVotes(const COutPoint& mnCollateralOutpoint, vote_rec_t& voteRecord) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); // FUNCTIONS FOR DEALING WITH DATA STRING @@ -257,16 +265,19 @@ class CGovernanceObject void GetData(UniValue& objResult) const; bool ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, - const CGovernanceVote& vote, CGovernanceException& exception); + const CGovernanceVote& vote, CGovernanceException& exception) + EXCLUSIVE_LOCKS_REQUIRED(!cs); /// Called when MN's which have voted on this object have been removed - void ClearMasternodeVotes(const CDeterministicMNList& tip_mn_list); + void ClearMasternodeVotes(const CDeterministicMNList& tip_mn_list) + EXCLUSIVE_LOCKS_REQUIRED(!cs); // Revalidate all votes from this MN and delete them if validation fails. // This is the case for DIP3 MNs that changed voting or operator keys and // also for MNs that were removed from the list completely. // Returns deleted vote hashes. - std::set RemoveInvalidVotes(const CDeterministicMNList& tip_mn_list, const COutPoint& mnOutpoint); + std::set RemoveInvalidVotes(const CDeterministicMNList& tip_mn_list, const COutPoint& mnOutpoint) + EXCLUSIVE_LOCKS_REQUIRED(!cs); }; #endif // BITCOIN_GOVERNANCE_OBJECT_H From 14c44cbd375f121e308d0881cd1c7586b8548539 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Sep 2025 20:14:27 +0000 Subject: [PATCH 13/18] refactor: switch `CGovernanceObject::cs` to `Mutex` --- src/governance/object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/governance/object.h b/src/governance/object.h index 32d5a88e3737..439c9d190b77 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -90,7 +90,7 @@ class CGovernanceObject public: /// critical section to protect the inner data structures - mutable RecursiveMutex cs; + mutable Mutex cs; private: Governance::Object m_obj; From 035dfeefa484d2e32b744665f5348a1ff2680dfd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 20:13:48 +0530 Subject: [PATCH 14/18] fix: resolve lock order issues --- src/governance/governance.cpp | 21 ++++++++++++++------- src/governance/governance.h | 11 +++++++++-- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 9ad5c6942da5..d944c1e86ed8 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -314,7 +314,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman return ret; } - AddGovernanceObject(govobj, &peer); + AddGovernanceObjectInternal(govobj, &peer); return ret; } @@ -392,9 +392,12 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) } } -void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom) +void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& govobj, const CNode* pfrom) { + AssertLockHeld(::cs_main); + AssertLockHeld(cs_store); AssertLockNotHeld(cs_relay); + uint256 nHash = govobj.GetHash(); std::string strHash = nHash.ToString(); @@ -404,7 +407,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 @@ -453,6 +455,12 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj.Object()), nHash.ToString()); } +void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom) +{ + LOCK2(::cs_main, cs_store); + AddGovernanceObjectInternal(govobj, pfrom); +} + void CGovernanceManager::CheckAndRemove() { AssertLockNotHeld(cs_store); @@ -980,12 +988,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; @@ -997,7 +1004,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); + AddGovernanceObjectInternal(govobj); } else { LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString()); } @@ -1364,7 +1371,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(); } diff --git a/src/governance/governance.h b/src/governance/governance.h index f63b10597124..7093e574126d 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -56,6 +56,8 @@ using vote_time_pair_t = std::pair; static constexpr int RATE_BUFFER_SIZE = 5; static constexpr bool DEFAULT_GOVERNANCE_ENABLE{true}; +extern RecursiveMutex cs_main; + class CRateCheckBuffer { private: @@ -341,7 +343,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::vector> 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, @@ -399,6 +401,11 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs_store); + // Internal counterpart to "Signer interface" + void AddGovernanceObjectInternal(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); @@ -406,7 +413,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent 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); From 0bc28c314cfffb68391d2128a68a22ee4648eb14 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 20:28:30 +0530 Subject: [PATCH 15/18] fix: resolve potential deadlock --- src/governance/governance.cpp | 13 ++++++------- src/governance/governance.h | 2 -- src/governance/object.cpp | 1 - 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index d944c1e86ed8..3394556a1bf5 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -981,7 +981,12 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, return false; } - bool fOk = govobj.ProcessVote(m_mn_metaman, *this, Assert(m_dmnman)->GetListAtChainTip(), vote, exception) && cmapVoteToObject.Insert(nHashVote, &govobj); + bool fOk = govobj.ProcessVote(m_mn_metaman, *this, Assert(m_dmnman)->GetListAtChainTip(), vote, exception); + if (fOk) { + fOk = cmapVoteToObject.Insert(nHashVote, &govobj); + } else if (exception.GetType() == GOVERNANCE_EXCEPTION_PERMANENT_ERROR && exception.GetNodePenalty() == 20) { + cmapInvalidVotes.Insert(nHashVote, vote); + } LEAVE_CRITICAL_SECTION(cs_store); return fOk; } @@ -1086,12 +1091,6 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter)); } -void CGovernanceManager::AddInvalidVote(const CGovernanceVote& vote) -{ - LOCK(cs_store); - cmapInvalidVotes.Insert(vote.GetHash(), vote); -} - int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const { const std::vector vNodeCopy{&peer}; diff --git a/src/governance/governance.h b/src/governance/governance.h index 7093e574126d..420e8f13dd1d 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -296,8 +296,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // CGovernanceObject bool AreRateChecksEnabled() const { return fRateChecksEnabled; } - void AddInvalidVote(const CGovernanceVote& vote) - EXCLUSIVE_LOCKS_REQUIRED(!cs_store); // Getters/Setters int GetCachedBlockHeight() const override { return nCachedBlockHeight; } diff --git a/src/governance/object.cpp b/src/governance/object.cpp index b46660600905..3b44aaaffd1a 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -144,7 +144,6 @@ bool CGovernanceObject::ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceM __func__, vote.GetMasternodeOutpoint().ToStringShort(), GetHash().ToString(), vote.GetHash().ToString())}; LogPrintf("%s\n", msg); exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); - govman.AddInvalidVote(vote); return false; } From 828cad56c4e0e06f107134e6db0bd73dda25f0cf Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 16:45:56 +0530 Subject: [PATCH 16/18] trivial: use structured bindings where possible, limit iterator scope --- src/governance/governance.cpp | 141 +++++++++++++++------------------- src/governance/object.cpp | 5 +- src/rpc/governance.cpp | 5 +- 3 files changed, 66 insertions(+), 85 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 3394556a1bf5..549ee214b24f 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -377,10 +377,10 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) int64_t nNow = GetAdjustedTime(); const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); for (const auto& pairVote : vecVotePairs) { + const auto& [vote, time] = pairVote; bool fRemove = false; - const CGovernanceVote& vote = pairVote.first; CGovernanceException e; - if (pairVote.second < nNow) { + if (time < nNow) { fRemove = true; } else if (govobj.ProcessVote(m_mn_metaman, *this, tip_mn_list, vote, e)) { RelayVote(vote); @@ -392,50 +392,51 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) } } -void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& govobj, const CNode* pfrom) +void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_obj, const CNode* pfrom) { AssertLockHeld(::cs_main); AssertLockHeld(cs_store); AssertLockNotHeld(cs_relay); - uint256 nHash = govobj.GetHash(); + uint256 nHash = insert_obj.GetHash(); std::string strHash = nHash.ToString(); const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); // UPDATE CACHED VARIABLES FOR THIS OBJECT AND ADD IT TO OUR MANAGED DATA - govobj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object + insert_obj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object std::string strError; // MAKE SURE THIS OBJECT IS OK - if (!govobj.IsValidLocally(tip_mn_list, m_chainman, strError, true)) { + if (!insert_obj.IsValidLocally(tip_mn_list, m_chainman, strError, true)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- invalid governance object - %s - (nCachedBlockHeight %d) \n", strError, nCachedBlockHeight); return; } LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(), - ToUnderlying(govobj.GetObjectType())); + ToUnderlying(insert_obj.GetObjectType())); // INSERT INTO OUR GOVERNANCE OBJECT MEMORY // IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY - auto objpair = mapObjects.emplace(nHash, govobj); + auto [emplace_ret, emplace_status] = mapObjects.emplace(nHash, insert_obj); - if (!objpair.second) { + if (!emplace_status) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); return; } // SHOULD WE ADD THIS OBJECT TO ANY OTHER MANAGERS? + auto& [_, govobj] = *emplace_ret; LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n", govobj.GetDataAsPlainString(), ToUnderlying(govobj.GetObjectType())); if (govobj.GetObjectType() == GovernanceObject::TRIGGER && !AddNewTrigger(nHash)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); - objpair.first->second.PrepareDeletion(GetTime().count()); + govobj.PrepareDeletion(GetTime().count()); return; } @@ -491,41 +492,36 @@ void CGovernanceManager::CheckAndRemove() // Clean up any expired or invalid triggers CleanAndRemoveTriggers(); - auto it = mapObjects.begin(); const auto nNow = GetTime(); - - while (it != mapObjects.end()) { - CGovernanceObject* pObj = &((*it).second); - - uint256 nHash = it->first; + for (auto it = mapObjects.begin(); it != mapObjects.end();) { + auto [nHash, pObj] = *it; std::string strHash = nHash.ToString(); // IF CACHE IS NOT DIRTY, WHY DO THIS? - if (pObj->IsSetDirtyCache()) { + if (pObj.IsSetDirtyCache()) { // UPDATE LOCAL VALIDITY AGAINST CRYPTO DATA - pObj->UpdateLocalValidity(tip_mn_list, m_chainman); + pObj.UpdateLocalValidity(tip_mn_list, m_chainman); // UPDATE SENTINEL SIGNALING VARIABLES - pObj->UpdateSentinelVariables(tip_mn_list); + pObj.UpdateSentinelVariables(tip_mn_list); } // IF DELETE=TRUE, THEN CLEAN THE MESS UP! - const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj->GetDeletionTime()}; + const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj.GetDeletionTime()}; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- Checking object for deletion: %s, deletion time = %d, time since deletion = %d, delete flag = %d, expired flag = %d\n", - strHash, pObj->GetDeletionTime(), nTimeSinceDeletion.count(), pObj->IsSetCachedDelete(), pObj->IsSetExpired()); + strHash, pObj.GetDeletionTime(), nTimeSinceDeletion.count(), pObj.IsSetCachedDelete(), pObj.IsSetExpired()); - if ((pObj->IsSetCachedDelete() || pObj->IsSetExpired()) && + if ((pObj.IsSetCachedDelete() || pObj.IsSetExpired()) && (nTimeSinceDeletion >= GOVERNANCE_DELETION_DELAY)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- erase obj %s\n", (*it).first.ToString()); - m_mn_metaman.RemoveGovernanceObject(pObj->GetHash()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- erase obj %s\n", nHash.ToString()); + m_mn_metaman.RemoveGovernanceObject(pObj.GetHash()); // Remove vote references const object_ref_cm_t::list_t& listItems = cmapVoteToObject.GetItemList(); - auto lit = listItems.begin(); - while (lit != listItems.end()) { - if (lit->value == pObj) { + for (auto lit = listItems.begin(); lit != listItems.end();) { + if (lit->value == &pObj) { uint256 nKey = lit->key; ++lit; cmapVoteToObject.Erase(nKey); @@ -536,12 +532,12 @@ void CGovernanceManager::CheckAndRemove() int64_t nTimeExpired{0}; - if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { + if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) { // keep hashes of deleted proposals forever nTimeExpired = std::numeric_limits::max(); } else { int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing; - nTimeExpired = (std::chrono::seconds{pObj->GetCreationTime()} + + nTimeExpired = (std::chrono::seconds{pObj.GetCreationTime()} + std::chrono::seconds{2 * nSuperblockCycleSeconds} + GOVERNANCE_DELETION_DELAY) .count(); } @@ -549,11 +545,11 @@ void CGovernanceManager::CheckAndRemove() mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired)); mapObjects.erase(it++); } else { - if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { - CProposalValidator validator(pObj->GetDataAsHexString()); + if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) { + CProposalValidator validator(pObj.GetDataAsHexString()); if (!validator.Validate()) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", strHash); - pObj->PrepareDeletion(nNow.count()); + pObj.PrepareDeletion(nNow.count()); } } ++it; @@ -561,8 +557,7 @@ void CGovernanceManager::CheckAndRemove() } // forget about expired deleted objects - auto s_it = mapErasedGovernanceObjects.begin(); - while (s_it != mapErasedGovernanceObjects.end()) { + for (auto s_it = mapErasedGovernanceObjects.begin(); s_it != mapErasedGovernanceObjects.end();) { if (s_it->second < nNow.count()) { mapErasedGovernanceObjects.erase(s_it++); } else { @@ -571,8 +566,7 @@ void CGovernanceManager::CheckAndRemove() } // forget about expired requests - auto r_it = m_requested_hash_time.begin(); - while (r_it != m_requested_hash_time.end()) { + for (auto r_it = m_requested_hash_time.begin(); r_it != m_requested_hash_time.end();) { if (r_it->second < nNow) { m_requested_hash_time.erase(r_it++); } else { @@ -619,8 +613,8 @@ CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint { AssertLockNotHeld(cs_store); LOCK(cs_store); - for (const auto& [nHash, object] : mapObjects) { - if (object.GetDataHash() == nDataHash) return &mapObjects[nHash]; + for (const auto& [nHash, govobj] : mapObjects) { + if (govobj.GetDataHash() == nDataHash) return &mapObjects[nHash]; } return nullptr; } @@ -649,13 +643,13 @@ std::vector CGovernanceManager::GetCurrentVotes(const uint256& } // Loop through each MN collateral outpoint and get the votes for the `nParentHash` governance object - for (const auto& mnpair : mapMasternodes) { + for (const auto& [outpoint, _] : mapMasternodes) { // get a vote_rec_t from the govobj vote_rec_t voteRecord; - if (!govobj.GetCurrentMNVotes(mnpair.first, voteRecord)) continue; + if (!govobj.GetCurrentMNVotes(outpoint, voteRecord)) continue; for (const auto& [signal, vote_instance] : voteRecord.mapInstances) { - CGovernanceVote vote = CGovernanceVote(mnpair.first, nParentHash, (vote_signal_enum_t)signal, + CGovernanceVote vote = CGovernanceVote(outpoint, nParentHash, (vote_signal_enum_t)signal, vote_instance.eOutcome); vote.SetTime(vote_instance.nCreationTime); vecResult.push_back(vote); @@ -669,14 +663,14 @@ void CGovernanceManager::GetAllNewerThan(std::vector& objs, i { LOCK(cs_store); - for (const auto& objPair : mapObjects) { + for (const auto& [_, govobj] : mapObjects) { // IF THIS OBJECT IS OLDER THAN TIME, CONTINUE - if (objPair.second.GetCreationTime() < nMoreThanTime) { + if (govobj.GetCreationTime() < nMoreThanTime) { continue; } // ADD GOVERNANCE OBJECT TO LIST - objs.push_back(objPair.second); + objs.push_back(govobj); } } @@ -799,11 +793,8 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c // all valid objects, no votes MessageProcessingResult ret{}; - for (const auto& objPair : mapObjects) { - uint256 nHash = objPair.first; - const CGovernanceObject& govobj = objPair.second; + for (const auto& [nHash, govobj] : mapObjects) { std::string strHash = nHash.ToString(); - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { @@ -1140,8 +1131,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& for (const auto& [nHash, govobj] : mapObjects) { if (govobj.IsSetCachedDelete()) continue; if (mapAskedRecently.count(nHash)) { - auto it = mapAskedRecently[nHash].begin(); - while (it != mapAskedRecently[nHash].end()) { + for (auto it = mapAskedRecently[nHash].begin(); it != mapAskedRecently[nHash].end();) { if (it->second < nNow) { mapAskedRecently[nHash].erase(it++); } else { @@ -1229,8 +1219,7 @@ void CGovernanceManager::RebuildIndexes() AssertLockHeld(cs_store); cmapVoteToObject.Clear(); - for (auto& objPair : mapObjects) { - CGovernanceObject& govobj = objPair.second; + for (auto& [_, govobj] : mapObjects) { std::vector vecVotes = WITH_LOCK(govobj.cs, return govobj.GetVoteFile().GetVotes()); for (const auto& vecVote : vecVotes) { cmapVoteToObject.Insert(vecVote.GetHash(), &govobj); @@ -1244,9 +1233,7 @@ void CGovernanceManager::AddCachedTriggers() int64_t nNow = GetTime().count(); - for (auto& objpair : mapObjects) { - CGovernanceObject& govobj = objpair.second; - + for (auto& [_, govobj] : mapObjects) { if (govobj.GetObjectType() != GovernanceObject::TRIGGER) { continue; } @@ -1297,8 +1284,8 @@ std::string GovernanceStore::ToString() const int nTriggerCount = 0; int nOtherCount = 0; - for (const auto& objPair : mapObjects) { - switch (objPair.second.GetObjectType()) { + for (const auto& [_, govobj] : mapObjects) { + switch (govobj.GetObjectType()) { case GovernanceObject::PROPOSAL: nProposalCount++; break; @@ -1330,8 +1317,8 @@ UniValue CGovernanceManager::ToJson() const int nTriggerCount = 0; int nOtherCount = 0; - for (const auto& objpair : mapObjects) { - switch (objpair.second.GetObjectType()) { + for (const auto& [_, govobj] : mapObjects) { + switch (govobj.GetObjectType()) { case GovernanceObject::PROPOSAL: nProposalCount++; break; @@ -1415,12 +1402,11 @@ void CGovernanceManager::CleanOrphanObjects() int64_t nNow = GetTime().count(); - auto it = items.begin(); - while (it != items.end()) { + for (auto it = items.begin(); it != items.end();) { auto prevIt = it; ++it; - const vote_time_pair_t& pairVote = prevIt->value; - if (pairVote.second < nNow) { + const auto& [vote, time] = prevIt->value; + if (time < nNow) { cmmapOrphanVotes.Erase(prevIt->key, prevIt->value); } } @@ -1438,14 +1424,14 @@ void CGovernanceManager::RemoveInvalidVotes() auto diff = lastMNListForVotingKeys->BuildDiff(tip_mn_list); std::vector changedKeyMNs; - for (const auto& p : diff.updatedMNs) { - auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(p.first); + for (const auto& [internalId, pdmnState] : diff.updatedMNs) { + auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId); // BuildDiff will construct itself with MNs that we already have knowledge // of, meaning that fetch operations should never fail. assert(oldDmn); - if ((p.second.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && p.second.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) { + if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) { changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); - } else if ((p.second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && p.second.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { + } else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } } @@ -1458,8 +1444,8 @@ void CGovernanceManager::RemoveInvalidVotes() } for (const auto& outpoint : changedKeyMNs) { - for (auto& p : mapObjects) { - auto removed = p.second.RemoveInvalidVotes(tip_mn_list, outpoint); + for (auto& [_, govobj] : mapObjects) { + auto removed = govobj.RemoveInvalidVotes(tip_mn_list, outpoint); if (removed.empty()) { continue; } @@ -1526,8 +1512,7 @@ void CGovernanceManager::CleanAndRemoveTriggers() // Remove triggers that are invalid or expired LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- mapTrigger.size() = %d\n", __func__, mapTrigger.size()); - auto it = mapTrigger.begin(); - while (it != mapTrigger.end()) { + for (auto it = mapTrigger.begin(); it != mapTrigger.end();) { bool remove = false; CGovernanceObject* pObj = nullptr; const CSuperblock_sptr& pSuperblock = it->second; @@ -1601,10 +1586,10 @@ std::vector CGovernanceManager::GetActiveTriggersInternal() co std::vector vecResults; // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS - for (const auto& pair : mapTrigger) { - const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(pair.first); + for (const auto& [nHash, pSuperblock] : mapTrigger) { + const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash); if (pObj) { - vecResults.push_back(pair.second); + vecResults.push_back(pSuperblock); } } @@ -1792,13 +1777,13 @@ std::vector> CGovernanceManager::GetApp const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); LOCK(cs_store); - for (const auto& [unused, object] : mapObjects) { + for (const auto& [_, govobj] : mapObjects) { // Skip all non-proposals objects - if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; + if (govobj.GetObjectType() != GovernanceObject::PROPOSAL) continue; // Skip non-passing proposals - const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + const int absYesCount = govobj.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); if (absYesCount < nAbsVoteReq) continue; - ret.emplace_back(std::make_shared(object)); + ret.emplace_back(std::make_shared(govobj)); } // Sort approved proposals by absolute Yes votes descending diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 3b44aaaffd1a..cc961d4d4d81 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -534,13 +534,12 @@ int CGovernanceObject::CountMatchingVotes(const CDeterministicMNList& tip_mn_lis LOCK(cs); int nCount = 0; - for (const auto& votepair : mapCurrentMNVotes) { - const vote_rec_t& recVote = votepair.second; + for (const auto& [outpoint, recVote] : mapCurrentMNVotes) { auto it2 = recVote.mapInstances.find(eVoteSignalIn); if (it2 != recVote.mapInstances.end() && it2->second.eOutcome == eVoteOutcomeIn) { // 4x times weight vote for EvoNode owners. // No need to check if v19 is active since no EvoNode are allowed to register before v19s - auto dmn = tip_mn_list.GetMNByCollateral(votepair.first); + auto dmn = tip_mn_list.GetMNByCollateral(outpoint); if (dmn != nullptr) nCount += GetMnType(dmn->nType).voting_weight; } } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 42bec108ed1c..4ebfb69cf081 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -427,10 +427,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet UniValue resultsObj(UniValue::VOBJ); - for (const auto& p : votingKeys) { - const auto& proTxHash = p.first; - const auto& keyID = p.second; - + for (const auto& [proTxHash, keyID] : votingKeys) { UniValue statusObj(UniValue::VOBJ); auto dmn = mnList.GetValidMN(proTxHash); From dc9787fcb5361a646a4f9a66ef79fa584f132b51 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 16:45:27 +0530 Subject: [PATCH 17/18] fix: use `std::shared_ptr` to manage `CGovernanceObject` lifetime --- src/governance/governance.cpp | 123 +++++++++++++++++----------------- src/governance/governance.h | 16 ++--- src/governance/object.h | 2 + src/governance/signing.cpp | 2 +- src/governance/signing.h | 4 +- src/rpc/governance.cpp | 8 +-- 6 files changed, 79 insertions(+), 76 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 549ee214b24f..39d9bd9792af 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -186,7 +186,7 @@ bool CGovernanceManager::HaveVoteForHash(const uint256& nHash) const { LOCK(cs_store); - CGovernanceObject* pGovobj = nullptr; + std::shared_ptr pGovobj{nullptr}; return cmapVoteToObject.Get(nHash, pGovobj) && WITH_LOCK(pGovobj->cs, return pGovobj->GetVoteFile().HasVote(nHash)); } @@ -200,7 +200,7 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& { LOCK(cs_store); - CGovernanceObject* pGovobj = nullptr; + std::shared_ptr pGovobj{nullptr}; return cmapVoteToObject.Get(nHash, pGovobj) && WITH_LOCK(pGovobj->cs, return pGovobj->GetVoteFile().SerializeVoteToStream(nHash, ss)); } @@ -213,7 +213,7 @@ void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) void CGovernanceManager::AddPostponedObjectInternal(const CGovernanceObject& govobj) { AssertLockHeld(cs_store); - mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); + mapPostponedObjects.emplace(govobj.GetHash(), std::make_shared(govobj)); } MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, @@ -421,7 +421,7 @@ void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_o // INSERT INTO OUR GOVERNANCE OBJECT MEMORY // IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY - auto [emplace_ret, emplace_status] = mapObjects.emplace(nHash, insert_obj); + auto [emplace_ret, emplace_status] = mapObjects.emplace(nHash, std::make_shared(insert_obj)); if (!emplace_status) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); @@ -432,28 +432,28 @@ void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_o auto& [_, govobj] = *emplace_ret; LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n", - govobj.GetDataAsPlainString(), ToUnderlying(govobj.GetObjectType())); + Assert(govobj)->GetDataAsPlainString(), ToUnderlying(govobj->GetObjectType())); - if (govobj.GetObjectType() == GovernanceObject::TRIGGER && !AddNewTrigger(nHash)) { + if (govobj->GetObjectType() == GovernanceObject::TRIGGER && !AddNewTrigger(nHash)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); - govobj.PrepareDeletion(GetTime().count()); + govobj->PrepareDeletion(GetTime().count()); return; } LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, pfrom ? pfrom->GetLogString() : "nullptr"); - RelayObject(govobj); + RelayObject(*govobj); // Update the rate buffer - MasternodeRateUpdate(govobj); + MasternodeRateUpdate(*govobj); m_mn_sync.BumpAssetLastTime("CGovernanceManager::AddGovernanceObject"); // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT - CheckOrphanVotes(govobj); + CheckOrphanVotes(*govobj); // SEND NOTIFICATION TO SCRIPT/ZMQ - GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj.Object()), nHash.ToString()); + GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj->Object()), nHash.ToString()); } void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom) @@ -484,7 +484,7 @@ void CGovernanceManager::CheckAndRemove() if (it == mapObjects.end()) { continue; } - it->second.ClearMasternodeVotes(tip_mn_list); + Assert(it->second)->ClearMasternodeVotes(tip_mn_list); } ScopedLockBool guard(cs_store, fRateChecksEnabled, false); @@ -498,30 +498,30 @@ void CGovernanceManager::CheckAndRemove() std::string strHash = nHash.ToString(); // IF CACHE IS NOT DIRTY, WHY DO THIS? - if (pObj.IsSetDirtyCache()) { + if (Assert(pObj)->IsSetDirtyCache()) { // UPDATE LOCAL VALIDITY AGAINST CRYPTO DATA - pObj.UpdateLocalValidity(tip_mn_list, m_chainman); + pObj->UpdateLocalValidity(tip_mn_list, m_chainman); // UPDATE SENTINEL SIGNALING VARIABLES - pObj.UpdateSentinelVariables(tip_mn_list); + pObj->UpdateSentinelVariables(tip_mn_list); } // IF DELETE=TRUE, THEN CLEAN THE MESS UP! - const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj.GetDeletionTime()}; + const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj->GetDeletionTime()}; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- Checking object for deletion: %s, deletion time = %d, time since deletion = %d, delete flag = %d, expired flag = %d\n", - strHash, pObj.GetDeletionTime(), nTimeSinceDeletion.count(), pObj.IsSetCachedDelete(), pObj.IsSetExpired()); + strHash, pObj->GetDeletionTime(), nTimeSinceDeletion.count(), pObj->IsSetCachedDelete(), pObj->IsSetExpired()); - if ((pObj.IsSetCachedDelete() || pObj.IsSetExpired()) && + if ((pObj->IsSetCachedDelete() || pObj->IsSetExpired()) && (nTimeSinceDeletion >= GOVERNANCE_DELETION_DELAY)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- erase obj %s\n", nHash.ToString()); - m_mn_metaman.RemoveGovernanceObject(pObj.GetHash()); + m_mn_metaman.RemoveGovernanceObject(pObj->GetHash()); // Remove vote references const object_ref_cm_t::list_t& listItems = cmapVoteToObject.GetItemList(); for (auto lit = listItems.begin(); lit != listItems.end();) { - if (lit->value == &pObj) { + if (lit->value == pObj) { uint256 nKey = lit->key; ++lit; cmapVoteToObject.Erase(nKey); @@ -532,12 +532,12 @@ void CGovernanceManager::CheckAndRemove() int64_t nTimeExpired{0}; - if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) { + if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { // keep hashes of deleted proposals forever nTimeExpired = std::numeric_limits::max(); } else { int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing; - nTimeExpired = (std::chrono::seconds{pObj.GetCreationTime()} + + nTimeExpired = (std::chrono::seconds{pObj->GetCreationTime()} + std::chrono::seconds{2 * nSuperblockCycleSeconds} + GOVERNANCE_DELETION_DELAY) .count(); } @@ -545,11 +545,11 @@ void CGovernanceManager::CheckAndRemove() mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired)); mapObjects.erase(it++); } else { - if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) { - CProposalValidator validator(pObj.GetDataAsHexString()); + if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { + CProposalValidator validator(pObj->GetDataAsHexString()); if (!validator.Validate()) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", strHash); - pObj.PrepareDeletion(nNow.count()); + pObj->PrepareDeletion(nNow.count()); } } ++it; @@ -579,42 +579,42 @@ void CGovernanceManager::CheckAndRemove() ToString(), m_requested_hash_time.size()); } -const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const +std::shared_ptr CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const { LOCK(cs_store); return FindConstGovernanceObjectInternal(nHash); } -const CGovernanceObject* CGovernanceManager::FindConstGovernanceObjectInternal(const uint256& nHash) const +std::shared_ptr CGovernanceManager::FindConstGovernanceObjectInternal(const uint256& nHash) const { AssertLockHeld(cs_store); auto it = mapObjects.find(nHash); - if (it != mapObjects.end()) return &(it->second); + if (it != mapObjects.end()) return (it->second); return nullptr; } -CGovernanceObject* CGovernanceManager::FindGovernanceObject(const uint256& nHash) +std::shared_ptr CGovernanceManager::FindGovernanceObject(const uint256& nHash) { AssertLockNotHeld(cs_store); LOCK(cs_store); return FindGovernanceObjectInternal(nHash); } -CGovernanceObject* CGovernanceManager::FindGovernanceObjectInternal(const uint256& nHash) +std::shared_ptr CGovernanceManager::FindGovernanceObjectInternal(const uint256& nHash) { AssertLockHeld(cs_store); - if (mapObjects.count(nHash)) return &mapObjects[nHash]; + if (mapObjects.count(nHash)) return mapObjects[nHash]; return nullptr; } -CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint256 &nDataHash) +std::shared_ptr CGovernanceManager::FindGovernanceObjectByDataHash(const uint256 &nDataHash) { AssertLockNotHeld(cs_store); LOCK(cs_store); for (const auto& [nHash, govobj] : mapObjects) { - if (govobj.GetDataHash() == nDataHash) return &mapObjects[nHash]; + if (Assert(govobj)->GetDataHash() == nDataHash) return govobj; } return nullptr; } @@ -627,7 +627,7 @@ std::vector CGovernanceManager::GetCurrentVotes(const uint256& // Find the governance object or short-circuit. auto it = mapObjects.find(nParentHash); if (it == mapObjects.end()) return vecResult; - const CGovernanceObject& govobj = it->second; + const auto& govobj = *Assert(it->second); const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); std::map mapMasternodes; @@ -665,12 +665,12 @@ void CGovernanceManager::GetAllNewerThan(std::vector& objs, i for (const auto& [_, govobj] : mapObjects) { // IF THIS OBJECT IS OLDER THAN TIME, CONTINUE - if (govobj.GetCreationTime() < nMoreThanTime) { + if (Assert(govobj)->GetCreationTime() < nMoreThanTime) { continue; } // ADD GOVERNANCE OBJECT TO LIST - objs.push_back(govobj); + objs.push_back(*govobj); } } @@ -735,7 +735,7 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- no matching object for hash %s, peer=%d\n", __func__, nProp.ToString(), peer.GetId()); return {}; } - const CGovernanceObject& govobj = it->second; + const auto& govobj = *Assert(it->second); std::string strHash = it->first.ToString(); LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); @@ -797,7 +797,7 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c std::string strHash = nHash.ToString(); LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); - if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { + if (Assert(govobj)->IsSetCachedDelete() || govobj->IsSetExpired()) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- not syncing deleted/expired govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); continue; @@ -963,7 +963,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, return false; } - CGovernanceObject& govobj = it->second; + auto& govobj = *Assert(it->second); if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- ignoring vote for expired or deleted object, hash = %s\n", @@ -974,7 +974,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, bool fOk = govobj.ProcessVote(m_mn_metaman, *this, Assert(m_dmnman)->GetListAtChainTip(), vote, exception); if (fOk) { - fOk = cmapVoteToObject.Insert(nHashVote, &govobj); + fOk = cmapVoteToObject.Insert(nHashVote, it->second); } else if (exception.GetType() == GOVERNANCE_EXCEPTION_PERMANENT_ERROR && exception.GetNodePenalty() == 20) { cmapInvalidVotes.Insert(nHashVote, vote); } @@ -992,7 +992,7 @@ void CGovernanceManager::CheckPostponedObjects() // Check postponed proposals for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) { const uint256& nHash = it->first; - CGovernanceObject& govobj = it->second; + auto& govobj = *Assert(it->second); assert(govobj.GetObjectType() != GovernanceObject::TRIGGER); @@ -1023,7 +1023,7 @@ void CGovernanceManager::CheckPostponedObjects() for (auto it = setAdditionalRelayObjects.begin(); it != setAdditionalRelayObjects.end();) { auto itObject = mapObjects.find(*it); if (itObject != mapObjects.end()) { - const CGovernanceObject& govobj = itObject->second; + const auto& govobj = *Assert(itObject->second); int64_t nTimestamp = govobj.GetCreationTime(); @@ -1066,7 +1066,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH size_t nVoteCount = 0; if (fUseFilter) { LOCK(cs_store); - const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash); + auto pObj = FindConstGovernanceObjectInternal(nHash); if (pObj) { filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRand(/*nMax=*/999999), BLOOM_UPDATE_ALL); @@ -1129,7 +1129,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& if (mapObjects.empty()) return -2; for (const auto& [nHash, govobj] : mapObjects) { - if (govobj.IsSetCachedDelete()) continue; + if (Assert(govobj)->IsSetCachedDelete()) continue; if (mapAskedRecently.count(nHash)) { for (auto it = mapAskedRecently[nHash].begin(); it != mapAskedRecently[nHash].end();) { if (it->second < nNow) { @@ -1141,7 +1141,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& if (mapAskedRecently[nHash].size() >= nPeersPerHashMax) continue; } - if (govobj.GetObjectType() == GovernanceObject::TRIGGER) { + if (govobj->GetObjectType() == GovernanceObject::TRIGGER) { vTriggerObjHashes.push_back(nHash); } else { vOtherObjHashes.push_back(nHash); @@ -1220,9 +1220,10 @@ void CGovernanceManager::RebuildIndexes() cmapVoteToObject.Clear(); for (auto& [_, govobj] : mapObjects) { - std::vector vecVotes = WITH_LOCK(govobj.cs, return govobj.GetVoteFile().GetVotes()); + assert(govobj); + std::vector vecVotes = WITH_LOCK(govobj->cs, return govobj->GetVoteFile().GetVotes()); for (const auto& vecVote : vecVotes) { - cmapVoteToObject.Insert(vecVote.GetHash(), &govobj); + cmapVoteToObject.Insert(vecVote.GetHash(), govobj); } } } @@ -1234,12 +1235,12 @@ void CGovernanceManager::AddCachedTriggers() int64_t nNow = GetTime().count(); for (auto& [_, govobj] : mapObjects) { - if (govobj.GetObjectType() != GovernanceObject::TRIGGER) { + if (Assert(govobj)->GetObjectType() != GovernanceObject::TRIGGER) { continue; } - if (!AddNewTrigger(govobj.GetHash())) { - govobj.PrepareDeletion(nNow); + if (!AddNewTrigger(govobj->GetHash())) { + govobj->PrepareDeletion(nNow); } } } @@ -1285,7 +1286,7 @@ std::string GovernanceStore::ToString() const int nOtherCount = 0; for (const auto& [_, govobj] : mapObjects) { - switch (govobj.GetObjectType()) { + switch (Assert(govobj)->GetObjectType()) { case GovernanceObject::PROPOSAL: nProposalCount++; break; @@ -1318,7 +1319,7 @@ UniValue CGovernanceManager::ToJson() const int nOtherCount = 0; for (const auto& [_, govobj] : mapObjects) { - switch (govobj.GetObjectType()) { + switch (Assert(govobj)->GetObjectType()) { case GovernanceObject::PROPOSAL: nProposalCount++; break; @@ -1445,7 +1446,7 @@ void CGovernanceManager::RemoveInvalidVotes() for (const auto& outpoint : changedKeyMNs) { for (auto& [_, govobj] : mapObjects) { - auto removed = govobj.RemoveInvalidVotes(tip_mn_list, outpoint); + auto removed = Assert(govobj)->RemoveInvalidVotes(tip_mn_list, outpoint); if (removed.empty()) { continue; } @@ -1479,7 +1480,7 @@ bool CGovernanceManager::AddNewTrigger(uint256 nHash) CSuperblock_sptr pSuperblock; try { - const CGovernanceObject* pGovObj = FindGovernanceObjectInternal(nHash); + auto pGovObj = FindGovernanceObjectInternal(nHash); if (!pGovObj) { throw std::runtime_error("CSuperblock: Failed to find Governance Object"); } @@ -1514,7 +1515,7 @@ void CGovernanceManager::CleanAndRemoveTriggers() for (auto it = mapTrigger.begin(); it != mapTrigger.end();) { bool remove = false; - CGovernanceObject* pObj = nullptr; + std::shared_ptr pObj = nullptr; const CSuperblock_sptr& pSuperblock = it->second; if (!pSuperblock) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- nullptr superblock\n", __func__); @@ -1587,7 +1588,7 @@ std::vector CGovernanceManager::GetActiveTriggersInternal() co // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS for (const auto& [nHash, pSuperblock] : mapTrigger) { - const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash); + auto pObj = FindConstGovernanceObjectInternal(nHash); if (pObj) { vecResults.push_back(pSuperblock); } @@ -1616,7 +1617,7 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m continue; } - CGovernanceObject* pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); + auto pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); if (!pObj) { LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); continue; @@ -1673,7 +1674,7 @@ bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& t continue; } - const CGovernanceObject* pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); + auto pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); if (!pObj) { continue; } @@ -1779,11 +1780,11 @@ std::vector> CGovernanceManager::GetApp LOCK(cs_store); for (const auto& [_, govobj] : mapObjects) { // Skip all non-proposals objects - if (govobj.GetObjectType() != GovernanceObject::PROPOSAL) continue; + if (Assert(govobj)->GetObjectType() != GovernanceObject::PROPOSAL) continue; // Skip non-passing proposals - const int absYesCount = govobj.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + const int absYesCount = govobj->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); if (absYesCount < nAbsVoteReq) continue; - ret.emplace_back(std::make_shared(govobj)); + ret.emplace_back(govobj); } // Sort approved proposals by absolute Yes votes descending diff --git a/src/governance/governance.h b/src/governance/governance.h index 420e8f13dd1d..8a662de236df 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -184,7 +184,7 @@ class GovernanceStore mutable Mutex cs_store; // keep track of the scanning errors - std::map mapObjects GUARDED_BY(cs_store); + std::map> mapObjects GUARDED_BY(cs_store); // mapErasedGovernanceObjects contains key-value pairs, where // key - governance object's hash // value - expiration time for deleted objects @@ -248,7 +248,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent private: using db_type = CFlatDB; - using object_ref_cm_t = CacheMap; + using object_ref_cm_t = CacheMap>; private: const std::unique_ptr m_db; @@ -264,7 +264,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // keep track of current block height int nCachedBlockHeight; object_ref_cm_t cmapVoteToObject; - std::map mapPostponedObjects; + std::map> mapPostponedObjects; std::set setAdditionalRelayObjects; std::map m_requested_hash_time; bool fRateChecksEnabled; @@ -336,7 +336,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Signer interface bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override + std::shared_ptr FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs_store); @@ -365,7 +365,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - CGovernanceObject* FindGovernanceObject(const uint256& nHash) override + std::shared_ptr FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs_store); int GetVoteCount() const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); @@ -374,7 +374,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent void AddPostponedObject(const CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const + std::shared_ptr FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); private: @@ -391,12 +391,12 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs_store); - CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) + std::shared_ptr FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs_store); std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs_store); - const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const + std::shared_ptr FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs_store); // Internal counterpart to "Signer interface" diff --git a/src/governance/object.h b/src/governance/object.h index 439c9d190b77..01bd7b65b35d 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -135,6 +135,8 @@ class CGovernanceObject CGovernanceObject(); CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTime, const uint256& nCollateralHashIn, const std::string& strDataHexIn); CGovernanceObject(const CGovernanceObject& other); + template + CGovernanceObject(deserialize_type, Stream& s) { s >> *this; } // Getters bool IsSetCachedFunding() const { return fCachedFunding; } diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 13dd74d88637..ea06dee33051 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -214,7 +214,7 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetGovernanceObjHash()); + auto govobj = m_govman.FindGovernanceObject(trigger->GetGovernanceObjHash()); if (!govobj) { LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for unknown trigger %s\n", __func__, trigger->GetGovernanceObjHash().ToString()); diff --git a/src/governance/signing.h b/src/governance/signing.h index 750e2c168c18..c037e9f46ffc 100644 --- a/src/governance/signing.h +++ b/src/governance/signing.h @@ -37,8 +37,8 @@ class GovernanceSignerParent virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0; virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) = 0; virtual int GetCachedBlockHeight() const = 0; - virtual CGovernanceObject* FindGovernanceObject(const uint256& nHash) = 0; - virtual CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0; + virtual std::shared_ptr FindGovernanceObject(const uint256& nHash) = 0; + virtual std::shared_ptr FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0; virtual std::vector> GetActiveTriggers() const = 0; virtual std::vector> GetApprovedProposals( const CDeterministicMNList& tip_mn_list) = 0; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 4ebfb69cf081..997b9fb34e30 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -414,7 +414,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); { - const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hash); + auto pGovObj = node.govman->FindConstGovernanceObject(hash); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); } @@ -726,7 +726,7 @@ static RPCHelpMan gobject_get() CHECK_NONFATAL(node.govman); LOCK(cs_main); - const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash); + auto pGovObj = node.govman->FindConstGovernanceObject(hash); if (pGovObj == nullptr) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown governance object"); @@ -824,7 +824,7 @@ static RPCHelpMan gobject_getcurrentvotes() const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); - const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash); + auto pGovObj = node.govman->FindConstGovernanceObject(hash); if (pGovObj == nullptr) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown governance-hash"); @@ -919,7 +919,7 @@ static RPCHelpMan voteraw() const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); GovernanceObject govObjType = [&]() { - const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hashGovObj); + auto pGovObj = node.govman->FindConstGovernanceObject(hashGovObj); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); } From f88f15fed0e4cbf9d1cca25d9ffa6339e9b804b6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 28 Oct 2025 16:29:21 +0530 Subject: [PATCH 18/18] fix: `CGovernanceManager::Clear()` should clear all fields --- src/governance/governance.cpp | 15 ++++++++++----- src/governance/governance.h | 6 +++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 39d9bd9792af..108f3690b163 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -73,11 +73,8 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi m_chainman{chainman}, m_dmnman{dmnman}, m_mn_sync{mn_sync}, - nTimeLastDiff(0), - nCachedBlockHeight(0), - cmapVoteToObject(MAX_CACHE_SIZE), - mapPostponedObjects(), - fRateChecksEnabled(true), + cmapVoteToObject{MAX_CACHE_SIZE}, + mapPostponedObjects{}, mapTrigger{} { } @@ -1267,6 +1264,7 @@ void GovernanceStore::Clear() cmapInvalidVotes.Clear(); cmmapOrphanVotes.Clear(); mapLastMasternodeObject.clear(); + lastMNListForVotingKeys = std::make_shared(); } void CGovernanceManager::Clear() @@ -1274,7 +1272,14 @@ void CGovernanceManager::Clear() AssertLockNotHeld(cs_store); LogPrint(BCLog::GOBJECT, "Governance object manager was cleared\n"); GovernanceStore::Clear(); + nTimeLastDiff = 0; + nCachedBlockHeight = 0; cmapVoteToObject.Clear(); + mapPostponedObjects.clear(); + setAdditionalRelayObjects.clear(); + m_requested_hash_time.clear(); + fRateChecksEnabled = true; + mapTrigger.clear(); } std::string GovernanceStore::ToString() const diff --git a/src/governance/governance.h b/src/governance/governance.h index 8a662de236df..d5b009b924fa 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -260,14 +260,14 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent const std::unique_ptr& m_dmnman; CMasternodeSync& m_mn_sync; - int64_t nTimeLastDiff; + int64_t nTimeLastDiff{0}; // keep track of current block height - int nCachedBlockHeight; + int nCachedBlockHeight{0}; object_ref_cm_t cmapVoteToObject; std::map> mapPostponedObjects; std::set setAdditionalRelayObjects; std::map m_requested_hash_time; - bool fRateChecksEnabled; + bool fRateChecksEnabled{true}; std::map> mapTrigger; mutable Mutex cs_relay;