From 0aa08ba80dce46774a1d830bc2908a64ca35e524 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:16:12 +0000 Subject: [PATCH] refactor: remove CGovernanceManager global, move to NodeContext --- src/governance/classes.cpp | 16 ++++++++-------- src/governance/classes.h | 4 ++-- src/governance/governance.cpp | 6 ++---- src/governance/governance.h | 2 -- src/governance/object.cpp | 6 +++--- src/governance/object.h | 6 +++--- src/init.cpp | 8 +++----- src/node/context.cpp | 1 + src/node/context.h | 2 +- src/test/util/setup_common.cpp | 6 ++---- 10 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index ea5daaac5d..702ad380a8 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -111,7 +111,7 @@ bool CGovernanceManager::AddNewTrigger(uint256 nHash) CSuperblock_sptr pSuperblock; try { - auto pSuperblockTmp = std::make_shared(nHash); + auto pSuperblockTmp = std::make_shared(*this, nHash); pSuperblock = pSuperblockTmp; } catch (std::exception& e) { LogPrintf("CGovernanceManager::%s -- Error creating superblock: %s\n", __func__, e.what()); @@ -125,7 +125,7 @@ bool CGovernanceManager::AddNewTrigger(uint256 nHash) mapTrigger.insert(std::make_pair(nHash, pSuperblock)); - return !pSuperblock->IsExpired(*governance); + return !pSuperblock->IsExpired(*this); } /** @@ -166,7 +166,7 @@ void CGovernanceManager::CleanAndRemoveTriggers() case SeenObjectStatus::Valid: case SeenObjectStatus::Executed: { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Valid trigger found\n", __func__); - if (pSuperblock->IsExpired(*governance)) { + if (pSuperblock->IsExpired(*this)) { // update corresponding object pObj->SetExpired(); remove = true; @@ -369,7 +369,7 @@ bool CSuperblockManager::IsValid(CGovernanceManager& governanceManager, const CT CSuperblock_sptr pSuperblock; if (CSuperblockManager::GetBestSuperblock(governanceManager, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(txNew, nBlockHeight, blockReward); + return pSuperblock->IsValid(governanceManager, txNew, nBlockHeight, blockReward); } return false; @@ -398,13 +398,13 @@ CSuperblock:: } CSuperblock:: - CSuperblock(uint256& nHash) : + CSuperblock(CGovernanceManager& govman, uint256& nHash) : nGovObjHash(nHash), nBlockHeight(0), nStatus(SeenObjectStatus::Unknown), vecPayments() { - const CGovernanceObject* pGovObj = GetGovernanceObject(*governance); + const CGovernanceObject* pGovObj = GetGovernanceObject(govman); if (!pGovObj) { throw std::runtime_error("CSuperblock: Failed to find Governance Object"); @@ -612,7 +612,7 @@ CAmount CSuperblock::GetPaymentsTotalAmount() * - Does this transaction match the superblock? */ -bool CSuperblock::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward) +bool CSuperblock::IsValid(CGovernanceManager& govman, const CTransaction& txNew, int nBlockHeight, CAmount blockReward) { // TODO : LOCK(cs); // No reason for a lock here now since this method only accesses data @@ -631,7 +631,7 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount b int nMinerAndMasternodePayments = nOutputs - nPayments; LogPrint(BCLog::GOBJECT, "CSuperblock::IsValid -- nOutputs = %d, nPayments = %d, GetDataAsHexString = %s\n", - nOutputs, nPayments, GetGovernanceObject(*governance)->GetDataAsHexString()); + nOutputs, nPayments, GetGovernanceObject(govman)->GetDataAsHexString()); // We require an exact match (including order) between the expected // superblock payments and the payments actually in the block. diff --git a/src/governance/classes.h b/src/governance/classes.h index ede192c10c..e89fb96f16 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -101,7 +101,7 @@ private: public: CSuperblock(); CSuperblock(int nBlockHeight, std::vector vecPayments); - explicit CSuperblock(uint256& nHash); + explicit CSuperblock(CGovernanceManager& govman, uint256& nHash); static bool IsValidBlockHeight(int nBlockHeight); static void GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuperblockRet, int& nNextSuperblockRet); @@ -126,7 +126,7 @@ public: bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet); CAmount GetPaymentsTotalAmount(); - bool IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward); + bool IsValid(CGovernanceManager& govman, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); bool IsExpired(const CGovernanceManager& governanceManager) const; std::vector GetProposalHashes() const; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index ac443aac18..729f4cf202 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -25,8 +25,6 @@ #include #include -std::unique_ptr governance; - int nSubmittedFinalBudget; const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16"; @@ -270,7 +268,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, CConnman& c CGovernanceException e; if (pairVote.second < nNow) { fRemove = true; - } else if (govobj.ProcessVote(vote, e)) { + } else if (govobj.ProcessVote(*this, vote, e)) { vote.Relay(connman); fRemove = true; } @@ -1095,7 +1093,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, return false; } - bool fOk = govobj.ProcessVote(vote, exception) && cmapVoteToObject.Insert(nHashVote, &govobj); + bool fOk = govobj.ProcessVote(*this, vote, exception) && cmapVoteToObject.Insert(nHashVote, &govobj); LEAVE_CRITICAL_SECTION(cs); return fOk; } diff --git a/src/governance/governance.h b/src/governance/governance.h index f47a6786c1..3e92d19053 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -25,8 +25,6 @@ class CGovernanceObject; class CGovernanceVote; class CSporkManager; -extern std::unique_ptr governance; - static constexpr int RATE_BUFFER_SIZE = 5; class CDeterministicMNList; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index b9b4cb28c3..00cd5c428e 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -79,7 +79,7 @@ CGovernanceObject::CGovernanceObject(const CGovernanceObject& other) : { } -bool CGovernanceObject::ProcessVote(const CGovernanceVote& vote, CGovernanceException& exception) +bool CGovernanceObject::ProcessVote(CGovernanceManager& govman, const CGovernanceVote& vote, CGovernanceException& exception) { LOCK(cs); @@ -149,7 +149,7 @@ bool CGovernanceObject::ProcessVote(const CGovernanceVote& vote, CGovernanceExce int64_t nNow = GetAdjustedTime(); int64_t nVoteTimeUpdate = voteInstanceRef.nTime; - if (governance->AreRateChecksEnabled()) { + if (govman.AreRateChecksEnabled()) { int64_t nTimeDelta = nNow - voteInstanceRef.nTime; if (nTimeDelta < GOVERNANCE_UPDATE_MIN) { std::ostringstream ostr; @@ -175,7 +175,7 @@ bool CGovernanceObject::ProcessVote(const CGovernanceVote& vote, CGovernanceExce << ", vote hash = " << vote.GetHash().ToString(); LogPrintf("%s\n", ostr.str()); exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); - governance->AddInvalidVote(vote); + govman.AddInvalidVote(vote); return false; } diff --git a/src/governance/object.h b/src/governance/object.h index b8ccded3df..12a4280551 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -15,10 +15,10 @@ class CBLSSecretKey; class CBLSPublicKey; -class CNode; - +class CGovernanceManager; class CGovernanceObject; class CGovernanceVote; +class CNode; extern RecursiveMutex cs_main; @@ -288,7 +288,7 @@ public: void LoadData(); void GetData(UniValue& objResult) const; - bool ProcessVote(const CGovernanceVote& vote, CGovernanceException& exception); + bool ProcessVote(CGovernanceManager& govman, const CGovernanceVote& vote, CGovernanceException& exception); /// Called when MN's which have voted on this object have been removed void ClearMasternodeVotes(); diff --git a/src/init.cpp b/src/init.cpp index d0c1050330..52087aa47c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -314,8 +314,7 @@ void PrepareShutdown(NodeContext& node) node.mn_sync = nullptr; ::masternodeSync.reset(); node.sporkman.reset(); - node.govman = nullptr; - ::governance.reset(); + node.govman.reset(); // Stop and delete all indexes only after flushing background callbacks. if (g_txindex) { @@ -1712,9 +1711,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc node.chainman = &g_chainman; ChainstateManager& chainman = *Assert(node.chainman); - assert(!::governance); - ::governance = std::make_unique(); - node.govman = ::governance.get(); + assert(!node.govman); + node.govman = std::make_unique(); assert(!node.sporkman); node.sporkman = std::make_unique(); diff --git a/src/node/context.cpp b/src/node/context.cpp index 1269b8294a..78d69dac36 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index e9f430ea33..7d06e241e5 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -73,6 +73,7 @@ struct NodeContext { //! Dash std::unique_ptr evodb; std::unique_ptr chain_helper; + std::unique_ptr govman; std::unique_ptr cj_ctx; std::unique_ptr mnhf_manager; std::unique_ptr sporkman; @@ -80,7 +81,6 @@ struct NodeContext { CCreditPoolManager* cpoolman{nullptr}; CDeterministicMNManager* dmnman{nullptr}; CDSTXManager* dstxman{nullptr}; - CGovernanceManager* govman{nullptr}; CMasternodeMetaMan* mn_metaman{nullptr}; CMasternodeSync* mn_sync{nullptr}; CNetFulfilledRequestManager* netfulfilledman{nullptr}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 76745760de..1cb1f8a7fb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -223,8 +223,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. m_node.sporkman = std::make_unique(); - ::governance = std::make_unique(); - m_node.govman = ::governance.get(); + m_node.govman = std::make_unique(); ::masternodeSync = std::make_unique(*m_node.connman, *m_node.govman); m_node.mn_sync = ::masternodeSync.get(); ::dstxManager = std::make_unique(); @@ -254,8 +253,7 @@ ChainTestingSetup::~ChainTestingSetup() ::dstxManager.reset(); m_node.mn_sync = nullptr; ::masternodeSync.reset(); - m_node.govman = nullptr; - ::governance.reset(); + m_node.govman.reset(); m_node.sporkman.reset(); m_node.connman.reset(); m_node.addrman.reset();