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)

2 files changed, 15 insertions, 14 deletions

diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 25647ba6af..f2f85edad9 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -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;
 }
diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h
index d46d3294ca..288892462d 100644
--- a/src/consensus/tx_verify.h
+++ b/src/consensus/tx_verify.h
@@ -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) */