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 ba38dad502
commit 6210b70291
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() { 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); bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) { if (fOk) {
cacheCoins.clear(); cacheCoins.clear();
@ -261,7 +261,7 @@ bool CCoinsViewCache::Flush() {
bool CCoinsViewCache::Sync() 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); bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) { if (fOk) {
if (m_sentinel.second.Next() != &m_sentinel) { 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. //! 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 //! 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. //! 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, CoinsViewCacheCursor(CoinsCachePair& sentinel LIFETIMEBOUND,
CoinsCachePair& sentinel LIFETIMEBOUND, CCoinsMap& map LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND, bool will_erase) noexcept
bool will_erase) noexcept : m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
: m_usage(usage), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); } inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); }
inline CoinsCachePair* End() const noexcept { return &m_sentinel; } inline CoinsCachePair* End() const noexcept { return &m_sentinel; }
@ -288,7 +287,7 @@ struct CoinsViewCacheCursor
// Otherwise, clear the state of the entry. // Otherwise, clear the state of the entry.
if (!m_will_erase) { if (!m_will_erase) {
if (current.second.coin.IsSpent()) { 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); m_map.erase(current.first);
} else { } else {
current.second.SetClean(); current.second.SetClean();
@ -299,7 +298,6 @@ struct CoinsViewCacheCursor
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); } inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
private: private:
size_t& m_usage;
CoinsCachePair& m_sentinel; CoinsCachePair& m_sentinel;
CCoinsMap& m_map; CCoinsMap& m_map;
bool m_will_erase; bool m_will_erase;

View file

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

View file

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