From 982fc9a069988b06ab9829a6f0bbd7e61dd6054e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 22:49:50 +0700 Subject: [PATCH 01/15] fix: avoid lock annotation for govman.cs in voteraw --- src/rpc/governance.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 45a7ea26c4..a4bab7ea84 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -955,14 +955,14 @@ static RPCHelpMan voteraw() const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.govman); - GovernanceObject govObjType = WITH_LOCK(node.govman->cs, return [&](){ - AssertLockHeld(node.govman->cs); + GovernanceObject govObjType = [&]() { + LOCK(node.govman->cs); const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hashGovObj); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); } return pGovObj->GetObjectType(); - }()); + }(); int64_t nTime = request.params[5].get_int64(); From 41f1a43236a716d977a5cf3f79ba1c712e5f8951 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Sep 2024 02:16:30 +0700 Subject: [PATCH 02/15] fix: add missing const for member functions of CRateCheckBuffer --- src/governance/governance.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/governance/governance.h b/src/governance/governance.h index 1417344551..100e303d13 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -69,7 +69,7 @@ public: fBufferEmpty = false; } - int64_t GetMinTimestamp() + int64_t GetMinTimestamp() const { int nIndex = nDataStart; int64_t nMin = std::numeric_limits::max(); @@ -85,7 +85,7 @@ public: return nMin; } - int64_t GetMaxTimestamp() + int64_t GetMaxTimestamp() const { int nIndex = nDataStart; int64_t nMax = 0; @@ -112,7 +112,7 @@ public: return RATE_BUFFER_SIZE - nDataStart + nDataEnd; } - double GetRate() + double GetRate() const { int nCount = GetCount(); if (nCount < RATE_BUFFER_SIZE) { From 7aafb5a393338b05e6ec65565df4f1ec85161e5d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Sep 2024 02:27:10 +0700 Subject: [PATCH 03/15] fix: add one more file to list of non-backported (flat-database.h) --- src/flat-database.h | 14 +++++++------- test/util/data/non-backported.txt | 1 + 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/flat-database.h b/src/flat-database.h index 8ad452c554..5034d7a7da 100644 --- a/src/flat-database.h +++ b/src/flat-database.h @@ -117,9 +117,9 @@ private: } - unsigned char pchMsgTmp[4]; - std::string strMagicMessageTmp; try { + unsigned char pchMsgTmp[4]; + std::string strMagicMessageTmp; // de-serialize file header (file specific magic message) and .. ssObj >> strMagicMessageTmp; @@ -178,11 +178,11 @@ private: } public: - CFlatDB(std::string strFilenameIn, std::string strMagicMessageIn) + CFlatDB(std::string&& strFilenameIn, std::string&& strMagicMessageIn) : + pathDB{gArgs.GetDataDirNet() / strFilenameIn}, + strFilename{strFilenameIn}, + strMagicMessage{strMagicMessageIn} { - pathDB = gArgs.GetDataDirNet() / strFilenameIn; - strFilename = strFilenameIn; - strMagicMessage = strMagicMessageIn; } bool Load(T& objToLoad) @@ -191,7 +191,7 @@ public: return Read(objToLoad); } - bool Store(T& objToSave) + bool Store(const T& objToSave) { LogPrintf("Verifying %s format...\n", strFilename); T tmpObjToLoad; diff --git a/test/util/data/non-backported.txt b/test/util/data/non-backported.txt index 7684074874..b37f350112 100644 --- a/test/util/data/non-backported.txt +++ b/test/util/data/non-backported.txt @@ -9,6 +9,7 @@ src/coinjoin/*.h src/ctpl_stl.h src/cxxtimer.hpp src/dsnotificationinterface.* +src/flat-database.h src/evo/*.cpp src/evo/*.h src/governance/*.cpp From 1570a02c89b21694fd08bdc0a2711720578be409 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Sep 2024 02:30:52 +0700 Subject: [PATCH 04/15] refactor: move ScopedLockBool from header to cpp file --- src/governance/governance.cpp | 19 +++++++++++++++++++ src/governance/governance.h | 20 -------------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 00a130e7d1..b9310a4480 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -33,6 +33,25 @@ const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceMa const int CGovernanceManager::MAX_TIME_FUTURE_DEVIATION = 60 * 60; const int CGovernanceManager::RELIABLE_PROPAGATION_TIME = 60; +namespace { +class ScopedLockBool +{ + bool& ref; + bool fPrevValue; + +public: + ScopedLockBool(RecursiveMutex& _cs, bool& _ref, bool _value) : + ref(_ref) + { + AssertLockHeld(_cs); + fPrevValue = ref; + ref = _value; + } + + ~ScopedLockBool() { ref = fPrevValue; } +}; +} // anonymous namespace + GovernanceStore::GovernanceStore() : cs(), mapObjects(), diff --git a/src/governance/governance.h b/src/governance/governance.h index 100e303d13..f9fe3c2e1f 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -231,26 +231,6 @@ private: using hash_s_t = std::set; using db_type = CFlatDB; - class ScopedLockBool - { - bool& ref; - bool fPrevValue; - - public: - ScopedLockBool(RecursiveMutex& _cs, bool& _ref, bool _value) : - ref(_ref) - { - AssertLockHeld(_cs); - fPrevValue = ref; - ref = _value; - } - - ~ScopedLockBool() - { - ref = fPrevValue; - } - }; - private: static const int MAX_TIME_FUTURE_DEVIATION; static const int RELIABLE_PROPAGATION_TIME; From 7eb16346862ab81c58926d7cb76c12d11b760422 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Sep 2024 02:32:14 +0700 Subject: [PATCH 05/15] refactor: drop alias that is used only once --- src/governance/governance.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/governance/governance.h b/src/governance/governance.h index f9fe3c2e1f..a67a5df875 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -228,7 +228,6 @@ class CGovernanceManager : public GovernanceStore friend class CGovernanceObject; private: - using hash_s_t = std::set; using db_type = CFlatDB; private: @@ -249,7 +248,7 @@ private: // keep track of current block height int nCachedBlockHeight; std::map mapPostponedObjects; - hash_s_t setAdditionalRelayObjects; + std::set setAdditionalRelayObjects; std::map m_requested_hash_time; bool fRateChecksEnabled; std::optional votedFundingYesTriggerHash; From 9638fdce6d39cd0e249786c5ed8d32b505b78f9a Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 8 Oct 2024 17:16:42 +0300 Subject: [PATCH 06/15] refactor: pass mn_sync to CGovernanceManager ctor as a reference --- src/governance/governance.cpp | 46 +++++++++++++++++----------------- src/governance/governance.h | 9 ++++--- src/init.cpp | 6 ++--- src/masternode/sync.cpp | 24 ++++++++---------- src/masternode/sync.h | 7 +++--- src/test/util/setup_common.cpp | 4 +-- 6 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index b9310a4480..659f66ce0d 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -65,8 +65,8 @@ GovernanceStore::GovernanceStore() : } CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman, - const ChainstateManager& chainman, const std::unique_ptr& dmnman, - const std::unique_ptr& mn_sync) : + const ChainstateManager& chainman, + const std::unique_ptr& dmnman, CMasternodeSync& mn_sync) : m_db{std::make_unique("governance.dat", "magicGovernanceCache")}, m_mn_metaman{mn_metaman}, m_netfulfilledman{netfulfilledman}, @@ -143,7 +143,7 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) { if (!IsValid()) return {}; - if (m_mn_sync == nullptr || !m_mn_sync->IsBlockchainSynced()) return {}; + if (!m_mn_sync.IsBlockchainSynced()) return {}; const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); // ANOTHER USER IS ASKING US TO HELP THEM SYNC GOVERNANCE OBJECT DATA @@ -151,7 +151,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe // Ignore such requests until we are fully synced. // We could start processing this after masternode list is synced // but this is a heavy one so it's better to finish sync first. - if (!m_mn_sync->IsSynced()) return {}; + if (!m_mn_sync.IsSynced()) return {}; uint256 nProp; CBloomFilter filter; @@ -179,7 +179,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe WITH_LOCK(::cs_main, peerman.EraseObjectRequest(peer.GetId(), CInv(MSG_GOVERNANCE_OBJECT, nHash))); - if (!m_mn_sync->IsBlockchainSynced()) { + if (!m_mn_sync.IsBlockchainSynced()) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode list not synced\n"); return {}; } @@ -243,7 +243,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe WITH_LOCK(::cs_main, peerman.EraseObjectRequest(peer.GetId(), CInv(MSG_GOVERNANCE_OBJECT_VOTE, nHash))); // Ignore such messages until masternode list is synced - if (!m_mn_sync->IsBlockchainSynced()) { + if (!m_mn_sync.IsBlockchainSynced()) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- masternode list not synced\n"); return {}; } @@ -261,11 +261,11 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe CGovernanceException exception; if (ProcessVote(&peer, vote, exception, connman)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- %s new\n", strHash); - m_mn_sync->BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE"); - vote.Relay(peerman, *m_mn_sync, tip_mn_list); + m_mn_sync.BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE"); + vote.Relay(peerman, m_mn_sync, tip_mn_list); } else { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Rejected vote, error = %s\n", exception.what()); - if ((exception.GetNodePenalty() != 0) && m_mn_sync->IsSynced()) { + if ((exception.GetNodePenalty() != 0) && m_mn_sync.IsSynced()) { return tl::unexpected{exception.GetNodePenalty()}; } return {}; @@ -291,7 +291,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager if (pairVote.second < nNow) { fRemove = true; } else if (govobj.ProcessVote(m_mn_metaman, *this, tip_mn_list, vote, e)) { - vote.Relay(peerman, *Assert(m_mn_sync), tip_mn_list); + vote.Relay(peerman, m_mn_sync, tip_mn_list); fRemove = true; } if (fRemove) { @@ -345,12 +345,12 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana } LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, pfrom ? pfrom->GetLogString() : "nullptr"); - govobj.Relay(peerman, *Assert(m_mn_sync)); + govobj.Relay(peerman, m_mn_sync); // Update the rate buffer MasternodeRateUpdate(govobj); - m_mn_sync->BumpAssetLastTime("CGovernanceManager::AddGovernanceObject"); + m_mn_sync.BumpAssetLastTime("CGovernanceManager::AddGovernanceObject"); // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT @@ -365,7 +365,7 @@ void CGovernanceManager::CheckAndRemove() assert(m_mn_metaman.IsValid()); // Return on initial sync, spammed the debug.log and provided no use - if (m_mn_sync == nullptr || !m_mn_sync->IsBlockchainSynced()) return; + if (!m_mn_sync.IsBlockchainSynced()) return; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean\n"); @@ -591,7 +591,7 @@ struct sortProposalsByVotes { std::optional CGovernanceManager::CreateSuperblockCandidate(int nHeight) const { if (!IsValid()) return std::nullopt; - if (m_mn_sync == nullptr || !m_mn_sync->IsSynced()) return std::nullopt; + if (!m_mn_sync.IsSynced()) return std::nullopt; if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; if (HasAlreadyVotedFundingTrigger()) return std::nullopt; @@ -861,7 +861,7 @@ void CGovernanceManager::ResetVotedFundingTrigger() void CGovernanceManager::DoMaintenance(CConnman& connman) { if (!IsValid()) return; - if (m_mn_sync == nullptr || !m_mn_sync->IsSynced()) return; + if (!m_mn_sync.IsSynced()) return; if (ShutdownRequested()) return; // CHECK OBJECTS WE'VE ASKED FOR, REMOVE OLD ENTRIES @@ -875,7 +875,7 @@ void CGovernanceManager::DoMaintenance(CConnman& connman) bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) { // do not request objects until it's time to sync - if (!Assert(m_mn_sync)->IsBlockchainSynced()) return false; + if (!m_mn_sync.IsBlockchainSynced()) return false; LOCK(cs); @@ -918,7 +918,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) void CGovernanceManager::SyncSingleObjVotes(CNode& peer, PeerManager& peerman, const uint256& nProp, const CBloomFilter& filter, CConnman& connman) { // do not provide any data until our node is synced - if (!Assert(m_mn_sync)->IsSynced()) return; + if (!m_mn_sync.IsSynced()) return; int nVoteCount = 0; @@ -970,7 +970,7 @@ PeerMsgRet CGovernanceManager::SyncObjects(CNode& peer, PeerManager& peerman, CC assert(m_netfulfilledman.IsValid()); // do not provide any data until our node is synced - if (!Assert(m_mn_sync)->IsSynced()) return {}; + if (!m_mn_sync.IsSynced()) return {}; if (m_netfulfilledman.HasFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC)) { // Asking for the whole list multiple times in a short period of time is no good @@ -1060,7 +1060,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo fRateCheckBypassed = false; - if (!Assert(m_mn_sync)->IsSynced() || !fRateChecksEnabled) { + if (!m_mn_sync.IsSynced() || !fRateChecksEnabled) { return true; } @@ -1122,7 +1122,7 @@ bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGover { bool fOK = ProcessVote(/* pfrom = */ nullptr, vote, exception, connman); if (fOK) { - vote.Relay(peerman, *Assert(m_mn_sync), Assert(m_dmnman)->GetListAtChainTip()); + vote.Relay(peerman, m_mn_sync, Assert(m_dmnman)->GetListAtChainTip()); } return fOK; } @@ -1183,7 +1183,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) { - if (!Assert(m_mn_sync)->IsSynced()) return; + if (!m_mn_sync.IsSynced()) return; LOCK2(cs_main, cs); @@ -1231,7 +1231,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) if (fValid) { if (fReady) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- additional relay: hash = %s\n", govobj.GetHash().ToString()); - govobj.Relay(peerman, *m_mn_sync); + govobj.Relay(peerman, m_mn_sync); } else { it++; continue; @@ -1592,7 +1592,7 @@ void CGovernanceManager::CleanOrphanObjects() void CGovernanceManager::RemoveInvalidVotes() { - if (!Assert(m_mn_sync)->IsSynced()) { + if (!m_mn_sync.IsSynced()) { return; } diff --git a/src/governance/governance.h b/src/governance/governance.h index a67a5df875..0ee80d9afb 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -242,7 +242,7 @@ private: CNetFulfilledRequestManager& m_netfulfilledman; const ChainstateManager& m_chainman; const std::unique_ptr& m_dmnman; - const std::unique_ptr& m_mn_sync; + CMasternodeSync& m_mn_sync; int64_t nTimeLastDiff; // keep track of current block height @@ -255,9 +255,9 @@ private: std::map> mapTrigger; public: - explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman, const ChainstateManager& chainman, - const std::unique_ptr& dmnman, - const std::unique_ptr& mn_sync); + explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman, + const ChainstateManager& chainman, + const std::unique_ptr& dmnman, CMasternodeSync& mn_sync); ~CGovernanceManager(); bool LoadCache(bool load_cache); @@ -278,6 +278,7 @@ public: void ResetVotedFundingTrigger(); +public: void DoMaintenance(CConnman& connman); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/init.cpp b/src/init.cpp index 86429fe62b..b8c9b78748 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1840,9 +1840,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) * need it or not further down and then query if the database is initialized * to check if validation is enabled. */ - node.govman = std::make_unique(*node.mn_metaman, *node.netfulfilledman, *node.chainman, node.dmnman, node.mn_sync); + node.mn_sync = std::make_unique(*node.connman, *node.netfulfilledman); - node.mn_sync = std::make_unique(*node.connman, *node.netfulfilledman, *node.govman); + node.govman = std::make_unique(*node.mn_metaman, *node.netfulfilledman, *node.chainman, node.dmnman, *node.mn_sync); const bool fReset = fReindex; auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { @@ -2255,7 +2255,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 10a: schedule Dash-specific tasks node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1}); - node.scheduler->scheduleEvery(std::bind(&CMasternodeSync::DoMaintenance, std::ref(*node.mn_sync), std::cref(*node.peerman)), std::chrono::seconds{1}); + node.scheduler->scheduleEvery(std::bind(&CMasternodeSync::DoMaintenance, std::ref(*node.mn_sync), std::cref(*node.peerman), std::cref(*node.govman)), std::chrono::seconds{1}); node.scheduler->scheduleEvery(std::bind(&CMasternodeUtils::DoMaintenance, std::ref(*node.connman), std::ref(*node.dmnman), std::ref(*node.mn_sync), std::ref(*node.cj_ctx)), std::chrono::minutes{1}); node.scheduler->scheduleEvery(std::bind(&CDeterministicMNManager::DoMaintenance, std::ref(*node.dmnman)), std::chrono::seconds{10}); diff --git a/src/masternode/sync.cpp b/src/masternode/sync.cpp index d4cf62a735..0b47862cad 100644 --- a/src/masternode/sync.cpp +++ b/src/masternode/sync.cpp @@ -16,12 +16,11 @@ class CMasternodeSync; -CMasternodeSync::CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman, const CGovernanceManager& govman) : +CMasternodeSync::CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman) : nTimeAssetSyncStarted(GetTime()), nTimeLastBumped(GetTime()), connman(_connman), - m_netfulfilledman(netfulfilledman), - m_govman(govman) + m_netfulfilledman(netfulfilledman) { } @@ -115,7 +114,7 @@ void CMasternodeSync::ProcessMessage(const CNode& peer, std::string_view msg_typ LogPrint(BCLog::MNSYNC, "SYNCSTATUSCOUNT -- got inventory count: nItemID=%d nCount=%d peer=%d\n", nItemID, nCount, peer.GetId()); } -void CMasternodeSync::ProcessTick(const PeerManager& peerman) +void CMasternodeSync::ProcessTick(const PeerManager& peerman, const CGovernanceManager& govman) { assert(m_netfulfilledman.IsValid()); @@ -144,7 +143,7 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman) // gradually request the rest of the votes after sync finished if(IsSynced()) { - m_govman.RequestGovernanceObjectVotes(snap.Nodes(), connman, peerman); + govman.RequestGovernanceObjectVotes(snap.Nodes(), connman, peerman); return; } @@ -219,7 +218,7 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman) // GOVOBJ : SYNC GOVERNANCE ITEMS FROM OUR PEERS if(nCurrentAsset == MASTERNODE_SYNC_GOVERNANCE) { - if (!m_govman.IsValid()) { + if (!govman.IsValid()) { SwitchToNextAsset(); return; } @@ -264,7 +263,7 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman) if(!m_netfulfilledman.HasFulfilledRequest(pnode->addr, "governance-sync")) { continue; // to early for this node } - int nObjsLeftToAsk = m_govman.RequestGovernanceObjectVotes(*pnode, connman, peerman); + int nObjsLeftToAsk = govman.RequestGovernanceObjectVotes(*pnode, connman, peerman); // check for data if(nObjsLeftToAsk == 0) { static int64_t nTimeNoObjectsLeft = 0; @@ -276,9 +275,8 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman) } // make sure the condition below is checked only once per tick if(nLastTick == nTick) continue; - if(GetTime() - nTimeNoObjectsLeft > MASTERNODE_SYNC_TIMEOUT_SECONDS && - m_govman.GetVoteCount() - nLastVotes < std::max(int(0.0001 * nLastVotes), MASTERNODE_SYNC_TICK_SECONDS) - ) { + if (GetTime() - nTimeNoObjectsLeft > MASTERNODE_SYNC_TIMEOUT_SECONDS && + govman.GetVoteCount() - nLastVotes < std::max(int(0.0001 * nLastVotes), MASTERNODE_SYNC_TICK_SECONDS)) { // We already asked for all objects, waited for MASTERNODE_SYNC_TIMEOUT_SECONDS // after that and less then 0.01% or MASTERNODE_SYNC_TICK_SECONDS // (i.e. 1 per second) votes were received during the last tick. @@ -290,7 +288,7 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman) return; } nLastTick = nTick; - nLastVotes = m_govman.GetVoteCount(); + nLastVotes = govman.GetVoteCount(); } } } @@ -368,9 +366,9 @@ void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexTip, const CBlock pindexNew->nHeight, pindexTip->nHeight, fInitialDownload, fReachedBestHeader); } -void CMasternodeSync::DoMaintenance(const PeerManager& peerman) +void CMasternodeSync::DoMaintenance(const PeerManager& peerman, const CGovernanceManager& govman) { if (ShutdownRequested()) return; - ProcessTick(peerman); + ProcessTick(peerman, govman); } diff --git a/src/masternode/sync.h b/src/masternode/sync.h index d61ef070ba..2bea1bce88 100644 --- a/src/masternode/sync.h +++ b/src/masternode/sync.h @@ -51,10 +51,9 @@ private: CConnman& connman; CNetFulfilledRequestManager& m_netfulfilledman; - const CGovernanceManager& m_govman; public: - explicit CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman, const CGovernanceManager& govman); + explicit CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman); void SendGovernanceSyncRequest(CNode* pnode) const; @@ -72,13 +71,13 @@ public: void SwitchToNextAsset(); void ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv) const; - void ProcessTick(const PeerManager& peerman); + void ProcessTick(const PeerManager& peerman, const CGovernanceManager& govman); void AcceptedBlockHeader(const CBlockIndex *pindexNew); void NotifyHeaderTip(const CBlockIndex *pindexNew, bool fInitialDownload); void UpdatedBlockTip(const CBlockIndex *pindexTip, const CBlockIndex *pindexNew, bool fInitialDownload); - void DoMaintenance(const PeerManager& peerman); + void DoMaintenance(const PeerManager& peerman, const CGovernanceManager& govman); }; #endif // BITCOIN_MASTERNODE_SYNC_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index b8fba9dd06..3f5600a847 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -245,8 +245,8 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.mn_metaman = std::make_unique(); m_node.netfulfilledman = std::make_unique(); m_node.sporkman = std::make_unique(); - m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.netfulfilledman, *m_node.chainman, m_node.dmnman, m_node.mn_sync); - m_node.mn_sync = std::make_unique(*m_node.connman, *m_node.netfulfilledman, *m_node.govman); + m_node.mn_sync = std::make_unique(*m_node.connman, *m_node.netfulfilledman); + m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.netfulfilledman, *m_node.chainman, m_node.dmnman, *m_node.mn_sync); // Start script-checking threads. Set g_parallel_script_checks to true so they are used. constexpr int script_check_threads = 2; From 7a470c441e3101c78b535c94839e5369ab03330d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:25:57 +0700 Subject: [PATCH 07/15] refactor: move IsSuperblockTriggered to CGovernanceManager --- src/governance/classes.cpp | 35 +++++++++++++++-------------------- src/governance/classes.h | 2 -- src/governance/governance.h | 11 +++++++++++ src/masternode/payments.cpp | 6 +++--- src/rpc/masternode.cpp | 2 +- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 7d410a790f..fec30027ea 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -218,46 +218,41 @@ std::vector CGovernanceManager::GetActiveTriggers() return vecResults; } -/** -* Is Superblock Triggered -* -* - Does this block have a non-executed and activated trigger? -*/ - -bool CSuperblockManager::IsSuperblockTriggered(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight) +bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; } - LOCK(govman.cs); + LOCK(cs); // GET ALL ACTIVE TRIGGERS - std::vector vecTriggers = govman.GetActiveTriggers(); + std::vector vecTriggers = GetActiveTriggers(); - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- vecTriggers.size() = %d\n", vecTriggers.size()); + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- vecTriggers.size() = %d\n", vecTriggers.size()); for (const auto& pSuperblock : vecTriggers) { if (!pSuperblock) { - LogPrintf("CSuperblockManager::IsSuperblockTriggered -- Non-superblock found, continuing\n"); + LogPrintf("IsSuperblockTriggered -- Non-superblock found, continuing\n"); continue; } - CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(govman); + CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this); if (!pObj) { - LogPrintf("CSuperblockManager::IsSuperblockTriggered -- pObj == nullptr, continuing\n"); + LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); continue; } - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- data = %s\n", pObj->GetDataAsPlainString()); + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- data = %s\n", pObj->GetDataAsPlainString()); // note : 12.1 - is epoch calculation correct? if (nBlockHeight != pSuperblock->GetBlockHeight()) { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- block height doesn't match nBlockHeight = %d, blockStart = %d, continuing\n", - nBlockHeight, - pSuperblock->GetBlockHeight()); + LogPrint(BCLog::GOBJECT, + "IsSuperblockTriggered -- block height doesn't match nBlockHeight = %d, blockStart = %d, " + "continuing\n", + nBlockHeight, pSuperblock->GetBlockHeight()); continue; } @@ -266,10 +261,10 @@ bool CSuperblockManager::IsSuperblockTriggered(CGovernanceManager& govman, const pObj->UpdateSentinelVariables(tip_mn_list); if (pObj->IsSetCachedFunding()) { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- fCacheFunding = true, returning true\n"); + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = true, returning true\n"); return true; } else { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::IsSuperblockTriggered -- fCacheFunding = false, continuing\n"); + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = false, continuing\n"); } } diff --git a/src/governance/classes.h b/src/governance/classes.h index 7ff8eff2e4..759325ed0f 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -33,8 +33,6 @@ private: static bool GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); public: - static bool IsSuperblockTriggered(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight); - static bool GetSuperblockPayments(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet); static void ExecuteBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight); diff --git a/src/governance/governance.h b/src/governance/governance.h index 0ee80d9afb..3166c39c4a 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -350,6 +350,17 @@ public: bool AddNewTrigger(uint256 nHash); void CleanAndRemoveTriggers(); + // Superblocks related: + + /** + * Is Superblock Triggered + * + * - Does this block have a non-executed and activated trigger? + */ + + bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + + private: std::optional CreateSuperblockCandidate(int nHeight) const; std::optional CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index cdb5f4c56c..853b587024 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -246,7 +246,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if (!CSuperblockManager::IsSuperblockTriggered(m_govman, tip_mn_list, nBlockHeight)) { + if (!m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { // we are on a valid superblock height but a superblock was not triggered // revert to block reward limits in this case if(!isBlockRewardValueMet) { @@ -302,7 +302,7 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB if (AreSuperblocksEnabled(m_sporkman)) { if (!check_superblock) return true; const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if (CSuperblockManager::IsSuperblockTriggered(m_govman, tip_mn_list, nBlockHeight)) { + if (m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { if (CSuperblockManager::IsValid(m_govman, m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight, blockSubsidy + feeReward)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */ // continue validation, should also pay MN @@ -330,7 +330,7 @@ void CMNPaymentsProcessor::FillBlockPayments(CMutableTransaction& txNew, const C // only create superblocks if spork is enabled AND if superblock is actually triggered // (height should be validated inside) const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if(AreSuperblocksEnabled(m_sporkman) && CSuperblockManager::IsSuperblockTriggered(m_govman, tip_mn_list, nBlockHeight)) { + if (AreSuperblocksEnabled(m_sporkman) && m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Triggered superblock creation at height %d\n", __func__, nBlockHeight); CSuperblockManager::GetSuperblockPayments(m_govman, tip_mn_list, nBlockHeight, voutSuperblockPaymentsRet); } diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 88819f5dbd..8f86534ad3 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -253,7 +253,7 @@ static std::string GetRequiredPaymentsString(CGovernanceManager& govman, const C strPayments += ", " + EncodeDestination(dest); } } - if (CSuperblockManager::IsSuperblockTriggered(govman, tip_mn_list, nBlockHeight)) { + if (govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { std::vector voutSuperblock; if (!CSuperblockManager::GetSuperblockPayments(govman, tip_mn_list, nBlockHeight, voutSuperblock)) { return strPayments + ", error"; From 107d5b49415712a13db16a8e51b66399c2900b54 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:30:32 +0700 Subject: [PATCH 08/15] refactor: move GetSuperblockPayments to CGovernanceManager --- src/governance/classes.cpp | 21 ++++++++------------- src/governance/classes.h | 3 +-- src/governance/governance.h | 8 +++++++- src/masternode/payments.cpp | 2 +- src/rpc/masternode.cpp | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index fec30027ea..98850bf97a 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -305,21 +305,16 @@ bool CSuperblockManager::GetBestSuperblock(CGovernanceManager& govman, const CDe return nYesCount > 0; } -/** -* Get Superblock Payments -* -* - Returns payments for superblock -*/ - -bool CSuperblockManager::GetSuperblockPayments(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet) +bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, + std::vector& voutSuperblockRet) { - LOCK(govman.cs); + LOCK(cs); // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT CSuperblock_sptr pSuperblock; - if (!CSuperblockManager::GetBestSuperblock(govman, tip_mn_list, pSuperblock, nBlockHeight)) { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); + if (!CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); return false; } @@ -347,10 +342,10 @@ bool CSuperblockManager::GetSuperblockPayments(CGovernanceManager& govman, const CTxDestination dest; ExtractDestination(payment.script, dest); - LogPrint(BCLog::GOBJECT, "CSuperblockManager::GetSuperblockPayments -- NEW Superblock: output %d (addr %s, amount %d.%08d)\n", - i, EncodeDestination(dest), payment.nAmount / COIN, payment.nAmount % COIN); + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- NEW Superblock: output %d (addr %s, amount %d.%08d)\n", + i, EncodeDestination(dest), payment.nAmount / COIN, payment.nAmount % COIN); } else { - LogPrint(BCLog::GOBJECT, "CSuperblockManager::GetSuperblockPayments -- Payment not found\n"); + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Payment not found\n"); } } diff --git a/src/governance/classes.h b/src/governance/classes.h index 759325ed0f..ee73a25a0d 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -29,11 +29,10 @@ CAmount ParsePaymentAmount(const std::string& strAmount); class CSuperblockManager { -private: +public: static bool GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); public: - static bool GetSuperblockPayments(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet); static void ExecuteBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight); static bool IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); diff --git a/src/governance/governance.h b/src/governance/governance.h index 3166c39c4a..64d0938b0c 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -357,9 +357,15 @@ public: * * - 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); private: std::optional CreateSuperblockCandidate(int nHeight) const; diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index 853b587024..b7d264a7bb 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -332,7 +332,7 @@ void CMNPaymentsProcessor::FillBlockPayments(CMutableTransaction& txNew, const C const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (AreSuperblocksEnabled(m_sporkman) && m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Triggered superblock creation at height %d\n", __func__, nBlockHeight); - CSuperblockManager::GetSuperblockPayments(m_govman, tip_mn_list, nBlockHeight, voutSuperblockPaymentsRet); + m_govman.GetSuperblockPayments(tip_mn_list, nBlockHeight, voutSuperblockPaymentsRet); } if (!GetMasternodeTxOuts(pindexPrev, blockSubsidy, feeReward, voutMasternodePaymentsRet)) { diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 8f86534ad3..271c15451d 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -255,7 +255,7 @@ static std::string GetRequiredPaymentsString(CGovernanceManager& govman, const C } if (govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { std::vector voutSuperblock; - if (!CSuperblockManager::GetSuperblockPayments(govman, tip_mn_list, nBlockHeight, voutSuperblock)) { + if (!govman.GetSuperblockPayments(tip_mn_list, nBlockHeight, voutSuperblock)) { return strPayments + ", error"; } std::string strSBPayees = "Unknown"; From de8969f463b964398ae0c6d365583def779f7465 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:34:32 +0700 Subject: [PATCH 09/15] refactor: move ExecuteBestSuperblock to CGovernanceManager --- src/governance/classes.cpp | 8 ++++---- src/governance/classes.h | 2 -- src/governance/governance.cpp | 2 +- src/governance/governance.h | 3 +++ 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 98850bf97a..b50bef71c7 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -365,16 +365,16 @@ bool CSuperblockManager::IsValid(CGovernanceManager& govman, const CChain& activ return false; } -void CSuperblockManager::ExecuteBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight) +void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) { - LOCK(govman.cs); + LOCK(cs); CSuperblock_sptr pSuperblock; - if (GetBestSuperblock(govman, tip_mn_list, pSuperblock, nBlockHeight)) { + if (CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. pSuperblock->SetExecuted(); - govman.ResetVotedFundingTrigger(); + ResetVotedFundingTrigger(); } } diff --git a/src/governance/classes.h b/src/governance/classes.h index ee73a25a0d..f6ea2db7eb 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -33,8 +33,6 @@ public: static bool GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); public: - static void ExecuteBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight); - static bool IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); }; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 659f66ce0d..932f5b3726 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -1542,7 +1542,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, CConnman& co CheckPostponedObjects(peerman); - CSuperblockManager::ExecuteBestSuperblock(*this, Assert(m_dmnman)->GetListAtChainTip(), pindex->nHeight); + ExecuteBestSuperblock(Assert(m_dmnman)->GetListAtChainTip(), pindex->nHeight); } void CGovernanceManager::RequestOrphanObjects(CConnman& connman) diff --git a/src/governance/governance.h b/src/governance/governance.h index 64d0938b0c..25c357c59d 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -276,6 +276,7 @@ public: PeerMsgRet ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv); +private: void ResetVotedFundingTrigger(); public: @@ -368,6 +369,8 @@ public: std::vector& voutSuperblockRet); private: + void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + std::optional CreateSuperblockCandidate(int nHeight) const; std::optional CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, const CActiveMasternodeManager& mn_activeman); From 36416531744405eb7288fb4e5f9d0a0cfc4b1a2e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:40:10 +0700 Subject: [PATCH 10/15] refactor: move CSuperblockManager::IsValid to CGoveranceManager::IsValidSuperblock --- src/governance/classes.cpp | 9 +++++---- src/governance/classes.h | 1 - src/governance/governance.h | 3 +++ src/masternode/payments.cpp | 5 +++-- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index b50bef71c7..9e0e0ba7bc 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -352,14 +352,15 @@ bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_m return true; } -bool CSuperblockManager::IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward) +bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, + const CTransaction& txNew, int nBlockHeight, CAmount blockReward) { // GET BEST SUPERBLOCK, SHOULD MATCH - LOCK(govman.cs); + LOCK(cs); CSuperblock_sptr pSuperblock; - if (CSuperblockManager::GetBestSuperblock(govman, tip_mn_list, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(govman, active_chain, txNew, nBlockHeight, blockReward); + if (CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { + return pSuperblock->IsValid(*this, active_chain, txNew, nBlockHeight, blockReward); } return false; diff --git a/src/governance/classes.h b/src/governance/classes.h index f6ea2db7eb..ce19bdbe5d 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -33,7 +33,6 @@ public: static bool GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); public: - static bool IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); }; /** diff --git a/src/governance/governance.h b/src/governance/governance.h index 25c357c59d..864d9b423d 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -368,6 +368,9 @@ public: 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); + private: void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index b7d264a7bb..d2c0065394 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -257,7 +257,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo } // this actually also checks for correct payees and not only amount - if (!CSuperblockManager::IsValid(m_govman, m_chainman.ActiveChain(), tip_mn_list, *block.vtx[0], nBlockHeight, blockReward)) { + if (!m_govman.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, *block.vtx[0], nBlockHeight, blockReward)) { // triggered but invalid? that's weird LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, block.vtx[0]->ToString()); /* Continued */ // should NOT allow invalid superblocks, when superblocks are enabled @@ -303,7 +303,8 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB if (!check_superblock) return true; const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { - if (CSuperblockManager::IsValid(m_govman, m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight, blockSubsidy + feeReward)) { + if (m_govman.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight, + blockSubsidy + feeReward)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */ // continue validation, should also pay MN } else { From b240d08e0996c56580acd314b63165ff5a8c0e37 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:54:13 +0700 Subject: [PATCH 11/15] refactor: move GetBestSuperblock to CGovernanceManager --- src/governance/classes.cpp | 15 ++++++++------- src/governance/classes.h | 15 --------------- src/governance/governance.h | 1 + 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 9e0e0ba7bc..4b12212940 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -272,14 +272,15 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m } -bool CSuperblockManager::GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) +bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) const { if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; } - AssertLockHeld(govman.cs); - std::vector vecTriggers = govman.GetActiveTriggers(); + AssertLockHeld(cs); + std::vector vecTriggers = GetActiveTriggers(); int nYesCount = 0; for (const auto& pSuperblock : vecTriggers) { @@ -287,7 +288,7 @@ bool CSuperblockManager::GetBestSuperblock(CGovernanceManager& govman, const CDe continue; } - const CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(govman); + const CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this); if (!pObj) { continue; @@ -313,7 +314,7 @@ bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_m // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT CSuperblock_sptr pSuperblock; - if (!CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { + if (!GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); return false; } @@ -359,7 +360,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe LOCK(cs); CSuperblock_sptr pSuperblock; - if (CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { + if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { return pSuperblock->IsValid(*this, active_chain, txNew, nBlockHeight, blockReward); } @@ -371,7 +372,7 @@ void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_m LOCK(cs); CSuperblock_sptr pSuperblock; - if (CSuperblockManager::GetBestSuperblock(*this, tip_mn_list, pSuperblock, nBlockHeight)) { + if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. pSuperblock->SetExecuted(); diff --git a/src/governance/classes.h b/src/governance/classes.h index ce19bdbe5d..1d5e610c5d 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -13,7 +13,6 @@ class CChain; class CGovernanceManager; class CSuperblock; -class CSuperblockManager; class CTxOut; class CTransaction; @@ -21,20 +20,6 @@ using CSuperblock_sptr = std::shared_ptr; CAmount ParsePaymentAmount(const std::string& strAmount); -/** -* Superblock Manager -* -* Class for querying superblock information -*/ - -class CSuperblockManager -{ -public: - static bool GetBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); - -public: -}; - /** * Governance Object Payment * diff --git a/src/governance/governance.h b/src/governance/governance.h index 864d9b423d..b6e64b979b 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -373,6 +373,7 @@ public: private: void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); std::optional CreateSuperblockCandidate(int nHeight) const; std::optional CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, From 5031f294412f932c3d7b4766b45c52befac80af3 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 20:59:58 +0700 Subject: [PATCH 12/15] refactor: add couple missing `const` for CGovernanceManager --- src/governance/classes.cpp | 2 +- src/governance/governance.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 4b12212940..70fac9b93c 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -202,7 +202,7 @@ void CGovernanceManager::CleanAndRemoveTriggers() * - Return the triggers in a list */ -std::vector CGovernanceManager::GetActiveTriggers() +std::vector CGovernanceManager::GetActiveTriggers() const { AssertLockHeld(cs); std::vector vecResults; diff --git a/src/governance/governance.h b/src/governance/governance.h index b6e64b979b..bb3eb46e46 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -347,7 +347,7 @@ public: * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - std::vector> GetActiveTriggers(); + std::vector> GetActiveTriggers() const; bool AddNewTrigger(uint256 nHash); void CleanAndRemoveTriggers(); From 350a5ca47c74a54c1db4fb3b63029ad6c43e4e61 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 21:13:26 +0700 Subject: [PATCH 13/15] refactor: drop CSuperblock::GetGovernanceObject to simplify thread safety analysis over FindGovernanceObject --- src/governance/classes.cpp | 21 +++++++-------------- src/governance/classes.h | 4 ++-- src/governance/governance.cpp | 2 +- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 70fac9b93c..3e15bd280e 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -237,8 +237,7 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m continue; } - CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this); - + CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); if (!pObj) { LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); continue; @@ -288,8 +287,7 @@ bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_li continue; } - const CGovernanceObject* pObj = pSuperblock->GetGovernanceObject(*this); - + const CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); if (!pObj) { continue; } @@ -396,7 +394,7 @@ CSuperblock:: nStatus(SeenObjectStatus::Unknown), vecPayments() { - const CGovernanceObject* pGovObj = GetGovernanceObject(govman); + const CGovernanceObject* pGovObj = govman.FindGovernanceObject(nGovObjHash); if (!pGovObj) { throw std::runtime_error("CSuperblock: Failed to find Governance Object"); @@ -434,14 +432,6 @@ CSuperblock::CSuperblock(int nBlockHeight, std::vector vecPa nGovObjHash = GetHash(); } -CGovernanceObject* CSuperblock::GetGovernanceObject(CGovernanceManager& govman) -{ - AssertLockHeld(govman.cs); - CGovernanceObject* pObj = govman.FindGovernanceObject(nGovObjHash); - return pObj; -} - - /** * Is Valid Superblock Height * @@ -592,6 +582,8 @@ CAmount CSuperblock::GetPaymentsTotalAmount() bool CSuperblock::IsValid(CGovernanceManager& govman, const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward) { + AssertLockHeld(govman.cs); + // TODO : LOCK(cs); // No reason for a lock here now since this method only accesses data // internal to *this and since CSuperblock's are accessed only through @@ -608,8 +600,9 @@ bool CSuperblock::IsValid(CGovernanceManager& govman, const CChain& active_chain int nPayments = CountPayments(); int nMinerAndMasternodePayments = nOutputs - nPayments; + const CGovernanceObject* obj = govman.FindGovernanceObject(nGovObjHash); LogPrint(BCLog::GOBJECT, "CSuperblock::IsValid -- nOutputs = %d, nPayments = %d, GetDataAsHexString = %s\n", - nOutputs, nPayments, GetGovernanceObject(govman)->GetDataAsHexString()); + nOutputs, nPayments, obj ? obj->GetDataAsHexString() : ""); // We require an exact match (including order) between the expected // superblock payments and the payments actually in the block. diff --git a/src/governance/classes.h b/src/governance/classes.h index 1d5e610c5d..1e393f08c0 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -94,13 +94,13 @@ public: // TELL THE ENGINE WE EXECUTED THIS EVENT void SetExecuted() { nStatus = SeenObjectStatus::Executed; } - CGovernanceObject* GetGovernanceObject(CGovernanceManager& govman); - int GetBlockHeight() const { return nBlockHeight; } + const uint256 GetGovernanceObjHash() const { return nGovObjHash; } + int CountPayments() const { return (int)vecPayments.size(); } bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet); CAmount GetPaymentsTotalAmount(); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 932f5b3726..39a0ca21ac 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -795,7 +795,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalGetGovernanceObject(*this); + const auto govobj = FindGovernanceObject(trigger->GetGovernanceObjHash()); const uint256 trigger_hash = govobj->GetHash(); if (trigger->GetBlockHeight() <= nCachedBlockHeight) { // ignore triggers from the past From 39f18ab15483227aaea9fd1b846237b94a14a6e7 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 21:48:31 +0700 Subject: [PATCH 14/15] refactor: move CGoveranceManager code from classes.cpp to governace.cpp There's only code move, no changes. For review use `git show --color-moved=dimmed-zebra` --- src/governance/classes.cpp | 284 --------------------------------- src/governance/governance.cpp | 287 ++++++++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+), 284 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 3e15bd280e..df3c6f3774 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -94,290 +94,6 @@ CAmount ParsePaymentAmount(const std::string& strAmount) return nAmount; } -/** -* Add Governance Object -*/ - -bool CGovernanceManager::AddNewTrigger(uint256 nHash) -{ - AssertLockHeld(cs); - - // IF WE ALREADY HAVE THIS HASH, RETURN - if (mapTrigger.count(nHash)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Already have hash, nHash = %s, count = %d, size = %s\n", - __func__, nHash.GetHex(), mapTrigger.count(nHash), mapTrigger.size()); - return false; - } - - CSuperblock_sptr pSuperblock; - try { - auto pSuperblockTmp = std::make_shared(*this, nHash); - pSuperblock = pSuperblockTmp; - } catch (std::exception& e) { - LogPrintf("CGovernanceManager::%s -- Error creating superblock: %s\n", __func__, e.what()); - return false; - } catch (...) { - LogPrintf("CGovernanceManager::%s -- Unknown Error creating superblock\n", __func__); - return false; - } - - pSuperblock->SetStatus(SeenObjectStatus::Valid); - - mapTrigger.insert(std::make_pair(nHash, pSuperblock)); - - return !pSuperblock->IsExpired(*this); -} - -/** -* -* Clean And Remove -* -*/ - -void CGovernanceManager::CleanAndRemoveTriggers() -{ - AssertLockHeld(cs); - - // 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()) { - bool remove = false; - CGovernanceObject* pObj = nullptr; - const CSuperblock_sptr& pSuperblock = it->second; - if (!pSuperblock) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- nullptr superblock\n", __func__); - remove = true; - } else { - pObj = FindGovernanceObject(it->first); - if (!pObj || pObj->GetObjectType() != GovernanceObject::TRIGGER) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Unknown or non-trigger superblock\n", __func__); - pSuperblock->SetStatus(SeenObjectStatus::ErrorInvalid); - } - - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- superblock status = %d\n", __func__, ToUnderlying(pSuperblock->GetStatus())); - switch (pSuperblock->GetStatus()) { - case SeenObjectStatus::ErrorInvalid: - case SeenObjectStatus::Unknown: - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Unknown or invalid trigger found\n", __func__); - remove = true; - break; - case SeenObjectStatus::Valid: - case SeenObjectStatus::Executed: { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Valid trigger found\n", __func__); - if (pSuperblock->IsExpired(*this)) { - // update corresponding object - pObj->SetExpired(); - remove = true; - } - break; - } - default: - break; - } - } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- %smarked for removal\n", __func__, remove ? "" : "NOT "); - - if (remove) { - std::string strDataAsPlainString = "nullptr"; - if (pObj) { - strDataAsPlainString = pObj->GetDataAsPlainString(); - // mark corresponding object for deletion - pObj->PrepareDeletion(GetTime().count()); - } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Removing trigger object %s\n", __func__, strDataAsPlainString); - // delete the trigger - mapTrigger.erase(it++); - } else { - ++it; - } - } -} - -/** -* Get Active Triggers -* -* - Look through triggers and scan for active ones -* - Return the triggers in a list -*/ - -std::vector CGovernanceManager::GetActiveTriggers() const -{ - AssertLockHeld(cs); - std::vector vecResults; - - // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS - for (const auto& pair : mapTrigger) { - const CGovernanceObject* pObj = FindConstGovernanceObject(pair.first); - if (pObj) { - vecResults.push_back(pair.second); - } - } - - return vecResults; -} - -bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) -{ - LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); - if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { - return false; - } - - LOCK(cs); - // GET ALL ACTIVE TRIGGERS - std::vector vecTriggers = GetActiveTriggers(); - - LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- vecTriggers.size() = %d\n", vecTriggers.size()); - - for (const auto& pSuperblock : vecTriggers) { - if (!pSuperblock) { - LogPrintf("IsSuperblockTriggered -- Non-superblock found, continuing\n"); - continue; - } - - CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); - if (!pObj) { - LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); - continue; - } - - LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- data = %s\n", pObj->GetDataAsPlainString()); - - // note : 12.1 - is epoch calculation correct? - - if (nBlockHeight != pSuperblock->GetBlockHeight()) { - LogPrint(BCLog::GOBJECT, - "IsSuperblockTriggered -- block height doesn't match nBlockHeight = %d, blockStart = %d, " - "continuing\n", - nBlockHeight, pSuperblock->GetBlockHeight()); - continue; - } - - // MAKE SURE THIS TRIGGER IS ACTIVE VIA FUNDING CACHE FLAG - - pObj->UpdateSentinelVariables(tip_mn_list); - - if (pObj->IsSetCachedFunding()) { - LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = true, returning true\n"); - return true; - } else { - LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = false, continuing\n"); - } - } - - return false; -} - - -bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) const -{ - if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { - return false; - } - - AssertLockHeld(cs); - std::vector vecTriggers = GetActiveTriggers(); - int nYesCount = 0; - - for (const auto& pSuperblock : vecTriggers) { - if (!pSuperblock || nBlockHeight != pSuperblock->GetBlockHeight()) { - continue; - } - - const CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); - if (!pObj) { - continue; - } - - // DO WE HAVE A NEW WINNER? - - int nTempYesCount = pObj->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - if (nTempYesCount > nYesCount) { - nYesCount = nTempYesCount; - pSuperblockRet = pSuperblock; - } - } - - return nYesCount > 0; -} - -bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, - std::vector& voutSuperblockRet) -{ - LOCK(cs); - - // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT - - CSuperblock_sptr pSuperblock; - if (!GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { - LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); - return false; - } - - // make sure it's empty, just in case - voutSuperblockRet.clear(); - - // GET SUPERBLOCK OUTPUTS - - // Superblock payments will be appended to the end of the coinbase vout vector - - // TODO: How many payments can we add before things blow up? - // Consider at least following limits: - // - max coinbase tx size - // - max "budget" available - for (int i = 0; i < pSuperblock->CountPayments(); i++) { - CGovernancePayment payment; - if (pSuperblock->GetPayment(i, payment)) { - // SET COINBASE OUTPUT TO SUPERBLOCK SETTING - - CTxOut txout = CTxOut(payment.nAmount, payment.script); - voutSuperblockRet.push_back(txout); - - // PRINT NICE LOG OUTPUT FOR SUPERBLOCK PAYMENT - - CTxDestination dest; - ExtractDestination(payment.script, dest); - - LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- NEW Superblock: output %d (addr %s, amount %d.%08d)\n", - i, EncodeDestination(dest), payment.nAmount / COIN, payment.nAmount % COIN); - } else { - LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Payment not found\n"); - } - } - - return true; -} - -bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, - const CTransaction& txNew, int nBlockHeight, CAmount blockReward) -{ - // GET BEST SUPERBLOCK, SHOULD MATCH - LOCK(cs); - - CSuperblock_sptr pSuperblock; - if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(*this, active_chain, txNew, nBlockHeight, blockReward); - } - - return false; -} - -void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) -{ - LOCK(cs); - - CSuperblock_sptr pSuperblock; - if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { - // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, - // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. - pSuperblock->SetExecuted(); - ResetVotedFundingTrigger(); - } -} - CSuperblock:: CSuperblock() : nGovObjHash(), diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 39a0ca21ac..920cedee16 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -1634,6 +1634,293 @@ void CGovernanceManager::RemoveInvalidVotes() lastMNListForVotingKeys = std::make_shared(tip_mn_list); } +/** + * Add Governance Object + */ + +bool CGovernanceManager::AddNewTrigger(uint256 nHash) +{ + AssertLockHeld(cs); + + // IF WE ALREADY HAVE THIS HASH, RETURN + if (mapTrigger.count(nHash)) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Already have hash, nHash = %s, count = %d, size = %s\n", + __func__, nHash.GetHex(), mapTrigger.count(nHash), mapTrigger.size()); + return false; + } + + CSuperblock_sptr pSuperblock; + try { + auto pSuperblockTmp = std::make_shared(*this, nHash); + pSuperblock = pSuperblockTmp; + } catch (std::exception& e) { + LogPrintf("CGovernanceManager::%s -- Error creating superblock: %s\n", __func__, e.what()); + return false; + } catch (...) { + LogPrintf("CGovernanceManager::%s -- Unknown Error creating superblock\n", __func__); + return false; + } + + pSuperblock->SetStatus(SeenObjectStatus::Valid); + + mapTrigger.insert(std::make_pair(nHash, pSuperblock)); + + return !pSuperblock->IsExpired(*this); +} + +/** + * + * Clean And Remove + * + */ + +void CGovernanceManager::CleanAndRemoveTriggers() +{ + AssertLockHeld(cs); + + // 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()) { + bool remove = false; + CGovernanceObject* pObj = nullptr; + const CSuperblock_sptr& pSuperblock = it->second; + if (!pSuperblock) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- nullptr superblock\n", __func__); + remove = true; + } else { + pObj = FindGovernanceObject(it->first); + if (!pObj || pObj->GetObjectType() != GovernanceObject::TRIGGER) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Unknown or non-trigger superblock\n", __func__); + pSuperblock->SetStatus(SeenObjectStatus::ErrorInvalid); + } + + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- superblock status = %d\n", __func__, + ToUnderlying(pSuperblock->GetStatus())); + switch (pSuperblock->GetStatus()) { + case SeenObjectStatus::ErrorInvalid: + case SeenObjectStatus::Unknown: + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Unknown or invalid trigger found\n", __func__); + remove = true; + break; + case SeenObjectStatus::Valid: + case SeenObjectStatus::Executed: { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Valid trigger found\n", __func__); + if (pSuperblock->IsExpired(*this)) { + // update corresponding object + pObj->SetExpired(); + remove = true; + } + break; + } + default: + break; + } + } + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- %smarked for removal\n", __func__, remove ? "" : "NOT "); + + if (remove) { + std::string strDataAsPlainString = "nullptr"; + if (pObj) { + strDataAsPlainString = pObj->GetDataAsPlainString(); + // mark corresponding object for deletion + pObj->PrepareDeletion(GetTime().count()); + } + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Removing trigger object %s\n", __func__, + strDataAsPlainString); + // delete the trigger + mapTrigger.erase(it++); + } else { + ++it; + } + } +} + +/** + * Get Active Triggers + * + * - Look through triggers and scan for active ones + * - Return the triggers in a list + */ + +std::vector CGovernanceManager::GetActiveTriggers() const +{ + AssertLockHeld(cs); + std::vector vecResults; + + // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS + for (const auto& pair : mapTrigger) { + const CGovernanceObject* pObj = FindConstGovernanceObject(pair.first); + if (pObj) { + vecResults.push_back(pair.second); + } + } + + return vecResults; +} + +bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) +{ + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- Start nBlockHeight = %d\n", nBlockHeight); + if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { + return false; + } + + LOCK(cs); + // GET ALL ACTIVE TRIGGERS + std::vector vecTriggers = GetActiveTriggers(); + + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- vecTriggers.size() = %d\n", vecTriggers.size()); + + for (const auto& pSuperblock : vecTriggers) { + if (!pSuperblock) { + LogPrintf("IsSuperblockTriggered -- Non-superblock found, continuing\n"); + continue; + } + + CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); + if (!pObj) { + LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); + continue; + } + + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- data = %s\n", pObj->GetDataAsPlainString()); + + // note : 12.1 - is epoch calculation correct? + + if (nBlockHeight != pSuperblock->GetBlockHeight()) { + LogPrint(BCLog::GOBJECT, /* Continued */ + "IsSuperblockTriggered -- block height doesn't match nBlockHeight = %d, blockStart = %d, " + "continuing\n", + nBlockHeight, pSuperblock->GetBlockHeight()); + continue; + } + + // MAKE SURE THIS TRIGGER IS ACTIVE VIA FUNDING CACHE FLAG + + pObj->UpdateSentinelVariables(tip_mn_list); + + if (pObj->IsSetCachedFunding()) { + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = true, returning true\n"); + return true; + } else { + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- fCacheFunding = false, continuing\n"); + } + } + + return false; +} + + +bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) +{ + if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { + return false; + } + + AssertLockHeld(cs); + std::vector vecTriggers = GetActiveTriggers(); + int nYesCount = 0; + + for (const auto& pSuperblock : vecTriggers) { + if (!pSuperblock || nBlockHeight != pSuperblock->GetBlockHeight()) { + continue; + } + + const CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); + if (!pObj) { + continue; + } + + // DO WE HAVE A NEW WINNER? + + int nTempYesCount = pObj->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + if (nTempYesCount > nYesCount) { + nYesCount = nTempYesCount; + pSuperblockRet = pSuperblock; + } + } + + return nYesCount > 0; +} + +bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, + std::vector& voutSuperblockRet) +{ + LOCK(cs); + + // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT + + CSuperblock_sptr pSuperblock; + if (!GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); + return false; + } + + // make sure it's empty, just in case + voutSuperblockRet.clear(); + + // GET SUPERBLOCK OUTPUTS + + // Superblock payments will be appended to the end of the coinbase vout vector + + // TODO: How many payments can we add before things blow up? + // Consider at least following limits: + // - max coinbase tx size + // - max "budget" available + for (int i = 0; i < pSuperblock->CountPayments(); i++) { + CGovernancePayment payment; + if (pSuperblock->GetPayment(i, payment)) { + // SET COINBASE OUTPUT TO SUPERBLOCK SETTING + + CTxOut txout = CTxOut(payment.nAmount, payment.script); + voutSuperblockRet.push_back(txout); + + // PRINT NICE LOG OUTPUT FOR SUPERBLOCK PAYMENT + + CTxDestination dest; + ExtractDestination(payment.script, dest); + + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- NEW Superblock: output %d (addr %s, amount %d.%08d)\n", + i, EncodeDestination(dest), payment.nAmount / COIN, payment.nAmount % COIN); + } else { + LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Payment not found\n"); + } + } + + return true; +} + +bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, + const CTransaction& txNew, int nBlockHeight, CAmount blockReward) +{ + // GET BEST SUPERBLOCK, SHOULD MATCH + LOCK(cs); + + CSuperblock_sptr pSuperblock; + if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + return pSuperblock->IsValid(*this, active_chain, txNew, nBlockHeight, blockReward); + } + + return false; +} + +void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight) +{ + LOCK(cs); + + CSuperblock_sptr pSuperblock; + if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, + // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. + pSuperblock->SetExecuted(); + ResetVotedFundingTrigger(); + } +} + + bool AreSuperblocksEnabled(const CSporkManager& sporkman) { return sporkman.IsSporkActive(SPORK_9_SUPERBLOCKS_ENABLED); From 2e368329826c9cd7fbd287266b848df509e4e590 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 6 Oct 2024 22:06:15 +0700 Subject: [PATCH 15/15] refactor: drop circular dependency governance/classes over governance/governance --- src/governance/classes.cpp | 33 +++++++++---------------- src/governance/classes.h | 7 +++--- src/governance/governance.cpp | 13 ++++++---- test/lint/lint-circular-dependencies.sh | 1 - 4 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index df3c6f3774..35202a01d6 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include