diff --git a/src/coins.cpp b/src/coins.cpp index 6ee253ad40d..6f7982c258c 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -170,6 +170,19 @@ std::vector> CCoinsViewCache::AccessCoins(con return coins; } +std::vector CCoinsViewCache::GetUnspentOutputs(const CTransaction& tx) const +{ + std::vector 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()); diff --git a/src/coins.h b/src/coins.h index a1a2848f740..907ab4cdf2d 100644 --- a/src/coins.h +++ b/src/coins.h @@ -424,6 +424,13 @@ public: */ std::vector> 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 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 af36a956931..6268b0177b4 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, unsigned int flags, bool cacheSigStore, + std::vector&& spent_outputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, ValidationCache& validation_cache, std::vector* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -124,6 +124,9 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail { PrecomputedTransactionData txdata; + std::vector 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, uint32_t fail // 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, uint32_t fail if (ret && add_to_cache) { // Check that we get a cache hit if the tx was valid std::vector scriptchecks; - BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(tx, state, {}, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); BOOST_CHECK(scriptchecks.empty()); } else { // Check that we get script executions to check, if the transaction // was invalid, or we didn't add to cache. std::vector scriptchecks; - BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks)); + BOOST_CHECK(CheckInputScripts(tx, state, {}, 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 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 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 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 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 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 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 532c445f042..f35acb7ee21 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -137,11 +137,11 @@ const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locato } bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, - const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, + std::vector&& spent_outputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, ValidationCache& validation_cache, std::vector* pvChecks = nullptr) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) { @@ -428,8 +428,10 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationS } } + std::vector 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 { @@ -1234,15 +1236,17 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws) constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; + std::vector 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())) { // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we // need to turn both off, and compare against just turning off CLEANSTACK // to see if the failure is specifically due to witness validation. TxValidationState state_dummy; // Want reported failures to be from first CheckInputScripts - if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache()) && - !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache())) { + if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, {}, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache()) && + !CheckInputScripts(tx, state_dummy, {}, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache())) { // Only the witness is missing, so the transaction itself may be fine. state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED, state.GetRejectReason(), state.GetDebugMessage()); @@ -2162,7 +2166,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, unsigned int flags, bool cacheSigStore, + std::vector&& spent_outputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, ValidationCache& validation_cache, std::vector* pvChecks) @@ -2187,15 +2191,6 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, } if (!txdata.m_spent_outputs_ready) { - std::vector 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()); @@ -2703,7 +2698,10 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ TxValidationState tx_state; - if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, parallel_script_checks ? &vChecks : nullptr)) { + + std::vector spent_outputs{view.GetUnspentOutputs(tx)}; + + if (fScriptChecks && !CheckInputScripts(tx, tx_state, std::move(spent_outputs), flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, parallel_script_checks ? &vChecks : nullptr)) { // Any transaction validation failure in ConnectBlock is a block consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage());