[Net] Mockable PoissonNextSend helper and tests mempool sync fixes

src/net.cpp

@@ -371,6 +371,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char* pszDest, bool fCo
         uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
         CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, pszDest ? pszDest : "", false);
         pnode->nServicesExpected = ServiceFlags(addrConnect.nServices & nRelevantServices);
+        pnode->fWhitelisted = IsWhitelistedRange(addrConnect);
         pnode->AddRef();
 
         // We're making a new connection, harvest entropy from the time (and our peer count)
@@ -2332,7 +2333,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     fGetAddr = false;
     nNextLocalAddrSend = 0;
     nNextAddrSend = 0;
-    nNextInvSend = 0;
     fRelayTxes = false;
     pfilter = new CBloomFilter();
     timeLastMempoolReq = 0;

src/net.h

@@ -401,19 +401,6 @@ struct CombinerAll {
     }
 };
 
-/**
- * Interface for message handling
- */
-class NetEventsInterface
-{
-public:
-    virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
-    virtual bool SendMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
-    virtual void InitializeNode(CNode* pnode) = 0;
-    virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0;
-};
-
-
 enum {
     LOCAL_NONE,   // unknown
     LOCAL_IF,     // address a local interface listens on
@@ -623,7 +610,7 @@ class CNode
     std::multimap<int64_t, CInv> mapAskFor;
     std::set<uint256> setAskFor;
     std::vector<uint256> vBlockRequested;
-    int64_t nNextInvSend;
+    std::chrono::microseconds nNextInvSend{0};
     // Used for BIP35 mempool sending, also protected by cs_inventory
     bool fSendMempool;
 
@@ -810,9 +797,25 @@ class CExplicitNetCleanup
     static void callCleanup();
 };
 
-
+/**
+ * Interface for message handling
+ */
+class NetEventsInterface
+{
+public:
+    virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
+    virtual bool SendMessages(CNode* pnode, std::atomic<bool>& interrupt) EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_sendProcessing) = 0;
+    virtual void InitializeNode(CNode* pnode) = 0;
+    virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0;
+};
 
 /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */
 int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds);
 
+/** Wrapper to return mockable type */
+inline std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval)
+{
+    return std::chrono::microseconds{PoissonNextSend(now.count(), average_interval.count())};
+}
+
 #endif // BITCOIN_NET_H

src/net_processing.cpp

@@ -1286,7 +1286,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         // Don't want addr from older versions unless seeding
         if (pfrom->nVersion < CADDR_TIME_VERSION && connman->GetAddressCount() > 1000)
             return true;
-        if (vAddr.size() > 1000) {
+        if (vAddr.size() > MAX_ADDR_TO_SEND) {
             LOCK(cs_main);
             Misbehaving(pfrom->GetId(), 20);
             return error("message addr size() = %u", vAddr.size());
@@ -2132,6 +2132,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM
 
         // Address refresh broadcast
         int64_t nNow = GetTimeMicros();
+        auto current_time = GetTime<std::chrono::microseconds>();
+
         if (!IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
             AdvertiseLocal(pto);
             pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
@@ -2214,10 +2216,10 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM
 
             // Check whether periodic send should happen
             bool fSendTrickle = pto->fWhitelisted;
-            if (pto->nNextInvSend < nNow) {
+            if (pto->nNextInvSend < current_time) {
                 fSendTrickle = true;
                 // Use half the delay for outbound peers, as there is less privacy concern for them.
-                pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound);
+                pto->nNextInvSend = PoissonNextSend(current_time, std::chrono::seconds{INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound});
             }
 
             // Time to send but the peer has requested we not relay transactions.
@@ -2301,6 +2303,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM
             connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
 
         // Detect whether we're stalling
+        current_time = GetTime<std::chrono::microseconds>();
         nNow = GetTimeMicros();
         if (state.nStallingSince && state.nStallingSince < nNow - 1000000 * BLOCK_STALLING_TIMEOUT) {
             // Stalling only triggers when the block download window cannot move. During normal steady state,

src/net_processing.h

@@ -56,7 +56,7 @@ class PeerLogicValidation : public CValidationInterface, public NetEventsInterfa
     * @param[in]   interrupt       Interrupt condition for processing threads
     * @return                      True if there is more work to be done
     */
-    bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override;
+    bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing);
 };
 
 struct CNodeStateStats {

src/test/DoS_tests.cpp

@@ -64,7 +64,10 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
     misbehave(dummyNode1.GetId(), 100); // Should get banned
-    peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    }
     BOOST_CHECK(connman->IsBanned(addr1));
     BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
 
@@ -75,11 +78,17 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     dummyNode2.nVersion = 1;
     dummyNode2.fSuccessfullyConnected = true;
     misbehave(dummyNode2.GetId(), 50);
-    peerLogic->SendMessages(&dummyNode2, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode2.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode2, interruptDummy);
+    }
     BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet...
     BOOST_CHECK(connman->IsBanned(addr1));  // ... but 1 still should be
     misbehave(dummyNode2.GetId(), 50);
-    peerLogic->SendMessages(&dummyNode2, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode2.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode2, interruptDummy);
+    }
     BOOST_CHECK(connman->IsBanned(addr2));
 }
 
@@ -96,13 +105,22 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
     misbehave(dummyNode1.GetId(), 100);
-    peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    }
     BOOST_CHECK(!connman->IsBanned(addr1));
     misbehave(dummyNode1.GetId(), 10);
-    peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    }
     BOOST_CHECK(!connman->IsBanned(addr1));
     misbehave(dummyNode1.GetId(), 1);
-    peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode1, interruptDummy);
+    }
     BOOST_CHECK(connman->IsBanned(addr1));
     gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
 }
@@ -123,7 +141,10 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     dummyNode.fSuccessfullyConnected = true;
 
     misbehave(dummyNode.GetId(), 100);
-    peerLogic->SendMessages(&dummyNode, interruptDummy);
+    {
+        LOCK2(cs_main, dummyNode.cs_sendProcessing);
+        peerLogic->SendMessages(&dummyNode, interruptDummy);
+    }
     BOOST_CHECK(connman->IsBanned(addr));
 
     SetMockTime(nStartTime+60*60);

src/test/net_tests.cpp

@@ -215,4 +215,19 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
     BOOST_CHECK(1);
 }
 
+BOOST_AUTO_TEST_CASE(PoissonNextSend)
+{
+    g_mock_deterministic_tests = true;
+
+    int64_t now = 5000;
+    int average_interval_seconds = 600;
+
+    auto poisson = ::PoissonNextSend(now, average_interval_seconds);
+    std::chrono::microseconds poisson_chrono = ::PoissonNextSend(std::chrono::microseconds{now}, std::chrono::seconds{average_interval_seconds});
+
+    BOOST_CHECK_EQUAL(poisson, poisson_chrono.count());
+
+    g_mock_deterministic_tests = false;
+}
+
 BOOST_AUTO_TEST_SUITE_END()

test/functional/mining_pos_coldStaking.py

@@ -29,7 +29,8 @@ class PIVX_ColdStakingTest(PivxTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 3
-        self.extra_args = [['-nuparams=v5_shield:201']] * self.num_nodes
+        # whitelist all peers to speed up tx relay / mempool sync
+        self.extra_args = [['-nuparams=v5_shield:201', "-whitelist=127.0.0.1"]] * self.num_nodes
         self.extra_args[0].append('-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi')
         self.enable_mocktime()
 

test/functional/mining_pos_fakestake.py

@@ -44,6 +44,7 @@
 """
 
 from io import BytesIO
+import time
 from time import sleep
 
 from test_framework.authproxy import JSONRPCException
@@ -59,13 +60,10 @@
 class FakeStakeTest(PivxTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-        # nodes[0] moves the chain and checks the spam blocks, nodes[1] sends them
-
-    def setup_chain(self):
-        # Start with PoW cache: 200 blocks
-        self.log.info("Initializing test directory " + self.options.tmpdir)
-        self._initialize_chain()
+        # whitelist all peers to speed up tx relay / mempool sync
+        self.extra_args = [["-whitelist=127.0.0.1"]] * self.num_nodes
         self.enable_mocktime()
+        # nodes[0] moves the chain and checks the spam blocks, nodes[1] sends them
 
     def log_title(self):
         title = "*** Starting %s ***" % self.__class__.__name__
@@ -76,17 +74,16 @@ def log_title(self):
                       "3) Stake on a fork chain with coinstake input spent (later) in main chain\n"
         self.log.info("\n\n%s\n%s\n%s\n", title, underline, description)
 
-
     def run_test(self):
         # init custom fields
-        self.mocktime -= (131 * 60)
+        self.mocktime = int(time.time())
+        set_node_times(self.nodes, self.mocktime)
         self.recipient_0 = self.nodes[0].getnewaddress()
         self.recipient_1 = self.nodes[1].getnewaddress()
         self.init_dummy_key()
 
         # start test
         self.log_title()
-        set_node_times(self.nodes, self.mocktime)
 
         # nodes[0] mines 50 blocks (201-250) to reach PoS activation
         self.log.info("Mining 50 blocks to reach PoS phase...")
@@ -131,7 +128,7 @@ def test_2(self):
         assert_equal(self.nodes[1].getblockcount(), 255)
         txid = self.spend_utxos(1, self.utxos_to_spend, self.recipient_0)[0]
         self.log.info("'utxos_to_spend' spent on txid=(%s...) on block 256" % txid[:16])
-        self.sync_all()
+        self.sync_mempools()
 
         # nodes[0] mines 5 more blocks (256-260) to include the spends
         self.log.info("Mining 5 blocks to include the spends...")

test/functional/mining_pos_reorg.py

@@ -18,7 +18,7 @@ class ReorgStakeTest(PivxTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 3
-        self.extra_args = [['-nuparams=PoS:201', '-nuparams=PoS_v2:201']] * self.num_nodes
+        self.extra_args = [['-nuparams=PoS:201', '-nuparams=PoS_v2:201', "-whitelist=127.0.0.1"]] * self.num_nodes
 
     def setup_chain(self):
         self.log.info("Initializing test directory " + self.options.tmpdir)

test/functional/test_framework/test_framework.py

@@ -1371,7 +1371,7 @@ def set_test_params(self):
         self.minerPos = 4
         self.remoteDMN1Pos = 5
 
-        self.extra_args = [["-nuparams=v5_shield:249", "-nuparams=v6_evo:250"]] * self.num_nodes
+        self.extra_args = [["-nuparams=v5_shield:249", "-nuparams=v6_evo:250", "-whitelist=127.0.0.1"]] * self.num_nodes
         for i in [self.remoteOnePos, self.remoteTwoPos, self.remoteDMN1Pos]:
             self.extra_args[i] += ["-listen", "-externalip=127.0.0.1"]
         self.extra_args[self.minerPos].append("-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi")

test/functional/tiertwo_mn_compatibility.py

@@ -33,7 +33,7 @@ def set_test_params(self):
         self.masternodeOneAlias = "mnOne"
         self.masternodeTwoAlias = "mntwo"
 
-        self.extra_args = [["-nuparams=v5_shield:249", "-nuparams=v6_evo:250"]] * self.num_nodes
+        self.extra_args = [["-nuparams=v5_shield:249", "-nuparams=v6_evo:250", "-whitelist=127.0.0.1"]] * self.num_nodes
         for i in [self.remoteOnePos, self.remoteTwoPos, self.remoteDMN1Pos, self.remoteDMN2Pos, self.remoteDMN3Pos]:
             self.extra_args[i] += ["-listen", "-externalip=127.0.0.1"]
         self.extra_args[self.minerPos].append("-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi")

test/functional/wallet_listtransactions.py

@@ -23,6 +23,8 @@ def txFromHex(hexstring):
 class ListTransactionsTest(PivxTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
+        # whitelist all peers to speed up tx relay / mempool sync
+        self.extra_args = [["-whitelist=127.0.0.1"]] * self.num_nodes
         self.enable_mocktime()
 
     def run_test(self):