diff --git a/src/seeder/bitcoin.h b/src/seeder/bitcoin.h --- a/src/seeder/bitcoin.h +++ b/src/seeder/bitcoin.h @@ -50,6 +50,9 @@ int64_t doneAfter; CAddress you; CheckpointState checkpointState; + bool isReliable; + bool hasBeenTriedBefore; + int64_t timeSinceFoundOrLastSuccess; int GetTimeout() { return you.IsTor() ? 120 : 30; } @@ -69,8 +72,12 @@ PeerMessagingState ProcessMessage(std::string strCommand, CDataStream &recv); + void DetermineBan(bool testResult); + public: - CSeederNode(const CService &ip, std::vector *vAddrIn); + CSeederNode(const CService &ip, const bool reliable, const bool triedBefore, + const int64_t timeFirstFoundOrLastSuccess, + std::vector *vAddrIn); bool Run(); diff --git a/src/seeder/bitcoin.cpp b/src/seeder/bitcoin.cpp --- a/src/seeder/bitcoin.cpp +++ b/src/seeder/bitcoin.cpp @@ -284,17 +284,32 @@ return false; } -CSeederNode::CSeederNode(const CService &ip, std::vector *vAddrIn) +CSeederNode::CSeederNode(const CService &ip, const bool reliable, + const bool triedBefore, + const int64_t timeFirstFoundOrLastSuccess, + std::vector *vAddrIn) : sock(INVALID_SOCKET), vSend(SER_NETWORK, 0), vRecv(SER_NETWORK, 0), nHeaderStart(-1), nMessageStart(-1), nVersion(0), vAddr(vAddrIn), ban(0), doneAfter(0), you(ip, ServiceFlags(NODE_NETWORK | NODE_BITCOIN_CASH)), - checkpointState(CheckpointState::NotChecked) { + checkpointState(CheckpointState::NotChecked), isReliable(reliable), + hasBeenTriedBefore(triedBefore), + timeSinceFoundOrLastSuccess(timeFirstFoundOrLastSuccess) { if (time(nullptr) > 1329696000) { vSend.SetVersion(209); vRecv.SetVersion(209); } } +void CSeederNode::DetermineBan(bool testResult) { + // Post p2p-commincation banning logic + // Ban nodes that have failed this test, are not reliable, have been tested + // before, and whose last successful test was more than 2 days ago. + if (!testResult && !isReliable && hasBeenTriedBefore && + timeSinceFoundOrLastSuccess > 2 * 86400) { + ban = 100000; + } +} + bool CSeederNode::Run() { // FIXME: This logic is duplicated with CConnman::ConnectNode for no // good reason. @@ -385,5 +400,6 @@ } close(sock); sock = INVALID_SOCKET; + DetermineBan(res); return (ban == 0) && res; } diff --git a/src/seeder/db.h b/src/seeder/db.h --- a/src/seeder/db.h +++ b/src/seeder/db.h @@ -80,6 +80,7 @@ private: CService ip; uint64_t services; + int64_t timeFirstFound; int64_t lastTry; int64_t ourLastTry; int64_t ourLastSuccess; @@ -95,10 +96,20 @@ int success; std::string clientSubVersion; +protected: + bool hasBeenTriedBefore() const { return total > 0; } + int64_t timeSinceFoundOrLastSuccess() { + int64_t now = time(nullptr); + int64_t lastSuccessOrFirstFoundTime = + success > 0 ? ourLastSuccess : timeFirstFound; + return now - lastSuccessOrFirstFoundTime; + } + public: CAddrInfo() - : services(0), lastTry(0), ourLastTry(0), ourLastSuccess(0), - ignoreTill(0), clientVersion(0), blocks(0), total(0), success(0) {} + : services(0), timeFirstFound(time(nullptr)), lastTry(0), ourLastTry(0), + ourLastSuccess(0), ignoreTill(0), clientVersion(0), blocks(0), + total(0), success(0) {} CAddrReport GetReport() const { CAddrReport ret; @@ -221,6 +232,7 @@ return; } + READWRITE(timeFirstFound); READWRITE(ourLastTry); READWRITE(ignoreTill); READWRITE(stat2H); @@ -260,11 +272,14 @@ struct CServiceResult { CService service; bool fGood; + bool fReliable; + bool hasBeenTriedBefore; int nBanTime; int nHeight; int nClientV; std::string strClientV; int64_t ourLastSuccess; + int64_t timeSinceFoundOrLastSuccess; }; /** diff --git a/src/seeder/db.cpp b/src/seeder/db.cpp --- a/src/seeder/db.cpp +++ b/src/seeder/db.cpp @@ -69,6 +69,10 @@ } else { ip.service = idToInfo[ret].ip; ip.ourLastSuccess = idToInfo[ret].ourLastSuccess; + ip.fReliable = idToInfo[ret].IsReliable(); + ip.hasBeenTriedBefore = idToInfo[ret].hasBeenTriedBefore(); + ip.timeSinceFoundOrLastSuccess = + idToInfo[ret].timeSinceFoundOrLastSuccess(); break; } } while (1); diff --git a/src/seeder/main.cpp b/src/seeder/main.cpp --- a/src/seeder/main.cpp +++ b/src/seeder/main.cpp @@ -191,7 +191,9 @@ res.strClientV = ""; bool getaddr = res.ourLastSuccess + 86400 < now; try { - CSeederNode node(res.service, getaddr ? &addr : nullptr); + CSeederNode node( + res.service, res.fReliable, res.hasBeenTriedBefore, + res.timeSinceFoundOrLastSuccess, getaddr ? &addr : nullptr); bool ret = node.Run(); if (!ret) { res.nBanTime = node.GetBan(); diff --git a/src/seeder/test/CMakeLists.txt b/src/seeder/test/CMakeLists.txt --- a/src/seeder/test/CMakeLists.txt +++ b/src/seeder/test/CMakeLists.txt @@ -10,6 +10,7 @@ add_boost_unit_tests_to_suite(bitcoin-seeder test_bitcoin-seeder TESTS + banning_tests.cpp p2p_messaging_tests.cpp parse_name_tests.cpp write_name_tests.cpp diff --git a/src/seeder/test/banning_tests.cpp b/src/seeder/test/banning_tests.cpp new file mode 100644 --- /dev/null +++ b/src/seeder/test/banning_tests.cpp @@ -0,0 +1,136 @@ +// Copyright (c) 2020 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include + +BOOST_AUTO_TEST_SUITE(banning_tests) + +static const int64_t RECENT_SUCCESS_OR_DISCOVERY_TIME = 0; +static const int64_t OLD_SUCCESS_OR_DISCOVERY_TIME = 2 * 86400 + 1; + +class TestBanCSeederNode : public CSeederNode { +public: + TestBanCSeederNode(bool reliable, bool testedBefore, + int64_t timeSinceLastSuccess) + : CSeederNode(CService(), reliable, testedBefore, timeSinceLastSuccess, + nullptr) {} + + void TestDetermineBan(const bool results, const int &expectedBanTime) { + DetermineBan(results); + BOOST_CHECK_EQUAL(GetBan(), expectedBanTime); + } +}; + +struct TestNodeState { + TestNodeState(bool reliable, bool triedBefore, int64_t time, bool isGood) + : isReliable(reliable), hasBeenTriedBefore(triedBefore), + timeSinceLastSuccess(time), testResults(isGood) {} + + bool isReliable; + bool hasBeenTriedBefore; + int64_t timeSinceLastSuccess; + bool testResults; +}; + +static void CheckDetermineBan(TestNodeState state, int expectedBanTime) { + TestBanCSeederNode testNode(state.isReliable, state.hasBeenTriedBefore, + state.timeSinceLastSuccess); + BOOST_CHECK_EQUAL(testNode.GetBan(), 0); + testNode.TestDetermineBan(state.testResults, expectedBanTime); +} + +BOOST_AUTO_TEST_CASE(ban_test) { + // Node banning is very srict. Only a node that is unreliable, has been + // tested before, has no success/was not discovered within the last 2 days, + // and has failed the current test will be banned. + + // Node is reliable, has been tested before, had a recent success, and + // passed the current test + CheckDetermineBan( + TestNodeState(true, true, RECENT_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is reliable, has been tested before, had a recent success, and + // failed the current test + CheckDetermineBan( + TestNodeState(true, true, RECENT_SUCCESS_OR_DISCOVERY_TIME, false), 0); + + // Node is reliable, has been tested before, had no recent success, and + // passed the current test + CheckDetermineBan( + TestNodeState(true, true, OLD_SUCCESS_OR_DISCOVERY_TIME, true), 0); + // Node is reliable, has been tested before, had no recent success, and + // failed the current test + CheckDetermineBan( + TestNodeState(true, true, OLD_SUCCESS_OR_DISCOVERY_TIME, false), 0); + + // Node is reliable, has not been tested before, was discovered recently, + // and passed the current test + CheckDetermineBan( + TestNodeState(true, false, RECENT_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is reliable, has not been tested before, was discovered recently, + // and failed the current test + CheckDetermineBan( + TestNodeState(true, false, RECENT_SUCCESS_OR_DISCOVERY_TIME, false), 0); + + // Node is reliable, has not been tested before, was not discovered + // recently, and passed the current test + CheckDetermineBan( + TestNodeState(true, false, OLD_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is reliable, has not been tested before, was not discovered + // recently, and failed the current test + CheckDetermineBan( + TestNodeState(true, false, OLD_SUCCESS_OR_DISCOVERY_TIME, false), 0); + + // Node is not reliable, has been tested before, had a recent success, and + // passed the current test + CheckDetermineBan( + TestNodeState(false, true, RECENT_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is not reliable, has been tested before, had a recent success, and + // failed the current test + CheckDetermineBan( + TestNodeState(false, true, RECENT_SUCCESS_OR_DISCOVERY_TIME, false), 0); + + // Node is not reliable, has been tested before, had no recent success, and + // passed the current test + CheckDetermineBan( + TestNodeState(false, true, OLD_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is not reliable, has been tested before, had no recent success, and + // failed the current test + // This type of node is not only very likely to be following the wrong chain + // but is unlikely to be helpful as a bootstrap node to help (at least + // partially) catch up to the current chain tip. As such, it should be + // banned + CheckDetermineBan( + TestNodeState(false, true, OLD_SUCCESS_OR_DISCOVERY_TIME, false), + 100000); + + // Node is not reliable, has not been tested before, was discovered + // recently, and passed the current test + CheckDetermineBan( + TestNodeState(false, false, RECENT_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is not reliable, has been not tested before, was discovered + // recently, and failed the current test + CheckDetermineBan( + TestNodeState(false, false, RECENT_SUCCESS_OR_DISCOVERY_TIME, false), + 0); + + // Node is not reliable, has not been tested before, was not discovered + // recently, and passed the current test + CheckDetermineBan( + TestNodeState(false, false, OLD_SUCCESS_OR_DISCOVERY_TIME, true), 0); + + // Node is not reliable, has not been tested before, was not discovered + // recently, and failed the current test + CheckDetermineBan( + TestNodeState(false, false, OLD_SUCCESS_OR_DISCOVERY_TIME, false), 0); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/seeder/test/p2p_messaging_tests.cpp b/src/seeder/test/p2p_messaging_tests.cpp --- a/src/seeder/test/p2p_messaging_tests.cpp +++ b/src/seeder/test/p2p_messaging_tests.cpp @@ -32,7 +32,7 @@ class TestCSeederNode : public CSeederNode { public: TestCSeederNode(const CService &service, std::vector *vAddrIn) - : CSeederNode(service, vAddrIn) { + : CSeederNode(service, true, true, time(nullptr), vAddrIn) { SelectParams(CBaseChainParams::MAIN); }