diff --git a/src/coins.cpp b/src/coins.cpp index 8e8edcca63..739cc7bc3b 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -34,7 +34,9 @@ size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) : CCoinsViewBacked(baseIn), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) -{} +{ + m_sentinel.second.SelfRef(m_sentinel); +} size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; @@ -51,7 +53,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.AddFlags(CCoinsCacheEntry::FRESH); + ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel); } cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); return ret; @@ -96,7 +98,7 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi fresh = !it->second.IsDirty(); } it->second.coin = std::move(coin); - it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0)); + it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0), *it, m_sentinel); cachedCoinsUsage += it->second.coin.DynamicMemoryUsage(); TRACE5(utxocache, add, outpoint.hash.data(), @@ -113,7 +115,7 @@ void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coi std::forward_as_tuple(std::move(outpoint)), std::forward_as_tuple(std::move(coin))); if (inserted) { - it->second.AddFlags(CCoinsCacheEntry::DIRTY); + it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_sentinel); } } @@ -144,7 +146,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { if (it->second.IsFresh()) { cacheCoins.erase(it); } else { - it->second.AddFlags(CCoinsCacheEntry::DIRTY); + it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_sentinel); it->second.coin.Clear(); } return true; @@ -196,7 +198,8 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn if (!(it->second.IsFresh() && 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]; + itUs = cacheCoins.try_emplace(it->first).first; + CCoinsCacheEntry& entry{itUs->second}; if (erase) { // The `move` call here is purely an optimization; we rely on the // `mapCoins.erase` call in the `for` expression to actually remove @@ -206,12 +209,12 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn entry.coin = it->second.coin; } cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); - entry.AddFlags(CCoinsCacheEntry::DIRTY); + entry.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_sentinel); // 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.IsFresh()) { - entry.AddFlags(CCoinsCacheEntry::FRESH); + entry.AddFlags(CCoinsCacheEntry::FRESH, *itUs, m_sentinel); } } } else { @@ -241,7 +244,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn itUs->second.coin = it->second.coin; } cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); - itUs->second.AddFlags(CCoinsCacheEntry::DIRTY); + itUs->second.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_sentinel); // 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 diff --git a/src/coins.h b/src/coins.h index 95baf3f700..8cdee3763b 100644 --- a/src/coins.h +++ b/src/coins.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -136,7 +137,14 @@ public: CCoinsCacheEntry() noexcept = default; explicit CCoinsCacheEntry(Coin&& coin_) noexcept : coin(std::move(coin_)) {} - inline void AddFlags(uint8_t flags) noexcept { m_flags |= flags; } + //! Adding a flag also requires a self reference to the pair that contains + //! this entry in the CCoinsCache map and a reference to the sentinel of the + //! flagged pair linked list. + inline void AddFlags(uint8_t flags, CoinsCachePair& self, CoinsCachePair& sentinel) noexcept + { + Assume(&self.second == this); + m_flags |= flags; + } inline void ClearFlags() noexcept { m_flags = 0; @@ -144,6 +152,12 @@ public: inline uint8_t GetFlags() const noexcept { return m_flags; } inline bool IsDirty() const noexcept { return m_flags & DIRTY; } inline bool IsFresh() const noexcept { return m_flags & FRESH; } + + //! Only use this for initializing the linked list sentinel + inline void SelfRef(CoinsCachePair& self) noexcept + { + Assume(&self.second == this); + } }; /** @@ -251,6 +265,8 @@ protected: */ mutable uint256 hashBlock; mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{}; + /* The starting sentinel of the flagged entry circular doubly linked list. */ + mutable CoinsCachePair m_sentinel; mutable CCoinsMap cacheCoins; /* Cached dynamic memory usage for the inner Coin objects. */ diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 8ac972c4e0..4fe560df95 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -92,6 +92,7 @@ public: } CCoinsMap& map() const { return cacheCoins; } + CoinsCachePair& sentinel() const { return m_sentinel; } size_t& usage() const { return cachedCoinsUsage; } }; @@ -576,7 +577,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& sentinel, CAmount value, char flags) { if (value == ABSENT) { assert(flags == NO_ENTRY); @@ -584,10 +585,10 @@ static size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags) } assert(flags != NO_ENTRY); CCoinsCacheEntry entry; - entry.AddFlags(flags); SetCoinsValue(value, entry.coin); auto inserted = map.emplace(OUTPOINT, std::move(entry)); assert(inserted.second); + inserted.first->second.AddFlags(flags, *inserted.first, sentinel); return inserted.first->second.coin.DynamicMemoryUsage(); } @@ -610,9 +611,11 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags, const C void WriteCoinsViewEntry(CCoinsView& view, CAmount value, char flags) { + CoinsCachePair sentinel{}; + sentinel.second.SelfRef(sentinel); CCoinsMapMemoryResource resource; CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource}; - InsertCoinsMapEntry(map, value, flags); + InsertCoinsMapEntry(map, sentinel, value, flags); BOOST_CHECK(view.BatchWrite(map, {})); } @@ -622,7 +625,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.sentinel(), cache_value, cache_flags); } CCoinsView root; diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 1f24d78dca..8b7592f28c 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -120,12 +120,14 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) random_mutable_transaction = *opt_mutable_transaction; }, [&] { + CoinsCachePair sentinel{}; + sentinel.second.SelfRef(sentinel); 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.AddFlags(fuzzed_data_provider.ConsumeIntegral()); + const auto flags{fuzzed_data_provider.ConsumeIntegral()}; if (fuzzed_data_provider.ConsumeBool()) { coins_cache_entry.coin = random_coin; } else { @@ -136,7 +138,8 @@ 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, sentinel); } bool expected_code_path = false; try {