From 8e5fc7507b7cb4d606b7b6a8df710d3a6c89e1ae Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 18 Apr 2025 11:09:34 +0200
Subject: [PATCH 1/9] consensus: Use Coin span in GetP2SHSigOpCount

Move the responsibility of retrieving coins from GetP2SHSigOpCount to
its caller.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.
---
 src/coins.h                    | 15 +++++++++++++++
 src/consensus/tx_verify.cpp    | 22 ++++++++++++++++------
 src/consensus/tx_verify.h      | 12 ++++++++++--
 src/test/fuzz/coins_view.cpp   |  2 +-
 src/test/script_p2sh_tests.cpp | 14 +++++++++-----
 src/test/transaction_tests.cpp |  8 +++++---
 6 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/src/coins.h b/src/coins.h
index ae7f34f46581..c005b2f6fea6 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -458,6 +458,21 @@ class CCoinsViewCache : public CCoinsViewBacked
      */
     const Coin& AccessCoin(const COutPoint &output) const;
 
+    /**
+     * Retrieves a vector of references to Coins in the cache in order they get
+     * consumed by the tx's inputs, and passes it to the invoked callback.
+     */
+    template <typename Callable>
+    auto AccessCoins(const CTransaction& tx, Callable&& callback) const
+    {
+        std::vector<std::reference_wrapper<const Coin>> coins;
+        coins.reserve(tx.vin.size());
+        for (const CTxIn& input : tx.vin) {
+            coins.emplace_back(AccessCoin(input.prevout));
+        }
+        return callback(std::span{coins});
+    }
+
     /**
      * Add a coin. Set possible_overwrite to true if an unspent version may
      * already exist in the cache.
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 4efed70fd411..140c4b522dc0 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -14,6 +14,8 @@
 #include <util/check.h>
 #include <util/moneystr.h>
 
+#include <span>
+
 bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
 {
     if (tx.nLockTime == 0)
@@ -123,23 +125,31 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx)
     return nSigOps;
 }
 
-unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& inputs)
+template <Consensus::CoinRef T>
+unsigned int GetP2SHSigOpCount(const CTransaction& tx, const std::span<T> coins)
 {
     if (tx.IsCoinBase())
         return 0;
 
     unsigned int nSigOps = 0;
-    for (unsigned int i = 0; i < tx.vin.size(); i++)
-    {
-        const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout);
+    Assert(coins.size() == tx.vin.size());
+    auto input_it = tx.vin.begin();
+    for (auto it = coins.begin(); it != coins.end(); ++it, ++input_it) {
+        const Coin& coin = *it;
         assert(!coin.IsSpent());
         const CTxOut &prevout = coin.out;
         if (prevout.scriptPubKey.IsPayToScriptHash())
-            nSigOps += prevout.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig);
+            nSigOps += prevout.scriptPubKey.GetSigOpCount(input_it->scriptSig);
     }
     return nSigOps;
 }
 
+template unsigned int GetP2SHSigOpCount<const Coin>(
+    const CTransaction& tx, const std::span<const Coin>);
+
+template unsigned int GetP2SHSigOpCount<std::reference_wrapper<const Coin>>(
+    const CTransaction& tx, const std::span<std::reference_wrapper<const Coin>>);
+
 int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, script_verify_flags flags)
 {
     int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR;
@@ -148,7 +158,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
         return nSigOps;
 
     if (flags & SCRIPT_VERIFY_P2SH) {
-        nSigOps += GetP2SHSigOpCount(tx, inputs) * WITNESS_SCALE_FACTOR;
+        nSigOps += inputs.AccessCoins(tx, [&tx](auto&& coins) { return GetP2SHSigOpCount(tx, coins); }) * WITNESS_SCALE_FACTOR;
     }
 
     for (unsigned int i = 0; i < tx.vin.size(); i++)
diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h
index ed44d435c1b9..48cad6730bb1 100644
--- a/src/consensus/tx_verify.h
+++ b/src/consensus/tx_verify.h
@@ -5,10 +5,13 @@
 #ifndef BITCOIN_CONSENSUS_TX_VERIFY_H
 #define BITCOIN_CONSENSUS_TX_VERIFY_H
 
+#include <coins.h>
 #include <consensus/amount.h>
 #include <script/verify_flags.h>
 
+#include <concepts>
 #include <cstdint>
+#include <span>
 #include <vector>
 
 class CBlockIndex;
@@ -19,6 +22,10 @@ class TxValidationState;
 /** Transaction validation functions */
 
 namespace Consensus {
+
+template <typename T>
+concept CoinRef = std::convertible_to<T, const Coin&>;
+
 /**
  * Check whether all inputs of this transaction are valid (no double spends and amounts)
  * This does not modify the UTXO set. This does not check scripts and sigs.
@@ -40,11 +47,12 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx);
 /**
  * Count ECDSA signature operations in pay-to-script-hash inputs.
  *
- * @param[in] mapInputs Map of previous transactions that have outputs we're spending
+ * @param[in] coins span of Coins containing previous transaction outputs in the order we're spending them
  * @return maximum number of sigops required to validate this transaction's inputs
  * @see CTransaction::FetchInputs
  */
-unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& mapInputs);
+template <Consensus::CoinRef T>
+unsigned int GetP2SHSigOpCount(const CTransaction& tx, std::span<T> coins);
 
 /**
  * Compute total signature operation cost of a transaction.
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index c11581d2d3c3..a6ec38ad1957 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -299,7 +299,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
                     // consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed.
                     return;
                 }
-                (void)GetP2SHSigOpCount(transaction, coins_view_cache);
+                (void)coins_view_cache.AccessCoins(transaction, [&transaction](auto&& coins) { return GetP2SHSigOpCount(transaction, coins); });
             },
             [&] {
                 const CTransaction transaction{random_mutable_transaction};
diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp
index f80090aa2b11..f7524bd1cc82 100644
--- a/src/test/script_p2sh_tests.cpp
+++ b/src/test/script_p2sh_tests.cpp
@@ -378,13 +378,14 @@ BOOST_AUTO_TEST_CASE(ValidateInputsStandardness)
 
     BOOST_CHECK(::ValidateInputsStandardness(CTransaction(txTo), coins).IsValid());
     // 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), coins), 22U);
+    auto count = coins.AccessCoins(CTransaction(txTo), [&txTo](auto&& coins) { return GetP2SHSigOpCount(CTransaction(txTo), coins); });
+    BOOST_CHECK_EQUAL(count, 22U);
 
     CMutableTransaction coinbase_tx_mut;
     coinbase_tx_mut.vin.resize(1);
     CTransaction coinbase_tx{coinbase_tx_mut};
     BOOST_CHECK(coinbase_tx.IsCoinBase());
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(coinbase_tx, coins), 0U);
+    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(coinbase_tx, std::span<const Coin>{}), 0U);
 
     // TxoutType::SCRIPTHASH
     CMutableTransaction txToNonStd1;
@@ -401,7 +402,8 @@ BOOST_AUTO_TEST_CASE(ValidateInputsStandardness)
     BOOST_CHECK_EQUAL(txToNonStd1_res.GetRejectReason(), "bad-txns-nonstandard-inputs");
     BOOST_CHECK_EQUAL(txToNonStd1_res.GetDebugMessage(), "p2sh redeemscript sigops exceed limit (input 0: 16 > 15)");
 
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U);
+    count = coins.AccessCoins(CTransaction(txToNonStd1), [&txToNonStd1](auto&& coins) { return GetP2SHSigOpCount(CTransaction(txToNonStd1), coins); });
+    BOOST_CHECK_EQUAL(count, 16U);
 
     CMutableTransaction txToNonStd2;
     txToNonStd2.vout.resize(1);
@@ -416,7 +418,8 @@ BOOST_AUTO_TEST_CASE(ValidateInputsStandardness)
     BOOST_CHECK(txToNonStd2_res.IsInvalid());
     BOOST_CHECK_EQUAL(txToNonStd2_res.GetRejectReason(), "bad-txns-nonstandard-inputs");
     BOOST_CHECK_EQUAL(txToNonStd2_res.GetDebugMessage(), "p2sh redeemscript sigops exceed limit (input 0: 20 > 15)");
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
+    count = coins.AccessCoins(CTransaction(txToNonStd2), [&txToNonStd2](auto&& coins) { return GetP2SHSigOpCount(CTransaction(txToNonStd2), coins); });
+    BOOST_CHECK_EQUAL(count, 20U);
 
     CMutableTransaction txToNonStd2_no_scriptSig;
     txToNonStd2_no_scriptSig.vout.resize(1);
@@ -430,7 +433,8 @@ BOOST_AUTO_TEST_CASE(ValidateInputsStandardness)
     BOOST_CHECK(txToNonStd2_no_scriptSig_res.IsInvalid());
     BOOST_CHECK_EQUAL(txToNonStd2_no_scriptSig_res.GetRejectReason(), "bad-txns-nonstandard-inputs");
     BOOST_CHECK_EQUAL(txToNonStd2_no_scriptSig_res.GetDebugMessage(), "input 0 P2SH redeemscript missing");
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
+    count = coins.AccessCoins(CTransaction(txToNonStd2), [&txToNonStd2](auto&& coins) { return GetP2SHSigOpCount(CTransaction(txToNonStd2), coins); });
+    BOOST_CHECK_EQUAL(count, 20U);
 
     // TxoutType::NONSTANDARD
     CMutableTransaction txToNonStd3;
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 19c416c225f3..dd76a3cbbe9d 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -1049,7 +1049,8 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops)
     AddCoins(coins, CTransaction(tx_create), 0, false);
 
     // 2490 sigops is below the limit.
-    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2490);
+    auto count = coins.AccessCoins(CTransaction(tx_max_sigops), [&tx_max_sigops](auto&& coins) { return GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins); });
+    BOOST_CHECK_EQUAL(count, 2490);
     BOOST_CHECK(::ValidateInputsStandardness(CTransaction(tx_max_sigops), coins).IsValid());
 
     // Adding one more input will bump this to 2505, hitting the limit.
@@ -1060,8 +1061,10 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops)
     }
     tx_max_sigops.vin.emplace_back(prev_txid, p2sh_inputs_count, CScript() << ToByteVector(max_sigops_redeem_script));
     AddCoins(coins, CTransaction(tx_create), 0, false);
+    count = coins.AccessCoins(CTransaction(tx_max_sigops), [&tx_max_sigops](auto&& coins) { return GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins); });
+    BOOST_CHECK_EQUAL(count, 2505);
     BOOST_CHECK_GT((p2sh_inputs_count + 1) * MAX_P2SH_SIGOPS, MAX_TX_LEGACY_SIGOPS);
-    auto legacy_sigops_count = GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins);
+    auto legacy_sigops_count = coins.AccessCoins(CTransaction(tx_max_sigops), [&tx_max_sigops](auto&& coins) { return GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins); });
     BOOST_CHECK_EQUAL(legacy_sigops_count, 2505);
     std::string reject_reason("bad-txns-nonstandard-inputs");
     std::string sigop_limit_reject_debug_message("non-witness sigops exceed bip54 limit");
@@ -1072,7 +1075,6 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops)
         BOOST_CHECK_EQUAL(validation_state.GetDebugMessage(), sigop_limit_reject_debug_message);
     }
 
-
     // Now, check the limit can be reached with regular P2PK outputs too. Use a separate
     // preparation transaction, to demonstrate spending coins from a single tx is irrelevant.
     CMutableTransaction tx_create_p2pk;

From 1134d4ca50ddc22ce2c4194bf061639752fd4000 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 18 Apr 2025 12:51:31 +0200
Subject: [PATCH 2/9] consensus: Use Coin span in GetTransactionSigOpCost

Move the responsibility of retrieving coins from GetTransactionSigOpCost
to its caller.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.
---
 src/consensus/tx_verify.cpp   | 19 +++++++----
 src/consensus/tx_verify.h     |  7 ++--
 src/node/psbt.cpp             |  5 ++-
 src/test/fuzz/coins_view.cpp  |  4 ++-
 src/test/sigopcount_tests.cpp | 61 ++++++++++++++++++++++++++++-------
 src/validation.cpp            | 16 ++++++---
 6 files changed, 84 insertions(+), 28 deletions(-)

diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 140c4b522dc0..e572bee634e1 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -150,7 +150,8 @@ template unsigned int GetP2SHSigOpCount<const Coin>(
 template unsigned int GetP2SHSigOpCount<std::reference_wrapper<const Coin>>(
     const CTransaction& tx, const std::span<std::reference_wrapper<const Coin>>);
 
-int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, script_verify_flags flags)
+template <Consensus::CoinRef T>
+int64_t GetTransactionSigOpCost(const CTransaction& tx, const std::span<T> coins, script_verify_flags flags)
 {
     int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR;
 
@@ -158,18 +159,24 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
         return nSigOps;
 
     if (flags & SCRIPT_VERIFY_P2SH) {
-        nSigOps += inputs.AccessCoins(tx, [&tx](auto&& coins) { return GetP2SHSigOpCount(tx, coins); }) * WITNESS_SCALE_FACTOR;
+        nSigOps += GetP2SHSigOpCount(tx, coins) * WITNESS_SCALE_FACTOR;
     }
 
-    for (unsigned int i = 0; i < tx.vin.size(); i++)
-    {
-        const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout);
+    Assert(coins.size() == tx.vin.size());
+    auto input_it = tx.vin.begin();
+    for (auto it = coins.begin(); it != coins.end(); ++it, ++input_it) {
+        const Coin& coin = *it;
         assert(!coin.IsSpent());
         const CTxOut &prevout = coin.out;
-        nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, tx.vin[i].scriptWitness, flags);
+        nSigOps += CountWitnessSigOps(input_it->scriptSig, prevout.scriptPubKey, input_it->scriptWitness, flags);
     }
     return nSigOps;
 }
+template int64_t GetTransactionSigOpCost<const Coin>(
+    const CTransaction& tx, std::span<const Coin> coins, script_verify_flags flags);
+
+template int64_t GetTransactionSigOpCost<std::reference_wrapper<const Coin>>(
+    const CTransaction& tx, const std::span<std::reference_wrapper<const Coin>> coins, script_verify_flags flags);
 
 bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee)
 {
diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h
index 48cad6730bb1..84ff1e2f9aa1 100644
--- a/src/consensus/tx_verify.h
+++ b/src/consensus/tx_verify.h
@@ -56,12 +56,13 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, std::span<T> coins);
 
 /**
  * Compute total signature operation cost of a transaction.
- * @param[in] tx     Transaction for which we are computing the cost
- * @param[in] inputs Map of previous transactions that have outputs we're spending
+ * @param[in] tx    Transaction for which we are computing the cost
+ * @param[in] coins span of Coins containing previous transaction outputs in the order we're spending them
  * @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);
+template <Consensus::CoinRef T>
+int64_t GetTransactionSigOpCost(const CTransaction& tx, std::span<T> coins, script_verify_flags flags);
 
 /**
  * Check if transaction is final and can be included in a block with the
diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp
index 9d79b42f29a2..cb69a616ba94 100644
--- a/src/node/psbt.cpp
+++ b/src/node/psbt.cpp
@@ -136,7 +136,10 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
 
         if (success) {
             CTransaction ctx = CTransaction(mtx);
-            size_t size(GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS), ::nBytesPerSigOp));
+            auto tx_ops = view.AccessCoins(ctx, [&ctx](auto&& coins) {
+                return GetTransactionSigOpCost(ctx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
+            });
+            size_t size(GetVirtualTransactionSize(ctx, tx_ops, ::nBytesPerSigOp));
             result.estimated_vsize = size;
             // Estimate fee rate
             CFeeRate feerate(fee, size);
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index a6ec38ad1957..c2285633a7ba 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -314,7 +314,9 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
                     // script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness &, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed.
                     return;
                 }
-                (void)GetTransactionSigOpCost(transaction, coins_view_cache, flags);
+                (void)coins_view_cache.AccessCoins(transaction, [&transaction, &flags](auto&& coins) {
+                    return GetTransactionSigOpCost(transaction, coins, flags);
+                });
             },
             [&] {
                 (void)IsWitnessStandard(CTransaction{random_mutable_transaction}, coins_view_cache);
diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp
index a463f43ea886..30da3b97ca04 100644
--- a/src/test/sigopcount_tests.cpp
+++ b/src/test/sigopcount_tests.cpp
@@ -133,10 +133,16 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
         // Legacy counting only includes signature operations in scriptSigs and scriptPubKeys
         // of a transaction and does not take the actual executed sig operations into account.
         // spendingTx in itself does not contain a signature operation.
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 0);
         // creationTx contains two signature operations in its scriptPubKey, but legacy counting
         // is not accurate.
-        assert(GetTransactionSigOpCost(CTransaction(creationTx), coins, flags) == MAX_PUBKEYS_PER_MULTISIG * WITNESS_SCALE_FACTOR);
+        tx_sigops = coins.AccessCoins(CTransaction(creationTx), [&creationTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(creationTx), coins, flags);
+        });
+        assert(tx_sigops == MAX_PUBKEYS_PER_MULTISIG * WITNESS_SCALE_FACTOR);
         // Sanity check: script verification fails because of an invalid signature.
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
     }
@@ -148,11 +154,17 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
         CScript scriptSig = CScript() << OP_0 << OP_0 << ToByteVector(redeemScript);
 
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CScriptWitness());
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2 * WITNESS_SCALE_FACTOR);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 2 * WITNESS_SCALE_FACTOR);
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
 
         // P2SH sigops are not counted if we don't set the SCRIPT_VERIFY_P2SH flag
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, /*flags=*/0) == 0);
+        tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, /*flags=*/0);
+        });
+        assert(tx_sigops == 0);
     }
 
     // P2WPKH witness program
@@ -165,22 +177,35 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
 
 
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 1);
         // No signature operations if we don't verify the witness.
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0);
+        tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS);
+        });
+
+        assert(tx_sigops == 0);
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_EQUALVERIFY);
 
         // The sig op cost for witness version != 0 is zero.
         assert(scriptPubKey[0] == 0x00);
         scriptPubKey[0] = 0x51;
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
+        tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 0);
         scriptPubKey[0] = 0x00;
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
 
         // The witness of a coinbase transaction is not taken into account.
         spendingTx.vin[0].prevout.SetNull();
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
+        tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 0);
     }
 
     // P2WPKH nested in P2SH
@@ -193,7 +218,10 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
         scriptWitness.stack.emplace_back(0);
 
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 1);
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_EQUALVERIFY);
     }
 
@@ -208,8 +236,14 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
         scriptWitness.stack.emplace_back(witnessScript.begin(), witnessScript.end());
 
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 2);
+        tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS);
+        });
+        assert(tx_sigops == 0);
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
     }
 
@@ -225,7 +259,10 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
         scriptWitness.stack.emplace_back(witnessScript.begin(), witnessScript.end());
 
         BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
-        assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2);
+        auto tx_sigops = coins.AccessCoins(CTransaction(spendingTx), [&spendingTx, &flags](auto&& coins) {
+            return GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags);
+        });
+        assert(tx_sigops == 2);
         assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
     }
 }
diff --git a/src/validation.cpp b/src/validation.cpp
index f85a834f2a4a..3816b6cda5c0 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -902,7 +902,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard");
     }
 
-    int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS);
+    int64_t nSigOpsCost = m_view.AccessCoins(tx, [&tx](auto&& coins) {
+        return GetTransactionSigOpCost(tx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
+    });
 
     // Keep track of transactions that spend a coinbase, which we re-scan
     // during reorgs to ensure COINBASE_MATURITY is still met.
@@ -2562,10 +2564,14 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         // * legacy (always)
         // * p2sh (when P2SH enabled in flags and excludes coinbase)
         // * witness (when witness enabled in flags and excludes coinbase)
-        nSigOpsCost += GetTransactionSigOpCost(tx, view, flags);
-        if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) {
-            state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops", "too many sigops");
-            break;
+        {
+            nSigOpsCost += view.AccessCoins(tx, [&tx, &flags](auto&& coins) {
+                return GetTransactionSigOpCost(tx, coins, flags);
+            });
+            if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) {
+                state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops", "too many sigops");
+                break;
+            }
         }
 
         if (!tx.IsCoinBase() && fScriptChecks)

From 9afbec43ab32c62b0fcd6d0134810bd07e6786c7 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 18 Apr 2025 14:59:25 +0200
Subject: [PATCH 3/9] consensus: Use Coin span in CheckTxInputs

Move the responsibility of retrieving coins from CheckTxInputs
to its caller. The missing inputs check will be moved in an upcoming
commit.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.
---
 src/consensus/tx_verify.cpp    | 18 +++++++++++++-----
 src/consensus/tx_verify.h      |  4 +++-
 src/test/fuzz/coins_view.cpp   |  7 ++++++-
 src/test/transaction_tests.cpp |  4 +++-
 src/txmempool.cpp              |  4 +++-
 src/validation.cpp             | 13 ++++++++++---
 6 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index e572bee634e1..6c17f755675a 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -178,7 +178,8 @@ template int64_t GetTransactionSigOpCost<const Coin>(
 template int64_t GetTransactionSigOpCost<std::reference_wrapper<const Coin>>(
     const CTransaction& tx, const std::span<std::reference_wrapper<const Coin>> coins, script_verify_flags flags);
 
-bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee)
+template <Consensus::CoinRef T>
+bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<T> coins, int nSpendHeight, CAmount& txfee)
 {
     // are the actual inputs available?
     if (!inputs.HaveInputs(tx)) {
@@ -187,15 +188,16 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state,
     }
 
     CAmount nValueIn = 0;
-    for (unsigned int i = 0; i < tx.vin.size(); ++i) {
-        const COutPoint &prevout = tx.vin[i].prevout;
-        const Coin& coin = inputs.AccessCoin(prevout);
+    Assert(coins.size() == tx.vin.size());
+    auto input_it = tx.vin.begin();
+    for (auto it = coins.begin(); it != coins.end(); ++it, ++input_it) {
+        const Coin& coin = *it;
         assert(!coin.IsSpent());
 
         // If prev is coinbase, check that it's matured
         if (coin.IsCoinBase() && nSpendHeight - coin.nHeight < COINBASE_MATURITY) {
             return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "bad-txns-premature-spend-of-coinbase",
-                strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight));
+                strprintf("tried to spend coinbase at depth %d", static_cast<int>(nSpendHeight - coin.nHeight)));
         }
 
         // Check for negative or overflow input values
@@ -229,3 +231,9 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state,
     txfee = txfee_aux;
     return true;
 }
+
+template bool Consensus::CheckTxInputs<const Coin>(
+    const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<const Coin> coins, int nSpendHeight, CAmount& txfee);
+
+template bool Consensus::CheckTxInputs<std::reference_wrapper<const Coin>>(
+    const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<std::reference_wrapper<const Coin>> coins, int nSpendHeight, CAmount& txfee);
diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h
index 84ff1e2f9aa1..746c3181af57 100644
--- a/src/consensus/tx_verify.h
+++ b/src/consensus/tx_verify.h
@@ -29,10 +29,12 @@ concept CoinRef = std::convertible_to<T, const Coin&>;
 /**
  * Check whether all inputs of this transaction are valid (no double spends and amounts)
  * This does not modify the UTXO set. This does not check scripts and sigs.
+ * @param[in] coins span of Coins containing previous transaction outputs in the order we're spending them
  * @param[out] txfee Set to the transaction fee if successful.
  * Preconditions: tx.IsCoinBase() is false.
  */
-[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee);
+template <Consensus::CoinRef T>
+[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, std::span<T> coins, int nSpendHeight, CAmount& txfee);
 } // namespace Consensus
 
 /** Auxiliary functions for transaction validation (ideally should not be exposed) */
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index c2285633a7ba..82561917989c 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -288,7 +288,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
                     // It is not allowed to call CheckTxInputs if CheckTransaction failed
                     return;
                 }
-                if (Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out)) {
+                // are the actual inputs available?
+                if (!coins_view_cache.HaveInputs(transaction)) return;
+                auto check_tx_inputs_res = coins_view_cache.AccessCoins(transaction, [&transaction, &state, &coins_view_cache, &fuzzed_data_provider, &tx_fee_out](auto&& coins) {
+                    return Consensus::CheckTxInputs(transaction, state, coins_view_cache, std::span{coins}, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out);
+                });
+                if (check_tx_inputs_res) {
                     assert(MoneyRange(tx_fee_out));
                 }
             },
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index dd76a3cbbe9d..05961073b2a1 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -1142,7 +1142,9 @@ BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test)
 
         TxValidationState state;
         CAmount txfee{0};
-        BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction{mtx}, state, inputs, spend_height, txfee));
+        inputs.AccessCoins(CTransaction(mtx), [&mtx, &state, &inputs, spend_height, &txfee](auto&& coins) {
+            BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction{mtx}, state, inputs, coins, spend_height, txfee));
+        });
         BOOST_CHECK(state.IsInvalid());
         BOOST_CHECK_EQUAL(state.GetResult(), expected_result);
         BOOST_CHECK_EQUAL(state.GetRejectReason(), expected_reason);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index a22cd2b199d7..4cbefa8b8762 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -534,7 +534,9 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
         TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass
         CAmount txfee = 0;
         assert(!tx.IsCoinBase());
-        assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee));
+        mempoolDuplicate.AccessCoins(tx, [&](auto&& coins) {
+            assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, std::span{coins}, spendheight, txfee));
+        });
         for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout);
         AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max());
     }
diff --git a/src/validation.cpp b/src/validation.cpp
index 3816b6cda5c0..2f298c61ab6b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -886,7 +886,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     }
 
     // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
-    if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) {
+    auto check_tx_inputs_res = m_view.AccessCoins(tx, [&tx, &state, &ws, this](auto&& coins) {
+        return Consensus::CheckTxInputs(tx, state, m_view, std::span{coins}, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees);
+    });
+    if (!check_tx_inputs_res) {
         return false; // state filled in by CheckTxInputs
     }
 
@@ -2524,14 +2527,18 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     {
         if (!state.IsValid()) break;
         const CTransaction &tx = *(block.vtx[i]);
-
         nInputs += tx.vin.size();
 
         if (!tx.IsCoinBase())
         {
             CAmount txfee = 0;
             TxValidationState tx_state;
-            if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee)) {
+
+            auto check_tx_inputs_res = view.AccessCoins(tx, [&tx, &tx_state, &view, pindex, &txfee](auto&& coins) {
+                return Consensus::CheckTxInputs(tx, tx_state, view, coins, pindex->nHeight, txfee);
+            });
+
+            if (!check_tx_inputs_res) {
                 // Any transaction validation failure in ConnectBlock is a block consensus failure
                 state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,
                               tx_state.GetRejectReason(),

From 1bfe03f05b7983d4957d3a262cf75a338b8d9bcc Mon Sep 17 00:00:00 2001
From: sedited <seb.kung@gmail.com>
Date: Tue, 24 Mar 2026 13:23:43 +0100
Subject: [PATCH 4/9] validation: Consolidate AccessCoins calls in PreChecks

This should introduce no behaviour change, but avoids a few extra vector
allocations and coins lookups
---
 src/validation.cpp | 58 ++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 2f298c61ab6b..c96cac428761 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -885,40 +885,42 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final");
     }
 
-    // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
-    auto check_tx_inputs_res = m_view.AccessCoins(tx, [&tx, &state, &ws, this](auto&& coins) {
-        return Consensus::CheckTxInputs(tx, state, m_view, std::span{coins}, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees);
-    });
-    if (!check_tx_inputs_res) {
-        return false; // state filled in by CheckTxInputs
-    }
+    int64_t nSigOpsCost = 0;
+    bool fSpendsCoinbase = false;
+    // Wrap access to required coins in their own scope to avoid coin lifetime extension issues
+    auto res = m_view.AccessCoins(tx, [&, this](auto&& coins) {
+        // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
+        if (!Consensus::CheckTxInputs(tx, state, m_view, std::span{coins}, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) {
+            return false; // state filled in by CheckTxInputs
+        }
 
-    if (m_pool.m_opts.require_standard) {
-        state = ValidateInputsStandardness(tx, m_view);
-        if (state.IsInvalid()) {
-            return false;
+        if (m_pool.m_opts.require_standard) {
+            state = ValidateInputsStandardness(tx, m_view);
+            if (state.IsInvalid()) {
+                return false;
+            }
         }
-    }
 
-    // Check for non-standard witnesses.
-    if (tx.HasWitness() && m_pool.m_opts.require_standard && !IsWitnessStandard(tx, m_view)) {
-        return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard");
-    }
+        // Check for non-standard witnesses.
+        if (tx.HasWitness() && m_pool.m_opts.require_standard && !IsWitnessStandard(tx, m_view)) {
+            return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard");
+        }
 
-    int64_t nSigOpsCost = m_view.AccessCoins(tx, [&tx](auto&& coins) {
-        return GetTransactionSigOpCost(tx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
-    });
+        nSigOpsCost = GetTransactionSigOpCost(tx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
 
-    // Keep track of transactions that spend a coinbase, which we re-scan
-    // during reorgs to ensure COINBASE_MATURITY is still met.
-    bool fSpendsCoinbase = false;
-    for (const CTxIn &txin : tx.vin) {
-        const Coin &coin = m_view.AccessCoin(txin.prevout);
-        if (coin.IsCoinBase()) {
-            fSpendsCoinbase = true;
-            break;
+        // Keep track of transactions that spend a coinbase, which we re-scan
+        // during reorgs to ensure COINBASE_MATURITY is still met.
+        for (const Coin& coin : coins) {
+            if (coin.IsCoinBase()) {
+                fSpendsCoinbase = true;
+                break;
+            }
         }
-    }
+
+        return true;
+    });
+
+    if (!res) return res;
 
     // Set entry_sequence to 0 when bypass_limits is used; this allows txs from a block
     // reorg to be marked earlier than any child txs that were already in the mempool.

From 8b19265f597049179602653c55399f9bb5b2c1ed Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 18 Apr 2025 22:24:03 +0200
Subject: [PATCH 5/9] validation: Use vector of outputs instead of
 CCoinsViewCache in CheckInputScripts

Move the responsibility of retrieving coins from the CCoinsViewCache
in CheckInputScripts to its caller.

Add a helper method in CCoinsViewCache to collect all Coin's outputs
spent by a transaction's inputs.

Callers of CCoinsViewCache are updated to either pre-fetch the spent
outputs, or pass in an empty vector if the precomputed transaction data
has already been initialized with the required outputs.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.
---
 src/coins.cpp                        | 13 +++++++++++
 src/coins.h                          |  7 ++++++
 src/test/txvalidationcache_tests.cpp | 35 ++++++++++++++++++++--------
 src/validation.cpp                   | 28 ++++++++++------------
 4 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/src/coins.cpp b/src/coins.cpp
index c403e006c85c..4b4da8b78f69 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -164,6 +164,19 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const {
     }
 }
 
+std::vector<CTxOut> CCoinsViewCache::GetUnspentOutputs(const CTransaction& tx) const
+{
+    std::vector<CTxOut> spent_outputs;
+    spent_outputs.reserve(tx.vin.size());
+    for (const auto& txin : tx.vin) {
+        const COutPoint& prevout = txin.prevout;
+        const Coin& coin = AccessCoin(prevout);
+        assert(!coin.IsSpent());
+        spent_outputs.emplace_back(coin.out);
+    }
+    return spent_outputs;
+}
+
 bool CCoinsViewCache::HaveCoin(const COutPoint& outpoint) const
 {
     CCoinsMap::const_iterator it = FetchCoin(outpoint);
diff --git a/src/coins.h b/src/coins.h
index c005b2f6fea6..2f16cf33f7c0 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -473,6 +473,13 @@ class CCoinsViewCache : public CCoinsViewBacked
         return callback(std::span{coins});
     }
 
+    /**
+     * Return a vector of unspent outputs of coins in the cache that are spent
+     * by the provided transaction. The coins they belong to must be unspent in
+     * the cache.
+     */
+    std::vector<CTxOut> GetUnspentOutputs(const CTransaction& tx) const;
+
     /**
      * Add a coin. Set possible_overwrite to true if an unspent version may
      * already exist in the cache.
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 9ea39c9150a7..e429f5585b06 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -21,7 +21,7 @@ struct Dersig100Setup : public TestChain100Setup {
 };
 
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
-                       const CCoinsViewCache& inputs, script_verify_flags flags, bool cacheSigStore,
+                       std::vector<CTxOut>&& spent_outputs, script_verify_flags flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
                        std::vector<CScriptCheck>* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -124,6 +124,9 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, script_verify
 {
     PrecomputedTransactionData txdata;
 
+    std::vector<CTxOut> spent_outputs{active_coins_tip.GetUnspentOutputs(tx)};
+    txdata.Init(tx, std::move(spent_outputs));
+
     FastRandomContext insecure_rand(true);
 
     for (int count = 0; count < 10000; ++count) {
@@ -142,7 +145,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, {}, 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 +155,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<CScriptCheck> scriptchecks;
-            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
+            BOOST_CHECK(CheckInputScripts(tx, state, {}, 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<CScriptCheck> scriptchecks;
-            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
+            BOOST_CHECK(CheckInputScripts(tx, state, {}, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
             BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size());
         }
     }
@@ -216,13 +219,16 @@ 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));
+        std::vector<CTxOut> spent_outputs{m_node.chainman->ActiveChainstate().CoinsTip().GetUnspentOutputs(CTransaction(spend_tx))};
+        ptd_spend_tx.Init(spend_tx, std::move(spent_outputs));
+
+        BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, {}, 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<CScriptCheck> 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, {}, 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
@@ -284,7 +290,10 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
         TxValidationState state;
         PrecomputedTransactionData txdata;
-        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
+        std::vector<CTxOut> spent_outputs{m_node.chainman->ActiveChainstate().CoinsTip().GetUnspentOutputs(CTransaction(invalid_with_cltv_tx))};
+        txdata.Init(invalid_with_cltv_tx, std::move(spent_outputs));
+
+        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, {}, SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
     }
 
     // TEST CHECKSEQUENCEVERIFY
@@ -312,7 +321,10 @@ 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));
+        std::vector<CTxOut> spent_outputs{m_node.chainman->ActiveChainstate().CoinsTip().GetUnspentOutputs(CTransaction(invalid_with_csv_tx))};
+        txdata.Init(invalid_with_csv_tx, std::move(spent_outputs));
+
+        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, {}, SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
     }
 
     // TODO: add tests for remaining script flags
@@ -373,13 +385,16 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
 
         TxValidationState state;
         PrecomputedTransactionData txdata;
+        std::vector<CTxOut> spent_outputs{m_node.chainman->ActiveChainstate().CoinsTip().GetUnspentOutputs(CTransaction(tx))};
+        txdata.Init(tx, std::move(spent_outputs));
+
         // 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, {}, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
 
         std::vector<CScriptCheck> 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, {}, 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/validation.cpp b/src/validation.cpp
index c96cac428761..d56caf7b0260 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -139,11 +139,11 @@ const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locato
 }
 
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
-                       const CCoinsViewCache& inputs, script_verify_flags flags, bool cacheSigStore,
+                       std::vector<CTxOut>&& spent_outputs, script_verify_flags flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
                        std::vector<CScriptCheck>* pvChecks = nullptr)
-                       EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx)
 {
@@ -427,8 +427,10 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationS
         }
     }
 
+    std::vector<CTxOut> spent_outputs{view.GetUnspentOutputs(tx)};
+
     // Call CheckInputScripts() to cache signature and script validity against current tip consensus rules.
-    return CheckInputScripts(tx, state, view, flags, /* cacheSigStore= */ true, /* cacheFullScriptStore= */ true, txdata, validation_cache);
+    return CheckInputScripts(tx, state, std::move(spent_outputs), flags, /* cacheSigStore= */ true, /* cacheFullScriptStore= */ true, txdata, validation_cache);
 }
 
 namespace {
@@ -1145,9 +1147,11 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws)
 
     constexpr script_verify_flags scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
 
+    std::vector<CTxOut> spent_outputs{m_view.GetUnspentOutputs(tx)};
+
     // Check input scripts and signatures.
     // This is done last to help prevent CPU exhaustion denial-of-service attacks.
-    if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
+    if (!CheckInputScripts(tx, state, std::move(spent_outputs), scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
         // Detect a failure due to a missing witness so that p2p code can handle rejection caching appropriately.
         if (!tx.HasWitness() && SpendsNonAnchorWitnessProg(tx, m_view)) {
             state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
@@ -2063,7 +2067,7 @@ ValidationCache::ValidationCache(const size_t script_execution_cache_bytes, cons
  * Non-static (and redeclared) in src/test/txvalidationcache_tests.cpp
  */
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
-                       const CCoinsViewCache& inputs, script_verify_flags flags, bool cacheSigStore,
+                       std::vector<CTxOut>&& spent_outputs, script_verify_flags flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
                        std::vector<CScriptCheck>* pvChecks)
@@ -2088,15 +2092,6 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     }
 
     if (!txdata.m_spent_outputs_ready) {
-        std::vector<CTxOut> spent_outputs;
-        spent_outputs.reserve(tx.vin.size());
-
-        for (const auto& txin : tx.vin) {
-            const COutPoint& prevout = txin.prevout;
-            const Coin& coin = inputs.AccessCoin(prevout);
-            assert(!coin.IsSpent());
-            spent_outputs.emplace_back(coin.out);
-        }
         txdata.Init(tx, std::move(spent_outputs));
     }
     assert(txdata.m_spent_outputs.size() == tx.vin.size());
@@ -2588,14 +2583,15 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
             bool tx_ok;
             TxValidationState tx_state;
+            std::vector<CTxOut> spent_outputs{view.GetUnspentOutputs(tx)};
             // If CheckInputScripts is called with a pointer to a checks vector, the resulting checks are appended to it. In that case
             // they need to be added to control which runs them asynchronously. Otherwise, CheckInputScripts runs the checks before returning.
             if (control) {
                 std::vector<CScriptCheck> vChecks;
-                tx_ok = CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, &vChecks);
+                tx_ok = CheckInputScripts(tx, tx_state, std::move(spent_outputs), flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, &vChecks);
                 if (tx_ok) control->Add(std::move(vChecks));
             } else {
-                tx_ok = CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache);
+                tx_ok = CheckInputScripts(tx, tx_state, std::move(spent_outputs), flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache);
             }
             if (!tx_ok) {
                 // Any transaction validation failure in ConnectBlock is a block consensus failure

From 9741689fd4f352a9c73e995ff6906d3756f1029b Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sat, 19 Apr 2025 22:55:59 +0200
Subject: [PATCH 6/9] validation: Add SpendBlock function

Move the BIP30 checks from ConnectBlock to a new SpendBlock method.
This is a move-only change, more content to SpendBlock is added in the
next commits. The goal is to move logic requiring access to the
CCoinsViewCache out of ConnectBlock and to the new SpendBlock method.
SpendBlock will in future handle all UTXO set interactions that
previously took place in ConnectBlock.

Callers of ConnectBlock now also need to call SpendBlock before. This is
enforced in a future commit by adding a CBlockUndo argument that needs
to be passed to both.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
---
 src/bench/connectblock.cpp               |   1 +
 src/test/coinstatsindex_tests.cpp        |   1 +
 src/test/validation_chainstate_tests.cpp |  36 +++
 src/validation.cpp                       | 283 ++++++++++++++---------
 src/validation.h                         |   2 +
 5 files changed, 208 insertions(+), 115 deletions(-)

diff --git a/src/bench/connectblock.cpp b/src/bench/connectblock.cpp
index 434bcdcbfe2c..d66c99c35623 100644
--- a/src/bench/connectblock.cpp
+++ b/src/bench/connectblock.cpp
@@ -100,6 +100,7 @@ void BenchmarkConnectBlock(benchmark::Bench& bench, std::vector<CKey>& keys, std
         auto* pindex{chainman->m_blockman.AddToBlockIndex(test_block, chainman->m_best_header)}; // Doing this here doesn't impact the benchmark
         CCoinsViewCache viewNew{&chainstate.CoinsTip()};
 
+        assert(chainstate.SpendBlock(test_block, pindex, viewNew, test_block_state));
         assert(chainstate.ConnectBlock(test_block, test_block_state, pindex, viewNew));
     });
 }
diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp
index f7f97c9b371a..dea72ec2736f 100644
--- a/src/test/coinstatsindex_tests.cpp
+++ b/src/test/coinstatsindex_tests.cpp
@@ -92,6 +92,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
             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());
+            BOOST_CHECK(chainstate.SpendBlock(block, new_block_index, view, state));
             BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view));
         }
         // Send block connected notification, then stop the index without
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index 38a04d8e46ce..f55be7b6b7ec 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -4,8 +4,10 @@
 //
 #include <chainparams.h>
 #include <consensus/amount.h>
+#include <consensus/merkle.h>
 #include <consensus/validation.h>
 #include <node/kernel_notifications.h>
+#include <pow.h>
 #include <random.h>
 #include <rpc/blockchain.h>
 #include <script/script.h>
@@ -166,4 +168,38 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
     BOOST_CHECK_EQUAL(curr_tip, get_notify_tip());
 }
 
+BOOST_FIXTURE_TEST_CASE(activate_best_chain_does_not_invalidate_mutated_spendblock_failure, TestChain100Setup)
+{
+    Chainstate& chainstate = Assert(m_node.chainman)->ActiveChainstate();
+
+    auto block = std::make_shared<CBlock>(CreateBlock({}, CScript{} << OP_TRUE, chainstate));
+    block->hashMerkleRoot = uint256{1};
+    BOOST_REQUIRE(block->hashMerkleRoot != BlockMerkleRoot(*block));
+    block->nNonce = 0;
+    while (!CheckProofOfWork(block->GetHash(), block->nBits, Params().GetConsensus())) ++block->nNonce;
+
+    CBlockIndex index_dummy{*block};
+    const uint256 block_hash{block->GetHash()};
+
+    {
+        LOCK(cs_main);
+        CBlockIndex* tip = Assert(chainstate.m_chain.Tip());
+        index_dummy.pprev = tip;
+        index_dummy.nHeight = tip->nHeight + 1;
+        index_dummy.phashBlock = &block_hash;
+        index_dummy.nChainWork = tip->nChainWork + GetBlockProof(index_dummy);
+        index_dummy.nTx = block->vtx.size();
+        index_dummy.m_chain_tx_count = tip->m_chain_tx_count + index_dummy.nTx;
+        index_dummy.nStatus = BlockStatus(BLOCK_VALID_TRANSACTIONS | BLOCK_HAVE_DATA);
+        chainstate.setBlockIndexCandidates.insert(&index_dummy);
+    }
+
+    BlockValidationState state;
+    BOOST_CHECK(!chainstate.ActivateBestChain(state, block));
+    BOOST_CHECK(state.IsError());
+    BOOST_CHECK_EQUAL(state.GetResult(), BlockValidationResult::BLOCK_MUTATED);
+    LOCK(cs_main);
+    BOOST_CHECK(!(index_dummy.nStatus & BLOCK_FAILED_VALID));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/validation.cpp b/src/validation.cpp
index d56caf7b0260..1f579ba0a40f 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2303,34 +2303,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     const auto time_start{SteadyClock::now()};
     const CChainParams& params{m_chainman.GetParams()};
 
-    // Check it again in case a previous version let a bad block in
-    // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or
-    // ContextualCheckBlockHeader() here. This means that if we add a new
-    // consensus rule that is enforced in one of those two functions, then we
-    // may have let in a block that violates the rule prior to updating the
-    // software, and we would NOT be enforcing the rule here. Fully solving
-    // upgrade from one software version to the next after a consensus rule
-    // change is potentially tricky and issue-specific (see NeedsRedownload()
-    // for one approach that was used for BIP 141 deployment).
-    // Also, currently the rule against blocks more than 2 hours in the future
-    // is enforced in ContextualCheckBlockHeader(); we wouldn't want to
-    // re-enforce that rule here (at least until we make it impossible for
-    // the clock to go backward).
-    if (!CheckBlock(block, state, params.GetConsensus(), !fJustCheck, !fJustCheck)) {
-        if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) {
-            // We don't write down blocks to disk if they may have been
-            // corrupted, so this should be impossible unless we're having hardware
-            // problems.
-            return FatalError(m_chainman.GetNotifications(), state, _("Corrupt block found indicating potential hardware failure."));
-        }
-        LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString());
-        return false;
-    }
-
-    // verify that the view's current state corresponds to the previous block
-    uint256 hashPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->GetBlockHash();
-    assert(hashPrevBlock == view.GetBestBlock());
-
     m_chainman.num_blocks_total++;
 
     // Special case for the genesis block, skipping connection of its transactions
@@ -2388,92 +2360,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
              Ticks<SecondsDouble>(m_chainman.time_check),
              Ticks<MillisecondsDouble>(m_chainman.time_check) / m_chainman.num_blocks_total);
 
-    // Do not allow blocks that contain transactions which 'overwrite' older transactions,
-    // unless those are already completely spent.
-    // If such overwrites are allowed, coinbases and transactions depending upon those
-    // can be duplicated to remove the ability to spend the first instance -- even after
-    // being sent to another address.
-    // See BIP30, CVE-2012-1909, and https://r6.ca/blog/20120206T005236Z.html for more information.
-    // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC.
-    // Now that the whole chain is irreversibly beyond that time it is applied to all blocks except the
-    // two in the chain that violate it. This prevents exploiting the issue against nodes during their
-    // initial block download.
-    bool fEnforceBIP30 = !IsBIP30Repeat(*pindex);
-
-    // Once BIP34 activated it was not possible to create new duplicate coinbases and thus other than starting
-    // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs.  But by the
-    // time BIP34 activated, in each of the existing pairs the duplicate coinbase had overwritten the first
-    // before the first had been spent.  Since those coinbases are sufficiently buried it's no longer possible to create further
-    // duplicate transactions descending from the known pairs either.
-    // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check.
-
-    // BIP34 requires that a block at height X (block X) has its coinbase
-    // scriptSig start with a CScriptNum of X (indicated height X).  The above
-    // logic of no longer requiring BIP30 once BIP34 activates is flawed in the
-    // case that there is a block X before the BIP34 height of 227,931 which has
-    // an indicated height Y where Y is greater than X.  The coinbase for block
-    // X would also be a valid coinbase for block Y, which could be a BIP30
-    // violation.  An exhaustive search of all mainnet coinbases before the
-    // BIP34 height which have an indicated height greater than the block height
-    // reveals many occurrences. The 3 lowest indicated heights found are
-    // 209,921, 490,897, and 1,983,702 and thus coinbases for blocks at these 3
-    // heights would be the first opportunity for BIP30 to be violated.
-
-    // The search reveals a great many blocks which have an indicated height
-    // greater than 1,983,702, so we simply remove the optimization to skip
-    // BIP30 checking for blocks at height 1,983,702 or higher.  Before we reach
-    // that block in another 25 years or so, we should take advantage of a
-    // future consensus change to do a new and improved version of BIP34 that
-    // will actually prevent ever creating any duplicate coinbases in the
-    // future.
-    static constexpr int BIP34_IMPLIES_BIP30_LIMIT = 1983702;
-
-    // There is no potential to create a duplicate coinbase at block 209,921
-    // because this is still before the BIP34 height and so explicit BIP30
-    // checking is still active.
-
-    // The final case is block 176,684 which has an indicated height of
-    // 490,897. Unfortunately, this issue was not discovered until about 2 weeks
-    // before block 490,897 so there was not much opportunity to address this
-    // case other than to carefully analyze it and determine it would not be a
-    // problem. Block 490,897 was, in fact, mined with a different coinbase than
-    // block 176,684, but it is important to note that even if it hadn't been or
-    // is remined on an alternate fork with a duplicate coinbase, we would still
-    // not run into a BIP30 violation.  This is because the coinbase for 176,684
-    // is spent in block 185,956 in transaction
-    // d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781.  This
-    // spending transaction can't be duplicated because it also spends coinbase
-    // 0328dd85c331237f18e781d692c92de57649529bd5edf1d01036daea32ffde29.  This
-    // coinbase has an indicated height of over 4.2 billion, and wouldn't be
-    // duplicatable until that height, and it's currently impossible to create a
-    // chain that long. Nevertheless we may wish to consider a future soft fork
-    // which retroactively prevents block 490,897 from creating a duplicate
-    // coinbase. The two historical BIP30 violations often provide a confusing
-    // edge case when manipulating the UTXO and it would be simpler not to have
-    // another edge case to deal with.
-
-    // testnet3 has no blocks before the BIP34 height with indicated heights
-    // post BIP34 before approximately height 486,000,000. After block
-    // 1,983,702 testnet3 starts doing unnecessary BIP30 checking again.
-    assert(pindex->pprev);
-    CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(params.GetConsensus().BIP34Height);
-    //Only continue to enforce if we're below BIP34 activation height or the block hash at that height doesn't correspond.
-    fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == params.GetConsensus().BIP34Hash));
-
-    // TODO: Remove BIP30 checking from block height 1,983,702 on, once we have a
-    // consensus change that ensures coinbases at those heights cannot
-    // duplicate earlier coinbases.
-    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))) {
-                    state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-BIP30",
-                                  "tried to overwrite transaction");
-                }
-            }
-        }
-    }
-
     // Enforce BIP68 (sequence locks)
     int nLockTimeFlags = 0;
     if (DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_CSV)) {
@@ -2887,6 +2773,153 @@ static void UpdateTipLog(
                    !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : "");
 }
 
+bool Chainstate::SpendBlock(
+    const CBlock& block,
+    const CBlockIndex* pindex,
+    CCoinsViewCache& view,
+    BlockValidationState& state,
+    bool fJustCheck)
+{
+    AssertLockHeld(cs_main);
+    assert(pindex);
+    assert(pindex->GetBlockHash() == block.GetHash());
+    auto block_hash{pindex->GetBlockHash()};
+
+    const CChainParams& params{m_chainman.GetParams()};
+
+    // Check it again in case a previous version let a bad block in
+    // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or
+    // ContextualCheckBlockHeader() here. This means that if we add a new
+    // consensus rule that is enforced in one of those two functions, then we
+    // may have let in a block that violates the rule prior to updating the
+    // software, and we would NOT be enforcing the rule here. Fully solving
+    // upgrade from one software version to the next after a consensus rule
+    // change is potentially tricky and issue-specific (see NeedsRedownload()
+    // for one approach that was used for BIP 141 deployment).
+    // Also, currently the rule against blocks more than 2 hours in the future
+    // is enforced in ContextualCheckBlockHeader(); we wouldn't want to
+    // re-enforce that rule here (at least until we make it impossible for
+    // the clock to go backward).
+    if (!CheckBlock(block, state, params.GetConsensus(), !fJustCheck, !fJustCheck)) {
+        if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) {
+            // We don't write down blocks to disk if they may have been
+            // corrupted, so this should be impossible unless we're having hardware
+            // problems.
+            return FatalError(m_chainman.GetNotifications(), state, _("Corrupt block found indicating potential hardware failure."));
+        }
+        LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString());
+        return false;
+    }
+
+    // Special case for the genesis block, skipping connection of its transactions
+    // (its coinbase is unspendable)
+    if (block_hash == params.GetConsensus().hashGenesisBlock) {
+        return true;
+    }
+
+    // verify that the view's current state corresponds to the previous block
+    uint256 hashPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->GetBlockHash();
+    assert(hashPrevBlock == view.GetBestBlock());
+
+    const auto time_start{SteadyClock::now()};
+
+    // Do not allow blocks that contain transactions which 'overwrite' older transactions,
+    // unless those are already completely spent.
+    // If such overwrites are allowed, coinbases and transactions depending upon those
+    // can be duplicated to remove the ability to spend the first instance -- even after
+    // being sent to another address.
+    // See BIP30, CVE-2012-1909, and https://r6.ca/blog/20120206T005236Z.html for more information.
+    // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC.
+    // Now that the whole chain is irreversibly beyond that time it is applied to all blocks except the
+    // two in the chain that violate it. This prevents exploiting the issue against nodes during their
+    // initial block download.
+    bool fEnforceBIP30 = !IsBIP30Repeat(*pindex);
+
+    // Once BIP34 activated it was not possible to create new duplicate coinbases and thus other than starting
+    // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs.  But by the
+    // time BIP34 activated, in each of the existing pairs the duplicate coinbase had overwritten the first
+    // before the first had been spent.  Since those coinbases are sufficiently buried it's no longer possible to create further
+    // duplicate transactions descending from the known pairs either.
+    // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check.
+
+    // BIP34 requires that a block at height X (block X) has its coinbase
+    // scriptSig start with a CScriptNum of X (indicated height X).  The above
+    // logic of no longer requiring BIP30 once BIP34 activates is flawed in the
+    // case that there is a block X before the BIP34 height of 227,931 which has
+    // an indicated height Y where Y is greater than X.  The coinbase for block
+    // X would also be a valid coinbase for block Y, which could be a BIP30
+    // violation.  An exhaustive search of all mainnet coinbases before the
+    // BIP34 height which have an indicated height greater than the block height
+    // reveals many occurrences. The 3 lowest indicated heights found are
+    // 209,921, 490,897, and 1,983,702 and thus coinbases for blocks at these 3
+    // heights would be the first opportunity for BIP30 to be violated.
+
+    // The search reveals a great many blocks which have an indicated height
+    // greater than 1,983,702, so we simply remove the optimization to skip
+    // BIP30 checking for blocks at height 1,983,702 or higher.  Before we reach
+    // that block in another 25 years or so, we should take advantage of a
+    // future consensus change to do a new and improved version of BIP34 that
+    // will actually prevent ever creating any duplicate coinbases in the
+    // future.
+    static constexpr int BIP34_IMPLIES_BIP30_LIMIT = 1983702;
+
+    // There is no potential to create a duplicate coinbase at block 209,921
+    // because this is still before the BIP34 height and so explicit BIP30
+    // checking is still active.
+
+    // The final case is block 176,684 which has an indicated height of
+    // 490,897. Unfortunately, this issue was not discovered until about 2 weeks
+    // before block 490,897 so there was not much opportunity to address this
+    // case other than to carefully analyze it and determine it would not be a
+    // problem. Block 490,897 was, in fact, mined with a different coinbase than
+    // block 176,684, but it is important to note that even if it hadn't been or
+    // is remined on an alternate fork with a duplicate coinbase, we would still
+    // not run into a BIP30 violation.  This is because the coinbase for 176,684
+    // is spent in block 185,956 in transaction
+    // d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781.  This
+    // spending transaction can't be duplicated because it also spends coinbase
+    // 0328dd85c331237f18e781d692c92de57649529bd5edf1d01036daea32ffde29.  This
+    // coinbase has an indicated height of over 4.2 billion, and wouldn't be
+    // duplicatable until that height, and it's currently impossible to create a
+    // chain that long. Nevertheless we may wish to consider a future soft fork
+    // which retroactively prevents block 490,897 from creating a duplicate
+    // coinbase. The two historical BIP30 violations often provide a confusing
+    // edge case when manipulating the UTXO and it would be simpler not to have
+    // another edge case to deal with.
+
+    // testnet3 has no blocks before the BIP34 height with indicated heights
+    // post BIP34 before approximately height 486,000,000. After block
+    // 1,983,702 testnet3 starts doing unnecessary BIP30 checking again.
+    assert(pindex->pprev);
+    CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(params.GetConsensus().BIP34Height);
+    // Only continue to enforce if we're below BIP34 activation height or the block hash at that height doesn't correspond.
+    fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == params.GetConsensus().BIP34Hash));
+
+    // TODO: Remove BIP30 checking from block height 1,983,702 on, once we have a
+    // consensus change that ensures coinbases at those heights cannot
+    // duplicate earlier coinbases.
+    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))) {
+                    return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-BIP30",
+                                  "tried to overwrite transaction");
+                }
+            }
+        }
+    }
+
+    const auto time_1{SteadyClock::now()};
+    m_chainman.time_connect += time_1 - time_start;
+    LogDebug(BCLog::BENCH, "      - Spend Block processed %u transactions: %.2fms (%.3fms/tx) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(),
+             Ticks<MillisecondsDouble>(time_1 - time_start),
+             block.vtx.size() == 0 ? 0 : Ticks<MillisecondsDouble>(time_1 - time_start) / block.vtx.size(),
+             Ticks<SecondsDouble>(m_chainman.time_connect),
+             m_chainman.num_blocks_total == 0 ? 0 : Ticks<MillisecondsDouble>(m_chainman.time_connect) / m_chainman.num_blocks_total);
+
+    return true;
+}
+
 void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
 {
     AssertLockHeld(::cs_main);
@@ -3042,6 +3075,16 @@ bool Chainstate::ConnectTip(
     {
         CCoinsViewCache& view{*m_coins_views->m_connect_block_view};
         const auto reset_guard{view.CreateResetGuard()};
+        if (!SpendBlock(*block_to_connect, pindexNew, view, state)) {
+            if (m_chainman.m_options.signals) {
+                m_chainman.m_options.signals->BlockChecked(block_to_connect, state);
+            }
+            if (state.IsInvalid()) {
+                InvalidBlockFound(pindexNew, state);
+            }
+            LogError("%s: SpendBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString());
+            return false;
+        }
         bool rv = ConnectBlock(*block_to_connect, state, pindexNew, view);
         if (m_chainman.m_options.signals) {
             m_chainman.m_options.signals->BlockChecked(block_to_connect, state);
@@ -4523,7 +4566,13 @@ BlockValidationState TestBlockValidity(
     CCoinsViewCache view_dummy(&chainstate.CoinsTip());
 
     // Set fJustCheck to true in order to update, and not clear, validation caches.
-    if(!chainstate.ConnectBlock(block, state, &index_dummy, view_dummy, /*fJustCheck=*/true)) {
+    if (!chainstate.SpendBlock(block, &index_dummy, view_dummy, state, /*fJustCheck=*/true)) {
+        if (state.IsValid()) NONFATAL_UNREACHABLE();
+        return state;
+    }
+
+    // Set fJustCheck to true in order to update, and not clear, validation caches.
+    if (!chainstate.ConnectBlock(block, state, &index_dummy, view_dummy, true)) {
         if (state.IsValid()) NONFATAL_UNREACHABLE();
         return state;
     }
@@ -4729,6 +4778,10 @@ VerifyDBResult CVerifyDB::VerifyDB(
                 LogError("Verification error: ReadBlock failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
             }
+            if (!chainstate.SpendBlock(block, pindex, coins, state)) {
+                LogError("SpendBlock failed %s\n", state.ToString());
+                return VerifyDBResult::CORRUPTED_BLOCK_DB;
+            }
             if (!chainstate.ConnectBlock(block, state, pindex, coins)) {
                 LogError("Verification error: found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
diff --git a/src/validation.h b/src/validation.h
index 4cb5dc631e95..39729b4a34f1 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -781,6 +781,8 @@ class Chainstate
     bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex,
                       CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
+    bool SpendBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view, BlockValidationState& state, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
     // Apply the effects of a block disconnection on the UTXO set.
     bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
 

From 4ce79019c2c6b4660ce2864f4ab07b55852acc4f Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sun, 20 Apr 2025 12:48:43 +0200
Subject: [PATCH 7/9] validation: Move SetBestBlock out of ConnectBlock

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.
---
 src/validation.cpp | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 1f579ba0a40f..2952fb556598 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2308,8 +2308,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     // Special case for the genesis block, skipping connection of its transactions
     // (its coinbase is unspendable)
     if (block_hash == params.GetConsensus().hashGenesisBlock) {
-        if (!fJustCheck)
-            view.SetBestBlock(pindex->GetBlockHash());
         return true;
     }
 
@@ -2544,9 +2542,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         m_blockman.m_dirty_blockindex.insert(pindex);
     }
 
-    // add this block to the view's block chain
-    view.SetBestBlock(pindex->GetBlockHash());
-
     const auto time_6{SteadyClock::now()};
     m_chainman.time_index += time_6 - time_5;
     LogDebug(BCLog::BENCH, "    - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n",
@@ -3095,6 +3090,8 @@ bool Chainstate::ConnectTip(
             LogError("%s: ConnectBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString());
             return false;
         }
+        // add this block to the view's block chain
+        view.SetBestBlock(pindexNew->GetBlockHash());
         time_3 = SteadyClock::now();
         m_chainman.time_connect_total += time_3 - time_2;
         assert(m_chainman.num_blocks_total > 0);
@@ -4786,6 +4783,7 @@ VerifyDBResult CVerifyDB::VerifyDB(
                 LogError("Verification error: found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
             }
+            coins.SetBestBlock(pindex->GetBlockHash());
             if (chainstate.m_chainman.m_interrupt) return VerifyDBResult::INTERRUPTED;
         }
     }

From 9701b46b721261961b5680b1ca21a6b23384e19d Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 18 Apr 2025 13:59:56 +0200
Subject: [PATCH 8/9] validation: Move coin existence and spend check to
 SpendBlock

Move the remaining UTXO-related operations from ConnectBlock to
SpendBlock. This includes moving the existence check, the UpdateCoins
call, and CBlockUndo generation.

ConnectBlock now takes a pre-populated CBlockUndo as an argument and no
longer accesses or manipulates the UTXO set.

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
---
 src/bench/connectblock.cpp               |   6 +-
 src/consensus/tx_verify.cpp              |  12 +--
 src/consensus/tx_verify.h                |   4 +-
 src/test/coinstatsindex_tests.cpp        |   6 +-
 src/test/fuzz/coins_view.cpp             |   4 +-
 src/test/transaction_tests.cpp           |   4 +-
 src/test/validation_chainstate_tests.cpp |  33 ++++++-
 src/txmempool.cpp                        |   2 +-
 src/validation.cpp                       | 112 ++++++++++++++---------
 src/validation.h                         |   7 +-
 test/functional/feature_block.py         |   2 +-
 11 files changed, 122 insertions(+), 70 deletions(-)

diff --git a/src/bench/connectblock.cpp b/src/bench/connectblock.cpp
index d66c99c35623..4108476fbdfd 100644
--- a/src/bench/connectblock.cpp
+++ b/src/bench/connectblock.cpp
@@ -9,6 +9,7 @@
 #include <script/interpreter.h>
 #include <sync.h>
 #include <test/util/setup_common.h>
+#include <undo.h>
 #include <validation.h>
 
 #include <cassert>
@@ -100,8 +101,9 @@ void BenchmarkConnectBlock(benchmark::Bench& bench, std::vector<CKey>& keys, std
         auto* pindex{chainman->m_blockman.AddToBlockIndex(test_block, chainman->m_best_header)}; // Doing this here doesn't impact the benchmark
         CCoinsViewCache viewNew{&chainstate.CoinsTip()};
 
-        assert(chainstate.SpendBlock(test_block, pindex, viewNew, test_block_state));
-        assert(chainstate.ConnectBlock(test_block, test_block_state, pindex, viewNew));
+        CBlockUndo blockundo;
+        assert(chainstate.SpendBlock(test_block, pindex, viewNew, test_block_state, blockundo));
+        assert(chainstate.ConnectBlock(test_block, blockundo, test_block_state, pindex));
     });
 }
 
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 6c17f755675a..de448c43724b 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -179,14 +179,8 @@ template int64_t GetTransactionSigOpCost<std::reference_wrapper<const Coin>>(
     const CTransaction& tx, const std::span<std::reference_wrapper<const Coin>> coins, script_verify_flags flags);
 
 template <Consensus::CoinRef T>
-bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<T> coins, int nSpendHeight, CAmount& txfee)
+bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const std::span<T> coins, int nSpendHeight, CAmount& txfee)
 {
-    // are the actual inputs available?
-    if (!inputs.HaveInputs(tx)) {
-        return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent",
-                         strprintf("%s: inputs missing/spent", __func__));
-    }
-
     CAmount nValueIn = 0;
     Assert(coins.size() == tx.vin.size());
     auto input_it = tx.vin.begin();
@@ -233,7 +227,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state,
 }
 
 template bool Consensus::CheckTxInputs<const Coin>(
-    const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<const Coin> coins, int nSpendHeight, CAmount& txfee);
+    const CTransaction& tx, TxValidationState& state, const std::span<const Coin> coins, int nSpendHeight, CAmount& txfee);
 
 template bool Consensus::CheckTxInputs<std::reference_wrapper<const Coin>>(
-    const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, const std::span<std::reference_wrapper<const Coin>> coins, int nSpendHeight, CAmount& txfee);
+    const CTransaction& tx, TxValidationState& state, const std::span<std::reference_wrapper<const Coin>> coins, int nSpendHeight, CAmount& txfee);
diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h
index 746c3181af57..2cdd20c4996a 100644
--- a/src/consensus/tx_verify.h
+++ b/src/consensus/tx_verify.h
@@ -27,14 +27,14 @@ template <typename T>
 concept CoinRef = std::convertible_to<T, const Coin&>;
 
 /**
- * Check whether all inputs of this transaction are valid (no double spends and amounts)
+ * Check whether all inputs of this transaction are valid (amounts and maturity)
  * This does not modify the UTXO set. This does not check scripts and sigs.
  * @param[in] coins span of Coins containing previous transaction outputs in the order we're spending them
  * @param[out] txfee Set to the transaction fee if successful.
  * Preconditions: tx.IsCoinBase() is false.
  */
 template <Consensus::CoinRef T>
-[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, std::span<T> coins, int nSpendHeight, CAmount& txfee);
+[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, std::span<T> coins, int nSpendHeight, CAmount& txfee);
 } // namespace Consensus
 
 /** Auxiliary functions for transaction validation (ideally should not be exposed) */
diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp
index dea72ec2736f..e840303ab248 100644
--- a/src/test/coinstatsindex_tests.cpp
+++ b/src/test/coinstatsindex_tests.cpp
@@ -9,6 +9,7 @@
 #include <kernel/types.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
+#include <undo.h>
 #include <util/byte_units.h>
 #include <validation.h>
 
@@ -92,8 +93,9 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
             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());
-            BOOST_CHECK(chainstate.SpendBlock(block, new_block_index, view, state));
-            BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view));
+            CBlockUndo blockundo;
+            BOOST_CHECK(chainstate.SpendBlock(block, new_block_index, view, state, blockundo));
+            BOOST_CHECK(chainstate.ConnectBlock(block, blockundo, state, new_block_index));
         }
         // Send block connected notification, then stop the index without
         // sending a chainstate flushed notification. Prior to #24138, this
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 82561917989c..59c915ccb464 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -290,8 +290,8 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
                 }
                 // are the actual inputs available?
                 if (!coins_view_cache.HaveInputs(transaction)) return;
-                auto check_tx_inputs_res = coins_view_cache.AccessCoins(transaction, [&transaction, &state, &coins_view_cache, &fuzzed_data_provider, &tx_fee_out](auto&& coins) {
-                    return Consensus::CheckTxInputs(transaction, state, coins_view_cache, std::span{coins}, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out);
+                auto check_tx_inputs_res = coins_view_cache.AccessCoins(transaction, [&transaction, &state, &fuzzed_data_provider, &tx_fee_out](auto&& coins) {
+                    return Consensus::CheckTxInputs(transaction, state, coins, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out);
                 });
                 if (check_tx_inputs_res) {
                     assert(MoneyRange(tx_fee_out));
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 05961073b2a1..dcc2f3e2b853 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -1142,8 +1142,8 @@ BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test)
 
         TxValidationState state;
         CAmount txfee{0};
-        inputs.AccessCoins(CTransaction(mtx), [&mtx, &state, &inputs, spend_height, &txfee](auto&& coins) {
-            BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction{mtx}, state, inputs, coins, spend_height, txfee));
+        inputs.AccessCoins(CTransaction(mtx), [&mtx, &state, spend_height, &txfee](auto&& coins) {
+            BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction{mtx}, state, coins, spend_height, txfee));
         });
         BOOST_CHECK(state.IsInvalid());
         BOOST_CHECK_EQUAL(state.GetResult(), expected_result);
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index f55be7b6b7ec..f7510bba8097 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -13,19 +13,20 @@
 #include <script/script.h>
 #include <sync.h>
 #include <test/util/chainstate.h>
-#include <test/util/common.h>
 #include <test/util/coins.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
+#include <undo.h>
 #include <util/byte_units.h>
 #include <util/check.h>
 #include <validation.h>
 
-#include <vector>
-
 #include <boost/test/unit_test.hpp>
 
+#include <vector>
+
 BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, ChainTestingSetup)
 
 //! Test resizing coins-related Chainstate caches during runtime.
@@ -202,4 +203,30 @@ BOOST_FIXTURE_TEST_CASE(activate_best_chain_does_not_invalidate_mutated_spendblo
     BOOST_CHECK(!(index_dummy.nStatus & BLOCK_FAILED_VALID));
 }
 
+BOOST_FIXTURE_TEST_CASE(spendblock_rejects_empty_block, TestChain100Setup)
+{
+    Chainstate& chainstate = Assert(m_node.chainman)->ActiveChainstate();
+
+    LOCK(cs_main);
+    CBlockIndex* tip = Assert(chainstate.m_chain.Tip());
+
+    CBlock block;
+    // CheckBlock() rejects empty blocks, ensure it gets run in SpendBlock
+    block.hashPrevBlock = tip->GetBlockHash();
+
+    CBlockIndex index_dummy{block};
+    const uint256 block_hash{block.GetHash()};
+    index_dummy.pprev = tip;
+    index_dummy.nHeight = tip->nHeight + 1;
+    index_dummy.phashBlock = &block_hash;
+
+    CCoinsViewCache view_dummy(&chainstate.CoinsTip());
+    BlockValidationState state;
+    CBlockUndo blockundo;
+
+    BOOST_CHECK(!chainstate.SpendBlock(block, &index_dummy, view_dummy, state, blockundo));
+    BOOST_CHECK(state.IsInvalid());
+    BOOST_CHECK_EQUAL(state.GetRejectReason(), "high-hash");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 4cbefa8b8762..09124b7d925a 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -535,7 +535,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
         CAmount txfee = 0;
         assert(!tx.IsCoinBase());
         mempoolDuplicate.AccessCoins(tx, [&](auto&& coins) {
-            assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, std::span{coins}, spendheight, txfee));
+            assert(Consensus::CheckTxInputs(tx, dummy_state, std::span{coins}, spendheight, txfee));
         });
         for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout);
         AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max());
diff --git a/src/validation.cpp b/src/validation.cpp
index 2952fb556598..d8b88e7875c6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -887,12 +887,18 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final");
     }
 
+    // are the actual inputs available?
+    if (!m_view.HaveInputs(tx)) {
+        return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent",
+                             strprintf("%s: inputs missing/spent", __func__));
+    }
+
     int64_t nSigOpsCost = 0;
     bool fSpendsCoinbase = false;
     // Wrap access to required coins in their own scope to avoid coin lifetime extension issues
     auto res = m_view.AccessCoins(tx, [&, this](auto&& coins) {
         // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
-        if (!Consensus::CheckTxInputs(tx, state, m_view, std::span{coins}, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) {
+        if (!Consensus::CheckTxInputs(tx, state, std::span{coins}, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) {
             return false; // state filled in by CheckTxInputs
         }
 
@@ -2287,22 +2293,19 @@ script_verify_flags GetBlockScriptFlags(const CBlockIndex& block_index, const Ch
     return flags;
 }
 
-
-/** Apply the effects of this block (with given index) on the UTXO set represented by coins.
- *  Validity checks that depend on the UTXO set are also done; ConnectBlock()
- *  can fail if those validity checks fail (among other reasons). */
-bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex,
-                               CCoinsViewCache& view, bool fJustCheck)
+bool Chainstate::ConnectBlock(const CBlock& block, const CBlockUndo& blockundo, BlockValidationState& state,
+                              CBlockIndex* pindex, bool fJustCheck)
 {
     AssertLockHeld(cs_main);
     assert(pindex);
-
-    uint256 block_hash{block.GetHash()};
-    assert(*pindex->phashBlock == block_hash);
+    assert(pindex->GetBlockHash() == block.GetHash());
+    auto block_hash{pindex->GetBlockHash()};
 
     const auto time_start{SteadyClock::now()};
     const CChainParams& params{m_chainman.GetParams()};
 
+    assert(block.vtx.size() == blockundo.vtxundo.size() + 1); // blockundo does not contain a CTxUndo for the coinbase
+
     m_chainman.num_blocks_total++;
 
     // Special case for the genesis block, skipping connection of its transactions
@@ -2387,8 +2390,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         m_last_script_check_reason_logged = script_check_reason;
     }
 
-    CBlockUndo blockundo;
-
     // Precomputed transaction data pointers must not be invalidated
     // until after `control` has run the script checks (potentially
     // in multiple threads). Preallocate the vector size so a new allocation
@@ -2403,7 +2404,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     CAmount nFees = 0;
     int nInputs = 0;
     int64_t nSigOpsCost = 0;
-    blockundo.vtxundo.reserve(block.vtx.size() - 1);
     for (unsigned int i = 0; i < block.vtx.size(); i++)
     {
         if (!state.IsValid()) break;
@@ -2412,14 +2412,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
 
         if (!tx.IsCoinBase())
         {
+            auto spent_coins = std::span<const Coin>{blockundo.vtxundo[i - 1].vprevout};
             CAmount txfee = 0;
             TxValidationState tx_state;
 
-            auto check_tx_inputs_res = view.AccessCoins(tx, [&tx, &tx_state, &view, pindex, &txfee](auto&& coins) {
-                return Consensus::CheckTxInputs(tx, tx_state, view, coins, pindex->nHeight, txfee);
-            });
-
-            if (!check_tx_inputs_res) {
+            if (!Consensus::CheckTxInputs(tx, tx_state, spent_coins, pindex->nHeight, txfee)) {
                 // Any transaction validation failure in ConnectBlock is a block consensus failure
                 state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,
                               tx_state.GetRejectReason(),
@@ -2438,7 +2435,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             // be in ConnectBlock because they require the UTXO set
             prevheights.resize(tx.vin.size());
             for (size_t j = 0; j < tx.vin.size(); j++) {
-                prevheights[j] = view.AccessCoin(tx.vin[j].prevout).nHeight;
+                prevheights[j] = spent_coins[j].nHeight;
             }
 
             if (!SequenceLocks(tx, nLockTimeFlags, prevheights, *pindex)) {
@@ -2452,14 +2449,14 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         // * legacy (always)
         // * p2sh (when P2SH enabled in flags and excludes coinbase)
         // * witness (when witness enabled in flags and excludes coinbase)
-        {
-            nSigOpsCost += view.AccessCoins(tx, [&tx, &flags](auto&& coins) {
-                return GetTransactionSigOpCost(tx, coins, flags);
-            });
-            if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) {
-                state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops", "too many sigops");
-                break;
-            }
+        if (!tx.IsCoinBase()) {
+            nSigOpsCost += GetTransactionSigOpCost(tx, std::span<const Coin>{blockundo.vtxundo[i - 1].vprevout}, flags);
+        } else {
+            nSigOpsCost += GetTransactionSigOpCost(tx, std::span<const Coin>{}, flags);
+        }
+        if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) {
+            state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops", "too many sigops");
+            break;
         }
 
         if (!tx.IsCoinBase() && fScriptChecks)
@@ -2467,7 +2464,13 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
             bool tx_ok;
             TxValidationState tx_state;
-            std::vector<CTxOut> spent_outputs{view.GetUnspentOutputs(tx)};
+            const std::vector<Coin>& spent_coins = blockundo.vtxundo[i - 1].vprevout;
+            std::vector<CTxOut> spent_outputs;
+            spent_outputs.reserve(tx.vin.size());
+            for (const Coin& coin : spent_coins) {
+                spent_outputs.push_back(coin.out);
+            }
+
             // If CheckInputScripts is called with a pointer to a checks vector, the resulting checks are appended to it. In that case
             // they need to be added to control which runs them asynchronously. Otherwise, CheckInputScripts runs the checks before returning.
             if (control) {
@@ -2484,12 +2487,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
                 break;
             }
         }
-
-        CTxUndo undoDummy;
-        if (i > 0) {
-            blockundo.vtxundo.emplace_back();
-        }
-        UpdateCoins(tx, view, i == 0 ? undoDummy : blockundo.vtxundo.back(), pindex->nHeight);
     }
     const auto time_3{SteadyClock::now()};
     m_chainman.time_connect += time_3 - time_2;
@@ -2773,6 +2770,7 @@ bool Chainstate::SpendBlock(
     const CBlockIndex* pindex,
     CCoinsViewCache& view,
     BlockValidationState& state,
+    CBlockUndo& blockundo,
     bool fJustCheck)
 {
     AssertLockHeld(cs_main);
@@ -2904,14 +2902,39 @@ bool Chainstate::SpendBlock(
         }
     }
 
+    int nInputs = 0;
+
+    blockundo.vtxundo.reserve(block.vtx.size() - 1);
+    for (const CTransactionRef& tx : block.vtx) {
+        nInputs += tx->vin.size();
+        // are the actual inputs available?
+        if (!view.HaveInputs(*tx)) {
+            state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-inputs-missingorspent",
+                          strprintf("%s: inputs missing/spent in transaction %s", __func__, tx->GetHash().ToString()));
+            break;
+        }
+        if (!tx->IsCoinBase()) {
+            blockundo.vtxundo.emplace_back();
+        }
+        CTxUndo dummy;
+        UpdateCoins(*tx, view, tx->IsCoinBase() ? dummy : blockundo.vtxundo.back(), pindex->nHeight);
+    }
+
     const auto time_1{SteadyClock::now()};
     m_chainman.time_connect += time_1 - time_start;
-    LogDebug(BCLog::BENCH, "      - Spend Block processed %u transactions: %.2fms (%.3fms/tx) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(),
+    LogDebug(BCLog::BENCH, "      - Spend Block processed %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(),
              Ticks<MillisecondsDouble>(time_1 - time_start),
              block.vtx.size() == 0 ? 0 : Ticks<MillisecondsDouble>(time_1 - time_start) / block.vtx.size(),
+             nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_1 - time_start) / (nInputs - 1),
              Ticks<SecondsDouble>(m_chainman.time_connect),
              m_chainman.num_blocks_total == 0 ? 0 : Ticks<MillisecondsDouble>(m_chainman.time_connect) / m_chainman.num_blocks_total);
 
+    if (!state.IsValid()) {
+        LogInfo("Block validation error: %s", state.ToString());
+        return false;
+    }
+
+    assert(blockundo.vtxundo.size() == block.vtx.size() - 1);
     return true;
 }
 
@@ -3070,7 +3093,8 @@ bool Chainstate::ConnectTip(
     {
         CCoinsViewCache& view{*m_coins_views->m_connect_block_view};
         const auto reset_guard{view.CreateResetGuard()};
-        if (!SpendBlock(*block_to_connect, pindexNew, view, state)) {
+        CBlockUndo blockundo;
+        if (!SpendBlock(*block_to_connect, pindexNew, view, state, blockundo)) {
             if (m_chainman.m_options.signals) {
                 m_chainman.m_options.signals->BlockChecked(block_to_connect, state);
             }
@@ -3080,7 +3104,7 @@ bool Chainstate::ConnectTip(
             LogError("%s: SpendBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString());
             return false;
         }
-        bool rv = ConnectBlock(*block_to_connect, state, pindexNew, view);
+        bool rv = ConnectBlock(*block_to_connect, blockundo, state, pindexNew);
         if (m_chainman.m_options.signals) {
             m_chainman.m_options.signals->BlockChecked(block_to_connect, state);
         }
@@ -4562,14 +4586,14 @@ BlockValidationState TestBlockValidity(
     index_dummy.phashBlock = &block_hash;
     CCoinsViewCache view_dummy(&chainstate.CoinsTip());
 
-    // Set fJustCheck to true in order to update, and not clear, validation caches.
-    if (!chainstate.SpendBlock(block, &index_dummy, view_dummy, state, /*fJustCheck=*/true)) {
+    CBlockUndo blockundo;
+    if (!chainstate.SpendBlock(block, &index_dummy, view_dummy, state, blockundo, /*fJustCheck=*/true)) {
         if (state.IsValid()) NONFATAL_UNREACHABLE();
         return state;
     }
 
     // Set fJustCheck to true in order to update, and not clear, validation caches.
-    if (!chainstate.ConnectBlock(block, state, &index_dummy, view_dummy, true)) {
+    if (!chainstate.ConnectBlock(block, blockundo, state, &index_dummy, true)) {
         if (state.IsValid()) NONFATAL_UNREACHABLE();
         return state;
     }
@@ -4775,12 +4799,14 @@ VerifyDBResult CVerifyDB::VerifyDB(
                 LogError("Verification error: ReadBlock failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
             }
-            if (!chainstate.SpendBlock(block, pindex, coins, state)) {
+            CBlockUndo blockundo;
+            if (!chainstate.SpendBlock(block, pindex, coins, state, blockundo)) {
                 LogError("SpendBlock failed %s\n", state.ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
             }
-            if (!chainstate.ConnectBlock(block, state, pindex, coins)) {
-                LogError("Verification error: found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString());
+
+            if (!chainstate.ConnectBlock(block, blockundo, state, pindex)) {
+                LogError("Verification error: found unconnectable block at %d, hash=%s (%s)\n", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString());
                 return VerifyDBResult::CORRUPTED_BLOCK_DB;
             }
             coins.SetBestBlock(pindex->GetBlockHash());
diff --git a/src/validation.h b/src/validation.h
index 39729b4a34f1..7a5fd89389e2 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -778,10 +778,11 @@ class Chainstate
     // Block (dis)connection on a given view:
     DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-    bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex,
-                      CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    bool ConnectBlock(const CBlock& block, const CBlockUndo& blockundo, BlockValidationState& state,
+                      CBlockIndex* pindex, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
-    bool SpendBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view, BlockValidationState& state, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    bool SpendBlock(const CBlock& block, const CBlockIndex* pindex,
+                    CCoinsViewCache& view, BlockValidationState& state, CBlockUndo& blockundo, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
     // Apply the effects of a block disconnection on the UTXO set.
     bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 5600eeec8642..df454cd7909f 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -154,7 +154,7 @@ def run_test(self):
 
         # MAX_SCRIPT_SIZE + 1 wasn't added to the utxo set
         tx.vin[0] = min_size_unspendable_output
-        assert_raises_rpc_error(-25, f'TestBlockValidity failed: bad-txns-inputs-missingorspent, CheckTxInputs: inputs missing/spent in transaction {tx.txid_hex}', self.generateblock, self.nodes[0],  output="raw(55)", transactions=[tx.serialize().hex()])
+        assert_raises_rpc_error(-25, f'TestBlockValidity failed: bad-txns-inputs-missingorspent, SpendBlock: inputs missing/spent in transaction {tx.txid_hex}', self.generateblock, self.nodes[0],  output="raw(55)", transactions=[tx.serialize().hex()])
 
         # collect spendable outputs now to avoid cluttering the code later on
         out = []

From edc952b200285a197a2c0dbc4e57239514447155 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Mon, 21 Apr 2025 21:36:56 +0200
Subject: [PATCH 9/9] doc: Add docstrings for ConnectBlock and SpendBlock

---
 src/validation.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/validation.h b/src/validation.h
index 7a5fd89389e2..3e69c8fe737a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -778,9 +778,33 @@ class Chainstate
     // Block (dis)connection on a given view:
     DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+    /**
+     * Validates the block against the coins in the blockundo data, writes the
+     * undo data to disk, and raises the block validity in the block index.
+     *
+     * @param[in] block       The block to be validated
+     * @param[in] blockundo   Has to contain all coins spent by block. Written to disk on successful validation
+     * @param[out] state      This is set to an Error state if any error occurred while validating block
+     * @param[in, out] pindex Points to the block map entry associated with block. On successful validation, its nStatus block validity is raised.
+     * @param[in] fJustCheck  If set, no data is written to disk and pindex's validity is not raised
+     */
     bool ConnectBlock(const CBlock& block, const CBlockUndo& blockundo, BlockValidationState& state,
                       CBlockIndex* pindex, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
+    /**
+     * Spends the coins associated with a block. First checks that the block
+     * introduces no duplicate transactions (BIP30), then for each transaction in
+     * the block checks that the outputs it is spending exist, spends them, and
+     * populates the CBlockUndo data structure.
+     *
+     * @param[in] block      The block to be spent
+     * @param[in] pindex     Points to the block map entry associated with block
+     * @param[in, out] view  Its coins are spent and used to populate CBlockUndo during its execution
+     * @param[out] state     This may be set to an Error state if any error occurred processing them
+     * @param[out] blockundo Coins consumed by the block are added to it.
+     * @param[in] fJustCheck If set, skip some block level checks.
+     */
     bool SpendBlock(const CBlock& block, const CBlockIndex* pindex,
                     CCoinsViewCache& view, BlockValidationState& state, CBlockUndo& blockundo, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);