Repository: bitcoin/bitcoin
File: src/coins.cpp (C++)

Code before:
        if (will_reuse_cache) {
            ReallocateCache();
        }
        cachedCoinsUsage = 0;
    }
    return fOk;
}

bool CCoinsViewCache::Sync()
{
    auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/false)};
    bool fOk = base->BatchWrite(cursor, hashBlock);
    if (fOk) {
        if (m_sentinel.second.Next() != &m_sentinel) {
            /* BatchWrite must clear flags of all entries */
            throw std::logic_error("Not all unspent flagged entries were cleared");
        }
    }
    return fOk;
}

void CCoinsViewCache::Reset() noexcept
{
    cacheCoins.clear();
    cachedCoinsUsage = 0;
    hashBlock.SetNull();
}

void CCoinsViewCache::Uncache(const COutPoint& hash)
{
    CCoinsMap::iterator it = cacheCoins.find(hash);
    if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
        cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
        TRACEPOINT(utxocache, uncache,
               hash.hash.data(),
               (uint32_t)hash.n,
               (uint32_t)it->second.coin.nHeight,
               (int64_t)it->second.coin.out.nValue,
               (bool)it->second.coin.IsCoinBase());
        cacheCoins.erase(it);
    }
}

unsigned int CCoinsViewCache::GetCacheSize() const {
    return cacheCoins.size();
}

bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const
{
    if (!tx.IsCoinBase()) {
        for (unsigned int i = 0; i < tx.vin.size(); i++) {

Review comment (bug):
This resets `hashBlock`, but `Flush()`/`Sync()` pass the raw `hashBlock` into `base->BatchWrite()`:
* https://github.com/bitcoin/bitcoin/blob/adbb4b320834516dc97fa33aaa853a45cbabca67/src/coins.cpp#L252-L254
* https://github.com/bitcoin/bitcoin/blob/adbb4b320834516dc97fa33aaa853a45cbabca67/src/coins.cpp#L265-L268

This means that the behavior of these methods depends on whether the `GetBestBlock` was called before these methods (since the "getter" would set the field after which accessing it directly would yield a different result):
https://github.com/bitcoin/bitcoin/blob/adbb4b320834516dc97fa33aaa853a45cbabca67/src/coins.cpp#L176-L180
While it happens to be safe in this particular case as far as I can tell, adding a public `Reset()` makes the footgun more relevant.

This isn't just theoretical, we have calls to that getter only for its side-effect:
https://github.com/bitcoin/bitcoin/blob/adbb4b320834516dc97fa33aaa853a45cbabca67/src/validation.cpp#L874-L876

I understand that some of this wasn't introduced in this commit, but this change makes this potential misuse more obvious.

----

Nit: In `BatchWrite()` we're setting `hashBlock` explicitly:
https://github.com/bitcoin/bitcoin/blob/adbb4b320834516dc97fa33aaa853a45cbabca67/src/coins.cpp#L248
Consider using the setter consistently so best-block updates go through one path (helps with debugging and consistency). It's not strictly related, but seems like a good opportunity to make this area more reliable (otherwise it's hard to reason about the state if mutations are spread around).

Please see [l0rinc/bitcoin@`2838a34` (#79)](https://github.com/l0rinc/bitcoin/pull/79/commits/2838a3445516a6ef16a5cd64f51fb2cc715b600e) for such an attempt.

Code after:
void CCoinsViewCache::Flush(bool reallocate_cache)
{
    auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)};
    base->BatchWrite(cursor, hashBlock);
    cacheCoins.clear();
    if (reallocate_cache) {
        ReallocateCache();
    }
    cachedCoinsUsage = 0;
}

void CCoinsViewCache::Sync()
{
    auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/false)};
    base->BatchWrite(cursor, hashBlock);
    if (m_sentinel.second.Next() != &m_sentinel) {
        /* BatchWrite must clear flags of all entries */
        throw std::logic_error("Not all unspent flagged entries were cleared");
    }
}

void CCoinsViewCache::Reset() noexcept
{
    cacheCoins.clear();
    cachedCoinsUsage = 0;
    SetBestBlock(uint256::ZERO);
}

void CCoinsViewCache::Uncache(const COutPoint& hash)
{
    CCoinsMap::iterator it = cacheCoins.find(hash);
    if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
        cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
        TRACEPOINT(utxocache, uncache,
               hash.hash.data(),
               (uint32_t)hash.n,
               (uint32_t)it->second.coin.nHeight,
               (int64_t)it->second.coin.out.nValue,
               (bool)it->second.coin.IsCoinBase());
        cacheCoins.erase(it);
    }
}

unsigned int CCoinsViewCache::GetCacheSize() const {
    return cacheCoins.size();
}

bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const
{
    if (!tx.IsCoinBase()) {
        for (unsigned int i = 0; i < tx.vin.size(); i++) {