highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-04-18 19:47:53 +02:00
Code Issues Packages Projects Releases Wiki Activity

coins: fix cachedCoinsUsage accounting to prevent underflow

Browse Source 
Move the `cachedCoinsUsage` subtract in `AddCoin()` to after the `possible_overwrite` check.
Previously a throw before assignment decremented the counter without changing the entry, which corrupted accounting and later underflowed.

In `Flush()`, reset `cachedCoinsUsage` to `0` only when `BatchWrite()` succeeds and `cacheCoins` is actually cleared. In production `BatchWrite()` returns `true`, so this mostly affects tests. On failure, leave the counter unchanged to keep it in sync with the cache.

The existing `Flush()` workaround in fuzzing was also removed now that the source of the problem was fixed, so the fuzzer no longer needs `coins_view_cache.Flush()` to realign `cachedCoinsUsage` after an exception.
Replace the prior `expected_code_path` tracking with direct assertions. The role of the variable was to verify that code execution follows only expected paths, either successful addition, or if it's an exception, the message is verified and checked that overwrite was disallowed.

With these changes the counter stays consistent across success and exception paths, so we can finally remove the `UBSan` suppressions for `CCoinsViewCache` that were masking the issue.

Included a unit test as well, attempting to add a different coin to the same outpoint without allowing overwrites and make sure it throws.
We use `SelfTest()` to validates accounting, and check that the cache remains usable.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: w0xlt <woltx@protonmail.com>
This commit is contained in:
Lőrinc
2025-04-18 11:23:27 +02:00
parent 39cf8bb3d0
commit d7c9d6c291
4 changed files with 28 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/coins.cpp
View File

@@ -76,9 +76,6 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
bool inserted;
std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>());
bool fresh = false;
if (!inserted) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
}
if (!possible_overwrite) {
if (!it->second.coin.IsSpent()) {
throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)");
@@ -98,6 +95,9 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
// DIRTY, then it can be marked FRESH.
fresh = !it->second.IsDirty();
}
if (!inserted) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
}
it->second.coin = std::move(coin);
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it, m_sentinel);
@@ -254,8 +254,8 @@ bool CCoinsViewCache::Flush() {
if (fOk) {
cacheCoins.clear();
ReallocateCache();
cachedCoinsUsage = 0;
}
cachedCoinsUsage = 0;
return fOk;
}