coins: use number of dirty cache entries in flush warnings/logs

l0rinc commented at 2:24 am on October 1, 2025: contributor

Revival of #31703 with all outstanding review feedback addressed.

EmplaceCoinInternalDANGER is also fuzz tested now
Warning and logging moved from FlushStateToDisk to CCoinsViewDB::BatchWrite, ensuring AssumeUTXO snapshot writes also display progress warnings
Removed the redundant changed counter in BatchWrite since we now track dirty_count directly
All instances use dirty_count (not num_dirty) and parameter alignment is corrected

Since #28280 and #28233, the percentage of dirty entries in the cache when a flush happens may in practice be far from 100%. The log output, the decision to warn about a large flush (#31534), and the disk-full detection, however always use the entire size of the cache.

Fix this by keeping track of the number of dirty entries in CCoinsViewCache, and using that number. We’ve dropped the usage of DynamicMemoryUsage as it’s the wrong metric, even before the non-wiping flushes were introduced (if everything is dirty, memory usage is correlated with, but not the same as, the amount of disk space written or used). They could be improved by keeping track of proper write size estimates in a follow-up, but here we’re just using a fixed 48 bytes per entry estimate.

DrahtBot commented at 2:24 am on October 1, 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/33512.

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:

#33602 ([IBD] coins: reduce lookups in dbcache layer propagation by l0rinc)
#33042 (refactor: inline constant return values from dbwrapper write methods by l0rinc)
#33018 (coins: remove SetFresh method from CCoinsCacheEntry by andrewtoth)
#29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)

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.

fanquake commented at 2:49 am on October 1, 2025: member

https://github.com/bitcoin/bitcoin/actions/runs/18149289661/job/51656823259?pr=33512#step:9:2059:

 0Run coins_view_db with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]INFO: Running with entropic power schedule (0xFF, 100).
 1INFO: Seed: 1436700023
 2INFO: Loaded 1 modules   (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92), 
 3INFO: Loaded 1 PC tables (611994 PCs): 611994 [0x6082fc937e98,0x6082fd28e838), 
 4INFO:     2396 files found in /home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db
 5INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 275937 bytes
 6INFO: seed corpus: files: 2396 min: 2b max: 275937b total: 27207582b rss: 109Mb
 7/home/admin/actions-runner/_work/_temp/src/coins.h:288:23: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
 8    [#0](/bitcoin-bitcoin/0/) 0x6082f97eb6c4 in CoinsViewCacheCursor::NextAndMaybeErase(std::pair<COutPoint const, CCoinsCacheEntry>&) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./coins.h:288:23
 9    [#1](/bitcoin-bitcoin/1/) 0x6082fb3e30bb in CCoinsViewDB::BatchWrite(CoinsViewCacheCursor&, uint256 const&) /home/admin/actions-runner/_work/_temp/build/src/./txdb.cpp:137:21
10    [#2](/bitcoin-bitcoin/2/) 0x6082f9ec4573 in CCoinsViewCache::Flush() /home/admin/actions-runner/_work/_temp/build/src/./coins.cpp:265:22
11    [#3](/bitcoin-bitcoin/3/) 0x6082f97cb77f in TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0::operator()() const /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:77:48
12    [#4](/bitcoin-bitcoin/4/) 0x6082f97cb77f in unsigned long CallOneOf<TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_10>(FuzzedDataProvider&, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_10) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/util.h:42:27
13    [#5](/bitcoin-bitcoin/5/) 0x6082f97cb77f in TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:56:9
14    [#6](/bitcoin-bitcoin/6/) 0x6082f97cfc0c in coins_view_db_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:323:5
15    [#7](/bitcoin-bitcoin/7/) 0x6082f9e6efae 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/13/../../../../include/c++/13/bits/std_function.h:591:9
16    [#8](/bitcoin-bitcoin/8/) 0x6082f9e6efae in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/util/./test/fuzz/fuzz.cpp:88:5
17    [#9](/bitcoin-bitcoin/9/) 0x6082f9e6efae in LLVMFuzzerTestOneInput /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/util/./test/fuzz/fuzz.cpp:216:5
18    [#10](/bitcoin-bitcoin/10/) 0x6082f947d66f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a8366f) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
19    [#11](/bitcoin-bitcoin/11/) 0x6082f947cc79 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a82c79) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
20    [#12](/bitcoin-bitcoin/12/) 0x6082f947e9e2 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a849e2) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
21    [#13](/bitcoin-bitcoin/13/) 0x6082f947ef00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a84f00) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
22    [#14](/bitcoin-bitcoin/14/) 0x6082f946b585 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a71585) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
23    [#15](/bitcoin-bitcoin/15/) 0x6082f9497946 in main (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a9d946) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
24    [#16](/bitcoin-bitcoin/16/) 0x756d565fa1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
25    [#17](/bitcoin-bitcoin/17/) 0x756d565fa28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
26    [#18](/bitcoin-bitcoin/18/) 0x6082f945fb54 in _start (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a65b54) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
27
28SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/admin/actions-runner/_work/_temp/src/coins.h:288:23 
29MS: 0 ; base unit: 0000000000000000000000000000000000000000
300x1,0xff,0xff,0xff,0xf2,0xfc,0xff,0x2f,0x1f,0xa9,0x5d,0x30,0x30,0x90,0x0,0x0,0x0,0x14,0x69,0x0,0x0,0x0,0x0,0x0,0x0,0xc1,0xc1,0xc1,0xc1,0xc1,0xc1,0xb7,0xc1,0xc1,0x5c,0x91,0x3b,0x72,0x0,0xff,0xfa,0x21,
31\001\377\377\377\362\374\377/\037\251]00\220\000\000\000\024i\000\000\000\000\000\000\301\301\301\301\301\301\267\301\301\\\221;r\000\377\372!
32artifact_prefix='./'; Test unit written to ./crash-ebc99ca6d40a3c7a37202cdba9a8b36a2cb6007b
33Base64: Af////L8/y8fqV0wMJAAAAAUaQAAAAAAAMHBwcHBwbfBwVyRO3IA//oh
34
35INFO: Running with entropic power schedule (0xFF, 100).
36INFO: Seed: 1436700023
37INFO: Loaded 1 modules   (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92), 
38INFO: Loaded 1 PC tables (611994 PCs): 611994 [0x6082fc937e98,0x6082fd28e838), 
39INFO:     2396 files found in /home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db
40INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 275937 bytes
41INFO: seed corpus: files: 2396 min: 2b max: 275937b total: 27207582b rss: 109Mb
42/home/admin/actions-runner/_work/_temp/src/coins.h:288:23: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
43    [#0](/bitcoin-bitcoin/0/) 0x6082f97eb6c4 in CoinsViewCacheCursor::NextAndMaybeErase(std::pair<COutPoint const, CCoinsCacheEntry>&) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./coins.h:288:23
44    [#1](/bitcoin-bitcoin/1/) 0x6082fb3e30bb in CCoinsViewDB::BatchWrite(CoinsViewCacheCursor&, uint256 const&) /home/admin/actions-runner/_work/_temp/build/src/./txdb.cpp:137:21
45    [#2](/bitcoin-bitcoin/2/) 0x6082f9ec4573 in CCoinsViewCache::Flush() /home/admin/actions-runner/_work/_temp/build/src/./coins.cpp:265:22
46    [#3](/bitcoin-bitcoin/3/) 0x6082f97cb77f in TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0::operator()() const /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:77:48
47    [#4](/bitcoin-bitcoin/4/) 0x6082f97cb77f in unsigned long CallOneOf<TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_10>(FuzzedDataProvider&, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool)::$_10) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/util.h:42:27
48    [#5](/bitcoin-bitcoin/5/) 0x6082f97cb77f in TestCoinsView(FuzzedDataProvider&, CCoinsView&, bool) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:56:9
49    [#6](/bitcoin-bitcoin/6/) 0x6082f97cfc0c in coins_view_db_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/./test/fuzz/coins_view.cpp:323:5
50    [#7](/bitcoin-bitcoin/7/) 0x6082f9e6efae 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/13/../../../../include/c++/13/bits/std_function.h:591:9
51    [#8](/bitcoin-bitcoin/8/) 0x6082f9e6efae in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/util/./test/fuzz/fuzz.cpp:88:5
52    [#9](/bitcoin-bitcoin/9/) 0x6082f9e6efae in LLVMFuzzerTestOneInput /home/admin/actions-runner/_work/_temp/build/src/test/fuzz/util/./test/fuzz/fuzz.cpp:216:5
53    [#10](/bitcoin-bitcoin/10/) 0x6082f947d66f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a8366f) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
54    [#11](/bitcoin-bitcoin/11/) 0x6082f947cc79 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a82c79) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
55    [#12](/bitcoin-bitcoin/12/) 0x6082f947e9e2 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a849e2) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
56    [#13](/bitcoin-bitcoin/13/) 0x6082f947ef00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a84f00) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
57    [#14](/bitcoin-bitcoin/14/) 0x6082f946b585 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a71585) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
58    [#15](/bitcoin-bitcoin/15/) 0x6082f9497946 in main (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a9d946) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
59    [#16](/bitcoin-bitcoin/16/) 0x756d565fa1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
60    [#17](/bitcoin-bitcoin/17/) 0x756d565fa28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
61    [#18](/bitcoin-bitcoin/18/) 0x6082f945fb54 in _start (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1a65b54) (BuildId: c46e5f78c23c1d0b2ce0fb85b0817bc7fae97dd8)
62
63SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/admin/actions-runner/_work/_temp/src/coins.h:288:23 
64MS: 0 ; base unit: 0000000000000000000000000000000000000000
650x1,0xff,0xff,0xff,0xf2,0xfc,0xff,0x2f,0x1f,0xa9,0x5d,0x30,0x30,0x90,0x0,0x0,0x0,0x14,0x69,0x0,0x0,0x0,0x0,0x0,0x0,0xc1,0xc1,0xc1,0xc1,0xc1,0xc1,0xb7,0xc1,0xc1,0x5c,0x91,0x3b,0x72,0x0,0xff,0xfa,0x21,
66\001\377\377\377\362\374\377/\037\251]00\220\000\000\000\024i\000\000\000\000\000\000\301\301\301\301\301\301\267\301\301\\\221;r\000\377\372!
67artifact_prefix='./'; Test unit written to ./crash-ebc99ca6d40a3c7a37202cdba9a8b36a2cb6007b
68Base64: Af////L8/y8fqV0wMJAAAAAUaQAAAAAAAMHBwcHBwbfBwVyRO3IA//oh
69
70⚠️ Failure generated from target with exit code 1: ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]

l0rinc force-pushed on Oct 1, 2025

l0rinc commented at 2:51 am on October 1, 2025: contributor

@fanquake the fuzz test is fixed in a separate PR, I have reverted a similar attempt here

l0rinc force-pushed on Oct 1, 2025

l0rinc renamed this:
~~Use number of dirty cache entries in flush warnings/logs~~
coins: use number of dirty cache entries in flush warnings/logs
on Oct 1, 2025

DrahtBot added the label UTXO Db and Indexes on Oct 1, 2025

l0rinc force-pushed on Oct 1, 2025

in src/test/coins_tests.cpp:199 in 7afa718777 outdated

193@@ -194,8 +194,11 @@ void SimulationTest(CCoinsView* base, bool fake_best_block)
194                     (coin.IsSpent() ? added_an_entry : updated_an_entry) = true;
195                     coin = newcoin;
196                 }
197-                bool is_overwrite = !coin.IsSpent() || m_rng.rand32() & 1;
198-                stack.back()->AddCoin(COutPoint(txid, 0), std::move(newcoin), is_overwrite);
199+                if (COutPoint op(txid, 0); !stack.back()->map().contains(op) && !coin.IsSpent() && m_rng.randbool()) {
200+                    stack.back()->EmplaceCoinInternalDANGER(std::move(op), std::move(newcoin));

andrewtoth commented at 1:35 am on October 2, 2025:

Adds test coverage by randomly calling EmplaceCoinInternalDANGER in SimulationTest to verify the accounting remains correct.

Where do we verify that the accounting is correct? This also checks that the map does not contain this coin, so the if (inserted) { block will always be true and we never test the other case.

l0rinc commented at 3:06 am on October 2, 2025:

0    /**
1     * Emplace a coin into cacheCoins without performing any checks, marking
2     * the emplaced coin as dirty.
3     *
4     * NOT FOR GENERAL USE. Used only when loading coins from a UTXO snapshot.
5     * [@sa](/bitcoin-bitcoin/contributor/sa/) ChainstateManager::PopulateAndValidateSnapshot()
6     */
7    void EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin);

EmplaceCoinInternalDANGER is not meant to be used for an overwrite according to the docs, so to mimick the assumeutxo inserts I only called this when it’s not already in the set. Not exactly sure why there’s an if (inserted) { branch in EmplaceCoinInternalDANGER the first place, but don’t want to modify too much here. What do you suggest?

l0rinc commented at 3:21 pm on October 2, 2025:

You also answered something similar in #31132 (review)

andrewtoth commented at 2:49 pm on October 4, 2025:

I see, so if this gets merged the test here can be modified in #31132 to test for insertion if already exists?

I don’t see where we verify the accounting though. You are referring to the cachedCoinsUsage accounting? This test doesn’t seem to check that. Maybe that can be added?

l0rinc commented at 4:34 am on October 7, 2025:

Thanks, added a fuzz test and allowed existing entries to be attempted in EmplaceCoinInternalDANGER

l0rinc commented at 3:04 am on October 8, 2025:

Update: had to remove the fuzz test, the accounting is broken here, but it’s fixed in another PR, see: #32313 (comment)

andrewtoth commented at 0:10 am on October 12, 2025:

Does this PR depend on #32313?

l0rinc commented at 0:27 am on October 12, 2025:

The two are related, but I made sure we can merge either of them - but the other one will likely need to be rebase after that

sipa commented at 5:02 pm on October 17, 2025:

I think this is resolved now, with the merge of #32313?

andrewtoth commented at 5:44 pm on October 17, 2025:

Yes, I think we can now remove the conditions !stack.back()->map().contains(op) && !coin.IsSpent().

l0rinc commented at 5:45 am on October 18, 2025:

I wanted to make it more realistic, but the tests do pass now without them, thanks for the feedback, removed.

l0rinc commented at 8:21 pm on October 20, 2025:

Seems we were wrong, the test assumes that after this call the given COutPoint will correspond to the current coins - which isn’t the case with EmplaceCoinInternalDANGER which never replaces, so I have reverted https://github.com/bitcoin/bitcoin/compare/62cb2a2ed84e695c0959d8e27fc5bedd9b5a135e..54151d612b63f4e42fe03a143540e597728a4cc3

See the previous errors that the suggestions resulted in: https://github.com/bitcoin/bitcoin/actions/runs/18611576758/job/53070427535?pr=33512#step:7:40482

andrewtoth commented at 11:34 pm on October 20, 2025:

the test assumes that after this call the given COutPoint will correspond to the current coins - which isn’t the case with EmplaceCoinInternalDANGER which never replaces

Right, so we still need !stack.back()->map().contains(op) but we don’t need the !coin.IsSpent() check. Also, a few lines up we randomly set the pubkey to unspendable. In AddCoin we check if it is unspendable and don’t add it, but in EmplaceCoinInternalDANGER we would add it. So I think we should also not use EmplaceCoinInternalDANGER if newcoin.out.scriptPubKey.IsUnspendable().

in src/test/coins_tests.cpp:102 in 6aecb42c40 outdated

 98@@ -99,7 +99,7 @@ class CCoinsViewCacheTest : public CCoinsViewCache
 99 
100     CCoinsMap& map() const { return cacheCoins; }
101     CoinsCachePair& sentinel() const { return m_sentinel; }
102-    size_t& usage() const { return cachedCoinsUsage; }
103+    void AddUsage(size_t usage) const { cachedCoinsUsage += usage; }

andrewtoth commented at 1:48 am on October 2, 2025:

The benefit of the change in this commit is unclear. I think it could just be removed to make review easier.