diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -8,6 +8,7 @@ #include #include +class Amount; class CBlockIndex; class CCoinsViewCache; class Config; @@ -26,10 +27,13 @@ /** * 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. + * 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); + const CCoinsViewCache &inputs, int nSpendHeight, + Amount &txfee); } // namespace Consensus diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -276,15 +277,16 @@ namespace Consensus { bool CheckTxInputs(const CTransaction &tx, CValidationState &state, - const CCoinsViewCache &inputs, int nSpendHeight) { - // 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. + const CCoinsViewCache &inputs, int nSpendHeight, + Amount &txfee) { + // 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__)); } Amount nValueIn = Amount::zero(); - Amount nFees = Amount::zero(); for (const auto &in : tx.vin) { const COutPoint &prevout = in.prevout; const Coin &coin = inputs.AccessCoin(prevout); @@ -309,24 +311,21 @@ } } - if (nValueIn < tx.GetValueOut()) { + const Amount 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()))); + FormatMoney(value_out))); } // Tally transaction fees - Amount nTxFee = nValueIn - tx.GetValueOut(); - if (nTxFee < Amount::zero()) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative"); - } - - nFees += nTxFee; - if (!MoneyRange(nFees)) { + const Amount 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; } } // namespace Consensus diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -681,6 +681,18 @@ _clear(); } +static void CheckInputsAndUpdateCoins(const CTransaction &tx, + CCoinsViewCache &mempoolDuplicate, + const int64_t spendheight) { + CValidationState state; + Amount txfee = Amount::zero(); + bool fCheckResult = + tx.IsCoinBase() || Consensus::CheckTxInputs(tx, state, mempoolDuplicate, + spendheight, txfee); + assert(fCheckResult); + UpdateCoins(mempoolDuplicate, tx, 1000000); +} + void CTxMemPool::check(const CCoinsViewCache *pcoins) const { LOCK(cs); if (nCheckFrequency == 0) { @@ -699,7 +711,7 @@ uint64_t innerUsage = 0; CCoinsViewCache mempoolDuplicate(const_cast(pcoins)); - const int64_t nSpendHeight = GetSpendHeight(mempoolDuplicate); + const int64_t spendheight = GetSpendHeight(mempoolDuplicate); std::list waitingOnDependants; for (indexed_transaction_set::const_iterator it = mapTx.begin(); @@ -787,12 +799,7 @@ if (fDependsWait) { waitingOnDependants.push_back(&(*it)); } else { - CValidationState state; - bool fCheckResult = tx.IsCoinBase() || - Consensus::CheckTxInputs( - tx, state, mempoolDuplicate, nSpendHeight); - assert(fCheckResult); - UpdateCoins(mempoolDuplicate, tx, 1000000); + CheckInputsAndUpdateCoins(tx, mempoolDuplicate, spendheight); } } @@ -806,12 +813,8 @@ stepsSinceLastRemove++; assert(stepsSinceLastRemove < waitingOnDependants.size()); } else { - bool fCheckResult = - entry->GetTx().IsCoinBase() || - Consensus::CheckTxInputs(entry->GetTx(), state, - mempoolDuplicate, nSpendHeight); - assert(fCheckResult); - UpdateCoins(mempoolDuplicate, entry->GetTx(), 1000000); + CheckInputsAndUpdateCoins(entry->GetTx(), mempoolDuplicate, + spendheight); stepsSinceLastRemove = 0; } } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -523,7 +523,6 @@ CCoinsView dummy; CCoinsViewCache view(&dummy); - Amount nValueIn = Amount::zero(); LockPoints lp; CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool); view.SetBackend(viewMemPool); @@ -566,8 +565,6 @@ // 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); @@ -581,6 +578,13 @@ return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final"); } + Amount nFees = Amount::zero(); + if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), + nFees)) { + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, + tx.GetId().ToString(), FormatStateMessage(state)); + } + // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, view)) { return state.Invalid(false, REJECT_NONSTANDARD, @@ -590,8 +594,6 @@ int64_t nSigOpsCount = GetTransactionSigOpCount(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); - Amount nValueOut = tx.GetValueOut(); - Amount nFees = nValueIn - nValueOut; // nModifiedFees includes any fee deltas from PrioritiseTransaction Amount nModifiedFees = nFees; double nPriorityDummy = 0; @@ -1178,13 +1180,6 @@ std::vector *pvChecks) { assert(!tx.IsCoinBase()); - // This call does all the inexpensive checks on all the inputs. Only if ALL - // inputs pass do we perform expensive ECDSA signature checks. Helps prevent - // CPU exhaustion attacks. - if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs))) { - return false; - } - if (pvChecks) { pvChecks->reserve(tx.vin.size()); } @@ -1823,9 +1818,19 @@ continue; } - if (!view.HaveInputs(tx)) { - return state.DoS(100, error("ConnectBlock(): inputs missing/spent"), - REJECT_INVALID, "bad-txns-inputs-missingorspent"); + Amount txfee = Amount::zero(); + if (!Consensus::CheckTxInputs(tx, state, view, pindex->nHeight, + txfee)) { + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, + tx.GetId().ToString(), FormatStateMessage(state)); + } + nFees += txfee; + if (!MoneyRange(nFees)) { + return state.DoS( + 100, + error("%s: accumulated fee in the block out of range.", + __func__), + REJECT_INVALID, "bad-txns-accumulated-fee-outofrange"); } // Check that transaction is BIP68 final BIP68 lock checks (as @@ -1857,9 +1862,6 @@ REJECT_INVALID, "bad-blk-sigops"); } - Amount fee = view.GetValueIn(tx) - tx.GetValueOut(); - nFees += fee; - // Don't cache results if we're actually connecting blocks (still // consult the cache, though). bool fCacheResults = fJustCheck;