governance: introduce task for relaying governance objects

Author: kwvg

src/dsnotificationinterface.cpp

@@ -24,15 +24,13 @@
 CDSNotificationInterface::CDSNotificationInterface(CConnman& connman,
                                                    CMasternodeSync& mn_sync,
                                                    CGovernanceManager& govman,
-                                                   PeerManager& peerman,
                                                    const ChainstateManager& chainman,
                                                    const std::unique_ptr<CDeterministicMNManager>& dmnman,
                                                    const std::unique_ptr<LLMQContext>& llmq_ctx,
                                                    const std::unique_ptr<CJContext>& cj_ctx)
   : m_connman(connman),
     m_mn_sync(mn_sync),
     m_govman(govman),
-    m_peerman(peerman),
     m_chainman(chainman),
     m_dmnman(dmnman),
     m_llmq_ctx(llmq_ctx),
@@ -92,7 +90,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con
     m_llmq_ctx->qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload);
 
     if (m_govman.IsValid()) {
-        m_govman.UpdatedBlockTip(pindexNew, m_peerman);
+        m_govman.UpdatedBlockTip(pindexNew);
     }
 }
 

src/dsnotificationinterface.h

@@ -12,7 +12,6 @@ class CDeterministicMNManager;
 class CGovernanceManager;
 class ChainstateManager;
 class CMasternodeSync;
-class PeerManager;
 struct CJContext;
 struct LLMQContext;
 
@@ -22,7 +21,6 @@ class CDSNotificationInterface : public CValidationInterface
     explicit CDSNotificationInterface(CConnman& connman,
                                       CMasternodeSync& mn_sync,
                                       CGovernanceManager& govman,
-                                      PeerManager& peerman,
                                       const ChainstateManager& chainman,
                                       const std::unique_ptr<CDeterministicMNManager>& dmnman,
                                       const std::unique_ptr<LLMQContext>& llmq_ctx,
@@ -50,7 +48,6 @@ class CDSNotificationInterface : public CValidationInterface
     CConnman& m_connman;
     CMasternodeSync& m_mn_sync;
     CGovernanceManager& m_govman;
-    PeerManager& m_peerman;
     const ChainstateManager& m_chainman;
     const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
     const std::unique_ptr<LLMQContext>& m_llmq_ctx;

src/governance/governance.cpp

@@ -23,6 +23,7 @@
 #include <spork.h>
 #include <timedata.h>
 #include <util/ranges.h>
+#include <util/thread.h>
 #include <util/time.h>
 #include <validation.h>
 
@@ -87,6 +88,22 @@ CGovernanceManager::~CGovernanceManager()
     m_db->Store(*this);
 }
 
+void CGovernanceManager::Schedule(CScheduler& scheduler, PeerManager& peerman)
+{
+    assert(IsValid());
+
+    scheduler.scheduleEvery(
+        [this, &peerman]() -> void {
+            LOCK(cs_relay);
+            for (const auto& inv : m_relay_invs) {
+                peerman.RelayInv(inv);
+            }
+            m_relay_invs.clear();
+        },
+        // Tests need tighter timings to avoid timeouts, use more relaxed pacing otherwise
+        Params().IsMockableChain() ? std::chrono::seconds{1} : std::chrono::seconds{5});
+}
+
 bool CGovernanceManager::LoadCache(bool load_cache)
 {
     assert(m_db != nullptr);
@@ -98,18 +115,18 @@ bool CGovernanceManager::LoadCache(bool load_cache)
     return is_valid;
 }
 
-void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const
+void CGovernanceManager::RelayObject(const CGovernanceObject& obj)
 {
     if (!m_mn_sync.IsSynced()) {
         LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__);
         return;
     }
 
-    CInv inv(MSG_GOVERNANCE_OBJECT, obj.GetHash());
-    peerman.RelayInv(inv);
+    LOCK(cs_relay);
+    m_relay_invs.emplace_back(MSG_GOVERNANCE_OBJECT, obj.GetHash());
 }
 
-void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const
+void CGovernanceManager::RelayVote(const CGovernanceVote& vote)
 {
     if (!m_mn_sync.IsSynced()) {
         LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__);
@@ -122,8 +139,8 @@ void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceVot
         return;
     }
 
-    CInv inv(MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash());
-    peerman.RelayInv(inv);
+    LOCK(cs_relay);
+    m_relay_invs.emplace_back(MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash());
 }
 
 // Accessors for thread-safe access to maps
@@ -174,7 +191,7 @@ void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj)
     mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj));
 }
 
-MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
+MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv)
 {
     if (!IsValid()) return {};
     if (!m_mn_sync.IsBlockchainSynced()) return {};
@@ -267,7 +284,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
             return ret;
         }
 
-        AddGovernanceObject(govobj, peerman, &peer);
+        AddGovernanceObject(govobj, &peer);
         return ret;
     }
 
@@ -301,7 +318,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
         if (ProcessVote(&peer, vote, exception, connman)) {
             LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- %s new\n", strHash);
             m_mn_sync.BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE");
-            RelayMessage(peerman, vote);
+            RelayVote(vote);
         } else {
             LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Rejected vote, error = %s\n", exception.what());
             if ((exception.GetNodePenalty() != 0) && m_mn_sync.IsSynced()) {
@@ -316,7 +333,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
     return {};
 }
 
-void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman)
+void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj)
 {
     uint256 nHash = govobj.GetHash();
     std::vector<vote_time_pair_t> vecVotePairs;
@@ -333,7 +350,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)) {
-            RelayMessage(peerman, vote);
+            RelayVote(vote);
             fRemove = true;
         }
         if (fRemove) {
@@ -342,7 +359,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager
     }
 }
 
-void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom)
+void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom)
 {
     uint256 nHash = govobj.GetHash();
     std::string strHash = nHash.ToString();
@@ -387,7 +404,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana
     }
 
     LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, pfrom ? pfrom->GetLogString() : "nullptr");
-    RelayMessage(peerman, govobj);
+    RelayObject(govobj);
 
     // Update the rate buffer
     MasternodeRateUpdate(govobj);
@@ -396,7 +413,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana
 
     // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT
 
-    CheckOrphanVotes(govobj, peerman);
+    CheckOrphanVotes(govobj);
 
     // SEND NOTIFICATION TO SCRIPT/ZMQ
     GetMainSignals().NotifyGovernanceObject(std::make_shared<const Governance::Object>(govobj.Object()), nHash.ToString());
@@ -878,11 +895,11 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo
     return false;
 }
 
-bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman)
+bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
 {
     bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman);
     if (fOK) {
-        RelayMessage(peerman, vote);
+        RelayVote(vote);
     }
     return fOK;
 }
@@ -940,7 +957,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
     return fOk;
 }
 
-void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman)
+void CGovernanceManager::CheckPostponedObjects()
 {
     if (!m_mn_sync.IsSynced()) return;
 
@@ -957,7 +974,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman)
         bool fMissingConfirmations;
         if (govobj.IsCollateralValid(m_chainman, strError, fMissingConfirmations)) {
             if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), m_chainman, strError, false)) {
-                AddGovernanceObject(govobj, peerman);
+                AddGovernanceObject(govobj);
             } else {
                 LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString());
             }
@@ -990,7 +1007,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman)
             if (fValid) {
                 if (fReady) {
                     LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- additional relay: hash = %s\n", govobj.GetHash().ToString());
-                    RelayMessage(peerman, govobj);
+                    RelayObject(govobj);
                 } else {
                     it++;
                     continue;
@@ -1289,7 +1306,7 @@ UniValue CGovernanceManager::ToJson() const
     return jsonObj;
 }
 
-void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman)
+void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex)
 {
     // Note this gets called from ActivateBestChain without cs_main being held
     // so it should be safe to lock our mutex here without risking a deadlock
@@ -1307,7 +1324,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, PeerManager&
         RemoveInvalidVotes();
     }
 
-    CheckPostponedObjects(peerman);
+    CheckPostponedObjects();
 
     ExecuteBestSuperblock(Assert(m_dmnman)->GetListAtChainTip(), pindex->nHeight);
 }

src/governance/governance.h

@@ -31,6 +31,7 @@ template<typename T>
 class CFlatDB;
 class CInv;
 class CNode;
+class CScheduler;
 class PeerManager;
 
 class CDeterministicMNList;
@@ -271,18 +272,23 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     std::optional<uint256> votedFundingYesTriggerHash;
     std::map<uint256, std::shared_ptr<CSuperblock>> mapTrigger;
 
+    mutable Mutex cs_relay;
+    std::vector<CInv> m_relay_invs GUARDED_BY(cs_relay);
+
 public:
     explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman,
                                 const ChainstateManager& chainman,
                                 const std::unique_ptr<CDeterministicMNManager>& dmnman, CMasternodeSync& mn_sync);
     ~CGovernanceManager();
 
+    void Schedule(CScheduler& scheduler, PeerManager& peerman);
+
     bool LoadCache(bool load_cache);
 
     bool IsValid() const override { return is_valid; }
 
-    void RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const;
-    void RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const;
+    void RelayObject(const CGovernanceObject& obj);
+    void RelayVote(const CGovernanceVote& vote);
 
     /**
      * This is called by AlreadyHave in net_processing.cpp as part of the inventory
@@ -294,7 +300,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman);
     [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const;
 
-    [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv);
+    [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv);
 
     void DoMaintenance(CConnman& connman);
 
@@ -307,13 +313,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     std::vector<CGovernanceVote> GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const;
     void GetAllNewerThan(std::vector<CGovernanceObject>& objs, int64_t nMoreThanTime) const;
 
-    void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) override;
+    void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override;
 
     void CheckAndRemove();
 
     UniValue ToJson() const;
 
-    void UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman);
+    void UpdatedBlockTip(const CBlockIndex* pindex);
     int64_t GetLastDiffTime() const { return nTimeLastDiff; }
     void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; }
 
@@ -338,9 +344,9 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
 
     bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed);
 
-    bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) override;
+    bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override;
 
-    void CheckPostponedObjects(PeerManager& peerman);
+    void CheckPostponedObjects();
 
     bool AreRateChecksEnabled() const
     {
@@ -407,7 +413,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
     /// Called to indicate a requested object or vote has been received
     bool AcceptMessage(const uint256& nHash);
 
-    void CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman);
+    void CheckOrphanVotes(CGovernanceObject& govobj);
 
     void RebuildIndexes();
 

src/governance/signing.cpp

@@ -18,12 +18,11 @@
 #include <algorithm>
 
 GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman,
-                                   PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
-                                   const ChainstateManager& chainman, const CMasternodeSync& mn_sync) :
+                                   const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman,
+                                   const CMasternodeSync& mn_sync) :
     m_connman{connman},
     m_dmnman{dmnman},
     m_govman{govman},
-    m_peerman{peerman},
     m_mn_activeman{mn_activeman},
     m_chainman{chainman},
     m_mn_sync{mn_sync}
@@ -155,7 +154,7 @@ std::optional<const CGovernanceObject> GovernanceSigner::CreateGovernanceTrigger
     }
 
     // The trigger we just created looks good, submit it
-    m_govman.AddGovernanceObject(gov_sb, m_peerman);
+    m_govman.AddGovernanceObject(gov_sb);
     return std::make_optional<CGovernanceObject>(gov_sb);
 }
 
@@ -250,7 +249,7 @@ bool GovernanceSigner::VoteFundingTrigger(const uint256& nHash, const vote_outco
     vote.SetSignature(m_mn_activeman.SignBasic(vote.GetSignatureHash()));
 
     CGovernanceException exception;
-    if (!m_govman.ProcessVoteAndRelay(vote, exception, m_connman, m_peerman)) {
+    if (!m_govman.ProcessVoteAndRelay(vote, exception, m_connman)) {
         LogPrint(BCLog::GOBJECT, "%s -- Vote FUNDING %d for trigger:%s failed:%s\n", __func__, outcome, nHash.ToString(), exception.what());
         return false;
     }

src/governance/signing.h

@@ -24,7 +24,6 @@ class CGovernanceVote;
 class ChainstateManager;
 class CMasternodeSync;
 class CNode;
-class PeerManager;
 enum vote_outcome_enum_t : int;
 
 class GovernanceSignerParent
@@ -35,13 +34,13 @@ class GovernanceSignerParent
     virtual bool IsValid() const = 0;
     virtual bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, std::shared_ptr<CSuperblock>& pSuperblockRet, int nBlockHeight) = 0;
     virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0;
-    virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) = 0;
+    virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) = 0;
     virtual int GetCachedBlockHeight() const = 0;
     virtual CGovernanceObject* FindGovernanceObject(const uint256& nHash) = 0;
     virtual CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0;
     virtual std::vector<std::shared_ptr<CSuperblock>> GetActiveTriggers() const = 0;
     virtual std::vector<std::shared_ptr<const CGovernanceObject>> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) = 0;
-    virtual void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) = 0;
+    virtual void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) = 0;
 };
 
 class GovernanceSigner
@@ -50,7 +49,6 @@ class GovernanceSigner
     CConnman& m_connman;
     CDeterministicMNManager& m_dmnman;
     GovernanceSignerParent& m_govman;
-    PeerManager& m_peerman;
     const CActiveMasternodeManager& m_mn_activeman;
     const ChainstateManager& m_chainman;
     const CMasternodeSync& m_mn_sync;
@@ -60,7 +58,7 @@ class GovernanceSigner
 
 public:
     explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman,
-                              PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
+                              const CActiveMasternodeManager& mn_activeman,
                               const ChainstateManager& chainman, const CMasternodeSync& mn_sync);
     ~GovernanceSigner();