This commit is contained in:
Andrew Toth 2024-04-29 04:30:11 +02:00 committed by GitHub
commit 7605f53fae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 360 additions and 79 deletions

View File

@ -84,6 +84,7 @@ BITCOIN_TESTS =\
test/bswap_tests.cpp \
test/checkqueue_tests.cpp \
test/coins_tests.cpp \
test/coinscachepair_tests.cpp \
test/coinstatsindex_tests.cpp \
test/common_url_tests.cpp \
test/compilerbug_tests.cpp \

View File

@ -12,7 +12,7 @@
bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; }
uint256 CCoinsView::GetBestBlock() const { return uint256(); }
std::vector<uint256> CCoinsView::GetHeadBlocks() const { return std::vector<uint256>(); }
bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) { return false; }
bool CCoinsView::BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase) { return false; }
std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; }
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
@ -27,7 +27,7 @@ bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base->
uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); }
std::vector<uint256> CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); }
void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; }
bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) { return base->BatchWrite(mapCoins, hashBlock, erase); }
bool CCoinsViewBacked::BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase) { return base->BatchWrite(pairs, hashBlock, will_erase); }
std::unique_ptr<CCoinsViewCursor> CCoinsViewBacked::Cursor() const { return base->Cursor(); }
size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); }
@ -51,7 +51,7 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const
if (ret->second.coin.IsSpent()) {
// The parent only has an empty entry for this outpoint; we can consider our
// version as fresh.
ret->second.flags = CCoinsCacheEntry::FRESH;
ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_flagged_head);
}
cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage();
return ret;
@ -93,10 +93,10 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
//
// 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);
fresh = !(it->second.GetFlags() & CCoinsCacheEntry::DIRTY);
}
it->second.coin = std::move(coin);
it->second.flags |= CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0);
it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0), *it, m_flagged_head);
cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
TRACE5(utxocache, add,
outpoint.hash.data(),
@ -108,10 +108,13 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin) {
cachedCoinsUsage += coin.DynamicMemoryUsage();
cacheCoins.emplace(
auto [it, inserted] = cacheCoins.emplace(
std::piecewise_construct,
std::forward_as_tuple(std::move(outpoint)),
std::forward_as_tuple(std::move(coin), CCoinsCacheEntry::DIRTY));
std::forward_as_tuple(std::move(coin)));
if (inserted) {
it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_flagged_head);
}
}
void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool check_for_overwrite) {
@ -138,10 +141,10 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
if (moveout) {
*moveout = std::move(it->second.coin);
}
if (it->second.flags & CCoinsCacheEntry::FRESH) {
if (it->second.GetFlags() & CCoinsCacheEntry::FRESH) {
cacheCoins.erase(it);
} else {
it->second.flags |= CCoinsCacheEntry::DIRTY;
it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_flagged_head);
it->second.coin.Clear();
}
return true;
@ -178,42 +181,42 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) {
hashBlock = hashBlockIn;
}
bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn, bool erase) {
for (CCoinsMap::iterator it = mapCoins.begin();
it != mapCoins.end();
it = erase ? mapCoins.erase(it) : std::next(it)) {
bool CCoinsViewCache::BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlockIn, bool will_erase) {
for (auto it{pairs};
it != nullptr;
it = it->second.Next(/*clear_flags=*/will_erase)) {
// Ignore non-dirty entries (optimization).
if (!(it->second.flags & CCoinsCacheEntry::DIRTY)) {
if (!(it->second.GetFlags() & CCoinsCacheEntry::DIRTY)) {
continue;
}
CCoinsMap::iterator itUs = cacheCoins.find(it->first);
if (itUs == cacheCoins.end()) {
// 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())) {
if (!(it->second.GetFlags() & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) {
// Create the coin in the parent cache, move the data up
// and mark it as dirty.
CCoinsCacheEntry& entry = cacheCoins[it->first];
if (erase) {
// The `move` call here is purely an optimization; we rely on the
// `mapCoins.erase` call in the `for` expression to actually remove
// the entry from the child map.
itUs = cacheCoins.try_emplace(it->first).first;
CCoinsCacheEntry& entry{itUs->second};
if (will_erase) {
// Since the child will erase the coins after this batch write,
// we can move the coin into the parent instead of copying it
entry.coin = std::move(it->second.coin);
} else {
entry.coin = it->second.coin;
}
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
entry.flags = CCoinsCacheEntry::DIRTY;
entry.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_flagged_head);
// We can mark it FRESH in the parent if it was FRESH in the child
// Otherwise it might have just been flushed from the parent's cache
// and already exist in the grandparent
if (it->second.flags & CCoinsCacheEntry::FRESH) {
entry.flags |= CCoinsCacheEntry::FRESH;
if (it->second.GetFlags() & CCoinsCacheEntry::FRESH) {
entry.AddFlags(CCoinsCacheEntry::FRESH, *itUs, m_flagged_head);
}
}
} else {
// Found the entry in the parent cache
if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) {
if ((it->second.GetFlags() & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) {
// 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
@ -221,7 +224,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
throw std::logic_error("FRESH flag misapplied to coin that exists in parent cache");
}
if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
if ((itUs->second.GetFlags() & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
// 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();
@ -229,16 +232,15 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
} else {
// A normal modification.
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
if (erase) {
// The `move` call here is purely an optimization; we rely on the
// `mapCoins.erase` call in the `for` expression to actually remove
// the entry from the child map.
if (will_erase) {
// Since the child will erase the coins after this batch write,
// we can move the coin into the parent instead of copying it
itUs->second.coin = std::move(it->second.coin);
} else {
itUs->second.coin = it->second.coin;
}
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
itUs->second.flags |= CCoinsCacheEntry::DIRTY;
itUs->second.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_flagged_head);
// 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
@ -251,11 +253,11 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
}
bool CCoinsViewCache::Flush() {
bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/true);
const auto fOk{base->BatchWrite(m_flagged_head.second.Next(), hashBlock, /*will_erase=*/true)};
if (fOk) {
if (!cacheCoins.empty()) {
/* BatchWrite must erase all cacheCoins elements when erase=true. */
throw std::logic_error("Not all cached coins were erased");
if (m_flagged_head.second.Next() != nullptr) {
/* BatchWrite must unset all flagged entries when will_erase=true. */
throw std::logic_error("Not all flagged cached coins were unset");
}
ReallocateCache();
}
@ -265,17 +267,16 @@ bool CCoinsViewCache::Flush() {
bool CCoinsViewCache::Sync()
{
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(); ) {
const auto fOk{base->BatchWrite(m_flagged_head.second.Next(), hashBlock, /*will_erase=*/false)};
// Instead of clearing `cacheCoins` as we would in Flush(), just erase the
// spent coins and clear the FRESH/DIRTY flags of any coin that isn't spent.
for (auto it{m_flagged_head.second.Next()}; it != nullptr; ) {
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
if (it->second.coin.IsSpent()) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
it = cacheCoins.erase(it);
} else {
it->second.flags = 0;
++it;
cacheCoins.erase(it->first);
}
it = next_entry;
}
return fOk;
}
@ -283,7 +284,7 @@ bool CCoinsViewCache::Sync()
void CCoinsViewCache::Uncache(const COutPoint& hash)
{
CCoinsMap::iterator it = cacheCoins.find(hash);
if (it != cacheCoins.end() && it->second.flags == 0) {
if (it != cacheCoins.end() && it->second.GetFlags() == 0) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
TRACE5(utxocache, uncache,
hash.hash.data(),
@ -313,8 +314,6 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const
void CCoinsViewCache::ReallocateCache()
{
// Cache should be empty when we're calling this.
assert(cacheCoins.size() == 0);
cacheCoins.~CCoinsMap();
m_cache_coins_memory_resource.~CCoinsMapMemoryResource();
::new (&m_cache_coins_memory_resource) CCoinsMapMemoryResource{};
@ -326,8 +325,8 @@ void CCoinsViewCache::SanityCheck() const
size_t recomputed_usage = 0;
for (const auto& [_, entry] : cacheCoins) {
unsigned attr = 0;
if (entry.flags & CCoinsCacheEntry::DIRTY) attr |= 1;
if (entry.flags & CCoinsCacheEntry::FRESH) attr |= 2;
if (entry.GetFlags() & CCoinsCacheEntry::DIRTY) attr |= 1;
if (entry.GetFlags() & CCoinsCacheEntry::FRESH) attr |= 2;
if (entry.coin.IsSpent()) attr |= 4;
// Only 5 combinations are possible.
assert(attr != 2 && attr != 4 && attr != 7);

View File

@ -86,6 +86,9 @@ public:
}
};
struct CCoinsCacheEntry;
using CoinsCachePair = std::pair<const COutPoint, CCoinsCacheEntry>;
/**
* A Coin in one level of the coins database caching hierarchy.
*
@ -103,8 +106,31 @@ public:
*/
struct CCoinsCacheEntry
{
private:
/**
* These are used to create a doubly linked list of flagged entries.
* They are set in AddFlags and unset in ClearFlags.
* A flagged entry is any entry that is either DIRTY, FRESH, or both.
*
* DIRTY entries are tracked so that only modified entries can be passed to
* the parent cache to do a batch write. This is a performance optimization
* compared to giving all entries in the cache to the parent and having the
* parent scan for only modified entries.
*
* FRESH entries are tracked because a FRESH-but-not-DIRTY entry can only be
* a spent coin that exists in the parent cache. When performing a batch
* write but not clearing the cache (CCoinsViewCache::Sync), we must also
* erase all spent entries. Since only DIRTY or FRESH entries can be spent,
* we can track them in the linked list to erase them. This is a performance
* optimization compared to scanning all cache entries to find any that are
* spent-but-not-DIRTY.
*/
CoinsCachePair* m_prev{nullptr};
CoinsCachePair* m_next{nullptr};
uint8_t m_flags{0};
public:
Coin coin; // The actual cached data.
unsigned char flags;
enum Flags {
/**
@ -127,9 +153,58 @@ struct CCoinsCacheEntry
FRESH = (1 << 1),
};
CCoinsCacheEntry() : flags(0) {}
explicit CCoinsCacheEntry(Coin&& coin_) : coin(std::move(coin_)), flags(0) {}
CCoinsCacheEntry(Coin&& coin_, unsigned char flag) : coin(std::move(coin_)), flags(flag) {}
CCoinsCacheEntry() = default;
explicit CCoinsCacheEntry(Coin&& coin_) : coin(std::move(coin_)) {}
~CCoinsCacheEntry()
{
// We must always clear the flags when destroying this to remove it from
// the flagged linked list
ClearFlags();
}
//! Adding a flag also requires a self reference to the pair that contains
//! this entry in the CCoinsCache map and a reference to the head of the
//! flagged pair linked list.
inline void AddFlags(uint8_t flags, CoinsCachePair& self, CoinsCachePair& head)
{
if (flags == 0) return;
if (m_flags != 0) {
m_flags |= flags;
return;
}
m_prev = &head;
m_next = head.second.m_next;
head.second.m_next = &self;
if (m_next != nullptr) {
m_next->second.m_prev = &self;
}
m_flags = flags;
}
inline void ClearFlags()
{
if (m_flags == 0) return;
m_prev->second.m_next = m_next;
if (m_next != nullptr) {
m_next->second.m_prev = m_prev;
}
m_prev = nullptr;
m_next = nullptr;
m_flags = 0;
}
inline uint8_t GetFlags() const { return m_flags; }
//! Get the next entry in the flagged linked list, optionally removing the
//! current entry and clearing the flags.
inline CoinsCachePair* Next(bool clear_flags = false)
{
const auto ret{m_next};
if (clear_flags) {
ClearFlags();
}
return ret;
}
};
/**
@ -144,8 +219,8 @@ using CCoinsMap = std::unordered_map<COutPoint,
CCoinsCacheEntry,
SaltedOutpointHasher,
std::equal_to<COutPoint>,
PoolAllocator<std::pair<const COutPoint, CCoinsCacheEntry>,
sizeof(std::pair<const COutPoint, CCoinsCacheEntry>) + sizeof(void*) * 4>>;
PoolAllocator<CoinsCachePair,
sizeof(CoinsCachePair) + sizeof(void*) * 4>>;
using CCoinsMapMemoryResource = CCoinsMap::allocator_type::ResourceType;
@ -191,8 +266,10 @@ public:
virtual std::vector<uint256> GetHeadBlocks() const;
//! Do a bulk modification (multiple Coin changes + BestBlock change).
//! The passed mapCoins can be modified.
virtual bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true);
//! The passed pairs are a linked list that can be modified.
//! If will_erase is true, the coins will be erased by the caller afterwards,
//! so the coins can be moved out of the pairs instead of copied
virtual bool BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase = true);
//! Get a cursor to iterate over the whole state
virtual std::unique_ptr<CCoinsViewCursor> Cursor() const;
@ -218,7 +295,7 @@ public:
uint256 GetBestBlock() const override;
std::vector<uint256> GetHeadBlocks() const override;
void SetBackend(CCoinsView &viewIn);
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override;
bool BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase = true) override;
std::unique_ptr<CCoinsViewCursor> Cursor() const override;
size_t EstimateSize() const override;
};
@ -237,6 +314,12 @@ protected:
*/
mutable uint256 hashBlock;
mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{};
/**
* The head of the flagged entry linked list.
* Destroy it after cacheCoins, since any existing entries could still be
* flagged and reference the head in CCoinsCacheEntry's destructor
*/
mutable CoinsCachePair m_flagged_head;
mutable CCoinsMap cacheCoins;
/* Cached dynamic memory usage for the inner Coin objects. */
@ -255,7 +338,7 @@ public:
bool HaveCoin(const COutPoint &outpoint) const override;
uint256 GetBestBlock() const override;
void SetBestBlock(const uint256 &hashBlock);
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override;
bool BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase = true) override;
std::unique_ptr<CCoinsViewCursor> Cursor() const override {
throw std::logic_error("CCoinsViewCache cursor iteration not supported.");
}

View File

@ -55,10 +55,10 @@ public:
uint256 GetBestBlock() const override { return hashBestBlock_; }
bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock, bool erase = true) override
bool BatchWrite(CoinsCachePair* pairs, const uint256& hashBlock, bool will_erase = true) override
{
for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = erase ? mapCoins.erase(it) : std::next(it)) {
if (it->second.flags & CCoinsCacheEntry::DIRTY) {
for (auto it{pairs}; it != nullptr; it = it->second.Next(will_erase)) {
if (it->second.GetFlags() & CCoinsCacheEntry::DIRTY) {
// Same optimization used in CCoinsViewDB is to only write dirty entries.
map_[it->first] = it->second.coin;
if (it->second.coin.IsSpent() && InsecureRandRange(3) == 0) {
@ -92,6 +92,7 @@ public:
}
CCoinsMap& map() const { return cacheCoins; }
CoinsCachePair& entries() const { return m_flagged_head; }
size_t& usage() const { return cachedCoinsUsage; }
};
@ -579,7 +580,7 @@ static void SetCoinsValue(CAmount value, Coin& coin)
}
}
static size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags)
static size_t InsertCoinsMapEntry(CCoinsMap& map, CoinsCachePair& flagged_head, CAmount value, char flags)
{
if (value == ABSENT) {
assert(flags == NO_ENTRY);
@ -587,10 +588,10 @@ static size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags)
}
assert(flags != NO_ENTRY);
CCoinsCacheEntry entry;
entry.flags = flags;
SetCoinsValue(value, entry.coin);
auto inserted = map.emplace(OUTPOINT, std::move(entry));
assert(inserted.second);
inserted.first->second.AddFlags(flags, *inserted.first, flagged_head);
return inserted.first->second.coin.DynamicMemoryUsage();
}
@ -606,17 +607,21 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags, const C
} else {
value = it->second.coin.out.nValue;
}
flags = it->second.flags;
flags = it->second.GetFlags();
assert(flags != NO_ENTRY);
}
}
void WriteCoinsViewEntry(CCoinsView& view, CAmount value, char flags)
{
// flagged_head must go out of scope and be destroyed after map.
// Any remaining flagged entries in map need to reference the head
// when they are destroyed.
CoinsCachePair flagged_head{};
CCoinsMapMemoryResource resource;
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
InsertCoinsMapEntry(map, value, flags);
BOOST_CHECK(view.BatchWrite(map, {}));
InsertCoinsMapEntry(map, flagged_head, value, flags);
BOOST_CHECK(view.BatchWrite(flagged_head.second.Next(), {}));
}
class SingleEntryCacheTest
@ -625,7 +630,7 @@ public:
SingleEntryCacheTest(CAmount base_value, CAmount cache_value, char cache_flags)
{
WriteCoinsViewEntry(base, base_value, base_value == ABSENT ? NO_ENTRY : DIRTY);
cache.usage() += InsertCoinsMapEntry(cache.map(), cache_value, cache_flags);
cache.usage() += InsertCoinsMapEntry(cache.map(), cache.entries(), cache_value, cache_flags);
}
CCoinsView root;

View File

@ -0,0 +1,186 @@
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <coins.h>
#include <boost/test/unit_test.hpp>
#include <list>
BOOST_AUTO_TEST_SUITE(coinscachepair_tests)
static constexpr auto NUM_NODES{4};
std::list<CoinsCachePair> CreatePairs(CoinsCachePair& head)
{
std::list<CoinsCachePair> nodes;
for (auto i{0}; i < NUM_NODES; ++i) {
nodes.emplace_front();
auto node{nodes.begin()};
node->second.AddFlags(CCoinsCacheEntry::DIRTY, *node, head);
BOOST_CHECK_EQUAL(node->second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(head.second.Next(), &(*node));
if (i > 0) {
BOOST_CHECK_EQUAL(&(*std::next(node)), node->second.Next());
} else {
BOOST_CHECK_EQUAL(nullptr, node->second.Next());
}
}
return nodes;
}
BOOST_AUTO_TEST_CASE(linked_list_iteration)
{
CoinsCachePair head;
auto nodes{CreatePairs(head)};
// Check iterating through pairs is identical to iterating through a list
auto lit{nodes.begin()};
for (auto it{head.second.Next()}; it != nullptr; it = it->second.Next(/*clear_flags=*/false), ++lit) {
BOOST_CHECK_EQUAL(&(*lit), it);
}
// Check iterating through pairs is identical to iterating through a list
// Clear the flags during iteration
lit = nodes.begin();
for (auto it{head.second.Next()}; it != nullptr; it = it->second.Next(/*clear_flags=*/true), ++lit) {
BOOST_CHECK_EQUAL(&(*lit), it);
}
// Check that head is empty
BOOST_CHECK_EQUAL(head.second.Next(), nullptr);
// Delete the nodes from the list to make sure there are no dangling pointers
for (auto it{nodes.begin()}; it != nodes.end(); it = nodes.erase(it)) {
BOOST_CHECK_EQUAL(it->second.GetFlags(), 0);
BOOST_CHECK_EQUAL(it->second.Next(), nullptr);
}
}
BOOST_AUTO_TEST_CASE(linked_list_iterate_erase)
{
CoinsCachePair head;
auto nodes{CreatePairs(head)};
// Check iterating through pairs is identical to iterating through a list
// Erase the nodes as we iterate through, but don't clear flags
// The flags will be cleared by the CCoinsCacheEntry's destructor
auto lit{nodes.begin()};
for (auto it{head.second.Next()}; it != nullptr; it = it->second.Next(/*clear_flags=*/false), lit = nodes.erase(lit)) {
BOOST_CHECK_EQUAL(&(*lit), it);
}
// Check that head is empty
BOOST_CHECK_EQUAL(head.second.Next(), nullptr);
}
BOOST_AUTO_TEST_CASE(linked_list_random_deletion)
{
CoinsCachePair head;
auto nodes{CreatePairs(head)};
// Create linked list head->n1->n2->n3->n4->nullptr
auto n1{nodes.begin()};
auto n2{std::next(n1)};
auto n3{std::next(n2)};
auto n4{std::next(n3)};
// Delete n2
// head->n1->n3->n4->nullptr
nodes.erase(n2);
// Check that n1 now points to n3, and n3 still points to n4
// Also check that flags were not altered
BOOST_CHECK_EQUAL(n1->second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(n1->second.Next(), &(*n3));
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(n3->second.Next(), &(*n4));
// Delete n1
// head->n3->n4->nullptr
nodes.erase(n1);
// Check that head now points to n3, and n3 still points to n4
// Also check that flags were not altered
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(head.second.Next(), &(*n3));
BOOST_CHECK_EQUAL(n3->second.Next(), &(*n4));
// Delete n4
// head->n3->nullptr
nodes.erase(n4);
// Check that head still points to n3, and n3 points to nullptr
// Also check that flags were not altered
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(head.second.Next(), &(*n3));
BOOST_CHECK_EQUAL(n3->second.Next(), nullptr);
// Delete n3
// head->nullptr
nodes.erase(n3);
// Check that head now points to nullptr
BOOST_CHECK_EQUAL(head.second.Next(), nullptr);
}
BOOST_AUTO_TEST_CASE(linked_list_add_flags)
{
CoinsCachePair head;
CoinsCachePair n1;
CoinsCachePair n2;
// Check that adding 0 flag has no effect
n1.second.AddFlags(0, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
BOOST_CHECK_EQUAL(n1.second.Next(), nullptr);
BOOST_CHECK_EQUAL(head.second.Next(), nullptr);
// Check that adding DIRTY flag inserts it into linked list and sets flags
n1.second.AddFlags(CCoinsCacheEntry::DIRTY, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(n1.second.Next(), nullptr);
BOOST_CHECK_EQUAL(head.second.Next(), &n1);
// Check that adding FRESH flag on new node inserts it after head
n2.second.AddFlags(CCoinsCacheEntry::FRESH, n2, head);
BOOST_CHECK_EQUAL(n2.second.GetFlags(), CCoinsCacheEntry::FRESH);
BOOST_CHECK_EQUAL(n2.second.Next(), &n1);
BOOST_CHECK_EQUAL(head.second.Next(), &n2);
// Check that adding 0 flag has no effect, and doesn't change position
n1.second.AddFlags(0, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(n1.second.Next(), nullptr);
BOOST_CHECK_EQUAL(head.second.Next(), &n2);
BOOST_CHECK_EQUAL(n2.second.Next(), &n1);
// Check that we can add extra flags, but they don't change our position
n1.second.AddFlags(CCoinsCacheEntry::FRESH, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY | CCoinsCacheEntry::FRESH);
BOOST_CHECK_EQUAL(n1.second.Next(), nullptr);
BOOST_CHECK_EQUAL(head.second.Next(), &n2);
BOOST_CHECK_EQUAL(n2.second.Next(), &n1);
// Check that we can clear flags then re-add them
n1.second.ClearFlags();
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
BOOST_CHECK_EQUAL(n2.second.Next(), nullptr);
// Check that we calling `ClearFlags` with 0 flags has no effect
n1.second.ClearFlags();
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
BOOST_CHECK_EQUAL(n2.second.Next(), nullptr);
// Adding 0 still has no effect
n1.second.AddFlags(0, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
BOOST_CHECK_EQUAL(n1.second.Next(), nullptr);
BOOST_CHECK_EQUAL(head.second.Next(), &n2);
// But adding DIRTY re-inserts it after head
n1.second.AddFlags(CCoinsCacheEntry::DIRTY, n1, head);
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
BOOST_CHECK_EQUAL(head.second.Next(), &n1);
BOOST_CHECK_EQUAL(n1.second.Next(), &n2);
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -122,12 +122,16 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
random_mutable_transaction = *opt_mutable_transaction;
},
[&] {
// flagged_head must go out of scope and be destroyed after coins_map.
// Any remaining flagged entries in coins_map need to reference the head
// when they are destroyed.
CoinsCachePair flagged_head;
CCoinsMapMemoryResource resource;
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
{
CCoinsCacheEntry coins_cache_entry;
coins_cache_entry.flags = fuzzed_data_provider.ConsumeIntegral<unsigned char>();
const auto flags{fuzzed_data_provider.ConsumeIntegral<uint8_t>()};
if (fuzzed_data_provider.ConsumeBool()) {
coins_cache_entry.coin = random_coin;
} else {
@ -138,11 +142,12 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
}
coins_cache_entry.coin = *opt_coin;
}
coins_map.emplace(random_out_point, std::move(coins_cache_entry));
auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first};
it->second.AddFlags(flags, *it, flagged_head);
}
bool expected_code_path = false;
try {
coins_view_cache.BatchWrite(coins_map, fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock());
coins_view_cache.BatchWrite(flagged_head.second.Next(), fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock());
expected_code_path = true;
} catch (const std::logic_error& e) {
if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}) {

View File

@ -172,13 +172,13 @@ public:
std::unique_ptr<CCoinsViewCursor> Cursor() const final { return {}; }
size_t EstimateSize() const final { return m_data.size(); }
bool BatchWrite(CCoinsMap& data, const uint256&, bool erase) final
bool BatchWrite(CoinsCachePair *pairs, const uint256&, bool will_erase) final
{
for (auto it = data.begin(); it != data.end(); it = erase ? data.erase(it) : std::next(it)) {
if (it->second.flags & CCoinsCacheEntry::DIRTY) {
for (auto it{pairs}; it != nullptr; it = it->second.Next(will_erase)) {
if (it->second.GetFlags() & CCoinsCacheEntry::DIRTY) {
if (it->second.coin.IsSpent() && (it->first.n % 5) != 4) {
m_data.erase(it->first);
} else if (erase) {
} else if (will_erase) {
m_data[it->first] = std::move(it->second.coin);
} else {
m_data[it->first] = it->second.coin;

View File

@ -88,7 +88,7 @@ std::vector<uint256> CCoinsViewDB::GetHeadBlocks() const {
return vhashHeadBlocks;
}
bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) {
bool CCoinsViewDB::BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase) {
CDBBatch batch(*m_db);
size_t count = 0;
size_t changed = 0;
@ -114,8 +114,8 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, boo
batch.Erase(DB_BEST_BLOCK);
batch.Write(DB_HEAD_BLOCKS, Vector(hashBlock, old_tip));
for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) {
if (it->second.flags & CCoinsCacheEntry::DIRTY) {
for (auto it{pairs}; it != nullptr;) {
if (it->second.GetFlags() & CCoinsCacheEntry::DIRTY) {
CoinEntry entry(&it->first);
if (it->second.coin.IsSpent())
batch.Erase(entry);
@ -124,7 +124,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, boo
changed++;
}
count++;
it = erase ? mapCoins.erase(it) : std::next(it);
it = it->second.Next(will_erase);
if (batch.SizeEstimate() > m_options.batch_write_bytes) {
LogPrint(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0));
m_db->WriteBatch(batch);

View File

@ -63,7 +63,7 @@ public:
bool HaveCoin(const COutPoint &outpoint) const override;
uint256 GetBestBlock() const override;
std::vector<uint256> GetHeadBlocks() const override;
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override;
bool BatchWrite(CoinsCachePair *pairs, const uint256 &hashBlock, bool will_erase = true) override;
std::unique_ptr<CCoinsViewCursor> Cursor() const override;
//! Whether an unsupported database format is used.

View File

@ -2747,8 +2747,10 @@ bool Chainstate::FlushStateToDisk(
return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!"));
}
// Flush the chainstate (which may refer to block index entries).
if (!CoinsTip().Flush())
const auto empty_cache{(mode == FlushStateMode::ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicFlush};
if (empty_cache ? !CoinsTip().Flush() : !CoinsTip().Sync()) {
return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to coin database."));
}
m_last_flush = nNow;
full_flush_completed = true;
TRACE5(utxocache, flush,