Merge commit '00a3917ab1d4d59e9d01f951593e37f66874b076' into validation_helpers

Author: sedited

libbitcoinkernel-sys/bitcoin/CMakeLists.txt

@@ -488,9 +488,8 @@ try_append_cxx_flags("-fno-extended-identifiers" TARGET core_interface SKIP_LINK
 # which can cause issues with coverage builds, particularly when using
 # Clang in the OSS-Fuzz environment due to its use of other options
 # and a third party script, or with GCC.
-try_append_cxx_flags("-fdebug-prefix-map=A=B" TARGET core_interface SKIP_LINK
-  IF_CHECK_PASSED "-fdebug-prefix-map=${PROJECT_SOURCE_DIR}/src=."
-)
+# Set `-fmacro-prefix-map`, so that source file names are expanded without the
+# src prefix.
 try_append_cxx_flags("-fmacro-prefix-map=A=B" TARGET core_interface SKIP_LINK
   IF_CHECK_PASSED "-fmacro-prefix-map=${PROJECT_SOURCE_DIR}/src=."
 )

libbitcoinkernel-sys/bitcoin/CONTRIBUTING.md

@@ -78,6 +78,13 @@ The codebase is maintained using the "contributor workflow" where everyone
 without exception contributes patch proposals using "pull requests" (PRs). This
 facilitates social contribution, easy testing and peer review.
 
+Pull request authors must fully and confidently understand their own changes
+and must have tested them. Contributors should mention which tests cover their
+changes, or include the manual steps they used to confirm the change.
+Contributors are expected to be prepared to clearly motivate and explain their
+changes. If there is doubt, the pull request may be closed.
+Please refer to the [peer review](#peer-review) section below for more details.
+
 To contribute a patch, the workflow is as follows:
 
   1. Fork repository ([only for the first time](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo))
@@ -338,6 +345,11 @@ reviewers that the changes warrant the review effort, and if reviewers are
 "Concept NACK'ing" the PR, the author may need to present arguments and/or do
 research backing their suggested changes.
 
+Moreover, if there is reasonable doubt that the pull request author does not
+fully understand the changes they are submitting themselves, or if it becomes
+clear that they have not tested the changes on a basic level themselves, the
+pull request may be closed immediately.
+
 #### Conceptual Review
 
 A review can be a conceptual review, where the reviewer leaves a comment

libbitcoinkernel-sys/bitcoin/contrib/guix/libexec/build.sh

@@ -211,6 +211,7 @@ CONFIGFLAGS="-DREDUCE_EXPORTS=ON -DBUILD_BENCH=OFF -DBUILD_GUI_TESTS=OFF -DBUILD
 # CFLAGS
 HOST_CFLAGS="-O2 -g"
 HOST_CFLAGS+=$(find /gnu/store -maxdepth 1 -mindepth 1 -type d -exec echo -n " -ffile-prefix-map={}=/usr" \;)
+HOST_CFLAGS+=" -fdebug-prefix-map=${DISTSRC}/src=."
 case "$HOST" in
     *mingw*)  HOST_CFLAGS+=" -fno-ident" ;;
     *darwin*) unset HOST_CFLAGS ;;

libbitcoinkernel-sys/bitcoin/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,7 +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.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));
     });
 }
 

libbitcoinkernel-sys/bitcoin/src/bench/mempool_stress.cpp

@@ -106,7 +106,7 @@ static void MemPoolAddTransactions(benchmark::Bench& bench)
     std::vector<CTransactionRef> transactions;
     // Create 1000 clusters of 100 transactions each
     for (int i=0; i<100; i++) {
-        auto new_txs = CreateCoinCluster(det_rand, childTxs, /*min_ancestors*/ 1);
+        auto new_txs = CreateCoinCluster(det_rand, childTxs, /*min_ancestors=*/ 1);
         transactions.insert(transactions.end(), new_txs.begin(), new_txs.end());
     }
 
@@ -156,7 +156,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
     // in the same state at the end of the function, so we benchmark both
     // mining a block and reorging the block's contents back into the mempool.
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
-        pool.removeForBlock(tx_remove_for_block, /*nBlockHeight*/100);
+        pool.removeForBlock(tx_remove_for_block, /*nBlockHeight=*/100);
         for (auto& tx: tx_remove_for_block) {
             AddTx(tx, pool, det_rand);
         }

libbitcoinkernel-sys/bitcoin/src/coins.cpp

@@ -167,6 +167,29 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const {
     }
 }
 
+std::vector<std::reference_wrapper<const Coin>> CCoinsViewCache::AccessCoins(const CTransaction& tx) 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 coins;
+}
+
+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);
     return (it != cacheCoins.end() && !it->second.coin.IsSpent());

libbitcoinkernel-sys/bitcoin/src/coins.h

@@ -413,6 +413,23 @@ class CCoinsViewCache : public CCoinsViewBacked
      */
     const Coin& AccessCoin(const COutPoint &output) const;
 
+    /**
+     * Return a vector of references to Coins in the cache in order they get
+     * consumed by the tx's inputs, or coinEmpty if the Coin is not
+     * found.
+     *
+     * Generally, this should only be held for a short scope. The coins should
+     * generally not be held through any other calls to this cache.
+     */
+    std::vector<std::reference_wrapper<const Coin>> AccessCoins(const CTransaction& tx) const;
+
+    /**
+     * 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.

libbitcoinkernel-sys/bitcoin/src/common/system.cpp

@@ -37,9 +37,6 @@
 
 using util::ReplaceAll;
 
-// Application startup time (used for uptime calculation)
-const int64_t nStartupTime = GetTime();
-
 #ifndef WIN32
 std::string ShellEscape(const std::string& arg)
 {
@@ -130,8 +127,8 @@ std::optional<size_t> GetTotalRAM()
     return std::nullopt;
 }
 
-// Obtain the application startup time (used for uptime calculation)
-int64_t GetStartupTime()
+SteadyClock::duration GetUptime()
 {
-    return nStartupTime;
+    static const auto g_startup_time{SteadyClock::now()};
+    return SteadyClock::now() - g_startup_time;
 }

libbitcoinkernel-sys/bitcoin/src/common/system.h

@@ -7,13 +7,15 @@
 #define BITCOIN_COMMON_SYSTEM_H
 
 #include <bitcoin-build-config.h> // IWYU pragma: keep
+#include <util/time.h>
 
+#include <chrono>
 #include <cstdint>
 #include <optional>
 #include <string>
 
-// Application startup time (used for uptime calculation)
-int64_t GetStartupTime();
+/// Monotonic uptime (not affected by system time changes).
+SteadyClock::duration GetUptime();
 
 void SetupEnvironment();
 [[nodiscard]] bool SetupNetworking();

libbitcoinkernel-sys/bitcoin/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,62 +125,73 @@ 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;
 }
 
-int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, script_verify_flags flags)
+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>>);
+
+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;
 
     if (tx.IsCoinBase())
         return nSigOps;
 
     if (flags & SCRIPT_VERIFY_P2SH) {
-        nSigOps += GetP2SHSigOpCount(tx, inputs) * 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);
 
-bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, 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__));
-    }
+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 std::span<T> coins, int nSpendHeight, CAmount& txfee)
+{
     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
@@ -203,3 +216,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 std::span<const Coin> coins, int nSpendHeight, CAmount& txfee);
+
+template bool Consensus::CheckTxInputs<std::reference_wrapper<const Coin>>(
+    const CTransaction& tx, TxValidationState& state, const std::span<std::reference_wrapper<const Coin>> coins, int nSpendHeight, CAmount& txfee);