diff options
Diffstat (limited to 'src')
29 files changed, 163 insertions, 96 deletions
diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index 9235d5fe6a..9b81380a9b 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -17,39 +17,40 @@ static const char* DEFAULT_PLOT_PLOTLYURL = "https://cdn.plot.ly/plotly-latest.m static const int64_t DEFAULT_PLOT_WIDTH = 1024; static const int64_t DEFAULT_PLOT_HEIGHT = 768; -static void SetupBenchArgs() +static void SetupBenchArgs(ArgsManager& argsman) { - SetupHelpOptions(gArgs); + SetupHelpOptions(argsman); - gArgs.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-evals=<n>", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-scaling=<n>", strprintf("Scaling factor for benchmark's runtime (default: %u)", DEFAULT_BENCH_SCALING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-printer=(console|plot)", strprintf("Choose printer format. console: print data to console. plot: Print results as HTML graph (default: %s)", DEFAULT_BENCH_PRINTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-plotlyurl=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-evals=<n>", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-scaling=<n>", strprintf("Scaling factor for benchmark's runtime (default: %u)", DEFAULT_BENCH_SCALING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-printer=(console|plot)", strprintf("Choose printer format. console: print data to console. plot: Print results as HTML graph (default: %s)", DEFAULT_BENCH_PRINTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-plotlyurl=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } int main(int argc, char** argv) { - SetupBenchArgs(); + ArgsManager argsman; + SetupBenchArgs(argsman); std::string error; - if (!gArgs.ParseParameters(argc, argv, error)) { + if (!argsman.ParseParameters(argc, argv, error)) { tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error); return EXIT_FAILURE; } - if (HelpRequested(gArgs)) { - std::cout << gArgs.GetHelpMessage(); + if (HelpRequested(argsman)) { + std::cout << argsman.GetHelpMessage(); return EXIT_SUCCESS; } - int64_t evaluations = gArgs.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS); - std::string regex_filter = gArgs.GetArg("-filter", DEFAULT_BENCH_FILTER); - std::string scaling_str = gArgs.GetArg("-scaling", DEFAULT_BENCH_SCALING); - bool is_list_only = gArgs.GetBoolArg("-list", false); + int64_t evaluations = argsman.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS); + std::string regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); + std::string scaling_str = argsman.GetArg("-scaling", DEFAULT_BENCH_SCALING); + bool is_list_only = argsman.GetBoolArg("-list", false); if (evaluations == 0) { return EXIT_SUCCESS; @@ -65,12 +66,12 @@ int main(int argc, char** argv) } std::unique_ptr<benchmark::Printer> printer = MakeUnique<benchmark::ConsolePrinter>(); - std::string printer_arg = gArgs.GetArg("-printer", DEFAULT_BENCH_PRINTER); + std::string printer_arg = argsman.GetArg("-printer", DEFAULT_BENCH_PRINTER); if ("plot" == printer_arg) { printer.reset(new benchmark::PlotlyPrinter( - gArgs.GetArg("-plot-plotlyurl", DEFAULT_PLOT_PLOTLYURL), - gArgs.GetArg("-plot-width", DEFAULT_PLOT_WIDTH), - gArgs.GetArg("-plot-height", DEFAULT_PLOT_HEIGHT))); + argsman.GetArg("-plot-plotlyurl", DEFAULT_PLOT_PLOTLYURL), + argsman.GetArg("-plot-width", DEFAULT_PLOT_WIDTH), + argsman.GetArg("-plot-height", DEFAULT_PLOT_HEIGHT))); } benchmark::BenchRunner::RunAll(*printer, evaluations, scaling_factor, regex_filter, is_list_only); diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index f26eb45fce..bf444dd0bc 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -53,7 +53,7 @@ static bool AppInit(int argc, char* argv[]) // Parameters // // If Qt is used, parameters/bitcoin.conf are parsed in qt/bitcoin.cpp's main() - SetupServerArgs(); + SetupServerArgs(node); std::string error; if (!gArgs.ParseParameters(argc, argv, error)) { return InitError(strprintf("Error parsing command line arguments: %s\n", error)); diff --git a/src/bloom.cpp b/src/bloom.cpp index bd6069b31f..30af507243 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -102,19 +102,6 @@ bool CBloomFilter::contains(const uint256& hash) const return contains(data); } -void CBloomFilter::clear() -{ - vData.assign(vData.size(),0); - isFull = false; - isEmpty = true; -} - -void CBloomFilter::reset(const unsigned int nNewTweak) -{ - clear(); - nTweak = nNewTweak; -} - bool CBloomFilter::IsWithinSizeConstraints() const { return vData.size() <= MAX_BLOOM_FILTER_SIZE && nHashFuncs <= MAX_HASH_FUNCS; diff --git a/src/bloom.h b/src/bloom.h index 68e76a0258..8e3b7be54d 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -84,9 +84,6 @@ public: bool contains(const COutPoint& outpoint) const; bool contains(const uint256& hash) const; - void clear(); - void reset(const unsigned int nNewTweak); - //! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS //! (catch a filter which was just deserialized which was too big) bool IsWithinSizeConstraints() const; diff --git a/src/init.cpp b/src/init.cpp index de2db694fd..de32c0ad71 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -297,6 +297,7 @@ void Shutdown(NodeContext& node) GetMainSignals().UnregisterBackgroundSignalScheduler(); globalVerifyHandle.reset(); ECC_Stop(); + node.args = nullptr; if (node.mempool) node.mempool = nullptr; node.scheduler.reset(); @@ -360,8 +361,11 @@ static void OnRPCStopped() LogPrint(BCLog::RPC, "RPC stopped.\n"); } -void SetupServerArgs() +void SetupServerArgs(NodeContext& node) { + assert(!node.args); + node.args = &gArgs; + SetupHelpOptions(gArgs); gArgs.AddArg("-help-debug", "Print help message with debugging options and exit", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); // server-only for now diff --git a/src/init.h b/src/init.h index f74ae5a47a..0c44035b00 100644 --- a/src/init.h +++ b/src/init.h @@ -54,9 +54,9 @@ bool AppInitLockDataDirectory(); bool AppInitMain(NodeContext& node); /** - * Setup the arguments for gArgs + * Register all arguments with the ArgsManager */ -void SetupServerArgs(); +void SetupServerArgs(NodeContext& node); /** Returns licensing information (for -version) */ std::string LicenseInfo(); diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c8311b2986..0e7641ae32 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -275,6 +275,8 @@ public: const CBlockIndex* block1 = LookupBlockIndex(block_hash1); const CBlockIndex* block2 = LookupBlockIndex(block_hash2); const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr; + // Using & instead of && below to avoid short circuiting and leaving + // output uninitialized. return FillBlock(ancestor, ancestor_out, lock) & FillBlock(block1, block1_out, lock) & FillBlock(block2, block2_out, lock); } void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); } diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 6e5fdc61b5..5ebbd61584 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -97,7 +97,7 @@ public: StopMapPort(); } } - void setupServerArgs() override { return SetupServerArgs(); } + void setupServerArgs() override { return SetupServerArgs(m_context); } bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); } size_t getNodeCount(CConnman::NumConnections flags) override { diff --git a/src/node/context.h b/src/node/context.h index 1c592b456b..7b6dd37361 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -8,6 +8,7 @@ #include <memory> #include <vector> +class ArgsManager; class BanMan; class CConnman; class CScheduler; @@ -33,6 +34,7 @@ struct NodeContext { CTxMemPool* mempool{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr<PeerLogicValidation> peer_logic; std::unique_ptr<BanMan> banman; + ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr<interfaces::Chain> chain; std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients; std::unique_ptr<CScheduler> scheduler; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 0ec1687dc5..8939b566f7 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -381,7 +381,7 @@ void BitcoinApplication::shutdownResult() void BitcoinApplication::handleRunawayException(const QString &message) { - QMessageBox::critical(nullptr, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. Bitcoin can no longer continue safely and will quit.") + QString("\n\n") + message); + QMessageBox::critical(nullptr, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) + QString("\n\n") + message); ::exit(EXIT_FAILURE); } diff --git a/src/qt/res/icons/bitcoin.ico b/src/qt/res/icons/bitcoin.ico Binary files differindex 8f5045015d..8f5045015d 100755..100644 --- a/src/qt/res/icons/bitcoin.ico +++ b/src/qt/res/icons/bitcoin.ico diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 50f7806ba5..d70960a8bd 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1291,18 +1291,29 @@ uint256 GetOutputsHash(const T& txTo) } // namespace template <class T> -PrecomputedTransactionData::PrecomputedTransactionData(const T& txTo) +void PrecomputedTransactionData::Init(const T& txTo) { + assert(!m_ready); + // Cache is calculated only for transactions with witness if (txTo.HasWitness()) { hashPrevouts = GetPrevoutHash(txTo); hashSequence = GetSequenceHash(txTo); hashOutputs = GetOutputsHash(txTo); - ready = true; } + + m_ready = true; +} + +template <class T> +PrecomputedTransactionData::PrecomputedTransactionData(const T& txTo) +{ + Init(txTo); } // explicit instantiation +template void PrecomputedTransactionData::Init(const CTransaction& txTo); +template void PrecomputedTransactionData::Init(const CMutableTransaction& txTo); template PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo); template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo); @@ -1315,7 +1326,7 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn uint256 hashPrevouts; uint256 hashSequence; uint256 hashOutputs; - const bool cacheready = cache && cache->ready; + const bool cacheready = cache && cache->m_ready; if (!(nHashType & SIGHASH_ANYONECANPAY)) { hashPrevouts = cacheready ? cache->hashPrevouts : GetPrevoutHash(txTo); diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 2b104a608c..71f2436369 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -121,7 +121,12 @@ bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned i struct PrecomputedTransactionData { uint256 hashPrevouts, hashSequence, hashOutputs; - bool ready = false; + bool m_ready = false; + + PrecomputedTransactionData() = default; + + template <class T> + void Init(const T& tx); template <class T> explicit PrecomputedTransactionData(const T& tx); diff --git a/src/script/script.h b/src/script/script.h index 7aaa10b60b..dd2ab2f070 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -433,7 +433,7 @@ public: return ret; } - CScript(int64_t b) { operator<<(b); } + explicit CScript(int64_t b) { operator<<(b); } explicit CScript(opcodetype b) { operator<<(b); } explicit CScript(const CScriptNum& b) { operator<<(b); } diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 4a7ad9b38b..bcf2e8ccff 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -27,6 +27,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) { CBloomFilter filter(3, 0.01, 0, BLOOM_UPDATE_ALL); + BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter should be empty!"); filter.insert(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")); BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); // One bit different in first byte @@ -50,8 +51,6 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); - filter.clear(); - BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter should be empty!"); } BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 436c1bffa0..c91621e227 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -8,6 +8,7 @@ #include <script/standard.h> #include <streams.h> #include <test/util/setup_common.h> +#include <txdb.h> #include <uint256.h> #include <undo.h> #include <util/strencodings.h> @@ -109,7 +110,12 @@ static const unsigned int NUM_SIMULATION_ITERATIONS = 40000; // // During the process, booleans are kept to make sure that the randomized // operation hits all branches. -BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) +// +// If fake_best_block is true, assign a random uint256 to mock the recording +// of best block on flush. This is necessary when using CCoinsViewDB as the base, +// otherwise we'll hit an assertion in BatchWrite. +// +void SimulationTest(CCoinsView* base, bool fake_best_block) { // Various coverage trackers. bool removed_all_caches = false; @@ -126,9 +132,8 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) std::map<COutPoint, Coin> result; // The cache stack. - CCoinsViewTest base; // A CCoinsViewTest at the bottom. std::vector<CCoinsViewCacheTest*> stack; // A stack of CCoinsViewCaches on top. - stack.push_back(new CCoinsViewCacheTest(&base)); // Start with one cache. + stack.push_back(new CCoinsViewCacheTest(base)); // Start with one cache. // Use a limited set of random transaction ids, so we do test overwriting entries. std::vector<uint256> txids; @@ -211,6 +216,7 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) // Every 100 iterations, flush an intermediate cache if (stack.size() > 1 && InsecureRandBool() == 0) { unsigned int flushIndex = InsecureRandRange(stack.size() - 1); + if (fake_best_block) stack[flushIndex]->SetBestBlock(InsecureRand256()); BOOST_CHECK(stack[flushIndex]->Flush()); } } @@ -218,13 +224,14 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) // Every 100 iterations, change the cache stack. if (stack.size() > 0 && InsecureRandBool() == 0) { //Remove the top cache + if (fake_best_block) stack.back()->SetBestBlock(InsecureRand256()); BOOST_CHECK(stack.back()->Flush()); delete stack.back(); stack.pop_back(); } if (stack.size() == 0 || (stack.size() < 4 && InsecureRandBool())) { //Add a new cache - CCoinsView* tip = &base; + CCoinsView* tip = base; if (stack.size() > 0) { tip = stack.back(); } else { @@ -256,6 +263,16 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) BOOST_CHECK(uncached_an_entry); } +// Run the above simulation for multiple base types. +BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) +{ + CCoinsViewTest base; + SimulationTest(&base, false); + + CCoinsViewDB db_base{"test", /*nCacheSize*/ 1 << 23, /*fMemory*/ true, /*fWipe*/ false}; + SimulationTest(&db_base, true); +} + // Store of all necessary tx and undo data for next test typedef std::map<COutPoint, std::tuple<CTransaction,CTxUndo,Coin>> UtxoData; UtxoData utxoData; diff --git a/src/test/fuzz/bloom_filter.cpp b/src/test/fuzz/bloom_filter.cpp index d1112f8e62..50036ce5bd 100644 --- a/src/test/fuzz/bloom_filter.cpp +++ b/src/test/fuzz/bloom_filter.cpp @@ -25,7 +25,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) fuzzed_data_provider.ConsumeIntegral<unsigned int>(), static_cast<unsigned char>(fuzzed_data_provider.PickValueInArray({BLOOM_UPDATE_NONE, BLOOM_UPDATE_ALL, BLOOM_UPDATE_P2PUBKEY_ONLY, BLOOM_UPDATE_MASK}))}; while (fuzzed_data_provider.remaining_bytes() > 0) { - switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 6)) { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 4)) { case 0: { const std::vector<unsigned char> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); (void)bloom_filter.contains(b); @@ -56,13 +56,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) assert(present); break; } - case 3: - bloom_filter.clear(); - break; - case 4: - bloom_filter.reset(fuzzed_data_provider.ConsumeIntegral<unsigned int>()); - break; - case 5: { + case 3: { const Optional<CMutableTransaction> mut_tx = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); if (!mut_tx) { break; @@ -71,7 +65,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) (void)bloom_filter.IsRelevantAndUpdate(tx); break; } - case 6: + case 4: bloom_filter.UpdateEmptyFull(); break; } diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 9e3586d162..0ed3721636 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -57,12 +57,17 @@ const std::map<std::string, std::set<std::string>> EXPECTED_DESERIALIZATION_EXCE {"Unknown transaction optional data: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}}, }; -const RegTestingSetup* g_setup; +const TestingSetup* g_setup; } // namespace void initialize() { - static RegTestingSetup setup{}; + static TestingSetup setup{ + CBaseChainParams::REGTEST, + { + "-nodebuglogfile", + }, + }; g_setup = &setup; for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 12a5dbb607..bcbf65bdca 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -16,11 +16,16 @@ #include <validation.h> #include <validationinterface.h> -const RegTestingSetup* g_setup; +const TestingSetup* g_setup; void initialize() { - static RegTestingSetup setup{}; + static TestingSetup setup{ + CBaseChainParams::REGTEST, + { + "-nodebuglogfile", + }, + }; g_setup = &setup; for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp index fab3571756..b0d4de89f3 100644 --- a/src/test/interfaces_tests.cpp +++ b/src/test/interfaces_tests.cpp @@ -116,6 +116,12 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor) BOOST_CHECK_EQUAL(orig_height, orig_tip->nHeight); BOOST_CHECK_EQUAL(fork_height, orig_tip->nHeight - 10); BOOST_CHECK_EQUAL(fork_hash, active[fork_height]->GetBlockHash()); + + uint256 active_hash, orig_hash; + BOOST_CHECK(!chain->findCommonAncestor(active.Tip()->GetBlockHash(), {}, {}, FoundBlock().hash(active_hash), {})); + BOOST_CHECK(!chain->findCommonAncestor({}, orig_tip->GetBlockHash(), {}, {}, FoundBlock().hash(orig_hash))); + BOOST_CHECK_EQUAL(active_hash, active.Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(orig_hash, orig_tip->GetBlockHash()); } BOOST_AUTO_TEST_CASE(hasBlocks) diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 3550a02316..721e3498e6 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -70,12 +70,8 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) // shutdown sequence (c.f. Shutdown() in init.cpp) txindex.Stop(); - // txindex job may be scheduled, so stop scheduler before destructing - m_node.scheduler->stop(); - threadGroup.interrupt_all(); - threadGroup.join_all(); - - // Rest of shutdown sequence and destructors happen in ~TestingSetup() + // Let scheduler events finish running to avoid accessing any memory related to txindex after it is destructed + SyncWithValidationInterfaceQueue(); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 32d50e49b9..0d455d48b3 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -28,6 +28,7 @@ #include <util/time.h> #include <util/translation.h> #include <util/url.h> +#include <util/vector.h> #include <validation.h> #include <validationinterface.h> @@ -65,17 +66,34 @@ std::ostream& operator<<(std::ostream& os, const uint256& num) return os; } -BasicTestingSetup::BasicTestingSetup(const std::string& chainName) +BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::vector<const char*>& extra_args) : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()} { + const std::vector<const char*> arguments = Cat( + { + "dummy", + "-printtoconsole=0", + "-logtimemicros", + "-debug", + "-debugexclude=libevent", + "-debugexclude=leveldb", + }, + extra_args); fs::create_directories(m_path_root); gArgs.ForceSetArg("-datadir", m_path_root.string()); ClearDatadirCache(); + { + SetupServerArgs(m_node); + std::string error; + const bool success{m_node.args->ParseParameters(arguments.size(), arguments.data(), error)}; + assert(success); + assert(error.empty()); + } SelectParams(chainName); SeedInsecureRand(); - gArgs.ForceSetArg("-printtoconsole", "0"); if (G_TEST_LOG_FUN) LogInstance().PushBackCallback(G_TEST_LOG_FUN); InitLogging(); + AppInitParameterInteraction(); LogInstance().StartLogging(); SHA256AutoDetect(); ECC_Start(); @@ -95,10 +113,12 @@ BasicTestingSetup::~BasicTestingSetup() { LogInstance().DisconnectTestLogger(); fs::remove_all(m_path_root); + gArgs.ClearArgs(); ECC_Stop(); } -TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) +TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const char*>& extra_args) + : BasicTestingSetup(chainName, extra_args) { const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework @@ -159,6 +179,7 @@ TestingSetup::~TestingSetup() g_rpc_node = nullptr; m_node.connman.reset(); m_node.banman.reset(); + m_node.args = nullptr; m_node.mempool = nullptr; m_node.scheduler.reset(); UnloadBlockIndex(); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 0930309c3a..47fcf92ea0 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -73,9 +73,11 @@ static constexpr CAmount CENT{1000000}; */ struct BasicTestingSetup { ECCVerifyHandle globalVerifyHandle; + NodeContext m_node; - explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector<const char*>& extra_args = {}); ~BasicTestingSetup(); + private: const fs::path m_path_root; }; @@ -84,10 +86,9 @@ private: * Included are coins database, script check threads setup. */ struct TestingSetup : public BasicTestingSetup { - NodeContext m_node; boost::thread_group threadGroup; - explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector<const char*>& extra_args = {}); ~TestingSetup(); }; diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 6e7186db22..e79ae1f4d5 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -3,12 +3,13 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include <chainparams.h> -#include <random.h> -#include <uint256.h> #include <consensus/validation.h> +#include <random.h> #include <sync.h> #include <test/util/setup_common.h> +#include <uint256.h> #include <validation.h> +#include <validationinterface.h> #include <vector> @@ -97,7 +98,9 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) exp_tip = c1.m_chain.Tip(); BOOST_CHECK_EQUAL(validated_tip, exp_tip); - // Avoid triggering the address sanitizer. + // Let scheduler events finish running to avoid accessing memory that is going to be unloaded + SyncWithValidationInterfaceQueue(); + WITH_LOCK(::cs_main, manager.Unload()); } diff --git a/src/util/system.cpp b/src/util/system.cpp index b0a538b527..69a7be96dc 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -226,10 +226,11 @@ static bool CheckValid(const std::string& key, const util::SettingsValue& val, u return true; } -ArgsManager::ArgsManager() -{ - // nothing to do -} +// Define default constructor and destructor that are not inline, so code instantiating this class doesn't need to +// #include class definitions for all members. +// For example, m_settings has an internal dependency on univalue. +ArgsManager::ArgsManager() {} +ArgsManager::~ArgsManager() {} const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const { diff --git a/src/util/system.h b/src/util/system.h index 3138522b5c..96f51e6074 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -192,6 +192,7 @@ protected: public: ArgsManager(); + ~ArgsManager(); /** * Select the network in use diff --git a/src/validation.cpp b/src/validation.cpp index 9f5c59e52b..60d028336f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1016,7 +1016,7 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs // scripts (ie, other policy checks pass). We perform the inexpensive // checks first and avoid hashing and signature verification unless those // checks pass, to mitigate CPU exhaustion denial-of-service attacks. - PrecomputedTransactionData txdata(*ptx); + PrecomputedTransactionData txdata; if (!PolicyScriptChecks(args, workspace, txdata)) return false; @@ -1516,6 +1516,10 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const C return true; } + if (!txdata.m_ready) { + txdata.Init(tx); + } + for (unsigned int i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; const Coin& coin = inputs.AccessCoin(prevout); @@ -2079,15 +2083,19 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockUndo blockundo; + // Precomputed transaction data pointers must not be invalidated + // until after `control` has run the script checks (potentially + // in multiple threads). Preallocate the vector size so a new allocation + // doesn't invalidate pointers into the vector, and keep txsdata in scope + // for as long as `control`. CCheckQueueControl<CScriptCheck> control(fScriptChecks && g_parallel_script_checks ? &scriptcheckqueue : nullptr); + std::vector<PrecomputedTransactionData> txsdata(block.vtx.size()); std::vector<int> prevheights; CAmount nFees = 0; int nInputs = 0; int64_t nSigOpsCost = 0; blockundo.vtxundo.reserve(block.vtx.size() - 1); - std::vector<PrecomputedTransactionData> txdata; - txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction &tx = *(block.vtx[i]); @@ -2134,13 +2142,12 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops"); } - txdata.emplace_back(tx); if (!tx.IsCoinBase()) { std::vector<CScriptCheck> vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ TxValidationState tx_state; - if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txdata[i], g_parallel_script_checks ? &vChecks : nullptr)) { + if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) { // Any transaction validation failure in ConnectBlock is a block consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 5bbb2c0ad0..079a5d3d53 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -106,6 +106,9 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_v best_selection = curr_selection; best_selection.resize(utxo_pool.size()); best_waste = curr_waste; + if (best_waste == 0) { + break; + } } curr_waste -= (curr_value - actual_target); // Remove the excess value as we will be selecting different coins now backtrack = true; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 21d57cb898..2081518909 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -176,8 +176,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) selection.clear(); // Select 5 Cent - add_coin(3 * CENT, 3, actual_selection); - add_coin(2 * CENT, 2, actual_selection); + add_coin(4 * CENT, 4, actual_selection); + add_coin(1 * CENT, 1, actual_selection); BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); BOOST_CHECK(equal_sets(selection, actual_selection)); BOOST_CHECK_EQUAL(value_ret, 5 * CENT); @@ -204,9 +204,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Select 10 Cent add_coin(5 * CENT, 5, utxo_pool); + add_coin(5 * CENT, 5, actual_selection); add_coin(4 * CENT, 4, actual_selection); - add_coin(3 * CENT, 3, actual_selection); - add_coin(2 * CENT, 2, actual_selection); add_coin(1 * CENT, 1, actual_selection); BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); BOOST_CHECK(equal_sets(selection, actual_selection)); |