Merge #9049: Remove duplicatable duplicate-input check from CheckTransaction

e2b3fb3 Optimize vInOutPoints insertion a bit (Matt Corallo)
eecffe5 Remove redundant duplicate-input check from CheckTransaction (Matt Corallo)
b2e178a Add deserialize + CheckBlock benchmarks, and a full block hex (Matt Corallo)

Author: laanwj

src/Makefile.bench.include

@@ -6,11 +6,15 @@ bin_PROGRAMS += bench/bench_bitcoin
 BENCH_SRCDIR = bench
 BENCH_BINARY = bench/bench_bitcoin$(EXEEXT)
 
+RAW_TEST_FILES = \
+  bench/data/block413567.raw
+GENERATED_TEST_FILES = $(RAW_TEST_FILES:.raw=.raw.h)
 
 bench_bench_bitcoin_SOURCES = \
   bench/bench_bitcoin.cpp \
   bench/bench.cpp \
   bench/bench.h \
+  bench/checkblock.cpp \
   bench/Examples.cpp \
   bench/rollingbloom.cpp \
   bench/crypto_hash.cpp \
@@ -20,6 +24,8 @@ bench_bench_bitcoin_SOURCES = \
   bench/base58.cpp \
   bench/lockedpool.cpp
 
+nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_TEST_FILES)
+
 bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/
 bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 bench_bench_bitcoin_LDADD = \
@@ -45,14 +51,23 @@ endif
 bench_bench_bitcoin_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS)
 bench_bench_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
 
-CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno
+CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_TEST_FILES)
 
 CLEANFILES += $(CLEAN_BITCOIN_BENCH)
 
+bench/checkblock.cpp: bench/data/block413567.raw.h
+
 bitcoin_bench: $(BENCH_BINARY)
 
 bench: $(BENCH_BINARY) FORCE
 	$(BENCH_BINARY)
 
 bitcoin_bench_clean : FORCE
 	rm -f $(CLEAN_BITCOIN_BENCH) $(bench_bench_bitcoin_OBJECTS) $(BENCH_BINARY)
+
+%.raw.h: %.raw
+	@$(MKDIR_P) $(@D)
+	@echo "static unsigned const char $(*F)[] = {" >> $@
+	@$(HEXDUMP) -v -e '8/1 "0x%02x, "' -e '"\n"' $< | $(SED) -e 's/0x  ,//g' >> $@
+	@echo "};" >> $@
+	@echo "Generated $@"

src/bench/checkblock.cpp

@@ -0,0 +1,55 @@
+// Copyright (c) 2016 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.h"
+
+#include "chainparams.h"
+#include "main.h"
+#include "consensus/validation.h"
+
+namespace block_bench {
+#include "bench/data/block413567.raw.h"
+}
+
+// These are the two major time-sinks which happen after we have fully received
+// a block off the wire, but before we can relay the block on to peers using
+// compact block relay.
+
+static void DeserializeBlockTest(benchmark::State& state)
+{
+    CDataStream stream((const char*)block_bench::block413567,
+            (const char*)&block_bench::block413567[sizeof(block_bench::block413567)],
+            SER_NETWORK, PROTOCOL_VERSION);
+    char a;
+    stream.write(&a, 1); // Prevent compaction
+
+    while (state.KeepRunning()) {
+        CBlock block;
+        stream >> block;
+        assert(stream.Rewind(sizeof(block_bench::block413567)));
+    }
+}
+
+static void DeserializeAndCheckBlockTest(benchmark::State& state)
+{
+    CDataStream stream((const char*)block_bench::block413567,
+            (const char*)&block_bench::block413567[sizeof(block_bench::block413567)],
+            SER_NETWORK, PROTOCOL_VERSION);
+    char a;
+    stream.write(&a, 1); // Prevent compaction
+
+    Consensus::Params params = Params(CBaseChainParams::MAIN).GetConsensus();
+
+    while (state.KeepRunning()) {
+        CBlock block; // Note that CBlock caches its checked state, so we need to recreate it here
+        stream >> block;
+        assert(stream.Rewind(sizeof(block_bench::block413567)));
+
+        CValidationState state;
+        assert(CheckBlock(block, state, params));
+    }
+}
+
+BENCHMARK(DeserializeBlockTest);
+BENCHMARK(DeserializeAndCheckBlockTest);

src/bench/data/block413567.raw

@@ -1104,7 +1104,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
 
 
 
-bool CheckTransaction(const CTransaction& tx, CValidationState &state)
+bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
 {
     // Basic checks that don't depend on any context
     if (tx.vin.empty())
@@ -1128,13 +1128,14 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state)
             return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge");
     }
 
-    // Check for duplicate inputs
-    set<COutPoint> vInOutPoints;
-    for (const auto& txin : tx.vin)
-    {
-        if (vInOutPoints.count(txin.prevout))
-            return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
-        vInOutPoints.insert(txin.prevout);
+    // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
+    if (fCheckDuplicateInputs) {
+        set<COutPoint> vInOutPoints;
+        for (const auto& txin : tx.vin)
+        {
+            if (!vInOutPoints.insert(txin.prevout).second)
+                return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
+        }
     }
 
     if (tx.IsCoinBase())
@@ -3461,7 +3462,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
 
     // Check transactions
     for (const auto& tx : block.vtx)
-        if (!CheckTransaction(tx, state))
+        if (!CheckTransaction(tx, state, false))
             return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(),
                                  strprintf("Transaction check failed (tx hash %s) %s", tx.GetHash().ToString(), state.GetDebugMessage()));
 

src/main.cpp

@@ -343,7 +343,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
 /** Transaction validation functions */
 
 /** Context-independent validity checks */
-bool CheckTransaction(const CTransaction& tx, CValidationState& state);
+bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
 
 namespace Consensus {