consensus: Explain why fCheckDuplicateInputs can not be skipped and remove it

2 files changed, 11 insertions, 10 deletions

diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index 1206035839..6793f871cf 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -7,7 +7,7 @@
 #include <primitives/transaction.h>
 #include <consensus/validation.h>
 
-bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
+bool CheckTransaction(const CTransaction& tx, CValidationState& state)
 {
     // Basic checks that don't depend on any context
     if (tx.vin.empty())
@@ -31,14 +31,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
             return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge");
     }
 
-    // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
-    if (fCheckDuplicateInputs) {
-        std::set<COutPoint> vInOutPoints;
-        for (const auto& txin : tx.vin)
-        {
-            if (!vInOutPoints.insert(txin.prevout).second)
-                return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
-        }
+    // Check for duplicate inputs (see CVE-2018-17144)
+    // While Consensus::CheckTxInputs does check if all inputs of a tx are available, and UpdateCoins marks all inputs
+    // of a tx as spent, it does not check if the tx has duplicate inputs.
+    // Failure to run this check will result in either a crash or an inflation bug, depending on the implementation of
+    // the underlying coins database.
+    std::set<COutPoint> vInOutPoints;
+    for (const auto& txin : tx.vin) {
+        if (!vInOutPoints.insert(txin.prevout).second)
+            return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
     }
 
     if (tx.IsCoinBase())
diff --git a/src/consensus/tx_check.h b/src/consensus/tx_check.h
index bcfdf36bf9..6f3f8fe969 100644
--- a/src/consensus/tx_check.h
+++ b/src/consensus/tx_check.h
@@ -15,6 +15,6 @@
 class CTransaction;
 class CValidationState;
 
-bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
+bool CheckTransaction(const CTransaction& tx, CValidationState& state);
 
 #endif // BITCOIN_CONSENSUS_TX_CHECK_H