coins: spent erase in CoinsViewCacheCursor must not change usage

Assert that spent coins already have zero dynamic size, and remove unused leftover counter
This commit is contained in:
Lőrinc 2025-04-20 16:47:51 +02:00
parent 0189ab0939
commit a7f8b5f13b
4 changed files with 10 additions and 14 deletions

View file

@ -249,7 +249,7 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
}
bool CCoinsViewCache::Flush() {
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/true)};
auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)};
bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) {
cacheCoins.clear();
@ -261,7 +261,7 @@ bool CCoinsViewCache::Flush() {
bool CCoinsViewCache::Sync()
{
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/false)};
auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/false)};
bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) {
if (m_sentinel.second.Next() != &m_sentinel) {

View file

@ -271,11 +271,10 @@ struct CoinsViewCacheCursor
//! This is an optimization compared to erasing all entries as the cursor iterates them when will_erase is set.
//! Calling CCoinsMap::clear() afterwards is faster because a CoinsCachePair cannot be coerced back into a
//! CCoinsMap::iterator to be erased, and must therefore be looked up again by key in the CCoinsMap before being erased.
CoinsViewCacheCursor(size_t& usage LIFETIMEBOUND,
CoinsCachePair& sentinel LIFETIMEBOUND,
CoinsViewCacheCursor(CoinsCachePair& sentinel LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND,
bool will_erase) noexcept
: m_usage(usage), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
: m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); }
inline CoinsCachePair* End() const noexcept { return &m_sentinel; }
@ -288,7 +287,7 @@ struct CoinsViewCacheCursor
// Otherwise, clear the state of the entry.
if (!m_will_erase) {
if (current.second.coin.IsSpent()) {
m_usage -= current.second.coin.DynamicMemoryUsage();
assert(current.second.coin.DynamicMemoryUsage() == 0); // scriptPubKey was already cleared in SpendCoin
m_map.erase(current.first);
} else {
current.second.SetClean();
@ -299,7 +298,6 @@ struct CoinsViewCacheCursor
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
private:
size_t& m_usage;
CoinsCachePair& m_sentinel;
CCoinsMap& m_map;
bool m_will_erase;

View file

@ -651,8 +651,8 @@ static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
sentinel.second.SelfRef(sentinel);
CCoinsMapMemoryResource resource;
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
auto usage{cache_coin ? InsertCoinsMapEntry(map, sentinel, *cache_coin) : 0};
auto cursor{CoinsViewCacheCursor(usage, sentinel, map, /*will_erase=*/true)};
if (cache_coin) InsertCoinsMapEntry(map, sentinel, *cache_coin);
auto cursor{CoinsViewCacheCursor(sentinel, map, /*will_erase=*/true)};
BOOST_CHECK(view.BatchWrite(cursor, {}));
}

View file

@ -122,7 +122,6 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
[&] {
CoinsCachePair sentinel{};
sentinel.second.SelfRef(sentinel);
size_t usage{0};
CCoinsMapMemoryResource resource;
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
@ -143,11 +142,10 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first};
if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel);
usage += it->second.coin.DynamicMemoryUsage();
}
bool expected_code_path = false;
try {
auto cursor{CoinsViewCacheCursor(usage, sentinel, coins_map, /*will_erase=*/true)};
auto cursor{CoinsViewCacheCursor(sentinel, coins_map, /*will_erase=*/true)};
coins_view_cache.BatchWrite(cursor, fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock());
expected_code_path = true;
} catch (const std::logic_error& e) {