From d2c5d044d00ec805957ab246a7863d83ca075805 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 16 Aug 2016 15:35:45 +0200 Subject: [PATCH 1/3] Precompute sighashes Original version by Nicolas Dorier. Precomputing version by Pieter Wuille. --- src/main.cpp | 22 ++++++++------ src/main.h | 9 ++++-- src/script/bitcoinconsensus.cpp | 4 +-- src/script/interpreter.cpp | 54 ++++++++++++++++++++++----------- src/script/interpreter.h | 13 ++++++-- src/script/sigcache.h | 2 +- src/test/script_P2SH_tests.cpp | 6 ++-- src/test/transaction_tests.cpp | 6 ++-- 8 files changed, 78 insertions(+), 38 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index db457f6f53d57..7f48ccbae1de4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1492,12 +1492,13 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. - if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true)) { + CachedHashes cachedHashes(tx); + if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, cachedHashes)) { // 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. - if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true) && - !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true)) { + if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, cachedHashes) && + !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, cachedHashes)) { // Only the witness is wrong, so the transaction itself may be fine. state.SetCorruptionPossible(); } @@ -1513,7 +1514,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) { return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -1910,7 +1911,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight) bool CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; const CScriptWitness *witness = (nIn < ptxTo->wit.vtxinwit.size()) ? &ptxTo->wit.vtxinwit[nIn].scriptWitness : NULL; - if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore), &error)) { + if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *cachedHashes), &error)) { return false; } return true; @@ -1969,7 +1970,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } }// namespace Consensus -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -1996,7 +1997,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi assert(coins); // Verify signature - CScriptCheck check(*coins, tx, i, flags, cacheStore); + CScriptCheck check(*coins, tx, i, flags, cacheStore, &cachedHashes); if (pvChecks) { pvChecks->push_back(CScriptCheck()); check.swap(pvChecks->back()); @@ -2009,7 +2010,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // avoid splitting the network between upgraded and // non-upgraded nodes. CScriptCheck check2(*coins, tx, i, - flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore); + flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes); if (check2()) return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } @@ -2405,6 +2406,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::vector > vPos; vPos.reserve(block.vtx.size()); blockundo.vtxundo.reserve(block.vtx.size() - 1); + std::vector cachedHashes; + cachedHashes.reserve(block.vtx.size()); // Required so that pointers to individual CachedHashes don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction &tx = block.vtx[i]; @@ -2451,13 +2454,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return state.DoS(100, error("ConnectBlock(): too many sigops"), REJECT_INVALID, "bad-blk-sigops"); + cachedHashes.emplace_back(tx); if (!tx.IsCoinBase()) { nFees += view.GetValueIn(tx)-tx.GetValueOut(); std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ - if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL)) + if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, cachedHashes[i], nScriptCheckThreads ? &vChecks : NULL)) return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); control.Add(vChecks); diff --git a/src/main.h b/src/main.h index d4d70c018027d..fbf66c0481166 100644 --- a/src/main.h +++ b/src/main.h @@ -38,6 +38,7 @@ class CScriptCheck; class CTxMemPool; class CValidationInterface; class CValidationState; +class CachedHashes; struct CNodeStateStats; struct LockPoints; @@ -347,7 +348,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i * instead of being performed inline. */ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, - unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); + unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); @@ -421,12 +422,13 @@ class CScriptCheck unsigned int nFlags; bool cacheStore; ScriptError error; + CachedHashes *cachedHashes; public: CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} - CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) : + CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, CachedHashes* cachedHashesIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), - ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) { } + ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), cachedHashes(cachedHashesIn) { } bool operator()(); @@ -438,6 +440,7 @@ class CScriptCheck std::swap(nFlags, check.nFlags); std::swap(cacheStore, check.cacheStore); std::swap(error, check.error); + std::swap(cachedHashes, check.cachedHashes); } ScriptError GetScriptError() const { return error; } diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 62fd9031f853a..01a8babb679eb 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -84,8 +84,8 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP // Regardless of the verification result, the tx did not error. set_error(err, bitcoinconsensus_ERR_OK); - - return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount), NULL); + CachedHashes cachedHashes(tx); + return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount, cachedHashes), NULL); } catch (const std::exception&) { return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index bc027e9f0c113..459feaa0f4826 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1108,9 +1108,40 @@ class CTransactionSignatureSerializer { } }; +uint256 GetPrevoutHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].prevout; + } + return ss.GetHash(); +} + +uint256 GetSequenceHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].nSequence; + } + return ss.GetHash(); +} + +uint256 GetOutputsHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vout.size(); n++) { + ss << txTo.vout[n]; + } + return ss.GetHash(); +} + } // anon namespace -uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion) +CachedHashes::CachedHashes(const CTransaction& txTo) +{ + hashPrevouts = GetPrevoutHash(txTo); + hashSequence = GetSequenceHash(txTo); + hashOutputs = GetOutputsHash(txTo); +} + +uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache) { if (sigversion == SIGVERSION_WITNESS_V0) { uint256 hashPrevouts; @@ -1118,27 +1149,16 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig uint256 hashOutputs; if (!(nHashType & SIGHASH_ANYONECANPAY)) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].prevout; - } - hashPrevouts = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashPrevouts = cache ? cache->hashPrevouts : GetPrevoutHash(txTo); } if (!(nHashType & SIGHASH_ANYONECANPAY) && (nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].nSequence; - } - hashSequence = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashSequence = cache ? cache->hashSequence : GetSequenceHash(txTo); } + if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vout.size(); n++) { - ss << txTo.vout[n]; - } - hashOutputs = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashOutputs = cache ? cache->hashOutputs : GetOutputsHash(txTo); } else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) { CHashWriter ss(SER_GETHASH, 0); ss << txTo.vout[nIn]; @@ -1209,7 +1229,7 @@ bool TransactionSignatureChecker::CheckSig(const vector& vchSigIn int nHashType = vchSig.back(); vchSig.pop_back(); - uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->cachedHashes); if (!VerifySignature(vchSig, pubkey, sighash)) return false; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index bd2f21166368a..b4da113eec939 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -98,13 +98,20 @@ enum bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); +struct CachedHashes +{ + uint256 hashPrevouts, hashSequence, hashOutputs; + + CachedHashes(const CTransaction& tx); +}; + enum SigVersion { SIGVERSION_BASE = 0, SIGVERSION_WITNESS_V0 = 1, }; -uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion); +uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache = NULL); class BaseSignatureChecker { @@ -133,12 +140,14 @@ class TransactionSignatureChecker : public BaseSignatureChecker const CTransaction* txTo; unsigned int nIn; const CAmount amount; + const CachedHashes* cachedHashes; protected: virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(NULL) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const CachedHashes& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(&cachedHashesIn) {} bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const; bool CheckLockTime(const CScriptNum& nLockTime) const; bool CheckSequence(const CScriptNum& nSequence) const; diff --git a/src/script/sigcache.h b/src/script/sigcache.h index 050bf8cc42290..551f8b2253e76 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -22,7 +22,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn) : TransactionSignatureChecker(txToIn, nInIn, amount), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, CachedHashes& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {} bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; }; diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 5224b57ca4479..5cd7c93622977 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -107,18 +107,20 @@ BOOST_AUTO_TEST_CASE(sign) } // All of the above should be OK, and the txTos have valid signatures // Check to make sure signature verification fails if we use the wrong ScriptSig: - for (int i = 0; i < 8; i++) + for (int i = 0; i < 8; i++) { + CachedHashes cachedHashes(txTo[i]); for (int j = 0; j < 8; j++) { CScript sigSave = txTo[i].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; - bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false)(); + bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &cachedHashes)(); if (i == j) BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); else BOOST_CHECK_MESSAGE(!sigOK, strprintf("VerifySignature %d %d", i, j)); txTo[i].vin[0].scriptSig = sigSave; } + } } BOOST_AUTO_TEST_CASE(norecurse) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index fd4f174b40c80..c06515f7302b6 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -153,6 +153,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); BOOST_CHECK(state.IsValid()); + CachedHashes cachedHashes(tx); for (unsigned int i = 0; i < tx.vin.size(); i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -168,7 +169,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - witness, verify_flags, TransactionSignatureChecker(&tx, i, amount), &err), + witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err), strTest); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -237,6 +238,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CValidationState state; fValid = CheckTransaction(tx, state) && state.IsValid(); + CachedHashes cachedHashes(tx); for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -252,7 +254,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) } const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - witness, verify_flags, TransactionSignatureChecker(&tx, i, amount), &err); + witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err); } BOOST_CHECK_MESSAGE(!fValid, strTest); BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err)); From ab48c5e72156b34300db4a6521cb3c9969be3937 Mon Sep 17 00:00:00 2001 From: Nicolas DORIER Date: Tue, 16 Aug 2016 15:37:56 +0200 Subject: [PATCH 2/3] Unit test for sighash caching --- src/test/transaction_tests.cpp | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c06515f7302b6..8f8666c76d1dc 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -7,6 +7,7 @@ #include "test/test_bitcoin.h" #include "clientversion.h" +#include "checkqueue.h" #include "consensus/validation.h" #include "core_io.h" #include "key.h" @@ -421,6 +422,86 @@ void ReplaceRedeemScript(CScript& script, const CScript& redeemScript) script = PushAll(stack); } +BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { + CMutableTransaction mtx; + mtx.nVersion = 1; + + CKey key; + key.MakeNewKey(false); + CBasicKeyStore keystore; + keystore.AddKeyPubKey(key, key.GetPubKey()); + CKeyID hash = key.GetPubKey().GetID(); + CScript scriptPubKey = CScript() << OP_0 << std::vector(hash.begin(), hash.end()); + + vector sigHashes; + sigHashes.push_back(SIGHASH_NONE | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_SINGLE | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_ALL | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_NONE); + sigHashes.push_back(SIGHASH_SINGLE); + sigHashes.push_back(SIGHASH_ALL); + + // create a big transaction of 4500 inputs signed by the same key + for(uint32_t ij = 0; ij < 4500; ij++) { + uint32_t i = mtx.vin.size(); + uint256 prevId; + prevId.SetHex("0000000000000000000000000000000000000000000000000000000000000100"); + COutPoint outpoint(prevId, i); + + mtx.vin.resize(mtx.vin.size() + 1); + mtx.vin[i].prevout = outpoint; + mtx.vin[i].scriptSig = CScript(); + + mtx.vout.resize(mtx.vout.size() + 1); + mtx.vout[i].nValue = 1000; + mtx.vout[i].scriptPubKey = CScript() << OP_1; + } + + // sign all inputs + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + bool hashSigned = SignSignature(keystore, scriptPubKey, mtx, i, 1000, sigHashes.at(i % sigHashes.size())); + assert(hashSigned); + } + + CTransaction tx; + CDataStream ssout(SER_NETWORK, PROTOCOL_VERSION); + WithOrVersion(&ssout, 0) << mtx; + WithOrVersion(&ssout, 0) >> tx; + + // check all inputs concurrently, with the cache + CachedHashes cachedHashes(tx); + boost::thread_group threadGroup; + CCheckQueue scriptcheckqueue(128); + CCheckQueueControl control(&scriptcheckqueue); + + for (int i=0; i<20; i++) + threadGroup.create_thread(boost::bind(&CCheckQueue::Thread, boost::ref(scriptcheckqueue))); + + CCoins coins; + coins.nVersion = 1; + coins.fCoinBase = false; + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + CTxOut txout; + txout.nValue = 1000; + txout.scriptPubKey = scriptPubKey; + coins.vout.push_back(txout); + } + + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + std::vector vChecks; + CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &cachedHashes); + vChecks.push_back(CScriptCheck()); + check.swap(vChecks.back()); + control.Add(vChecks); + } + + bool controlCheck = control.Wait(); + assert(controlCheck); + + threadGroup.interrupt_all(); + threadGroup.join_all(); +} + BOOST_AUTO_TEST_CASE(test_witness) { CBasicKeyStore keystore, keystore2; From 35fe0393f216aa6020fc929272118eade5628636 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 26 Aug 2016 18:38:20 +0200 Subject: [PATCH 3/3] Rename to PrecomputedTransactionData --- src/main.cpp | 26 +++++++++++++------------- src/main.h | 12 ++++++------ src/script/bitcoinconsensus.cpp | 4 ++-- src/script/interpreter.cpp | 6 +++--- src/script/interpreter.h | 12 ++++++------ src/script/sigcache.h | 2 +- src/test/script_P2SH_tests.cpp | 4 ++-- src/test/transaction_tests.cpp | 12 ++++++------ 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 7f48ccbae1de4..3cb476f9577ba 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1492,13 +1492,13 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. - CachedHashes cachedHashes(tx); - if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, cachedHashes)) { + PrecomputedTransactionData txdata(tx); + if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, txdata)) { // 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. - if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, cachedHashes) && - !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, cachedHashes)) { + if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) && + !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) { // Only the witness is wrong, so the transaction itself may be fine. state.SetCorruptionPossible(); } @@ -1514,7 +1514,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) + if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, txdata)) { return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -1911,7 +1911,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight) bool CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; const CScriptWitness *witness = (nIn < ptxTo->wit.vtxinwit.size()) ? &ptxTo->wit.vtxinwit[nIn].scriptWitness : NULL; - if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *cachedHashes), &error)) { + if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *txdata), &error)) { return false; } return true; @@ -1970,7 +1970,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } }// namespace Consensus -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -1997,7 +1997,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi assert(coins); // Verify signature - CScriptCheck check(*coins, tx, i, flags, cacheStore, &cachedHashes); + CScriptCheck check(*coins, tx, i, flags, cacheStore, &txdata); if (pvChecks) { pvChecks->push_back(CScriptCheck()); check.swap(pvChecks->back()); @@ -2010,7 +2010,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // avoid splitting the network between upgraded and // non-upgraded nodes. CScriptCheck check2(*coins, tx, i, - flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes); + 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()))); } @@ -2406,8 +2406,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::vector > vPos; vPos.reserve(block.vtx.size()); blockundo.vtxundo.reserve(block.vtx.size() - 1); - std::vector cachedHashes; - cachedHashes.reserve(block.vtx.size()); // Required so that pointers to individual CachedHashes don't get invalidated + std::vector txdata; + txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction &tx = block.vtx[i]; @@ -2454,14 +2454,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return state.DoS(100, error("ConnectBlock(): too many sigops"), REJECT_INVALID, "bad-blk-sigops"); - cachedHashes.emplace_back(tx); + txdata.emplace_back(tx); if (!tx.IsCoinBase()) { nFees += view.GetValueIn(tx)-tx.GetValueOut(); std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ - if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, cachedHashes[i], nScriptCheckThreads ? &vChecks : NULL)) + if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, txdata[i], nScriptCheckThreads ? &vChecks : NULL)) return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); control.Add(vChecks); diff --git a/src/main.h b/src/main.h index fbf66c0481166..1c374c8a03ae3 100644 --- a/src/main.h +++ b/src/main.h @@ -38,7 +38,7 @@ class CScriptCheck; class CTxMemPool; class CValidationInterface; class CValidationState; -class CachedHashes; +class PrecomputedTransactionData; struct CNodeStateStats; struct LockPoints; @@ -348,7 +348,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i * instead of being performed inline. */ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, - unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks = NULL); + unsigned int flags, bool cacheStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); @@ -422,13 +422,13 @@ class CScriptCheck unsigned int nFlags; bool cacheStore; ScriptError error; - CachedHashes *cachedHashes; + PrecomputedTransactionData *txdata; public: CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} - CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, CachedHashes* cachedHashesIn) : + CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), - ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), cachedHashes(cachedHashesIn) { } + ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { } bool operator()(); @@ -440,7 +440,7 @@ class CScriptCheck std::swap(nFlags, check.nFlags); std::swap(cacheStore, check.cacheStore); std::swap(error, check.error); - std::swap(cachedHashes, check.cachedHashes); + std::swap(txdata, check.txdata); } ScriptError GetScriptError() const { return error; } diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 01a8babb679eb..b629f4278ba20 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -84,8 +84,8 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP // Regardless of the verification result, the tx did not error. set_error(err, bitcoinconsensus_ERR_OK); - CachedHashes cachedHashes(tx); - return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount, cachedHashes), NULL); + PrecomputedTransactionData txdata(tx); + return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata), NULL); } catch (const std::exception&) { return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 459feaa0f4826..47ea261e31040 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1134,14 +1134,14 @@ uint256 GetOutputsHash(const CTransaction& txTo) { } // anon namespace -CachedHashes::CachedHashes(const CTransaction& txTo) +PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo) { hashPrevouts = GetPrevoutHash(txTo); hashSequence = GetSequenceHash(txTo); hashOutputs = GetOutputsHash(txTo); } -uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache) +uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache) { if (sigversion == SIGVERSION_WITNESS_V0) { uint256 hashPrevouts; @@ -1229,7 +1229,7 @@ bool TransactionSignatureChecker::CheckSig(const vector& vchSigIn int nHashType = vchSig.back(); vchSig.pop_back(); - uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->cachedHashes); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata); if (!VerifySignature(vchSig, pubkey, sighash)) return false; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index b4da113eec939..e5d7865cd3de6 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -98,11 +98,11 @@ enum bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); -struct CachedHashes +struct PrecomputedTransactionData { uint256 hashPrevouts, hashSequence, hashOutputs; - CachedHashes(const CTransaction& tx); + PrecomputedTransactionData(const CTransaction& tx); }; enum SigVersion @@ -111,7 +111,7 @@ enum SigVersion SIGVERSION_WITNESS_V0 = 1, }; -uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache = NULL); +uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = NULL); class BaseSignatureChecker { @@ -140,14 +140,14 @@ class TransactionSignatureChecker : public BaseSignatureChecker const CTransaction* txTo; unsigned int nIn; const CAmount amount; - const CachedHashes* cachedHashes; + const PrecomputedTransactionData* txdata; protected: virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(NULL) {} - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const CachedHashes& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(&cachedHashesIn) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(NULL) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {} bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const; bool CheckLockTime(const CScriptNum& nLockTime) const; bool CheckSequence(const CScriptNum& nSequence) const; diff --git a/src/script/sigcache.h b/src/script/sigcache.h index 551f8b2253e76..44551ec2bc435 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -22,7 +22,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, CachedHashes& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amount, txdataIn), store(storeIn) {} bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; }; diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 5cd7c93622977..1a01593a8e163 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -108,12 +108,12 @@ BOOST_AUTO_TEST_CASE(sign) // All of the above should be OK, and the txTos have valid signatures // Check to make sure signature verification fails if we use the wrong ScriptSig: for (int i = 0; i < 8; i++) { - CachedHashes cachedHashes(txTo[i]); + PrecomputedTransactionData txdata(txTo[i]); for (int j = 0; j < 8; j++) { CScript sigSave = txTo[i].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; - bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &cachedHashes)(); + bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)(); if (i == j) BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); else diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8f8666c76d1dc..b5af400bc54b5 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); BOOST_CHECK(state.IsValid()); - CachedHashes cachedHashes(tx); + PrecomputedTransactionData txdata(tx); for (unsigned int i = 0; i < tx.vin.size(); i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err), + witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, txdata), &err), strTest); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CValidationState state; fValid = CheckTransaction(tx, state) && state.IsValid(); - CachedHashes cachedHashes(tx); + PrecomputedTransactionData txdata(tx); for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) } const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err); + witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, txdata), &err); } BOOST_CHECK_MESSAGE(!fValid, strTest); BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err)); @@ -469,7 +469,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { WithOrVersion(&ssout, 0) >> tx; // check all inputs concurrently, with the cache - CachedHashes cachedHashes(tx); + PrecomputedTransactionData txdata(tx); boost::thread_group threadGroup; CCheckQueue scriptcheckqueue(128); CCheckQueueControl control(&scriptcheckqueue); @@ -489,7 +489,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { for(uint32_t i = 0; i < mtx.vin.size(); i++) { std::vector vChecks; - CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &cachedHashes); + CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); vChecks.push_back(CScriptCheck()); check.swap(vChecks.back()); control.Add(vChecks);