diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 6fa9fe4fe8..e734433dca 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -22,6 +22,7 @@ static const int PREVECTOR_SIZE = 28; static const int QUEUE_BATCH_SIZE = 128; static void CCheckQueueSpeed(benchmark::State& state) { + /* FIXME struct FakeJobNoWork { bool operator()() { @@ -55,7 +56,7 @@ static void CCheckQueueSpeed(benchmark::State& state) control.Wait(); } tg.interrupt_all(); - tg.join_all(); + tg.join_all();*/ } // This Benchmark tests the CheckQueue with a slightly realistic workload, @@ -63,6 +64,7 @@ static void CCheckQueueSpeed(benchmark::State& state) // and there is a little bit of work done between calls to Add. static void CCheckQueueSpeedPrevectorJob(benchmark::State& state) { + /* FIXME struct PrevectorJob { prevector p; PrevectorJob(){ @@ -81,6 +83,7 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::State& state) for (auto x = 0; x < std::max(MIN_CORES, GetNumCores()); ++x) { tg.create_thread([&]{queue.Thread();}); } + while (state.KeepRunning()) { // Make insecure_rand here so that each iteration is identical. FastRandomContext insecure_rand(true); @@ -97,7 +100,7 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::State& state) control.Wait(); } tg.interrupt_all(); - tg.join_all(); + tg.join_all();*/ } BENCHMARK(CCheckQueueSpeed); BENCHMARK(CCheckQueueSpeedPrevectorJob); 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/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index ef95fd5f26..07982b9e57 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -481,10 +481,10 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { coins.vout.push_back(txout); } - CScriptCheck checks[mtx.vin.size()]; + CScriptCheck* checks[mtx.vin.size()]; for(uint32_t i = 0; i < mtx.vin.size(); i++) { - std::vector vChecks; - checks[i] = CScriptCheck(coins, tx, i, -1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); + std::vector vChecks; + checks[i] = new CScriptCheck(coins, tx, i, -1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); vChecks.push_back(checks[i]); control.Add(vChecks); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 58cec10341..6a3ed559c1 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -759,7 +759,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const CValidationState state; std::set > setWithdrawsSpent; bool fCheckResult = tx.IsCoinBase() || - Consensus::CheckTxInputs(tx, state, mempoolDuplicate, nSpendHeight, setWithdrawsSpent); + Consensus::CheckTxInputs(tx, state, mempoolDuplicate, nSpendHeight, setWithdrawsSpent, NULL); assert(fCheckResult); UpdateCoins(tx, mempoolDuplicate, 1000000); assert(setWithdrawsSpent == it->setWithdrawsSpent); @@ -780,7 +780,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const } else { std::set > setWithdrawsSpent; bool fCheckResult = entry->GetTx().IsCoinBase() || - Consensus::CheckTxInputs(entry->GetTx(), state, mempoolDuplicate, nSpendHeight, setWithdrawsSpent); + Consensus::CheckTxInputs(entry->GetTx(), state, mempoolDuplicate, nSpendHeight, setWithdrawsSpent, NULL); assert(fCheckResult); UpdateCoins(entry->GetTx(), mempoolDuplicate, 1000000); assert(setWithdrawsSpent == entry->setWithdrawsSpent); diff --git a/src/validation.cpp b/src/validation.cpp index 2a88bfdcec..25f5bc7153 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -545,10 +545,9 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe 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 { @@ -567,12 +566,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; @@ -623,21 +696,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; @@ -1634,7 +1711,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. @@ -1686,7 +1763,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins if (!MoneyRange(nFees)) 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))); @@ -1694,11 +1771,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, std::set >& setWithdrawsSpent, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& txdata, std::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) @@ -1721,11 +1798,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 @@ -1736,7 +1810,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 @@ -1745,10 +1819,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()) @@ -1980,7 +2054,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"); @@ -2279,7 +2353,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 prevheights; CAmount nFees = 0; @@ -2335,7 +2409,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin { nFees += tx.nTxFee; - 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", diff --git a/src/validation.h b/src/validation.h index 43198e5b4d..0542b74a2d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -38,7 +38,7 @@ class CBloomFilter; class CChainParams; class CInv; class CConnman; -class CScriptCheck; +class CCheck; class CTxMemPool; class CValidationInterface; class CValidationState; @@ -374,7 +374,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); @@ -391,18 +391,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); - -} // namespace Consensus - -namespace Consensus { - -/** - * Check whether all inputs of this transaction are valid (no double spends and amounts) - * 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 @@ -412,9 +401,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); /** @@ -460,7 +450,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; @@ -470,31 +476,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; } };