fuzz: keep coins_view fuzzers within caller contracts #34655

pull l0rinc wants to merge 4 commits into bitcoin:master from l0rinc:l0rinc/coins_view_fuzzer_cleanup changing 4 files +37 −56
  1. l0rinc commented at 10:45 am on February 23, 2026: contributor

    Problem

    This is an alternative approach to #34647, fixes #34645.

    Fix

    First, add CheckedSub and use it for decrements of m_dirty_count and cachedCoinsUsage, so unsigned underflows turn into immediate failures instead of silently wrapping and only failing later.

     0util/overflow.h:44 T CheckedSub(const T, const U) [T = unsigned long, U = bool]: Assertion `j <= i' failed.
     1==72817== ERROR: libFuzzer: deadly signal
     2    [#0](/bitcoin-bitcoin/0/) 0x556e9225eab5 in __sanitizer_print_stack_trace (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x191dab5) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
     3    [#1](/bitcoin-bitcoin/1/) 0x556e921acafc in fuzzer::PrintStackTrace() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x186bafc) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
     4    [#2](/bitcoin-bitcoin/2/) 0x556e92191bb7 in fuzzer::Fuzzer::CrashCallback() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1850bb7) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
     5    [#3](/bitcoin-bitcoin/3/) 0x7164cfc458cf  (/lib/x86_64-linux-gnu/libc.so.6+0x458cf) (BuildId: ae7440bbdce614e0e79280c3b2e45b1df44e639c)
     6    [#4](/bitcoin-bitcoin/4/) 0x7164cfca49bb in __pthread_kill_implementation nptl/pthread_kill.c:43:17
     7    [#5](/bitcoin-bitcoin/5/) 0x7164cfca49bb in __pthread_kill_internal nptl/pthread_kill.c:89:10
     8    [#6](/bitcoin-bitcoin/6/) 0x7164cfca49bb in pthread_kill nptl/pthread_kill.c:100:10
     9    [#7](/bitcoin-bitcoin/7/) 0x7164cfc4579d in raise signal/../sysdeps/posix/raise.c:26:13
    10    [#8](/bitcoin-bitcoin/8/) 0x7164cfc288cc in abort stdlib/abort.c:73:3
    11    [#9](/bitcoin-bitcoin/9/) 0x556e92f9d591 in assertion_fail(std::source_location const&, std::basic_string_view<char, std::char_traits<char>>) /mnt/my_storage/bitcoin/src/util/check.cpp:41:5
    12    [#10](/bitcoin-bitcoin/10/) 0x556e9250daf0 in bool&& inline_assertion_check<false, bool>(bool&&, std::source_location const&, std::basic_string_view<char, std::char_traits<char>>) /mnt/my_storage/bitcoin/src/util/check.h:90:13
    13    [#11](/bitcoin-bitcoin/11/) 0x556e9250daf0 in unsigned long CheckedSub<unsigned long, bool>(unsigned long, bool) /mnt/my_storage/bitcoin/src/util/overflow.h:44:5
    14    [#12](/bitcoin-bitcoin/12/) 0x556e9250daf0 in CoinsViewCacheCursor::NextAndMaybeErase(std::pair<COutPoint const, CCoinsCacheEntry>&) /mnt/my_storage/bitcoin/src/coins.h:282:25
    15    [#13](/bitcoin-bitcoin/13/) 0x556e92507eb2 in (anonymous namespace)::MutationGuardCoinsViewCache::BatchWrite(CoinsViewCacheCursor&, uint256 const&) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:90:75
    16    [#14](/bitcoin-bitcoin/14/) 0x556e92c17a2b in CCoinsViewCache::Flush(bool) /mnt/my_storage/bitcoin/src/coins.cpp:282:11
    17    [#15](/bitcoin-bitcoin/15/) 0x556e924fb732 in TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1::operator()() const /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:135:34
    18    [#16](/bitcoin-bitcoin/16/) 0x556e924fb732 in unsigned long CallOneOf<TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_10, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_11>(FuzzedDataProvider&, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_10, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_11) /mnt/my_storage/bitcoin/src/test/fuzz/util.h:42:27
    19    [#17](/bitcoin-bitcoin/17/) 0x556e924fb732 in TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:114:9
    20    [#18](/bitcoin-bitcoin/18/) 0x556e92503b0c in coins_view_overlay_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:404:5
    21    [#19](/bitcoin-bitcoin/19/) 0x556e92bcb7a5 in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/lib/gcc/x86_64-linux-gnu/15/../../../../include/c++/15/bits/std_function.h:593:9
    22    [#20](/bitcoin-bitcoin/20/) 0x556e92bcb7a5 in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /mnt/my_storage/bitcoin/src/test/fuzz/fuzz.cpp:88:5
    23    [#21](/bitcoin-bitcoin/21/) 0x556e92bcb7a5 in LLVMFuzzerTestOneInput /mnt/my_storage/bitcoin/src/test/fuzz/fuzz.cpp:216:5
    24    [#22](/bitcoin-bitcoin/22/) 0x556e9219318f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x185218f) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    25    [#23](/bitcoin-bitcoin/23/) 0x556e92192799 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1851799) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    26    [#24](/bitcoin-bitcoin/24/) 0x556e92194139 in fuzzer::Fuzzer::MutateAndTestOne() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1853139) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    27    [#25](/bitcoin-bitcoin/25/) 0x556e92194c95 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1853c95) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    28    [#26](/bitcoin-bitcoin/26/) 0x556e92181255 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1840255) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    29    [#27](/bitcoin-bitcoin/27/) 0x556e921ad696 in main (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x186c696) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    30    [#28](/bitcoin-bitcoin/28/) 0x7164cfc2a577 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    31    [#29](/bitcoin-bitcoin/29/) 0x7164cfc2a63a in __libc_start_main csu/../csu/libc-start.c:360:3
    32    [#30](/bitcoin-bitcoin/30/) 0x556e921757e4 in _start (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x18347e4) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
    33
    34NOTE: libFuzzer has rudimentary signal handlers.
    35      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
    36SUMMARY: libFuzzer: deadly signal
    37MS: 2 PersAutoDict-CopyPart- DE: "\005\000"-; base unit: ecb626aff8724f0fdde38a0a6965718f2096d474
    38artifact_prefix='/tmp/fuzz_artifacts/'; Test unit written to /tmp/fuzz_artifacts/crash-1d19026c1a23f08bfe693fd684a56ce51187c6e5
    39./build_fuzz/bin/fuzz /tmp/fuzz_corpus/coins_view_overlay -max_total_time=3600 -rss_limit_mb=2560 -artifact_prefix=/tmp/fuzz_artifacts/ >fuzz-16.log 2>&1
    

    The coins view fuzz targets can call AddCoin/AddCoins and construct BatchWrite cursors in ways that violate CCoinsViewCache caller contracts. These invalid states can trigger BatchWrite std::logic_error and can desync dirty-entry accounting (caught by Assume(m_dirty_count == 0) currently).

    Make the fuzzer avoid generating invalid states instead of catching and resetting:

    • Only use AddCoins(check=false) when we have confirmed the txid has no unspent outputs; otherwise fall back to check=true so AddCoins determines overwrites via the view.
    • When constructing a CoinsViewCacheCursor, avoid setting FRESH when the parent already has an unspent coin, and ensure FRESH implies DIRTY.

    Fuzzing

    The original error could be reproduced in ~10 minutes using coins_view_overlay. I ran the coins_view, coins_view_db, coins_view_overlay, and coinscache_sim fuzzers for this PR overnight and they didn’t fail anymore.

  2. DrahtBot added the label Fuzzing on Feb 23, 2026
  3. DrahtBot commented at 10:46 am on February 23, 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 sipa, achow101, andrewtoth

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34124 (refactor: make CCoinsView a pure virtual interface by l0rinc)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. in src/util/overflow.h:38 in a8e6c5ff43
    31@@ -31,6 +32,13 @@ template <class T>
    32     return i + j;
    33 }
    34 
    35+template <std::unsigned_integral T, std::unsigned_integral U>
    36+[[nodiscard]] constexpr T CheckedSub(const T i, const U j) noexcept
    37+{
    38+    Assume(i >= T{j});
    


    maflcko commented at 11:42 am on February 23, 2026:
    not sure we want subtraction return a different layout than addition (one returns option, the other does not). Same for left-shift. See also https://doc.rust-lang.org/std/primitive.u64.html#method.checked_sub

    l0rinc commented at 11:59 am on February 23, 2026:
    How would we use that in prod, though? The current calls are not critical, and in production we would not want to crash.

    maflcko commented at 1:11 pm on February 23, 2026:
    Not sure. Maybe put the Assume in the source code directly? An alternative could be to come up with a different name, so that similarly named functions are not behaving completely different.

    sipa commented at 1:55 pm on February 23, 2026:
    It’s simple enough that just an Assume(x >= y); x -= y; inline seems fine.

    l0rinc commented at 2:07 pm on February 23, 2026:

    If you mean we don’t need the T{j} cast, that broke for me on windows:

    0"D:\a\bitcoin\bitcoin\build\ALL_BUILD.vcxproj" (default target) (1) ->
    1"D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj" (default target) (5) ->
    2"D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj" (default target) (12) ->
    3D:\a\bitcoin\bitcoin\src\util\overflow.h(44,5): warning C4804: '<=': unsafe use of type 'bool' in operation [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
    

    sipa commented at 2:16 pm on February 23, 2026:
    If that’s in response to my comment, I meant avoiding CheckSub and just having the Assume()s at the call site like @maflcko suggested.

    l0rinc commented at 2:27 pm on February 23, 2026:
    I think I can do better than that, lemme’ try something…

    l0rinc commented at 2:59 pm on February 23, 2026:
    The result is cleaner than before, thanks for the suggestions, added you both as coauthors. It’s called TrySub now and it modifies the first param and returns a bool allowing the call sites to Assume cleanly. This avoids the previous repetition and brings the assertions to the call-site.
  5. in src/test/fuzz/coins_view.cpp:286 in 6ce4633969 outdated
    290-                    if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) {
    291-                        assert(!possible_overwrite);
    292-                        expected_code_path = true;
    293+                const bool check_for_overwrite{transaction.IsCoinBase() || fuzzed_data_provider.ConsumeBool() || [&] {
    294+                    for (uint32_t i{0}; i < transaction.vout.size(); ++i) {
    295+                        if (coins_view_cache.PeekCoin(COutPoint{transaction.GetHash(), i})) return true;
    


    sipa commented at 1:35 pm on February 23, 2026:
    It’s slightly unfortunate that this requires correctness of PeekCoin in order for the test to be meaningful; ideally the fuzzing harness itself knows whether it’s a potential overwrite. However, since coinscache_sim already uses such an approach, this seems fine here.

    l0rinc commented at 3:00 pm on February 23, 2026:
  6. in src/test/fuzz/coins_view.cpp:202 in a8e6c5ff43
    197@@ -211,26 +198,20 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
    198                         }
    199                         coins_cache_entry.coin = *opt_coin;
    200                     }
    201+                    // Avoid setting FRESH for an outpoint that already exists unspent in the parent view.
    202+                    bool fresh{fuzzed_data_provider.ConsumeBool() && !coins_view_cache.PeekCoin(random_out_point)};
    


    sipa commented at 1:37 pm on February 23, 2026:
    Swap these two conditions so that no fuzz input is consumed when PeekCoin returns false?

    l0rinc commented at 3:00 pm on February 23, 2026:
    I assumed the fuzzer is less expensive, but it makes sense not to burn bytes on a decision that can’t matter. Switched in the other cases, too.
  7. util: introduce `TrySub` to prevent unsigned underflow
    Introduce `TrySub(T&, U)` which subtracts an unsigned integral `U` from an unsigned integral `T`, returning `false` on underflow.
    Use with `Assume(TrySub(...))` at coins cache accounting decrement sites so invariant violations fail immediately rather than silently wrapping.
    
    Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    b8fa6f0f70
  8. fuzz: make `AddCoins` query view for overwrites
    In validation, `AddCoins(check_for_overwrite=false)` is only used after BIP30 has already ensured the transaction does not overwrite any unspent outputs in the UTXO view.
    The coins view fuzz target can call `AddCoins` with arbitrary txids, so using the `check_for_overwrite=false` fast path on non-coinbase transactions may violate the `AddCoin` caller contract and trigger logic errors.
    Only use `check_for_overwrite=false` when we have first confirmed that none of the outputs are currently unspent.
    Otherwise, fall back to `check_for_overwrite=true` so `AddCoins` determines overwrites via the view.
    d7e0d510f2
  9. fuzz: avoid invalid `AddCoin` overwrites
    The coins view fuzzer can call `AddCoin` with `possible_overwrite=false` for an outpoint that already exists unspent in the view, which violates the `AddCoin` caller contract.
    Derive `possible_overwrite` from `PeekCoin` so `possible_overwrite=false` is only used when the outpoint is absent.
    This matches the approach used by the `coinscache_sim` fuzzer, which derives the overwrite flag from simulated state.
    780f460635
  10. fuzz: prevent invalid `FRESH` entries and surface `BatchWrite` errors
    Modify fuzzer logic to avoid setting `FRESH` for an outpoint that already exists unspent in the parent view, and ensure `FRESH` implies `DIRTY`.
    This keeps cursor invariants realistic and lets `BatchWrite` failures expose real bugs without resetting state.
    3281824ecf
  11. l0rinc force-pushed on Feb 23, 2026
  12. DrahtBot added the label CI failed on Feb 23, 2026
  13. in src/util/overflow.h:35 in b8fa6f0f70
    30@@ -31,6 +31,14 @@ template <class T>
    31     return i + j;
    32 }
    33 
    34+template <std::unsigned_integral T, std::unsigned_integral U>
    35+[[nodiscard]] constexpr bool TrySub(T& i, const U j) noexcept
    


    sipa commented at 8:25 pm on February 23, 2026:

    Very nitty, but:

    • I think the name is slightly misleading, as there is no reason why subtracting would ever fail on an unsigned type, so it is not clear what is being “tried”. The name should suggest that it avoids wraparound.
    • It’s unfortunate that this has a runtime impact even outside fuzzing. It’s obviously fine within the context of this PR (nothing in it is that performance critical), but in that context just Assume(x >= y); x -= y; is fine too. If we’re introducing this as a new generic utility function that’s usable anywhere, I think bool ret = i >= T{j}; i -= T{j}; return ret; is nicer, because in contexts where the return value is ignored, it is exactly equivalent to just a subtraction, and may be optimized to it.

    l0rinc commented at 12:05 pm on February 24, 2026:

    no reason why subtracting would ever fail on an unsigned type

    TrySub is meant as “try to subtract without wraparound”: if i < j, it leaves i unchanged (hence the Try prefix) and returns false, so callers can act.

    It’s unfortunate that this has a runtime impact even outside fuzzing

    I think bool ret = i >= T{j}; i -= T{j}; return ret; is nicer, because in contexts where the return value is ignored, it is exactly equivalent to just a subtraction

    Do we want to be exactly equivalent though? The current cheap branching prevents the accounting counters from wrapping, so even if accounting is wrong elsewhere, the values stay closer to what we expect.


    I have tried a few variants to see the effect of the change (plain subtraction vs return + internal Assume vs mutating TrySub + caller Assume with [[unlikely]]), see https://godbolt.org/z/nf9nT9Mjj

    Both compilers emit the same single subtract (plus the bool zero-extend in the bool case). For example, GCC:

    0sub_return_size:
    1    sub     QWORD PTR [rdi], rsi
    2    ret
    

    and for bool:

    0sub_return_bool:
    1    movzx   esi, sil
    2    sub     QWORD PTR [rdi], rsi
    3    ret
    

    So return + internal Assume has zero codegen cost in non-abort builds, and adding [[unlikely]] doesn’t seem to change anything there either.


    Result 2: Only the mutating TrySub variant changes semantics (avoids wrap), and it necessarily adds a branch. [[unlikely]] didn’t change codegen here either. GCC (size_t case):

    0sub_trysub_size:
    1    mov     rax, QWORD PTR [rdi]
    2    cmp     rax, rsi
    3    jb      .Lskip
    4    sub     rax, rsi
    5    mov     QWORD PTR [rdi], rax
    6.Lskip:
    7    ret
    

    clang emits the same idea, just as sub then jb then store:

    0sub_trysub_size:
    1    mov     rax, qword ptr [rdi]
    2    sub     rax, rsi
    3    jb      .Lskip
    4    mov     qword ptr [rdi], rax
    5.Lskip:
    6    ret
    

    …and the sub_trysub_unlikely_* versions were identical to sub_trysub_* for both compilers (so [[unlikely]] doesn’t seem to buy anything here either; the cold path is already the taken jump).

    Note: I have experimented a bit with __builtin_sub_overflow(), it does change GCC codegen slighly to the same sub; jb shape clang already emits (removing the explicit cmp), but I’m not sure it’s an actual win, so I’ll leave it for a follow-up.


    sipa commented at 1:58 pm on February 24, 2026:

    (this is all very nitty, and no reason to hold up this PR)

    TrySub is meant as “try to subtract without wraparound”: if i < j, it leaves i unchanged (hence the Try prefix) and returns false, so callers can act.

    Yes, I understand what it does. My point is that this is not clear from the name “TrySub”, because it doesn’t suggest anything about avoiding wrapping (which is perfectly well-defined behavior on unsigned types).

    Do we want to be exactly equivalent though?

    If our assumptions are wrong, I don’t think we care about what actually happens to the m_dirty_count value, it’s going to be meaningless regardless. So I would suggest using whatever is fastest, and I’m guessing that x -= y; is faster than if (x >= y) x -= y;.

  14. DrahtBot removed the label CI failed on Feb 23, 2026
  15. sipa commented at 9:49 pm on February 23, 2026: member
    ACK 3281824ecfa72c4f69ab69c94003b7f5a82c7265. Ran the 4 relevant fuzz tests for ~1 CPU day each. Will run more overnight.
  16. achow101 commented at 11:27 pm on February 23, 2026: member
    ACK 3281824ecfa72c4f69ab69c94003b7f5a82c7265
  17. in src/test/fuzz/coins_view.cpp:203 in 3281824ecf
    197@@ -215,26 +198,20 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
    198                         }
    199                         coins_cache_entry.coin = *opt_coin;
    200                     }
    201+                    // Avoid setting FRESH for an outpoint that already exists unspent in the parent view.
    202+                    bool fresh{!coins_view_cache.PeekCoin(random_out_point) && fuzzed_data_provider.ConsumeBool()};
    203+                    bool dirty{fresh || fuzzed_data_provider.ConsumeBool()};
    


    andrewtoth commented at 2:45 pm on February 24, 2026:

    nit: I think this is backwards. fresh is dependent on dirty, not vice versa. See https://github.com/bitcoin/bitcoin/pull/33018/changes/797fcc4eac6cf218deb8ef0a22af0887da29ed71.

    I also don’t think this change to dirty is relevant to the fix here. We only need to gate fresh on PeekCoin. The exception being thrown is not relevant to FRESH-but-not-DIRTY. It’s actually reducing coverage here.


    l0rinc commented at 2:59 pm on February 24, 2026:
    I don’t have the capacity at the moment to react to this in detail. Is this important to do it here, or do you think we can tackle it independently in #33018?

    andrewtoth commented at 3:12 pm on February 24, 2026:
    This isn’t a blocker. The change here fixes the linked issue.
  18. andrewtoth approved
  19. andrewtoth commented at 2:48 pm on February 24, 2026: contributor

    ACK 3281824ecfa72c4f69ab69c94003b7f5a82c7265

    Fuzzed coins_view_overlay overnight with no issues.

  20. achow101 added this to the milestone 31.0 on Feb 24, 2026
  21. achow101 merged this on Feb 24, 2026
  22. achow101 closed this on Feb 24, 2026


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-03-03 00:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me