Skip to content

Commit 8126551

Browse files
Merge bitcoin/bitcoin#27011: Add simulation-based CCoinsViewCache fuzzer
561848a Exercise non-DIRTY spent coins in caches in fuzz test (Pieter Wuille) 59e6828 Add deterministic mode to CCoinsViewCache (Pieter Wuille) b0ff310 Add CCoinsViewCache::SanityCheck() and use it in fuzz test (Pieter Wuille) 3c9cea1 Add simulation-based CCoinsViewCache fuzzer (Pieter Wuille) Pull request description: The fuzzer goes through a sequence of operations that get applied to both a real stack of `CCoinsViewCache` objects, and to simulation data, comparing the two at the end. ACKs for top commit: jamesob: re-ACK bitcoin/bitcoin@561848a dergoegge: Code review ACK 561848a Tree-SHA512: 68634f251fdb39436b128ecba093f651bff12ac11508dc9885253e57fd21efd44edf3b22b0f821c228175ec507df7d46c7f9f5404fc1eb8187fdbd136a5d5ee2
2 parents 141115a + 561848a commit 8126551

File tree

7 files changed

+514
-6
lines changed

7 files changed

+514
-6
lines changed

src/Makefile.test.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ test_fuzz_fuzz_SOURCES = \
248248
test/fuzz/chain.cpp \
249249
test/fuzz/checkqueue.cpp \
250250
test/fuzz/coins_view.cpp \
251+
test/fuzz/coinscache_sim.cpp \
251252
test/fuzz/connman.cpp \
252253
test/fuzz/crypto.cpp \
253254
test/fuzz/crypto_aes256.cpp \

src/coins.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock,
3232
std::unique_ptr<CCoinsViewCursor> CCoinsViewBacked::Cursor() const { return base->Cursor(); }
3333
size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); }
3434

35-
CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn) : CCoinsViewBacked(baseIn) {}
35+
CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) :
36+
CCoinsViewBacked(baseIn), m_deterministic(deterministic),
37+
cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic))
38+
{}
3639

3740
size_t CCoinsViewCache::DynamicMemoryUsage() const {
3841
return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage;
@@ -311,7 +314,24 @@ void CCoinsViewCache::ReallocateCache()
311314
// Cache should be empty when we're calling this.
312315
assert(cacheCoins.size() == 0);
313316
cacheCoins.~CCoinsMap();
314-
::new (&cacheCoins) CCoinsMap();
317+
::new (&cacheCoins) CCoinsMap(0, SaltedOutpointHasher(/*deterministic=*/m_deterministic));
318+
}
319+
320+
void CCoinsViewCache::SanityCheck() const
321+
{
322+
size_t recomputed_usage = 0;
323+
for (const auto& [_, entry] : cacheCoins) {
324+
unsigned attr = 0;
325+
if (entry.flags & CCoinsCacheEntry::DIRTY) attr |= 1;
326+
if (entry.flags & CCoinsCacheEntry::FRESH) attr |= 2;
327+
if (entry.coin.IsSpent()) attr |= 4;
328+
// Only 5 combinations are possible.
329+
assert(attr != 2 && attr != 4 && attr != 7);
330+
331+
// Recompute cachedCoinsUsage.
332+
recomputed_usage += entry.coin.DynamicMemoryUsage();
333+
}
334+
assert(recomputed_usage == cachedCoinsUsage);
315335
}
316336

317337
static const size_t MIN_TRANSACTION_OUTPUT_WEIGHT = WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut(), PROTOCOL_VERSION);

src/coins.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ class CCoinsViewBacked : public CCoinsView
211211
/** CCoinsView that adds a memory cache for transactions to another CCoinsView */
212212
class CCoinsViewCache : public CCoinsViewBacked
213213
{
214+
private:
215+
const bool m_deterministic;
216+
214217
protected:
215218
/**
216219
* Make mutable so that we can "fill the cache" even from Get-methods
@@ -223,7 +226,7 @@ class CCoinsViewCache : public CCoinsViewBacked
223226
mutable size_t cachedCoinsUsage{0};
224227

225228
public:
226-
CCoinsViewCache(CCoinsView *baseIn);
229+
CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false);
227230

228231
/**
229232
* By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache.
@@ -320,6 +323,9 @@ class CCoinsViewCache : public CCoinsViewBacked
320323
//! See: https://stackoverflow.com/questions/42114044/how-to-release-unordered-map-memory
321324
void ReallocateCache();
322325

326+
//! Run an internal sanity check on the cache data structure. */
327+
void SanityCheck() const;
328+
323329
private:
324330
/**
325331
* @note this is marked const, but may actually append to `cacheCoins`, increasing

src/test/fuzz/coins_view.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view)
4646
{
4747
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
4848
CCoinsView backend_coins_view;
49-
CCoinsViewCache coins_view_cache{&backend_coins_view};
49+
CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true};
5050
COutPoint random_out_point;
5151
Coin random_coin;
5252
CMutableTransaction random_mutable_transaction;

0 commit comments

Comments
 (0)