diff --git a/src/coins.cpp b/src/coins.cpp index c403e006c85c..b2398f4241e1 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -170,6 +170,19 @@ bool CCoinsViewCache::HaveCoin(const COutPoint& outpoint) const return (it != cacheCoins.end() && !it->second.coin.IsSpent()); } +/** + * Check for the existence of a coin in cache and in base. + * Doesn't add it to the cache if found. + */ +bool CCoinsViewCache::HaveCoinDontCache(const COutPoint& outpoint) const +{ + if (const auto it{cacheCoins.find(outpoint)}; it != cacheCoins.end()) { + return !it->second.coin.IsSpent(); + } else { + return base->HaveCoinDontCache(outpoint); + } +} + bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = cacheCoins.find(outpoint); return (it != cacheCoins.end() && !it->second.coin.IsSpent()); @@ -403,6 +416,11 @@ bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); } +bool CCoinsViewErrorCatcher::HaveCoinDontCache(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoinDontCache(outpoint); }, m_err_callbacks); +} + std::optional CCoinsViewErrorCatcher::PeekCoin(const COutPoint& outpoint) const { return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::PeekCoin(outpoint); }, m_err_callbacks); diff --git a/src/coins.h b/src/coins.h index ae7f34f46581..748bf724d673 100644 --- a/src/coins.h +++ b/src/coins.h @@ -319,9 +319,15 @@ class CCoinsView virtual std::optional PeekCoin(const COutPoint& outpoint) const = 0; //! Just check whether a given outpoint is unspent. - //! May populate the cache. Use PeekCoin() to perform a non-caching lookup. + //! May populate the cache. Use HaveCoinDontCache() to perform a non-caching lookup. virtual bool HaveCoin(const COutPoint& outpoint) const = 0; + //! Check for the existence of a coin, similar to HaveCoin(), but with + //! the expectation that it does not exist. + //! It doesn't cache (in case the item is found), and this makes it more + //! efficient (no need to retrieve the item from lower cache). + virtual bool HaveCoinDontCache(const COutPoint &outpoint) const = 0; + //! Retrieve the block hash whose state this CCoinsView currently represents virtual uint256 GetBestBlock() const = 0; @@ -357,6 +363,7 @@ class CoinsViewEmpty : public CCoinsView std::optional GetCoin(const COutPoint&) const override { return {}; } std::optional PeekCoin(const COutPoint& outpoint) const override { return GetCoin(outpoint); } bool HaveCoin(const COutPoint& outpoint) const override { return !!GetCoin(outpoint); } + bool HaveCoinDontCache(const COutPoint& outpoint) const override { return !!GetCoin(outpoint); } uint256 GetBestBlock() const override { return {}; } std::vector GetHeadBlocks() const override { return {}; } void BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) override @@ -381,6 +388,7 @@ class CCoinsViewBacked : public CCoinsView std::optional GetCoin(const COutPoint& outpoint) const override { return base->GetCoin(outpoint); } std::optional PeekCoin(const COutPoint& outpoint) const override { return base->PeekCoin(outpoint); } bool HaveCoin(const COutPoint& outpoint) const override { return base->HaveCoin(outpoint); } + bool HaveCoinDontCache(const COutPoint& outpoint) const override { return base->HaveCoinDontCache(outpoint); } uint256 GetBestBlock() const override { return base->GetBestBlock(); } std::vector GetHeadBlocks() const override { return base->GetHeadBlocks(); } void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override { base->BatchWrite(cursor, block_hash); } @@ -432,6 +440,7 @@ class CCoinsViewCache : public CCoinsViewBacked std::optional GetCoin(const COutPoint& outpoint) const override; std::optional PeekCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint& outpoint) const override; + bool HaveCoinDontCache(const COutPoint& outpoint) const override; uint256 GetBestBlock() const override; void SetBestBlock(const uint256& block_hash); void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; @@ -605,6 +614,7 @@ class CCoinsViewErrorCatcher final : public CCoinsViewBacked std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint& outpoint) const override; + bool HaveCoinDontCache(const COutPoint& outpoint) const override; std::optional PeekCoin(const COutPoint& outpoint) const override; private: diff --git a/src/txdb.cpp b/src/txdb.cpp index a41dfd1657ac..b3e60bdbba7e 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -89,6 +89,11 @@ bool CCoinsViewDB::HaveCoin(const COutPoint& outpoint) const return m_db->Exists(CoinEntry(&outpoint)); } +bool CCoinsViewDB::HaveCoinDontCache(const COutPoint& outpoint) const +{ + return m_db->Exists(CoinEntry(&outpoint)); +} + uint256 CCoinsViewDB::GetBestBlock() const { uint256 hashBestChain; if (!m_db->Read(DB_BEST_BLOCK, hashBestChain)) diff --git a/src/txdb.h b/src/txdb.h index b19b312a4b60..6876a495725c 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -43,6 +43,7 @@ class CCoinsViewDB final : public CCoinsView std::optional GetCoin(const COutPoint& outpoint) const override; std::optional PeekCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint& outpoint) const override; + bool HaveCoinDontCache(const COutPoint& outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; diff --git a/src/validation.cpp b/src/validation.cpp index f85a834f2a4a..539763bd7da4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2464,7 +2464,10 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, if (fEnforceBIP30 || pindex->nHeight >= BIP34_IMPLIES_BIP30_LIMIT) { for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { - if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { + // Note: the likely case is that the new TXO doesn't exist, + // so it doesn't make sense to try to cache it. + // It will result in a cache miss (a miss for an non-existent item). + if (view.HaveCoinDontCache(COutPoint(tx->GetHash(), o))) { state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-BIP30", "tried to overwrite transaction"); }