aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/coins.cpp49
-rw-r--r--src/coins.h44
2 files changed, 65 insertions, 28 deletions
diff --git a/src/coins.cpp b/src/coins.cpp
index b71362c6a0..9e0dd6cd9f 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -77,8 +77,21 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
}
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);
@@ -152,11 +165,11 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
}
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();
@@ -169,19 +182,18 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
}
}
} 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 {
@@ -190,11 +202,10 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
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.
}
}
}
diff --git a/src/coins.h b/src/coins.h
index e71c8a47bc..d5484bcf65 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -109,19 +109,45 @@ public:
}
};
+/**
+ * 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
{
Coin coin; // The actual cached data.
unsigned char flags;
enum Flags {
- DIRTY = (1 << 0), // This cache entry is potentially different from the version in the parent view.
- FRESH = (1 << 1), // The parent view does not have this entry (or it is pruned).
- /* 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.
+ /**
+ * 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),
+ /**
+ * 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),
};
CCoinsCacheEntry() : flags(0) {}
@@ -246,7 +272,7 @@ public:
bool HaveCoinInCache(const COutPoint &outpoint) const;
/**
- * Return a reference to Coin in the cache, or a pruned one if not found. This is
+ * 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 scope.
@@ -258,8 +284,8 @@ public:
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);