diff --git a/src/coins.h b/src/coins.h --- a/src/coins.h +++ b/src/coins.h @@ -99,21 +99,48 @@ } }; +/** + * A Coin in one level of the coins database caching hierarchy. + * + * A coin can either be: + * - unspent or spent (in which case the Coin object will be nulled out - see + * Coin.Clear()) + * - DIRTY or not DIRTY + * - FRESH or not FRESH + * + * Out of these 2^3 = 8 states, only some combinations are valid: + * - 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, 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) + */ struct CCoinsCacheEntry { // The actual cached data. Coin coin; uint8_t flags; enum Flags { - // This cache entry is potentially different from the version in the - // parent view. + /** + * DIRTY means the CCoinsCacheEntry is potentially different from the + * version in the parent cache. Failure to mark a coin as DIRTY when + * it is potentially different from the parent cache will cause a + * consensus failure, since the coin's state won't get written to the + * parent when the cache is flushed. + */ DIRTY = (1 << 0), - // The parent view does not have this entry (or it is pruned). + /** + * FRESH means the parent cache does not have this coin or that it is a + * spent coin in the parent cache. If a FRESH coin in the cache is + * later spent, it can be deleted entirely and doesn't ever need to be + * flushed to the parent. This is a performance optimization. Marking a + * coin as FRESH when it exists unspent in the parent cache will cause a + * consensus failure, since it might not be deleted from the parent + * when this cache is flushed. + */ FRESH = (1 << 1), - /* Note that FRESH is a performance optimization with which we can erase - coins that are fully spent if we know we do not need to flush the - changes to the parent cache. It is always safe to not mark FRESH if - that condition is not guaranteed. */ }; CCoinsCacheEntry() : flags(0) {} @@ -241,7 +268,7 @@ bool HaveCoinInCache(const COutPoint &outpoint) const; /** - * Return a reference to Coin in the cache, or a pruned one if not found. + * Return a reference to Coin in the cache, or coinEmpty if not found. * This is more efficient than GetCoin. * * Generally, do not hold the reference returned for more than a short @@ -253,8 +280,8 @@ const Coin &AccessCoin(const COutPoint &output) const; /** - * Add a coin. Set potential_overwrite to true if a non-pruned version may - * already exist. + * Add a coin. Set potential_overwrite to true if an unspent version may + * already exist in the cache. */ void AddCoin(const COutPoint &outpoint, Coin coin, bool potential_overwrite); diff --git a/src/coins.cpp b/src/coins.cpp --- a/src/coins.cpp +++ b/src/coins.cpp @@ -117,9 +117,22 @@ } if (!possible_overwrite) { if (!it->second.coin.IsSpent()) { - throw std::logic_error( - "Adding new coin that replaces non-pruned entry"); + 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 + // 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. + // If we mark it FRESH and then spend it before the cache is flushed + // we would remove it from this cache and would never flush spentness + // to the parent cache. + // + // 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 + // 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 + // DIRTY, then it can be marked FRESH. fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY); } it->second.coin = std::move(coin); @@ -202,12 +215,12 @@ } CCoinsMap::iterator itUs = cacheCoins.find(it->first); if (itUs == cacheCoins.end()) { - // The parent cache does not have an entry, while the child does - // We can ignore it if it's both FRESH and pruned in the child + // 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 if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) { - // Otherwise we will need to create it in the parent and - // move the data up and mark it as dirty + // Create the coin in the parent cache, move the data up + // and mark it as dirty. CCoinsCacheEntry &entry = cacheCoins[it->first]; entry.coin = std::move(it->second.coin); cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); @@ -220,23 +233,21 @@ } } } else { - // Assert that the child cache entry was not marked FRESH if the - // parent cache entry has unspent outputs. If this ever happens, - // it means the FRESH flag was misapplied and there is a logic - // error in the calling code. + // Found the entry in the parent cache if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) { - throw std::logic_error("FRESH flag misapplied to cache " - "entry for base transaction with " - "spendable outputs"); + // The coin was marked FRESH in the child cache, but the coin + // exists in the parent cache. If this ever happens, it means + // the FRESH flag was misapplied and there is a logic error in + // the calling code. + throw std::logic_error("FRESH flag misapplied to coin that " + "exists in parent cache"); } - // Found the entry in the parent cache if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) { - // The grandparent does not have an entry, and the child is - // modified and being pruned. This means we can just delete - // it from the parent. + // The grandparent cache does not have an entry, and the coin + // has been spent. We can just delete it from the parent cache. cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); cacheCoins.erase(itUs); } else { @@ -245,11 +256,10 @@ itUs->second.coin = std::move(it->second.coin); cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); itUs->second.flags |= CCoinsCacheEntry::DIRTY; - // NOTE: It is possible the child has a FRESH flag here in - // the event the entry we found in the parent is pruned. But - // we must not copy that FRESH flag to the parent as that - // pruned state likely still needs to be communicated to the - // grandparent. + // NOTE: It isn't safe to mark the coin as FRESH in the parent + // cache. If it already existed and was spent in the parent + // cache then marking it FRESH would prevent that spentness + // from being flushed to the grandparent. } } }