From 20b902ea6865cdd617cd5f72def85d1ad83668f7 Mon Sep 17 00:00:00 2001 From: optout21 <13562139+optout21@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:22:02 +0200 Subject: [PATCH 1/4] Split CCoinsView into Read, ReadCacheMutable, and Write --- src/coins.cpp | 2 +- src/coins.h | 64 ++++++++++++++++++----------- src/kernel/coinstats.cpp | 4 +- src/kernel/coinstats.h | 4 +- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 4 +- src/test/coins_tests.cpp | 10 ++--- src/test/coinsviewoverlay_tests.cpp | 2 +- src/test/fuzz/coins_view.cpp | 2 +- src/txdb.h | 2 +- src/txmempool.cpp | 2 +- src/txmempool.h | 2 +- src/validation.cpp | 8 ++-- src/validation.h | 4 +- 14 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index c403e006c85c..8163789ca3e3 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -28,7 +28,7 @@ std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const return base->PeekCoin(outpoint); } -CCoinsViewCache::CCoinsViewCache(CCoinsView* in_base, bool deterministic) : +CCoinsViewCache::CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic) : CCoinsViewBacked(in_base), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) { diff --git a/src/coins.h b/src/coins.h index ae7f34f46581..21e327325490 100644 --- a/src/coins.h +++ b/src/coins.h @@ -249,7 +249,7 @@ class CCoinsViewCursor * * This is a helper struct to encapsulate the diverging logic between a non-erasing * CCoinsViewCache::Sync and an erasing CCoinsViewCache::Flush. This allows the receiver - * of CCoinsView::BatchWrite to iterate through the flagged entries without knowing + * of CCoinsViewWrite::BatchWrite to iterate through the flagged entries without knowing * the caller's intent. * * However, the receiver can still call CoinsViewCacheCursor::WillErase to see if the @@ -303,26 +303,18 @@ struct CoinsViewCacheCursor bool m_will_erase; }; -/** Pure abstract view on the open txout dataset. */ -class CCoinsView +/** Pure abstract view on the open txout dataset, read-only. */ +class CCoinsViewReadOnly { public: //! As we use CCoinsViews polymorphically, have a virtual destructor - virtual ~CCoinsView() = default; - - //! Retrieve the Coin (unspent transaction output) for a given outpoint. - //! May populate the cache. Use PeekCoin() to perform a non-caching lookup. - virtual std::optional GetCoin(const COutPoint& outpoint) const = 0; + virtual ~CCoinsViewReadOnly() = default; //! Retrieve the Coin (unspent transaction output) for a given outpoint, without caching results. //! Does not populate the cache. Use GetCoin() to cache the result. 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. - virtual bool HaveCoin(const COutPoint& outpoint) const = 0; - - //! Retrieve the block hash whose state this CCoinsView currently represents + //! Retrieve the block hash whose state this CCoinsViewReadCacheMutable currently represents virtual uint256 GetBestBlock() const = 0; //! Retrieve the range of blocks that may have been only partially written. @@ -331,10 +323,6 @@ class CCoinsView //! the old block hash, in that order. virtual std::vector GetHeadBlocks() const = 0; - //! Do a bulk modification (multiple Coin changes + BestBlock change). - //! The passed cursor is used to iterate through the coins. - virtual void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) = 0; - //! Get a cursor to iterate over the whole state. Implementations may return nullptr. virtual std::unique_ptr Cursor() const = 0; @@ -342,8 +330,36 @@ class CCoinsView virtual size_t EstimateSize() const = 0; }; +/** Pure abstract view on the open txout dataset, read-only view with potential cache mutations. */ +class CCoinsViewReadCacheMutable : public CCoinsViewReadOnly +{ +public: + //! As we use CCoinsViews polymorphically, have a virtual destructor + virtual ~CCoinsViewReadCacheMutable() = default; + + //! Retrieve the Coin (unspent transaction output) for a given outpoint. + //! May populate the cache. Use PeekCoin() to perform a non-caching lookup. + virtual std::optional GetCoin(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. + virtual bool HaveCoin(const COutPoint& outpoint) const = 0; +}; + +/** Pure abstract view on the open txout dataset, writeable view. */ +class CCoinsViewWrite : public CCoinsViewReadCacheMutable +{ +public: + //! As we use CCoinsViews polymorphically, have a virtual destructor + virtual ~CCoinsViewWrite() = default; + + //! Do a bulk modification (multiple Coin changes + BestBlock change). + //! The passed cursor is used to iterate through the coins. + virtual void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) = 0; +}; + /** Noop coins view. */ -class CoinsViewEmpty : public CCoinsView +class CoinsViewEmpty : public CCoinsViewWrite { protected: CoinsViewEmpty() = default; @@ -368,15 +384,15 @@ class CoinsViewEmpty : public CCoinsView }; /** CCoinsView backed by another CCoinsView */ -class CCoinsViewBacked : public CCoinsView +class CCoinsViewBacked : public CCoinsViewWrite { protected: - CCoinsView* base; + CCoinsViewWrite* base; public: - explicit CCoinsViewBacked(CCoinsView* in_view) : base{Assert(in_view)} {} + explicit CCoinsViewBacked(CCoinsViewWrite* in_view) : base{Assert(in_view)} {} - void SetBackend(CCoinsView& in_view) { base = &in_view; } + void SetBackend(CCoinsViewWrite& in_view) { base = &in_view; } std::optional GetCoin(const COutPoint& outpoint) const override { return base->GetCoin(outpoint); } std::optional PeekCoin(const COutPoint& outpoint) const override { return base->PeekCoin(outpoint); } @@ -421,7 +437,7 @@ class CCoinsViewCache : public CCoinsViewBacked virtual std::optional FetchCoinFromBase(const COutPoint& outpoint) const; public: - CCoinsViewCache(CCoinsView* in_base, bool deterministic = false); + CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic = false); /** * By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache. @@ -597,7 +613,7 @@ const Coin& AccessByTxid(const CCoinsViewCache& cache, const Txid& txid); class CCoinsViewErrorCatcher final : public CCoinsViewBacked { public: - explicit CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {} + explicit CCoinsViewErrorCatcher(CCoinsViewWrite* view) : CCoinsViewBacked(view) {} void AddReadErrCallback(std::function f) { m_err_callbacks.emplace_back(std::move(f)); diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index 4f2f3feaf40d..576a2ecdc33c 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -108,7 +108,7 @@ static void ApplyStats(CCoinsStats& stats, const std::map& outpu //! Calculate statistics about the unspent transaction output set template -static std::optional ComputeUTXOStats(T hash_obj, CCoinsView* view, node::BlockManager& blockman, const std::function& interruption_point) +static std::optional ComputeUTXOStats(T hash_obj, CCoinsViewReadCacheMutable* view, node::BlockManager& blockman, const std::function& interruption_point) { std::unique_ptr pcursor; CBlockIndex* pindex; @@ -152,7 +152,7 @@ static std::optional ComputeUTXOStats(T hash_obj, CCoinsView* view, return stats; } -std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function& interruption_point) +std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsViewReadCacheMutable* view, node::BlockManager& blockman, const std::function& interruption_point) { return [&]() -> std::optional { switch (hash_type) { diff --git a/src/kernel/coinstats.h b/src/kernel/coinstats.h index 92ace06ba5d2..279372f100c6 100644 --- a/src/kernel/coinstats.h +++ b/src/kernel/coinstats.h @@ -13,7 +13,7 @@ #include #include -class CCoinsView; +class CCoinsViewReadCacheMutable; class Coin; class COutPoint; class CScript; @@ -77,7 +77,7 @@ uint64_t GetBogoSize(const CScript& script_pub_key); void ApplyCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin); void RemoveCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin); -std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function& interruption_point = {}); +std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsViewReadCacheMutable* view, node::BlockManager& blockman, const std::function& interruption_point = {}); } // namespace kernel #endif // BITCOIN_KERNEL_COINSTATS_H diff --git a/src/rest.cpp b/src/rest.cpp index d2c5a9b4899d..81f4fffe9a4e 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -1001,7 +1001,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: decltype(chainman.ActiveHeight()) active_height; uint256 active_hash; { - auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) { + auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsViewReadCacheMutable& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) { for (const COutPoint& vOutPoint : vOutPoints) { auto coin = !mempool || !mempool->isSpent(vOutPoint) ? view.GetCoin(vOutPoint) : std::nullopt; hits.push_back(coin.has_value()); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 14412c90fcc9..19bc1ad21415 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -995,7 +995,7 @@ CoinStatsHashType ParseHashType(std::string_view hash_type_input) * * @param[in] index_requested Signals if the coinstatsindex should be used (when available). */ -static std::optional GetUTXOStats(CCoinsView* view, node::BlockManager& blockman, +static std::optional GetUTXOStats(CCoinsViewReadCacheMutable* view, node::BlockManager& blockman, kernel::CoinStatsHashType hash_type, const std::function& interruption_point = {}, const CBlockIndex* pindex = nullptr, @@ -1086,7 +1086,7 @@ static RPCMethod gettxoutsetinfo() Chainstate& active_chainstate = chainman.ActiveChainstate(); active_chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false); - CCoinsView* coins_view; + CCoinsViewReadCacheMutable* coins_view; BlockManager* blockman; { LOCK(::cs_main); diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 14ccb1c443c4..05bef698eba8 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -76,7 +76,7 @@ class CCoinsViewTest : public CoinsViewEmpty class CCoinsViewCacheTest : public CCoinsViewCache { public: - explicit CCoinsViewCacheTest(CCoinsView* _base) : CCoinsViewCache(_base) {} + explicit CCoinsViewCacheTest(CCoinsViewWrite* _base) : CCoinsViewCache(_base) {} void SelfTest(bool sanity_check = true) const { @@ -119,7 +119,7 @@ struct CacheTest : BasicTestingSetup { // of best block on flush. This is necessary when using CCoinsViewDB as the base, // otherwise we'll hit an assertion in BatchWrite. // -void SimulationTest(CCoinsView* base, bool fake_best_block) +void SimulationTest(CCoinsViewWrite* base, bool fake_best_block) { // Various coverage trackers. bool removed_all_caches = false; @@ -254,7 +254,7 @@ void SimulationTest(CCoinsView* base, bool fake_best_block) } if (stack.size() == 0 || (stack.size() < 4 && m_rng.randbool())) { //Add a new cache - CCoinsView* tip = base; + CCoinsViewWrite* tip = base; if (stack.size() > 0) { tip = stack.back().get(); } else { @@ -507,7 +507,7 @@ BOOST_FIXTURE_TEST_CASE(updatecoins_simulation_test, UpdateTest) stack.pop_back(); } if (stack.size() == 0 || (stack.size() < 4 && m_rng.randbool())) { - CCoinsView* tip = &base; + CCoinsViewWrite* tip = &base; if (stack.size() > 0) { tip = stack.back().get(); } @@ -652,7 +652,7 @@ static MaybeCoin GetCoinsMapEntry(const CCoinsMap& map, const COutPoint& outp = return MISSING; } -static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin) +static void WriteCoinsViewEntry(CCoinsViewWrite& view, const MaybeCoin& cache_coin) { CoinsCachePair sentinel{}; sentinel.second.SelfRef(sentinel); diff --git a/src/test/coinsviewoverlay_tests.cpp b/src/test/coinsviewoverlay_tests.cpp index 6b20b31211a6..fc1f00480f88 100644 --- a/src/test/coinsviewoverlay_tests.cpp +++ b/src/test/coinsviewoverlay_tests.cpp @@ -39,7 +39,7 @@ CBlock CreateBlock() noexcept return block; } -void PopulateView(const CBlock& block, CCoinsView& view, bool spent = false) +void PopulateView(const CBlock& block, CCoinsViewWrite& view, bool spent = false) { CCoinsViewCache cache{&view}; cache.SetBestBlock(uint256::ONE); diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index c11581d2d3c3..eb8cae14fffa 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -90,7 +90,7 @@ void initialize_coins_view() static const auto testing_setup = MakeNoLogFileContext<>(); } -void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& coins_view_cache, CCoinsView* backend_coins_view) +void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& coins_view_cache, CCoinsViewWrite* backend_coins_view) { const bool is_db{dynamic_cast(backend_coins_view) != nullptr}; bool good_data{true}; diff --git a/src/txdb.h b/src/txdb.h index b19b312a4b60..e943b82b7ddf 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -31,7 +31,7 @@ struct CoinsViewOptions { }; /** CCoinsView backed by the coin database (chainstate/) */ -class CCoinsViewDB final : public CCoinsView +class CCoinsViewDB final : public CCoinsViewWrite { protected: DBParams m_db_params; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index a22cd2b199d7..b1278a3c05fe 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -737,7 +737,7 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const return true; } -CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } +CCoinsViewMemPool::CCoinsViewMemPool(CCoinsViewWrite* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } std::optional CCoinsViewMemPool::GetCoin(const COutPoint& outpoint) const { diff --git a/src/txmempool.h b/src/txmempool.h index c4723f891557..4ce8f09c5406 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -766,7 +766,7 @@ class CCoinsViewMemPool : public CCoinsViewBacked const CTxMemPool& mempool; public: - CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); + CCoinsViewMemPool(CCoinsViewWrite* baseIn, const CTxMemPool& mempoolIn); /** GetCoin, returning whether it exists and is not spent. Also updates m_non_base_coins if the * coin is not fetched from base. May populate the base view on cache misses. */ std::optional GetCoin(const COutPoint& outpoint) const override; diff --git a/src/validation.cpp b/src/validation.cpp index f85a834f2a4a..87cf5a1349ed 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -180,7 +180,7 @@ namespace { */ std::optional> CalculatePrevHeights( const CBlockIndex& tip, - const CCoinsView& coins, + const CCoinsViewReadCacheMutable& coins, const CTransaction& tx) { std::vector prev_heights; @@ -201,7 +201,7 @@ std::optional> CalculatePrevHeights( std::optional CalculateLockPointsAtTip( CBlockIndex* tip, - const CCoinsView& coins_view, + const CCoinsViewReadCacheMutable& coins_view, const CTransaction& tx) { assert(tip); @@ -4601,7 +4601,7 @@ CVerifyDB::~CVerifyDB() VerifyDBResult CVerifyDB::VerifyDB( Chainstate& chainstate, const Consensus::Params& consensus_params, - CCoinsView& coinsview, + CCoinsViewWrite& coinsview, int nCheckLevel, int nCheckDepth) { AssertLockHeld(cs_main); @@ -4764,7 +4764,7 @@ bool Chainstate::ReplayBlocks() { LOCK(cs_main); - CCoinsView& db = this->CoinsDB(); + CCoinsViewWrite& db = this->CoinsDB(); CCoinsViewCache cache(&db); std::vector hashHeads = db.GetHeadBlocks(); diff --git a/src/validation.h b/src/validation.h index 4cb5dc631e95..01d12cbf5419 100644 --- a/src/validation.h +++ b/src/validation.h @@ -316,7 +316,7 @@ bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& */ std::optional CalculateLockPointsAtTip( CBlockIndex* tip, - const CCoinsView& coins_view, + const CCoinsViewReadCacheMutable& coins_view, const CTransaction& tx); /** @@ -443,7 +443,7 @@ class CVerifyDB [[nodiscard]] VerifyDBResult VerifyDB( Chainstate& chainstate, const Consensus::Params& consensus_params, - CCoinsView& coinsview, + CCoinsViewWrite& coinsview, int nCheckLevel, int nCheckDepth) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; From 8f046372dd7afb32278937fdb52765bc3ce6e55a Mon Sep 17 00:00:00 2001 From: optout21 <13562139+optout21@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:26:43 +0200 Subject: [PATCH 2/4] Split CCoinsViewBacked into ReadCacheMutable and Write --- src/coins.cpp | 9 ++++---- src/coins.h | 54 +++++++++++++++++++++++++++++++++++++++++------ src/txmempool.cpp | 3 ++- src/txmempool.h | 3 ++- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 8163789ca3e3..c415ca66baa3 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -29,7 +29,7 @@ std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const } CCoinsViewCache::CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic) : - CCoinsViewBacked(in_base), m_deterministic(deterministic), + CCoinsViewBackedWrite(in_base), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) { m_sentinel.second.SelfRef(m_sentinel); @@ -393,17 +393,18 @@ static ReturnType ExecuteBackedWrapper(Func func, const std::vector CCoinsViewErrorCatcher::GetCoin(const COutPoint& outpoint) const { - return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::GetCoin(outpoint); }, m_err_callbacks); + return ExecuteBackedWrapper>([&]() { return CCoinsViewBackedWrite::GetCoin(outpoint); }, m_err_callbacks); } bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const { - return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); + return ExecuteBackedWrapper([&]() { return CCoinsViewBackedWrite::HaveCoin(outpoint); }, m_err_callbacks); } std::optional CCoinsViewErrorCatcher::PeekCoin(const COutPoint& outpoint) const { - return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::PeekCoin(outpoint); }, m_err_callbacks); + return ExecuteBackedWrapper>([&]() { return CCoinsViewBackedWrite::PeekCoin(outpoint); }, m_err_callbacks); } diff --git a/src/coins.h b/src/coins.h index 21e327325490..c44cc42ef1f8 100644 --- a/src/coins.h +++ b/src/coins.h @@ -383,14 +383,56 @@ class CoinsViewEmpty : public CCoinsViewWrite size_t EstimateSize() const override { return 0; } }; -/** CCoinsView backed by another CCoinsView */ -class CCoinsViewBacked : public CCoinsViewWrite +/** Non-virtual wrapper for CCoinsViewReadOnly * / +class CCoinsViewBackedReadOnly : public CCoinsViewReadOnly +{ +protected: + CCoinsViewReadOnly* base; + +public: + explicit CCoinsViewBackedReadOnly(CCoinsViewReadOnly* in_view) : base{Assert(in_view)} {} + + void SetBackend(CCoinsViewReadOnly& in_view) { base = &in_view; } + + std::optional PeekCoin(const COutPoint& outpoint) const override { return base->PeekCoin(outpoint); } + uint256 GetBestBlock() const override { return base->GetBestBlock(); } + std::vector GetHeadBlocks() const override { return base->GetHeadBlocks(); } + std::unique_ptr Cursor() const override { return base->Cursor(); } + size_t EstimateSize() const override { return base->EstimateSize(); } +}; +*/ + +/** + * Non-virtual wrapper for CCoinsViewReadCacheMutable + * Alternative for not using: just include the base and implement the methods. + */ +class CCoinsViewBackedReadCacheMutable : public CCoinsViewReadCacheMutable +{ +protected: + CCoinsViewReadCacheMutable* base; + +public: + explicit CCoinsViewBackedReadCacheMutable(CCoinsViewReadCacheMutable* in_view) : base{Assert(in_view)} {} + + void SetBackend(CCoinsViewReadCacheMutable& in_view) { base = &in_view; } + + 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); } + uint256 GetBestBlock() const override { return base->GetBestBlock(); } + std::vector GetHeadBlocks() const override { return base->GetHeadBlocks(); } + std::unique_ptr Cursor() const override { return base->Cursor(); } + size_t EstimateSize() const override { return base->EstimateSize(); } +}; + +/** Non-virtual wrapper for CCoinsViewWrite */ +class CCoinsViewBackedWrite : public CCoinsViewWrite { protected: CCoinsViewWrite* base; public: - explicit CCoinsViewBacked(CCoinsViewWrite* in_view) : base{Assert(in_view)} {} + explicit CCoinsViewBackedWrite(CCoinsViewWrite* in_view) : base{Assert(in_view)} {} void SetBackend(CCoinsViewWrite& in_view) { base = &in_view; } @@ -406,7 +448,7 @@ class CCoinsViewBacked : public CCoinsViewWrite /** CCoinsView that adds a memory cache for transactions to another CCoinsView */ -class CCoinsViewCache : public CCoinsViewBacked +class CCoinsViewCache : public CCoinsViewBackedWrite { private: const bool m_deterministic; @@ -610,10 +652,10 @@ const Coin& AccessByTxid(const CCoinsViewCache& cache, const Txid& txid); * * Writes do not need similar protection, as failure to write is handled by the caller. */ -class CCoinsViewErrorCatcher final : public CCoinsViewBacked +class CCoinsViewErrorCatcher final : public CCoinsViewBackedWrite { public: - explicit CCoinsViewErrorCatcher(CCoinsViewWrite* view) : CCoinsViewBacked(view) {} + explicit CCoinsViewErrorCatcher(CCoinsViewWrite* view) : CCoinsViewBackedWrite(view) {} void AddReadErrCallback(std::function f) { m_err_callbacks.emplace_back(std::move(f)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b1278a3c05fe..4d2d9a6834d8 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -737,7 +737,8 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const return true; } -CCoinsViewMemPool::CCoinsViewMemPool(CCoinsViewWrite* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } +// TODO: swtich to CCoinsViewBackedReadCacheMutable +CCoinsViewMemPool::CCoinsViewMemPool(CCoinsViewWrite* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBackedWrite(baseIn), mempool(mempoolIn) { } std::optional CCoinsViewMemPool::GetCoin(const COutPoint& outpoint) const { diff --git a/src/txmempool.h b/src/txmempool.h index 4ce8f09c5406..1b66f167022c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -749,7 +749,8 @@ class CTxMemPool * signrawtransactionwithkey and signrawtransactionwithwallet, * as long as the conflicting transaction is not yet confirmed. */ -class CCoinsViewMemPool : public CCoinsViewBacked +// TODO: switch to CCoinsViewBackedReadCacheMutable +class CCoinsViewMemPool : public CCoinsViewBackedWrite { /** * Coins made available by transactions being validated. Tracking these allows for package From ab8a72062b04ef2d4fa68a6c23a7a4b610937f4d Mon Sep 17 00:00:00 2001 From: optout21 <13562139+optout21@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:41:54 +0200 Subject: [PATCH 3/4] Carve out non-write CCoinsViewCacheRead from CCoinsViewCache --- src/coins.cpp | 33 +++++---- src/coins.h | 107 ++++++++++++++++++--------- src/node/coin.cpp | 2 +- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 6 +- src/rpc/rawtransaction.cpp | 3 +- src/test/coins_tests.cpp | 14 ++-- src/test/coinstatsindex_tests.cpp | 2 +- src/test/coinsviewoverlay_tests.cpp | 12 +-- src/test/miner_tests.cpp | 2 +- src/test/txvalidationcache_tests.cpp | 16 ++-- src/txmempool.cpp | 2 +- src/validation.cpp | 10 +-- 13 files changed, 126 insertions(+), 85 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index c415ca66baa3..7fe6c55c45c0 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -20,7 +20,7 @@ CoinsViewEmpty& CoinsViewEmpty::Get() return instance; } -std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const +std::optional CCoinsViewCacheRead::PeekCoin(const COutPoint& outpoint) const { if (auto it{cacheCoins.find(outpoint)}; it != cacheCoins.end()) { return it->second.coin.IsSpent() ? std::nullopt : std::optional{it->second.coin}; @@ -28,23 +28,23 @@ std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const return base->PeekCoin(outpoint); } -CCoinsViewCache::CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic) : - CCoinsViewBackedWrite(in_base), m_deterministic(deterministic), +CCoinsViewCacheRead::CCoinsViewCacheRead(CCoinsViewReadCacheMutable* in_base, bool deterministic) : + CCoinsViewBackedReadCacheMutable(in_base), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) { m_sentinel.second.SelfRef(m_sentinel); } -size_t CCoinsViewCache::DynamicMemoryUsage() const { +size_t CCoinsViewCacheRead::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; } -std::optional CCoinsViewCache::FetchCoinFromBase(const COutPoint& outpoint) const +std::optional CCoinsViewCacheRead::FetchCoinFromBase(const COutPoint& outpoint) const { return base->GetCoin(outpoint); } -CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const { +CCoinsMap::iterator CCoinsViewCacheRead::FetchCoin(const COutPoint &outpoint) const { const auto [ret, inserted] = cacheCoins.try_emplace(outpoint); if (inserted) { if (auto coin{FetchCoinFromBase(outpoint)}) { @@ -59,7 +59,7 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const return ret; } -std::optional CCoinsViewCache::GetCoin(const COutPoint& outpoint) const +std::optional CCoinsViewCacheRead::GetCoin(const COutPoint& outpoint) const { if (auto it{FetchCoin(outpoint)}; it != cacheCoins.end() && !it->second.coin.IsSpent()) return it->second.coin; return std::nullopt; @@ -155,7 +155,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { static const Coin coinEmpty; -const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { +const Coin& CCoinsViewCacheRead::AccessCoin(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = FetchCoin(outpoint); if (it == cacheCoins.end()) { return coinEmpty; @@ -164,18 +164,18 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { } } -bool CCoinsViewCache::HaveCoin(const COutPoint& outpoint) const +bool CCoinsViewCacheRead::HaveCoin(const COutPoint& outpoint) const { CCoinsMap::const_iterator it = FetchCoin(outpoint); return (it != cacheCoins.end() && !it->second.coin.IsSpent()); } -bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const { +bool CCoinsViewCacheRead::HaveCoinInCache(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = cacheCoins.find(outpoint); return (it != cacheCoins.end() && !it->second.coin.IsSpent()); } -uint256 CCoinsViewCache::GetBestBlock() const { +uint256 CCoinsViewCacheRead::GetBestBlock() const { if (m_block_hash.IsNull()) m_block_hash = base->GetBestBlock(); return m_block_hash; @@ -186,6 +186,7 @@ void CCoinsViewCache::SetBestBlock(const uint256& in_block_hash) m_block_hash = in_block_hash; } +// TODO move order void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in_block_hash) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { @@ -260,7 +261,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in void CCoinsViewCache::Flush(bool reallocate_cache) { auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/true)}; - base->BatchWrite(cursor, m_block_hash); + base_write->BatchWrite(cursor, m_block_hash); Assume(m_dirty_count == 0); cacheCoins.clear(); if (reallocate_cache) { @@ -272,7 +273,7 @@ void CCoinsViewCache::Flush(bool reallocate_cache) void CCoinsViewCache::Sync() { auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/false)}; - base->BatchWrite(cursor, m_block_hash); + base_write->BatchWrite(cursor, m_block_hash); Assume(m_dirty_count == 0); if (m_sentinel.second.Next() != &m_sentinel) { /* BatchWrite must clear flags of all entries */ @@ -303,11 +304,11 @@ void CCoinsViewCache::Uncache(const COutPoint& hash) } } -unsigned int CCoinsViewCache::GetCacheSize() const { +unsigned int CCoinsViewCacheRead::GetCacheSize() const { return cacheCoins.size(); } -bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const +bool CCoinsViewCacheRead::HaveInputs(const CTransaction& tx) const { if (!tx.IsCoinBase()) { for (unsigned int i = 0; i < tx.vin.size(); i++) { @@ -329,7 +330,7 @@ void CCoinsViewCache::ReallocateCache() ::new (&cacheCoins) CCoinsMap{0, SaltedOutpointHasher{/*deterministic=*/m_deterministic}, CCoinsMap::key_equal{}, &m_cache_coins_memory_resource}; } -void CCoinsViewCache::SanityCheck() const +void CCoinsViewCacheRead::SanityCheck() const { size_t recomputed_usage = 0; size_t count_dirty = 0; diff --git a/src/coins.h b/src/coins.h index c44cc42ef1f8..6c9b253580f3 100644 --- a/src/coins.h +++ b/src/coins.h @@ -414,7 +414,7 @@ class CCoinsViewBackedReadCacheMutable : public CCoinsViewReadCacheMutable public: explicit CCoinsViewBackedReadCacheMutable(CCoinsViewReadCacheMutable* in_view) : base{Assert(in_view)} {} - void SetBackend(CCoinsViewReadCacheMutable& in_view) { base = &in_view; } + virtual void SetBackend(CCoinsViewReadCacheMutable& in_view) { base = &in_view; } std::optional GetCoin(const COutPoint& outpoint) const override { return base->GetCoin(outpoint); } std::optional PeekCoin(const COutPoint& outpoint) const override { return base->PeekCoin(outpoint); } @@ -446,10 +446,14 @@ class CCoinsViewBackedWrite : public CCoinsViewWrite size_t EstimateSize() const override { return base->EstimateSize(); } }; +class CCoinsViewCache; /** CCoinsView that adds a memory cache for transactions to another CCoinsView */ -class CCoinsViewCache : public CCoinsViewBackedWrite +class CCoinsViewCacheRead : public CCoinsViewBackedReadCacheMutable { + // Needed to be able to keep FetchCoin private (not protected) + friend CCoinsViewCache; + private: const bool m_deterministic; @@ -469,30 +473,22 @@ class CCoinsViewCache : public CCoinsViewBackedWrite /* Running count of dirty Coin cache entries. */ mutable size_t m_dirty_count{0}; - /** - * Discard all modifications made to this cache without flushing to the base view. - * This can be used to efficiently reuse a cache instance across multiple operations. - */ - void Reset() noexcept; - /* Fetch the coin from base. Used for cache misses in FetchCoin. */ virtual std::optional FetchCoinFromBase(const COutPoint& outpoint) const; public: - CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic = false); + CCoinsViewCacheRead(CCoinsViewReadCacheMutable* in_base, bool deterministic = false); /** * By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache. */ - CCoinsViewCache(const CCoinsViewCache &) = delete; + CCoinsViewCacheRead(const CCoinsViewCacheRead &) = delete; // Standard CCoinsView methods std::optional GetCoin(const COutPoint& outpoint) const override; std::optional PeekCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint& outpoint) const override; uint256 GetBestBlock() const override; - void SetBestBlock(const uint256& block_hash); - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; std::unique_ptr Cursor() const override { throw std::logic_error("CCoinsViewCache cursor iteration not supported."); } @@ -516,6 +512,71 @@ class CCoinsViewCache : public CCoinsViewBackedWrite */ const Coin& AccessCoin(const COutPoint &output) const; + //! Size of the cache (in number of transaction outputs) + unsigned int GetCacheSize() const; + + //! Number of dirty cache entries (transaction outputs) + size_t GetDirtyCount() const noexcept { return m_dirty_count; } + + //! Calculate the size of the cache (in bytes) + size_t DynamicMemoryUsage() const; + + //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view + bool HaveInputs(const CTransaction& tx) const; + + //! Run an internal sanity check on the cache data structure. */ + void SanityCheck() const; + +private: + /** + * @note this is marked const, but may actually append to `cacheCoins`, increasing + * memory usage. + */ + CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; +}; + +class CCoinsViewCache : public CCoinsViewCacheRead +{ +protected: + CCoinsViewWrite* base_write; + + /** + * Discard all modifications made to this cache without flushing to the base view. + * This can be used to efficiently reuse a cache instance across multiple operations. + */ + void Reset() noexcept; + +public: + CCoinsViewCache(CCoinsViewWrite* in_base, bool deterministic = false) + : + CCoinsViewCacheRead(dynamic_cast(in_base), deterministic), + base_write(in_base) + {} + + /** + * By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache. + */ + CCoinsViewCache(const CCoinsViewCache &) = delete; + + // Make sure base and base_write are in sync + void SetBackend(CCoinsViewReadCacheMutable& in_view) override { throw std::logic_error("CCoinsViewCache::SetBackend(): Use writeable version"); } + void SetBackend(CCoinsViewWrite& in_view) { + // Make sure base and base_write are in sync + base = (CCoinsViewReadCacheMutable*)&in_view; + base_write = &in_view; + } + + CCoinsViewCacheRead& ReadOnly() { return *dynamic_cast(this); } + // Note: Simple type cast can be used insread of this + CCoinsViewReadCacheMutable* AsRead() const { return base; } + CCoinsViewWrite* AsWrite() const { return base_write; } + operator CCoinsViewWrite*() const { return base_write; } + operator CCoinsViewWrite&() const { return *base_write; } + + void SetBestBlock(const uint256& block_hash); + + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash); + /** * Add a coin. Set possible_overwrite to true if an unspent version may * already exist in the cache. @@ -561,18 +622,6 @@ class CCoinsViewCache : public CCoinsViewBackedWrite */ void Uncache(const COutPoint &outpoint); - //! Size of the cache (in number of transaction outputs) - unsigned int GetCacheSize() const; - - //! Number of dirty cache entries (transaction outputs) - size_t GetDirtyCount() const noexcept { return m_dirty_count; } - - //! Calculate the size of the cache (in bytes) - size_t DynamicMemoryUsage() const; - - //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view - bool HaveInputs(const CTransaction& tx) const; - //! Force a reallocation of the cache map. This is required when downsizing //! the cache because the map's allocator may be hanging onto a lot of //! memory despite having called .clear(). @@ -580,9 +629,6 @@ class CCoinsViewCache : public CCoinsViewBackedWrite //! See: https://stackoverflow.com/questions/42114044/how-to-release-unordered-map-memory void ReallocateCache(); - //! Run an internal sanity check on the cache data structure. */ - void SanityCheck() const; - class ResetGuard { private: @@ -601,13 +647,6 @@ class CCoinsViewCache : public CCoinsViewBackedWrite //! Create a scoped guard that will call `Reset()` on this cache when it goes out of scope. [[nodiscard]] ResetGuard CreateResetGuard() noexcept { return ResetGuard{*this}; } - -private: - /** - * @note this is marked const, but may actually append to `cacheCoins`, increasing - * memory usage. - */ - CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; }; /** diff --git a/src/node/coin.cpp b/src/node/coin.cpp index a4ff9aed7025..a3274db124c2 100644 --- a/src/node/coin.cpp +++ b/src/node/coin.cpp @@ -15,7 +15,7 @@ void FindCoins(const NodeContext& node, std::map& coins) assert(node.chainman); LOCK2(cs_main, node.mempool->cs); CCoinsViewCache& chain_view = node.chainman->ActiveChainstate().CoinsTip(); - CCoinsViewMemPool mempool_view(&chain_view, *node.mempool); + CCoinsViewMemPool mempool_view(chain_view.AsWrite(), *node.mempool); for (auto& [outpoint, coin] : coins) { if (auto c{mempool_view.GetCoin(outpoint)}) { coin = std::move(*c); diff --git a/src/rest.cpp b/src/rest.cpp index 81f4fffe9a4e..e366293f413d 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -1017,7 +1017,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: // use db+mempool as cache backend in case user likes to query mempool LOCK2(cs_main, mempool->cs); CCoinsViewCache& viewChain = chainman.ActiveChainstate().CoinsTip(); - CCoinsViewMemPool viewMempool(&viewChain, *mempool); + CCoinsViewMemPool viewMempool(viewChain.AsWrite(), *mempool); process_utxos(viewMempool, mempool); } else { LOCK(cs_main); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 19bc1ad21415..c3339908b5fd 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1247,7 +1247,7 @@ static RPCMethod gettxout() if (fMempool) { const CTxMemPool& mempool = EnsureMemPool(node); LOCK(mempool.cs); - CCoinsViewMemPool view(coins_view, mempool); + CCoinsViewMemPool view(coins_view->AsWrite(), mempool); if (!mempool.isSpent(out)) coin = view.GetCoin(out); } else { coin = coins_view->GetCoin(out); @@ -1298,7 +1298,7 @@ static RPCMethod verifychain() Chainstate& active_chainstate = chainman.ActiveChainstate(); return CVerifyDB(chainman.GetNotifications()).VerifyDB( - active_chainstate, chainman.GetParams().GetConsensus(), active_chainstate.CoinsTip(), check_level, check_depth) == VerifyDBResult::SUCCESS; + active_chainstate, chainman.GetParams().GetConsensus(), *active_chainstate.CoinsTip().AsWrite(), check_level, check_depth) == VerifyDBResult::SUCCESS; }, }; } @@ -2915,7 +2915,7 @@ static RPCMethod getdescriptoractivity() const CTxMemPool& mempool = EnsureMemPool(node); LOCK(::cs_main); LOCK(mempool.cs); - const CCoinsViewCache& coins_view = &active_chainstate.CoinsTip(); + const CCoinsViewCache& coins_view = active_chainstate.CoinsTip(); for (const CTxMemPoolEntry& e : mempool.entryAll()) { const auto& tx = e.GetSharedTx(); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index a0f93f0c79c0..0d0816cb5091 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -623,6 +623,7 @@ static RPCMethod combinerawtransaction() CMutableTransaction mergedTx(txVariants[0]); // Fetch previous transactions (inputs): + // TODO: use CCoinsViewCacheRead CCoinsViewCache view{&CoinsViewEmpty::Get()}; { NodeContext& node = EnsureAnyNodeContext(request.context); @@ -630,7 +631,7 @@ static RPCMethod combinerawtransaction() ChainstateManager& chainman = EnsureChainman(node); LOCK2(cs_main, mempool.cs); CCoinsViewCache &viewChain = chainman.ActiveChainstate().CoinsTip(); - CCoinsViewMemPool viewMempool(&viewChain, mempool); + CCoinsViewMemPool viewMempool(viewChain.AsWrite(), mempool); view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view for (const CTxIn& txin : mergedTx.vin) { diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 05bef698eba8..d5bf0d41a56e 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -256,7 +256,7 @@ void SimulationTest(CCoinsViewWrite* base, bool fake_best_block) //Add a new cache CCoinsViewWrite* tip = base; if (stack.size() > 0) { - tip = stack.back().get(); + tip = stack.back().get()->AsWrite(); } else { removed_all_caches = true; } @@ -509,7 +509,7 @@ BOOST_FIXTURE_TEST_CASE(updatecoins_simulation_test, UpdateTest) if (stack.size() == 0 || (stack.size() < 4 && m_rng.randbool())) { CCoinsViewWrite* tip = &base; if (stack.size() > 0) { - tip = stack.back().get(); + tip = stack.back().get()->AsWrite(); } stack.push_back(std::make_unique(tip)); } @@ -671,15 +671,15 @@ class SingleEntryCacheTest SingleEntryCacheTest(const CAmount base_value, const MaybeCoin& cache_coin) { auto base_cache_coin{base_value == ABSENT ? MISSING : CoinEntry{base_value, CoinEntry::State::DIRTY}}; - WriteCoinsViewEntry(base, base_cache_coin); + WriteCoinsViewEntry(*base.AsWrite(), base_cache_coin); if (cache_coin) { cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), *cache_coin); cache.dirty() += cache_coin->IsDirty(); } } - CCoinsViewCacheTest base{&CoinsViewEmpty::Get()}; - CCoinsViewCacheTest cache{&base}; + CCoinsViewCacheTest base{(CCoinsViewWrite*)&CoinsViewEmpty::Get()}; + CCoinsViewCacheTest cache{base.AsWrite()}; }; static void CheckAccessCoin(const CAmount base_value, const MaybeCoin& cache_coin, const MaybeCoin& expected) @@ -792,7 +792,7 @@ BOOST_AUTO_TEST_CASE(ccoins_add) static void CheckWriteCoins(const MaybeCoin& parent, const MaybeCoin& child, const CoinOrError& expected) { SingleEntryCacheTest test{ABSENT, parent}; - auto write_coins{[&] { WriteCoinsViewEntry(test.cache, child); }}; + auto write_coins{[&] { WriteCoinsViewEntry(*test.cache.AsWrite(), child); }}; if (auto* expected_coin{std::get_if(&expected)}) { write_coins(); test.cache.SelfTest(/*sanity_check=*/false); @@ -1052,7 +1052,7 @@ BOOST_FIXTURE_TEST_CASE(ccoins_flush_behavior, FlushTest) CCoinsViewDB base{{.path = "test", .cache_bytes = 8_MiB, .memory_only = true}, {}}; std::vector> caches; caches.push_back(std::make_unique(&base)); - caches.push_back(std::make_unique(caches.back().get())); + caches.push_back(std::make_unique(caches.back().get()->AsWrite())); for (const auto& view : caches) { TestFlushBehavior(view.get(), base, caches, /*do_erasing_flush=*/false); diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index f7f97c9b371a..7f6e46909e12 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -91,7 +91,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) BlockValidationState state; BOOST_CHECK(CheckBlock(block, state, params.GetConsensus())); BOOST_CHECK(m_node.chainman->AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true)); - CCoinsViewCache view(&chainstate.CoinsTip()); + CCoinsViewCache view((CCoinsViewWrite*)chainstate.CoinsTip()); BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view)); } // Send block connected notification, then stop the index without diff --git a/src/test/coinsviewoverlay_tests.cpp b/src/test/coinsviewoverlay_tests.cpp index fc1f00480f88..d64e82df794a 100644 --- a/src/test/coinsviewoverlay_tests.cpp +++ b/src/test/coinsviewoverlay_tests.cpp @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(fetch_inputs_from_db) CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; PopulateView(block, db); CCoinsViewCache main_cache{&db}; - CoinsViewOverlay view{&main_cache}; + CoinsViewOverlay view{main_cache.AsWrite()}; const auto& outpoint{block.vtx[1]->vin[0].prevout}; BOOST_CHECK(view.HaveCoin(outpoint)); @@ -110,8 +110,8 @@ BOOST_AUTO_TEST_CASE(fetch_inputs_from_cache) const auto block{CreateBlock()}; CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; CCoinsViewCache main_cache{&db}; - PopulateView(block, main_cache); - CoinsViewOverlay view{&main_cache}; + PopulateView(block, *((CCoinsViewWrite*)main_cache)); + CoinsViewOverlay view{(CCoinsViewWrite*)main_cache}; CheckCache(block, view); const auto& outpoint{block.vtx[1]->vin[0].prevout}; @@ -130,8 +130,8 @@ BOOST_AUTO_TEST_CASE(fetch_no_double_spend) PopulateView(block, db); CCoinsViewCache main_cache{&db}; // Add all inputs as spent already in cache - PopulateView(block, main_cache, /*spent=*/true); - CoinsViewOverlay view{&main_cache}; + PopulateView(block, (CCoinsViewWrite&)main_cache, /*spent=*/true); + CoinsViewOverlay view{(CCoinsViewWrite*)main_cache}; for (const auto& tx : block.vtx) { for (const auto& in : tx->vin) { const auto& c{view.AccessCoin(in.prevout)}; @@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(fetch_no_inputs) const auto block{CreateBlock()}; CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; CCoinsViewCache main_cache{&db}; - CoinsViewOverlay view{&main_cache}; + CoinsViewOverlay view{(CCoinsViewWrite*)main_cache}; for (const auto& tx : block.vtx) { for (const auto& in : tx->vin) { const auto& c{view.AccessCoin(in.prevout)}; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 6b4e85a59e45..38b0e3cc84c6 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -45,7 +45,7 @@ struct MinerTestingSetup : public TestingSetup { void TestPrioritisedMining(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool TestSequenceLocks(const CTransaction& tx, CTxMemPool& tx_mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - CCoinsViewMemPool view_mempool{&m_node.chainman->ActiveChainstate().CoinsTip(), tx_mempool}; + CCoinsViewMemPool view_mempool{(CCoinsViewWrite*)m_node.chainman->ActiveChainstate().CoinsTip(), tx_mempool}; CBlockIndex* tip{m_node.chainman->ActiveChain().Tip()}; const std::optional lock_points{CalculateLockPointsAtTip(tip, view_mempool, tx)}; return lock_points.has_value() && CheckSequenceLocksAtTip(tip, *lock_points); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 9ea39c9150a7..37c9ee5008bb 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -142,7 +142,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, script_verify // WITNESS requires P2SH test_flags |= SCRIPT_VERIFY_P2SH; } - bool ret = CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, nullptr); + bool ret = CheckInputScripts(tx, state, active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, nullptr); // CheckInputScripts should succeed iff test_flags doesn't intersect with // failing_flags bool expected_return_value = !(test_flags & failing_flags); @@ -152,13 +152,13 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, script_verify if (ret && add_to_cache) { // Check that we get a cache hit if the tx was valid std::vector scriptchecks; - BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(tx, state, active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); BOOST_CHECK(scriptchecks.empty()); } else { // Check that we get script executions to check, if the transaction // was invalid, or we didn't add to cache. std::vector scriptchecks; - BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(tx, state, active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); } } @@ -216,13 +216,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) TxValidationState state; PrecomputedTransactionData ptd_spend_tx; - BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, nullptr)); + BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, nullptr)); // If we call again asking for scriptchecks (as happens in // ConnectBlock), we should add a script check object for this -- we're // not caching invalidity (if that changes, delete this test case). std::vector scriptchecks; - BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); // Test that CheckInputScripts returns true iff DERSIG-enforcing flags are @@ -312,7 +312,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; TxValidationState state; PrecomputedTransactionData txdata; - BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr)); + BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr)); } // TODO: add tests for remaining script flags @@ -374,12 +374,12 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) TxValidationState state; PrecomputedTransactionData txdata; // This transaction is now invalid under segwit, because of the second input. - BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, nullptr)); + BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, nullptr)); std::vector scriptchecks; // Make sure this transaction was not cached (ie because the first // input was valid) - BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, &scriptchecks)); // Should get 2 script checks back -- caching is on a whole-transaction basis. BOOST_CHECK_EQUAL(scriptchecks.size(), 2U); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4d2d9a6834d8..977c989f45a4 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -449,7 +449,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei assert(!m_txgraph->IsOversized(TxGraph::Level::MAIN)); m_txgraph->SanityCheck(); - CCoinsViewCache mempoolDuplicate(const_cast(&active_coins_tip)); + CCoinsViewCache mempoolDuplicate(active_coins_tip.AsWrite()); const auto score_with_topo{GetSortedScoreWithTopology()}; diff --git a/src/validation.cpp b/src/validation.cpp index 87cf5a1349ed..d88a9c66c41e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -356,7 +356,7 @@ void Chainstate::MaybeUpdateMempoolForReorg( return true; } } else { - const CCoinsViewMemPool view_mempool{&CoinsTip(), *m_mempool}; + const CCoinsViewMemPool view_mempool{CoinsTip().AsWrite(), *m_mempool}; const std::optional new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)}; if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) { // Now update the mempool entry lockpoints as well. @@ -439,7 +439,7 @@ class MemPoolAccept explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : m_pool(mempool), m_view(&CoinsViewEmpty::Get()), - m_viewmempool(&active_chainstate.CoinsTip(), m_pool), + m_viewmempool(active_chainstate.CoinsTip().AsWrite(), m_pool), m_active_chainstate(active_chainstate) { } @@ -1854,7 +1854,7 @@ void CoinsViews::InitCache() { AssertLockHeld(::cs_main); m_cacheview = std::make_unique(&m_catcherview); - m_connect_block_view = std::make_unique(&*m_cacheview); + m_connect_block_view = std::make_unique(m_cacheview->AsWrite()); } Chainstate::Chainstate( @@ -2941,7 +2941,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra // Apply the block atomically to the chain state. const auto time_start{SteadyClock::now()}; { - CCoinsViewCache view(&CoinsTip()); + CCoinsViewCache view(CoinsTip().AsWrite()); assert(view.GetBestBlock() == pindexDelete->GetBlockHash()); if (DisconnectBlock(block, pindexDelete, view) != DISCONNECT_OK) { LogError("DisconnectTip(): DisconnectBlock %s failed\n", pindexDelete->GetBlockHash().ToString()); @@ -4509,7 +4509,7 @@ BlockValidationState TestBlockValidity( index_dummy.pprev = tip; index_dummy.nHeight = tip->nHeight + 1; index_dummy.phashBlock = &block_hash; - CCoinsViewCache view_dummy(&chainstate.CoinsTip()); + CCoinsViewCache view_dummy(chainstate.CoinsTip().AsWrite()); // Set fJustCheck to true in order to update, and not clear, validation caches. if(!chainstate.ConnectBlock(block, state, &index_dummy, view_dummy, /*fJustCheck=*/true)) { From 8b62c77c6b32dbead8ca06978202d92927f16692 Mon Sep 17 00:00:00 2001 From: optout21 <13562139+optout21@users.noreply.github.com> Date: Thu, 30 Apr 2026 09:59:33 +0200 Subject: [PATCH 4/4] Make GetP2SHSigOpCount and GetTransactionSigOpCost get readonly view --- src/coins.h | 1 + src/consensus/tx_verify.cpp | 4 ++-- src/consensus/tx_verify.h | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coins.h b/src/coins.h index 6c9b253580f3..970363583b21 100644 --- a/src/coins.h +++ b/src/coins.h @@ -566,6 +566,7 @@ class CCoinsViewCache : public CCoinsViewCacheRead base_write = &in_view; } + const CCoinsViewCacheRead& ReadOnly() const { return *dynamic_cast(this); } CCoinsViewCacheRead& ReadOnly() { return *dynamic_cast(this); } // Note: Simple type cast can be used insread of this CCoinsViewReadCacheMutable* AsRead() const { return base; } diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 4efed70fd411..cb4aff5519d8 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -123,7 +123,7 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx) return nSigOps; } -unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& inputs) +unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCacheRead& inputs) { if (tx.IsCoinBase()) return 0; @@ -140,7 +140,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in return nSigOps; } -int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, script_verify_flags flags) +int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCacheRead& inputs, script_verify_flags flags) { int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR; diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index ed44d435c1b9..36832db49c8b 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -13,6 +13,7 @@ class CBlockIndex; class CCoinsViewCache; +class CCoinsViewCacheRead; class CTransaction; class TxValidationState; @@ -44,7 +45,7 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx); * @return maximum number of sigops required to validate this transaction's inputs * @see CTransaction::FetchInputs */ -unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& mapInputs); +unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCacheRead& mapInputs); /** * Compute total signature operation cost of a transaction. @@ -53,7 +54,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma * @param[in] flags Script verification flags * @return Total signature operation cost of tx */ -int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, script_verify_flags flags); +int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCacheRead& inputs, script_verify_flags flags); /** * Check if transaction is final and can be included in a block with the