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

brunoerg commented at 7:14 PM on February 16, 2026: contributor

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 --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;

refactor: addrman: move consts to .h f611d3bdaf

DrahtBot added the label Tests on Feb 16, 2026

DrahtBot commented at 7:15 PM on February 16, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	frankomosh, naiyoma, danielabrozzoni, sedited
Concept ACK	exd02

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

in src/test/addrman_tests.cpp:119 in 7c397597f3 outdated

 114 | +    addr_helper.nTime = Now<NodeSeconds>();
 115 | +    BOOST_CHECK(addrman->Add({addr_helper}, source));
 116 | +    BOOST_CHECK(addrman->Good(addr_helper));
 117 | +
 118 | +    for (int i = 0; i < ADDRMAN_MAX_FAILURES; ++i) {
 119 | +        addrman->Attempt(addr, /*fCountFailure=*/true, Now<NodeSeconds>() - 61s);

frankomosh commented at 3:50 PM on February 19, 2026:

nit: Would it help to comment on why 61s is used as the timestamp here?

brunoerg commented at 4:21 PM on February 19, 2026:

Done.

frankomosh commented at 3:52 PM on February 19, 2026: contributor

tACK 7c397597f3c30776a09dbfd8bcb15658168a4c4c. Built from source, and ran the new test both clean and against the target mutant (flipping return true -> return false in the ADDRMAN_MIN_FAIL branch of IsTerrible()). Clean run: *** No errors detected Mutant run: check filtered.size() == 1U has failed [2 != 1], mutant correctly killed.

test: addrman: successive failures in the last week for IsTerrible 6202acd284

brunoerg force-pushed on Feb 19, 2026

naiyoma commented at 6:50 PM on February 19, 2026: contributor

Concept ACK

I have been trying to test this mutant locally

I started by generating and then analyzing

mutation-core mutate -f="src/test/addrman.cpp

mutation-core analyze -f="muts-addrman-cpp" -c="cmake --build build -j4 && ./build/src/test/test_bitcoin --run_test=addrman_tests" While the analysis was running, I was able to view the diff for the mutant in a separate window

In the end i was surprised to see that all the mutants had been killed [759/759] Analyzing addrman.mutant.217.cpp Running: cmake --build build -j4 && ./build/src/test/test_bitcoin --run_test=addrman_tests KILLED ✅

MUTATION SCORE: 100.0%

I’m wondering if I’m doing something wrong. I was expecting to see some mutants alive.

brunoerg commented at 6:54 PM on February 19, 2026: contributor

Concept ACK

I have been trying to test this mutant locally

I started by generating and then analyzing

mutation-core mutate -f="src/test/addrman.cpp

mutation-core analyze -f="muts-addrman-cpp" -c="cmake --build build -j4 && ./build/src/test/test_bitcoin --run_test=addrman_tests" While the analysis was running, I was able to view the diff for the mutant in a separate window

In the end i was surprised to see that all the mutants had been killed [759/759] Analyzing addrman.mutant.217.cpp Running: cmake --build build -j4 && ./build/src/test/test_bitcoin --run_test=addrman_tests KILLED ✅

MUTATION SCORE: 100.0%

I’m wondering if I’m doing something wrong. I was expecting to see some mutants alive.

Yes, the issue is that you're analyzing the mutants with ./build/src/test/test_bitcoin --run_test=addrman_tests which is wrong, it should be ./build/bin/test_bitcoin --run_test=addrman_tests. Since ./build/src/test/test_bitcoin doesn't exist and fail, it will consider all the mutants as killed.

frankomosh commented at 5:25 AM on February 20, 2026: contributor

re-ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2

DrahtBot requested review from naiyoma on Feb 20, 2026

exd02 commented at 4:42 AM on February 22, 2026: none

tACK. This PR moves the addrman consts to the header file and kills a mutant by adding a test case addrman_terrible_many_failures that emulates a terrible address (if it had N successive failures in the last week).

I built this PR from source and ran the tests

./build/bin/test_bitcoin --run_test=addrman_tests
Running 25 test cases...

*** No errors detected

After flipping the return true to return false and building again, the test failed:

./build/bin/test_bitcoin --run_test=addrman_tests
Running 25 test cases...
test/addrman_tests.cpp(124): error: in "addrman_tests/addrman_terrible_many_failures": check filtered.size() == 1U has failed [2 != 1]

*** 1 failure is detected in the test module "Bitcoin Core Test Suite

On the master branch flipping the return true/return false wasn't catched by any test.

naiyoma commented at 2:26 PM on February 23, 2026: contributor

tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2

I locally generated mutants for this condition. using(https://github.com/brunoerg/bcore-mutation)

https://github.com/bitcoin/bitcoin/blob/d9c7364ac56781a16c7224b2c7a6db9db97f17d8/src/addrman.cpp#L87

Before this test, I got 6 mutants.

if (1==0) { // N successive failures in the last week\n

if (now - m_last_success >= ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES)

if (now - m_last_success > ADDRMAN_MIN_FAIL && nAttempts > ADDRMAN_MAX_FAILURES)

if (now - m_last_success < ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES)

if (now - m_last_success <= ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES)

return true;\n+ return false;\

After this test, only 1 mutant was surviving, and 5 had been killed.

if (now - m_last_success >= ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES)

if fixing all mutants is an overkill, it’s okay to leave it as is. However, you might consider including the other mutants that have also been killed in the description. On Corecheck, I could see one other mutant related to this condition, and I believe that one has been fixed as well.

DrahtBot requested review from exd02 on Feb 23, 2026

danielabrozzoni commented at 2:14 PM on March 13, 2026: member

tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2

sedited approved

sedited commented at 9:22 PM on March 16, 2026: contributor

ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2

sedited merged this on Mar 16, 2026

sedited closed this on Mar 16, 2026