From f1136200a67fc1df894d45fba51b40f748e0b889 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 8 Sep 2012 17:33:10 +0200 Subject: [PATCH 1/5] Move VerifySignature to main --- src/main.cpp | 11 +++++++++++ src/main.h | 2 ++ src/script.cpp | 11 ----------- src/script.h | 4 +--- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 28fbc8b45..c0446370a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1348,6 +1348,17 @@ bool CTransaction::HaveInputs(CCoinsViewCache &inputs) const return true; } +bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType) +{ + assert(nIn < txTo.vin.size()); + const CTxIn& txin = txTo.vin[nIn]; + if (txin.prevout.n >= txFrom.vout.size()) + return false; + const CTxOut& txout = txFrom.vout[txin.prevout.n]; + + return VerifyScript(txin.scriptSig, txout.scriptPubKey, txTo, nIn, flags, nHashType); +} + bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmode, unsigned int flags) const { if (!IsCoinBase()) diff --git a/src/main.h b/src/main.h index 8bceffbaf..a098cc775 100644 --- a/src/main.h +++ b/src/main.h @@ -164,6 +164,8 @@ bool SetBestChain(CBlockIndex* pindexNew); bool ConnectBestBlock(); /** Create a new block index entry for a given block hash */ CBlockIndex * InsertBlockIndex(uint256 hash); +/** Verify a signature */ +bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType); diff --git a/src/script.cpp b/src/script.cpp index f65508aac..52a8f2c11 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -1723,17 +1723,6 @@ bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CTrans return SignSignature(keystore, txout.scriptPubKey, txTo, nIn, nHashType); } -bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType) -{ - assert(nIn < txTo.vin.size()); - const CTxIn& txin = txTo.vin[nIn]; - if (txin.prevout.n >= txFrom.vout.size()) - return false; - const CTxOut& txout = txFrom.vout[txin.prevout.n]; - - return VerifyScript(txin.scriptSig, txout.scriptPubKey, txTo, nIn, flags, nHashType); -} - static CScript PushAll(const vector& values) { CScript result; diff --git a/src/script.h b/src/script.h index 7b0643f70..f7cf7e8e9 100644 --- a/src/script.h +++ b/src/script.h @@ -673,9 +673,7 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std::vector& addressRet, int& nRequiredRet); bool SignSignature(const CKeyStore& keystore, const CScript& fromPubKey, CTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); -bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, - unsigned int flags, int nHashType); -bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType); +bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType); // Given two sets of signatures for scriptPubKey, possibly with OP_0 placeholders, // combine them intelligently and return the result. From 2800ce7367760a9bb8fd5209feddf71e574a592d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 1 Dec 2012 22:30:06 +0100 Subject: [PATCH 2/5] Add CScriptCheck: a closure representing a script check --- src/main.cpp | 15 ++++++++------- src/main.h | 27 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index c0446370a..4441c65fd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1348,15 +1348,16 @@ bool CTransaction::HaveInputs(CCoinsViewCache &inputs) const return true; } +bool CScriptCheck::operator()() const { + const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; + if (!VerifyScript(scriptSig, scriptPubKey, *ptxTo, nIn, nFlags, nHashType)) + return error("CScriptCheck() : %s VerifySignature failed", ptxTo->GetHash().ToString().substr(0,10).c_str()); + return true; +} + bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType) { - assert(nIn < txTo.vin.size()); - const CTxIn& txin = txTo.vin[nIn]; - if (txin.prevout.n >= txFrom.vout.size()) - return false; - const CTxOut& txout = txFrom.vout[txin.prevout.n]; - - return VerifyScript(txin.scriptSig, txout.scriptPubKey, txTo, nIn, flags, nHashType); + return CScriptCheck(txFrom, txTo, nIn, flags, nHashType)(); } bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmode, unsigned int flags) const diff --git a/src/main.h b/src/main.h index a098cc775..83454e331 100644 --- a/src/main.h +++ b/src/main.h @@ -430,7 +430,6 @@ enum GetMinFee_mode GMF_SEND, }; -// Modes for script/signature checking enum CheckSig_mode { CS_NEVER, // never validate scripts @@ -1015,7 +1014,33 @@ public: } }; +/** Closure representing one script verification + * Note that this stores references to the spending transaction */ +class CScriptCheck +{ +private: + CScript scriptPubKey; + const CTransaction *ptxTo; + unsigned int nIn; + unsigned int nFlags; + int nHashType; + +public: + CScriptCheck() {} + CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, int nHashTypeIn) : + scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), + ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), nHashType(nHashTypeIn) { } + + bool operator()() const; + void swap(CScriptCheck &check) { + scriptPubKey.swap(check.scriptPubKey); + std::swap(ptxTo, check.ptxTo); + std::swap(nIn, check.nIn); + std::swap(nFlags, check.nFlags); + std::swap(nHashType, check.nHashType); + } +}; /** A transaction with a merkle branch linking it to the block chain. */ class CMerkleTx : public CTransaction From 1d70f4bde8f6adc4df65397f486186a694a74c60 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 1 Dec 2012 22:51:10 +0100 Subject: [PATCH 3/5] Remove CheckSig_mode and move logic out of CheckInputs() --- src/main.cpp | 13 +++++++------ src/main.h | 9 +-------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 4441c65fd..7b37d0644 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -772,7 +772,7 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. - if (!tx.CheckInputs(view, CS_ALWAYS, SCRIPT_VERIFY_P2SH)) + if (!tx.CheckInputs(view, true, SCRIPT_VERIFY_P2SH)) { return error("CTxMemPool::accept() : ConnectInputs failed %s", hash.ToString().substr(0,10).c_str()); } @@ -1360,7 +1360,7 @@ bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned in return CScriptCheck(txFrom, txTo, nIn, flags, nHashType)(); } -bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmode, unsigned int flags) const +bool CTransaction::CheckInputs(CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags) const { if (!IsCoinBase()) { @@ -1410,8 +1410,7 @@ bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmod // Skip ECDSA signature verification when connecting blocks // before the last block chain checkpoint. This is safe because block merkle hashes are // still computed and checked, and any change will be caught at the next checkpoint. - if (csmode == CS_ALWAYS || - (csmode == CS_AFTER_CHECKPOINT && inputs.GetBestBlock()->nHeight >= Checkpoints::GetTotalBlocksEstimate())) { + if (fScriptChecks) { for (unsigned int i = 0; i < vin.size(); i++) { const COutPoint &prevout = vin[i].prevout; const CCoins &coins = inputs.GetCoins(prevout.hash); @@ -1577,6 +1576,8 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust // verify that the view's current state corresponds to the previous block assert(pindex->pprev == view.GetBestBlock()); + bool fScriptChecks = pindex->nHeight >= Checkpoints::GetTotalBlocksEstimate(); + // Do not allow blocks that contain transactions which 'overwrite' older transactions, // unless those are already completely spent. // If such overwrites are allowed, coinbases and transactions depending upon those @@ -1637,7 +1638,7 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust nFees += tx.GetValueIn(view)-tx.GetValueOut(); - if (!tx.CheckInputs(view, CS_AFTER_CHECKPOINT, fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE)) + if (!tx.CheckInputs(view, fScriptChecks, fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE)) return false; } @@ -3924,7 +3925,7 @@ CBlock* CreateNewBlock(CReserveKey& reservekey) if (nBlockSigOps + nTxSigOps >= MAX_BLOCK_SIGOPS) continue; - if (!tx.CheckInputs(viewTemp, CS_ALWAYS, SCRIPT_VERIFY_P2SH)) + if (!tx.CheckInputs(viewTemp, true, SCRIPT_VERIFY_P2SH)) continue; CTxUndo txundo; diff --git a/src/main.h b/src/main.h index 83454e331..abd3eaf7d 100644 --- a/src/main.h +++ b/src/main.h @@ -430,13 +430,6 @@ enum GetMinFee_mode GMF_SEND, }; -enum CheckSig_mode -{ - CS_NEVER, // never validate scripts - CS_AFTER_CHECKPOINT, // validate scripts after the last checkpoint - CS_ALWAYS // always validate scripts -}; - /** The basic transaction that is broadcasted on the network and contained in * blocks. A transaction can contain multiple inputs and outputs. */ @@ -641,7 +634,7 @@ public: // Check whether all inputs of this transaction are valid (no double spends, scripts & sigs, amounts) // This does not modify the UTXO set - bool CheckInputs(CCoinsViewCache &view, enum CheckSig_mode csmode, unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC) const; + bool CheckInputs(CCoinsViewCache &view, bool fScriptChecks = true, unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC) const; // Apply the effects of this transaction on the UTXO set represented by view bool UpdateCoins(CCoinsViewCache &view, CTxUndo &txundo, int nHeight, const uint256 &txhash) const; From f9cae832e6f56c6abe89b3bf05d1f176c2a7c913 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 1 Dec 2012 23:04:14 +0100 Subject: [PATCH 4/5] Parallelize script verification * During block verification (when parallelism is requested), script check actions are stored instead of being executed immediately. * After every processed transactions, its signature actions are pushed to a CScriptCheckQueue, which maintains a queue and some synchronization mechanism. * Two or more threads (if enabled) start processing elements from this queue, * When the block connection code is finished processing transactions, it joins the worker pool until the queue is empty. As cs_main is held the entire time, and all verification must be finished before the block continues processing, this does not reach the best possible performance. It is a less drastic change than some more advanced mechanisms (like doing verification out-of-band entirely, and rolling back blocks when a failure is detected). The -par=N flag controls the number of threads (1-16). 0 means auto, and is the default. --- bitcoin-qt.pro | 1 + src/checkqueue.h | 206 ++++++++++++++++++++++++++++++++++++++ src/init.cpp | 20 ++++ src/main.cpp | 40 +++++++- src/main.h | 15 ++- src/net.h | 1 + src/test/test_bitcoin.cpp | 4 + 7 files changed, 281 insertions(+), 6 deletions(-) create mode 100644 src/checkqueue.h diff --git a/bitcoin-qt.pro b/bitcoin-qt.pro index 2ca142add..31780b5b8 100644 --- a/bitcoin-qt.pro +++ b/bitcoin-qt.pro @@ -156,6 +156,7 @@ HEADERS += src/qt/bitcoingui.h \ src/init.h \ src/irc.h \ src/mruset.h \ + src/checkqueue.h \ src/json/json_spirit_writer_template.h \ src/json/json_spirit_writer.h \ src/json/json_spirit_value.h \ diff --git a/src/checkqueue.h b/src/checkqueue.h new file mode 100644 index 000000000..36141dd74 --- /dev/null +++ b/src/checkqueue.h @@ -0,0 +1,206 @@ +// Copyright (c) 2012 The Bitcoin developers +// Distributed under the MIT/X11 software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#ifndef CHECKQUEUE_H +#define CHECKQUEUE_H + +#include +#include +#include + +#include +#include + +template class CCheckQueueControl; + +/** Queue for verifications that have to be performed. + * The verifications are represented by a type T, which must provide an + * operator(), returning a bool. + * + * One thread (the master) is assumed to push batches of verifications + * onto the queue, where they are processed by N-1 worker threads. When + * the master is done adding work, it temporarily joins the worker pool + * as an N'th worker, until all jobs are done. + */ +template class CCheckQueue { +private: + // Mutex to protect the inner state + boost::mutex mutex; + + // Worker threads block on this when out of work + boost::condition_variable condWorker; + + // Master thread blocks on this when out of work + boost::condition_variable condMaster; + + // Quit method blocks on this until all workers are gone + boost::condition_variable condQuit; + + // The queue of elements to be processed. + // As the order of booleans doesn't matter, it is used as a LIFO (stack) + std::vector queue; + + // The number of workers (including the master) that are idle. + int nIdle; + + // The total number of workers (including the master). + int nTotal; + + // The temporary evaluation result. + bool fAllOk; + + // Number of verifications that haven't completed yet. + // This includes elements that are not anymore in queue, but still in + // worker's own batches. + unsigned int nTodo; + + // Whether we're shutting down. + bool fQuit; + + // The maximum number of elements to be processed in one batch + unsigned int nBatchSize; + + // Internal function that does bulk of the verification work. + bool Loop(bool fMaster = false) { + boost::condition_variable &cond = fMaster ? condMaster : condWorker; + std::vector vChecks; + vChecks.reserve(nBatchSize); + unsigned int nNow = 0; + bool fOk = true; + do { + { + boost::unique_lock lock(mutex); + // first do the clean-up of the previous loop run (allowing us to do it in the same critsect) + if (nNow) { + fAllOk &= fOk; + nTodo -= nNow; + if (nTodo == 0 && !fMaster) + // We processed the last element; inform the master he can exit and return the result + condMaster.notify_one(); + } else { + // first iteration + nTotal++; + } + // logically, the do loop starts here + while (queue.empty()) { + if ((fMaster || fQuit) && nTodo == 0) { + nTotal--; + if (nTotal==0) + condQuit.notify_one(); + bool fRet = fAllOk; + // reset the status for new work later + if (fMaster) + fAllOk = true; + // return the current status + return fRet; + } + nIdle++; + cond.wait(lock); // wait + nIdle--; + } + // Decide how many work units to process now. + // * Do not try to do everything at once, but aim for increasingly smaller batches so + // all workers finish approximately simultaneously. + // * Try to account for idle jobs which will instantly start helping. + // * Don't do batches smaller than 1 (duh), or larger than nBatchSize. + nNow = std::max(1U, std::min(nBatchSize, (unsigned int)queue.size() / (nTotal + nIdle + 1))); + vChecks.resize(nNow); + for (unsigned int i = 0; i < nNow; i++) { + // We want the lock on the mutex to be as short as possible, so swap jobs from the global + // queue to the local batch vector instead of copying. + vChecks[i].swap(queue.back()); + queue.pop_back(); + } + // Check whether we need to do work at all + fOk = fAllOk; + } + // execute work + BOOST_FOREACH(T &check, vChecks) + if (fOk) + fOk = check(); + vChecks.clear(); + } while(true); + } + +public: + // Create a new check queue + CCheckQueue(unsigned int nBatchSizeIn) : + nIdle(0), nTotal(0), fAllOk(true), nTodo(0), fQuit(false), nBatchSize(nBatchSizeIn) {} + + // Worker thread + void Thread() { + Loop(); + } + + // Wait until execution finishes, and return whether all evaluations where succesful. + bool Wait() { + return Loop(true); + } + + // Add a batch of checks to the queue + void Add(std::vector &vChecks) { + boost::unique_lock lock(mutex); + BOOST_FOREACH(T &check, vChecks) { + queue.push_back(T()); + check.swap(queue.back()); + } + nTodo += vChecks.size(); + if (vChecks.size() == 1) + condWorker.notify_one(); + else if (vChecks.size() > 1) + condWorker.notify_all(); + } + + // Shut the queue down + void Quit() { + boost::unique_lock lock(mutex); + fQuit = true; + // No need to wake the master, as he will quit automatically when all jobs are + // done. + condWorker.notify_all(); + + while (nTotal > 0) + condQuit.wait(lock); + } + + friend class CCheckQueueControl; +}; + +/** RAII-style controller object for a CCheckQueue that guarantees the passed + * queue is finished before continuing. + */ +template class CCheckQueueControl { +private: + CCheckQueue *pqueue; + bool fDone; + +public: + CCheckQueueControl(CCheckQueue *pqueueIn) : pqueue(pqueueIn), fDone(false) { + // passed queue is supposed to be unused, or NULL + if (pqueue != NULL) { + assert(pqueue->nTotal == pqueue->nIdle); + assert(pqueue->nTodo == 0); + assert(pqueue->fAllOk == true); + } + } + + bool Wait() { + if (pqueue == NULL) + return true; + bool fRet = pqueue->Wait(); + fDone = true; + return fRet; + } + + void Add(std::vector &vChecks) { + if (pqueue != NULL) + pqueue->Add(vChecks); + } + + ~CCheckQueueControl() { + if (!fDone) + Wait(); + } +}; + +#endif diff --git a/src/init.cpp b/src/init.cpp index 557c23fc9..087750cf9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -84,6 +84,10 @@ void Shutdown(void* parg) fShutdown = true; nTransactionsUpdated++; bitdb.Flush(false); + { + LOCK(cs_main); + ThreadScriptCheckQuit(); + } StopNode(); { LOCK(cs_main); @@ -303,6 +307,7 @@ std::string HelpMessage() " -checklevel= " + _("How thorough the block verification is (0-6, default: 1)") + "\n" + " -loadblock= " + _("Imports blocks from external blk000??.dat file") + "\n" + " -reindex " + _("Rebuild blockchain index from current blk000??.dat files") + "\n" + + " -par=N " + _("Set the number of script verification threads (1-16, 0=auto, default: 0)") + "\n" + "\n" + _("Block creation options:") + "\n" + " -blockminsize= " + _("Set minimum block size in bytes (default: 0)") + "\n" + @@ -484,6 +489,15 @@ bool AppInit2() fDebug = GetBoolArg("-debug"); fBenchmark = GetBoolArg("-benchmark"); + // -par=0 means autodetect, but nScriptCheckThreads==0 means no concurrency + nScriptCheckThreads = GetArg("-par", 0); + if (nScriptCheckThreads == 0) + nScriptCheckThreads = boost::thread::hardware_concurrency(); + if (nScriptCheckThreads <= 1) + nScriptCheckThreads = 0; + else if (nScriptCheckThreads > MAX_SCRIPTCHECK_THREADS) + nScriptCheckThreads = MAX_SCRIPTCHECK_THREADS; + // -debug implies fDebug* if (fDebug) fDebugNet = true; @@ -579,6 +593,12 @@ bool AppInit2() if (fDaemon) fprintf(stdout, "Bitcoin server starting\n"); + if (nScriptCheckThreads) { + printf("Using %u threads for script verification\n", nScriptCheckThreads); + for (int i=0; i #include #include @@ -40,6 +41,7 @@ uint256 hashBestChain = 0; CBlockIndex* pindexBest = NULL; set setBlockIndexValid; // may contain all CBlockIndex*'s that have validness >=BLOCK_VALID_TRANSACTIONS, and must contain those who aren't failed int64 nTimeBestReceived = 0; +int nScriptCheckThreads = 0; bool fImporting = false; bool fReindex = false; bool fBenchmark = false; @@ -1360,10 +1362,13 @@ bool VerifySignature(const CCoins& txFrom, const CTransaction& txTo, unsigned in return CScriptCheck(txFrom, txTo, nIn, flags, nHashType)(); } -bool CTransaction::CheckInputs(CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags) const +bool CTransaction::CheckInputs(CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, std::vector *pvChecks) const { if (!IsCoinBase()) { + if (pvChecks) + pvChecks->reserve(vin.size()); + // This doesn't trigger the DoS code on purpose; if it did, it would make it easier // for an attacker to attempt to split the network. if (!HaveInputs(inputs)) @@ -1416,8 +1421,12 @@ bool CTransaction::CheckInputs(CCoinsViewCache &inputs, bool fScriptChecks, unsi const CCoins &coins = inputs.GetCoins(prevout.hash); // Verify signature - if (!VerifySignature(coins, *this, i, flags, 0)) - return DoS(100,error("CheckInputs() : %s VerifySignature failed", GetHash().ToString().substr(0,10).c_str())); + CScriptCheck check(coins, *this, i, flags, 0); + if (pvChecks) { + pvChecks->push_back(CScriptCheck()); + check.swap(pvChecks->back()); + } else if (!check()) + return DoS(100,false); } } } @@ -1567,6 +1576,19 @@ void static FlushBlockFile() bool FindUndoPos(int nFile, CDiskBlockPos &pos, unsigned int nAddSize); +static CCheckQueue scriptcheckqueue(128); + +void ThreadScriptCheck(void*) { + vnThreadsRunning[THREAD_SCRIPTCHECK]++; + RenameThread("bitcoin-scriptch"); + scriptcheckqueue.Thread(); + vnThreadsRunning[THREAD_SCRIPTCHECK]--; +} + +void ThreadScriptCheckQuit() { + scriptcheckqueue.Quit(); +} + bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJustCheck) { // Check it again in case a previous version let a bad block in @@ -1607,6 +1629,8 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust CBlockUndo blockundo; + CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); + int64 nStart = GetTimeMicros(); int64 nFees = 0; int nInputs = 0; @@ -1638,8 +1662,10 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust nFees += tx.GetValueIn(view)-tx.GetValueOut(); - if (!tx.CheckInputs(view, fScriptChecks, fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE)) + std::vector vChecks; + if (!tx.CheckInputs(view, fScriptChecks, fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, nScriptCheckThreads ? &vChecks : NULL)) return false; + control.Add(vChecks); } CTxUndo txundo; @@ -1656,6 +1682,12 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust if (vtx[0].GetValueOut() > GetBlockValue(pindex->nHeight, nFees)) return error("ConnectBlock() : coinbase pays too much (actual=%"PRI64d" vs limit=%"PRI64d")", vtx[0].GetValueOut(), GetBlockValue(pindex->nHeight, nFees)); + if (!control.Wait()) + return DoS(100, false); + int64 nTime2 = GetTimeMicros() - nStart; + if (fBenchmark) + printf("- Verify %u txins: %.2fms (%.3fms/txin)\n", nInputs - 1, 0.001 * nTime2, nInputs <= 1 ? 0 : 0.001 * nTime2 / (nInputs-1)); + if (fJustCheck) return true; diff --git a/src/main.h b/src/main.h index abd3eaf7d..3b7621959 100644 --- a/src/main.h +++ b/src/main.h @@ -53,6 +53,8 @@ inline bool MoneyRange(int64 nValue) { return (nValue >= 0 && nValue <= MAX_MONE static const int COINBASE_MATURITY = 100; /** Threshold for nLockTime: below this value it is interpreted as block number, otherwise as UNIX timestamp. */ static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC +/** Maximum number of script-checking threads allowed */ +static const int MAX_SCRIPTCHECK_THREADS = 16; #ifdef USE_UPNP static const int fHaveUPnP = true; #else @@ -90,6 +92,7 @@ extern unsigned char pchMessageStart[4]; extern bool fImporting; extern bool fReindex; extern bool fBenchmark; +extern int nScriptCheckThreads; extern unsigned int nCoinCacheSize; // Settings @@ -107,6 +110,7 @@ class CCoins; class CTxUndo; class CCoinsView; class CCoinsViewCache; +class CScriptCheck; /** Register a wallet to receive updates from core */ void RegisterWallet(CWallet* pwalletIn); @@ -136,6 +140,10 @@ bool ProcessMessages(CNode* pfrom); bool SendMessages(CNode* pto, bool fSendTrickle); /** Run the importer thread, which deals with reindexing, loading bootstrap.dat, and whatever is passed to -loadblock */ void ThreadImport(void *parg); +/** Run an instance of the script checking thread */ +void ThreadScriptCheck(void* parg); +/** Stop the script checking threads */ +void ThreadScriptCheckQuit(); /** Run the miner threads */ void GenerateBitcoins(bool fGenerate, CWallet* pwallet); /** Generate a new block, without valid proof-of-work */ @@ -633,8 +641,11 @@ public: bool HaveInputs(CCoinsViewCache &view) const; // Check whether all inputs of this transaction are valid (no double spends, scripts & sigs, amounts) - // This does not modify the UTXO set - bool CheckInputs(CCoinsViewCache &view, bool fScriptChecks = true, unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC) const; + // This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it + // instead of being performed inline. + bool CheckInputs(CCoinsViewCache &view, bool fScriptChecks = true, + unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, + std::vector *pvChecks = NULL) const; // Apply the effects of this transaction on the UTXO set represented by view bool UpdateCoins(CCoinsViewCache &view, CTxUndo &txundo, int nHeight, const uint256 &txhash) const; diff --git a/src/net.h b/src/net.h index e37953772..81caf541b 100644 --- a/src/net.h +++ b/src/net.h @@ -82,6 +82,7 @@ enum threadId THREAD_DUMPADDRESS, THREAD_RPCHANDLER, THREAD_IMPORT, + THREAD_SCRIPTCHECK, THREAD_MAX }; diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index b98816d53..f75b762f1 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -33,9 +33,13 @@ struct TestingSetup { pwalletMain = new CWallet("wallet.dat"); pwalletMain->LoadWallet(fFirstRun); RegisterWallet(pwalletMain); + nScriptCheckThreads = 3; + for (int i=0; i < nScriptCheckThreads-1; i++) + NewThread(ThreadScriptCheck, NULL); } ~TestingSetup() { + ThreadScriptCheckQuit(); delete pwalletMain; pwalletMain = NULL; delete pcoinsTip; From ef0f422519de4a3ce47d923e5f8f90cd12349f3e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 8 Dec 2012 22:49:04 +0100 Subject: [PATCH 5/5] Remove contention on signature cache during block validation Since block validation happens in parallel, multiple threads may be accessing the signature cache simultaneously. To prevent contention: * Turn the signature cache lock into a shared mutex * Make reading from the cache only acquire a shared lock * Let block validations not store their results in the cache --- src/main.cpp | 5 ++++- src/script.cpp | 20 +++++++++++--------- src/script.h | 1 + 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 18a1c59e1..92be0cfd3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1627,6 +1627,9 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust int64 nBIP16SwitchTime = 1333238400; bool fStrictPayToScriptHash = (pindex->nTime >= nBIP16SwitchTime); + unsigned int flags = SCRIPT_VERIFY_NOCACHE | + (fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE); + CBlockUndo blockundo; CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); @@ -1663,7 +1666,7 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust nFees += tx.GetValueIn(view)-tx.GetValueOut(); std::vector vChecks; - if (!tx.CheckInputs(view, fScriptChecks, fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, nScriptCheckThreads ? &vChecks : NULL)) + if (!tx.CheckInputs(view, fScriptChecks, flags, nScriptCheckThreads ? &vChecks : NULL)) return false; control.Add(vChecks); } diff --git a/src/script.cpp b/src/script.cpp index 52a8f2c11..70adf1f9d 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -16,7 +16,7 @@ using namespace boost; #include "sync.h" #include "util.h" -bool CheckSig(vector vchSig, vector vchPubKey, CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType); +bool CheckSig(vector vchSig, vector vchPubKey, CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, int flags); @@ -1007,7 +1007,7 @@ bool EvalScript(vector >& stack, const CScript& script, co bool fSuccess = (!fStrictEncodings || (IsCanonicalSignature(vchSig) && IsCanonicalPubKey(vchPubKey))); if (fSuccess) - fSuccess = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType); + fSuccess = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags); popstack(stack); popstack(stack); @@ -1069,7 +1069,7 @@ bool EvalScript(vector >& stack, const CScript& script, co // Check signature bool fOk = (!fStrictEncodings || (IsCanonicalSignature(vchSig) && IsCanonicalPubKey(vchPubKey))); if (fOk) - fOk = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType); + fOk = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags); if (fOk) { isig++; @@ -1199,13 +1199,13 @@ private: // sigdata_type is (signature hash, signature, public key): typedef boost::tuple, std::vector > sigdata_type; std::set< sigdata_type> setValid; - CCriticalSection cs_sigcache; + boost::shared_mutex cs_sigcache; public: bool Get(uint256 hash, const std::vector& vchSig, const std::vector& pubKey) { - LOCK(cs_sigcache); + boost::shared_lock lock(cs_sigcache); sigdata_type k(hash, vchSig, pubKey); std::set::iterator mi = setValid.find(k); @@ -1223,7 +1223,7 @@ public: int64 nMaxCacheSize = GetArg("-maxsigcachesize", 50000); if (nMaxCacheSize <= 0) return; - LOCK(cs_sigcache); + boost::unique_lock lock(cs_sigcache); while (static_cast(setValid.size()) > nMaxCacheSize) { @@ -1246,7 +1246,7 @@ public: }; bool CheckSig(vector vchSig, vector vchPubKey, CScript scriptCode, - const CTransaction& txTo, unsigned int nIn, int nHashType) + const CTransaction& txTo, unsigned int nIn, int nHashType, int flags) { static CSignatureCache signatureCache; @@ -1271,7 +1271,9 @@ bool CheckSig(vector vchSig, vector vchPubKey, CSc if (!key.Verify(sighash, vchSig)) return false; - signatureCache.Set(sighash, vchSig, vchPubKey); + if (!(flags & SCRIPT_VERIFY_NOCACHE)) + signatureCache.Set(sighash, vchSig, vchPubKey); + return true; } @@ -1761,7 +1763,7 @@ static CScript CombineMultisig(CScript scriptPubKey, const CTransaction& txTo, u if (sigs.count(pubkey)) continue; // Already got a sig for this pubkey - if (CheckSig(sig, pubkey, scriptPubKey, txTo, nIn, 0)) + if (CheckSig(sig, pubkey, scriptPubKey, txTo, nIn, 0, 0)) { sigs[pubkey] = sig; break; diff --git a/src/script.h b/src/script.h index f7cf7e8e9..ae316b33c 100644 --- a/src/script.h +++ b/src/script.h @@ -32,6 +32,7 @@ enum SCRIPT_VERIFY_NONE = 0, SCRIPT_VERIFY_P2SH = (1U << 0), SCRIPT_VERIFY_STRICTENC = (1U << 1), + SCRIPT_VERIFY_NOCACHE = (1U << 2), }; enum txnouttype