Merge #2686: [TierTwo] Introduce network requests manager

be1c0fc test: coverage for netfulfilledman requests existence and expiration. (furszy)
42f76c9 Connect net requests manager load, dump and maintenance job. (furszy)
ad60ad7 Create fulfilled requests manager. (furszy)

Pull request description:

  Built on top of #2684

  An object in charge of cache and store tier two network requests information, preventing possible bans for asking the same data too often.

ACKs for top commit:
  random-zebra:
    ACK be1c0fc
  Fuzzbawls:
    ACK be1c0fc

Tree-SHA512: 9abb5f01372cf85af7d7e21c581394008d8e1cc941a3bf2b388c34b7fd2cff391f953fdf6ace8afed717c0ed3879e04bb102d6c1b9db884d895445639229c12b

CMakeLists.txt

@@ -454,6 +454,7 @@ set(COMMON_SOURCES
         ./src/tiertwo/masternode_meta_manager.cpp
         ./src/evo/mnauth.cpp
         ./src/tiertwo/net_masternodes.cpp
+        ./src/tiertwo/netfulfilledman.cpp
         ./src/tiertwo/tiertwo_sync_state.cpp
         ./src/warnings.cpp
         )

src/Makefile.am

@@ -291,6 +291,7 @@ BITCOIN_CORE_H = \
   threadinterrupt.h \
   timedata.h \
   tinyformat.h \
+  tiertwo/netfulfilledman.h \
   tiertwo/tiertwo_sync_state.h \
   torcontrol.h \
   txdb.h \
@@ -556,6 +557,7 @@ libbitcoin_common_a_SOURCES = \
   script/sign.cpp \
   script/standard.cpp \
   tiertwo_networksync.cpp \
+  tiertwo/netfulfilledman.cpp \
   tiertwo/tiertwo_sync_state.cpp \
   warnings.cpp \
   script/script_error.cpp \

src/Makefile.test.include

@@ -127,6 +127,7 @@ BITCOIN_TESTS =\
   test/miner_tests.cpp \
   test/net_tests.cpp \
   test/netbase_tests.cpp \
+  test/netfulfilledman_tests.cpp \
   test/pmt_tests.cpp \
   test/policyestimator_tests.cpp \
   test/prevector_tests.cpp \

src/chainparams.cpp

@@ -352,6 +352,9 @@ class CMainParams : public CChainParams
         consensus.llmqs[Consensus::LLMQ_400_85] = llmq400_85;
 
         nLLMQConnectionRetryTimeout = 60;
+
+        // Tier two
+        nFulfilledRequestExpireTime = 60 * 60; // fulfilled requests expire in 1 hour
     }
 
     const CCheckpointData& Checkpoints() const
@@ -490,6 +493,9 @@ class CTestNetParams : public CChainParams
         consensus.llmqs[Consensus::LLMQ_400_85] = llmq400_85;
 
         nLLMQConnectionRetryTimeout = 60;
+
+        // Tier two
+        nFulfilledRequestExpireTime = 60 * 60; // fulfilled requests expire in 1 hour
     }
 
     const CCheckpointData& Checkpoints() const
@@ -625,6 +631,9 @@ class CRegTestParams : public CChainParams
         // long living quorum params
         consensus.llmqs[Consensus::LLMQ_TEST] = llmq_test;
         nLLMQConnectionRetryTimeout = 5;
+
+        // Tier two
+        nFulfilledRequestExpireTime = 60 * 60; // fulfilled requests expire in 1 hour
     }
 
     const CCheckpointData& Checkpoints() const

src/chainparams.h

@@ -97,6 +97,9 @@ class CChainParams
     bool IsRegTestNet() const { return NetworkIDString() == CBaseChainParams::REGTEST; }
     bool IsTestnet() const { return NetworkIDString() == CBaseChainParams::TESTNET; }
 
+    /** Tier two requests blockage mark expiration time */
+    int FulfilledRequestExpireTime() const { return nFulfilledRequestExpireTime; }
+
     void UpdateNetworkUpgradeParameters(Consensus::UpgradeIndex idx, int nActivationHeight);
 protected:
     CChainParams() {}
@@ -114,6 +117,7 @@ class CChainParams
 
     // Tier two
     int nLLMQConnectionRetryTimeout;
+    int nFulfilledRequestExpireTime;
 };
 
 /**

src/init.cpp

@@ -1715,7 +1715,16 @@ bool AppInitMain()
 
     // ********************************************************* Step 10: setup layer 2 data
 
-    LoadTierTwo(chain_active_height, fReindexChainState);
+    bool load_cache_files = !(fReindex || fReindexChainState);
+    {
+        LOCK(cs_main);
+        // was blocks/chainstate deleted?
+        if (chainActive.Tip() == nullptr) {
+            load_cache_files = false;
+        }
+    }
+
+    LoadTierTwo(chain_active_height, load_cache_files);
     if (!InitActiveMN()) return false;
     RegisterTierTwoValidationInterface();
 

src/masternode-payments.cpp

@@ -11,6 +11,7 @@
 #include "budget/budgetmanager.h"
 #include "masternodeman.h"
 #include "netmessagemaker.h"
+#include "tiertwo/netfulfilledman.h"
 #include "spork.h"
 #include "sync.h"
 #include "tiertwo/tiertwo_sync_state.h"
@@ -411,13 +412,13 @@ bool CMasternodePayments::ProcessMessageMasternodePayments(CNode* pfrom, std::st
         vRecv >> nCountNeeded;
 
         if (Params().NetworkIDString() == CBaseChainParams::MAIN) {
-            if (pfrom->HasFulfilledRequest(NetMsgType::GETMNWINNERS)) {
+            if (g_netfulfilledman.HasFulfilledRequest(pfrom->addr, NetMsgType::GETMNWINNERS)) {
                 LogPrint(BCLog::MASTERNODE, "%s: mnget - peer already asked me for the list\n", __func__);
                 return state.DoS(20, false, REJECT_INVALID, "getmnwinners-request-already-fulfilled");
             }
         }
 
-        pfrom->FulfilledRequest(NetMsgType::GETMNWINNERS);
+        g_netfulfilledman.AddFulfilledRequest(pfrom->addr, NetMsgType::GETMNWINNERS);
         Sync(pfrom, nCountNeeded);
         LogPrint(BCLog::MASTERNODE, "mnget - Sent Masternode winners to peer %i\n", pfrom->GetId());
     } else if (strCommand == NetMsgType::MNWINNER) {

src/masternode-sync.cpp

@@ -12,6 +12,7 @@
 #include "masternode.h"
 #include "masternodeman.h"
 #include "netmessagemaker.h"
+#include "tiertwo/netfulfilledman.h"
 #include "spork.h"
 #include "tiertwo/tiertwo_sync_state.h"
 #include "util/system.h"
@@ -177,12 +178,7 @@ void CMasternodeSync::ProcessSyncStatusMsg(int nItemID, int nCount)
 
 void CMasternodeSync::ClearFulfilledRequest()
 {
-    g_connman->ForEachNode([](CNode* pnode) {
-        pnode->ClearFulfilledRequest("getspork");
-        pnode->ClearFulfilledRequest("mnsync");
-        pnode->ClearFulfilledRequest("mnwsync");
-        pnode->ClearFulfilledRequest("busync");
-    });
+    g_netfulfilledman.Clear();
 }
 
 void CMasternodeSync::Process()
@@ -276,8 +272,8 @@ bool CMasternodeSync::SyncWithNode(CNode* pnode, bool fLegacyMnObsolete)
         }
 
         // Request sporks sync if we haven't requested it yet.
-        if (pnode->HasFulfilledRequest("getspork")) return true;
-        pnode->FulfilledRequest("getspork");
+        if (g_netfulfilledman.HasFulfilledRequest(pnode->addr, "getspork")) return true;
+        g_netfulfilledman.AddFulfilledRequest(pnode->addr, "getspork");
 
         g_connman->PushMessage(pnode, msgMaker.Make(NetMsgType::GETSPORKS));
         RequestedMasternodeAttempt++;
@@ -318,15 +314,15 @@ bool CMasternodeSync::SyncWithNode(CNode* pnode, bool fLegacyMnObsolete)
         if (RequestedMasternodeAttempt >= MASTERNODE_SYNC_THRESHOLD * 4) return false;
 
         // Request mnb sync if we haven't requested it yet.
-        if (pnode->HasFulfilledRequest("mnsync")) return true;
+        if (g_netfulfilledman.HasFulfilledRequest(pnode->addr, "mnsync")) return true;
 
         // Try to request MN list sync.
         if (!mnodeman.RequestMnList(pnode)) {
             return true; // Failed, try next peer.
         }
 
         // Mark sync requested.
-        pnode->FulfilledRequest("mnsync");
+        g_netfulfilledman.AddFulfilledRequest(pnode->addr, "mnsync");
         // Increase the sync attempt count
         RequestedMasternodeAttempt++;
 
@@ -369,10 +365,10 @@ bool CMasternodeSync::SyncWithNode(CNode* pnode, bool fLegacyMnObsolete)
         if (RequestedMasternodeAttempt >= MASTERNODE_SYNC_THRESHOLD * 2) return false;
 
         // Request mnw sync if we haven't requested it yet.
-        if (pnode->HasFulfilledRequest("mnwsync")) return true;
+        if (g_netfulfilledman.HasFulfilledRequest(pnode->addr, "mnwsync")) return true;
 
         // Mark sync requested.
-        pnode->FulfilledRequest("mnwsync");
+        g_netfulfilledman.AddFulfilledRequest(pnode->addr, "mnwsync");
 
         // Sync mn winners
         int nMnCount = mnodeman.CountEnabled(true /* only_legacy */);
@@ -408,10 +404,10 @@ bool CMasternodeSync::SyncWithNode(CNode* pnode, bool fLegacyMnObsolete)
         if (RequestedMasternodeAttempt >= MASTERNODE_SYNC_THRESHOLD * 3) return false;
 
         // Request bud sync if we haven't requested it yet.
-        if (pnode->HasFulfilledRequest("busync")) return true;
+        if (g_netfulfilledman.HasFulfilledRequest(pnode->addr, "busync")) return true;
 
         // Mark sync requested.
-        pnode->FulfilledRequest("busync");
+        g_netfulfilledman.AddFulfilledRequest(pnode->addr, "busync");
 
         // Sync proposals, finalizations and votes
         uint256 n;

src/net.h

@@ -710,8 +710,6 @@ class CNode
     mapMsgCmdSize mapSendBytesPerMsgCmd;
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
 
-    std::vector<std::string> vecRequestsFulfilled; //keep track of what client has asked for
-
 public:
     uint256 hashContinue;
     std::atomic<int> nStartingHeight;
@@ -892,32 +890,6 @@ class CNode
     // inv response received, clear it from the waiting inv set.
     void AskForInvReceived(const uint256& invHash);
 
-    bool HasFulfilledRequest(std::string strRequest)
-    {
-        for (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();
     bool DisconnectOldProtocol(int nVersionIn, int nVersionRequired);
 

src/test/CMakeLists.txt

@@ -144,6 +144,7 @@ set(BITCOIN_TESTS
         ${CMAKE_CURRENT_SOURCE_DIR}/multisig_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/net_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/netbase_tests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/netfulfilledman_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/pmt_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/policyestimator_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/prevector_tests.cpp

src/test/netfulfilledman_tests.cpp

@@ -0,0 +1,41 @@
+// Copyright (c) 2022 The PIVX developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#include "netbase.h"
+#include "test/test_pivx.h"
+#include "tiertwo/netfulfilledman.h"
+#include "utiltime.h"
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(netfulfilledman_tests, RegTestingSetup)
+
+BOOST_AUTO_TEST_CASE(netfulfilledman_simple_add_and_expire)
+{
+    int64_t now = GetTime();
+    SetMockTime(now);
+
+    CNetFulfilledRequestManager fulfilledMan;
+    CService service = LookupNumeric("1.1.1.1", 9999);
+    std::string request = "request";
+    BOOST_ASSERT(!fulfilledMan.HasFulfilledRequest(service, request));
+
+    // Add request
+    fulfilledMan.AddFulfilledRequest(service, request);
+    // Verify that the request is there
+    BOOST_ASSERT(fulfilledMan.HasFulfilledRequest(service, request));
+
+    // Advance mock time to surpass FulfilledRequestExpireTime
+    SetMockTime(now + 60 * 60 + 1);
+
+    // Verify that the request exists and expired now
+    BOOST_CHECK(fulfilledMan.Size() == 1);
+    BOOST_CHECK(!fulfilledMan.HasFulfilledRequest(service, request));
+
+    // Verify request removal
+    fulfilledMan.CheckAndRemove();
+    BOOST_CHECK(fulfilledMan.Size() == 0);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/tiertwo/init.cpp

@@ -14,7 +14,9 @@
 #include "masternode-payments.h"
 #include "masternodeconfig.h"
 #include "llmq/quorums_init.h"
+#include "scheduler.h"
 #include "tiertwo/masternode_meta_manager.h"
+#include "tiertwo/netfulfilledman.h"
 #include "validation.h"
 #include "wallet/wallet.h"
 
@@ -95,7 +97,7 @@ static void LoadBlockHashesCache(CMasternodeMan& man)
     }
 }
 
-bool LoadTierTwo(int chain_active_height, bool fReindexChainState)
+bool LoadTierTwo(int chain_active_height, bool load_cache_files)
 {
     // ################################# //
     // ## Legacy Masternodes Manager ### //
@@ -156,11 +158,9 @@ bool LoadTierTwo(int chain_active_height, bool fReindexChainState)
     // ############################## //
     // ## Net MNs Metadata Manager ## //
     // ############################## //
-    bool fLoadCacheFiles = !(fReindex || fReindexChainState);
-    fs::path pathDB = GetDataDir();
     uiInterface.InitMessage(_("Loading masternode cache..."));
     CFlatDB<CMasternodeMetaMan> metadb(MN_META_CACHE_FILENAME, MN_META_CACHE_FILE_ID);
-    if (fLoadCacheFiles) {
+    if (load_cache_files) {
         if (!metadb.Load(g_mmetaman)) {
             return UIError(strprintf(_("Failed to load masternode metadata cache from: %s"), metadb.GetDbPath().string()));
         }
@@ -171,6 +171,22 @@ bool LoadTierTwo(int chain_active_height, bool fReindexChainState)
         }
     }
 
+    // ############################## //
+    // ## Network Requests Manager ## //
+    // ############################## //
+    uiInterface.InitMessage(_("Loading network requests cache..."));
+    CFlatDB<CNetFulfilledRequestManager> netRequestsDb(NET_REQUESTS_CACHE_FILENAME, NET_REQUESTS_CACHE_FILE_ID);
+    if (load_cache_files) {
+        if (!netRequestsDb.Load(g_netfulfilledman)) {
+            LogPrintf("Failed to load network requests cache from %s", netRequestsDb.GetDbPath().string());
+        }
+    } else {
+        CNetFulfilledRequestManager netfulfilledmanTmp;
+        if (!netRequestsDb.Dump(netfulfilledmanTmp)) {
+            LogPrintf("Failed to clear network requests cache at %s", netRequestsDb.GetDbPath().string());
+        }
+    }
+
     return true;
 }
 
@@ -186,7 +202,8 @@ void DumpTierTwo()
     DumpMasternodes();
     DumpBudgets(g_budgetman);
     DumpMasternodePayments();
-    CFlatDB<CMasternodeMetaMan>("mnmetacache.dat", "magicMasternodeMetaCache").Dump(g_mmetaman);
+    CFlatDB<CMasternodeMetaMan>(MN_META_CACHE_FILENAME, MN_META_CACHE_FILE_ID).Dump(g_mmetaman);
+    CFlatDB<CNetFulfilledRequestManager>(NET_REQUESTS_CACHE_FILENAME, NET_REQUESTS_CACHE_FILE_ID).Dump(g_netfulfilledman);
 }
 
 void SetBudgetFinMode(const std::string& mode)
@@ -278,6 +295,7 @@ bool InitActiveMN()
 void StartTierTwoThreadsAndScheduleJobs(boost::thread_group& threadGroup, CScheduler& scheduler)
 {
     threadGroup.create_thread(std::bind(&ThreadCheckMasternodes));
+    scheduler.scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(g_netfulfilledman)), 60 * 1000);
 
     // Start LLMQ system
     if (gArgs.GetBoolArg("-disabledkg", false)) {

src/tiertwo/init.h

@@ -6,6 +6,7 @@
 #define PIVX_TIERTWO_INIT_H
 
 #include <string>
+#include "fs.h"
 
 static const bool DEFAULT_MASTERNODE  = false;
 static const bool DEFAULT_MNCONFLOCK = true;
@@ -33,7 +34,7 @@ void InitTierTwoPostCoinsCacheLoad();
 void InitTierTwoChainTip();
 
 /** Loads from disk all the tier two related objects */
-bool LoadTierTwo(int chain_active_height, bool fReindexChainState);
+bool LoadTierTwo(int chain_active_height, bool load_cache_files);
 
 /** Register all tier two objects */
 void RegisterTierTwoValidationInterface();