cryptogera/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-31 10:41:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

coins: only adjust cachedCoinsUsage on EmplaceCoinInternalDANGER insert

Browse Source 
`EmplaceCoinInternalDANGER()` incremented `cachedCoinsUsage` even when `try_emplace` did not insert (duplicate key), inflating the counter.
This is mostly reachable in tests today since `AssumeUTXO` does not overwrite.

Increment only on successful insert, and capture `coin.DynamicMemoryUsage()` before the move so accounting uses the correct value.

Fuzz: add an `EmplaceCoinInternalDANGER` path to exercise insert-only accounting.
Unit test: emplace two different coins at the same outpoint (with different `DynamicMemoryUsage()`), verify `SelfTest()` passes and `AccessCoin(outpoint)` returns the first coin.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Co-authored-by: w0xlt <woltx@protonmail.com>
This commit is contained in:
Lőrinc 2025-04-18 11:24:30 +02:00
parent d7c9d6c291
commit 24d861da78
3 changed files with 33 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/coins.cpp
View File

@ -111,9 +111,12 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
}
void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin) {
cachedCoinsUsage += coin.DynamicMemoryUsage();
const auto mem_usage{coin.DynamicMemoryUsage()};
auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint), std::move(coin));
if (inserted) CCoinsCacheEntry::SetDirty(*it, m_sentinel);
if (inserted) {
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
cachedCoinsUsage += mem_usage;
}
}
void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool check_for_overwrite) {

18
src/test/coins_tests.cpp
View File

@ -1103,4 +1103,22 @@ BOOST_AUTO_TEST_CASE(ccoins_addcoin_exception_keeps_usage_balanced)
BOOST_CHECK(cache.AccessCoin(outpoint) == coin1);
}
BOOST_AUTO_TEST_CASE(ccoins_emplace_duplicate_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.EmplaceCoinInternalDANGER(COutPoint{outpoint}, Coin{coin1});
cache.SelfTest();
const Coin coin2{CTxOut{m_rng.randrange(20), CScript{} << m_rng.randbytes(CScriptBase::STATIC_SIZE + 2)}, 2, false};
cache.EmplaceCoinInternalDANGER(COutPoint{outpoint}, Coin{coin2});
cache.SelfTest();
BOOST_CHECK(cache.AccessCoin(outpoint) == coin1);
}
BOOST_AUTO_TEST_SUITE_END()

16
src/test/fuzz/coins_view.cpp
View File

@ -61,12 +61,16 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
}
COutPoint outpoint{random_out_point};
Coin coin{random_coin};
const bool possible_overwrite{fuzzed_data_provider.ConsumeBool()};
try {
coins_view_cache.AddCoin(outpoint, std::move(coin), possible_overwrite);
} catch (const std::logic_error& e) {
assert(e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"});
assert(!possible_overwrite);
if (fuzzed_data_provider.ConsumeBool()) {
const bool possible_overwrite{fuzzed_data_provider.ConsumeBool()};
try {
coins_view_cache.AddCoin(outpoint, std::move(coin), possible_overwrite);
} catch (const std::logic_error& e) {
assert(e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"});
assert(!possible_overwrite);
}
} else {
coins_view_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin));
}
},
[&] {