Merge bitcoin/bitcoin#34602: test: addrman: successive failures in the last week for IsTerrible

6202acd284bc0284fc9b144fdc39774f112fcdf2 test: addrman: successive failures in the last week for IsTerrible (brunoerg)
f611d3bdaf035280f44d5183a8322c0a0c6e3d27 refactor: addrman: move consts to .h (brunoerg)

Pull request description:

  This PR adds test coverage for the case that an address is considered terrible if we had N successive failures in the last week.

  It kills the following mutant (https://corecheck.dev/mutation/src/addrman.cpp#L88):
  ```diff
  diff --git a/src/addrman.cpp b/src/addrman.cpp
  index e3981e6a40..f8045491c1 100644
  --- a/src/addrman.cpp
  +++ b/src/addrman.cpp
  @@ -65,7 +65,7 @@ bool AddrInfo::IsTerrible(NodeSeconds now) const
       }

       if (now - m_last_success > ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES) { // N successive failures in the last week
  -        return true;
  +        return false;
       }

       return false;

  ```

ACKs for top commit:
  frankomosh:
    re-ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
  naiyoma:
    tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
  danielabrozzoni:
    tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
  sedited:
    ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2

Tree-SHA512: b4736ef91b75ba4c060dc18a2b796aee94d0d8be5ca58b9b873156248cd0dc6910595595e0f56d75bffff4f94c035319ac8428f7d79f07fe685bdba27a188829
This commit is contained in:
merge-script 2026-03-16 22:29:38 +01:00
commit 8d55154655
No known key found for this signature in database
GPG Key ID: 9B79B45691DB4173
3 changed files with 57 additions and 20 deletions

View File

@ -24,26 +24,6 @@
#include <cmath>
#include <optional>
/** Over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread */
static constexpr uint32_t ADDRMAN_TRIED_BUCKETS_PER_GROUP{8};
/** Over how many buckets entries with new addresses originating from a single group are spread */
static constexpr uint32_t ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP{64};
/** Maximum number of times an address can occur in the new table */
static constexpr int32_t ADDRMAN_NEW_BUCKETS_PER_ADDRESS{8};
/** How old addresses can maximally be */
static constexpr auto ADDRMAN_HORIZON{30 * 24h};
/** After how many failed attempts we give up on a new node */
static constexpr int32_t ADDRMAN_RETRIES{3};
/** How many successive failures are allowed ... */
static constexpr int32_t ADDRMAN_MAX_FAILURES{10};
/** ... in at least this duration */
static constexpr auto ADDRMAN_MIN_FAIL{7 * 24h};
/** How recent a successful connection should be before we allow an address to be evicted from tried */
static constexpr auto ADDRMAN_REPLACEMENT{4h};
/** The maximum number of tried addr collisions to store */
static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10};
/** The maximum time we'll spend trying to resolve a tried table collision */
static constexpr auto ADDRMAN_TEST_WINDOW{40min};
int AddrInfo::GetTriedBucket(const uint256& nKey, const NetGroupManager& netgroupman) const
{

View File

@ -19,6 +19,27 @@
#include <utility>
#include <vector>
/** Over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread */
static constexpr uint32_t ADDRMAN_TRIED_BUCKETS_PER_GROUP{8};
/** Over how many buckets entries with new addresses originating from a single group are spread */
static constexpr uint32_t ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP{64};
/** Maximum number of times an address can occur in the new table */
static constexpr int32_t ADDRMAN_NEW_BUCKETS_PER_ADDRESS{8};
/** How old addresses can maximally be */
static constexpr auto ADDRMAN_HORIZON{30 * 24h};
/** After how many failed attempts we give up on a new node */
static constexpr int32_t ADDRMAN_RETRIES{3};
/** How many successive failures are allowed ... */
static constexpr int32_t ADDRMAN_MAX_FAILURES{10};
/** ... in at least this duration */
static constexpr auto ADDRMAN_MIN_FAIL{7 * 24h};
/** How recent a successful connection should be before we allow an address to be evicted from tried */
static constexpr auto ADDRMAN_REPLACEMENT{4h};
/** The maximum number of tried addr collisions to store */
static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10};
/** The maximum time we'll spend trying to resolve a tried table collision */
static constexpr auto ADDRMAN_TEST_WINDOW{40min};
class InvalidAddrManVersionError : public std::ios_base::failure
{
public:

View File

@ -93,6 +93,42 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
BOOST_CHECK(addrman->Size() >= 1);
}
BOOST_AUTO_TEST_CASE(addrman_terrible_many_failures)
{
auto now = Now<NodeSeconds>();
SetMockTime(now - (ADDRMAN_MIN_FAIL + 24h));
auto addrman{std::make_unique<AddrMan>(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node))};
CNetAddr source{ResolveIP("250.1.2.1")};
CAddress addr{CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE)};
addr.nTime = Now<NodeSeconds>();
BOOST_CHECK(addrman->Add({addr}, source));
BOOST_CHECK(addrman->Good(addr));
SetMockTime(now);
CAddress addr_helper{CAddress(ResolveService("251.252.2.3", 8333), NODE_NONE)};
addr_helper.nTime = Now<NodeSeconds>();
BOOST_CHECK(addrman->Add({addr_helper}, source));
BOOST_CHECK(addrman->Good(addr_helper));
for (int i = 0; i < ADDRMAN_MAX_FAILURES; ++i) {
// Use a time > 60s ago so IsTerrible doesn't bail out at the "tried in the last minute" check
addrman->Attempt(addr, /*fCountFailure=*/true, Now<NodeSeconds>() - 61s);
}
std::vector<CAddress> filtered{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)};
BOOST_CHECK_EQUAL(filtered.size(), 1U);
BOOST_CHECK_EQUAL(filtered[0].ToStringAddrPort(), "251.252.2.3:8333");
std::vector<CAddress> unfiltered{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false)};
BOOST_CHECK_EQUAL(unfiltered.size(), 2U);
}
BOOST_AUTO_TEST_CASE(addrman_penalty_self_announcement)
{
SetMockTime(Now<NodeSeconds>());