From 58e785afb8d40bf71e8990fc2b92bf1687c14dc0 Mon Sep 17 00:00:00 2001 From: TomZ Date: Thu, 10 Mar 2016 18:09:13 +0000 Subject: [PATCH 1/3] Extend checkpointing to a safe distance in the past Checkpoints are hardcoded points that are shipped in the client and we skip checking of script validity if they come before a checkpoint. This change builds on that and extends the no-check requirement from known good points to acceptable points. It would be absurd to have more than 100 block reorg, the cost of such an event would be catastrophic in itself (it is the reason why we have that coinbase spending limit there), but lets be certain and make it one day (approx 144 blocks) at minimum. Additionally, to avoid predictability, make a node have the amount of script-checking-threads as a multiplication factor. Conflicts: src/main.cpp --- src/main.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index fcf6ac714b407..aaf1889cc1d70 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2153,7 +2153,12 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return true; } - bool fScriptChecks = (!fCheckpointsEnabled || pindex->nHeight >= Checkpoints::GetTotalBlocksEstimate(chainparams.Checkpoints())); + const int64_t timeBarrier = GetTime() - 24 * 3600 * std::max(1, nScriptCheckThreads); + // Blocks that have varius days of POW behind them makes them secure in + // that actually online nodes checked the scripts, so during initial sync we + // don't need to check the scripts. + // All other block validity tests are still checked. + bool fScriptChecks = !fCheckpointsEnabled || block.nTime > timeBarrier; // Do not allow blocks that contain transactions which 'overwrite' older transactions, // unless those are already completely spent. From c854a3922c50ac5a3ed61e6095d22c052bfeeeeb Mon Sep 17 00:00:00 2001 From: Dagur Valberg Johannsson Date: Sat, 9 Apr 2016 12:27:21 -0400 Subject: [PATCH 2/3] Moved nScriptCheckThreads into options, added test Removed global variable from main.h, removed initialization from init.cpp --- src/Makefile.test.include | 1 + src/init.cpp | 15 +++--------- src/main.cpp | 5 ++-- src/main.h | 5 ---- src/options.cpp | 16 +++++++++++++ src/options.h | 11 ++++++++- src/qt/optionsdialog.cpp | 2 +- src/qt/optionsmodel.cpp | 2 +- src/test/clientversion_tests.cpp | 4 ++++ src/test/options_tests.cpp | 41 ++++++++++++++++++++++++++++++++ src/test/test_bitcoin.cpp | 5 ++-- 11 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 src/test/options_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 439dd798c3bce..34ccaeb168467 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -59,6 +59,7 @@ BITCOIN_TESTS =\ test/miner_tests.cpp \ test/multisig_tests.cpp \ test/netbase_tests.cpp \ + test/options_tests.cpp \ test/p2p_protocol_tests.cpp \ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ diff --git a/src/init.cpp b/src/init.cpp index fe3bdc20d29f9..919db17932788 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -798,15 +798,6 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (nMempoolSizeLimit < 0 || nMempoolSizeLimit < nMempoolDescendantSizeLimit * 40) return InitError(strprintf(_("Error: -maxmempool must be at least %d MB"), GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) / 25)); - // -par=0 means autodetect, but nScriptCheckThreads==0 means no concurrency - nScriptCheckThreads = GetArg("-par", DEFAULT_SCRIPTCHECK_THREADS); - if (nScriptCheckThreads <= 0) - nScriptCheckThreads += boost::thread::hardware_concurrency(); - if (nScriptCheckThreads <= 1) - nScriptCheckThreads = 0; - else if (nScriptCheckThreads > MAX_SCRIPTCHECK_THREADS) - nScriptCheckThreads = MAX_SCRIPTCHECK_THREADS; - fServer = GetBoolArg("-server", false); // block pruning; get the amount of disk space (in MB) to allot for block & undo files @@ -946,9 +937,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); std::ostringstream strErrors; - LogPrintf("Using %u threads for script verification\n", nScriptCheckThreads); - if (nScriptCheckThreads) { - for (int i=0; i sizeForkTime(std::numeric_limits::max()); int64_t nTimeBestReceived = 0; CWaitableCriticalSection csBestBlock; CConditionVariable cvBlockChange; -int nScriptCheckThreads = 0; bool fImporting = false; bool fReindex = false; bool fTxIndex = false; @@ -2210,7 +2209,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Post-fork, accurately counted sigop/sighash limits are used ValidationCostTracker costTracker(MaxBlockSigops(block.nTime), MaxBlockSighash(block.nTime)); - CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); + CCheckQueueControl control(fScriptChecks && Opt().ScriptCheckThreads() ? &scriptcheckqueue : NULL); CAmount nFees = 0; int nInputs = 0; @@ -2251,7 +2250,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::vector vChecks; if (!CheckInputs(tx, state, view, fScriptChecks, flags, false, - &costTracker, nScriptCheckThreads ? &vChecks : NULL)) + &costTracker, Opt().ScriptCheckThreads() ? &vChecks : NULL)) return false; control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index 0763d6ffe9add..8247a91c0e950 100644 --- a/src/main.h +++ b/src/main.h @@ -82,10 +82,6 @@ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB static const unsigned int BLOCKFILE_CHUNK_SIZE = 0x1000000; // 16 MiB /** The pre-allocation chunk size for rev?????.dat files (since 0.8) */ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB -/** Maximum number of script-checking threads allowed */ -static const int MAX_SCRIPTCHECK_THREADS = 16; -/** -par default (number of script-checking threads, 0 = auto) */ -static const int DEFAULT_SCRIPTCHECK_THREADS = 0; /** Number of blocks that can be requested at any given time from a single peer. */ static const int MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16; /** Timeout in seconds during which a peer must stall block download progress before being disconnected. */ @@ -124,7 +120,6 @@ extern CWaitableCriticalSection csBestBlock; extern CConditionVariable cvBlockChange; extern bool fImporting; extern bool fReindex; -extern int nScriptCheckThreads; extern bool fTxIndex; extern bool fIsBareMultisigStd; extern bool fCheckBlockIndex; diff --git a/src/options.cpp b/src/options.cpp index 19878286d322d..7c0aece1404cb 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -1,6 +1,7 @@ #include "options.h" #include "util.h" #include +#include static std::auto_ptr Args; @@ -14,6 +15,9 @@ struct DefaultGetter : public ArgGetter { return mapMultiArgs[arg]; } + virtual int64_t GetArg(const std::string& strArg, int64_t nDefault) { + return ::GetArg(strArg, nDefault); + } }; Opt::Opt() { @@ -48,6 +52,18 @@ std::vector Opt::UAComment(bool validate) { return uacomments; } +int Opt::ScriptCheckThreads() { + // -par=0 means autodetect, but nScriptCheckThreads==0 means no concurrency + int nScriptCheckThreads = Args->GetArg("-par", DEFAULT_SCRIPTCHECK_THREADS); + if (nScriptCheckThreads <= 0) + nScriptCheckThreads += boost::thread::hardware_concurrency(); + if (nScriptCheckThreads <= 1) + nScriptCheckThreads = 0; + else if (nScriptCheckThreads > MAX_SCRIPTCHECK_THREADS) + nScriptCheckThreads = MAX_SCRIPTCHECK_THREADS; + return nScriptCheckThreads; +} + std::auto_ptr SetDummyArgGetter(std::auto_ptr getter) { Args.reset(getter.release()); return std::auto_ptr(new ArgReset); diff --git a/src/options.h b/src/options.h index b44325a8f8999..1a3ea880533ed 100644 --- a/src/options.h +++ b/src/options.h @@ -1,8 +1,9 @@ #ifndef BITCOIN_OPTIONS_H #define BITCOIN_OPTIONS_H -#include #include +#include +#include #include struct Opt { @@ -11,8 +12,15 @@ struct Opt { bool IsStealthMode(); bool HidePlatform(); std::vector UAComment(bool validate = false); + int ScriptCheckThreads(); }; +/** Maximum number of script-checking threads allowed */ +static const int MAX_SCRIPTCHECK_THREADS = 16; +/** -par default (number of script-checking threads, 0 = auto) */ +static const int DEFAULT_SCRIPTCHECK_THREADS = 0; + + // // For unit testing // @@ -20,6 +28,7 @@ struct Opt { struct ArgGetter { virtual bool GetBool(const std::string& arg, bool def) = 0; virtual std::vector GetMultiArgs(const std::string& arg) = 0; + virtual int64_t GetArg(const std::string& strArg, int64_t nDefault) = 0; }; struct ArgReset { diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index f5bf7d7271679..43f692ba4f949 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -14,7 +14,7 @@ #include "guiutil.h" #include "optionsmodel.h" -#include "main.h" // for MAX_SCRIPTCHECK_THREADS +#include "options.h" // for MAX_SCRIPTCHECK_THREADS #include "netbase.h" #include "txdb.h" // for -dbcache defaults diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 85c9e7a61242d..c17b114213a68 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -14,7 +14,7 @@ #include "amount.h" #include "init.h" -#include "main.h" +#include "options.h" #include "net.h" #include "txdb.h" // for -dbcache defaults diff --git a/src/test/clientversion_tests.cpp b/src/test/clientversion_tests.cpp index 7ed9529ca6b5b..16f6b845667c6 100644 --- a/src/test/clientversion_tests.cpp +++ b/src/test/clientversion_tests.cpp @@ -34,6 +34,10 @@ struct DummyArgGetter : public ArgGetter { return uacomment; return std::vector(); } + virtual int64_t GetArg(const std::string& strArg, int64_t nDefault) { + assert(false); + } + bool stealthmode; bool hideplatform; diff --git a/src/test/options_tests.cpp b/src/test/options_tests.cpp new file mode 100644 index 0000000000000..84e16032a051e --- /dev/null +++ b/src/test/options_tests.cpp @@ -0,0 +1,41 @@ +// Copyright (c) 2016 The Bitcoin XT developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include "options.h" + +struct DummyArgGetter : public ArgGetter { + virtual int64_t GetArg(const std::string& strArg, int64_t nDefault) { + assert(strArg == "-par"); + return par; + } + virtual std::vector GetMultiArgs(const std::string& arg) { + assert(false); + } + virtual bool GetBool(const std::string& arg, bool def) { + assert(false); + } + int par; +}; + +BOOST_AUTO_TEST_SUITE(options_tests); + +BOOST_AUTO_TEST_CASE(scripthreads) { + std::auto_ptr arg(new DummyArgGetter); + DummyArgGetter* argPtr = arg.get(); + std::auto_ptr argraii + = SetDummyArgGetter(std::auto_ptr(arg.release())); + + argPtr->par = 0; // auto + BOOST_CHECK(Opt().ScriptCheckThreads() > 0); + + argPtr->par = 1; // not threaded + BOOST_CHECK_EQUAL(0, Opt().ScriptCheckThreads()); + + argPtr->par = 3; // 3 threads + BOOST_CHECK_EQUAL(3, Opt().ScriptCheckThreads()); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 2714ca5eee68c..17b833d2333f1 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -12,6 +12,7 @@ #include "txdb.h" #include "ui_interface.h" #include "util.h" +#include "options.h" #ifdef ENABLE_WALLET #include "wallet/db.h" #include "wallet/wallet.h" @@ -60,8 +61,8 @@ TestingSetup::TestingSetup() pwalletMain->LoadWallet(fFirstRun); RegisterValidationInterface(pwalletMain); #endif - nScriptCheckThreads = 3; - for (int i=0; i < nScriptCheckThreads-1; i++) + mapArgs["-par"] = "3"; + for (int i=0; i < Opt().ScriptCheckThreads()-1; i++) threadGroup.create_thread(&ThreadScriptCheck); RegisterNodeSignals(GetNodeSignals()); } From 23e0b01398990b12041c3276235954a8f463be4f Mon Sep 17 00:00:00 2001 From: Dagur Valberg Johannsson Date: Mon, 11 Apr 2016 12:23:10 -0400 Subject: [PATCH 3/3] Added option 'checkpoint-days' Configure how old blocks must be before we skip script validation --- src/init.cpp | 3 ++- src/main.cpp | 2 +- src/options.cpp | 5 +++++ src/options.h | 4 +++- src/test/options_tests.cpp | 42 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 919db17932788..01a0a302bdf5e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -371,7 +371,8 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageGroup(_("Debugging/Testing options:")); if (showDebug) { - strUsage += HelpMessageOpt("-checkpoints", strprintf("Only accept block chain matching built-in checkpoints (default: %u)", 1)); + strUsage += HelpMessageOpt("-checkpoints", strprintf("Skip validating scripts for old blocks with valid PoW (default: %u)", 1)); + strUsage += HelpMessageOpt("-checkpoint-days", strprintf("Minimum age of blocks (in days) to skip validation for (default: %u)", DEFAULT_CHECKPOINT_DAYS)); strUsage += HelpMessageOpt("-dblogsize=", strprintf("Flush database activity from memory pool to disk log every megabytes (default: %u)", 100)); strUsage += HelpMessageOpt("-disablesafemode", strprintf("Disable safemode, override a real safe mode event (default: %u)", 0)); strUsage += HelpMessageOpt("-testsafemode", strprintf("Force safe mode (default: %u)", 0)); diff --git a/src/main.cpp b/src/main.cpp index b695ec6825696..89411ac59590e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2152,7 +2152,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return true; } - const int64_t timeBarrier = GetTime() - 24 * 3600 * std::max(1, nScriptCheckThreads); + const int64_t timeBarrier = GetTime() - 24 * 3600 * Opt().CheckpointDays(); // Blocks that have varius days of POW behind them makes them secure in // that actually online nodes checked the scripts, so during initial sync we // don't need to check the scripts. diff --git a/src/options.cpp b/src/options.cpp index 7c0aece1404cb..8a3a51df08780 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -64,6 +64,11 @@ int Opt::ScriptCheckThreads() { return nScriptCheckThreads; } +int64_t Opt::CheckpointDays() { + int64_t def = DEFAULT_CHECKPOINT_DAYS * std::max(1, ScriptCheckThreads()); + return std::max(int64_t(1), Args->GetArg("-checkpoint-days", def)); +} + std::auto_ptr SetDummyArgGetter(std::auto_ptr getter) { Args.reset(getter.release()); return std::auto_ptr(new ArgReset); diff --git a/src/options.h b/src/options.h index 1a3ea880533ed..ccbe9578c88ec 100644 --- a/src/options.h +++ b/src/options.h @@ -13,13 +13,15 @@ struct Opt { bool HidePlatform(); std::vector UAComment(bool validate = false); int ScriptCheckThreads(); + int64_t CheckpointDays(); }; /** Maximum number of script-checking threads allowed */ static const int MAX_SCRIPTCHECK_THREADS = 16; /** -par default (number of script-checking threads, 0 = auto) */ static const int DEFAULT_SCRIPTCHECK_THREADS = 0; - +// Blocks newer than n days will have their script validated during sync. +static const int DEFAULT_CHECKPOINT_DAYS = 30; // // For unit testing diff --git a/src/test/options_tests.cpp b/src/test/options_tests.cpp index 84e16032a051e..e09371669850d 100644 --- a/src/test/options_tests.cpp +++ b/src/test/options_tests.cpp @@ -5,11 +5,25 @@ #include #include "options.h" +#include + +const int NOT_SET = std::numeric_limits::min(); struct DummyArgGetter : public ArgGetter { + + DummyArgGetter() : ArgGetter(), + par(NOT_SET), checkpdays(NOT_SET) + { + } + virtual int64_t GetArg(const std::string& strArg, int64_t nDefault) { - assert(strArg == "-par"); - return par; + if (strArg == "-par") + return par == NOT_SET ? nDefault : par; + + if (strArg == "-checkpoint-days") + return checkpdays == NOT_SET ? nDefault : checkpdays; + + assert(false); } virtual std::vector GetMultiArgs(const std::string& arg) { assert(false); @@ -18,6 +32,7 @@ struct DummyArgGetter : public ArgGetter { assert(false); } int par; + int checkpdays; }; BOOST_AUTO_TEST_SUITE(options_tests); @@ -38,4 +53,27 @@ BOOST_AUTO_TEST_CASE(scripthreads) { BOOST_CHECK_EQUAL(3, Opt().ScriptCheckThreads()); } +BOOST_AUTO_TEST_CASE(checkpointdays) { + std::auto_ptr arg(new DummyArgGetter); + DummyArgGetter* argPtr = arg.get(); + std::auto_ptr argraii + = SetDummyArgGetter(std::auto_ptr(arg.release())); + + // No multiplier + argPtr->par = 1; + BOOST_CHECK_EQUAL(DEFAULT_CHECKPOINT_DAYS, Opt().CheckpointDays()); + + // x3 multiplier (3 script threads) + argPtr->par = 3; + BOOST_CHECK_EQUAL(3 * DEFAULT_CHECKPOINT_DAYS, Opt().CheckpointDays()); + + // Explicitly set days overrides default and multipliers + argPtr->checkpdays = 1; + BOOST_CHECK_EQUAL(1, Opt().CheckpointDays()); + + // Can't have less than 1 day + argPtr->checkpdays = 0; + BOOST_CHECK_EQUAL(1, Opt().CheckpointDays()); +} + BOOST_AUTO_TEST_SUITE_END()