Rewrite fulfilled requests handling (#1040)

src/Makefile.am

@@ -122,6 +122,7 @@ BITCOIN_CORE_H = \
   miner.h \
   net.h \
   netbase.h \
+  netfulfilledman.h \
   noui.h \
   policy/fees.h \
   policy/policy.h \
@@ -205,6 +206,7 @@ libbitcoin_server_a_SOURCES = \
   merkleblock.cpp \
   miner.cpp \
   net.cpp \
+  netfulfilledman.cpp \
   noui.cpp \
   policy/fees.cpp \
   policy/policy.cpp \

src/chainparams.cpp

@@ -154,6 +154,7 @@ public:
         fTestnetToBeDeprecatedFieldRPC = false;
 
         nPoolMaxTransactions = 3;
+        nFulfilledRequestExpireTime = 60*60; // fulfilled requests expire in 1 hour
         strSporkPubKey = "04549ac134f694c0243f503e8c8a9a986f5de6610049c40b07816809b0d1d06a21b07be27b9bb555931773f62ba6cf35a25fd52f694d4e1106ccd237a7bb899fdd";
         strMasternodePaymentsPubKey = "04549ac134f694c0243f503e8c8a9a986f5de6610049c40b07816809b0d1d06a21b07be27b9bb555931773f62ba6cf35a25fd52f694d4e1106ccd237a7bb899fdd";
 
@@ -271,8 +272,10 @@ public:
         fTestnetToBeDeprecatedFieldRPC = true;
 
         nPoolMaxTransactions = 3;
+        nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes
         strSporkPubKey = "046f78dcf911fbd61910136f7f0f8d90578f68d0b3ac973b5040fb7afb501b5939f39b108b0569dca71488f5bbf498d92e4d1194f6f941307ffd95f75e76869f0e";
         strMasternodePaymentsPubKey = "046f78dcf911fbd61910136f7f0f8d90578f68d0b3ac973b5040fb7afb501b5939f39b108b0569dca71488f5bbf498d92e4d1194f6f941307ffd95f75e76869f0e";
+
         checkpointData = (CCheckpointData) {
             boost::assign::map_list_of
             (   261, uint256S("0x00000c26026d0815a7e2ce4fa270775f61403c040647ff2c3091f99e894a4618"))
@@ -354,6 +357,8 @@ public:
         fMineBlocksOnDemand = true;
         fTestnetToBeDeprecatedFieldRPC = false;
 
+        nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes
+
         checkpointData = (CCheckpointData){
             boost::assign::map_list_of
             ( 0, uint256S("0x000008ca1832a4baf228eb1553c03d3a2c8e02399550dd6ea8d65cec3ef23d2e")),

src/chainparams.h

@@ -78,6 +78,7 @@ public:
     const std::vector<SeedSpec6>& FixedSeeds() const { return vFixedSeeds; }
     const CCheckpointData& Checkpoints() const { return checkpointData; }
     int PoolMaxTransactions() const { return nPoolMaxTransactions; }
+    int FulfilledRequestExpireTime() const { return nFulfilledRequestExpireTime; }
     std::string SporkPubKey() const { return strSporkPubKey; }
     std::string MasternodePaymentPubKey() const { return strMasternodePaymentsPubKey; }
 protected:
@@ -102,6 +103,7 @@ protected:
     bool fTestnetToBeDeprecatedFieldRPC;
     CCheckpointData checkpointData;
     int nPoolMaxTransactions;
+    int nFulfilledRequestExpireTime;
     std::string strSporkPubKey;
     std::string strMasternodePaymentsPubKey;
 };

src/governance.cpp

@@ -15,6 +15,7 @@
 #include "darksend.h"
 #include "masternodeman.h"
 #include "masternode-sync.h"
+#include "netfulfilledman.h"
 #include "util.h"
 #include "addrman.h"
 #include <boost/lexical_cast.hpp>
@@ -110,18 +111,14 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C
         uint256 nProp;
         vRecv >> nProp;
 
-        // IF THE NETWORK IS MAIN, MAKE SURE THEY HAVEN'T ASKED US BEFORE
+        if(nProp == uint256()) {
-
+            if(netfulfilledman.HasFulfilledRequest(pfrom->addr, NetMsgType::MNGOVERNANCESYNC)) {
-        if(Params().NetworkIDString() == CBaseChainParams::MAIN){
+                // Asking for the whole list multiple times in a short period of time is no good
-            if(nProp == uint256()) {
+                LogPrint("gobject", "peer already asked me for the list\n");
-                if(pfrom->HasFulfilledRequest(NetMsgType::MNGOVERNANCESYNC)) {
+                Misbehaving(pfrom->GetId(), 20);
-                    LogPrint("gobject", "peer already asked me for the list\n");
+                return;
-                    // BAD PEER! BAD!
-                    Misbehaving(pfrom->GetId(), 20);
-                    return;
-                }
-                pfrom->FulfilledRequest(NetMsgType::MNGOVERNANCESYNC);
             }
+            netfulfilledman.AddFulfilledRequest(pfrom->addr, NetMsgType::MNGOVERNANCESYNC);
         }
 
         Sync(pfrom, nProp);

src/masternode-payments.cpp

@@ -9,6 +9,7 @@
 #include "masternode-payments.h"
 #include "masternode-sync.h"
 #include "masternodeman.h"
+#include "netfulfilledman.h"
 #include "spork.h"
 #include "util.h"
 
@@ -293,15 +294,14 @@ void CMasternodePayments::ProcessMessage(CNode* pfrom, std::string& strCommand,
         int nCountNeeded;
         vRecv >> nCountNeeded;
 
-        if(Params().NetworkIDString() == CBaseChainParams::MAIN){
+        if(netfulfilledman.HasFulfilledRequest(pfrom->addr, NetMsgType::MASTERNODEPAYMENTSYNC)) {
-            if(pfrom->HasFulfilledRequest(NetMsgType::MASTERNODEPAYMENTSYNC)) {
+            // Asking for the payments list multiple times in a short period of time is no good
-                LogPrintf("MASTERNODEPAYMENTSYNC -- peer already asked me for the list, peer=%d\n", pfrom->id);
+            LogPrintf("MASTERNODEPAYMENTSYNC -- peer already asked me for the list, peer=%d\n", pfrom->id);
-                Misbehaving(pfrom->GetId(), 20);
+            Misbehaving(pfrom->GetId(), 20);
-                return;
+            return;
-            }
         }
+        netfulfilledman.AddFulfilledRequest(pfrom->addr, NetMsgType::MASTERNODEPAYMENTSYNC);
 
-        pfrom->FulfilledRequest(NetMsgType::MASTERNODEPAYMENTSYNC);
         Sync(pfrom, nCountNeeded);
         LogPrintf("MASTERNODEPAYMENTSYNC -- Sent Masternode payment votes to peer %d\n", pfrom->id);
 

src/masternode-sync.cpp

@@ -9,6 +9,7 @@
 #include "masternode-payments.h"
 #include "masternode-sync.h"
 #include "masternodeman.h"
+#include "netfulfilledman.h"
 #include "spork.h"
 #include "util.h"
 
@@ -80,7 +81,7 @@ void CMasternodeSync::SwitchToNextAsset()
             throw std::runtime_error("Can't switch to next asset from failed, should use Reset() first!");
             break;
         case(MASTERNODE_SYNC_INITIAL):
-            ClearFulfilledRequest();
+            ClearFulfilledRequests();
             nRequestedMasternodeAssets = MASTERNODE_SYNC_SPORKS;
             break;
         case(MASTERNODE_SYNC_SPORKS):
@@ -101,6 +102,14 @@ void CMasternodeSync::SwitchToNextAsset()
             uiInterface.NotifyAdditionalDataSyncProgressChanged(1);
             //try to activate our masternode if possible
             activeMasternode.ManageState();
+
+            TRY_LOCK(cs_vNodes, lockRecv);
+            if(!lockRecv) return;
+
+            BOOST_FOREACH(CNode* pnode, vNodes) {
+                netfulfilledman.AddFulfilledRequest(pnode->addr, "full-sync");
+            }
+
             break;
     }
     nRequestedMasternodeAttempt = 0;
@@ -136,17 +145,18 @@ void CMasternodeSync::ProcessMessage(CNode* pfrom, std::string& strCommand, CDat
     }
 }
 
-void CMasternodeSync::ClearFulfilledRequest()
+void CMasternodeSync::ClearFulfilledRequests()
 {
     TRY_LOCK(cs_vNodes, lockRecv);
     if(!lockRecv) return;
 
     BOOST_FOREACH(CNode* pnode, vNodes)
     {
-        pnode->ClearFulfilledRequest("spork-sync");
+        netfulfilledman.RemoveFulfilledRequest(pnode->addr, "spork-sync");
-        pnode->ClearFulfilledRequest("masternode-payment-sync");
+        netfulfilledman.RemoveFulfilledRequest(pnode->addr, "masternode-list-sync");
-        pnode->ClearFulfilledRequest("governance-sync");
+        netfulfilledman.RemoveFulfilledRequest(pnode->addr, "masternode-payment-sync");
-        pnode->ClearFulfilledRequest("masternode-sync");
+        netfulfilledman.RemoveFulfilledRequest(pnode->addr, "governance-sync");
+        netfulfilledman.RemoveFulfilledRequest(pnode->addr, "full-sync");
     }
 }
 
@@ -221,12 +231,19 @@ void CMasternodeSync::ProcessTick()
 
         // NORMAL NETWORK MODE - TESTNET/MAINNET
         {
+            if(netfulfilledman.HasFulfilledRequest(pnode->addr, "full-sync")) {
+                // we already fully synced from this node recently,
+                // disconnect to free this connection slot for a new node
+                pnode->fDisconnect = true;
+                LogPrintf("CMasternodeSync::ProcessTick -- disconnecting from recently synced node %d\n", pnode->id);
+                continue;
+            }
+
             // SPORK : ALWAYS ASK FOR SPORKS AS WE SYNC (we skip this mode now)
 
-            if(!pnode->HasFulfilledRequest("spork-sync")) {
+            if(!netfulfilledman.HasFulfilledRequest(pnode->addr, "spork-sync")) {
                 // only request once from each peer
-                pnode->FulfilledRequest("spork-sync");
+                netfulfilledman.AddFulfilledRequest(pnode->addr, "spork-sync");
-
                 // get current network sporks
                 pnode->PushMessage(NetMsgType::GETSPORKS);
 
@@ -258,14 +275,14 @@ void CMasternodeSync::ProcessTick()
                    Surely doesn't work right for testnet currently */
                 // try to fetch data from at least two peers though
                 if(nRequestedMasternodeAttempt > 1 && nMnCount > mnodeman.GetEstimatedMasternodes(pCurrentBlockIndex->nHeight)*0.9) {
-                    LogPrintf("CMasternodeSync::Process -- nTick %d nRequestedMasternodeAssets %d -- found enough data\n", nTick, nRequestedMasternodeAssets);
+                    LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- found enough data\n", nTick, nRequestedMasternodeAssets);
                     SwitchToNextAsset();
                     return;
                 }
 
                 // only request once from each peer
-                if(pnode->HasFulfilledRequest("masternode-sync")) continue;
+                if(netfulfilledman.HasFulfilledRequest(pnode->addr, "masternode-list-sync")) continue;
-                pnode->FulfilledRequest("masternode-sync");
+                netfulfilledman.AddFulfilledRequest(pnode->addr, "masternode-list-sync");
 
                 if (pnode->nVersion < mnpayments.GetMinMasternodePaymentsProto()) continue;
                 nRequestedMasternodeAttempt++;
@@ -303,8 +320,8 @@ void CMasternodeSync::ProcessTick()
                 }
 
                 // only request once from each peer
-                if(pnode->HasFulfilledRequest("masternode-payment-sync")) continue;
+                if(netfulfilledman.HasFulfilledRequest(pnode->addr, "masternode-payment-sync")) continue;
-                pnode->FulfilledRequest("masternode-payment-sync");
+                netfulfilledman.AddFulfilledRequest(pnode->addr, "masternode-payment-sync");
 
                 if(pnode->nVersion < mnpayments.GetMinMasternodePaymentsProto()) continue;
                 nRequestedMasternodeAttempt++;
@@ -345,8 +362,8 @@ void CMasternodeSync::ProcessTick()
                 // }
 
                 // only request once from each peer
-                if(pnode->HasFulfilledRequest("governance-sync")) continue;
+                if(netfulfilledman.HasFulfilledRequest(pnode->addr, "governance-sync")) continue;
-                pnode->FulfilledRequest("governance-sync");
+                netfulfilledman.AddFulfilledRequest(pnode->addr, "governance-sync");
 
                 if (pnode->nVersion < MSG_GOVERNANCE_PEER_PROTO_VERSION) continue;
                 nRequestedMasternodeAttempt++;

src/masternode-sync.h

@@ -54,7 +54,7 @@ private:
     const CBlockIndex *pCurrentBlockIndex;
 
     void Fail();
-    void ClearFulfilledRequest();
+    void ClearFulfilledRequests();
 
 public:
     CMasternodeSync() { Reset(); }

src/net.h

@@ -371,8 +371,6 @@ protected:
     static CCriticalSection cs_setBanned;
     static bool setBannedIsDirty;
 
-    std::vector<std::string> vecRequestsFulfilled; //keep track of what client has asked for
-    
     // Whitelisted ranges. Any node connecting from these is automatically
     // whitelisted (as well as those connecting to whitelisted binds).
     static std::vector<CSubNet> vWhitelistedRange;
@@ -744,33 +742,6 @@ public:
         }
     }
 
-    bool HasFulfilledRequest(std::string strRequest)
-    {
-        BOOST_FOREACH(std::string& type, vecRequestsFulfilled)
-        {
-            if(type == strRequest) return true;
-        }
-        return false;
-    }
-
-    void ClearFulfilledRequest(std::string strRequest)
-    {
-        std::vector<std::string>::iterator it = vecRequestsFulfilled.begin();
-        while(it != vecRequestsFulfilled.end()){
-            if((*it) == strRequest) {
-                vecRequestsFulfilled.erase(it);
-                return;
-            }
-            ++it;
-        }
-    }
-
-    void FulfilledRequest(std::string strRequest)
-    {
-        if(HasFulfilledRequest(strRequest)) return;
-        vecRequestsFulfilled.push_back(strRequest);
-    }
-
     void CloseSocketDisconnect();
 
     // Denial-of-service detection/prevention

src/netfulfilledman.cpp

@@ -0,0 +1,72 @@
+// Copyright (c) 2014-2016 The Dash Core developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include "chainparams.h"
+#include "netfulfilledman.h"
+#include "util.h"
+
+CNetFulfilledRequestManager netfulfilledman;
+
+void CNetFulfilledRequestManager::AddFulfilledRequest(CAddress addr, std::string strRequest)
+{
+    LOCK(cs_mapFulfilledRequests);
+    mapFulfilledRequests[addr][strRequest] = GetTime() + Params().FulfilledRequestExpireTime();
+}
+
+bool CNetFulfilledRequestManager::HasFulfilledRequest(CAddress addr, std::string strRequest)
+{
+    LOCK(cs_mapFulfilledRequests);
+    fulfilledreqmap_t::iterator it = mapFulfilledRequests.find(addr);
+
+    return  it != mapFulfilledRequests.end() &&
+            it->second.find(strRequest) != it->second.end() &&
+            it->second[strRequest] > GetTime();
+}
+
+void CNetFulfilledRequestManager::RemoveFulfilledRequest(CAddress addr, std::string strRequest)
+{
+    LOCK(cs_mapFulfilledRequests);
+    fulfilledreqmap_t::iterator it = mapFulfilledRequests.find(addr);
+
+    if (it != mapFulfilledRequests.end()) {
+        it->second.erase(strRequest);
+    }
+}
+
+void CNetFulfilledRequestManager::CheckAndRemove()
+{
+    LOCK(cs_mapFulfilledRequests);
+
+    int64_t now = GetTime();
+    fulfilledreqmap_t::iterator it = mapFulfilledRequests.begin();
+
+    while(it != mapFulfilledRequests.end()) {
+        fulfilledreqmapentry_t::iterator it_entry = it->second.begin();
+        while(it_entry != it->second.end()) {
+            if(now > it_entry->second) {
+                it->second.erase(it_entry++);
+            } else {
+                ++it_entry;
+            }
+        }
+        if(it->second.size() == 0) {
+            mapFulfilledRequests.erase(it++);
+        } else {
+            ++it;
+        }
+    }
+}
+
+void CNetFulfilledRequestManager::Clear()
+{
+    LOCK(cs_mapFulfilledRequests);
+    mapFulfilledRequests.clear();
+}
+
+std::string CNetFulfilledRequestManager::ToString() const
+{
+    std::ostringstream info;
+    info << "Nodes with fulfilled requests: " << (int)mapFulfilledRequests.size();
+    return info.str();
+}

src/netfulfilledman.h

@@ -0,0 +1,49 @@
+// Copyright (c) 2014-2016 The Dash Core developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef NETFULFILLEDMAN_H
+#define NETFULFILLEDMAN_H
+
+#include "netbase.h"
+#include "protocol.h"
+#include "serialize.h"
+#include "sync.h"
+
+class CNetFulfilledRequestManager;
+extern CNetFulfilledRequestManager netfulfilledman;
+
+// Fulfilled requests are used to prevent nodes from asking for the same data on sync
+// and from being banned for doing so too often.
+class CNetFulfilledRequestManager
+{
+private:
+    typedef std::map<std::string, int64_t> fulfilledreqmapentry_t;
+    typedef std::map<CNetAddr, fulfilledreqmapentry_t> fulfilledreqmap_t;
+
+    //keep track of what node has/was asked for and when
+    fulfilledreqmap_t mapFulfilledRequests;
+    CCriticalSection cs_mapFulfilledRequests;
+
+public:
+    CNetFulfilledRequestManager() {}
+
+    ADD_SERIALIZE_METHODS;
+
+    template <typename Stream, typename Operation>
+    inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
+        LOCK(cs_mapFulfilledRequests);
+        READWRITE(mapFulfilledRequests);
+    }
+
+    void AddFulfilledRequest(CAddress addr, std::string strRequest); // expire after 1 hour by default
+    bool HasFulfilledRequest(CAddress addr, std::string strRequest);
+    void RemoveFulfilledRequest(CAddress addr, std::string strRequest);
+
+    void CheckAndRemove();
+    void Clear();
+
+    std::string ToString() const;
+};
+
+#endif