diff --git a/src/checkqueue.h b/src/checkqueue.h index 32e25d5c8c..9faf80b3b5 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -41,7 +41,8 @@ private: //! 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; + //! This should really be a vector of unique_ptr's, but that's C++11. + std::vector queue; //! The number of workers (including the master) that are idle. int nIdle; @@ -69,7 +70,7 @@ private: bool Loop(bool fMaster = false) { boost::condition_variable& cond = fMaster ? condMaster : condWorker; - std::vector vChecks; + std::vector vChecks; vChecks.reserve(nBatchSize); unsigned int nNow = 0; bool fOk = true; @@ -108,20 +109,18 @@ private: // * 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(); - } + vChecks.clear(); + vChecks.insert(vChecks.end(), queue.end() - nNow, queue.end()); + queue.resize(queue.size() - nNow); // Check whether we need to do work at all fOk = fAllOk; } // execute work - BOOST_FOREACH (T& check, vChecks) + BOOST_FOREACH (T* check, vChecks) { if (fOk) - fOk = check(); + fOk = (*check)(); + delete check; + } vChecks.clear(); } while (true); } @@ -142,14 +141,11 @@ public: return Loop(true); } - //! Add a batch of checks to the queue - void Add(std::vector& vChecks) + //! Add a batch of checks to the queue and takes ownership of them + void Add(const std::vector vChecks) { boost::unique_lock lock(mutex); - BOOST_FOREACH (T& check, vChecks) { - queue.push_back(T()); - check.swap(queue.back()); - } + queue.insert(queue.end(), vChecks.begin(), vChecks.end()); nTodo += vChecks.size(); if (vChecks.size() == 1) condWorker.notify_one(); @@ -159,6 +155,7 @@ public: ~CCheckQueue() { + assert(queue.empty()); } bool IsIdle() @@ -199,7 +196,7 @@ public: return fRet; } - void Add(std::vector& vChecks) + void Add(std::vector vChecks) { if (pqueue != NULL) pqueue->Add(vChecks); diff --git a/src/main.cpp b/src/main.cpp index 671de5f5d9..c5e2727ee3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1122,10 +1122,9 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state) return true; } -//static Secp256k1Ctx init_context_on_load; -//extern secp256k1_context* secp256k1_bitcoin_verify_context; +namespace { -static secp256k1_context* secp256k1_ctx_verify_amounts = NULL; +static secp256k1_context* secp256k1_ctx_verify_amounts; class Secp256k1Ctx { @@ -1144,12 +1143,86 @@ public: secp256k1_ctx_verify_amounts = NULL; } }; +static Secp256k1Ctx instance_of_secp256k1ctx; - - -bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess) +/** Closure representing one output range check. */ +class CRangeCheck : public CCheck { +private: + const CTxOutValue* val; + +public: + CRangeCheck(const CTxOutValue* val_) : val(val_) {} + + bool operator()(); +}; + +/** Closure representing a transaction amount balance check. */ +class CBalanceCheck : public CCheck +{ +private: + std::vector vchData; + std::vector vpchCommitsIn, vpchCommitsOut; + CAmount nPlainAmount; + +public: + CBalanceCheck(std::vector& vchData_, std::vector& vpchCommitsIn_, std::vector& vpchCommitsOut_, const CAmount& nPlainAmount_) : nPlainAmount(nPlainAmount_) { + vchData.swap(vchData_); + vpchCommitsIn.swap(vpchCommitsIn_); + vpchCommitsOut.swap(vpchCommitsOut_); + } + + bool operator()(); +}; + +// Destroys check, or passes its ownership to the queue. +static inline bool QueueCheck(std::vector* queue, CCheck* check) +{ + if (queue != NULL) { + queue->push_back(check); + return true; + } + bool ret = (*check)(); + delete check; + return ret; +} + + +bool CRangeCheck::operator()() +{ + if (val->IsAmount()) { + return true; + } + + uint64_t min_value, max_value; + if (!secp256k1_rangeproof_verify(secp256k1_ctx_verify_amounts, &min_value, &max_value, &val->vchCommitment[0], val->vchRangeproof.data(), val->vchRangeproof.size())) { + fAmountError = true; + return false; + } + + return true; +}; + +bool CBalanceCheck::operator()() +{ + if (!secp256k1_pedersen_verify_tally(secp256k1_ctx_verify_amounts, vpchCommitsIn.data(), vpchCommitsIn.size(), vpchCommitsOut.data(), vpchCommitsOut.size(), nPlainAmount)) { + fAmountError = true; + return false; + } + + return true; +} + +} // namespace + + + +bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess, std::vector* pvChecks) +{ + bool fNeedNoRangeProof = false; CAmount nPlainAmount = excess; + + { std::vector vchData; std::vector vpchCommitsIn, vpchCommitsOut; bool fNullRangeproof = false; @@ -1200,21 +1273,25 @@ bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const C if (vpchCommitsIn.size() + vpchCommitsOut.size() == 0) return (nPlainAmount == 0); - if (!secp256k1_pedersen_verify_tally(secp256k1_ctx_verify_amounts, vpchCommitsIn.data(), vpchCommitsIn.size(), vpchCommitsOut.data(), vpchCommitsOut.size(), nPlainAmount)) + fNeedNoRangeProof = ((!vpchCommitsIn.empty()) && vpchCommitsOut.size() == 1 && nPlainAmount <= 0 && fNullRangeproof); + + if (!QueueCheck(pvChecks, new CBalanceCheck(vchData, vpchCommitsIn, vpchCommitsOut, nPlainAmount))) { return false; + } + } // Rangeproof is optional in this case - if ((!vpchCommitsIn.empty()) && vpchCommitsOut.size() == 1 && nPlainAmount <= 0 && fNullRangeproof) + if (fNeedNoRangeProof) return true; - uint64_t min_value, max_value; for (size_t i = 0; i < tx.vout.size(); ++i) { const CTxOutValue& val = tx.vout[i].nValue; if (val.IsAmount()) continue; - if (!secp256k1_rangeproof_verify(secp256k1_ctx_verify_amounts, &min_value, &max_value, &val.vchCommitment[0], val.vchRangeproof.data(), val.vchRangeproof.size())) + if (!QueueCheck(pvChecks, new CRangeCheck(&val))) { return false; + } } return true; @@ -2211,7 +2288,7 @@ int GetSpendHeight(const CCoinsViewCache& inputs) } namespace Consensus { -bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, std::set >& setWithdrawsSpent) +bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, std::set >& setWithdrawsSpent, std::vector *pvChecks) { // 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. @@ -2259,7 +2336,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins if (!MoneyRange(nTxFee)) return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); - if (!VerifyAmounts(inputs, tx, nTxFee)) + if (!VerifyAmounts(inputs, tx, nTxFee, pvChecks)) return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false, strprintf("value in (%s) < value out", FormatMoney(nValueIn))); @@ -2267,11 +2344,11 @@ 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, PrecomputedTransactionData& txdata, set >& setWithdrawsSpent, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& txdata, set >& setWithdrawsSpent, std::vector *pvChecks) { if (!tx.IsCoinBase()) { - if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs), setWithdrawsSpent)) + if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs), setWithdrawsSpent, pvChecks)) return false; if (pvChecks) @@ -2295,11 +2372,8 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi assert(coins); // Verify signature - CScriptCheck check(*coins, tx, i, prevValueIn, flags, cacheStore, &txdata); - if (pvChecks) { - pvChecks->push_back(CScriptCheck()); - check.swap(pvChecks->back()); - } else if (!check()) { + CCheck* check = new CScriptCheck(*coins, tx, i, prevValueIn, flags, cacheStore, &txdata); + if (!QueueCheck(pvChecks, check)) { if (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) { // Check whether the failure was caused by a // non-mandatory script verification check, such as @@ -2310,7 +2384,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi CScriptCheck check2(*coins, tx, i, prevValueIn, 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()))); + return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check->GetScriptError()))); } // Failures of other flags indicate a transaction that is // invalid in new blocks, e.g. a invalid P2SH. We DoS ban @@ -2319,10 +2393,10 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // as to the correct behavior - we may want to continue // peering with non-upgraded nodes even after soft-fork // super-majority signaling has occurred. - if (check.GetScriptError() == SCRIPT_ERR_WITHDRAW_VERIFY_BLOCKCONFIRMED) - return state.Invalid(false, REJECT_SCRIPT, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); + if (check->GetScriptError() == SCRIPT_ERR_WITHDRAW_VERIFY_BLOCKCONFIRMED) + return state.Invalid(false, REJECT_SCRIPT, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check->GetScriptError()))); else - return state.DoS(100,false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); + return state.DoS(100,false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check->GetScriptError()))); } const CTxOutValue& value = coins->vout[tx.vin[i].prevout.n].nValue; if (value.IsAmount()) @@ -2554,7 +2628,7 @@ void static FlushBlockFile(bool fFinalize = false) bool FindUndoPos(CValidationState &state, int nFile, CDiskBlockPos &pos, unsigned int nAddSize); -static CCheckQueue scriptcheckqueue(128); +static CCheckQueue scriptcheckqueue(128); void ThreadScriptCheck() { RenameThread("bitcoin-scriptch"); @@ -2838,7 +2912,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CBlockUndo blockundo; - CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); + CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); std::vector vOrphanErase; std::vector prevheights; @@ -2908,7 +2982,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin if (!MoneyRange(nFees)) return state.DoS(100, error("ConnectBlock(): total tx fee overflowed"), REJECT_INVALID, "bad-txns-fee-outofrange"); - std::vector vChecks; + 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, txdata[i], setWithdrawsSpent == NULL ? setWithdrawsSpentDummy : *setWithdrawsSpent, nScriptCheckThreads ? &vChecks : NULL)) return error("ConnectBlock(): CheckInputs on %s failed with %s", @@ -2937,7 +3011,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, chainparams.GetConsensus()); if (!MoneyRange(blockReward)) return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); - if (VerifyAmounts(view, block.vtx[0], -blockReward)) + if (!VerifyAmounts(view, block.vtx[0], -blockReward)) return state.DoS(100, error("ConnectBlock(): coinbase pays too much (limit=%d)", blockReward), diff --git a/src/main.h b/src/main.h index 0343b4e4c1..ae388162b8 100644 --- a/src/main.h +++ b/src/main.h @@ -34,7 +34,7 @@ class CBlockTreeDB; class CBloomFilter; class CChainParams; class CInv; -class CScriptCheck; +class CCheck; class CTxMemPool; class CValidationInterface; class CValidationState; @@ -353,7 +353,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i */ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& txdata, std::set >& setWithdrawsSpent, - std::vector *pvChecks = NULL); + 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); @@ -368,7 +368,7 @@ namespace Consensus { * This does not modify the UTXO set. This does not check scripts and sigs. * Preconditions: tx.IsCoinBase() is false. */ -bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, std::set >& setWithdrawsSpent); +bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, std::set >& setWithdrawsSpent, std::vector *pvChecks); } // namespace Consensus @@ -378,9 +378,10 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins * @param[in] view CCoinsViewCache to find necessary outputs * @param[in] tx transaction for which we are checking totals * @param[in] excess additional amount to consider as input value (eg fees), can be negative + * @param[in] pvChecks multithreaded rangeproof and commitment checker * @return True if totals are identical */ -bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess); +bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess, std::vector* pvChecks = NULL); /** @@ -426,7 +427,23 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = NULL * Closure representing one script verification * Note that this stores references to the spending transaction */ -class CScriptCheck +class CCheck + { + protected: + ScriptError error; + bool fAmountError; + + public: + CCheck() : error(SCRIPT_ERR_UNKNOWN_ERROR), fAmountError(false) {} + virtual ~CCheck() {} + + virtual bool operator()() = 0; + + ScriptError GetScriptError() const { return error; } + bool IsAmountError() const { return fAmountError; } +}; + +class CScriptCheck : public CCheck { private: CScript scriptPubKey; @@ -436,31 +453,16 @@ private: unsigned int nIn; unsigned int nFlags; bool cacheStore; - ScriptError error; PrecomputedTransactionData *txdata; public: - CScriptCheck(): amount(0), amountPreviousInput(-1), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, const CTxOutValue& amountPreviousInputIn, 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), amountPreviousInput(amountPreviousInputIn), - ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { } + ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn) { } bool operator()(); - void swap(CScriptCheck &check) { - scriptPubKey.swap(check.scriptPubKey); - std::swap(ptxTo, check.ptxTo); - std::swap(amount, check.amount); - std::swap(amountPreviousInput, check.amountPreviousInput); - std::swap(nIn, check.nIn); - std::swap(nFlags, check.nFlags); - std::swap(cacheStore, check.cacheStore); - std::swap(error, check.error); - std::swap(txdata, check.txdata); - } - - ScriptError GetScriptError() const { return error; } }; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8948bb6734..2cfae5a017 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -497,7 +497,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { CScriptCheck *checks[mtx.vin.size()]; for(uint32_t i = 0; i < mtx.vin.size(); i++) { std::vector vChecks; - checks[i] = new CScriptCheck(coins, tx, i, 0, 0, 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); + checks[i] = new CScriptCheck(coins, tx, i, 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); vChecks.push_back(checks[i]); control.Add(vChecks); }