test: cover unused mempool space in coins cache limit #35490

w0xlt commented at 12:05 AM on June 9, 2026: contributor

This PR adds a unit test for Chainstate::GetCoinsCacheSizeState().

Chainstate::GetCoinsCacheSizeState() calculates when the UTXO/coins cache is too large and should be flushed. Part of that calculation includes unused -maxmempool space. For example, if the mempool limit is 300 MiB but the mempool is mostly empty, some of that unused space can be counted toward the coins cache limit.

The existing validation_flush_tests.cpp test checks this calculation by calling:

chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes)

This new test checks the no-argument call:

chainstate.GetCoinsCacheSizeState()

That is the call used by validation code during normal operation.

The test grows the coins cache above the coins-only limit, then checks that:

calling the explicit helper with max_mempool_size_bytes=0 reports CRITICAL
calling the normal no-argument method does not report CRITICAL, because it includes unused mempool space

This makes sure future refactors do not accidentally drop unused mempool space from the normal cache-size calculation.

test: cover unused mempool space in coins cache 843f163c66

DrahtBot added the label Tests on Jun 9, 2026

DrahtBot commented at 12:06 AM on June 9, 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/35490.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	sedited
Concept ACK	l0rinc

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

Conflicts

No conflicts as of last run.

sedited commented at 7:42 AM on June 9, 2026: contributor

Concept ACK

sedited requested review from l0rinc on Jun 9, 2026

sedited approved

sedited commented at 4:44 PM on June 29, 2026: contributor

ACK 843f163c665787d4c1f9c5edd902e48481417b30

in src/test/validation_flush_tests.cpp:85 in 843f163c66

  80 | +    for (size_t i{0}; i < MAX_ATTEMPTS && view.DynamicMemoryUsage() <= MAX_COINS_BYTES; ++i) {
  81 | +        AddTestCoin(m_rng, view);
  82 | +    }
  83 | +
  84 | +    BOOST_REQUIRE_GT(view.DynamicMemoryUsage(), MAX_COINS_BYTES);
  85 | +    BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::CRITICAL);

l0rinc commented at 8:05 PM on July 2, 2026:

Could we fold this into getcoinscachesizestate above? The existing test already grows the same cache through the relevant thresholds; resizing the chainstate cache first lets it cover the unused- mempool-space case without a second growth loop.

diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
--- a/src/test/validation_flush_tests.cpp	(revision 78f052a4053e71490cc3ac697a84c3d88a80fe20)
+++ b/src/test/validation_flush_tests.cpp	(revision 241cb62d7a1fc0713fede5a3ca7fa0c16d74ffc9)
@@ -18,33 +18,34 @@
 //! then with additional mempool head-room.
 BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
 {
+    constexpr uint64_t MAX_COINS_BYTES{8_MiB};
+    constexpr uint64_t MAX_MEMPOOL_BYTES{4_MiB};
+    constexpr uint64_t MAX_ATTEMPTS{40'000};
     Chainstate& chainstate{m_node.chainman->ActiveChainstate()};
 
     LOCK(::cs_main);
+    BOOST_REQUIRE(chainstate.ResizeCoinsCaches(MAX_COINS_BYTES, /*coinsdb_size=*/1_MiB));
     CCoinsViewCache& view{chainstate.CoinsTip()};
 
     // Sanity: an empty cache should be ≲ 1 chunk (~ 256 KiB).
     BOOST_CHECK_LT(view.DynamicMemoryUsage() / (256 * 1024.0), 1.1);
 
-    constexpr size_t MAX_COINS_BYTES{8_MiB};
-    constexpr size_t MAX_MEMPOOL_BYTES{4_MiB};
-    constexpr size_t MAX_ATTEMPTS{50'000};
 
     // Run the same growth-path twice: first with 0 head-room, then with extra head-room
-    for (size_t max_mempool_size_bytes : {size_t{0}, MAX_MEMPOOL_BYTES}) {
+    for (uint64_t max_mempool_size_bytes : {uint64_t{0}, MAX_MEMPOOL_BYTES}) {
         const int64_t full_cap{int64_t(MAX_COINS_BYTES + max_mempool_size_bytes)};
         const int64_t large_cap{LargeCoinsCacheThreshold(full_cap)};
 
         // OK → LARGE
         auto state{chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes)};
-        for (size_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= large_cap; ++i) {
+        for (uint64_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= large_cap; ++i) {
             BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::OK);
             AddTestCoin(m_rng, view);
             state = chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes);
         }
 
         // LARGE → CRITICAL
-        for (size_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= full_cap; ++i) {
+        for (uint64_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= full_cap; ++i) {
             BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::LARGE);
             AddTestCoin(m_rng, view);
             state = chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes);
@@ -52,7 +53,7 @@
         BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::CRITICAL);
     }
 
-    // Default thresholds (no explicit limits) permit many more coins.
+    // Unused mempool space permits many more coins.
     for (int i{0}; i < 1'000; ++i) {
         AddTestCoin(m_rng, view);
         BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(), CoinsCacheSizeState::OK);
@@ -60,30 +61,10 @@
 
     // CRITICAL → OK via Flush
     BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::CRITICAL);
+    BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(), CoinsCacheSizeState::OK);
     view.SetBestBlock(m_rng.rand256());
     view.Flush();
     BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::OK);
 }

l0rinc changes_requested

l0rinc commented at 8:09 PM on July 2, 2026: contributor

Concept ACK, thanks for thinking of this.

I left a suggestion to fold the new coverage into getcoinscachesizestate, since it already grows the cache through the relevant thresholds. The merged test can assert that zero mempool headroom is CRITICAL while GetCoinsCacheSizeState() still returns OK.

It can also lower MAX_ATTEMPTS to 40'000 and use uint64_t for the test limits/counters.