From a7e41326234d3a381fdde0924af74c6561b10798 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 18 Apr 2025 11:09:34 +0200 Subject: [PATCH] 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.cpp | 10 ++++++++++ src/coins.h | 9 +++++++++ src/consensus/tx_verify.cpp | 31 +++++++++++++++++++++++++------ src/consensus/tx_verify.h | 6 ++++-- src/test/fuzz/coins_view.cpp | 3 ++- src/test/script_p2sh_tests.cpp | 9 ++++++--- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 24a102b0bc1..6ee253ad40d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -160,6 +160,16 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { } } +std::vector> CCoinsViewCache::AccessCoins(const CTransaction& tx) const +{ + std::vector> coins; + coins.reserve(tx.vin.size()); + for (const CTxIn& input : tx.vin) { + coins.emplace_back(AccessCoin(input.prevout)); + } + return coins; +} + bool CCoinsViewCache::HaveCoin(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = FetchCoin(outpoint); return (it != cacheCoins.end() && !it->second.coin.IsSpent()); diff --git a/src/coins.h b/src/coins.h index 61fb4af6420..a1a2848f740 100644 --- a/src/coins.h +++ b/src/coins.h @@ -415,6 +415,15 @@ public: */ const Coin& AccessCoin(const COutPoint &output) const; + /** + * Return a vector of references to Coins in the cache, 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> AccessCoins(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/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 95466b759cb..90e99ff0ff4 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -14,6 +14,16 @@ #include #include +template +static constexpr const Coin& GetCoin(const T& item) +{ + if constexpr (std::is_same_v>) { + return item.get(); + } else { + return item; + } +} + bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) { if (tx.nLockTime == 0) @@ -123,23 +133,31 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx) return nSigOps; } -unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& inputs) +template +unsigned int GetP2SHSigOpCount(const CTransaction& tx, const 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{GetCoin(*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 CTransaction& tx, const std::span); + +template unsigned int GetP2SHSigOpCount>>( + const CTransaction& tx, const std::span>); + int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, uint32_t flags) { int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR; @@ -148,7 +166,8 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i return nSigOps; if (flags & SCRIPT_VERIFY_P2SH) { - nSigOps += GetP2SHSigOpCount(tx, inputs) * WITNESS_SCALE_FACTOR; + auto coins{inputs.AccessCoins(tx)}; + nSigOps += GetP2SHSigOpCount(tx, std::span{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 d2cf792cf3f..c226394bcb8 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_CONSENSUS_TX_VERIFY_H #define BITCOIN_CONSENSUS_TX_VERIFY_H +#include #include #include @@ -39,11 +40,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 Sorted span of Coins containing previous transaction outputs we're spending * @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 +unsigned int GetP2SHSigOpCount(const CTransaction& tx, const 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 9c6aa6e7a1e..f36704f64a3 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -266,7 +266,8 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) // consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed. return; } - (void)GetP2SHSigOpCount(transaction, coins_view_cache); + auto coins{coins_view_cache.AccessCoins(transaction)}; + (void)GetP2SHSigOpCount(transaction, std::span{coins}); }, [&] { const CTransaction transaction{random_mutable_transaction}; diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index bb408b7b0f5..e831bad64f9 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -362,7 +362,8 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins)); // 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 coinsTxTo{coins.AccessCoins(CTransaction(txTo))}; + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), std::span{coinsTxTo}), 22U); CMutableTransaction txToNonStd1; txToNonStd1.vout.resize(1); @@ -374,7 +375,8 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txToNonStd1.vin[0].scriptSig << std::vector(sixteenSigops.begin(), sixteenSigops.end()); BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins)); - BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U); + auto coinsTxToNonStd1{coins.AccessCoins(CTransaction(txToNonStd1))}; + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), std::span{coinsTxToNonStd1}), 16U); CMutableTransaction txToNonStd2; txToNonStd2.vout.resize(1); @@ -386,7 +388,8 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txToNonStd2.vin[0].scriptSig << std::vector(twentySigops.begin(), twentySigops.end()); BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins)); - BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U); + auto coinsTxToNonStd2{coins.AccessCoins(CTransaction(txToNonStd2))}; + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), std::span{coinsTxToNonStd2}), 20U); } BOOST_AUTO_TEST_SUITE_END()