From b8fa6f0f701f04cffca6a085337b508381016649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 22 Feb 2026 19:05:48 +0100 Subject: [PATCH] util: introduce `TrySub` to prevent unsigned underflow Introduce `TrySub(T&, U)` which subtracts an unsigned integral `U` from an unsigned integral `T`, returning `false` on underflow. Use with `Assume(TrySub(...))` at coins cache accounting decrement sites so invariant violations fail immediately rather than silently wrapping. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Co-authored-by: Pieter Wuille --- src/coins.cpp | 16 ++++++++-------- src/coins.h | 3 ++- src/util/overflow.h | 8 ++++++++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index a65522839f2..25b1ead0c1d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -113,8 +113,8 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi fresh = !it->second.IsDirty(); } if (!inserted) { - m_dirty_count -= it->second.IsDirty(); - cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); + Assume(TrySub(m_dirty_count, it->second.IsDirty())); + Assume(TrySub(cachedCoinsUsage, it->second.coin.DynamicMemoryUsage())); } it->second.coin = std::move(coin); CCoinsCacheEntry::SetDirty(*it, m_sentinel); @@ -153,8 +153,8 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { CCoinsMap::iterator it = FetchCoin(outpoint); if (it == cacheCoins.end()) return false; - m_dirty_count -= it->second.IsDirty(); - cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); + Assume(TrySub(m_dirty_count, it->second.IsDirty())); + Assume(TrySub(cachedCoinsUsage, it->second.coin.DynamicMemoryUsage())); TRACEPOINT(utxocache, spent, outpoint.hash.data(), (uint32_t)outpoint.n, @@ -248,12 +248,12 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha if (itUs->second.IsFresh() && 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. - m_dirty_count -= itUs->second.IsDirty(); - cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); + Assume(TrySub(m_dirty_count, itUs->second.IsDirty())); + Assume(TrySub(cachedCoinsUsage, itUs->second.coin.DynamicMemoryUsage())); cacheCoins.erase(itUs); } else { // A normal modification. - cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); + Assume(TrySub(cachedCoinsUsage, itUs->second.coin.DynamicMemoryUsage())); if (cursor.WillErase(*it)) { // Since this entry will be erased, // we can move the coin into us instead of copying it @@ -311,7 +311,7 @@ void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); if (it != cacheCoins.end() && !it->second.IsDirty()) { - cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); + Assume(TrySub(cachedCoinsUsage, it->second.coin.DynamicMemoryUsage())); TRACEPOINT(utxocache, uncache, hash.hash.data(), (uint32_t)hash.n, diff --git a/src/coins.h b/src/coins.h index ba23e3d3303..08c1886f930 100644 --- a/src/coins.h +++ b/src/coins.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -278,7 +279,7 @@ struct CoinsViewCacheCursor inline CoinsCachePair* NextAndMaybeErase(CoinsCachePair& current) noexcept { const auto next_entry{current.second.Next()}; - m_dirty_count -= current.second.IsDirty(); + Assume(TrySub(m_dirty_count, current.second.IsDirty())); // If we are not going to erase the cache, we must still erase spent entries. // Otherwise, clear the state of the entry. if (!m_will_erase) { diff --git a/src/util/overflow.h b/src/util/overflow.h index 8b8511f86f6..e7498fd8118 100644 --- a/src/util/overflow.h +++ b/src/util/overflow.h @@ -31,6 +31,14 @@ template return i + j; } +template +[[nodiscard]] constexpr bool TrySub(T& i, const U j) noexcept +{ + if (i < T{j}) return false; + i -= T{j}; + return true; +} + template [[nodiscard]] T SaturatingAdd(const T i, const T j) noexcept {