From 3b832c9349fb495f01041ca76041e0a419de6c0d Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 5 Nov 2019 11:06:53 +0100 Subject: [PATCH 1/7] [tools] add PoissonNextSend method that returns mockable time --- src/net.h | 8 ++++++-- src/test/net_tests.cpp | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/net.h b/src/net.h index 9af94d943eeb8..dae54d71836fc 100644 --- a/src/net.h +++ b/src/net.h @@ -810,9 +810,13 @@ class CExplicitNetCleanup static void callCleanup(); }; - - /** 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 diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index a2af0fbec9086..2bd4ca85e2354 100644 --- a/src/test/net_tests.cpp +++ b/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() From 00a685cb385e3767efeda981616f0326cb168200 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 11 Jun 2021 23:11:09 -0300 Subject: [PATCH 2/7] [tools] update nNextInvSend to use mockable time --- src/net.cpp | 1 - src/net.h | 2 +- src/net_processing.cpp | 7 +++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 503b07f449114..86782f3311ec2 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2332,7 +2332,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn fGetAddr = false; nNextLocalAddrSend = 0; nNextAddrSend = 0; - nNextInvSend = 0; fRelayTxes = false; pfilter = new CBloomFilter(); timeLastMempoolReq = 0; diff --git a/src/net.h b/src/net.h index dae54d71836fc..438e516fb8175 100644 --- a/src/net.h +++ b/src/net.h @@ -623,7 +623,7 @@ class CNode std::multimap mapAskFor; std::set setAskFor; std::vector vBlockRequested; - int64_t nNextInvSend; + std::chrono::microseconds nNextInvSend{0}; // Used for BIP35 mempool sending, also protected by cs_inventory bool fSendMempool; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d32186d76cc51..5fef55d276cdf 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2132,6 +2132,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic& interruptM // Address refresh broadcast int64_t nNow = GetTimeMicros(); + auto current_time = GetTime(); + 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& 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& interruptM connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); // Detect whether we're stalling + current_time = GetTime(); 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, From 76f4a0cd9cb1f7618d05c0e3b9a977677bf7863f Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 12 Jun 2021 12:42:36 -0300 Subject: [PATCH 3/7] Test: Add missing SendMessages required cs locks --- src/test/DoS_tests.cpp | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index d2b482ef8f376..8fb29bd446036 100644 --- a/src/test/DoS_tests.cpp +++ b/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); From cc3b7bb327c212d2459862381f75926766c6c263 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 12 Jun 2021 17:29:29 -0300 Subject: [PATCH 4/7] Refactor: Move NetEventsInterface class below CNode declaration so it can get access to the cs_sendProcessing field and detect missing cs_sendProcessing locks properly. --- src/net.h | 25 ++++++++++++------------- src/net_processing.h | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/net.h b/src/net.h index 438e516fb8175..480d9265179c9 100644 --- a/src/net.h +++ b/src/net.h @@ -401,19 +401,6 @@ struct CombinerAll { } }; -/** - * Interface for message handling - */ -class NetEventsInterface -{ -public: - virtual bool ProcessMessages(CNode* pnode, std::atomic& interrupt) = 0; - virtual bool SendMessages(CNode* pnode, std::atomic& 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 @@ -810,6 +797,18 @@ class CExplicitNetCleanup static void callCleanup(); }; +/** + * Interface for message handling + */ +class NetEventsInterface +{ +public: + virtual bool ProcessMessages(CNode* pnode, std::atomic& interrupt) = 0; + virtual bool SendMessages(CNode* pnode, std::atomic& 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); diff --git a/src/net_processing.h b/src/net_processing.h index 7f32e7968d446..148fa33ee4e37 100644 --- a/src/net_processing.h +++ b/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& interrupt) override; + bool SendMessages(CNode* pto, std::atomic& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); }; struct CNodeStateStats { From a2c8716c38d3d47f4f9921b53ca18a673381cd5b Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 14 Jun 2021 16:58:56 -0300 Subject: [PATCH 5/7] Test: Add peers whitelist for tests using mocked time. --- test/functional/mining_pos_coldStaking.py | 3 ++- test/functional/mining_pos_fakestake.py | 17 +++++++---------- test/functional/mining_pos_reorg.py | 2 +- .../functional/test_framework/test_framework.py | 2 +- test/functional/tiertwo_mn_compatibility.py | 2 +- test/functional/wallet_listtransactions.py | 2 ++ 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/test/functional/mining_pos_coldStaking.py b/test/functional/mining_pos_coldStaking.py index 05aaa41924de6..ec7463ec624c6 100755 --- a/test/functional/mining_pos_coldStaking.py +++ b/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() diff --git a/test/functional/mining_pos_fakestake.py b/test/functional/mining_pos_fakestake.py index 82b9990697495..5b37360a52441 100755 --- a/test/functional/mining_pos_fakestake.py +++ b/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...") diff --git a/test/functional/mining_pos_reorg.py b/test/functional/mining_pos_reorg.py index 1eae7fed8fa42..e3500b2c585aa 100755 --- a/test/functional/mining_pos_reorg.py +++ b/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) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 260972240d125..912df7df3883e 100755 --- a/test/functional/test_framework/test_framework.py +++ b/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") diff --git a/test/functional/tiertwo_mn_compatibility.py b/test/functional/tiertwo_mn_compatibility.py index d1abb81d991e1..7de471604dbcf 100755 --- a/test/functional/tiertwo_mn_compatibility.py +++ b/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") diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index fef16eec3eb10..4b36be2bedc41 100755 --- a/test/functional/wallet_listtransactions.py +++ b/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): From aee606a6fa363e88008c43160f6d2c83117c5c17 Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 28 Jun 2021 23:14:45 -0300 Subject: [PATCH 6/7] Add missing peer whitelist flag set for manual connections. --- src/net.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net.cpp b/src/net.cpp index 86782f3311ec2..f244fd9e336a0 100644 --- a/src/net.cpp +++ b/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) From f57de3fda50ba5fd317e23cf198337594043f42e Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 28 Jun 2021 23:33:57 -0300 Subject: [PATCH 7/7] net_processing: Use constant MAX_ADDR_TO_SEND instead of harcoded value. --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5fef55d276cdf..aecb02b521920 100644 --- a/src/net_processing.cpp +++ b/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());