https://github.com/bitcoin/bitcoin/pull/34320/commits/4b32181dbbe32f9cd9f8c2063bd31b27746f794d:
Consider merging both tests as done in the diff below. I also replaced the DB view with a dummy view because CCoinsViewDB is not under test here; the dummy is sufficient to test the cache.
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index ad182559a3..baba9c4e75 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -1050,23 +1050,13 @@ BOOST_FIXTURE_TEST_CASE(ccoins_flush_behavior, FlushTest)
}
}
-BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin)
+BOOST_AUTO_TEST_CASE(ccoins_cache_behavior)
{
- const COutPoint prevout{Txid::FromUint256(m_rng.rand256()), 0};
-
- CCoinsViewDB db{{.path = "test", .cache_bytes = 1_MiB, .memory_only = true}, {}};
- {
- CCoinsViewCache write_cache{&db};
- write_cache.SetBestBlock(m_rng.rand256());
- write_cache.AddCoin(prevout, Coin{CTxOut{1, CScript{}}, 1, false}, /*possible_overwrite=*/false);
- write_cache.Flush();
- }
-
class CCoinsViewSpy final : public CCoinsViewBacked
{
public:
const COutPoint expected;
- mutable size_t havecoin_calls{0}, getcoin_calls{0};
+ mutable size_t getcoin_calls{0};
explicit CCoinsViewSpy(CCoinsView* view, const COutPoint& out) : CCoinsViewBacked(view), expected{out} {}
@@ -1074,18 +1064,23 @@ BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin)
{
++getcoin_calls;
BOOST_CHECK(out == expected);
- return CCoinsViewBacked::GetCoin(out);
+ return Coin{CTxOut{1, CScript{}}, 1, false};
}
bool HaveCoin(const COutPoint& out) const override
{
- ++havecoin_calls;
- BOOST_CHECK(out == expected);
- return CCoinsViewBacked::HaveCoin(out);
+ // This should never be called, since HaveInputs() should call GetCoin()
+ // and not call HaveCoin() at all. If this is called, it means that
+ // HaveInputs() is calling HaveCoin() instead of GetCoin(), which
+ // is less efficient since it may require two lookups instead of one.
+ BOOST_FAIL("Base HaveCoin should never be called");
+ return false;
}
};
- CCoinsViewSpy base{&db, prevout};
+ const COutPoint prevout{Txid::FromUint256(m_rng.rand256()), 0};
+ CCoinsView dummy;
+ CCoinsViewSpy base{&dummy, prevout};
CCoinsViewCache cache{&base};
CMutableTransaction mtx;
@@ -1095,46 +1090,14 @@ BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin)
BOOST_CHECK(cache.HaveInputs(tx));
BOOST_CHECK_EQUAL(base.getcoin_calls, 1);
- BOOST_CHECK_EQUAL(base.havecoin_calls, 0);
-}
-BOOST_AUTO_TEST_CASE(ccoins_cache_hit_does_not_call_base)
-{
- class CCoinsViewNoCall final : public CCoinsView
- {
- public:
- std::optional<Coin> GetCoin(const COutPoint&) const override
- {
- BOOST_FAIL("Base GetCoin should not be called when input is cached");
- return std::nullopt;
- }
-
- bool HaveCoin(const COutPoint&) const override
- {
- BOOST_FAIL("Base HaveCoin should not be called when input is cached");
- return false;
- }
- };
-
- const COutPoint prevout{Txid::FromUint256(m_rng.rand256()), 0};
- CCoinsViewNoCall base;
- CCoinsViewCache cache{&base};
-
- cache.AddCoin(prevout, Coin{CTxOut{1, CScript{}}, 1, false}, /*possible_overwrite=*/false);
- BOOST_CHECK(cache.HaveCoinInCache(prevout));
-
- BOOST_CHECK(!cache.AccessCoin(prevout).IsSpent());
- BOOST_CHECK(cache.GetCoin(prevout));
- BOOST_CHECK(cache.HaveCoin(prevout));
-
- CMutableTransaction mtx;
- mtx.vin.emplace_back(prevout);
- const CTransaction tx{mtx};
- BOOST_CHECK(!tx.IsCoinBase());
BOOST_CHECK(cache.HaveInputs(tx));
-
- BOOST_CHECK(cache.SpendCoin(prevout));
- BOOST_CHECK(!cache.HaveCoinInCache(prevout));
+ BOOST_CHECK(cache.GetCoin(prevout));
+ BOOST_CHECK(!cache.AccessCoin(prevout).IsSpent());
+ // GetCoin should only have been called once,
+ // since the result should be cached after
+ // the first call to HaveInputs().
+ BOOST_CHECK_EQUAL(base.getcoin_calls, 1);
}
BOOST_AUTO_TEST_CASE(coins_resource_is_used)
