From 41f84d5eccd4c2620bf6fee616f2f8f717dbd6f6 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 20 Feb 2021 11:21:34 +0100 Subject: Move peer eviction tests to a separate test file out of net_tests, because the eviction tests: - are a different domain of test coverage, with different dependencies - run more slowly than the net tests - will be growing in size, in this PR branch and in the future, as eviction test coverage is improved --- src/test/net_peer_eviction_tests.cpp | 160 +++++++++++++++++++++++++++++++++++ src/test/net_tests.cpp | 143 ------------------------------- 2 files changed, 160 insertions(+), 143 deletions(-) create mode 100644 src/test/net_peer_eviction_tests.cpp (limited to 'src/test') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp new file mode 100644 index 0000000000..b80beeac5a --- /dev/null +++ b/src/test/net_peer_eviction_tests.cpp @@ -0,0 +1,160 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include + +#include + +#include +#include +#include +#include + +BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup) + +std::vector GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) +{ + std::vector candidates; + for (int id = 0; id < n_candidates; ++id) { + candidates.push_back({ + /* id */ id, + /* nTimeConnected */ static_cast(random_context.randrange(100)), + /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, + /* nLastBlockTime */ static_cast(random_context.randrange(100)), + /* nLastTXTime */ static_cast(random_context.randrange(100)), + /* fRelevantServices */ random_context.randbool(), + /* fRelayTxes */ random_context.randbool(), + /* fBloomFilter */ random_context.randbool(), + /* nKeyedNetGroup */ random_context.randrange(100), + /* prefer_evict */ random_context.randbool(), + /* m_is_local */ random_context.randbool(), + }); + } + return candidates; +} + +// Returns true if any of the node ids in node_ids are selected for eviction. +bool IsEvicted(std::vector candidates, const std::vector& node_ids, FastRandomContext& random_context) +{ + Shuffle(candidates.begin(), candidates.end(), random_context); + const std::optional evicted_node_id = SelectNodeToEvict(std::move(candidates)); + if (!evicted_node_id) { + return false; + } + return std::find(node_ids.begin(), node_ids.end(), *evicted_node_id) != node_ids.end(); +} + +// Create number_of_nodes random nodes, apply setup function candidate_setup_fn, +// apply eviction logic and then return true if any of the node ids in node_ids +// are selected for eviction. +bool IsEvicted(const int number_of_nodes, std::function candidate_setup_fn, const std::vector& node_ids, FastRandomContext& random_context) +{ + std::vector candidates = GetRandomNodeEvictionCandidates(number_of_nodes, random_context); + for (NodeEvictionCandidate& candidate : candidates) { + candidate_setup_fn(candidate); + } + return IsEvicted(candidates, node_ids, random_context); +} + +namespace { +constexpr int NODE_EVICTION_TEST_ROUNDS{10}; +constexpr int NODE_EVICTION_TEST_UP_TO_N_NODES{200}; +} // namespace + +BOOST_AUTO_TEST_CASE(peer_eviction_test) +{ + FastRandomContext random_context{true}; + + for (int i = 0; i < NODE_EVICTION_TEST_ROUNDS; ++i) { + for (int number_of_nodes = 0; number_of_nodes < NODE_EVICTION_TEST_UP_TO_N_NODES; ++number_of_nodes) { + // Four nodes with the highest keyed netgroup values should be + // protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nKeyedNetGroup = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Eight nodes with the lowest minimum ping time should be protected + // from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [](NodeEvictionCandidate& candidate) { + candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; + }, + {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); + + // Four nodes that most recently sent us novel transactions accepted + // into our mempool should be protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastTXTime = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Up to eight non-tx-relay peers that most recently sent us novel + // blocks should be protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + if (candidate.id <= 7) { + candidate.fRelayTxes = false; + candidate.fRelevantServices = true; + } + }, + {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); + + // Four peers that most recently sent us novel blocks should be + // protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Combination of the previous two tests. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + if (candidate.id <= 7) { + candidate.fRelayTxes = false; + candidate.fRelevantServices = true; + } + }, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, random_context)); + + // Combination of all tests above. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nKeyedNetGroup = number_of_nodes - candidate.id; // 4 protected + candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; // 8 protected + candidate.nLastTXTime = number_of_nodes - candidate.id; // 4 protected + candidate.nLastBlockTime = number_of_nodes - candidate.id; // 4 protected + }, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}, random_context)); + + // An eviction is expected given >= 29 random eviction candidates. The eviction logic protects at most + // four peers by net group, eight by lowest ping time, four by last time of novel tx, up to eight non-tx-relay + // peers by last novel block time, and four more peers by last novel block time. + if (number_of_nodes >= 29) { + BOOST_CHECK(SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); + } + + // No eviction is expected given <= 20 random eviction candidates. The eviction logic protects at least + // four peers by net group, eight by lowest ping time, four by last time of novel tx and four peers by last + // novel block time. + if (number_of_nodes <= 20) { + BOOST_CHECK(!SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); + } + + // Cases left to test: + // * "Protect the half of the remaining nodes which have been connected the longest. [...]" + // * "Pick out up to 1/4 peers that are localhost, sorted by longest uptime. [...]" + // * "If any remaining peers are preferred for eviction consider only them. [...]" + // * "Identify the network group with the most connections and youngest member. [...]" + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3172f119bd..3b3b71ea2a 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -802,147 +802,4 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle) BOOST_CHECK_EQUAL(IsLocal(addr), false); } -std::vector GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) -{ - std::vector candidates; - for (int id = 0; id < n_candidates; ++id) { - candidates.push_back({ - /* id */ id, - /* nTimeConnected */ static_cast(random_context.randrange(100)), - /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, - /* nLastBlockTime */ static_cast(random_context.randrange(100)), - /* nLastTXTime */ static_cast(random_context.randrange(100)), - /* fRelevantServices */ random_context.randbool(), - /* fRelayTxes */ random_context.randbool(), - /* fBloomFilter */ random_context.randbool(), - /* nKeyedNetGroup */ random_context.randrange(100), - /* prefer_evict */ random_context.randbool(), - /* m_is_local */ random_context.randbool(), - }); - } - return candidates; -} - -// Returns true if any of the node ids in node_ids are selected for eviction. -bool IsEvicted(std::vector candidates, const std::vector& node_ids, FastRandomContext& random_context) -{ - Shuffle(candidates.begin(), candidates.end(), random_context); - const std::optional evicted_node_id = SelectNodeToEvict(std::move(candidates)); - if (!evicted_node_id) { - return false; - } - return std::find(node_ids.begin(), node_ids.end(), *evicted_node_id) != node_ids.end(); -} - -// Create number_of_nodes random nodes, apply setup function candidate_setup_fn, -// apply eviction logic and then return true if any of the node ids in node_ids -// are selected for eviction. -bool IsEvicted(const int number_of_nodes, std::function candidate_setup_fn, const std::vector& node_ids, FastRandomContext& random_context) -{ - std::vector candidates = GetRandomNodeEvictionCandidates(number_of_nodes, random_context); - for (NodeEvictionCandidate& candidate : candidates) { - candidate_setup_fn(candidate); - } - return IsEvicted(candidates, node_ids, random_context); -} - -namespace { -constexpr int NODE_EVICTION_TEST_ROUNDS{10}; -constexpr int NODE_EVICTION_TEST_UP_TO_N_NODES{200}; -} // namespace - -BOOST_AUTO_TEST_CASE(node_eviction_test) -{ - FastRandomContext random_context{true}; - - for (int i = 0; i < NODE_EVICTION_TEST_ROUNDS; ++i) { - for (int number_of_nodes = 0; number_of_nodes < NODE_EVICTION_TEST_UP_TO_N_NODES; ++number_of_nodes) { - // Four nodes with the highest keyed netgroup values should be - // protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nKeyedNetGroup = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Eight nodes with the lowest minimum ping time should be protected - // from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [](NodeEvictionCandidate& candidate) { - candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; - }, - {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); - - // Four nodes that most recently sent us novel transactions accepted - // into our mempool should be protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastTXTime = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Up to eight non-tx-relay peers that most recently sent us novel - // blocks should be protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - if (candidate.id <= 7) { - candidate.fRelayTxes = false; - candidate.fRelevantServices = true; - } - }, - {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); - - // Four peers that most recently sent us novel blocks should be - // protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Combination of the previous two tests. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - if (candidate.id <= 7) { - candidate.fRelayTxes = false; - candidate.fRelevantServices = true; - } - }, - {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, random_context)); - - // Combination of all tests above. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nKeyedNetGroup = number_of_nodes - candidate.id; // 4 protected - candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; // 8 protected - candidate.nLastTXTime = number_of_nodes - candidate.id; // 4 protected - candidate.nLastBlockTime = number_of_nodes - candidate.id; // 4 protected - }, - {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}, random_context)); - - // An eviction is expected given >= 29 random eviction candidates. The eviction logic protects at most - // four peers by net group, eight by lowest ping time, four by last time of novel tx, up to eight non-tx-relay - // peers by last novel block time, and four more peers by last novel block time. - if (number_of_nodes >= 29) { - BOOST_CHECK(SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); - } - - // No eviction is expected given <= 20 random eviction candidates. The eviction logic protects at least - // four peers by net group, eight by lowest ping time, four by last time of novel tx and four peers by last - // novel block time. - if (number_of_nodes <= 20) { - BOOST_CHECK(!SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); - } - - // Cases left to test: - // * "Protect the half of the remaining nodes which have been connected the longest. [...]" - // * "Pick out up to 1/4 peers that are localhost, sorted by longest uptime. [...]" - // * "If any remaining peers are preferred for eviction consider only them. [...]" - // * "Identify the network group with the most connections and youngest member. [...]" - } - } -} - BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From ca63b53ecdf377ce777fd959d400748912266748 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 28 Feb 2021 15:53:07 +0100 Subject: Use std::unordered_set instead of std::vector in IsEvicted() An unordered set can tell if an element is present in ~O(1) time (constant on average, worst case linear to the size of the container), which speeds up and simplifies the lookup in IsEvicted(). Co-authored-by: Vasil Dimov --- src/test/net_peer_eviction_tests.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src/test') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index b80beeac5a..290a6b7eae 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup) @@ -36,20 +37,20 @@ std::vector GetRandomNodeEvictionCandidates(const int n_c } // Returns true if any of the node ids in node_ids are selected for eviction. -bool IsEvicted(std::vector candidates, const std::vector& node_ids, FastRandomContext& random_context) +bool IsEvicted(std::vector candidates, const std::unordered_set& node_ids, FastRandomContext& random_context) { Shuffle(candidates.begin(), candidates.end(), random_context); const std::optional evicted_node_id = SelectNodeToEvict(std::move(candidates)); if (!evicted_node_id) { return false; } - return std::find(node_ids.begin(), node_ids.end(), *evicted_node_id) != node_ids.end(); + return node_ids.count(*evicted_node_id); } // Create number_of_nodes random nodes, apply setup function candidate_setup_fn, // apply eviction logic and then return true if any of the node ids in node_ids // are selected for eviction. -bool IsEvicted(const int number_of_nodes, std::function candidate_setup_fn, const std::vector& node_ids, FastRandomContext& random_context) +bool IsEvicted(const int number_of_nodes, std::function candidate_setup_fn, const std::unordered_set& node_ids, FastRandomContext& random_context) { std::vector candidates = GetRandomNodeEvictionCandidates(number_of_nodes, random_context); for (NodeEvictionCandidate& candidate : candidates) { -- cgit v1.2.3 From 72e30e8e03f880eba4bd1c3fc18b5558d8cef680 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 21 Feb 2021 16:36:49 +0100 Subject: Add unit tests for ProtectEvictionCandidatesByRatio() Thank you to Vasil Dimov (vasild) for the suggestion to use std::unordered_set rather than std::vector for the IsProtected() peer id arguments. --- src/test/net_peer_eviction_tests.cpp | 104 ++++++++++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 7 deletions(-) (limited to 'src/test') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 290a6b7eae..418f5a4f71 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -15,6 +15,11 @@ BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup) +namespace { +constexpr int NODE_EVICTION_TEST_ROUNDS{10}; +constexpr int NODE_EVICTION_TEST_UP_TO_N_NODES{200}; +} // namespace + std::vector GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) { std::vector candidates; @@ -36,6 +41,98 @@ std::vector GetRandomNodeEvictionCandidates(const int n_c return candidates; } +// Create `num_peers` random nodes, apply setup function `candidate_setup_fn`, +// call ProtectEvictionCandidatesByRatio() to apply protection logic, and then +// return true if all of `protected_peer_ids` and none of `unprotected_peer_ids` +// are protected from eviction, i.e. removed from the eviction candidates. +bool IsProtected(int num_peers, + std::function candidate_setup_fn, + const std::unordered_set& protected_peer_ids, + const std::unordered_set& unprotected_peer_ids, + FastRandomContext& random_context) +{ + std::vector candidates{GetRandomNodeEvictionCandidates(num_peers, random_context)}; + for (NodeEvictionCandidate& candidate : candidates) { + candidate_setup_fn(candidate); + } + Shuffle(candidates.begin(), candidates.end(), random_context); + + const size_t size{candidates.size()}; + const size_t expected{size - size / 2}; // Expect half the candidates will be protected. + ProtectEvictionCandidatesByRatio(candidates); + BOOST_CHECK_EQUAL(candidates.size(), expected); + + size_t unprotected_count{0}; + for (const NodeEvictionCandidate& candidate : candidates) { + if (protected_peer_ids.count(candidate.id)) { + // this peer should have been removed from the eviction candidates + BOOST_TEST_MESSAGE(strprintf("expected candidate to be protected: %d", candidate.id)); + return false; + } + if (unprotected_peer_ids.count(candidate.id)) { + // this peer remains in the eviction candidates, as expected + ++unprotected_count; + } + } + + const bool is_protected{unprotected_count == unprotected_peer_ids.size()}; + if (!is_protected) { + BOOST_TEST_MESSAGE(strprintf("unprotected: expected %d, actual %d", + unprotected_peer_ids.size(), unprotected_count)); + } + return is_protected; +} + +BOOST_AUTO_TEST_CASE(peer_protection_test) +{ + FastRandomContext random_context{true}; + int num_peers{12}; + + // Expect half of the peers with greatest uptime (the lowest nTimeConnected) + // to be protected from eviction. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + }, + /* protected_peer_ids */ {0, 1, 2, 3, 4, 5}, + /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11}, + random_context)); + + // Verify in the opposite direction. + BOOST_CHECK(IsProtected( + num_peers, [num_peers](NodeEvictionCandidate& c) { + c.nTimeConnected = num_peers - c.id; + c.m_is_local = false; + }, + /* protected_peer_ids */ {6, 7, 8, 9, 10, 11}, + /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, + random_context)); + + // Test protection of localhost peers... + + // Expect 1/4 localhost peers to be protected from eviction, + // independently of other characteristics. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); + }, + /* protected_peer_ids */ {1, 9, 11}, + /* unprotected_peer_ids */ {}, + random_context)); + + // Expect 1/4 localhost peers and 1/4 of the others to be protected + // from eviction, sorted by longest uptime (lowest nTimeConnected). + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id > 6); + }, + /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, + /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, + random_context)); +} + // Returns true if any of the node ids in node_ids are selected for eviction. bool IsEvicted(std::vector candidates, const std::unordered_set& node_ids, FastRandomContext& random_context) { @@ -59,11 +156,6 @@ bool IsEvicted(const int number_of_nodes, std::function Date: Fri, 25 Dec 2020 23:56:17 +0100 Subject: Add m_inbound_onion to AttemptToEvictConnection() and an `m_is_onion` struct member to NodeEvictionCandidate and tests. We'll use these in the peer eviction logic to protect inbound onion peers in addition to the existing protection of localhost peers. --- src/test/fuzz/node_eviction.cpp | 1 + src/test/net_peer_eviction_tests.cpp | 1 + 2 files changed, 2 insertions(+) (limited to 'src/test') diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 603d520cf5..70ffc6bf37 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -31,6 +31,7 @@ FUZZ_TARGET(node_eviction) /* nKeyedNetGroup */ fuzzed_data_provider.ConsumeIntegral(), /* prefer_evict */ fuzzed_data_provider.ConsumeBool(), /* m_is_local */ fuzzed_data_provider.ConsumeBool(), + /* m_is_onion */ fuzzed_data_provider.ConsumeBool(), }); } // Make a copy since eviction_candidates may be in some valid but otherwise diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 418f5a4f71..517474bad4 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -36,6 +36,7 @@ std::vector GetRandomNodeEvictionCandidates(const int n_c /* nKeyedNetGroup */ random_context.randrange(100), /* prefer_evict */ random_context.randbool(), /* m_is_local */ random_context.randbool(), + /* m_is_onion */ random_context.randbool(), }); } return candidates; -- cgit v1.2.3 From caa21f586f951d626a67f391050c3644f1057f57 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 20 Feb 2021 17:17:26 +0100 Subject: Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() Now that we have a reliable way to detect inbound onion peers, this commit updates our existing eviction protection of 1/4 localhost peers to instead protect up to 1/4 onion peers (connected via our tor control service), sorted by longest uptime. Any remaining slots of the 1/4 are then allocated to protect localhost peers, or 2 localhost peers if no slots remain and 2 or more onion peers are protected, sorted by longest uptime. The goal is to avoid penalizing onion peers, due to their higher min ping times relative to IPv4 and IPv6 peers, and improve our diversity of peer connections. Thank you to Gregory Maxwell, Suhas Daftuar, Vasil Dimov and Pieter Wuille for valuable review feedback that shaped the direction. --- src/test/net_peer_eviction_tests.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src/test') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 517474bad4..0bd0aefcee 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; - c.m_is_local = false; + c.m_is_onion = c.m_is_local = false; }, /* protected_peer_ids */ {0, 1, 2, 3, 4, 5}, /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11}, @@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [num_peers](NodeEvictionCandidate& c) { c.nTimeConnected = num_peers - c.id; - c.m_is_local = false; + c.m_is_onion = c.m_is_local = false; }, /* protected_peer_ids */ {6, 7, 8, 9, 10, 11}, /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, @@ -113,20 +113,22 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) // Test protection of localhost peers... // Expect 1/4 localhost peers to be protected from eviction, - // independently of other characteristics. + // if no onion peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { + c.m_is_onion = false; c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); }, /* protected_peer_ids */ {1, 9, 11}, /* unprotected_peer_ids */ {}, random_context)); - // Expect 1/4 localhost peers and 1/4 of the others to be protected - // from eviction, sorted by longest uptime (lowest nTimeConnected). + // Expect 1/4 localhost peers and 1/4 of the other peers to be protected, + // sorted by longest uptime (lowest nTimeConnected), if no onion peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; + c.m_is_onion = false; c.m_is_local = (c.id > 6); }, /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, -- cgit v1.2.3 From 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 21 Feb 2021 22:22:42 +0100 Subject: Add unit test coverage for our onion peer eviction protection --- src/test/net_peer_eviction_tests.cpp | 96 +++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 0bd0aefcee..31d391bf7d 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -110,7 +110,29 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, random_context)); - // Test protection of localhost peers... + // Test protection of onion and localhost peers... + + // Expect 1/4 onion peers to be protected from eviction, + // independently of other characteristics. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.m_is_onion = (c.id == 3 || c.id == 8 || c.id == 9); + }, + /* protected_peer_ids */ {3, 8, 9}, + /* unprotected_peer_ids */ {}, + random_context)); + + // Expect 1/4 onion peers and 1/4 of the others to be protected + // from eviction, sorted by longest uptime (lowest nTimeConnected). + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + c.m_is_onion = (c.id == 3 || c.id > 7); + }, + /* protected_peer_ids */ {0, 1, 2, 3, 8, 9}, + /* unprotected_peer_ids */ {4, 5, 6, 7, 10, 11}, + random_context)); // Expect 1/4 localhost peers to be protected from eviction, // if no onion peers. @@ -134,6 +156,78 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, random_context)); + + // Combined test: expect 1/4 onion and 2 localhost peers to be protected + // from eviction, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id == 0 || c.id == 5 || c.id == 10); + c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); + }, + /* protected_peer_ids */ {0, 1, 2, 5, 9, 10}, + /* unprotected_peer_ids */ {3, 4, 6, 7, 8, 11}, + random_context)); + + // Combined test: expect having only 1 onion to allow allocating the + // remaining 2 of the 1/4 to localhost peers, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers + 4, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id == 15); + c.m_is_local = (c.id > 6 && c.id < 11); + }, + /* protected_peer_ids */ {0, 1, 2, 3, 7, 8, 9, 15}, + /* unprotected_peer_ids */ {4, 5, 6, 10, 11, 12, 13, 14}, + random_context)); + + // Combined test: expect 2 onions (< 1/4) to allow allocating the minimum 2 + // localhost peers, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id == 7 || c.id == 9); + c.m_is_local = (c.id == 6 || c.id == 11); + }, + /* protected_peer_ids */ {0, 1, 6, 7, 9, 11}, + /* unprotected_peer_ids */ {2, 3, 4, 5, 8, 10}, + random_context)); + + // Combined test: when > 1/4, expect max 1/4 onion and 2 localhost peers + // to be protected from eviction, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id > 3 && c.id < 8); + c.m_is_local = (c.id > 7); + }, + /* protected_peer_ids */ {0, 4, 5, 6, 8, 9}, + /* unprotected_peer_ids */ {1, 2, 3, 7, 10, 11}, + random_context)); + + // Combined test: idem > 1/4 with only 8 peers: expect 2 onion and 2 + // localhost peers (1/4 + 2) to be protected, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 8, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id > 1 && c.id < 5); + c.m_is_local = (c.id > 4); + }, + /* protected_peer_ids */ {2, 3, 5, 6}, + /* unprotected_peer_ids */ {0, 1, 4, 7}, + random_context)); + + // Combined test: idem > 1/4 with only 6 peers: expect 1 onion peer and no + // localhost peers (1/4 + 0) to be protected, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 6, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_onion = (c.id == 4 || c.id == 5); + c.m_is_local = (c.id == 3); + }, + /* protected_peer_ids */ {0, 1, 4}, + /* unprotected_peer_ids */ {2, 3, 5}, + random_context)); } // Returns true if any of the node ids in node_ids are selected for eviction. -- cgit v1.2.3