1 files changed, 199 insertions, 29 deletions

diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp
index ffbeeb7d91..6be176a1ad 100644
--- a/src/test/DoS_tests.cpp
+++ b/src/test/DoS_tests.cpp
@@ -4,17 +4,17 @@
 
 // Unit tests for denial-of-service detection/prevention code
 
-#include "chainparams.h"
-#include "keystore.h"
-#include "net.h"
-#include "net_processing.h"
-#include "pow.h"
-#include "script/sign.h"
-#include "serialize.h"
-#include "util.h"
-#include "validation.h"
-
-#include "test/test_bitcoin.h"
+#include <chainparams.h>
+#include <keystore.h>
+#include <net.h>
+#include <net_processing.h>
+#include <pow.h>
+#include <script/sign.h>
+#include <serialize.h>
+#include <util.h>
+#include <validation.h>
+
+#include <test/test_bitcoin.h>
 
 #include <stdint.h>
 
@@ -40,8 +40,141 @@ CService ip(uint32_t i)
 
 static NodeId id = 0;
 
+void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds);
+
 BOOST_FIXTURE_TEST_SUITE(DoS_tests, TestingSetup)
 
+// Test eviction of an outbound peer whose chain never advances
+// Mock a node connection, and use mocktime to simulate a peer
+// which never sends any headers messages.  PeerLogic should
+// decide to evict that outbound peer, after the appropriate timeouts.
+// Note that we protect 4 outbound nodes from being subject to
+// this logic; this test takes advantage of that protection only
+// being applied to nodes which send headers with sufficient
+// work.
+BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
+{
+    std::atomic<bool> interruptDummy(false);
+
+    // Mock an outbound peer
+    CAddress addr1(ip(0xa0b0c001), NODE_NONE);
+    CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", /*fInboundIn=*/ false);
+    dummyNode1.SetSendVersion(PROTOCOL_VERSION);
+
+    peerLogic->InitializeNode(&dummyNode1);
+    dummyNode1.nVersion = 1;
+    dummyNode1.fSuccessfullyConnected = true;
+
+    // This test requires that we have a chain with non-zero work.
+    LOCK(cs_main);
+    BOOST_CHECK(chainActive.Tip() != nullptr);
+    BOOST_CHECK(chainActive.Tip()->nChainWork > 0);
+
+    // Test starts here
+    LOCK(dummyNode1.cs_sendProcessing);
+    peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
+    LOCK(dummyNode1.cs_vSend);
+    BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
+    dummyNode1.vSendMsg.clear();
+
+    int64_t nStartTime = GetTime();
+    // Wait 21 minutes
+    SetMockTime(nStartTime+21*60);
+    peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
+    BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
+    // Wait 3 more minutes
+    SetMockTime(nStartTime+24*60);
+    peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect
+    BOOST_CHECK(dummyNode1.fDisconnect == true);
+    SetMockTime(0);
+
+    bool dummy;
+    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
+}
+
+void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic)
+{
+    CAddress addr(ip(GetRandInt(0xffffffff)), NODE_NONE);
+    vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false));
+    CNode &node = *vNodes.back();
+    node.SetSendVersion(PROTOCOL_VERSION);
+
+    peerLogic.InitializeNode(&node);
+    node.nVersion = 1;
+    node.fSuccessfullyConnected = true;
+
+    CConnmanTest::AddNode(node);
+}
+
+BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
+{
+    const Consensus::Params& consensusParams = Params().GetConsensus();
+    constexpr int nMaxOutbound = 8;
+    CConnman::Options options;
+    options.nMaxConnections = 125;
+    options.nMaxOutbound = nMaxOutbound;
+    options.nMaxFeeler = 1;
+
+    connman->Init(options);
+    std::vector<CNode *> vNodes;
+
+    // Mock some outbound peers
+    for (int i=0; i<nMaxOutbound; ++i) {
+        AddRandomOutboundPeer(vNodes, *peerLogic);
+    }
+
+    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+
+    // No nodes should be marked for disconnection while we have no extra peers
+    for (const CNode *node : vNodes) {
+        BOOST_CHECK(node->fDisconnect == false);
+    }
+
+    SetMockTime(GetTime() + 3*consensusParams.nPowTargetSpacing + 1);
+
+    // Now tip should definitely be stale, and we should look for an extra
+    // outbound peer
+    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+    BOOST_CHECK(connman->GetTryNewOutboundPeer());
+
+    // Still no peers should be marked for disconnection
+    for (const CNode *node : vNodes) {
+        BOOST_CHECK(node->fDisconnect == false);
+    }
+
+    // If we add one more peer, something should get marked for eviction
+    // on the next check (since we're mocking the time to be in the future, the
+    // required time connected check should be satisfied).
+    AddRandomOutboundPeer(vNodes, *peerLogic);
+
+    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+    for (int i=0; i<nMaxOutbound; ++i) {
+        BOOST_CHECK(vNodes[i]->fDisconnect == false);
+    }
+    // Last added node should get marked for eviction
+    BOOST_CHECK(vNodes.back()->fDisconnect == true);
+
+    vNodes.back()->fDisconnect = false;
+
+    // Update the last announced block time for the last
+    // peer, and check that the next newest node gets evicted.
+    UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime());
+
+    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+    for (int i=0; i<nMaxOutbound-1; ++i) {
+        BOOST_CHECK(vNodes[i]->fDisconnect == false);
+    }
+    BOOST_CHECK(vNodes[nMaxOutbound-1]->fDisconnect == true);
+    BOOST_CHECK(vNodes.back()->fDisconnect == false);
+
+    bool dummy;
+    for (const CNode *node : vNodes) {
+        peerLogic->FinalizeNode(node->GetId(), dummy);
+    }
+
+    CConnmanTest::ClearNodes();
+}
+
 BOOST_AUTO_TEST_CASE(DoS_banning)
 {
     std::atomic<bool> interruptDummy(false);
@@ -50,27 +183,42 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", true);
     dummyNode1.SetSendVersion(PROTOCOL_VERSION);
-    GetNodeSignals().InitializeNode(&dummyNode1, *connman);
+    peerLogic->InitializeNode(&dummyNode1);
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode1.GetId(), 100); // Should get banned
-    SendMessages(&dummyNode1, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 100); // Should get banned
+    }
+    LOCK(dummyNode1.cs_sendProcessing);
+    peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr1));
     BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
 
     CAddress addr2(ip(0xa0b0c002), NODE_NONE);
     CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true);
     dummyNode2.SetSendVersion(PROTOCOL_VERSION);
-    GetNodeSignals().InitializeNode(&dummyNode2, *connman);
+    peerLogic->InitializeNode(&dummyNode2);
     dummyNode2.nVersion = 1;
     dummyNode2.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode2.GetId(), 50);
-    SendMessages(&dummyNode2, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode2.GetId(), 50);
+    }
+    LOCK(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
-    Misbehaving(dummyNode2.GetId(), 50);
-    SendMessages(&dummyNode2, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode2.GetId(), 50);
+    }
+    peerLogic->SendMessages(&dummyNode2, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr2));
+
+    bool dummy;
+    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
+    peerLogic->FinalizeNode(dummyNode2.GetId(), dummy);
 }
 
 BOOST_AUTO_TEST_CASE(DoS_banscore)
@@ -82,19 +230,32 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", true);
     dummyNode1.SetSendVersion(PROTOCOL_VERSION);
-    GetNodeSignals().InitializeNode(&dummyNode1, *connman);
+    peerLogic->InitializeNode(&dummyNode1);
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode1.GetId(), 100);
-    SendMessages(&dummyNode1, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 100);
+    }
+    LOCK(dummyNode1.cs_sendProcessing);
+    peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(!connman->IsBanned(addr1));
-    Misbehaving(dummyNode1.GetId(), 10);
-    SendMessages(&dummyNode1, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 10);
+    }
+    peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(!connman->IsBanned(addr1));
-    Misbehaving(dummyNode1.GetId(), 1);
-    SendMessages(&dummyNode1, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 1);
+    }
+    peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr1));
     gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
+
+    bool dummy;
+    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
 }
 
 BOOST_AUTO_TEST_CASE(DoS_bantime)
@@ -108,12 +269,16 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     CAddress addr(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, CAddress(), "", true);
     dummyNode.SetSendVersion(PROTOCOL_VERSION);
-    GetNodeSignals().InitializeNode(&dummyNode, *connman);
+    peerLogic->InitializeNode(&dummyNode);
     dummyNode.nVersion = 1;
     dummyNode.fSuccessfullyConnected = true;
 
-    Misbehaving(dummyNode.GetId(), 100);
-    SendMessages(&dummyNode, *connman, interruptDummy);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode.GetId(), 100);
+    }
+    LOCK(dummyNode.cs_sendProcessing);
+    peerLogic->SendMessages(&dummyNode, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr));
 
     SetMockTime(nStartTime+60*60);
@@ -121,11 +286,15 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
 
     SetMockTime(nStartTime+60*60*24+1);
     BOOST_CHECK(!connman->IsBanned(addr));
+
+    bool dummy;
+    peerLogic->FinalizeNode(dummyNode.GetId(), dummy);
 }
 
 CTransactionRef RandomOrphan()
 {
     std::map<uint256, COrphanTx>::iterator it;
+    LOCK(cs_main);
     it = mapOrphanTransactions.lower_bound(InsecureRand256());
     if (it == mapOrphanTransactions.end())
         it = mapOrphanTransactions.begin();
@@ -195,6 +364,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i));
     }
 
+    LOCK(cs_main);
     // Test EraseOrphansFor:
     for (NodeId i = 0; i < 3; i++)
     {