This fixes cachedCoinsUsage drift and underflow in CCoinsViewCache, which previously showed up in UBSan as unsigned-integer-overflow in exception and failure paths.
AddCoin() used to subtract the old coin’s usage before validating possible_overwrite. If that validation threw, the map entry was unchanged but the counter was decremented, which corrupted accounting and later underflowed. Flush() also zeroed cachedCoinsUsage even when BatchWrite() failed in testing, leaving the map populated while the counter read zero.
The changes move the subtract in AddCoin() after the overwrite check, reset cachedCoinsUsage to 0 only when BatchWrite() succeeds and the map is cleared, make the cursor traversal-only by removing its usage adjustments and asserting that spent entries already have zero dynamic usage, and fix EmplaceCoinInternalDANGER() to increment only on successful insert while capturing the usage before the move.
The old Flush() workaround in fuzzing that attempted to realign accounting after an exception is dropped, the documented throw on illegal overwrite is asserted directly, and a path exercising EmplaceCoinInternalDANGER() is added so both accounting paths are covered.
Temporary underflow guards are added at the beginning and removed later and the UBSan suppressions for CCoinsViewCache are finally reenabled.
To reproduce the historical UBSan failures on the referenced baseline and to verify the fix, run:
0MAKEJOBS="-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}
or
0diff --git a/src/coins.cpp b/src/coins.cpp
1--- a/src/coins.cpp (revision fd3b1a7f4bb2ac527f23d4eb4cfa40a3215906e5)
2+++ b/src/coins.cpp (revision 872a05633bfdbd06ad82190d7fe34b42d13ebfe9)
3@@ -96,6 +96,7 @@
4 fresh = !it->second.IsDirty();
5 }
6 if (!inserted) {
7+ Assert(cachedCoinsUsage >= it->second.coin.DynamicMemoryUsage());
8 cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
9 }
10 it->second.coin = std::move(coin);
11@@ -133,6 +134,7 @@
12 bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
13 CCoinsMap::iterator it = FetchCoin(outpoint);
14 if (it == cacheCoins.end()) return false;
15+ Assert(cachedCoinsUsage >= it->second.coin.DynamicMemoryUsage());
16 cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
17 TRACEPOINT(utxocache, spent,
18 outpoint.hash.data(),
19@@ -226,10 +228,12 @@
20 if (itUs->second.IsFresh() && it->second.coin.IsSpent()) {
21 // The grandparent cache does not have an entry, and the coin
22 // has been spent. We can just delete it from the parent cache.
23+ Assert(cachedCoinsUsage >= itUs->second.coin.DynamicMemoryUsage());
24 cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
25 cacheCoins.erase(itUs);
26 } else {
27 // A normal modification.
28+ Assert(cachedCoinsUsage >= itUs->second.coin.DynamicMemoryUsage());
29 cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
30 if (cursor.WillErase(*it)) {
31 // Since this entry will be erased,
32@@ -279,6 +283,7 @@
33 {
34 CCoinsMap::iterator it = cacheCoins.find(hash);
35 if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
36+ Assert(cachedCoinsUsage >= it->second.coin.DynamicMemoryUsage());
37 cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
38 TRACEPOINT(utxocache, uncache,
39 hash.hash.data(),
