test: update tests for peer discouragement

Author: jonatack

src/test/denialofservice_tests.cpp

@@ -217,7 +217,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
     connman->ClearNodes();
 }
 
-BOOST_AUTO_TEST_CASE(DoS_banning)
+BOOST_AUTO_TEST_CASE(peer_discouragement)
 {
     auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
@@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     dummyNode2.fSuccessfullyConnected = true;
     {
         LOCK(cs_main);
-        Misbehaving(dummyNode2.GetId(), 50);
+        Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1);
     }
     {
         LOCK2(cs_main, dummyNode2.cs_sendProcessing);
@@ -259,64 +259,20 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     BOOST_CHECK(banman->IsDiscouraged(addr1));  // ... but 1 still should be
     {
         LOCK(cs_main);
-        Misbehaving(dummyNode2.GetId(), 50);
+        Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold
     }
     {
         LOCK2(cs_main, dummyNode2.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
     }
-    BOOST_CHECK(banman->IsDiscouraged(addr2));
+    BOOST_CHECK(banman->IsDiscouraged(addr1));  // Expect both 1 and 2
+    BOOST_CHECK(banman->IsDiscouraged(addr2));  // to be discouraged now
 
     bool dummy;
     peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
     peerLogic->FinalizeNode(dummyNode2.GetId(), dummy);
 }
 
-BOOST_AUTO_TEST_CASE(DoS_banscore)
-{
-    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
-    auto peerLogic = MakeUnique<PeerLogicValidation>(connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);
-
-    banman->ClearBanned();
-    CAddress addr1(ip(0xa0b0c001), NODE_NONE);
-    CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", true);
-    dummyNode1.SetSendVersion(PROTOCOL_VERSION);
-    peerLogic->InitializeNode(&dummyNode1);
-    dummyNode1.nVersion = 1;
-    dummyNode1.fSuccessfullyConnected = true;
-    {
-        LOCK(cs_main);
-        Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD - 11);
-    }
-    {
-        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
-        BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
-    }
-    BOOST_CHECK(!banman->IsDiscouraged(addr1));
-    {
-        LOCK(cs_main);
-        Misbehaving(dummyNode1.GetId(), 10);
-    }
-    {
-        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
-        BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
-    }
-    BOOST_CHECK(!banman->IsDiscouraged(addr1));
-    {
-        LOCK(cs_main);
-        Misbehaving(dummyNode1.GetId(), 1);
-    }
-    {
-        LOCK2(cs_main, dummyNode1.cs_sendProcessing);
-        BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
-    }
-    BOOST_CHECK(banman->IsDiscouraged(addr1));
-
-    bool dummy;
-    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
-}
-
 BOOST_AUTO_TEST_CASE(DoS_bantime)
 {
     auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);

test/functional/p2p_leak.py

@@ -65,9 +65,10 @@ def on_blocktxn(self, message): self.bad_message(message)
 
 # Node that never sends a version. We'll use this to send a bunch of messages
 # anyway, and eventually get disconnected.
-class CNodeNoVersionBan(CLazyNode):
-    # send a bunch of veracks without sending a message. This should get us disconnected.
-    # NOTE: implementation-specific check here. Remove if bitcoind ban behavior changes
+class CNodeNoVersionMisbehavior(CLazyNode):
+    # Send enough veracks without a message to reach the peer discouragement
+    # threshold. This should get us disconnected. NOTE: implementation-specific
+    # test; update if our discouragement policy for peer misbehavior changes.
     def on_open(self):
         super().on_open()
         for _ in range(DISCOURAGEMENT_THRESHOLD):
@@ -108,15 +109,16 @@ def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
-        no_version_bannode = self.nodes[0].add_p2p_connection(CNodeNoVersionBan(), send_version=False, wait_for_verack=False)
+        no_version_disconnect_node = self.nodes[0].add_p2p_connection(
+            CNodeNoVersionMisbehavior(), send_version=False, wait_for_verack=False)
         no_version_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVersionIdle(), send_version=False, wait_for_verack=False)
         no_verack_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVerackIdle(), wait_for_verack=False)
 
         # Wait until we got the verack in response to the version. Though, don't wait for the other node to receive the
         # verack, since we never sent one
         no_verack_idlenode.wait_for_verack()
 
-        wait_until(lambda: no_version_bannode.ever_connected, timeout=10, lock=mininode_lock)
+        wait_until(lambda: no_version_disconnect_node.ever_connected, timeout=10, lock=mininode_lock)
         wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock)
         wait_until(lambda: no_verack_idlenode.version_received, timeout=10, lock=mininode_lock)
 
@@ -126,13 +128,13 @@ def run_test(self):
         #Give the node enough time to possibly leak out a message
         time.sleep(5)
 
-        #This node should have been banned
-        assert not no_version_bannode.is_connected
+        # Expect this node to be disconnected for misbehavior
+        assert not no_version_disconnect_node.is_connected
 
         self.nodes[0].disconnect_p2ps()
 
         # Make sure no unexpected messages came in
-        assert no_version_bannode.unexpected_msg == False
+        assert no_version_disconnect_node.unexpected_msg == False
         assert no_version_idlenode.unexpected_msg == False
         assert no_verack_idlenode.unexpected_msg == False