merge #13868: Remove unused fScriptChecks parameter from CheckInputs

This commit is contained in:
Konstantin Akimov 2019-09-02 15:36:57 +08:00 committed by PastaPastaPasta
parent 15a9783e52
commit ce3e8072a7
2 changed files with 88 additions and 95 deletions

View File

@ -12,7 +12,7 @@
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks); bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks);
BOOST_AUTO_TEST_SUITE(txvalidationcache_tests) BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)
@ -118,7 +118,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
// script/interpreter.cpp // script/interpreter.cpp
test_flags |= SCRIPT_VERIFY_P2SH; test_flags |= SCRIPT_VERIFY_P2SH;
} }
bool ret = CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, nullptr); bool ret = CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, add_to_cache, txdata, nullptr);
// CheckInputs should succeed iff test_flags doesn't intersect with // CheckInputs should succeed iff test_flags doesn't intersect with
// failing_flags // failing_flags
bool expected_return_value = !(test_flags & failing_flags); bool expected_return_value = !(test_flags & failing_flags);
@ -128,13 +128,13 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
if (ret && add_to_cache) { if (ret && add_to_cache) {
// Check that we get a cache hit if the tx was valid // Check that we get a cache hit if the tx was valid
std::vector<CScriptCheck> scriptchecks; std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, add_to_cache, txdata, &scriptchecks));
BOOST_CHECK(scriptchecks.empty()); BOOST_CHECK(scriptchecks.empty());
} else { } else {
// Check that we get script executions to check, if the transaction // Check that we get script executions to check, if the transaction
// was invalid, or we didn't add to cache. // was invalid, or we didn't add to cache.
std::vector<CScriptCheck> scriptchecks; std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, add_to_cache, txdata, &scriptchecks));
BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size());
} }
} }
@ -196,13 +196,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
CValidationState state; CValidationState state;
PrecomputedTransactionData ptd_spend_tx; PrecomputedTransactionData ptd_spend_tx;
BOOST_CHECK(!CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr)); BOOST_CHECK(!CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr));
// If we call again asking for scriptchecks (as happens in // If we call again asking for scriptchecks (as happens in
// ConnectBlock), we should add a script check object for this -- we're // ConnectBlock), we should add a script check object for this -- we're
// not caching invalidity (if that changes, delete this test case). // not caching invalidity (if that changes, delete this test case).
std::vector<CScriptCheck> scriptchecks; std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks)); BOOST_CHECK(CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks));
BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U);
// Test that CheckInputs returns true iff DERSIG-enforcing flags are // Test that CheckInputs returns true iff DERSIG-enforcing flags are
@ -264,7 +264,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100; invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
CValidationState state; CValidationState state;
PrecomputedTransactionData txdata; PrecomputedTransactionData txdata;
BOOST_CHECK(CheckInputs(CTransaction(invalid_with_cltv_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr)); BOOST_CHECK(CheckInputs(CTransaction(invalid_with_cltv_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr));
} }
// TEST CHECKSEQUENCEVERIFY // TEST CHECKSEQUENCEVERIFY
@ -292,7 +292,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
CValidationState state; CValidationState state;
PrecomputedTransactionData txdata; PrecomputedTransactionData txdata;
BOOST_CHECK(CheckInputs(CTransaction(invalid_with_csv_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr)); BOOST_CHECK(CheckInputs(CTransaction(invalid_with_csv_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr));
} }
// TODO: add tests for remaining script flags // TODO: add tests for remaining script flags

View File

@ -201,7 +201,7 @@ std::unique_ptr<CBlockTreeDB> pblocktree;
// See definition for documentation // See definition for documentation
static void FindFilesToPruneManual(ChainstateManager& chainman, std::set<int>& setFilesToPrune, int nManualPruneHeight); static void FindFilesToPruneManual(ChainstateManager& chainman, std::set<int>& setFilesToPrune, int nManualPruneHeight);
static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight); static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight);
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks = nullptr); bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks = nullptr);
static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false); static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false);
static FlatFileSeq BlockFileSeq(); static FlatFileSeq BlockFileSeq();
static FlatFileSeq UndoFileSeq(); static FlatFileSeq UndoFileSeq();
@ -503,7 +503,7 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt
} }
// Call CheckInputs() to cache signature and script validity against current tip consensus rules. // Call CheckInputs() to cache signature and script validity against current tip consensus rules.
return CheckInputs(tx, state, view, true, flags, /* cacheSigStore = */ true, /* cacheFullSciptStore = */ true, txdata); return CheckInputs(tx, state, view, flags, /* cacheSigStore = */ true, /* cacheFullSciptStore = */ true, txdata);
} }
/** /**
@ -731,7 +731,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// Check against previous transactions // Check against previous transactions
// This is done last to help prevent CPU exhaustion denial-of-service attacks. // This is done last to help prevent CPU exhaustion denial-of-service attacks.
PrecomputedTransactionData txdata; PrecomputedTransactionData txdata;
if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, txdata)) { if (!CheckInputs(tx, state, view, scriptVerifyFlags, true, false, txdata)) {
assert(IsTransactionReason(state.GetReason())); assert(IsTransactionReason(state.GetReason()));
return false; // state filled in by CheckInputs return false; // state filled in by CheckInputs
} }
@ -1415,100 +1415,88 @@ void InitScriptExecutionCache() {
* *
* Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp * Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp
*/ */
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{ {
boost::posix_time::ptime start = boost::posix_time::microsec_clock::local_time(); boost::posix_time::ptime start = boost::posix_time::microsec_clock::local_time();
if (!tx.IsCoinBase()) if (tx.IsCoinBase()) return true;
{
if (pvChecks)
pvChecks->reserve(tx.vin.size());
// The first loop above does all the inexpensive checks. if (pvChecks) {
// Only if ALL inputs pass do we perform expensive ECDSA signature checks. pvChecks->reserve(tx.vin.size());
// Helps prevent CPU exhaustion attacks. }
// Skip script verification when connecting blocks under the // First check if script executions have been cached with the same
// assumevalid block. Assuming the assumevalid block is valid this // flags. Note that this assumes that the inputs provided are
// is safe because block merkle hashes are still computed and checked, // correct (ie that the transaction hash which is in tx's prevouts
// Of course, if an assumed valid block is invalid due to false scriptSigs // properly commits to the scriptPubKey in the inputs view of that
// this optimization would allow an invalid chain to be accepted. // transaction).
if (fScriptChecks) { uint256 hashCacheEntry;
// First check if script executions have been cached with the same CSHA256 hasher = g_scriptExecutionCacheHasher;
// flags. Note that this assumes that the inputs provided are hasher.Write(tx.GetHash().begin(), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
// correct (ie that the transaction hash which is in tx's prevouts AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks
// properly commits to the scriptPubKey in the inputs view of that if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) {
// transaction). return true;
uint256 hashCacheEntry; }
CSHA256 hasher = g_scriptExecutionCacheHasher;
hasher.Write(tx.GetHash().begin(), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin()); if (!txdata.m_ready) {
AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks txdata.Init(tx, {});
if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) { }
return true;
} for (unsigned int i = 0; i < tx.vin.size(); i++) {
const COutPoint &prevout = tx.vin[i].prevout;
if (!txdata.m_ready) { const Coin& coin = inputs.AccessCoin(prevout);
txdata.Init(tx, {}); assert(!coin.IsSpent());
}
// We very carefully only pass in things to CScriptCheck which
for (unsigned int i = 0; i < tx.vin.size(); i++) { // are clearly committed to by tx' witness hash. This provides
const COutPoint &prevout = tx.vin[i].prevout; // a sanity check that our caching is not introducing consensus
const Coin& coin = inputs.AccessCoin(prevout); // failures through additional data in, eg, the coins being
assert(!coin.IsSpent()); // spent being checked as a part of CScriptCheck.
// We very carefully only pass in things to CScriptCheck which // Verify signature
// are clearly committed to by tx' witness hash. This provides CScriptCheck check(coin.out, tx, i, flags, cacheSigStore, &txdata);
// a sanity check that our caching is not introducing consensus if (pvChecks) {
// failures through additional data in, eg, the coins being pvChecks->push_back(CScriptCheck());
// spent being checked as a part of CScriptCheck. check.swap(pvChecks->back());
} else if (!check()) {
// Verify signature const bool hasNonMandatoryFlags = (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) != 0;
CScriptCheck check(coin.out, tx, i, flags, cacheSigStore, &txdata); const bool hasDIP0020Opcodes = (flags & SCRIPT_ENABLE_DIP0020_OPCODES) != 0;
if (pvChecks) {
pvChecks->push_back(CScriptCheck()); if (hasNonMandatoryFlags || !hasDIP0020Opcodes) {
check.swap(pvChecks->back()); // Check whether the failure was caused by a
} else if (!check()) { // non-mandatory script verification check, such as
const bool hasNonMandatoryFlags = (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) != 0; // non-standard DER encodings or non-null dummy
const bool hasDIP0020Opcodes = (flags & SCRIPT_ENABLE_DIP0020_OPCODES) != 0; // arguments; if so, ensure we return NOT_STANDARD
// instead of CONSENSUS to avoid downstream users
if (hasNonMandatoryFlags || !hasDIP0020Opcodes) { // splitting the network between upgraded and
// Check whether the failure was caused by a // non-upgraded nodes by banning CONSENSUS-failing
// non-mandatory script verification check, such as // data providers.
// non-standard DER encodings or non-null dummy CScriptCheck check2(coin.out, tx, i,
// arguments; if so, ensure we return NOT_STANDARD (flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS) | SCRIPT_ENABLE_DIP0020_OPCODES, cacheSigStore, &txdata);
// instead of CONSENSUS to avoid downstream users if (check2())
// splitting the network between upgraded and return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
// non-upgraded nodes by banning CONSENSUS-failing
// data providers.
CScriptCheck check2(coin.out, tx, i,
(flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS) | SCRIPT_ENABLE_DIP0020_OPCODES, cacheSigStore, &txdata);
if (check2())
return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
}
// MANDATORY flag failures correspond to
// ValidationInvalidReason::CONSENSUS. Because CONSENSUS
// failures are the most serious case of validation
// failures, we may need to consider using
// RECENT_CONSENSUS_CHANGE for any script failure that
// could be due to non-upgraded nodes which we may want to
// support, to avoid splitting the network (but this
// depends on the details of how net_processing handles
// such errors).
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError())));
}
}
if (cacheFullScriptStore && !pvChecks) {
// We executed all of the provided scripts, and were told to
// cache the result. Do so now.
g_scriptExecutionCache.insert(hashCacheEntry);
} }
// MANDATORY flag failures correspond to
// ValidationInvalidReason::CONSENSUS. Because CONSENSUS
// failures are the most serious case of validation
// failures, we may need to consider using
// RECENT_CONSENSUS_CHANGE for any script failure that
// could be due to non-upgraded nodes which we may want to
// support, to avoid splitting the network (but this
// depends on the details of how net_processing handles
// such errors).
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError())));
} }
} }
if (cacheFullScriptStore && !pvChecks) {
// We executed all of the provided scripts, and were told to
// cache the result. Do so now.
g_scriptExecutionCache.insert(hashCacheEntry);
}
boost::posix_time::ptime finish = boost::posix_time::microsec_clock::local_time(); boost::posix_time::ptime finish = boost::posix_time::microsec_clock::local_time();
boost::posix_time::time_duration diff = finish - start; boost::posix_time::time_duration diff = finish - start;
statsClient.timing("CheckInputs_ms", diff.total_milliseconds(), 1.0f); statsClient.timing("CheckInputs_ms", diff.total_milliseconds(), 1.0f);
return true; return true;
} }
@ -2055,6 +2043,11 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
pindexBestHeader->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->GetAncestor(pindex->nHeight) == pindex &&
pindexBestHeader->nChainWork >= nMinimumChainWork) { pindexBestHeader->nChainWork >= nMinimumChainWork) {
// This block is a member of the assumed verified chain and an ancestor of the best header. // This block is a member of the assumed verified chain and an ancestor of the best header.
// Script verification is skipped when connecting blocks under the
// assumevalid block. Assuming the assumevalid block is valid this
// is safe because block merkle hashes are still computed and checked,
// Of course, if an assumed valid block is invalid due to false scriptSigs
// this optimization would allow an invalid chain to be accepted.
// The equivalent time check discourages hash power from extorting the network via DOS attack // The equivalent time check discourages hash power from extorting the network via DOS attack
// into accepting an invalid block through telling users they must manually set assumevalid. // into accepting an invalid block through telling users they must manually set assumevalid.
// Requiring a software change or burying the invalid block, regardless of the setting, makes // Requiring a software change or burying the invalid block, regardless of the setting, makes
@ -2252,7 +2245,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
std::vector<CScriptCheck> vChecks; std::vector<CScriptCheck> vChecks;
bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
if (fScriptChecks && !CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) { if (fScriptChecks && !CheckInputs(tx, state, view, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) {
if (state.GetReason() == ValidationInvalidReason::TX_NOT_STANDARD) { if (state.GetReason() == ValidationInvalidReason::TX_NOT_STANDARD) {
// CheckInputs may return NOT_STANDARD for extra flags we passed, // CheckInputs may return NOT_STANDARD for extra flags we passed,
// but we can't return that, as it's not defined for a block, so // but we can't return that, as it's not defined for a block, so