Only pass things committed to by tx's witness hash to CScriptCheck

This clarifies a bit more the ways in which the new script execution
cache could break consensus in the future if additional data from
the CCoins object were to be used as a part of script execution.

After this change, any such consensus breaks should be very visible
to reviewers, hopefully ensuring no such changes can be made.
This commit is contained in:
Matt Corallo
2017-04-27 10:37:33 -04:00
committed by Pieter Wuille
parent f68cdfe92b
commit c87b957a32
4 changed files with 16 additions and 6 deletions

View File

@@ -1119,8 +1119,16 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
const CCoins* coins = inputs.AccessCoins(prevout.hash);
assert(coins);
// We very carefully only pass in things to CScriptCheck which
// are clearly committed to by tx' witness hash. This provides
// a sanity check that our caching is not introducing consensus
// failures through additional data in, eg, the coins being
// spent being checked as a part of CScriptCheck.
const CScript& scriptPubKey = coins->vout[prevout.n].scriptPubKey;
const CAmount amount = coins->vout[prevout.n].nValue;
// Verify signature
CScriptCheck check(*coins, tx, i, flags, cacheStore, &txdata);
CScriptCheck check(scriptPubKey, amount, tx, i, flags, cacheStore, &txdata);
if (pvChecks) {
pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back());
@@ -1132,7 +1140,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// arguments; if so, don't trigger DoS protection to
// avoid splitting the network between upgraded and
// non-upgraded nodes.
CScriptCheck check2(*coins, tx, i,
CScriptCheck check2(scriptPubKey, amount, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &txdata);
if (check2())
return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));