diff --git a/src/coins.cpp b/src/coins.cpp index 28408481e46..42e83dab8c3 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -76,9 +76,6 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi bool inserted; std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>()); bool fresh = false; - if (!inserted) { - cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); - } if (!possible_overwrite) { if (!it->second.coin.IsSpent()) { throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)"); @@ -98,6 +95,9 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi // DIRTY, then it can be marked FRESH. fresh = !it->second.IsDirty(); } + if (!inserted) { + cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); + } it->second.coin = std::move(coin); CCoinsCacheEntry::SetDirty(*it, m_sentinel); if (fresh) CCoinsCacheEntry::SetFresh(*it, m_sentinel); @@ -254,8 +254,8 @@ bool CCoinsViewCache::Flush() { if (fOk) { cacheCoins.clear(); ReallocateCache(); + cachedCoinsUsage = 0; } - cachedCoinsUsage = 0; return fOk; } diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index f3926ef4044..46b1e2eb955 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -1085,4 +1085,22 @@ BOOST_AUTO_TEST_CASE(coins_resource_is_used) PoolResourceTester::CheckAllDataAccountedFor(resource); } +BOOST_AUTO_TEST_CASE(ccoins_addcoin_exception_keeps_usage_balanced) +{ + CCoinsView root; + CCoinsViewCacheTest cache{&root}; + + const COutPoint outpoint{Txid::FromUint256(m_rng.rand256()), m_rng.rand32()}; + + const Coin coin1{CTxOut{m_rng.randrange(10), CScript{} << m_rng.randbytes(CScriptBase::STATIC_SIZE + 1)}, 1, false}; + cache.AddCoin(outpoint, Coin{coin1}, /*possible_overwrite=*/false); + cache.SelfTest(); + + const Coin coin2{CTxOut{m_rng.randrange(20), CScript{} << m_rng.randbytes(CScriptBase::STATIC_SIZE + 2)}, 2, false}; + BOOST_CHECK_THROW(cache.AddCoin(outpoint, Coin{coin2}, /*possible_overwrite=*/false), std::logic_error); + cache.SelfTest(); + + BOOST_CHECK(cache.AccessCoin(outpoint) == coin1); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index e8ba655e54c..2b3557fffed 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -59,25 +59,15 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend if (random_coin.IsSpent()) { return; } - Coin coin = random_coin; - bool expected_code_path = false; - const bool possible_overwrite = fuzzed_data_provider.ConsumeBool(); + COutPoint outpoint{random_out_point}; + Coin coin{random_coin}; + const bool possible_overwrite{fuzzed_data_provider.ConsumeBool()}; try { - coins_view_cache.AddCoin(random_out_point, std::move(coin), possible_overwrite); - expected_code_path = true; + coins_view_cache.AddCoin(outpoint, std::move(coin), possible_overwrite); } catch (const std::logic_error& e) { - if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) { - assert(!possible_overwrite); - expected_code_path = true; - // AddCoin() decreases cachedCoinsUsage by the memory usage of the old coin at the beginning and - // increases it by the value of the new coin at the end. If it throws in the process, the value - // of cachedCoinsUsage would have been incorrectly decreased, leading to an underflow later on. - // To avoid this, use Flush() to reset the value of cachedCoinsUsage in sync with the cacheCoins - // mapping. - (void)coins_view_cache.Flush(); - } + assert(e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}); + assert(!possible_overwrite); } - assert(expected_code_path); }, [&] { (void)coins_view_cache.Flush(); diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 77aed1aa8a1..0151f9d0253 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -47,11 +47,6 @@ unsigned-integer-overflow:arith_uint256.h unsigned-integer-overflow:CBloomFilter::Hash unsigned-integer-overflow:CRollingBloomFilter::insert unsigned-integer-overflow:RollingBloomHash -unsigned-integer-overflow:CCoinsViewCache::AddCoin -unsigned-integer-overflow:CCoinsViewCache::BatchWrite -unsigned-integer-overflow:CCoinsViewCache::DynamicMemoryUsage -unsigned-integer-overflow:CCoinsViewCache::SpendCoin -unsigned-integer-overflow:CCoinsViewCache::Uncache unsigned-integer-overflow:CompressAmount unsigned-integer-overflow:DecompressAmount unsigned-integer-overflow:crypto/