From 9638fdce6d39cd0e249786c5ed8d32b505b78f9a Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 8 Oct 2024 17:16:42 +0300 Subject: [PATCH] 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;