From bb00357add2c44d4b2cf4341be963f6bb9bee63f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 30 Jan 2023 11:32:59 -0500 Subject: [PATCH 1/4] Make test/fuzz/coins_view exercise CCoinsViewCache::Sync() --- src/test/fuzz/coins_view.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 46026d8df3..e75dc3ce91 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -74,6 +74,9 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view) [&] { (void)coins_view_cache.Flush(); }, + [&] { + (void)coins_view_cache.Sync(); + }, [&] { coins_view_cache.SetBestBlock(ConsumeUInt256(fuzzed_data_provider)); }, From 98db35c2f81098b179136ed6c5f2a8570b3b5900 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 30 Jan 2023 13:13:20 -0500 Subject: [PATCH 2/4] Follow coding style for named arguments --- src/coins.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index ecbd625cd7..e240136576 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -249,7 +249,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn } bool CCoinsViewCache::Flush() { - bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ true); + bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/true); cacheCoins.clear(); cachedCoinsUsage = 0; return fOk; @@ -257,7 +257,7 @@ bool CCoinsViewCache::Flush() { bool CCoinsViewCache::Sync() { - bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ false); + bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/false); // Instead of clearing `cacheCoins` as we would in Flush(), just clear the // FRESH/DIRTY flags of any coin that isn't spent. for (auto it = cacheCoins.begin(); it != cacheCoins.end(); ) { From 941feb6ca28522806c4710f85ae5673abdbdb0b9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 30 Jan 2023 11:42:00 -0500 Subject: [PATCH 3/4] Avoid unclear {it = ++it;} --- src/test/coins_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 92bad8dd2e..312f417129 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -55,7 +55,7 @@ public: bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock, bool erase = true) override { - for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = erase ? mapCoins.erase(it) : ++it) { + for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = erase ? mapCoins.erase(it) : std::next(it)) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Same optimization used in CCoinsViewDB is to only write dirty entries. map_[it->first] = it->second.coin; From 2e16054a66b0576ec93d4032d6b74f0930a44fef Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 30 Jan 2023 11:45:14 -0500 Subject: [PATCH 4/4] Add assertions that BatchWrite(erase=true) erases --- src/coins.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coins.cpp b/src/coins.cpp index e240136576..31ac67674a 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -250,7 +250,10 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn bool CCoinsViewCache::Flush() { bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/true); - cacheCoins.clear(); + if (fOk && !cacheCoins.empty()) { + /* BatchWrite must erase all cacheCoins elements when erase=true. */ + throw std::logic_error("Not all cached coins were erased"); + } cachedCoinsUsage = 0; return fOk; }