coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`

l0rinc commented at 5:33 pm on April 20, 2025: contributor

Split out of #32296 (comment) since this needed more complicated production code changes.

The changes ensure cachedCoinsUsage remains balanced throughout all coin operations and that the sanitizers will catch future violations.

You can recreate the previous failures as described in #32313 (review):

MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" ./ci/test_run_all.sh

The change was tested with the related fuzz test, and asserted before/after each cachedCoinsUsage change (in production code and fuzz) that the calculations are still correct by recalculating it from scratch.

0bool CCoinsViewCache::CacheUsageValid() const
1{
2    size_t actual{0};
3    for (auto& entry : cacheCoins | std::views::values) actual += entry.coin.DynamicMemoryUsage();
4    return actual == cachedCoinsUsage;
5}

This part wasn’t committed to the code.

DrahtBot commented at 5:33 pm on April 20, 2025: 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/32313.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32602 (fuzz: Add target for coins database by marcofleon)
#31132 (validation: fetch block inputs on parallel threads 10% faster IBD by andrewtoth)

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.

in src/coins.cpp:82 in 4bd4805217 outdated

75@@ -76,13 +76,13 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
76     bool inserted;
77     std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>());
78     bool fresh = false;
79+    if (!possible_overwrite && !it->second.coin.IsSpent()) { // check before any other partial state change
80+        throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)");
81+    }
82     if (!inserted) {

andrewtoth commented at 5:36 pm on April 20, 2025:

Would it be simpler to just move this if block below the if block that throws, instead of pulling the throw out?

l0rinc commented at 8:57 pm on April 20, 2025:

If you think that’s better, I don’t mind - done

in src/coins.cpp:115 in 4bd4805217 outdated

114-    cachedCoinsUsage += coin.DynamicMemoryUsage();
115-    auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint), std::move(coin));
116-    if (inserted) CCoinsCacheEntry::SetDirty(*it, m_sentinel);
117+void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin)
118+{
119+    auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint));

andrewtoth commented at 5:41 pm on April 20, 2025:

I think this would be simpler if we kept the original behavior of moving the coin here as well, and then only increment the cachedCoinsUsage if the coin is inserted.

In practice it’s not possible to get !inserted unless the assume utxo payload was generated incorrectly.

l0rinc commented at 8:57 pm on April 20, 2025:

In practice it’s not possible to get !inserted unless the assume utxo payload was generated incorrectly.

As far as I can see that’s not a guarantee, but I’ve also noted it in the commit message that currently this is the case.

kept the original behavior of moving the coin here as well

Sure, if you think that’s better - changed!

in src/coins.cpp:204 in 4bd4805217 outdated

200@@ -195,6 +201,7 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
201                 // and mark it as dirty.
202                 itUs = cacheCoins.try_emplace(it->first).first;
203                 CCoinsCacheEntry& entry{itUs->second};
204+                cachedCoinsUsage -= entry.coin.DynamicMemoryUsage();

andrewtoth commented at 5:43 pm on April 20, 2025:

The entry has an empty coin here, it was just created. So this is just subtracting zero.

l0rinc commented at 8:57 pm on April 20, 2025:

Thanks, I’ll add an assert in that case to explain why this branch wasn’t symmetric with the other one.

andrewtoth commented at 11:23 pm on May 5, 2025:

I don’t think we should add an assert here to explain. The comment right above says Create the coin in the parent cache.

l0rinc commented at 7:14 am on May 6, 2025:

Doesn’t the presence of the original cache-usage-update indicate that the comment isn’t enough?

andrewtoth commented at 2:05 pm on May 6, 2025:

Sorry, what is the original cache-usage-update? AFAICT this line was added in this PR, and there is no cache-usage-update here?

l0rinc commented at 2:30 pm on May 6, 2025:

I prefer code over comments (the comment can be wrong, the code validates) - and since the usual pattern is basically:

0cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
1if (cursor.WillErase(*it)) {
2    itUs->second.coin = std::move(it->second.coin);
3} else {
4    itUs->second.coin = it->second.coin;
5}
6cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();

it makes sense to explain (via code) why here we don’t need the first part:

0assert(entry.coin.DynamicMemoryUsage() == 0); // i.e. we didn't forget the symmetry, it's on purpose
1if (cursor.WillErase(*it)) {
2    entry.coin = std::move(it->second.coin);
3} else {
4    entry.coin = it->second.coin;
5}
6cachedCoinsUsage += entry.coin.DynamicMemoryUsage();

Do you still think we don’t need it?

l0rinc force-pushed on Apr 20, 2025

l0rinc renamed this:
~~RFC: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`~~
coins: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`
on Apr 20, 2025

DrahtBot added the label UTXO Db and Indexes on Apr 20, 2025

l0rinc marked this as ready for review on Apr 20, 2025

DrahtBot added the label CI failed on Apr 28, 2025

l0rinc commented at 9:50 am on April 29, 2025: contributor

Rebased to allow CI running the new tests.

l0rinc force-pushed on Apr 29, 2025

l0rinc renamed this:
~~coins: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`~~
coins: fix `cachedCoinsUsage` usages in `CCoinsViewCache`
on Apr 29, 2025

l0rinc renamed this:
~~coins: fix `cachedCoinsUsage` usages in `CCoinsViewCache`~~
coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`
on Apr 29, 2025

DrahtBot removed the label CI failed on Apr 29, 2025

coins: check unspent‑overwrite before `cachedCoinsUsage` change in `AddCoin` 327ee453ca

coins: assert just-created-coin has no dynamic memory usage

This was added for the two branches to be symmetric and to explain why this branch is missing a subtraction.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

e20a1d9c72

coins: spent erase in `CoinsViewCacheCursor` must not change usage

Assert that spent coins already have zero dynamic size, and remove unused leftover counter

9e7fb3c0f4

coins: reset `cachedCoinsUsage` in Flush() only after the map is cleared

Prevents the counter from dropping to 0 while cacheCoins still holds objects

a0b520273f

coins: only adjust `cachedCoinsUsage` on `EmplaceCoinInternalDANGER` inserts

Guarantees counter stays balanced both on insert and on in‑place replacement.
Note that this is currently only called from AssumeUTXO code where it should only insert.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

6a1c930e3a

fuzz: drop leftover UBSan suppressions from `CCoinsViewCache`

These were fixed im previous commits:
    INFO: Seed: 3018625440
    INFO: Loaded 1 modules   (641435 inline 8-bit counters): 641435 [0x5810b64ebb78, 0x5810b6588513),
    INFO: Loaded 1 PC tables (641435 PCs): 641435 [0x5810b6588518,0x5810b6f51ec8),
    INFO:     1859 files found in /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/coins_view
    INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1005297 bytes
    INFO: seed corpus: files: 1859 min: 2b max: 1005297b total: 48403597b rss: 111Mb
    /ci_container_base/src/coins.cpp:133:22: runtime error: unsigned integer overflow: 0 - 64 cannot be represented in type 'size_t' (aka 'unsigned long')
        #0 0x5810b3768b67 in CCoinsViewCache::SpendCoin(COutPoint const&, Coin*) /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/./coins.cpp:133:22
        #1 0x5810b3095ff0 in coins_view_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>)::$_4::operator()() const /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz
    /./test/fuzz/coins_view.cpp:87:40
    ...
    SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /ci_container_base/src/coins.cpp:133:22

0feba07ff2

in test/sanitizer_suppressions/ubsan:51 in a1fe87eb66 outdated

45@@ -46,9 +46,6 @@ unsigned-integer-overflow:CRollingBloomFilter::insert
46 unsigned-integer-overflow:RollingBloomHash
47 unsigned-integer-overflow:CCoinsViewCache::AddCoin
48 unsigned-integer-overflow:CCoinsViewCache::BatchWrite
49-unsigned-integer-overflow:CCoinsViewCache::DynamicMemoryUsage
50-unsigned-integer-overflow:CCoinsViewCache::SpendCoin
51-unsigned-integer-overflow:CCoinsViewCache::Uncache

maflcko commented at 8:46 pm on April 29, 2025:

it would be better to file this separately and explain why it isn’t needed (or rather since when)

l0rinc commented at 7:54 am on April 30, 2025:

They were introduced in https://github.com/bitcoin/bitcoin/pull/28865/files#diff-bca63b54ad7fcc2df12f3237fd8775810c1b849c49110f702920655778fcc248R45-R49, I don’t see any concrete explanation for what the problem was - and couldn’t reproduce any fuzzing problem when I’ve removed these. @fanquake, can you help us out here, do you remember why these were added?

l0rinc commented at 12:51 pm on April 30, 2025:

I ran the following on Linux for a1fe87eb668e8a1eeeef335951547882b5fb52d8:

0cmake --preset=libfuzzer && cmake --build build_fuzz && \
1  FUZZ=coin_grinder build_fuzz/bin/fuzz -runs=500000 && \
2  FUZZ=coin_grinder_is_optimal build_fuzz/bin/fuzz -runs=500000 && \
3  FUZZ=coincontrol build_fuzz/bin/fuzz -runs=500000 && \
4  FUZZ=coins_deserialize build_fuzz/bin/fuzz -runs=500000 && \
5  FUZZ=coins_view build_fuzz/bin/fuzz -runs=500000 && \
6  FUZZ=coinscache_sim build_fuzz/bin/fuzz -runs=500000 && \
7  FUZZ=coinselection_bnb build_fuzz/bin/fuzz -runs=500000 && \
8  FUZZ=coinselection_knapsack build_fuzz/bin/fuzz -runs=500000 && \
9  FUZZ=coinselection_srd build_fuzz/bin/fuzz -runs=500000

They all passed (actually ran a few until -runs=1000000 but got bored at the end). Am I missing anything important here?

maflcko commented at 1:31 pm on April 30, 2025:

Am I missing anything important here?

Yes, the preset doesn’t check for those. You’ll have to run with the integer sanitizer to be able to detect integer sanitizer issues.

The testing steps are also explained in the comment: #28865#issue-1991140953

l0rinc commented at 9:15 pm on April 30, 2025:

As mentioned in #32296 (comment), I can’t always reproduce these locally, it’s why I used the CI to verify why these were needed in the first place. I have tried every combination I could think of locally, I couldn’t reproduce any such errors in coins. If you can, please let me know how to do that exactly. Is there any other CI run I should be looking at to see if there are other failures?

maflcko commented at 5:36 pm on May 4, 2025:

The CI should be deterministic and trivially reproducible, as explained in https://github.com/bitcoin/bitcoin/blob/master/ci/README.md. If you can’t reproduce it, please share the exact steps you tried, starting from a fresh system. For example, you can start with a fresh Hetzner Cloud VM (or any other fresh VM of your choice).

The integer sanitizer should equally be deterministic and reproducible.

l0rinc commented at 5:54 pm on May 4, 2025:

I meant I couldn’t reproduce any of the supposed failures that suppressions were meant to avoid - but neither is the CI, so I’m not sure what to explain about the removals: they don’t seem to be needed for CI to pass. Unless there’s a secret CI which shows these failures - or any other way that you can reproduce them, please let me know how to do it.

maflcko commented at 6:17 pm on May 4, 2025:

In #32313 (review) I was saying it would be good to known since when they are unused. To test an earlier commit, you can just switch to it in git.

l0rinc commented at 8:20 pm on May 4, 2025:

Whenever I tried going back to pre-cmake times, I couldn’t get it to compile and run again (on Mac or on Linux). Not the first time I try to do a git bisect, but it always turns bad really quickly.

I tried pushing a PR to my fork with the original PR that introduced the suppressions + removing DynamicMemoryUsage, SpendCoin and Uncache suppressionss from ubsan. The build is failing with similar problems that I see locally but don’t see any unsigned-integer-overflow complaints. #28865 https://github.com/l0rinc/bitcoin/pull/10

Can you give me a hint of what else to try?

fanquake commented at 1:41 pm on May 9, 2025:

They were introduced in https://github.com/bitcoin/bitcoin/pull/28865/files#diff-bca63b54ad7fcc2df12f3237fd8775810c1b849c49110f702920655778fcc248R45-R49, @fanquake, can you help us out here, do you remember why these were added?

You can recreate the failures by checking out to that commit (fd30e9688e15fe6e0f8b64202a6e9c7d96333394), applying this diff (which includes the supression being done here):

 0diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh
 1index 3585b2b417..ce45917e99 100755
 2--- a/ci/test/00_setup_env_native_fuzz.sh
 3+++ b/ci/test/00_setup_env_native_fuzz.sh
 4@@ -6,7 +6,7 @@
 5 
 6 export LC_ALL=C.UTF-8
 7 
 8-export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10"  # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version).
 9+export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"  # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version).
10 export CONTAINER_NAME=ci_native_fuzz
11 export PACKAGES="clang-17 llvm-17 libclang-rt-17-dev libevent-dev libboost-dev libsqlite3-dev"
12 export NO_DEPENDS=1
13diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan
14index 63d2215aab..d3fc2a59ff 100644
15--- a/test/sanitizer_suppressions/ubsan
16+++ b/test/sanitizer_suppressions/ubsan
17@@ -42,11 +42,6 @@ unsigned-integer-overflow:arith_uint256.h
18 unsigned-integer-overflow:CBloomFilter::Hash
19 unsigned-integer-overflow:CRollingBloomFilter::insert
20 unsigned-integer-overflow:RollingBloomHash
21-unsigned-integer-overflow:CCoinsViewCache::AddCoin
22-unsigned-integer-overflow:CCoinsViewCache::BatchWrite
23-unsigned-integer-overflow:CCoinsViewCache::DynamicMemoryUsage
24-unsigned-integer-overflow:CCoinsViewCache::SpendCoin
25-unsigned-integer-overflow:CCoinsViewCache::Uncache
26 unsigned-integer-overflow:CompressAmount
27 unsigned-integer-overflow:DecompressAmount
28 unsigned-integer-overflow:crypto/

and running the native_fuzz CI job (on aarch64):

 0time env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'MAKEJOBS="-j17" FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" ./ci/test_run_all.sh'
 1<snip>
 2[#170921](/bitcoin-bitcoin/170921/)	REDUCE cov: 1694 ft: 6197 corp: 478/22Kb lim: 240 exec/s: 3351 rss: 208Mb L: 20/219 MS: 1 EraseBytes-
 3[#171261](/bitcoin-bitcoin/171261/)	REDUCE cov: 1694 ft: 6200 corp: 479/22Kb lim: 240 exec/s: 3358 rss: 208Mb L: 193/219 MS: 5 ChangeBinInt-ChangeBit-ChangeByte-CrossOver-CrossOver-
 4[#171550](/bitcoin-bitcoin/171550/)	REDUCE cov: 1694 ft: 6200 corp: 479/22Kb lim: 240 exec/s: 3363 rss: 208Mb L: 42/219 MS: 4 CopyPart-ChangeByte-PersAutoDict-EraseBytes- DE: "\024\000"-
 5[#171657](/bitcoin-bitcoin/171657/)	REDUCE cov: 1694 ft: 6200 corp: 479/22Kb lim: 240 exec/s: 3365 rss: 208Mb L: 51/219 MS: 2 InsertByte-EraseBytes-
 6coins.cpp:132:22: runtime error: unsigned integer overflow: 0 - 64 cannot be represented in type 'size_t' (aka 'unsigned long')
 7    [#0](/bitcoin-bitcoin/0/) 0xaaaab73bc134 in CCoinsViewCache::SpendCoin(COutPoint const&, Coin*) src/coins.cpp:132:22
 8    [#1](/bitcoin-bitcoin/1/) 0xaaaab57cbc94 in coins_view_fuzz_target(Span<unsigned char const>)::$_4::operator()() const src/test/fuzz/coins_view.cpp:89:40
 9    [#2](/bitcoin-bitcoin/2/) 0xaaaab57cbc94 in unsigned long CallOneOf<coins_view_fuzz_target(Span<unsigned char const>)::$_0, coins_view_fuzz_target(Span<unsigned char const>)::$_1, coins_view_fuzz_target(Span<unsigned char const>)::$_2, coins_view_fuzz_target(Span<unsigned char const>)::$_3, coins_view_fuzz_target(Span<unsigned char const>)::$_4, coins_view_fuzz_target(Span<unsigned char const>)::$_5, coins_view_fuzz_target(Span<unsigned char const>)::$_6, coins_view_fuzz_target(Span<unsigned char const>)::$_7, coins_view_fuzz_target(Span<unsigned char const>)::$_8, coins_view_fuzz_target(Span<unsigned char const>)::$_9, coins_view_fuzz_target(Span<unsigned char const>)::$_10>(FuzzedDataProvider&, coins_view_fuzz_target(Span<unsigned char const>)::$_0, coins_view_fuzz_target(Span<unsigned char const>)::$_1, coins_view_fuzz_target(Span<unsigned char const>)::$_2, coins_view_fuzz_target(Span<unsigned char const>)::$_3, coins_view_fuzz_target(Span<unsigned char const>)::$_4, coins_view_fuzz_target(Span<unsigned char const>)::$_5, coins_view_fuzz_target(Span<unsigned char const>)::$_6, coins_view_fuzz_target(Span<unsigned char const>)::$_7, coins_view_fuzz_target(Span<unsigned char const>)::$_8, coins_view_fuzz_target(Span<unsigned char const>)::$_9, coins_view_fuzz_target(Span<unsigned char const>)::$_10) src/./test/fuzz/util.h:43:27
10    [#3](/bitcoin-bitcoin/3/) 0xaaaab57cbc94 in coins_view_fuzz_target(Span<unsigned char const>) src/test/fuzz/coins_view.cpp:58:9
11    [#4](/bitcoin-bitcoin/4/) 0xaaaab5d2319c in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
12    [#5](/bitcoin-bitcoin/5/) 0xaaaab5d2319c in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5
13    [#6](/bitcoin-bitcoin/6/) 0xaaaab5545050 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x1925050) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
14    [#7](/bitcoin-bitcoin/7/) 0xaaaab5544798 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x1924798) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
15    [#8](/bitcoin-bitcoin/8/) 0xaaaab5545f90 in fuzzer::Fuzzer::MutateAndTestOne() (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x1925f90) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
16    [#9](/bitcoin-bitcoin/9/) 0xaaaab5546cdc in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x1926cdc) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
17    [#10](/bitcoin-bitcoin/10/) 0xaaaab5534e70 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x1914e70) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
18    [#11](/bitcoin-bitcoin/11/) 0xaaaab555e0ac in main (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x193e0ac) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
19    [#12](/bitcoin-bitcoin/12/) 0xffffb4aa84c0  (/lib/aarch64-linux-gnu/libc.so.6+0x284c0) (BuildId: 1d7249a4f207d07166ff4be43acdc68a01faaa04)
20    [#13](/bitcoin-bitcoin/13/) 0xffffb4aa8594 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x28594) (BuildId: 1d7249a4f207d07166ff4be43acdc68a01faaa04)
21    [#14](/bitcoin-bitcoin/14/) 0xaaaab552afac in _start (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x190afac) (BuildId: 82e180570288de2f70ceeee1ed5d7cd3729696ab)
22
23SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow coins.cpp:132:22 in 
24MS: 1 CopyPart-; base unit: 81111e927920a57969bb7efb23d89de00d09a5ec
250xff,0xef,0xfe,0x0,0x1,0x3,0x0,0x1,0xff,0xff,0xff,0xff,0xff,0x3,0x0,0x1,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xfe,0xff,0xfa,0x0,0x0,0x0,0x0,0x80,0x76,0x0,0x0,0x0,0x0,0x68,0x5c,0xff,0xff,0xff,0xff,0xef,0xfe,0x0,0x1,0x3,0x0,0x1,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xfe,0xff,0xfa,0x0,0x0,0x0,0x0,0x80,0x76,0x0,0x0,0x0,0x0,0x68,0x5c,0xff,0xff,0xff,0xff,0xef,0xfe,0x0,0x1,0x3,0x0,0x1,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xfe,0xff,0xfa,0x7,
26\377\357\376\000\001\003\000\001\377\377\377\377\377\003\000\001\377\377\377\377\377\377\377\003\377\377\377\377\377\377\377\377\377\377\376\377\372\000\000\000\000\200v\000\000\000\000h\\\377\377\377\377\357\376\000\001\003\000\001\377\377\377\377\377\377\377\003\377\377\377\377\377\377\377\377\377\377\377\003\377\377\377\377\377\377\377\377\377\377\376\377\372\000\000\000\000\200v\000\000\000\000h\\\377\377\377\377\357\376\000\001\003\000\001\377\377\377\377\377\377\377\003\377\377\377\377\377\377\377\377\377\377\376\377\372\007
27artifact_prefix='./'; Test unit written to ./crash-2ea01576a6e0a74d403920761ea2ff6f0bf41752
28Base64: /+/+AAEDAAH//////wMAAf////////8D//////////////7/+gAAAACAdgAAAABoXP/////v/gABAwAB/////////wP//////////////wP//////////////v/6AAAAAIB2AAAAAGhc/////+/+AAEDAAH/////////A//////////////+//oH
29
30Target ['/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-max_total_time=60'] failed with exit code 1

l0rinc commented at 9:52 pm on May 10, 2025:

You can recreate the failures

Thanks a lot @fanquake, I managed to reproduce it this way (seeing that https://github.com/l0rinc/bitcoin/pull/10 was failing, I was trying to run them manually instead of relying on the ci scripts).

why it isn’t needed (or rather since when)

Thanks @maflcko for insisting, I have changed the order of commits to place the suppression removal at the very end since we still had a few failures for the first few commits - it’s easier to remove them all at the end only, where I couldn’t reproduce any such failure anymore.

maflcko commented at 2:43 pm on May 13, 2025:

Seems fine, but it may be better to remove the suppression in the exact commit that removes the need for it. This would make it easier to revert the commit atomically in one go, if there is need for whatever reason.

l0rinc commented at 3:09 pm on May 13, 2025:

Any accounting inconsistency could’ve caused it to become negative and fail, so instead of removing these suppressions in the last commit that fixed the problem (giving the impression that it was just one change that enabled us removing it), I did it separately instead. Is this important, is this holding back the ACK from your part?

l0rinc force-pushed on May 10, 2025

l0rinc requested review from fanquake on May 21, 2025

l0rinc requested review from maflcko on May 21, 2025

l0rinc requested review from andrewtoth on May 21, 2025

coins: fix cachedCoinsUsage accounting in CCoinsViewCache #32313

Code Coverage & Benchmarks

Reviews

Conflicts

coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache` #32313