fix(net): throttle per-object governance vote sync requests

src/governance/governance.cpp

@@ -34,6 +34,12 @@ constexpr std::chrono::seconds GOVERNANCE_ORPHAN_EXPIRATION_TIME{10min};
 constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h};
 using governance::RELIABLE_PROPAGATION_TIME;
 
+bool IsSyncableObject(const std::shared_ptr<CGovernanceObject>& govobj)
+{
+    const auto& obj = *Assert(govobj);
+    return !obj.IsSetCachedDelete() && !obj.IsSetExpired();
+}
+
 class ScopedLockBool
 {
     bool& ref;
@@ -137,6 +143,38 @@ bool CGovernanceManager::HaveObjectForHash(const uint256& nHash) const
     return (mapObjects.count(nHash) == 1 || mapPostponedObjects.count(nHash) == 1);
 }
 
+bool CGovernanceManager::HaveObjectForFetch(const uint256& nHash) const
+{
+    LOCK(cs_store);
+
+    if (mapErasedGovernanceObjects.count(nHash) != 0) {
+        return false;
+    }
+
+    auto it = mapObjects.find(nHash);
+    if (it != mapObjects.end()) {
+        return IsSyncableObject(it->second);
+    }
+
+    it = mapPostponedObjects.find(nHash);
+    if (it != mapPostponedObjects.end()) {
+        return IsSyncableObject(it->second);
+    }
+
+    return false;
+}
+
+bool CGovernanceManager::HaveSyncableObjectForHash(const uint256& nHash) const
+{
+    LOCK(cs_store);
+    const auto it = mapObjects.find(nHash);
+    if (it == mapObjects.end()) {
+        return false;
+    }
+
+    return IsSyncableObject(it->second);
+}
+
 bool CGovernanceManager::SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const
 {
     LOCK(cs_store);
@@ -334,6 +372,13 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const st
     AddGovernanceObjectInternal(govobj, peer_str);
 }
 
+void CGovernanceManager::AddGovernanceObjectForTesting(const CGovernanceObject& govobj)
+{
+    AssertLockNotHeld(cs_store);
+    LOCK(cs_store);
+    mapObjects.emplace(govobj.GetHash(), std::make_shared<CGovernanceObject>(govobj));
+}
+
 void CGovernanceManager::CheckAndRemove()
 {
     AssertLockNotHeld(cs_store);
@@ -624,14 +669,14 @@ std::vector<CInv> CGovernanceManager::GetSyncableVoteInvs(const uint256& nProp,
         return {};
     }
 
-    const auto& govobj = *Assert(it->second);
-    if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) {
+    if (!IsSyncableObject(it->second)) {
         return {};
     }
 
     std::vector<CInv> invs;
     const auto tip_mn_list = m_dmnman.GetListAtChainTip();
 
+    const auto& govobj = *Assert(it->second);
     LOCK(govobj.cs);
     const auto& fileVotes = govobj.GetVoteFile();
     for (const auto& vote : fileVotes.GetVotes()) {
@@ -656,7 +701,7 @@ std::vector<CInv> CGovernanceManager::GetSyncableObjectInvs() const
     invs.reserve(mapObjects.size());
 
     for (const auto& [nHash, govobj] : mapObjects) {
-        if (Assert(govobj)->IsSetCachedDelete() || govobj->IsSetExpired()) {
+        if (!IsSyncableObject(govobj)) {
             continue;
         }
         invs.emplace_back(MSG_GOVERNANCE_OBJECT, nHash);

src/governance/governance.h

@@ -323,10 +323,18 @@ class CGovernanceManager : public GovernanceStore
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
     void AddGovernanceObject(CGovernanceObject& govobj, const std::string& peer_str)
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay);
+    /** Test-only helper: inserts an object into the syncable object store without
+     *  running collateral or chain validation. */
+    void AddGovernanceObjectForTesting(const CGovernanceObject& govobj)
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
 
     // Thread-safe accessors
     bool HaveObjectForHash(const uint256& nHash) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
+    bool HaveObjectForFetch(const uint256& nHash) const
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
+    bool HaveSyncableObjectForHash(const uint256& nHash) const
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
     bool HaveVoteForHash(const uint256& nHash) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
     bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const

src/governance/net_governance.cpp

@@ -15,9 +15,27 @@
 #include <netfulfilledman.h>
 #include <netmessagemaker.h>
 #include <scheduler.h>
+#include <streams.h>
+#include <version.h>
+
+#include <vector>
 
 class CConnman;
 
+namespace {
+bool IsEmptyBloomFilter(const CBloomFilter& filter)
+{
+    // CBloomFilter does not expose its backing bytes. Governance uses an empty
+    // filter as an object-fetch signal, so inspect the serialized vector field.
+    CDataStream serialized_filter{SER_NETWORK, PROTOCOL_VERSION};
+    serialized_filter << filter;
+
+    std::vector<unsigned char> filter_data;
+    serialized_filter >> filter_data;
+    return filter_data.empty();
+}
+} // namespace
+
 void NetGovernance::Schedule(CScheduler& scheduler)
 {
     // Code below is meant to be running only if governance validation is enabled
@@ -86,17 +104,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)) {
-                // 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;
+        const bool full_sync{nProp == uint256()};
+        const bool object_fetch{!full_sync && IsEmptyBloomFilter(filter)};
+        // Nonzero govsync with an empty filter is used to retry missing-object
+        // fetches for orphan votes. Only full sync and actual known-object vote
+        // sync are fulfilled-request limited.
+        const bool track_request{full_sync || (!object_fetch && m_gov_manager.HaveSyncableObjectForHash(nProp))};
+        const std::string fulfilled_request{full_sync ?
+                                                NetMsgType::MNGOVERNANCESYNC :
+                                                strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC,
+                                                          nProp.ToString())};
+        assert(m_netfulfilledman.IsValid());
+        if (track_request && m_netfulfilledman.HasFulfilledRequest(peer.addr, fulfilled_request)) {
+            // Asking for the same governance data multiple times in a short period of time is no good
+            LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- peer already asked me for %s\n",
+                     full_sync ? "the list" : strprintf("votes for %s", nProp.ToString()));
+            m_peer_manager->PeerMisbehaving(peer.GetId(), 20);
+            return;
+        }
+        if (track_request) {
+            m_netfulfilledman.AddFulfilledRequest(peer.addr, fulfilled_request);
+        }
+
+        if (object_fetch) {
+            if (m_gov_manager.HaveObjectForFetch(nProp)) {
+                CNetMsgMaker msgMaker(peer.GetCommonVersion());
+                m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::INV,
+                                                           std::vector<CInv>{CInv{MSG_GOVERNANCE_OBJECT, nProp}}));
             }
-            m_netfulfilledman.AddFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC);
+            return;
+        }
 
+        if (full_sync) {
+            // Full sync of all governance objects
             auto invs = m_gov_manager.GetSyncableObjectInvs();
             LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing %d objects to peer=%d\n", invs.size(), peer.GetId());