Changeset View
Standalone View
src/validation.cpp
Show First 20 Lines • Show All 485 Lines • ▼ Show 20 Lines | IsReplayProtectionEnabledForCurrentBlock(const Consensus::Params ¶ms) | ||||
return IsReplayProtectionEnabled(params, chainActive.Tip()); | return IsReplayProtectionEnabled(params, chainActive.Tip()); | ||||
} | } | ||||
// Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool | // Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool | ||||
// were somehow broken and returning the wrong scriptPubKeys | // were somehow broken and returning the wrong scriptPubKeys | ||||
static bool CheckInputsFromMempoolAndCache( | static bool CheckInputsFromMempoolAndCache( | ||||
const CTransaction &tx, CValidationState &state, | const CTransaction &tx, CValidationState &state, | ||||
const CCoinsViewCache &view, const CTxMemPool &pool, const uint32_t flags, | const CCoinsViewCache &view, const CTxMemPool &pool, const uint32_t flags, | ||||
bool cacheSigStore, PrecomputedTransactionData &txdata) | bool cacheSigStore, PrecomputedTransactionData &txdata, | ||||
EXCLUSIVE_LOCKS_REQUIRED(cs_main) { | ScriptExecutionMetrics &metricsOut) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { | ||||
AssertLockHeld(cs_main); | AssertLockHeld(cs_main); | ||||
// pool.cs should be locked already, but go ahead and re-take the lock here | // pool.cs should be locked already, but go ahead and re-take the lock here | ||||
// to enforce that mempool doesn't change between when we check the view and | // to enforce that mempool doesn't change between when we check the view and | ||||
// when we actually call through to CheckInputs | // when we actually call through to CheckInputs | ||||
LOCK(pool.cs); | LOCK(pool.cs); | ||||
assert(!tx.IsCoinBase()); | assert(!tx.IsCoinBase()); | ||||
Show All 16 Lines | for (const CTxIn &txin : tx.vin) { | ||||
} else { | } else { | ||||
const Coin &coinFromDisk = pcoinsTip->AccessCoin(txin.prevout); | const Coin &coinFromDisk = pcoinsTip->AccessCoin(txin.prevout); | ||||
assert(!coinFromDisk.IsSpent()); | assert(!coinFromDisk.IsSpent()); | ||||
assert(coinFromDisk.GetTxOut() == coin.GetTxOut()); | assert(coinFromDisk.GetTxOut() == coin.GetTxOut()); | ||||
} | } | ||||
} | } | ||||
return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, | return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, | ||||
txdata); | txdata, metricsOut); | ||||
} | } | ||||
static bool | static bool | ||||
AcceptToMemoryPoolWorker(const Config &config, CTxMemPool &pool, | AcceptToMemoryPoolWorker(const Config &config, CTxMemPool &pool, | ||||
CValidationState &state, const CTransactionRef &ptx, | CValidationState &state, const CTransactionRef &ptx, | ||||
bool *pfMissingInputs, int64_t nAcceptTime, | bool *pfMissingInputs, int64_t nAcceptTime, | ||||
bool bypass_limits, const Amount nAbsurdFee, | bool bypass_limits, const Amount nAbsurdFee, | ||||
std::vector<COutPoint> &coins_to_uncache, | std::vector<COutPoint> &coins_to_uncache, | ||||
▲ Show 20 Lines • Show All 216 Lines • ▼ Show 20 Lines | for (const CTxIn &txin : tx.vin) { | ||||
// Make sure whatever we need to activate is actually activated. | // Make sure whatever we need to activate is actually activated. | ||||
const uint32_t scriptVerifyFlags = | const uint32_t scriptVerifyFlags = | ||||
STANDARD_SCRIPT_VERIFY_FLAGS | extraFlags; | STANDARD_SCRIPT_VERIFY_FLAGS | extraFlags; | ||||
// Check against previous transactions. This is done last to help | // Check against previous transactions. This is done last to help | ||||
// prevent CPU exhaustion denial-of-service attacks. | // prevent CPU exhaustion denial-of-service attacks. | ||||
PrecomputedTransactionData txdata(tx); | PrecomputedTransactionData txdata(tx); | ||||
ScriptExecutionMetrics metricsStandard; | |||||
if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, | if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, | ||||
txdata)) { | txdata, metricsStandard)) { | ||||
// State filled in by CheckInputs. | // State filled in by CheckInputs. | ||||
return false; | return false; | ||||
} | } | ||||
// Check again against the next block's script verification flags | // Check again against the next block's script verification flags | ||||
// to cache our script execution flags. | // to cache our script execution flags. | ||||
// | // | ||||
// This is also useful in case of bugs in the standard flags that cause | // This is also useful in case of bugs in the standard flags that cause | ||||
// transactions to pass as valid when they're actually invalid. For | // transactions to pass as valid when they're actually invalid. For | ||||
// instance the STRICTENC flag was incorrectly allowing certain CHECKSIG | // instance the STRICTENC flag was incorrectly allowing certain CHECKSIG | ||||
// NOT scripts to pass, even though they were invalid. | // NOT scripts to pass, even though they were invalid. | ||||
// | // | ||||
// There is a similar check in CreateNewBlock() to prevent creating | // There is a similar check in CreateNewBlock() to prevent creating | ||||
// invalid blocks (using TestBlockValidity), however allowing such | // invalid blocks (using TestBlockValidity), however allowing such | ||||
// transactions into the mempool can be exploited as a DoS attack. | // transactions into the mempool can be exploited as a DoS attack. | ||||
uint32_t nextBlockScriptVerifyFlags = | uint32_t nextBlockScriptVerifyFlags = | ||||
GetNextBlockScriptFlags(consensusParams, chainActive.Tip()); | GetNextBlockScriptFlags(consensusParams, chainActive.Tip()); | ||||
ScriptExecutionMetrics metricsConsensus; | |||||
if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, | if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, | ||||
nextBlockScriptVerifyFlags, true, | nextBlockScriptVerifyFlags, true, | ||||
txdata)) { | txdata, metricsConsensus)) { | ||||
// This can occur under some circumstances, if the node receives an | // This can occur under some circumstances, if the node receives an | ||||
// unrequested tx which is invalid due to new consensus rules not | // unrequested tx which is invalid due to new consensus rules not | ||||
// being activated yet (during IBD). | // being activated yet (during IBD). | ||||
return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed " | return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed " | ||||
"against next-block but not STANDARD flags %s, %s", | "against next-block but not STANDARD flags %s, %s", | ||||
__func__, txid.ToString(), FormatStateMessage(state)); | __func__, txid.ToString(), FormatStateMessage(state)); | ||||
} | } | ||||
if (!(metricsStandard == metricsConsensus)) { | |||||
deadalnix: `!=` ? | |||||
markblundebergAuthorUnsubmitted Done Inline Actionsno operator!= is defined. markblundeberg: no operator!= is defined. | |||||
// A poorly-designed standard flag might be introduced that allows | |||||
// scripts to branch depending on standard/consensus rules, and | |||||
// thereby succeed in different ways with different metrics. | |||||
// | |||||
// This is annoying, but it's safe to keep going, as we will anyway | |||||
// save the consensus metrics to the mempool so that block template | |||||
// creation is accurate. | |||||
// | |||||
// This may also happen if a new uncached field in | |||||
// ScriptExecutionMetrics is introduced and so we get different | |||||
// metrics cached/uncached. | |||||
LogPrintf("%s: BUG! PLEASE REPORT THIS! CheckInputs detected " | |||||
deadalnixUnsubmitted Not Done Inline ActionsIs this a bug though? We don't know what future flag we'll have, and by the time we introduce one, it won't be obvious this message needs to be updated. deadalnix: Is this a bug though? We don't know what future flag we'll have, and by the time we introduce… | |||||
markblundebergAuthorUnsubmitted Done Inline ActionsHmm... yes, I'd say it's a bug that indicates the standard flag needs to be changed. It's analogous to the CheckInputsFromMempoolAndCache above -- we could just silently reject such transactions without logging, since it's unambigously the correct response to a consensus-invalid tx, but it does mean there is a bug in standardness rules. markblundeberg: Hmm... yes, I'd say it's a bug that indicates the standard flag needs to be changed. It's… | |||||
deadalnixUnsubmitted Not Done Inline ActionsOK deadalnix: OK | |||||
"different script metrics for evaluation under " | |||||
"next-block and standard flags %s\n", | |||||
__func__, txid.ToString()); | |||||
} | |||||
if (test_accept) { | if (test_accept) { | ||||
// Tx was accepted, but not added | // Tx was accepted, but not added | ||||
return true; | return true; | ||||
} | } | ||||
// Store transaction in memory. | // Store transaction in memory. | ||||
pool.addUnchecked(entry, setAncestors); | pool.addUnchecked(entry, setAncestors); | ||||
▲ Show 20 Lines • Show All 406 Lines • ▼ Show 20 Lines | int GetSpendHeight(const CCoinsViewCache &inputs) { | ||||
return pindexPrev->nHeight + 1; | return pindexPrev->nHeight + 1; | ||||
} | } | ||||
bool CheckInputs(const CTransaction &tx, CValidationState &state, | bool CheckInputs(const CTransaction &tx, CValidationState &state, | ||||
const CCoinsViewCache &inputs, bool fScriptChecks, | const CCoinsViewCache &inputs, bool fScriptChecks, | ||||
const uint32_t flags, bool sigCacheStore, | const uint32_t flags, bool sigCacheStore, | ||||
bool scriptCacheStore, | bool scriptCacheStore, | ||||
const PrecomputedTransactionData &txdata, | const PrecomputedTransactionData &txdata, | ||||
ScriptExecutionMetrics &metricsOut, | |||||
std::vector<CScriptCheck> *pvChecks) { | std::vector<CScriptCheck> *pvChecks) { | ||||
AssertLockHeld(cs_main); | AssertLockHeld(cs_main); | ||||
assert(!tx.IsCoinBase()); | assert(!tx.IsCoinBase()); | ||||
if (pvChecks) { | if (pvChecks) { | ||||
pvChecks->reserve(tx.vin.size()); | pvChecks->reserve(tx.vin.size()); | ||||
} | } | ||||
// Skip script verification when connecting blocks under the assumevalid | // Skip script verification when connecting blocks under the assumevalid | ||||
// block. Assuming the assumevalid block is valid this is safe because | // block. Assuming the assumevalid block is valid this is safe because | ||||
// block merkle hashes are still computed and checked, of course, if an | // block merkle hashes are still computed and checked, of course, if an | ||||
// assumed valid block is invalid due to false scriptSigs this optimization | // assumed valid block is invalid due to false scriptSigs this optimization | ||||
// would allow an invalid chain to be accepted. | // would allow an invalid chain to be accepted. | ||||
if (!fScriptChecks) { | if (!fScriptChecks) { | ||||
return true; | return true; | ||||
} | } | ||||
// First check if script executions have been cached with the same flags. | // First check if script executions have been cached with the same flags. | ||||
// Note that this assumes that the inputs provided are correct (ie that the | // Note that this assumes that the inputs provided are correct (ie that the | ||||
// transaction hash which is in tx's prevouts properly commits to the | // transaction hash which is in tx's prevouts properly commits to the | ||||
// scriptPubKey in the inputs view of that transaction). | // scriptPubKey in the inputs view of that transaction). | ||||
ScriptCacheKey hashCacheEntry(tx, flags); | ScriptCacheKey hashCacheEntry(tx, flags); | ||||
ScriptExecutionMetrics metricsDummy; | if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, metricsOut)) { | ||||
if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, metricsDummy)) { | |||||
return true; | return true; | ||||
} | } | ||||
ScriptExecutionMetrics metricsTotal = {}; | |||||
// Sanity check: | |||||
// An input is at least 41 bytes large, and can contain up to | |||||
// MAX_OPS_PER_SCRIPT operations. Make sure the worst case can't overflow. | |||||
constexpr auto maxinputs_metrics = | |||||
std::numeric_limits<decltype(metricsTotal.nSigChecks)>::max() / | |||||
MAX_OPS_PER_SCRIPT; | |||||
constexpr auto maxinputs_tx = MAX_TX_SIZE / 41; | |||||
static_assert(maxinputs_metrics > maxinputs_tx, | |||||
"metrics accumulator sigchecks mustn't overflow"); | |||||
deadalnixUnsubmitted Not Done Inline ActionsThe thing that you want to actually check is that the maximum tx size won't give you numbers above 65k sigops. Because if not, then we won't cache precisely the script we'd benefit the most from caching. deadalnix: The thing that you want to actually check is that the maximum tx size won't give you numbers… | |||||
markblundebergAuthorUnsubmitted Done Inline ActionsThe only way we can guarantee such a thing is if either flags & SCRIPT_VERIFY_INPUT_SIGCHECKS is true at this point and it has the current definition, or parallel validation is on. Otherwise, the total sigchecks can be as high as 5000000 (which is the number the current code is testing). Is this going to be guaranteed? Yes, but only weakly and indirectly:
Anyway, that check you want about ensuring the cache can fit standard things is done in https://reviews.bitcoinabc.org/D4834. This static assert is to ensure no UB even in the worst case. If you're really worried about having uncacheable transactions due to weird things, we can allocate 23 bits to store sigchecks in the cache instead of 16, which will then be enough even for nonstandard transactions. markblundeberg: The only way we can guarantee such a thing is if either `flags & SCRIPT_VERIFY_INPUT_SIGCHECKS`… | |||||
deadalnixUnsubmitted Not Done Inline Actions
Note that this is a good argument pointing to the fact that using an int to count sigops wasn't such a good idea. You end up having to do a bunch of check that the number is positive and also have to add check for overflow anyways. I nw understand the problem of having sigops count that go over the limit before there is enforcement. But this still lead a ton of checks, and I'm not convinced that this is ideal. Why not simply not accumulate sigops when the flag is not set? No you'd only get one test at instead of the battery you need sprinkled all over the patchset. deadalnix: > This static assert is to ensure no UB even in the worst case.
Note that this is a good… | |||||
deadalnixUnsubmitted Not Done Inline ActionsNote that you end up having to do something similar to what is proposed here in D4940 anyways, so it doesn't looks like you are saving anything at all. deadalnix: Note that you end up having to do something similar to what is proposed here in D4940 anyways… | |||||
for (size_t i = 0; i < tx.vin.size(); i++) { | for (size_t i = 0; i < tx.vin.size(); i++) { | ||||
const COutPoint &prevout = tx.vin[i].prevout; | const COutPoint &prevout = tx.vin[i].prevout; | ||||
const Coin &coin = inputs.AccessCoin(prevout); | const Coin &coin = inputs.AccessCoin(prevout); | ||||
assert(!coin.IsSpent()); | assert(!coin.IsSpent()); | ||||
// We very carefully only pass in things to CScriptCheck which are | // We very carefully only pass in things to CScriptCheck which are | ||||
// clearly committed to by tx's hash. This provides a sanity | // clearly committed to by tx's hash. This provides a sanity | ||||
// check that our caching is not introducing consensus failures through | // check that our caching is not introducing consensus failures through | ||||
Show All 38 Lines | for (size_t i = 0; i < tx.vin.size(); i++) { | ||||
// careful thought should be taken as to the correct behavior - we | // careful thought should be taken as to the correct behavior - we | ||||
// may want to continue peering with non-upgraded nodes even after | // may want to continue peering with non-upgraded nodes even after | ||||
// soft-fork super-majority signaling has occurred. | // soft-fork super-majority signaling has occurred. | ||||
return state.DoS( | return state.DoS( | ||||
100, false, REJECT_INVALID, | 100, false, REJECT_INVALID, | ||||
strprintf("mandatory-script-verify-flag-failed (%s)", | strprintf("mandatory-script-verify-flag-failed (%s)", | ||||
ScriptErrorString(scriptError))); | ScriptErrorString(scriptError))); | ||||
} | } | ||||
metricsTotal += check.GetScriptExecutionMetrics(); | |||||
} | } | ||||
metricsOut = metricsTotal; | |||||
if (scriptCacheStore && !pvChecks) { | if (scriptCacheStore && !pvChecks) { | ||||
// We executed all of the provided scripts, and were told to cache the | // We executed all of the provided scripts, and were told to cache the | ||||
// result. Do so now. | // result. Do so now. | ||||
if (!TryAddKeyInScriptCache(hashCacheEntry, | if (!TryAddKeyInScriptCache(hashCacheEntry, metricsTotal)) { | ||||
ScriptExecutionMetrics(0))) { | |||||
// This shouldn't happen, but it's not a disaster since it just | // This shouldn't happen, but it's not a disaster since it just | ||||
// means we have to re-execute the scripts next time we see this | // means we have to re-execute the scripts next time we see this | ||||
// transaction (and some of its signatures ought to be cached, so it | // transaction (and some of its signatures ought to be cached, so it | ||||
// will be faster the second time). | // will be faster the second time). | ||||
LogPrintf("%s: BUG! PLEASE REPORT THIS! Unable to cache validity " | LogPrintf("%s: BUG! PLEASE REPORT THIS! Unable to cache validity " | ||||
"of tx %s (with flags %x).\n", | "of tx %s (with flags %x).\n", | ||||
__func__, tx.GetId().ToString(), flags); | __func__, tx.GetId().ToString(), flags); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 588 Lines • ▼ Show 20 Lines | for (const auto &ptx : block.vtx) { | ||||
REJECT_INVALID, "bad-blk-sigops"); | REJECT_INVALID, "bad-blk-sigops"); | ||||
} | } | ||||
// Don't cache results if we're actually connecting blocks (still | // Don't cache results if we're actually connecting blocks (still | ||||
// consult the cache, though). | // consult the cache, though). | ||||
bool fCacheResults = fJustCheck; | bool fCacheResults = fJustCheck; | ||||
std::vector<CScriptCheck> vChecks; | std::vector<CScriptCheck> vChecks; | ||||
// metricsRet may be accurate (found in cache) or 0 (checks were | |||||
// deferred into vChecks). | |||||
ScriptExecutionMetrics metricsRet; | |||||
if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, | if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, | ||||
fCacheResults, PrecomputedTransactionData(tx), | fCacheResults, PrecomputedTransactionData(tx), | ||||
&vChecks)) { | metricsRet, &vChecks)) { | ||||
return error("ConnectBlock(): CheckInputs on %s failed with %s", | return error("ConnectBlock(): CheckInputs on %s failed with %s", | ||||
tx.GetId().ToString(), FormatStateMessage(state)); | tx.GetId().ToString(), FormatStateMessage(state)); | ||||
} | } | ||||
control.Add(vChecks); | control.Add(vChecks); | ||||
blockundo.vtxundo.push_back(CTxUndo()); | blockundo.vtxundo.push_back(CTxUndo()); | ||||
SpendCoins(view, tx, blockundo.vtxundo.back(), pindex->nHeight); | SpendCoins(view, tx, blockundo.vtxundo.back(), pindex->nHeight); | ||||
▲ Show 20 Lines • Show All 3,751 Lines • Show Last 20 Lines |
!= ?