Page MenuHomePhabricator

coins: use dirty entry count for flush warnings and disk space checks
ClosedPublic

Authored by PiRK on Jun 11 2026, 14:37.

Details

Summary

fuzz: call EmplaceCoinInternalDANGER as well in SimulationTest

Adds test coverage by randomly calling EmplaceCoinInternalDANGER in SimulationTest to verify it remains correct as we modify it in a future commit.

coins: Keep track of number of dirty entries in CCoinsViewCache

Adds m_dirty_count member to track the running count of dirty cache entries as follows:

  • Incremented when entries are marked dirty via CCoinsCacheEntry::SetDirty
  • Decremented when dirty entries are removed or cleaned
  • Passed through CoinsViewCacheCursor and updated during iteration

The dirty count is needed because after non-wiping flushes (introduced in #28280 and #28233), the percentage of dirty entries in the cache may be far below 100%. Using total cache size for flush warnings and disk space checks is therefore misleading.

Updates all test code to properly initialize and maintain the dirty count.

validation: Use dirty entry count in flush warnings and disk space checks

Changes flush warnings to use the actual number of dirty entries being written rather than total cache size or memory usage:

  • Moves warning from FlushStateToDisk to CCoinsViewDB::BatchWrite so it applies to both regular flushes and AssumeUTXO snapshot writes
  • Changes threshold from WARN_FLUSH_COINS_SIZE (1 GiB) to WARN_FLUSH_COINS_COUNT (10M entries), approximately equivalent - this also helps with the confusion caused by UTXO size difference on-disk vs in-memory
  • Moves benchmark logging to BatchWrite where the actual disk I/O occurs to make sure AssumeUTXO also warns
  • Uses dirty count for disk space check (48 bytes per entry estimate)
  • Removes redundant changed counter since dirty_count is now tracked

This ensures users are warned appropriately even when only a fraction of the cache is dirty, and provides accurate warnings during AssumeUTXO loads.

Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Co-authored-by: optout <13562139+optout21@users.noreply.github.com>

This is a backport of core#33512 and a partial backport of core#34655
https://github.com/bitcoin/bitcoin/pull/34655/changes/b8fa6f0f701f04cffca6a085337b508381016649
Depends on D20104

Test Plan

ninja all check check-functional-extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Jun 11 2026, 14:37
src/validation.cpp
2616–2617 ↗(On Diff #59760)

they are potentially used in a tracepoint (line 2758)

Failed tests logs:

====== Bitcoin ABC extended functional tests with the next upgrade activated: chronik_electrum_basic.py ======

------- Stdout: -------
2026-06-11T15:43:27.780000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260611_154037_50379/chronik_electrum_basic_84
2026-06-11T15:43:29.360000Z TestFramework (INFO): Testing the server.peers endpoints
2026-06-11T15:43:31.691000Z TestFramework (INFO): Test the websocket transport
2026-06-11T15:43:31.897000Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 152, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 142, in _run_test_internal
    self.run_test()
  File "/work/test/functional/chronik_electrum_basic.py", line 61, in run_test
    self.test_init_errors()
  File "/work/test/functional/chronik_electrum_basic.py", line 354, in test_init_errors
    self.node.stop_node()
  File "/work/test/functional/test_framework/test_node.py", line 551, in stop_node
    self.wait_until_stopped(expected_stderr=expected_stderr)
  File "/work/test/functional/test_framework/test_node.py", line 590, in wait_until_stopped
    wait_until_helper_internal(
  File "/work/test/functional/test_framework/util.py", line 325, in wait_until_helper_internal
    if predicate():
       ^^^^^^^^^^^
  File "/work/test/functional/test_framework/test_node.py", line 591, in <lambda>
    lambda: self.is_node_stopped(**kwargs),
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/test/functional/test_framework/test_node.py", line 565, in is_node_stopped
    assert return_code == expected_ret_code, self._node_msg(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Node returned unexpected exit code (-6) vs (0) when stopping
2026-06-11T15:43:31.949000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2026-06-11T15:43:31.949000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260611_154037_50379/chronik_electrum_basic_84
2026-06-11T15:43:31.949000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260611_154037_50379/chronik_electrum_basic_84/test_framework.log
2026-06-11T15:43:31.949000Z TestFramework (ERROR): 
2026-06-11T15:43:31.949000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260611_154037_50379/chronik_electrum_basic_84' to consolidate all logs
2026-06-11T15:43:31.949000Z TestFramework (ERROR): 
2026-06-11T15:43:31.949000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2026-06-11T15:43:31.949000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2026-06-11T15:43:31.949000Z TestFramework (ERROR):
------- Stderr: -------
[node 2] Cleaning up leftover process
[node 1] Cleaning up leftover process
[node 0] Cleaning up leftover process

Each failure log is accessible here:
Bitcoin ABC extended functional tests with the next upgrade activated: chronik_electrum_basic.py

PiRK planned changes to this revision.Jun 11 2026, 20:24
PiRK requested review of this revision.Jun 12 2026, 10:10

Failed tests logs:

====== Bitcoin ABC extended functional tests with the next upgrade activated: chronik_electrum_basic.py ======

------- Stdout: -------
2026-06-12T10:45:34.248000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260612_104245_40938/chronik_electrum_basic_84
2026-06-12T10:45:35.792000Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 152, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 142, in _run_test_internal
    self.run_test()
  File "/work/test/functional/chronik_electrum_basic.py", line 53, in run_test
    self.test_donation_address()
  File "/work/test/functional/chronik_electrum_basic.py", line 90, in test_donation_address
    self.restart_node(
  File "/work/test/functional/test_framework/test_framework.py", line 646, in restart_node
    self.stop_node(i)
  File "/work/test/functional/test_framework/test_framework.py", line 632, in stop_node
    self.nodes[i].stop_node(expected_stderr, wait=wait)
  File "/work/test/functional/test_framework/test_node.py", line 551, in stop_node
    self.wait_until_stopped(expected_stderr=expected_stderr)
  File "/work/test/functional/test_framework/test_node.py", line 590, in wait_until_stopped
    wait_until_helper_internal(
  File "/work/test/functional/test_framework/util.py", line 325, in wait_until_helper_internal
    if predicate():
       ^^^^^^^^^^^
  File "/work/test/functional/test_framework/test_node.py", line 591, in <lambda>
    lambda: self.is_node_stopped(**kwargs),
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/test/functional/test_framework/test_node.py", line 565, in is_node_stopped
    assert return_code == expected_ret_code, self._node_msg(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Node returned unexpected exit code (-6) vs (0) when stopping
2026-06-12T10:45:35.844000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2026-06-12T10:45:35.844000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260612_104245_40938/chronik_electrum_basic_84
2026-06-12T10:45:35.844000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260612_104245_40938/chronik_electrum_basic_84/test_framework.log
2026-06-12T10:45:35.844000Z TestFramework (ERROR): 
2026-06-12T10:45:35.844000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-master/test/tmp/test_runner_₿₵_🏃_20260612_104245_40938/chronik_electrum_basic_84' to consolidate all logs
2026-06-12T10:45:35.845000Z TestFramework (ERROR): 
2026-06-12T10:45:35.845000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2026-06-12T10:45:35.845000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2026-06-12T10:45:35.845000Z TestFramework (ERROR):
------- Stderr: -------
[node 2] Cleaning up leftover process
[node 1] Cleaning up leftover process
[node 0] Cleaning up leftover process

Each failure log is accessible here:
Bitcoin ABC extended functional tests with the next upgrade activated: chronik_electrum_basic.py

Fabien requested changes to this revision.Jun 12 2026, 11:38
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/coins.cpp
131 ↗(On Diff #59760)

This is pretty dangerous imo, you could end up with an underflow. Is it the same (unsigned int) for cachedCoinsUsage ?

179 ↗(On Diff #59760)

same here, pre-decrement of an unsigned is always worrying

278 ↗(On Diff #59760)

iirc it can't be fresh and dirty anymore, so this is always -0 ?

This revision now requires changes to proceed.Jun 12 2026, 11:38
src/coins.cpp
131 ↗(On Diff #59760)

Nice catch. Apparently Core found some underflows while fuzzing (but maybe it is caused by a combination of this backport and another PR we don't already have, core#34165)

I'll see if we can grab the fix preemptively:
https://github.com/bitcoin/bitcoin/pull/34655

278 ↗(On Diff #59760)

It cannot be fresh-but-not-dirty. iiuc this should always be -1

src/coins.cpp
131 ↗(On Diff #59760)

Also I think the bug is a fuzzer problem, not something that should happen in production code

squash with the relevant defensive programming commit https://github.com/bitcoin/bitcoin/pull/34655/changes/b8fa6f0f701f04cffca6a085337b508381016649

the rest of pr34655 are fuzzer fixes to avoid triggering this underflow, i'll do that backport separately because it depends in pr34165 (we lack CCoinsView::PeekCoin)

This revision is now accepted and ready to land.Jun 15 2026, 07:14