Faster duplicate input check in CheckTransaction (alternative to #14387)

src/Makefile.bench.include

@@ -18,6 +18,7 @@ bench_bench_bitcoin_SOURCES = \
   bench/block_assemble.cpp \
   bench/checkblock.cpp \
   bench/checkqueue.cpp \
+  bench/duplicate_inputs.cpp \
   bench/examples.cpp \
   bench/rollingbloom.cpp \
   bench/crypto_hash.cpp \

src/bench/duplicate_inputs.cpp

@@ -0,0 +1,104 @@
+// Copyright (c) 2011-2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <chainparams.h>
+#include <coins.h>
+#include <consensus/merkle.h>
+#include <consensus/validation.h>
+#include <miner.h>
+#include <policy/policy.h>
+#include <pow.h>
+#include <scheduler.h>
+#include <txdb.h>
+#include <txmempool.h>
+#include <utiltime.h>
+#include <validation.h>
+#include <validationinterface.h>
+
+#include <boost/thread.hpp>
+
+#include <list>
+#include <vector>
+
+
+static void DuplicateInputs(benchmark::State& state)
+{
+    const std::vector<unsigned char> op_true{OP_TRUE};
+
+
+    const CScript SCRIPT_PUB{CScript(OP_TRUE)};
+
+    // Switch to regtest so we can mine faster
+    // Also segwit is active, so we can include witness transactions
+    SelectParams(CBaseChainParams::REGTEST);
+
+    InitScriptExecutionCache();
+
+    boost::thread_group thread_group;
+    CScheduler scheduler;
+    const CChainParams& chainparams = Params();
+    {
+        ::pblocktree.reset(new CBlockTreeDB(1 << 20, true));
+        ::pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true));
+        ::pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get()));
+
+        thread_group.create_thread(boost::bind(&CScheduler::serviceQueue, &scheduler));
+        GetMainSignals().RegisterBackgroundSignalScheduler(scheduler);
+        LoadGenesisBlock(chainparams);
+        CValidationState state;
+        ActivateBestChain(state, chainparams);
+        assert(::chainActive.Tip() != nullptr);
+        const bool witness_enabled{IsWitnessEnabled(::chainActive.Tip(), chainparams.GetConsensus())};
+        assert(witness_enabled);
+    }
+
+    CBlock block{};
+    CMutableTransaction coinbaseTx{};
+    CMutableTransaction naughtyTx{};
+
+    CBlockIndex* pindexPrev = ::chainActive.Tip();
+    assert(pindexPrev != nullptr);
+    block.nBits          = GetNextWorkRequired(pindexPrev, &block, chainparams.GetConsensus());
+    block.nNonce         = 0;
+    auto nHeight = pindexPrev->nHeight + 1;
+    block.nTime = ::chainActive.Tip()->GetMedianTimePast() + 1;
+
+    // Make a coinbase TX
+    coinbaseTx.vin.resize(1);
+    coinbaseTx.vin[0].prevout.SetNull();
+    coinbaseTx.vout.resize(1);
+    coinbaseTx.vout[0].scriptPubKey = SCRIPT_PUB;
+    coinbaseTx.vout[0].nValue = GetBlockSubsidy(nHeight, chainparams.GetConsensus());
+    coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0;
+
+
+    naughtyTx.vout.resize(1);
+    naughtyTx.vout[0].nValue = 0;
+    naughtyTx.vout[0].scriptPubKey = SCRIPT_PUB;
+
+    int n_inputs = (((MAX_BLOCK_SERIALIZED_SIZE/WITNESS_SCALE_FACTOR) - (CTransaction(coinbaseTx).GetTotalSize() + CTransaction(naughtyTx).GetTotalSize()))/41) - 100;
+    for (int x = 0; x < (n_inputs-1); ++x) {
+        naughtyTx.vin.emplace_back(GetRandHash(), 0, CScript(), 0);
+    }
+    naughtyTx.vin.emplace_back(naughtyTx.vin.back());
+
+    block.vtx.push_back(MakeTransactionRef(std::move(coinbaseTx)));
+    block.vtx.push_back(MakeTransactionRef(std::move(naughtyTx)));
+
+    block.hashMerkleRoot = BlockMerkleRoot(block);
+
+    while (state.KeepRunning()) {
+        CValidationState state{};
+        assert(!CheckBlock(block, state, chainparams.GetConsensus(), false, false));
+        assert(state.GetRejectReason() == "bad-txns-inputs-duplicate");
+    }
+
+    thread_group.interrupt_all();
+    thread_group.join_all();
+    GetMainSignals().FlushBackgroundCallbacks();
+    GetMainSignals().UnregisterBackgroundSignalScheduler();
+}
+
+BENCHMARK(DuplicateInputs, 10);

src/consensus/tx_verify.cpp

@@ -182,11 +182,14 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
 
     // 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)
+        std::vector<COutPoint> outpoints;
+        outpoints.reserve(tx.vin.size());
+        for (const auto& txin : tx.vin) outpoints.push_back(txin.prevout);
+        std::sort(outpoints.begin(), outpoints.end());
+        for (size_t pos = 0; pos + 1 < outpoints.size(); ++pos) {
+            if (outpoints[pos] == outpoints[pos + 1]) {
                 return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
+            }
         }
     }