Optimization: Minimize the number of times it is checked that no money is created

by individual transactions to 2 places (but call only once in each):

- ConnectBlock ( before calculated fees per txs twice )
- AcceptToMemoryPoolWorker ( before called CheckTxInputs 4 times and calculated
   fees per tx one extra time )

Also call tx.GetValueOut() only once per call of CheckTxInputs (instead of 2)
This commit is contained in:
Jorge Timón 2016-08-11 04:42:36 +02:00
parent 3f0ee3e501
commit 832e0744cb
No known key found for this signature in database
GPG key ID: 8866C18EA1C944A2
4 changed files with 31 additions and 30 deletions

View file

@ -205,16 +205,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
return true;
}
bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight)
bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee)
{
// 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.
// are the actual inputs available?
if (!inputs.HaveInputs(tx)) {
return state.Invalid(false, 0, "", "Inputs unavailable");
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", false,
strprintf("%s: inputs missing/spent", __func__));
}
CAmount nValueIn = 0;
CAmount nFees = 0;
for (unsigned int i = 0; i < tx.vin.size(); ++i) {
const COutPoint &prevout = tx.vin[i].prevout;
const Coin& coin = inputs.AccessCoin(prevout);
@ -234,19 +233,18 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c
}
}
if (nValueIn < tx.GetValueOut()) {
const CAmount value_out = tx.GetValueOut();
if (nValueIn < value_out) {
return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false,
strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(tx.GetValueOut())));
strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(value_out)));
}
// Tally transaction fees
CAmount nTxFee = nValueIn - tx.GetValueOut();
if (nTxFee < 0) {
return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative");
}
nFees += nTxFee;
if (!MoneyRange(nFees)) {
const CAmount txfee_aux = nValueIn - value_out;
if (!MoneyRange(txfee_aux)) {
return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange");
}
txfee = txfee_aux;
return true;
}

View file

@ -5,6 +5,8 @@
#ifndef BITCOIN_CONSENSUS_TX_VERIFY_H
#define BITCOIN_CONSENSUS_TX_VERIFY_H
#include "amount.h"
#include <stdint.h>
#include <vector>
@ -22,9 +24,10 @@ 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.
* @param[out] txfee Set to the transaction fee if successful.
* Preconditions: tx.IsCoinBase() is false.
*/
bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight);
bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee);
} // namespace Consensus
/** Auxiliary functions for transaction validation (ideally should not be exposed) */

View file

@ -625,7 +625,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
uint64_t innerUsage = 0;
CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(pcoins));
const int64_t nSpendHeight = GetSpendHeight(mempoolDuplicate);
const int64_t spendheight = GetSpendHeight(mempoolDuplicate);
LOCK(cs);
std::list<const CTxMemPoolEntry*> waitingOnDependants;
@ -705,8 +705,9 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
waitingOnDependants.push_back(&(*it));
else {
CValidationState state;
CAmount txfee = 0;
bool fCheckResult = tx.IsCoinBase() ||
Consensus::CheckTxInputs(tx, state, mempoolDuplicate, nSpendHeight);
Consensus::CheckTxInputs(tx, state, mempoolDuplicate, spendheight, txfee);
assert(fCheckResult);
UpdateCoins(tx, mempoolDuplicate, 1000000);
}
@ -721,8 +722,9 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
stepsSinceLastRemove++;
assert(stepsSinceLastRemove < waitingOnDependants.size());
} else {
CAmount txfee = 0;
bool fCheckResult = entry->GetTx().IsCoinBase() ||
Consensus::CheckTxInputs(entry->GetTx(), state, mempoolDuplicate, nSpendHeight);
Consensus::CheckTxInputs(entry->GetTx(), state, mempoolDuplicate, spendheight, txfee);
assert(fCheckResult);
UpdateCoins(entry->GetTx(), mempoolDuplicate, 1000000);
stepsSinceLastRemove = 0;

View file

@ -484,7 +484,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
CCoinsView dummy;
CCoinsViewCache view(&dummy);
CAmount nValueIn = 0;
LockPoints lp;
{
LOCK(pool.cs);
@ -519,8 +518,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// Bring the best block into scope
view.GetBestBlock();
nValueIn = view.GetValueIn(tx);
// we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool
view.SetBackend(dummy);
@ -531,6 +528,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// CoinsViewCache instead of create its own
if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");
} // end LOCK(pool.cs)
CAmount nFees = 0;
if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) {
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
}
// Check for non-standard pay-to-script-hash in inputs
@ -543,8 +546,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS);
CAmount nValueOut = tx.GetValueOut();
CAmount nFees = nValueIn-nValueOut;
// nModifiedFees includes any fee deltas from PrioritiseTransaction
CAmount nModifiedFees = nFees;
pool.ApplyDelta(hash, nModifiedFees);
@ -1161,9 +1162,6 @@ static bool CheckInputs(const CTransaction& tx, CValidationState &state, const C
{
if (!tx.IsCoinBase())
{
if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs)))
return false;
if (pvChecks)
pvChecks->reserve(tx.vin.size());
@ -1635,9 +1633,11 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
if (!tx.IsCoinBase())
{
if (!view.HaveInputs(tx))
return state.DoS(100, error("ConnectBlock(): inputs missing/spent"),
REJECT_INVALID, "bad-txns-inputs-missingorspent");
CAmount txfee = 0;
if (!Consensus::CheckTxInputs(tx, state, view, pindex->nHeight, txfee)) {
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
}
nFees += txfee;
// Check that transaction is BIP68 final
// BIP68 lock checks (as opposed to nLockTime checks) must
@ -1665,8 +1665,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
txdata.emplace_back(tx);
if (!tx.IsCoinBase())
{
nFees += view.GetValueIn(tx)-tx.GetValueOut();
std::vector<CScriptCheck> 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], nScriptCheckThreads ? &vChecks : NULL))