refactor: break circular dependency of CMasternodeManager over net.h

Author: knst

src/init.cpp

@@ -1990,7 +1990,7 @@ 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.mn_sync = std::make_unique<CMasternodeSync>(*node.connman, *node.netfulfilledman);
+        node.mn_sync = std::make_unique<CMasternodeSync>(std::make_unique<NodeSyncNotifierImpl>(*node.connman, *node.netfulfilledman));
 
         node.govman = std::make_unique<CGovernanceManager>(*node.mn_metaman, *node.netfulfilledman, *node.chainman, node.dmnman, *node.mn_sync);
 

src/masternode/sync.cpp

@@ -5,18 +5,15 @@
 #include <masternode/sync.h>
 
 #include <chain.h>
-#include <net.h>
-#include <netfulfilledman.h>
-#include <node/interface_ui.h>
 #include <util/time.h>
 #include <util/translation.h>
 
-CMasternodeSync::CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman) :
+CMasternodeSync::CMasternodeSync(std::unique_ptr<NodeSyncNotifier>&& sync_notifier) :
     nTimeAssetSyncStarted{GetTime()},
     nTimeLastBumped{GetTime()},
-    connman{_connman},
-    m_netfulfilledman{netfulfilledman}
+    m_sync_notifier{std::move(sync_notifier)}
 {
+    assert(m_sync_notifier != nullptr);
 }
 
 CMasternodeSync::~CMasternodeSync() = default;
@@ -36,7 +33,7 @@ void CMasternodeSync::Reset(bool fForce, bool fNotifyReset)
     nTimeLastUpdateBlockTip = 0;
     fReachedBestHeader = false;
     if (fNotifyReset) {
-        uiInterface.NotifyAdditionalDataSyncProgressChanged(-1);
+        m_sync_notifier->SyncReset();
     }
 }
 
@@ -60,8 +57,6 @@ std::string CMasternodeSync::GetAssetName() const
 
 void CMasternodeSync::SwitchToNextAsset()
 {
-    assert(m_netfulfilledman.IsValid());
-
     switch(nCurrentAsset)
     {
         case(MASTERNODE_SYNC_BLOCKCHAIN):
@@ -72,11 +67,7 @@ void CMasternodeSync::SwitchToNextAsset()
         case(MASTERNODE_SYNC_GOVERNANCE):
             LogPrintf("CMasternodeSync::SwitchToNextAsset -- Completed %s in %llds\n", GetAssetName(), GetTime() - nTimeAssetSyncStarted);
             nCurrentAsset = MASTERNODE_SYNC_FINISHED;
-            uiInterface.NotifyAdditionalDataSyncProgressChanged(1);
-            // TODO: move connman to notifier
-            connman.ForEachNode(CConnman::AllNodes, [this](const CNode* pnode) {
-                m_netfulfilledman.AddFulfilledRequest(pnode->addr, "full-sync");
-            });
+            m_sync_notifier->SyncFinished();
             LogPrintf("CMasternodeSync::SwitchToNextAsset -- Sync has finished\n");
 
             break;

src/masternode/sync.h

@@ -5,12 +5,10 @@
 #define BITCOIN_MASTERNODE_SYNC_H
 
 #include <atomic>
+#include <memory>
 #include <string>
 
-class CConnman;
 class CBlockIndex;
-class CMasternodeSync;
-class CNetFulfilledRequestManager;
 
 /** Default for -syncmempool */
 static const bool DEFAULT_SYNC_MEMPOOL = true;
@@ -25,10 +23,16 @@ static constexpr int MASTERNODE_SYNC_TICK_SECONDS    = 6;
 static constexpr int MASTERNODE_SYNC_TIMEOUT_SECONDS = 30; // our blocks are 2.5 minutes so 30 seconds should be fine
 static constexpr int MASTERNODE_SYNC_RESET_SECONDS   = 900; // Reset fReachedBestHeader in CMasternodeSync::Reset if UpdateBlockTip hasn't been called for this seconds
 
+class NodeSyncNotifier
+{
+public:
+    virtual void SyncReset() = 0;
+    virtual void SyncFinished() = 0;
+};
+
 //
 // CMasternodeSync : Sync masternode assets in stages
 //
-
 class CMasternodeSync
 {
 private:
@@ -47,14 +51,13 @@ class CMasternodeSync
     /// Last time UpdateBlockTip has been called
     std::atomic<int64_t> nTimeLastUpdateBlockTip{0};
 
-    CConnman& connman;
-    CNetFulfilledRequestManager& m_netfulfilledman;
+    std::unique_ptr<NodeSyncNotifier> m_sync_notifier;
 
 public:
     CMasternodeSync() = delete;
     CMasternodeSync(const CMasternodeSync&) = delete;
     CMasternodeSync& operator=(const CMasternodeSync&) = delete;
-    explicit CMasternodeSync(CConnman& _connman, CNetFulfilledRequestManager& netfulfilledman);
+    explicit CMasternodeSync(std::unique_ptr<NodeSyncNotifier>&& sync_notifier);
     ~CMasternodeSync();
 
     bool IsBlockchainSynced() const { return nCurrentAsset > MASTERNODE_SYNC_BLOCKCHAIN; }

src/node/sync_manager.cpp

@@ -333,3 +333,18 @@ void SyncManager::ProcessMessage(CNode& peer, CConnman&, const std::string& msg_
 
     LogPrint(BCLog::MNSYNC, "SYNCSTATUSCOUNT -- got inventory count: nItemID=%d  nCount=%d  peer=%d\n", nItemID, nCount, peer.GetId());
 }
+
+void NodeSyncNotifierImpl::SyncReset()
+{
+    uiInterface.NotifyAdditionalDataSyncProgressChanged(-1);
+}
+
+void NodeSyncNotifierImpl::SyncFinished()
+{
+    assert(m_netfulfilledman.IsValid());
+
+    uiInterface.NotifyAdditionalDataSyncProgressChanged(1);
+    m_connman.ForEachNode(CConnman::AllNodes, [this](const CNode* pnode) {
+        m_netfulfilledman.AddFulfilledRequest(pnode->addr, "full-sync");
+    });
+}

src/node/sync_manager.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_NODE_SYNC_MANAGER_H
 #define BITCOIN_NODE_SYNC_MANAGER_H
 
+#include <masternode/sync.h>
 #include <net_processing.h>
 
 class CGovernanceManager;
@@ -35,4 +36,18 @@ class SyncManager final : public NetHandler
     CNetFulfilledRequestManager& m_netfulfilledman;
 };
 
+class NodeSyncNotifierImpl : public NodeSyncNotifier
+{
+public:
+    NodeSyncNotifierImpl(CConnman& connman, CNetFulfilledRequestManager& netfulfilledman) :
+        m_connman(connman),
+        m_netfulfilledman(netfulfilledman)
+    {}
+
+    void SyncReset() override;
+    void SyncFinished() override;
+private:
+    CConnman& m_connman;
+    CNetFulfilledRequestManager& m_netfulfilledman;
+};
 #endif // BITCOIN_NODE_SYNC_MANAGER_H

src/test/util/setup_common.cpp

@@ -23,6 +23,7 @@
 #include <node/blockstorage.h>
 #include <node/chainstate.h>
 #include <node/miner.h>
+#include <node/sync_manager.h>
 #include <policy/fees.h>
 #include <pow.h>
 #include <rpc/blockchain.h>
@@ -273,7 +274,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     m_node.chainman = std::make_unique<ChainstateManager>();
     m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(m_cache_sizes.block_tree_db, true);
 
-    m_node.mn_sync = std::make_unique<CMasternodeSync>(*m_node.connman, *m_node.netfulfilledman);
+    m_node.mn_sync = std::make_unique<CMasternodeSync>(std::make_unique<NodeSyncNotifierImpl>(*m_node.connman, *m_node.netfulfilledman));
     m_node.govman = std::make_unique<CGovernanceManager>(*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.

test/lint/lint-circular-dependencies.py

@@ -48,7 +48,6 @@
     "evo/specialtxman -> validation -> evo/specialtxman",
     "governance/classes -> governance/object -> governance/governance -> governance/classes",
     "governance/governance -> governance/signing -> governance/object -> governance/governance",
-    "masternode/sync -> net -> masternode/sync",
     "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> instantsend/instantsend",
     "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> llmq/context -> instantsend/instantsend",
     "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> instantsend/instantsend",