Do not send dash-specific requests to masternodes before we are fully connected

Open all masternode connections via CConnman::ThreadOpenMasternodeConnections only. Queue requests and process them after some timeout.

Author: UdjinM6

src/masternodeman.cpp

@@ -740,6 +740,48 @@ std::pair<CService, std::set<uint256> > CMasternodeMan::PopScheduledMnbRequestCo
     return std::make_pair(pairFront.first, setResult);
 }
 
+void CMasternodeMan::ProcessQueuedMnbRequests(CConnman& connman)
+{
+    std::pair<CService, std::set<uint256> > p = mnodeman.PopScheduledMnbRequestConnection();
+    if (!(p.first == CService() || p.second.empty())) {
+        if (connman.IsMasternodeOrDisconnectRequested(p.first)) return;
+        mnbQueue.push_back(std::make_pair(GetTime(), p));
+        connman.AddPendingMasternode(p.first);
+    }
+
+    if (!mnbQueue.empty()) {
+        // wait at least 5 seconds since we tried to open corresponding connection
+        if (GetTime() - mnbQueue.front().first < 5) {
+            return;
+        }
+
+        auto request = mnbQueue.front().second;
+        LogPrint("masternode", "%s -- processing mnb queue for addr=%s\n", __func__, request.first.ToString());
+
+        connman.ForNode(request.first, [&](CNode* pnode) {
+
+            // compile request vector
+            std::vector<CInv> vToFetch;
+            std::set<uint256>::iterator it = request.second.begin();
+            while(it != request.second.end()) {
+                if(*it != uint256()) {
+                    vToFetch.push_back(CInv(MSG_MASTERNODE_ANNOUNCE, *it));
+                    LogPrint("masternode", "-- asking for mnb %s from addr=%s\n", it->ToString(), pnode->addr.ToString());
+                }
+                ++it;
+            }
+
+            // ask for data
+            CNetMsgMaker msgMaker(pnode->GetSendVersion());
+            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETDATA, vToFetch));
+            return true;
+        });
+
+        // drop it from the request queue regardless
+        mnbQueue.pop_front();
+        LogPrint("masternode", "%s -- mnb queue size: %d\n", __func__, mnbQueue.size());
+    }
+}
 
 void CMasternodeMan::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman)
 {
@@ -1049,25 +1091,45 @@ bool CMasternodeMan::SendVerifyRequest(const CAddress& addr, const std::vector<C
         return false;
     }
 
-    connman.OpenMasternodeConnection(addr);
-    bool fSuccess = connman.ForNode(addr, CConnman::AllNodes, [&](CNode* pnode){
-        netfulfilledman.AddFulfilledRequest(addr, strprintf("%s", NetMsgType::MNVERIFY)+"-request");
-        // use random nonce, store it and require node to reply with correct one later
-        CMasternodeVerification mnv(addr, GetRandInt(999999), nCachedBlockHeight - 1);
-        mWeAskedForVerification[addr] = mnv;
-        LogPrintf("CMasternodeMan::SendVerifyRequest -- verifying node using nonce %d addr=%s\n", mnv.nonce, addr.ToString());
-        CNetMsgMaker msgMaker(pnode->GetSendVersion()); // TODO this gives a warning about version not being set (we should wait for VERSION exchange)
-        connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNVERIFY, mnv));
-        return true;
-    });
-    if (!fSuccess) {
-        LogPrintf("CMasternodeMan::SendVerifyRequest -- can't connect to node to verify it, addr=%s\n", addr.ToString());
-        return false;
-    }
+    if (connman.IsMasternodeOrDisconnectRequested(addr)) return false;
 
+    connman.AddPendingMasternode(addr);
+    // use random nonce, store it and require node to reply with correct one later
+    CMasternodeVerification mnv(addr, GetRandInt(999999), nCachedBlockHeight - 1);
+    LOCK(cs_mnvQueue);
+    mnvQueue.push_back(std::make_pair(GetTime(), std::make_pair(addr, mnv)));
+    LogPrintf("CMasternodeMan::SendVerifyRequest -- verifying node using nonce %d addr=%s\n", mnv.nonce, addr.ToString());
     return true;
 }
 
+void CMasternodeMan::ProcessQueuedMnvRequests(CConnman& connman)
+{
+    LOCK(cs_mnvQueue);
+
+    if (!mnvQueue.empty()) {
+        // wait at least 5 seconds since we tried to open corresponding connection
+        if (GetTime() - mnvQueue.front().first < 5) {
+            return;
+        }
+
+        auto request = mnvQueue.front().second;
+        LogPrint("masternode", "CMasternodeMan::%s -- processing mnv queue for addr=%s\n", __func__, request.first.ToString());
+
+        connman.ForNode(request.first, [&](CNode* pnode) {
+            netfulfilledman.AddFulfilledRequest(pnode->addr, strprintf("%s", NetMsgType::MNVERIFY)+"-request");
+            // use random nonce, store it and require node to reply with correct one later
+            mWeAskedForVerification[pnode->addr] = request.second;
+            LogPrint("masternode", "-- verifying node using nonce %d addr=%s\n", request.second.nonce, pnode->addr.ToString());
+            CNetMsgMaker msgMaker(pnode->GetSendVersion()); // TODO this gives a warning about version not being set (we should wait for VERSION exchange)
+            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNVERIFY, request.second));
+            return true;
+        });
+
+        mnvQueue.pop_front();
+        LogPrint("masternode", "CMasternodeMan::%s -- mnv queue size: %d\n", __func__, mnvQueue.size());
+    }
+}
+
 void CMasternodeMan::SendVerifyReply(CNode* pnode, CMasternodeVerification& mnv, CConnman& connman)
 {
     // only masternodes can sign this, why would someone ask regular node?

src/masternodeman.h

@@ -63,6 +63,9 @@ class CMasternodeMan
     std::map<uint256, std::pair< int64_t, std::set<CNetAddr> > > mMnbRecoveryRequests;
     std::map<uint256, std::vector<CMasternodeBroadcast> > mMnbRecoveryGoodReplies;
     std::list< std::pair<CService, uint256> > listScheduledMnbRequestConnections;
+    std::deque<std::pair<int64_t, std::pair<CService, std::set<uint256> > > > mnbQueue;
+    std::deque<std::pair<int64_t, std::pair<CService, CMasternodeVerification > > > mnvQueue;
+    CCriticalSection cs_mnvQueue;
 
     /// Set when masternodes are added, cleared when CGovernanceManager is notified
     bool fMasternodesAdded;
@@ -180,12 +183,14 @@ class CMasternodeMan
 
     void ProcessMasternodeConnections(CConnman& connman);
     std::pair<CService, std::set<uint256> > PopScheduledMnbRequestConnection();
+    void ProcessQueuedMnbRequests(CConnman& connman);
 
     void ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman);
 
     void DoFullVerificationStep(CConnman& connman);
     void CheckSameAddr();
     bool SendVerifyRequest(const CAddress& addr, const std::vector<CMasternode*>& vSortedByAddr, CConnman& connman);
+    void ProcessQueuedMnvRequests(CConnman& connman);
     void SendVerifyReply(CNode* pnode, CMasternodeVerification& mnv, CConnman& connman);
     void ProcessVerifyReply(CNode* pnode, CMasternodeVerification& mnv);
     void ProcessVerifyBroadcast(CNode* pnode, const CMasternodeVerification& mnv);

src/net.cpp

@@ -1900,7 +1900,7 @@ void CConnman::ThreadOpenAddedConnections()
     }
 }
 
-void CConnman::ThreadMnbRequestConnections()
+void CConnman::ThreadOpenMasternodeConnections()
 {
     // Connecting to specific addresses, no masternode connections available
     if (IsArgSet("-connect") && mapMultiArgs.at("-connect").size() > 0)
@@ -1915,34 +1915,22 @@ void CConnman::ThreadMnbRequestConnections()
         if (interruptNet)
             return;
 
-        std::pair<CService, std::set<uint256> > p = mnodeman.PopScheduledMnbRequestConnection();
-        if(p.first == CService() || p.second.empty()) continue;
-
-        // compile request vector
-        std::vector<CInv> vToFetch;
-        std::set<uint256>::iterator it = p.second.begin();
-        while(it != p.second.end()) {
-            if(*it != uint256()) {
-                vToFetch.push_back(CInv(MSG_MASTERNODE_ANNOUNCE, *it));
-                LogPrint("masternode", "ThreadMnbRequestConnections -- asking for mnb %s from addr=%s\n", it->ToString(), p.first.ToString());
+        LOCK(cs_vPendingMasternodes);
+        std::vector<CService>::iterator it = vPendingMasternodes.begin();
+        while (it != vPendingMasternodes.end()) {
+            if (!IsMasternodeOrDisconnectRequested(*it)) {
+                OpenMasternodeConnection(CAddress(*it, NODE_NETWORK));
+                // should be in the list now if connection was opened
+                ForNode(*it, CConnman::AllNodes, [&](CNode* pnode) {
+                    if (pnode->fDisconnect) {
+                        return false;
+                    }
+                    grant.MoveTo(pnode->grantMasternodeOutbound);
+                    return true;
+                });
             }
-            ++it;
+            it = vPendingMasternodes.erase(it);
         }
-
-        CAddress addr(p.first, NODE_NETWORK);
-        OpenMasternodeConnection(addr);
-
-        ForNode(addr, CConnman::AllNodes, [&](CNode* pnode) {
-            if (pnode->fDisconnect) return false;
-
-            grant.MoveTo(pnode->grantMasternodeOutbound);
-
-            // ask for data
-            CNetMsgMaker msgMaker(pnode->GetSendVersion());
-            PushMessage(pnode, msgMaker.Make(NetMsgType::GETDATA, vToFetch));
-
-            return true;
-        });
     }
 }
 
@@ -2333,7 +2321,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
         threadOpenConnections = std::thread(&TraceThread<std::function<void()> >, "opencon", std::function<void()>(std::bind(&CConnman::ThreadOpenConnections, this)));
 
     // Initiate masternode connections
-    threadMnbRequestConnections = std::thread(&TraceThread<std::function<void()> >, "mnbcon", std::function<void()>(std::bind(&CConnman::ThreadMnbRequestConnections, this)));
+    threadOpenMasternodeConnections = std::thread(&TraceThread<std::function<void()> >, "mncon", std::function<void()>(std::bind(&CConnman::ThreadOpenMasternodeConnections, this)));
 
     // Process messages
     threadMessageHandler = std::thread(&TraceThread<std::function<void()> >, "msghand", std::function<void()>(std::bind(&CConnman::ThreadMessageHandler, this)));
@@ -2387,8 +2375,8 @@ void CConnman::Stop()
 {
     if (threadMessageHandler.joinable())
         threadMessageHandler.join();
-    if (threadMnbRequestConnections.joinable())
-        threadMnbRequestConnections.join();
+    if (threadOpenMasternodeConnections.joinable())
+        threadOpenMasternodeConnections.join();
     if (threadOpenConnections.joinable())
         threadOpenConnections.join();
     if (threadOpenAddedConnections.joinable())
@@ -2508,6 +2496,18 @@ bool CConnman::RemoveAddedNode(const std::string& strNode)
     return false;
 }
 
+bool CConnman::AddPendingMasternode(const CService& service)
+{
+    LOCK(cs_vPendingMasternodes);
+    for(std::vector<CService>::const_iterator it = vPendingMasternodes.begin(); it != vPendingMasternodes.end(); ++it) {
+        if (service == *it)
+            return false;
+    }
+
+    vPendingMasternodes.push_back(service);
+    return true;
+}
+
 size_t CConnman::GetNodeCount(NumConnections flags)
 {
     LOCK(cs_vNodes);

src/net.h

@@ -348,6 +348,7 @@ class CConnman
 
     bool AddNode(const std::string& node);
     bool RemoveAddedNode(const std::string& node);
+    bool AddPendingMasternode(const CService& addr);
     std::vector<AddedNodeInfo> GetAddedNodeInfo();
 
     size_t GetNodeCount(NumConnections num);
@@ -409,7 +410,7 @@ class CConnman
     void AcceptConnection(const ListenSocket& hListenSocket);
     void ThreadSocketHandler();
     void ThreadDNSAddressSeed();
-    void ThreadMnbRequestConnections();
+    void ThreadOpenMasternodeConnections();
 
     uint64_t CalculateKeyedNetGroup(const CAddress& ad) const;
 
@@ -475,6 +476,8 @@ class CConnman
     CCriticalSection cs_vOneShots;
     std::vector<std::string> vAddedNodes;
     CCriticalSection cs_vAddedNodes;
+    std::vector<CService> vPendingMasternodes;
+    CCriticalSection cs_vPendingMasternodes;
     std::vector<CNode*> vNodes;
     std::list<CNode*> vNodesDisconnected;
     mutable CCriticalSection cs_vNodes;
@@ -512,7 +515,7 @@ class CConnman
     std::thread threadSocketHandler;
     std::thread threadOpenAddedConnections;
     std::thread threadOpenConnections;
-    std::thread threadMnbRequestConnections;
+    std::thread threadOpenMasternodeConnections;
     std::thread threadMessageHandler;
 };
 extern std::unique_ptr<CConnman> g_connman;