This commit is contained in:
Andrew Toth 2025-03-13 02:02:41 +01:00 committed by GitHub
commit 721f2b591e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 86 additions and 116 deletions

View file

@ -51,10 +51,6 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const
if (auto coin{base->GetCoin(outpoint)}) { if (auto coin{base->GetCoin(outpoint)}) {
ret->second.coin = std::move(*coin); ret->second.coin = std::move(*coin);
cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage();
if (ret->second.coin.IsSpent()) { // TODO GetCoin cannot return spent coins
// The parent only has an empty entry for this outpoint; we can consider our version as fresh.
CCoinsCacheEntry::SetFresh(*ret, m_sentinel);
}
} else { } else {
cacheCoins.erase(ret); cacheCoins.erase(ret);
return cacheCoins.end(); return cacheCoins.end();
@ -83,20 +79,18 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
if (!it->second.coin.IsSpent()) { if (!it->second.coin.IsSpent()) {
throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)"); throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)");
} }
// If the coin exists in this cache as a spent coin and is DIRTY, then // If the coin exists in this cache as a spent coin, then
// its spentness hasn't been flushed to the parent cache. We're // its spentness hasn't been flushed to the parent cache. We're
// re-adding the coin to this cache now but we can't mark it as FRESH. // re-adding the coin to this cache now but we can't mark it as FRESH.
// If we mark it FRESH and then spend it before the cache is flushed // A spent FRESH coin cannot exist in the cache because a FRESH coin
// we would remove it from this cache and would never flush spentness // is simply erased when it is spent.
// to the parent cache.
// //
// Re-adding a spent coin can happen in the case of a re-org (the coin // Re-adding a spent coin can happen in the case of a re-org (the coin
// is 'spent' when the block adding it is disconnected and then // is 'spent' when the block adding it is disconnected and then
// re-added when it is also added in a newly connected block). // re-added when it is also added in a newly connected block).
// //
// If the coin doesn't exist in the current cache, or is spent but not // If the coin doesn't exist in the current cache then it can be marked FRESH.
// DIRTY, then it can be marked FRESH. fresh = inserted;
fresh = !it->second.IsDirty();
} }
it->second.coin = std::move(coin); it->second.coin = std::move(coin);
CCoinsCacheEntry::SetDirty(*it, m_sentinel); CCoinsCacheEntry::SetDirty(*it, m_sentinel);
@ -182,33 +176,32 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) {
bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlockIn) { bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlockIn) {
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) {
// Ignore non-dirty entries (optimization).
if (!it->second.IsDirty()) { if (!it->second.IsDirty()) {
continue; throw std::logic_error("A non-DIRTY coin was returned from the cursor in BatchWrite");
}
if (it->second.IsFresh() && it->second.coin.IsSpent()) {
throw std::logic_error("A FRESH coin was not removed when it was spent");
} }
CCoinsMap::iterator itUs = cacheCoins.find(it->first); CCoinsMap::iterator itUs = cacheCoins.find(it->first);
if (itUs == cacheCoins.end()) { if (itUs == cacheCoins.end()) {
// The parent cache does not have an entry, while the child cache does. // The parent cache does not have an entry, while the child cache does.
// We can ignore it if it's both spent and FRESH in the child // Create the coin in the parent cache, move the data up
if (!(it->second.IsFresh() && it->second.coin.IsSpent())) { // and mark it as dirty.
// Create the coin in the parent cache, move the data up itUs = cacheCoins.try_emplace(it->first).first;
// and mark it as dirty. CCoinsCacheEntry& entry{itUs->second};
itUs = cacheCoins.try_emplace(it->first).first; if (cursor.WillErase(*it)) {
CCoinsCacheEntry& entry{itUs->second}; // Since this entry will be erased,
if (cursor.WillErase(*it)) { // we can move the coin into us instead of copying it
// Since this entry will be erased, entry.coin = std::move(it->second.coin);
// we can move the coin into us instead of copying it } else {
entry.coin = std::move(it->second.coin); entry.coin = it->second.coin;
} else {
entry.coin = it->second.coin;
}
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
// We can mark it FRESH in the parent if it was FRESH in the child
// Otherwise it might have just been flushed from the parent's cache
// and already exist in the grandparent
if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs, m_sentinel);
} }
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
// We can mark it FRESH in the parent if it was FRESH in the child
// Otherwise it might have just been flushed from the parent's cache
// and already exist in the grandparent
if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs, m_sentinel);
} else { } else {
// Found the entry in the parent cache // Found the entry in the parent cache
if (it->second.IsFresh() && !itUs->second.coin.IsSpent()) { if (it->second.IsFresh() && !itUs->second.coin.IsSpent()) {
@ -273,8 +266,7 @@ bool CCoinsViewCache::Sync()
void CCoinsViewCache::Uncache(const COutPoint& hash) void CCoinsViewCache::Uncache(const COutPoint& hash)
{ {
CCoinsMap::iterator it = cacheCoins.find(hash); if (auto it{cacheCoins.find(hash)}; it != cacheCoins.end() && !it->second.IsDirty()) {
if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
TRACEPOINT(utxocache, uncache, TRACEPOINT(utxocache, uncache,
hash.hash.data(), hash.hash.data(),
@ -317,18 +309,19 @@ void CCoinsViewCache::SanityCheck() const
size_t recomputed_usage = 0; size_t recomputed_usage = 0;
size_t count_flagged = 0; size_t count_flagged = 0;
for (const auto& [_, entry] : cacheCoins) { for (const auto& [_, entry] : cacheCoins) {
unsigned attr = 0; if (entry.coin.IsSpent()) {
if (entry.IsDirty()) attr |= 1; // A spent coin must be dirty and cannot be fresh
if (entry.IsFresh()) attr |= 2; assert(entry.IsDirty() && !entry.IsFresh());
if (entry.coin.IsSpent()) attr |= 4; } else {
// Only 5 combinations are possible. // An unspent coin must not be fresh if not dirty
assert(attr != 2 && attr != 4 && attr != 7); assert(entry.IsDirty() || !entry.IsFresh());
}
// Recompute cachedCoinsUsage. // Recompute cachedCoinsUsage.
recomputed_usage += entry.coin.DynamicMemoryUsage(); recomputed_usage += entry.coin.DynamicMemoryUsage();
// Count the number of entries we expect in the linked list. // Count the number of entries we expect in the linked list.
if (entry.IsDirty() || entry.IsFresh()) ++count_flagged; if (entry.IsDirty()) ++count_flagged;
} }
// Iterate over the linked list of flagged entries. // Iterate over the linked list of flagged entries.
size_t count_linked = 0; size_t count_linked = 0;
@ -337,7 +330,7 @@ void CCoinsViewCache::SanityCheck() const
assert(it->second.Next()->second.Prev() == it); assert(it->second.Next()->second.Prev() == it);
assert(it->second.Prev()->second.Next() == it); assert(it->second.Prev()->second.Next() == it);
// Verify they are actually flagged. // Verify they are actually flagged.
assert(it->second.IsDirty() || it->second.IsFresh()); assert(it->second.IsDirty());
// Count the number of entries actually in the list. // Count the number of entries actually in the list.
++count_linked; ++count_linked;
} }

View file

@ -102,7 +102,6 @@ using CoinsCachePair = std::pair<const COutPoint, CCoinsCacheEntry>;
* - unspent, FRESH, DIRTY (e.g. a new coin created in the cache) * - unspent, FRESH, DIRTY (e.g. a new coin created in the cache)
* - unspent, not FRESH, DIRTY (e.g. a coin changed in the cache during a reorg) * - unspent, not FRESH, DIRTY (e.g. a coin changed in the cache during a reorg)
* - unspent, not FRESH, not DIRTY (e.g. an unspent coin fetched from the parent cache) * - unspent, not FRESH, not DIRTY (e.g. an unspent coin fetched from the parent cache)
* - spent, FRESH, not DIRTY (e.g. a spent coin fetched from the parent cache)
* - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent) * - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent)
*/ */
struct CCoinsCacheEntry struct CCoinsCacheEntry
@ -117,12 +116,6 @@ private:
* the parent cache for batch writing. This is a performance optimization * the parent cache for batch writing. This is a performance optimization
* compared to giving all entries in the cache to the parent and having the * compared to giving all entries in the cache to the parent and having the
* parent scan for only modified entries. * parent scan for only modified entries.
*
* FRESH-but-not-DIRTY coins can not occur in practice, since that would
* mean a spent coin exists in the parent CCoinsView and not in the child
* CCoinsViewCache. Nevertheless, if a spent coin is retrieved from the
* parent cache, the FRESH-but-not-DIRTY coin will be tracked by the linked
* list and deleted when Sync or Flush is called on the CCoinsViewCache.
*/ */
CoinsCachePair* m_prev{nullptr}; CoinsCachePair* m_prev{nullptr};
CoinsCachePair* m_next{nullptr}; CoinsCachePair* m_next{nullptr};
@ -188,17 +181,15 @@ public:
bool IsDirty() const noexcept { return m_flags & DIRTY; } bool IsDirty() const noexcept { return m_flags & DIRTY; }
bool IsFresh() const noexcept { return m_flags & FRESH; } bool IsFresh() const noexcept { return m_flags & FRESH; }
//! Only call Next when this entry is DIRTY, FRESH, or both
CoinsCachePair* Next() const noexcept CoinsCachePair* Next() const noexcept
{ {
Assume(m_flags); Assume(IsDirty());
return m_next; return m_next;
} }
//! Only call Prev when this entry is DIRTY, FRESH, or both
CoinsCachePair* Prev() const noexcept CoinsCachePair* Prev() const noexcept
{ {
Assume(m_flags); Assume(IsDirty());
return m_prev; return m_prev;
} }

View file

@ -48,10 +48,8 @@ public:
std::optional<Coin> GetCoin(const COutPoint& outpoint) const override std::optional<Coin> GetCoin(const COutPoint& outpoint) const override
{ {
if (auto it{map_.find(outpoint)}; it != map_.end()) { if (auto it{map_.find(outpoint)}; it != map_.end() && !it->second.IsSpent()) {
if (!it->second.IsSpent() || m_rng.randbool()) { return it->second;
return it->second; // TODO spent coins shouldn't be returned
}
} }
return std::nullopt; return std::nullopt;
} }
@ -60,14 +58,12 @@ public:
bool BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override bool BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override
{ {
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)){ for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) {
if (it->second.IsDirty()) { assert(it->second.IsDirty());
// Same optimization used in CCoinsViewDB is to only write dirty entries. map_[it->first] = it->second.coin;
map_[it->first] = it->second.coin; if (it->second.coin.IsSpent() && m_rng.randrange(3) == 0) {
if (it->second.coin.IsSpent() && m_rng.randrange(3) == 0) { // Randomly delete spent entries on write.
// Randomly delete empty entries on write. map_.erase(it->first);
map_.erase(it->first);
}
} }
} }
if (!hashBlock.IsNull()) if (!hashBlock.IsNull())
@ -611,6 +607,8 @@ constexpr MaybeCoin VALUE3_DIRTY_FRESH{{VALUE3, CoinEntry::State::DIRTY_FRESH}};
constexpr auto EX_OVERWRITE_UNSPENT{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}; constexpr auto EX_OVERWRITE_UNSPENT{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"};
constexpr auto EX_FRESH_MISAPPLIED {"FRESH flag misapplied to coin that exists in parent cache"}; constexpr auto EX_FRESH_MISAPPLIED {"FRESH flag misapplied to coin that exists in parent cache"};
constexpr auto EX_NON_DIRTY_WRITE{"A non-DIRTY coin was returned from the cursor in BatchWrite"};
constexpr auto EX_FRESH_AND_SPENT{"A FRESH coin was not removed when it was spent"};
static void SetCoinsValue(const CAmount value, Coin& coin) static void SetCoinsValue(const CAmount value, Coin& coin)
{ {
@ -758,7 +756,7 @@ BOOST_AUTO_TEST_CASE(ccoins_add)
CheckAddCoin(base_value, MISSING, VALUE3, VALUE3_DIRTY_FRESH, false); CheckAddCoin(base_value, MISSING, VALUE3, VALUE3_DIRTY_FRESH, false);
CheckAddCoin(base_value, MISSING, VALUE3, VALUE3_DIRTY, true ); CheckAddCoin(base_value, MISSING, VALUE3, VALUE3_DIRTY, true );
CheckAddCoin(base_value, SPENT_CLEAN, VALUE3, VALUE3_DIRTY_FRESH, false); CheckAddCoin(base_value, SPENT_CLEAN, VALUE3, VALUE3_DIRTY, false);
CheckAddCoin(base_value, SPENT_CLEAN, VALUE3, VALUE3_DIRTY, true ); CheckAddCoin(base_value, SPENT_CLEAN, VALUE3, VALUE3_DIRTY, true );
CheckAddCoin(base_value, SPENT_FRESH, VALUE3, VALUE3_DIRTY_FRESH, false); CheckAddCoin(base_value, SPENT_FRESH, VALUE3, VALUE3_DIRTY_FRESH, false);
CheckAddCoin(base_value, SPENT_FRESH, VALUE3, VALUE3_DIRTY_FRESH, true ); CheckAddCoin(base_value, SPENT_FRESH, VALUE3, VALUE3_DIRTY_FRESH, true );
@ -800,7 +798,7 @@ BOOST_AUTO_TEST_CASE(ccoins_write)
*/ */
CheckWriteCoins(MISSING, MISSING, MISSING ); CheckWriteCoins(MISSING, MISSING, MISSING );
CheckWriteCoins(MISSING, SPENT_DIRTY, SPENT_DIRTY ); CheckWriteCoins(MISSING, SPENT_DIRTY, SPENT_DIRTY );
CheckWriteCoins(MISSING, SPENT_DIRTY_FRESH, MISSING ); CheckWriteCoins(MISSING, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(MISSING, VALUE2_DIRTY, VALUE2_DIRTY ); CheckWriteCoins(MISSING, VALUE2_DIRTY, VALUE2_DIRTY );
CheckWriteCoins(MISSING, VALUE2_DIRTY_FRESH, VALUE2_DIRTY_FRESH ); CheckWriteCoins(MISSING, VALUE2_DIRTY_FRESH, VALUE2_DIRTY_FRESH );
CheckWriteCoins(SPENT_CLEAN, MISSING, SPENT_CLEAN ); CheckWriteCoins(SPENT_CLEAN, MISSING, SPENT_CLEAN );
@ -809,13 +807,13 @@ BOOST_AUTO_TEST_CASE(ccoins_write)
CheckWriteCoins(SPENT_DIRTY_FRESH, MISSING, SPENT_DIRTY_FRESH ); CheckWriteCoins(SPENT_DIRTY_FRESH, MISSING, SPENT_DIRTY_FRESH );
CheckWriteCoins(SPENT_CLEAN, SPENT_DIRTY, SPENT_DIRTY ); CheckWriteCoins(SPENT_CLEAN, SPENT_DIRTY, SPENT_DIRTY );
CheckWriteCoins(SPENT_CLEAN, SPENT_DIRTY_FRESH, SPENT_DIRTY ); CheckWriteCoins(SPENT_CLEAN, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(SPENT_FRESH, SPENT_DIRTY, MISSING ); CheckWriteCoins(SPENT_FRESH, SPENT_DIRTY, MISSING );
CheckWriteCoins(SPENT_FRESH, SPENT_DIRTY_FRESH, MISSING ); CheckWriteCoins(SPENT_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(SPENT_DIRTY, SPENT_DIRTY, SPENT_DIRTY ); CheckWriteCoins(SPENT_DIRTY, SPENT_DIRTY, SPENT_DIRTY );
CheckWriteCoins(SPENT_DIRTY, SPENT_DIRTY_FRESH, SPENT_DIRTY ); CheckWriteCoins(SPENT_DIRTY, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(SPENT_DIRTY_FRESH, SPENT_DIRTY, MISSING ); CheckWriteCoins(SPENT_DIRTY_FRESH, SPENT_DIRTY, MISSING );
CheckWriteCoins(SPENT_DIRTY_FRESH, SPENT_DIRTY_FRESH, MISSING ); CheckWriteCoins(SPENT_DIRTY_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(SPENT_CLEAN, VALUE2_DIRTY, VALUE2_DIRTY ); CheckWriteCoins(SPENT_CLEAN, VALUE2_DIRTY, VALUE2_DIRTY );
CheckWriteCoins(SPENT_CLEAN, VALUE2_DIRTY_FRESH, VALUE2_DIRTY ); CheckWriteCoins(SPENT_CLEAN, VALUE2_DIRTY_FRESH, VALUE2_DIRTY );
@ -831,13 +829,13 @@ BOOST_AUTO_TEST_CASE(ccoins_write)
CheckWriteCoins(VALUE1_DIRTY, MISSING, VALUE1_DIRTY ); CheckWriteCoins(VALUE1_DIRTY, MISSING, VALUE1_DIRTY );
CheckWriteCoins(VALUE1_DIRTY_FRESH, MISSING, VALUE1_DIRTY_FRESH ); CheckWriteCoins(VALUE1_DIRTY_FRESH, MISSING, VALUE1_DIRTY_FRESH );
CheckWriteCoins(VALUE1_CLEAN, SPENT_DIRTY, SPENT_DIRTY ); CheckWriteCoins(VALUE1_CLEAN, SPENT_DIRTY, SPENT_DIRTY );
CheckWriteCoins(VALUE1_CLEAN, SPENT_DIRTY_FRESH, EX_FRESH_MISAPPLIED); CheckWriteCoins(VALUE1_CLEAN, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(VALUE1_FRESH, SPENT_DIRTY, MISSING ); CheckWriteCoins(VALUE1_FRESH, SPENT_DIRTY, MISSING );
CheckWriteCoins(VALUE1_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_MISAPPLIED); CheckWriteCoins(VALUE1_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(VALUE1_DIRTY, SPENT_DIRTY, SPENT_DIRTY ); CheckWriteCoins(VALUE1_DIRTY, SPENT_DIRTY, SPENT_DIRTY );
CheckWriteCoins(VALUE1_DIRTY, SPENT_DIRTY_FRESH, EX_FRESH_MISAPPLIED); CheckWriteCoins(VALUE1_DIRTY, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(VALUE1_DIRTY_FRESH, SPENT_DIRTY, MISSING ); CheckWriteCoins(VALUE1_DIRTY_FRESH, SPENT_DIRTY, MISSING );
CheckWriteCoins(VALUE1_DIRTY_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_MISAPPLIED); CheckWriteCoins(VALUE1_DIRTY_FRESH, SPENT_DIRTY_FRESH, EX_FRESH_AND_SPENT );
CheckWriteCoins(VALUE1_CLEAN, VALUE2_DIRTY, VALUE2_DIRTY ); CheckWriteCoins(VALUE1_CLEAN, VALUE2_DIRTY, VALUE2_DIRTY );
CheckWriteCoins(VALUE1_CLEAN, VALUE2_DIRTY_FRESH, EX_FRESH_MISAPPLIED); CheckWriteCoins(VALUE1_CLEAN, VALUE2_DIRTY_FRESH, EX_FRESH_MISAPPLIED);
@ -855,10 +853,12 @@ BOOST_AUTO_TEST_CASE(ccoins_write)
for (const MaybeCoin& parent : {MISSING, for (const MaybeCoin& parent : {MISSING,
SPENT_CLEAN, SPENT_DIRTY, SPENT_FRESH, SPENT_DIRTY_FRESH, SPENT_CLEAN, SPENT_DIRTY, SPENT_FRESH, SPENT_DIRTY_FRESH,
VALUE1_CLEAN, VALUE1_DIRTY, VALUE1_FRESH, VALUE1_DIRTY_FRESH}) { VALUE1_CLEAN, VALUE1_DIRTY, VALUE1_FRESH, VALUE1_DIRTY_FRESH}) {
for (const MaybeCoin& child : {MISSING, for (const MaybeCoin& child : {MISSING, SPENT_CLEAN, VALUE2_CLEAN}) {
SPENT_CLEAN, SPENT_FRESH, auto expected{CoinOrError{parent}};
VALUE2_CLEAN, VALUE2_FRESH}) { CheckWriteCoins(parent, child, expected);
auto expected{CoinOrError{parent}}; // TODO test failure cases as well }
for (const MaybeCoin& child : {SPENT_FRESH, VALUE2_FRESH}) {
auto expected{CoinOrError{EX_NON_DIRTY_WRITE}};
CheckWriteCoins(parent, child, expected); CheckWriteCoins(parent, child, expected);
} }
} }

View file

@ -154,9 +154,10 @@ BOOST_AUTO_TEST_CASE(linked_list_set_state)
BOOST_CHECK_EQUAL(sentinel.second.Next(), &n1); BOOST_CHECK_EQUAL(sentinel.second.Next(), &n1);
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n1); BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n1);
// Check that setting FRESH on new node inserts it after n1 // Check that setting DIRTY and FRESH on new node inserts it after n1
CCoinsCacheEntry::SetDirty(n2, sentinel);
CCoinsCacheEntry::SetFresh(n2, sentinel); CCoinsCacheEntry::SetFresh(n2, sentinel);
BOOST_CHECK(n2.second.IsFresh() && !n2.second.IsDirty()); BOOST_CHECK(n2.second.IsDirty() && n2.second.IsFresh());
BOOST_CHECK_EQUAL(n2.second.Next(), &sentinel); BOOST_CHECK_EQUAL(n2.second.Next(), &sentinel);
BOOST_CHECK_EQUAL(n2.second.Prev(), &n1); BOOST_CHECK_EQUAL(n2.second.Prev(), &n1);
BOOST_CHECK_EQUAL(n1.second.Next(), &n2); BOOST_CHECK_EQUAL(n1.second.Next(), &n2);

View file

@ -151,7 +151,9 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
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) {
if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}) { if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}
|| e.what() == std::string{"A FRESH coin was not removed when it was spent"}
|| e.what() == std::string{"A non-DIRTY coin was returned from the cursor in BatchWrite"}) {
expected_code_path = true; expected_code_path = true;
} }
} }

View file

@ -138,8 +138,6 @@ struct CacheLevel
/** Class for the base of the hierarchy (roughly simulating a memory-backed CCoinsViewDB). /** Class for the base of the hierarchy (roughly simulating a memory-backed CCoinsViewDB).
* *
* The initial state consists of the empty UTXO set. * The initial state consists of the empty UTXO set.
* Coins whose output index is 4 (mod 5) have GetCoin() always succeed after being spent.
* This exercises code paths with spent, non-DIRTY cache entries.
*/ */
class CoinsViewBottom final : public CCoinsView class CoinsViewBottom final : public CCoinsView
{ {
@ -148,14 +146,13 @@ class CoinsViewBottom final : public CCoinsView
public: public:
std::optional<Coin> GetCoin(const COutPoint& outpoint) const final std::optional<Coin> GetCoin(const COutPoint& outpoint) const final
{ {
// TODO GetCoin shouldn't return spent coins if (auto it = m_data.find(outpoint); it != m_data.end() && !it->second.IsSpent()) return it->second;
if (auto it = m_data.find(outpoint); it != m_data.end()) return it->second;
return std::nullopt; return std::nullopt;
} }
bool HaveCoin(const COutPoint& outpoint) const final bool HaveCoin(const COutPoint& outpoint) const final
{ {
return m_data.count(outpoint); return GetCoin(outpoint).has_value();
} }
uint256 GetBestBlock() const final { return {}; } uint256 GetBestBlock() const final { return {}; }
@ -166,25 +163,13 @@ public:
bool BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) final bool BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) final
{ {
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) {
if (it->second.IsDirty()) { assert(it->second.IsDirty());
if (it->second.coin.IsSpent() && (it->first.n % 5) != 4) { if (it->second.coin.IsSpent()) {
m_data.erase(it->first); m_data.erase(it->first);
} else if (cursor.WillErase(*it)) { } else if (cursor.WillErase(*it)) {
m_data[it->first] = std::move(it->second.coin); m_data[it->first] = std::move(it->second.coin);
} else {
m_data[it->first] = it->second.coin;
}
} else { } else {
/* For non-dirty entries being written, compare them with what we have. */ m_data[it->first] = it->second.coin;
auto it2 = m_data.find(it->first);
if (it->second.coin.IsSpent()) {
assert(it2 == m_data.end() || it2->second.IsSpent());
} else {
assert(it2 != m_data.end());
assert(it->second.coin.out == it2->second.out);
assert(it->second.coin.fCoinBase == it2->second.fCoinBase);
assert(it->second.coin.nHeight == it2->second.nHeight);
}
} }
} }
return true; return true;

View file

@ -12,6 +12,7 @@
#include <random.h> #include <random.h>
#include <serialize.h> #include <serialize.h>
#include <uint256.h> #include <uint256.h>
#include <util/check.h>
#include <util/vector.h> #include <util/vector.h>
#include <cassert> #include <cassert>
@ -93,7 +94,6 @@ std::vector<uint256> CCoinsViewDB::GetHeadBlocks() const {
bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlock) { bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlock) {
CDBBatch batch(*m_db); CDBBatch batch(*m_db);
size_t count = 0; size_t count = 0;
size_t changed = 0;
assert(!hashBlock.IsNull()); assert(!hashBlock.IsNull());
uint256 old_tip = GetBestBlock(); uint256 old_tip = GetBestBlock();
@ -116,17 +116,15 @@ bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashB
batch.Erase(DB_BEST_BLOCK); batch.Erase(DB_BEST_BLOCK);
batch.Write(DB_HEAD_BLOCKS, Vector(hashBlock, old_tip)); batch.Write(DB_HEAD_BLOCKS, Vector(hashBlock, old_tip));
for (auto it{cursor.Begin()}; it != cursor.End();) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) {
if (it->second.IsDirty()) { Assume(it->second.IsDirty());
CoinEntry entry(&it->first); CoinEntry entry(&it->first);
if (it->second.coin.IsSpent()) if (it->second.coin.IsSpent()) {
batch.Erase(entry); batch.Erase(entry);
else } else {
batch.Write(entry, it->second.coin); batch.Write(entry, it->second.coin);
changed++;
} }
count++; count++;
it = cursor.NextAndMaybeErase(*it);
if (batch.SizeEstimate() > m_options.batch_write_bytes) { if (batch.SizeEstimate() > m_options.batch_write_bytes) {
LogDebug(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0)); LogDebug(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0));
m_db->WriteBatch(batch); m_db->WriteBatch(batch);
@ -147,7 +145,7 @@ bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashB
LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0)); LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0));
bool ret = m_db->WriteBatch(batch); bool ret = m_db->WriteBatch(batch);
LogDebug(BCLog::COINDB, "Committed %u changed transaction outputs (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count); LogDebug(BCLog::COINDB, "Committed %u changed transaction outputs to coin database...", count);
return ret; return ret;
} }