Skip to content

decouple FlushStateToDisk - #6

Draft
yuvicc wants to merge 4 commits into
masterfrom
2026-07-split_flushstatetodisk
Draft

decouple FlushStateToDisk#6
yuvicc wants to merge 4 commits into
masterfrom
2026-07-split_flushstatetodisk

Conversation

@yuvicc

@yuvicc yuvicc commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Motivation

Chainstate::FlushStateToDisk() currently does four separable things in one
body: it decides which block files can be pruned, flushes block and undo files
and writes the block index, writes the coins cache, and reports runtime
failures through a BlockValidationState& out-param.

This came up during review of bitcoin#35621, where AcceptBlock() had to grow a dummy
BlockValidationState so that flush failures would not be misread as block
validation failures. As per discussion:

passing a dummy state means the API does not express the real intent...A
cleaner approach would be to expose a validation-state-free method

and suggested either a wrapper method or splitting FlushStateToDisk() into
smaller composable helpers.

Worth stating explicitly, FlushStateToDisk() does not report block invalidity but runtime error.

gettxoutsetinfo, scantxoutset, dumptxoutset rpc and the rolled-back snapshot
helper all call ForceFlushStateToDisk() for reading CoinsDB() and need the dirty coins written out first.
Getting that via FlushStateToDisk() also gives them an opportunistic prune, an m_next_write timer reset, a ChainStateFlushed notification, and a possible chainstate compaction.

SyncCoinsToDisk() is FlushBlockFilesToDisk() (with nothing to prune)
followed by FlushCoinsCache().
The block/undo files and the block index are still written first, and
deliberately so that the coins cache may refer to block index entries that are so
far only dirty in memory, and syncing the coins without them could leave the
on-disk chainstate pointing at a best block the on-disk block index does not
contain, failing LoadChainTip() on restart after a crash.

@yuvicc
yuvicc force-pushed the 2026-07-split_flushstatetodisk branch from 04225ae to f601e72 Compare July 7, 2026 06:57
yuvicc pushed a commit that referenced this pull request Jul 27, 2026
…AFL fork

faada35 fuzz: [refactor] Use 100'000 digit separator in __AFL_LOOP (MarcoFalke)
fae067e fuzz: Avoid dangling prevoutfetch threads after AFL fork (MarcoFalke)

Pull request description:

  Presumably fixes https://issues.oss-fuzz.com/issues/536943806

  This is a bit confusing, because the issue was already fixed in commit f608a40, by removing the AFL forkserver.

  However, OSS-Fuzz doesn't go through the AFL_LOOP, but through the AFL libFuzzer driver:

  ```
            #0 0x7e055245baab in __pthread_clockjoin_ex /build/glibc-B3wQXB/glibc-2.31/nptl/pthread_join_common.c:89:6
      #1 0x5a27e904dcdd in operator() /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:293:14
      #2 0x5a27e904dcdd in Join<(lambda at /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:292:44)> /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h:75:9
      #3 0x5a27e904dcdd in ___interceptor_pthread_join /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:292:25
      #4 0x5a27e90f4044 in std::__1::thread::join()
      #5 0x5a27e9366277 in ThreadPool::Stop() [bitcoin-core/src/util/threadpool.h:146](https://github.com/bitcoin/bitcoin/blob/7d8137c1417b61589949b3e8b081e5310fe335f3/src/util/threadpool.h#L146):53
      #6 0x5a27e9365db9 in ThreadPool::~ThreadPool() [bitcoin-core/src/util/threadpool.h:94](https://github.com/bitcoin/bitcoin/blob/7d8137c1417b61589949b3e8b081e5310fe335f3/src/util/threadpool.h#L94):9
  ...
            bitcoin#32 0x5a27e95a9506 in (anonymous namespace)::ResetChainman(TestingSetup&) (.12669) [bitcoin-core/src/test/fuzz/process_messages.cpp:44](https://github.com/bitcoin/bitcoin/blob/7d8137c1417b61589949b3e8b081e5310fe335f3/src/test/fuzz/process_messages.cpp#L44):27
      bitcoin#33 0x5a27e95a8c60 in process_messages_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) [bitcoin-core/src/test/fuzz/process_messages.cpp:141](https://github.com/bitcoin/bitcoin/blob/7d8137c1417b61589949b3e8b081e5310fe335f3/src/test/fuzz/process_messages.cpp#L141):9
  ...
              bitcoin#36 0x5a27e97b7190 in test_one_input(std::__1::span<unsigned char const, 18446744073709551615ul>) bitcoin-core/src/test/fuzz/fuzz.cpp:86:5
              bitcoin#37 0x5a27e97b7190 in LLVMFuzzerTestOneInput bitcoin-core/src/test/fuzz/fuzz.cpp:214:5
              bitcoin#38 0x5a27e90ada19 in LLVMFuzzerRunDriver /src/aflplusplus/utils/aflpp_driver/aflpp_driver.c:427:13
              bitcoin#39 0x5a27e90ad69b in main /src/aflplusplus/utils/aflpp_driver/aflpp_driver.c:323:10
              bitcoin#40 0x7e055223b082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/libc-start.c:308:16
              bitcoin#41 0x5a27e8fc602d in _start
  ```

  So the correct fix would be to set `AFL_DRIVER_DONT_DEFER=1`. Ref: https://github.com/AFLplusplus/AFLplusplus/blob/ad5304010ae3be9d5cdc1ba51b09e14169c5cb87/utils/aflpp_driver/aflpp_driver.c#L161

  However, I don't know how to do this on OSS-Fuzz, so just drop the threads for now, because there are dedicated fuzz targets to test the multi-threaded case anyway.

ACKs for top commit:
  l0rinc:
    ACK faada35
  andrewtoth:
    lgtm ACK faada35
  sedited:
    ACK faada35

Tree-SHA512: c249d7267f789084968f8510531f60fc71c9fbd6b4e574a181fd6da1af19a3cb7c03ba60c3ca70244b56ce44c602293517cdb6b0e969b4a2cc9d1afaa49ab0f8
@yuvicc
yuvicc force-pushed the 2026-07-split_flushstatetodisk branch 4 times, most recently from c2e44b6 to 8096a96 Compare August 4, 2026 06:18
@optout21

optout21 commented Aug 4, 2026

Copy link
Copy Markdown

For reference: bitcoin#35646 changes FlushStateToDisk signature to be able to return FatalError only.
(Note: I'm not implying anything about the merits/relationship of the two approaches, only noting the overlap.)

@optout21

optout21 commented Aug 4, 2026

Copy link
Copy Markdown

For reference: bitcoin#35646 changes FlushStateToDisk signature

Oh, my bad, I've overlooked that it's the same author! >D Please ignore

@yuvicc

yuvicc commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

For reference: bitcoin#35646 changes FlushStateToDisk signature

Oh, my bad, I've overlooked that it's the same author! >D Please ignore

Correct, was experimenting out! I feel bit by bit changes looks more mergeable rather than a full refactor!

yuvicc added 4 commits August 5, 2026 09:29
FlushStateToDisk() decides which block files can be pruned, flushes block
and undo data, writes the block index, and writes the coins cache, all in
one body. Move the prune-selection step out first.
Move the block-side write step - the blocks_dir disk space check, the
block and undo file flush, the block index write and the unlinking of
pruned files - into its own method.
Move the coins-side write step, the datadir disk space check and the
Sync()/Flush() call into its own method.
gettxoutsetinfo, scantxoutset, dumptxoutset and the rolled-back snapshot
helper all call ForceFlushStateToDisk(/*wipe_cache=*/false) for a single
reason: they read CoinsDB() directly and need the dirty coins written out
first. Going through FlushStateToDisk(FORCE_SYNC) to get that also gives
them an opportunistic prune, a m_next_write timer reset, a
ChainStateFlushed notification and a possible chainstate compaction, none
of which these callers asked for.

Add a narrow entry point that writes out the coins cache, and use it in
those four places.

The method returns early on a null best block, so it is a no-op on a
fresh datadir. Exceptions are converted into the same fatal "System
error while flushing" notification that FlushStateToDisk() raises.
@yuvicc
yuvicc force-pushed the 2026-07-split_flushstatetodisk branch from 8096a96 to d5dd4f0 Compare August 5, 2026 03:59
yuvicc pushed a commit that referenced this pull request Aug 11, 2026
bb19f1d fuzz: don't connman.ReceiveMsgFrom oversized msg (Greg Sanders)

Pull request description:

  Got a report that tripped it. As this will never happen in p2p, avoid it.

  ```
  INFO: Running with entropic power schedule (0xFF, 100).
  INFO: Seed: 4207564465
  INFO: Loaded 1 modules   (531453 inline 8-bit counters): 531453 [0x5ae1af7ab4c0, 0x5ae1af82d0bd),
  INFO: Loaded 1 PC tables (531453 PCs): 531453 [0x5ae1af82d0c0,0x5ae1b0049090),
  /mnt/scratch0/clusterfuzz/bot/builds/clusterfuzz-builds_bitcoin-core_fcaf00df4dbc83b967efedfb94c0da52db5f507f/revisions/p2p_private_broadcast: Running 1 inputs 100 time(s) each.
  Running: /mnt/scratch0/clusterfuzz/bot/inputs/fuzzer-testcases/crash-7374a74f5b473b761282ebac8029bef22b32b1cd
  p2p_private_broadcast: test/util/net.cpp:93: void ConnmanTestMsg::NodeReceiveMsgBytes(CNode &, std::span<const uint8_t>, bool &) const: Assertion `node.ReceiveMsgBytes(msg_bytes, complete)' failed.
  ==251== ERROR: libFuzzer: deadly signal
      #0 0x5ae1adc5adf4 in __sanitizer_print_stack_trace /src/llvm-project/compiler-rt/lib/ubsan/ubsan_diag_standalone.cpp:31:3
      #1 0x5ae1adbceff8 in fuzzer::PrintStackTrace() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5
      #2 0x5ae1adbb1b45 in fuzzer::Fuzzer::CrashCallback() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:231:3
      #3 0x7e8bc7ef741f in libpthread.so.0
      #4 0x7e8bc7ceb00a in __libc_signal_restore_set /build/glibc-B3wQXB/glibc-2.31/sysdeps/unix/sysv/linux/internal-signals.h:86:3
      #5 0x7e8bc7ceb00a in raise /build/glibc-B3wQXB/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:48:3
      #6 0x7e8bc7cca858 in abort /build/glibc-B3wQXB/glibc-2.31/stdlib/abort.c:79:7
      #7 0x7e8bc7cca728 in __assert_fail_base /build/glibc-B3wQXB/glibc-2.31/assert/assert.c:94:3
      #8 0x7e8bc7cdbfd5 in __assert_fail /build/glibc-B3wQXB/glibc-2.31/assert/assert.c:103:3
      #9 0x5ae1ae9bd015 in NodeReceiveMsgBytes bitcoin-core/src/test/util/net.cpp:93:5
      #10 0x5ae1ae9bd015 in ConnmanTestMsg::ReceiveMsgFrom(CNode&, CSerializedNetMsg&&) const bitcoin-core/src/test/util/net.cpp:119:9
      #11 0x5ae1adfff8f7 in p2p_private_broadcast_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) bitcoin-core/src/test/fuzz/p2p_private_broadcast.cpp:235:27
      bitcoin#12 0x5ae1ae263e23 in operator() /usr/local/include/c++/v1/__functional/function.h:274:12
      bitcoin#13 0x5ae1ae263e23 in operator() /usr/local/include/c++/v1/__functional/function.h:772:10
      bitcoin#14 0x5ae1ae263e23 in test_one_input bitcoin-core/src/test/fuzz/fuzz.cpp:86:5
      bitcoin#15 0x5ae1ae263e23 in LLVMFuzzerTestOneInput bitcoin-core/src/test/fuzz/fuzz.cpp:214:5
      bitcoin#16 0x5ae1adbb322d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13
      bitcoin#17 0x5ae1adb9cf42 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:329:6
      bitcoin#18 0x5ae1adba2e10 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:865:9
      bitcoin#19 0x5ae1adbcf9a2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
      bitcoin#20 0x7e8bc7ccc082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/libc-start.c:308:16
      bitcoin#21 0x5ae1adb9602d in _start
  ```

ACKs for top commit:
  maflcko:
    review ACK bb19f1d 🔱
  sedited:
    ACK bb19f1d
  w0xlt:
    ACK bb19f1d

Tree-SHA512: 8ea61fa04ec02ffd42a1bbd6e8089a4753c43f0c790054cd3f8d7939f1e09972c3d2d0d0f5839055a4d66b632b77de1b5259ea994fdd0298fcd1eaef7518b1e9
@yuvicc
yuvicc marked this pull request as draft August 21, 2026 04:04
@yuvicc
yuvicc marked this pull request as ready for review September 2, 2026 14:00
@yuvicc
yuvicc marked this pull request as draft September 2, 2026 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants