Skip to content

Commit c205979

Browse files
author
John Newbery
committed
[docs] Improve commenting in coins.cpp|h
Remove references to 'pruned' coins, which don't exist since the move to per-txout coins db.
1 parent 646f0ad commit c205979

File tree

2 files changed

+65
-28
lines changed

2 files changed

+65
-28
lines changed

‎src/coins.cpp‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,21 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
7777
}
7878
if (!possible_overwrite) {
7979
if (!it->second.coin.IsSpent()) {
80-
throw std::logic_error("Adding new coin that replaces non-pruned entry");
80+
throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)");
8181
}
82+
// If the coin exists in this cache as a spent coin and is DIRTY, then
83+
// its spentness hasn't been flushed to the parent cache. We're
84+
// re-adding the coin to this cache now but we can't mark it as FRESH.
85+
// If we mark it FRESH and then spend it before the cache is flushed
86+
// we would remove it from this cache and would never flush spentness
87+
// to the parent cache.
88+
//
89+
// Re-adding a spent coin can happen in the case of a re-org (the coin
90+
// is 'spent' when the block adding it is disconnected and then
91+
// re-added when it is also added in a newly connected block).
92+
//
93+
// If the coin doesn't exist in the current cache, or is spent but not
94+
// DIRTY, then it can be marked FRESH.
8295
fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY);
8396
}
8497
it->second.coin = std::move(coin);
@@ -152,11 +165,11 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
152165
}
153166
CCoinsMap::iterator itUs = cacheCoins.find(it->first);
154167
if (itUs == cacheCoins.end()) {
155-
// The parent cache does not have an entry, while the child does
156-
// We can ignore it if it's both FRESH and pruned in the child
168+
// The parent cache does not have an entry, while the child cache does.
169+
// We can ignore it if it's both spent and FRESH in the child
157170
if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) {
158-
// Otherwise we will need to create it in the parent
159-
// and move the data up and mark it as dirty
171+
// Create the coin in the parent cache, move the data up
172+
// and mark it as dirty.
160173
CCoinsCacheEntry& entry = cacheCoins[it->first];
161174
entry.coin = std::move(it->second.coin);
162175
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
@@ -169,19 +182,18 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
169182
}
170183
}
171184
} else {
172-
// Assert that the child cache entry was not marked FRESH if the
173-
// parent cache entry has unspent outputs. If this ever happens,
174-
// it means the FRESH flag was misapplied and there is a logic
175-
// error in the calling code.
185+
// Found the entry in the parent cache
176186
if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) {
177-
throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs");
187+
// The coin was marked FRESH in the child cache, but the coin
188+
// exists in the parent cache. If this ever happens, it means
189+
// the FRESH flag was misapplied and there is a logic error in
190+
// the calling code.
191+
throw std::logic_error("FRESH flag misapplied to coin that exists in parent cache");
178192
}
179193

180-
// Found the entry in the parent cache
181194
if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
182-
// The grandparent does not have an entry, and the child is
183-
// modified and being pruned. This means we can just delete
184-
// it from the parent.
195+
// The grandparent cache does not have an entry, and the coin
196+
// has been spent. We can just delete it from the parent cache.
185197
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
186198
cacheCoins.erase(itUs);
187199
} else {
@@ -190,11 +202,10 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
190202
itUs->second.coin = std::move(it->second.coin);
191203
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
192204
itUs->second.flags |= CCoinsCacheEntry::DIRTY;
193-
// NOTE: It is possible the child has a FRESH flag here in
194-
// the event the entry we found in the parent is pruned. But
195-
// we must not copy that FRESH flag to the parent as that
196-
// pruned state likely still needs to be communicated to the
197-
// grandparent.
205+
// NOTE: It isn't safe to mark the coin as FRESH in the parent
206+
// cache. If it already existed and was spent in the parent
207+
// cache then marking it FRESH would prevent that spentness
208+
// from being flushed to the grandparent.
198209
}
199210
}
200211
}

‎src/coins.h‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,45 @@ class SaltedOutpointHasher
109109
}
110110
};
111111

112+
/**
113+
* A Coin in one level of the coins database caching hierarchy.
114+
*
115+
* A coin can either be:
116+
* - unspent or spent (in which case the Coin object will be nulled out - see Coin.Clear())
117+
* - DIRTY or not DIRTY
118+
* - FRESH or not FRESH
119+
*
120+
* Out of these 2^3 = 8 states, only some combinations are valid:
121+
* - unspent, FRESH, DIRTY (e.g. a new coin created in the cache)
122+
* - unspent, not FRESH, DIRTY (e.g. a coin changed in the cache during a reorg)
123+
* - unspent, not FRESH, not DIRTY (e.g. an unspent coin fetched from the parent cache)
124+
* - spent, FRESH, not DIRTY (e.g. a spent coin fetched from the parent cache)
125+
* - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent)
126+
*/
112127
struct CCoinsCacheEntry
113128
{
114129
Coin coin; // The actual cached data.
115130
unsigned char flags;
116131

117132
enum Flags {
118-
DIRTY = (1 << 0), // This cache entry is potentially different from the version in the parent view.
119-
FRESH = (1 << 1), // The parent view does not have this entry (or it is pruned).
120-
/* Note that FRESH is a performance optimization with which we can
121-
* erase coins that are fully spent if we know we do not need to
122-
* flush the changes to the parent cache. It is always safe to
123-
* not mark FRESH if that condition is not guaranteed.
133+
/**
134+
* DIRTY means the CCoinsCacheEntry is potentially different from the
135+
* version in the parent cache. Failure to mark a coin as DIRTY when
136+
* it is potentially different from the parent cache will cause a
137+
* consensus failure, since the coin's state won't get written to the
138+
* parent when the cache is flushed.
139+
*/
140+
DIRTY = (1 << 0),
141+
/**
142+
* FRESH means the parent cache does not have this coin or that it is a
143+
* spent coin in the parent cache. If a FRESH coin in the cache is
144+
* later spent, it can be deleted entirely and doesn't ever need to be
145+
* flushed to the parent. This is a performance optimization. Marking a
146+
* coin as FRESH when it exists unspent in the parent cache will cause a
147+
* consensus failure, since it might not be deleted from the parent
148+
* when this cache is flushed.
124149
*/
150+
FRESH = (1 << 1),
125151
};
126152

127153
CCoinsCacheEntry() : flags(0) {}
@@ -246,7 +272,7 @@ class CCoinsViewCache : public CCoinsViewBacked
246272
bool HaveCoinInCache(const COutPoint &outpoint) const;
247273

248274
/**
249-
* Return a reference to Coin in the cache, or a pruned one if not found. This is
275+
* Return a reference to Coin in the cache, or coinEmpty if not found. This is
250276
* more efficient than GetCoin.
251277
*
252278
* Generally, do not hold the reference returned for more than a short scope.
@@ -258,8 +284,8 @@ class CCoinsViewCache : public CCoinsViewBacked
258284
const Coin& AccessCoin(const COutPoint &output) const;
259285

260286
/**
261-
* Add a coin. Set potential_overwrite to true if a non-pruned version may
262-
* already exist.
287+
* Add a coin. Set potential_overwrite to true if an unspent version may
288+
* already exist in the cache.
263289
*/
264290
void AddCoin(const COutPoint& outpoint, Coin&& coin, bool potential_overwrite);
265291

0 commit comments

Comments
 (0)