From 4b40c9ef5e0c041bbff096393b1376a74ba4c779 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 16 Jan 2026 12:11:31 +0300 Subject: [PATCH 1/5] refactor: separate network operations from CGovernanceManager sync methods Replace SyncObjects() and SyncSingleObjVotes() with pure data accessors: - GetSyncableObjectInvs(): returns inventory of syncable governance objects - GetSyncableVoteInvs(): returns inventory of syncable votes for an object Move network message construction (SYNCSTATUSCOUNT) to NetGovernance, following the established pattern in NetSigning and NetInstantSend. This improves separation of concerns: - CGovernanceManager handles data access only - NetGovernance handles network protocol operations Co-Authored-By: Claude Opus 4.5 --- src/governance/governance.cpp | 58 ++++++------------------------- src/governance/governance.h | 7 ++-- src/governance/net_governance.cpp | 35 ++++++++++++++++--- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 671d9ec29ef9..d7737a232e03 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -591,37 +591,23 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return true; } -MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman) +std::vector CGovernanceManager::GetSyncableVoteInvs(const uint256& nProp, const CBloomFilter& filter) const { LOCK(cs_store); - // do not provide any data until our node is synced - if (!m_mn_sync.IsSynced()) return {}; - // SYNC GOVERNANCE OBJECTS WITH OTHER CLIENT - - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing single object to peer=%d, nProp = %s\n", __func__, peer.GetId(), nProp.ToString()); - - // single valid object and its valid votes auto it = mapObjects.find(nProp); if (it == mapObjects.end()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- no matching object for hash %s, peer=%d\n", __func__, nProp.ToString(), peer.GetId()); return {}; } - const auto& govobj = *Assert(it->second); - std::string strHash = it->first.ToString(); - - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); + const auto& govobj = *Assert(it->second); if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- not syncing deleted/expired govobj: %s, peer=%d\n", __func__, - strHash, peer.GetId()); return {}; } - MessageProcessingResult ret{}; + std::vector invs; const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); - { LOCK(govobj.cs); const auto& fileVotes = govobj.GetVoteFile(); for (const auto& vote : fileVotes.GetVotes()) { @@ -632,51 +618,27 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons if (filter.contains(nVoteHash) || !vote.IsValid(tip_mn_list, onlyVotingKeyAllowed)) { continue; } - ret.m_inventory.emplace_back(MSG_GOVERNANCE_OBJECT_VOTE, nVoteHash); - } + invs.emplace_back(MSG_GOVERNANCE_OBJECT_VOTE, nVoteHash); } - CNetMsgMaker msgMaker(peer.GetCommonVersion()); - connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ_VOTE, - static_cast(ret.m_inventory.size()))); - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- sent %d votes to peer=%d\n", __func__, ret.m_inventory.size(), - peer.GetId()); - return ret; + return invs; } -MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const +std::vector CGovernanceManager::GetSyncableObjectInvs() const { LOCK(cs_store); - // do not provide any data until our node is synced - if (!m_mn_sync.IsSynced()) return {}; - - // SYNC GOVERNANCE OBJECTS WITH OTHER CLIENT - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing all objects to peer=%d\n", __func__, peer.GetId()); + std::vector invs; + invs.reserve(mapObjects.size()); - // all valid objects, no votes - MessageProcessingResult ret{}; for (const auto& [nHash, govobj] : mapObjects) { - std::string strHash = nHash.ToString(); - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); - if (Assert(govobj)->IsSetCachedDelete() || govobj->IsSetExpired()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- not syncing deleted/expired govobj: %s, peer=%d\n", __func__, - strHash, peer.GetId()); continue; } - - // Push the inventory budget proposal message over to the other client - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing govobj: %s, peer=%d\n", __func__, strHash, peer.GetId()); - ret.m_inventory.emplace_back(MSG_GOVERNANCE_OBJECT, nHash); + invs.emplace_back(MSG_GOVERNANCE_OBJECT, nHash); } - CNetMsgMaker msgMaker(peer.GetCommonVersion()); - connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ, - static_cast(ret.m_inventory.size()))); - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- sent %d objects to peer=%d\n", __func__, ret.m_inventory.size(), - peer.GetId()); - return ret; + return invs; } void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) diff --git a/src/governance/governance.h b/src/governance/governance.h index a8d5f324acbc..2b3d5ffba155 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -373,10 +373,11 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const + /** Returns inventory items for all syncable (non-deleted, non-expired) governance objects */ + [[nodiscard]] std::vector GetSyncableObjectInvs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + /** Returns inventory items for syncable votes on a specific object, filtered by bloom filter */ + [[nodiscard]] std::vector GetSyncableVoteInvs(const uint256& nProp, const CBloomFilter& filter) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, - CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); /// Called to indicate a requested object or vote has been received bool AcceptMessage(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool ProcessObject(const CNode& peer, const uint256& hash, CGovernanceObject& govobj) diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index 560f26fdc5ec..2c022670dc41 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -5,12 +5,15 @@ #include #include +#include #include #include #include #include +#include #include #include +#include #include class CConnman; @@ -62,17 +65,39 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing governance objects to our peer %s\n", peer.GetLogString()); if (nProp == uint256()) { + // Full sync of all governance objects assert(m_netfulfilledman.IsValid()); - if (!m_netfulfilledman.HasFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC)) { - m_netfulfilledman.AddFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC); - m_peer_manager->PeerPostProcessMessage(m_gov_manager.SyncObjects(peer, m_connman)); - } else { + if (m_netfulfilledman.HasFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC)) { // Asking for the whole list multiple times in a short period of time is no good LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- peer already asked me for the list\n"); m_peer_manager->PeerMisbehaving(peer.GetId(), 20); + return; } + m_netfulfilledman.AddFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC); + + auto invs = m_gov_manager.GetSyncableObjectInvs(); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing %d objects to peer=%d\n", invs.size(), peer.GetId()); + + CNetMsgMaker msgMaker(peer.GetCommonVersion()); + m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ, + static_cast(invs.size()))); + + MessageProcessingResult ret; + ret.m_inventory = std::move(invs); + m_peer_manager->PeerPostProcessMessage(std::move(ret)); } else { - m_peer_manager->PeerPostProcessMessage(m_gov_manager.SyncSingleObjVotes(peer, nProp, filter, m_connman)); + // Sync votes for a specific governance object + auto invs = m_gov_manager.GetSyncableVoteInvs(nProp, filter); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing %d votes for %s to peer=%d\n", invs.size(), + nProp.ToString(), peer.GetId()); + + CNetMsgMaker msgMaker(peer.GetCommonVersion()); + m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ_VOTE, + static_cast(invs.size()))); + + MessageProcessingResult ret; + ret.m_inventory = std::move(invs); + m_peer_manager->PeerPostProcessMessage(std::move(ret)); } } // A NEW GOVERNANCE OBJECT HAS ARRIVED From af1ab7f943f3cd46ffaaee4b359c7291a95a26e4 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 16 Jan 2026 12:12:00 +0300 Subject: [PATCH 2/5] refactor: move RequestGovernanceObject to network layer Complete separation of network operations from CGovernanceManager by moving RequestGovernanceObject functionality to the appropriate network layer classes. Changes to CGovernanceManager (data access only): - Add GetVoteBloomFilter() to build bloom filter for sync requests - Add GetOrphanVoteObjectHashes() to get object hashes for orphan votes - Modify ProcessVote() to use output param hashToRequest instead of doing network operations internally - Remove RequestGovernanceObject() and RequestOrphanObjects() Changes to SyncManager: - Add SendGovernanceObjectSyncRequest() for vote sync requests - Update RequestGovernanceObjectVotes() to use new method Changes to NetGovernance: - Handle orphan vote requests in ProcessMessage() after ProcessVote - Move RequestOrphanObjects logic into Schedule() for periodic requests Co-Authored-By: Claude Opus 4.5 --- src/governance/governance.cpp | 100 ++++++++++++------------------ src/governance/governance.h | 11 ++-- src/governance/net_governance.cpp | 26 +++++++- src/node/sync_manager.cpp | 14 ++++- src/node/sync_manager.h | 1 + 5 files changed, 82 insertions(+), 70 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index d7737a232e03..aed4aa7baeda 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -740,16 +739,20 @@ bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGover { AssertLockNotHeld(cs_store); AssertLockNotHeld(cs_relay); - bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); + uint256 hashToRequest; // Ignored for local votes (no peer to request from) + bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, hashToRequest); if (fOK) { RelayVote(vote); } return fOK; } -bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) +bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, + uint256& hashToRequest) { AssertLockNotHeld(cs_store); + hashToRequest = uint256(); + ENTER_CRITICAL_SECTION(cs_store); uint256 nHashVote = vote.GetHash(); uint256 nHashGovobj = vote.GetParentHash(); @@ -777,12 +780,8 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING); if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, count_seconds(GetTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME)))) { - LEAVE_CRITICAL_SECTION(cs_store); - RequestGovernanceObject(pfrom, nHashGovobj, connman); - LogPrint(BCLog::GOBJECT, "%s\n", msg); - return false; + hashToRequest = nHashGovobj; // Caller should request this object } - LogPrint(BCLog::GOBJECT, "%s\n", msg); LEAVE_CRITICAL_SECTION(cs_store); return false; @@ -875,36 +874,24 @@ void CGovernanceManager::CheckPostponedObjects() } } -void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter) const +CBloomFilter CGovernanceManager::GetVoteBloomFilter(const uint256& nHash) const { - AssertLockNotHeld(cs_store); - if (!pfrom) { - return; - } - - LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObject -- nHash %s peer=%d\n", nHash.ToString(), pfrom->GetId()); - - CNetMsgMaker msgMaker(pfrom->GetCommonVersion()); + LOCK(cs_store); - CBloomFilter filter; + auto pObj = FindConstGovernanceObjectInternal(nHash); + if (!pObj) { + return CBloomFilter(); + } - size_t nVoteCount = 0; - if (fUseFilter) { - LOCK(cs_store); - auto pObj = FindConstGovernanceObjectInternal(nHash); + CBloomFilter filter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, + GetRand(/*nMax=*/999999), BLOOM_UPDATE_ALL); - if (pObj) { - filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRand(/*nMax=*/999999), BLOOM_UPDATE_ALL); - std::vector vecVotes = WITH_LOCK(pObj->cs, return pObj->GetVoteFile().GetVotes()); - nVoteCount = vecVotes.size(); - for (const auto& vote : vecVotes) { - filter.insert(vote.GetHash()); - } - } + std::vector vecVotes = WITH_LOCK(pObj->cs, return pObj->GetVoteFile().GetVotes()); + for (const auto& vote : vecVotes) { + filter.insert(vote.GetHash()); } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObject -- nHash %s nVoteCount %d peer=%d\n", nHash.ToString(), nVoteCount, pfrom->GetId()); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter)); + return filter; } CDeterministicMNManager& CGovernanceManager::GetMNManager() { return *Assert(m_dmnman); } @@ -1084,45 +1071,34 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) ExecuteBestSuperblock(Assert(m_dmnman)->GetListAtChainTip(), pindex->nHeight); } -void CGovernanceManager::RequestOrphanObjects(CConnman& connman) +std::vector CGovernanceManager::GetOrphanVoteObjectHashes() { - AssertLockNotHeld(cs_store); + LOCK(cs_store); - std::vector vecHashesFiltered; int64_t nNow = GetTime().count(); - { - LOCK(cs_store); - // clean up outdated before requesting - const vote_cmm_t::list_t& items = cmmapOrphanVotes.GetItemList(); - for (auto it = items.begin(); it != items.end();) { - auto prevIt = it; - ++it; - const auto& [_, time] = prevIt->value; - if (time < nNow) { - cmmapOrphanVotes.Erase(prevIt->key, prevIt->value); - } - } - - std::vector vecHashes; - cmmapOrphanVotes.GetKeys(vecHashes); - for (const uint256& nHash : vecHashes) { - if (mapObjects.find(nHash) == mapObjects.end()) { - vecHashesFiltered.push_back(nHash); - } + // Clean up expired orphan votes + const vote_cmm_t::list_t& items = cmmapOrphanVotes.GetItemList(); + for (auto it = items.begin(); it != items.end();) { + auto prevIt = it; + ++it; + const auto& [_, time] = prevIt->value; + if (time < nNow) { + cmmapOrphanVotes.Erase(prevIt->key, prevIt->value); } } - const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly}; - LogPrint(BCLog::GOBJECT, "CGovernanceObject::RequestOrphanObjects -- number objects = %d\n", vecHashesFiltered.size()); - for (const uint256& nHash : vecHashesFiltered) { - for (CNode* pnode : snap.Nodes()) { - if (!pnode->CanRelay()) { - continue; - } - RequestGovernanceObject(pnode, nHash, connman); + // Get hashes of objects we don't have yet + std::vector vecHashesFiltered; + std::vector vecHashes; + cmmapOrphanVotes.GetKeys(vecHashes); + for (const uint256& nHash : vecHashes) { + if (mapObjects.find(nHash) == mapObjects.end()) { + vecHashesFiltered.push_back(nHash); } } + + return vecHashesFiltered; } void CGovernanceManager::RemoveInvalidVotes() diff --git a/src/governance/governance.h b/src/governance/governance.h index 2b3d5ffba155..b2bc3ef62fe0 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -367,12 +367,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent // Used by NetGovernance std::vector FetchRelayInventory() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - void RequestOrphanObjects(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + /** Get hashes of governance objects for which we have orphan votes. Also cleans up expired orphans. */ + [[nodiscard]] std::vector GetOrphanVoteObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs_store); std::pair, std::vector> FetchGovernanceObjectVotes( size_t peers_per_hash_max, int64_t now, std::map>& map_asked_recently) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); - void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const - EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + /** Build bloom filter of existing votes for a governance object (for sync requests) */ + [[nodiscard]] CBloomFilter GetVoteBloomFilter(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); /** Returns inventory items for all syncable (non-deleted, non-expired) governance objects */ [[nodiscard]] std::vector GetSyncableObjectInvs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); /** Returns inventory items for syncable votes on a specific object, filtered by bloom filter */ @@ -384,7 +385,9 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !cs_store, !cs_relay); CDeterministicMNManager& GetMNManager(); - bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) + /** Process a governance vote. Returns true on success. + * If the vote is for an unknown object (orphan), hashToRequest is set to the object hash. */ + bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, uint256& hashToRequest) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index 2c022670dc41..c50cc724863f 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -27,8 +27,21 @@ void NetGovernance::Schedule(CScheduler& scheduler) [this]() -> void { if (!m_node_sync.IsSynced()) return; - // CHECK OBJECTS WE'VE ASKED FOR, REMOVE OLD ENTRIES - m_gov_manager.RequestOrphanObjects(m_connman); + // Request governance objects for orphan votes + auto vecOrphanHashes = m_gov_manager.GetOrphanVoteObjectHashes(); + if (!vecOrphanHashes.empty()) { + LogPrint(BCLog::GOBJECT, "NetGovernance::Schedule -- requesting %d orphan objects\n", + vecOrphanHashes.size()); + const CConnman::NodesSnapshot snap{m_connman, CConnman::FullyConnectedOnly}; + for (const uint256& nHash : vecOrphanHashes) { + for (CNode* pnode : snap.Nodes()) { + if (!pnode->CanRelay()) continue; + CNetMsgMaker msgMaker(pnode->GetCommonVersion()); + CBloomFilter filter; // Empty filter - we want the object, not votes + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter)); + } + } + } // CHECK AND REMOVE - REPROCESS GOVERNANCE OBJECTS m_gov_manager.CheckAndRemove(); @@ -158,7 +171,8 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa } CGovernanceException exception; - if (m_gov_manager.ProcessVote(&peer, vote, exception, m_connman)) { + uint256 hashToRequest; + if (m_gov_manager.ProcessVote(&peer, vote, exception, hashToRequest)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- %s new\n", strHash); m_node_sync.BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE"); @@ -175,6 +189,12 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa // m_peer_manager->PeerRelayInv(CInv{MSG_GOVERNANCE_OBJECT_VOTE, nHash}); } else { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Rejected vote, error = %s\n", exception.what()); + if (hashToRequest != uint256()) { + // Orphan vote - request the missing governance object + CNetMsgMaker msgMaker(peer.GetCommonVersion()); + CBloomFilter filter; // Empty filter - we just want the object, not votes + m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, hashToRequest, filter)); + } if ((exception.GetNodePenalty() != 0) && m_node_sync.IsSynced()) { m_peer_manager->PeerMisbehaving(peer.GetId(), exception.GetNodePenalty()); } diff --git a/src/node/sync_manager.cpp b/src/node/sync_manager.cpp index fb368f03e5df..2d4b97e01977 100644 --- a/src/node/sync_manager.cpp +++ b/src/node/sync_manager.cpp @@ -36,6 +36,18 @@ void SyncManager::SendGovernanceSyncRequest(CNode* pnode) const m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, uint256(), filter)); } +void SyncManager::SendGovernanceObjectSyncRequest(CNode* pnode, const uint256& nHash, bool fUseFilter) const +{ + if (!pnode) return; + + LogPrint(BCLog::GOBJECT, "SyncManager::%s -- nHash %s peer=%d\n", __func__, nHash.ToString(), pnode->GetId()); + + CBloomFilter filter = fUseFilter ? m_gov_manager.GetVoteBloomFilter(nHash) : CBloomFilter(); + + CNetMsgMaker msgMaker(pnode->GetCommonVersion()); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter)); +} + int SyncManager::RequestGovernanceObjectVotes(const std::vector& vNodesCopy) const { // Maximum number of nodes to request votes from for the same object hash on real networks @@ -107,7 +119,7 @@ int SyncManager::RequestGovernanceObjectVotes(const std::vector& vNodesC // to early to ask the same node if (mapAskedRecently[nHashGovobj].count(pnode->addr)) continue; - m_gov_manager.RequestGovernanceObject(pnode, nHashGovobj, m_connman, true); + SendGovernanceObjectSyncRequest(pnode, nHashGovobj, true); mapAskedRecently[nHashGovobj][pnode->addr] = nNow + nTimeout; fAsked = true; // stop loop if max number of peers per obj was asked diff --git a/src/node/sync_manager.h b/src/node/sync_manager.h index ac6d5fe75ec3..5dbdc3a73955 100644 --- a/src/node/sync_manager.h +++ b/src/node/sync_manager.h @@ -29,6 +29,7 @@ class SyncManager final : public NetHandler private: void SendGovernanceSyncRequest(CNode* pnode) const; + void SendGovernanceObjectSyncRequest(CNode* pnode, const uint256& nHash, bool fUseFilter) const; int RequestGovernanceObjectVotes(const std::vector& vNodesCopy) const; void ProcessTick(); From 4a47b4e750651a0cfcaeca22037282e396989dca Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 21 Jan 2026 18:12:38 +0700 Subject: [PATCH 3/5] refactor: replace manual lock/unlock to RAII LOCK in governance.cpp --- src/governance/governance.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index aed4aa7baeda..14b7326738aa 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -753,14 +753,13 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, AssertLockNotHeld(cs_store); hashToRequest = uint256(); - ENTER_CRITICAL_SECTION(cs_store); + LOCK(cs_store); uint256 nHashVote = vote.GetHash(); uint256 nHashGovobj = vote.GetParentHash(); if (cmapVoteToObject.HasKey(nHashVote)) { LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- skipping known valid vote %s for object %s\n", __func__, nHashVote.ToString(), nHashGovobj.ToString()); - LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -769,7 +768,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, __func__, vote.GetMasternodeOutpoint().ToStringShort(), nHashGovobj.ToString())}; LogPrint(BCLog::GOBJECT, "%s\n", msg); exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); - LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -783,7 +781,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, hashToRequest = nHashGovobj; // Caller should request this object } LogPrint(BCLog::GOBJECT, "%s\n", msg); - LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -792,7 +789,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- ignoring vote for expired or deleted object, hash = %s\n", __func__, nHashGovobj.ToString()); - LEAVE_CRITICAL_SECTION(cs_store); return false; } @@ -802,7 +798,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, } else if (exception.GetType() == GOVERNANCE_EXCEPTION_PERMANENT_ERROR && exception.GetNodePenalty() == 20) { cmapInvalidVotes.Insert(nHashVote, vote); } - LEAVE_CRITICAL_SECTION(cs_store); return fOk; } From 0a92e25eec1c7b65e6c36bad6efb8d9f2567154d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 21 Jan 2026 16:07:13 +0700 Subject: [PATCH 4/5] refactor: relay inventory directly without using MessageProcessingResult --- src/governance/core_write.cpp | 1 + src/governance/governance.h | 1 - src/governance/net_governance.cpp | 15 ++++++--------- src/rpc/masternode.cpp | 1 + 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/governance/core_write.cpp b/src/governance/core_write.cpp index 4bb3ca996066..3fab5b078030 100644 --- a/src/governance/core_write.cpp +++ b/src/governance/core_write.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include diff --git a/src/governance/governance.h b/src/governance/governance.h index b2bc3ef62fe0..7bdfbb4744e1 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -8,7 +8,6 @@ #include #include #include -#include #include #include diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index c50cc724863f..7849ff91544e 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -94,10 +93,9 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa CNetMsgMaker msgMaker(peer.GetCommonVersion()); m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ, static_cast(invs.size()))); - - MessageProcessingResult ret; - ret.m_inventory = std::move(invs); - m_peer_manager->PeerPostProcessMessage(std::move(ret)); + for (const auto& inv : invs) { + m_peer_manager->PeerRelayInv(inv); + } } else { // Sync votes for a specific governance object auto invs = m_gov_manager.GetSyncableVoteInvs(nProp, filter); @@ -107,10 +105,9 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa CNetMsgMaker msgMaker(peer.GetCommonVersion()); m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_GOVOBJ_VOTE, static_cast(invs.size()))); - - MessageProcessingResult ret; - ret.m_inventory = std::move(invs); - m_peer_manager->PeerPostProcessMessage(std::move(ret)); + for (const auto& inv : invs) { + m_peer_manager->PeerRelayInv(inv); + } } } // A NEW GOVERNANCE OBJECT HAS ARRIVED diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 6ba736403719..dfb9614497da 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include From 32b761b4857c2462b39880d584ee1e75a179e7a7 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 21 Jan 2026 21:24:13 +0300 Subject: [PATCH 5/5] refactor: apply review suggestions --- src/governance/governance.cpp | 4 ++-- src/node/sync_manager.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 14b7326738aa..df0e4b7093c2 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -751,7 +751,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, uint256& hashToRequest) { AssertLockNotHeld(cs_store); - hashToRequest = uint256(); + hashToRequest = uint256{}; LOCK(cs_store); uint256 nHashVote = vote.GetHash(); @@ -875,7 +875,7 @@ CBloomFilter CGovernanceManager::GetVoteBloomFilter(const uint256& nHash) const auto pObj = FindConstGovernanceObjectInternal(nHash); if (!pObj) { - return CBloomFilter(); + return CBloomFilter{}; } CBloomFilter filter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, diff --git a/src/node/sync_manager.cpp b/src/node/sync_manager.cpp index 2d4b97e01977..8acf829afd67 100644 --- a/src/node/sync_manager.cpp +++ b/src/node/sync_manager.cpp @@ -42,7 +42,7 @@ void SyncManager::SendGovernanceObjectSyncRequest(CNode* pnode, const uint256& n LogPrint(BCLog::GOBJECT, "SyncManager::%s -- nHash %s peer=%d\n", __func__, nHash.ToString(), pnode->GetId()); - CBloomFilter filter = fUseFilter ? m_gov_manager.GetVoteBloomFilter(nHash) : CBloomFilter(); + CBloomFilter filter = fUseFilter ? m_gov_manager.GetVoteBloomFilter(nHash) : CBloomFilter{}; CNetMsgMaker msgMaker(pnode->GetCommonVersion()); m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter));