test: add pow tests covering live mutants #36231

ViniciusCestarii commented at 3:19 PM on September 12, 2026: contributor

Kills 4 live mutants on pow.cpp found with https://github.com/ViniciusCestarii/mutant-harness. The first 2 affect consensus. The third affect header sync and could stall IBD. The forth cover header sync hardening against DoS. They are:

<details> <summary>pow.cpp (killed by 1811735255b98257e7bf385325599c8d9bb69ad0): <code>GetNextWorkRequired</code>: <code>pindexFirst->GetBlockTime()</code> -> <code>GetMedianTimePast()</code></summary>

diff --git a/src/pow.cpp b/src/pow.cpp
index 9a9f4e5..9872b2c 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -44,7 +44,7 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     const CBlockIndex* pindexFirst = pindexLast->GetAncestor(nHeightFirst);
     assert(pindexFirst);
 
-    return CalculateNextWorkRequired(pindexLast, pindexFirst->GetBlockTime(), params);
+    return CalculateNextWorkRequired(pindexLast, pindexFirst->GetMedianTimePast(), params);
 }
 
 unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nFirstBlockTime, const Consensus::Params& params)

</details>

<details> <summary>pow.cpp (killed by 7834db23914a3faa3660cb609144cd1c345a1ef3): <code>CalculateNextWorkRequired</code>: <code>int64_t nActualTimespan</code> -> <code>uint32_t</code>.</summary>

diff --git a/src/pow.cpp b/src/pow.cpp
index 9a9f4e5..1917b7b 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -53,7 +53,7 @@ unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nF
         return pindexLast->nBits;
 
     // Limit adjustment step
-    int64_t nActualTimespan = pindexLast->GetBlockTime() - nFirstBlockTime;
+    uint32_t nActualTimespan = pindexLast->GetBlockTime() - nFirstBlockTime;
     if (nActualTimespan < params.nPowTargetTimespan/4)
         nActualTimespan = params.nPowTargetTimespan/4;
     if (nActualTimespan > params.nPowTargetTimespan*4)

</details>

<details> <summary>pow.cpp (killed by a9873943e259b23884fb4295591a82168c831026): <code>PermittedDifficultyTransition</code>: compares <code>smallest_difficulty_target</code> directly instead of round-tripping it through <code>SetCompact(GetCompact())</code></summary>

diff --git a/src/pow.cpp b/src/pow.cpp
index 9a9f4e5..028fa9f 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -124,11 +124,8 @@ bool PermittedDifficultyTransition(const Consensus::Params& params, int64_t heig
             smallest_difficulty_target = pow_limit;
         }
 
-        // Round and then compare this new calculated value to what is
-        // observed.
-        arith_uint256 minimum_new_target;
-        minimum_new_target.SetCompact(smallest_difficulty_target.GetCompact());
-        if (minimum_new_target > observed_new_target) return false;
+        // Compare this new calculated value to what is observed.
+        if (smallest_difficulty_target > observed_new_target) return false;
     } else if (old_nbits != new_nbits) {
         return false;
     }

</details>

<details> <summary>pow.cpp (killed by e6975adf58c354dbd167b419ad7aadc7b8053af6): <code>PermittedDifficultyTransition</code>: drops the non-retarget height check that requires <code>old_nbits == new_nbits</code></summary>

diff --git a/src/pow.cpp b/src/pow.cpp
index 9a9f4e5..ef2e788 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -129,8 +129,6 @@ bool PermittedDifficultyTransition(const Consensus::Params& params, int64_t heig
         arith_uint256 minimum_new_target;
         minimum_new_target.SetCompact(smallest_difficulty_target.GetCompact());
         if (minimum_new_target > observed_new_target) return false;
-    } else if (old_nbits != new_nbits) {
-        return false;
     }
     return true;
 }

</details>

Recommend reviewing per commit.

DrahtBot added the label Tests on Sep 12, 2026

DrahtBot commented at 3:19 PM on September 12, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36231.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	brunoerg

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

fanquake renamed this:
~~test: add tests in pow_tests.cpp covering live mutants~~
test: add pow tests covering live mutants
on Sep 14, 2026

in src/test/pow_tests.cpp:86 in b2b97b9f35

  81 | +
  82 | +/* Test the lower bound for a negative actual time taken, which is possible
  83 | + * because timestamps only have to exceed the median of the previous eleven. */
  84 | +BOOST_AUTO_TEST_CASE(get_next_work_negative_actual)
  85 | +{
  86 | +    const auto chainParams = CreateChainParams(*m_node.args, ChainType::MAIN);

brunoerg commented at 5:30 PM on September 14, 2026:

nit: Why not using the same pattern of get_next_work_lower_limit_rounding?

    const auto consensus = CreateChainParams(*m_node.args, ChainType::MAIN)->GetConsensus();

ViniciusCestarii commented at 7:10 PM on September 14, 2026:

Thanks for spotting this, I forgot to change this one. Done 7834db23914a3faa3660cb609144cd1c345a1ef3

test: cover first block time selection in GetNextWorkRequired

Co-authored-by: brunoerg <brunoely.gc@gmail.com>

1811735255

test: cover negative timespan in CalculateNextWorkRequired 7834db2391

test: cover lower bound rounding in PermittedDifficultyTransition a9873943e2

test: cover non-retarget heights in PermittedDifficultyTransition e6975adf58

in src/test/pow_tests.cpp:116 in b2b97b9f35 outdated

 110 | @@ -81,6 +111,40 @@ BOOST_AUTO_TEST_CASE(get_next_work_upper_limit_actual)
 111 |      BOOST_CHECK(!PermittedDifficultyTransition(chainParams->GetConsensus(), pindexLast.nHeight+1, pindexLast.nBits, invalid_nbits));
 112 |  }
 113 |  
 114 | +/* Test that the retarget window starts at the timestamp of the block at
 115 | + * the start of the window. */
 116 | +BOOST_AUTO_TEST_CASE(get_next_work_first_block_time)

brunoerg commented at 5:45 PM on September 14, 2026:

This is good but I think get_next_work_first_block_time could be written in way that is simpler to understand (e.g. for me it is not clear why +5000?), perhaps:

BOOST_AUTO_TEST_CASE(get_next_work_first_block_time)
  {
      const auto consensus{CreateChainParams(*m_node.args, ChainType::MAIN)->GetConsensus()};
      const int64_t interval{consensus.DifficultyAdjustmentInterval()};

      // Two full retarget windows, so that the first block of the second one
      // has ancestors and hence a median time past distinct from its own time.
      std::vector<CBlockIndex> blocks(2 * interval);
      for (int64_t i = 0; i < 2 * interval; i++) {
          blocks[i].pprev = i ? &blocks[i - 1] : nullptr;
          blocks[i].nHeight = i;
          blocks[i].nTime = 1269211443 + i * consensus.nPowTargetSpacing;
          blocks[i].nBits = 0x1c05a3f4;
      }

      const CBlockIndex& first{blocks[interval]};
      const CBlockIndex& last{blocks[2 * interval - 1]};
      BOOST_CHECK_NE(first.GetBlockTime(), first.GetMedianTimePast());
      BOOST_CHECK_EQUAL(GetNextWorkRequired(&last, nullptr, consensus),
                        CalculateNextWorkRequired(&last, first.GetBlockTime(), consensus));
  }

ViniciusCestarii commented at 7:12 PM on September 14, 2026:

I agree, I thought the 5000 was needed but it isn't. I have included most of this diff on 1811735255b98257e7bf385325599c8d9bb69ad0. I didn't include the last line because the pattern in pow_tests.cpp is to hardcode the expected_nbits.

ViniciusCestarii force-pushed on Sep 14, 2026

ViniciusCestarii commented at 7:17 PM on September 14, 2026: contributor

Thanks for the review! Forced-push e6975adf58c354dbd167b419ad7aadc7b8053af6 addressing @brunoerg comments.

brunoerg approved

brunoerg commented at 11:46 AM on September 16, 2026: contributor

ACK e6975adf58c354dbd167b419ad7aadc7b8053af6