diff --git a/CMakeLists.txt b/CMakeLists.txt index 68e214aab2b0e..ad8643254999c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -324,10 +324,7 @@ if (CMAKE_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_SIZEOF_VOID_P EQUAL 4) endif() if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") - target_compile_definitions(core_interface INTERFACE - MAC_OSX - OBJC_OLD_DISPATCH_PROTOTYPES=0 - ) + target_compile_definitions(core_interface INTERFACE OBJC_OLD_DISPATCH_PROTOTYPES=0) # These flags are specific to ld64, and may cause issues with other linkers. # For example: GNU ld will interpret -dead_strip as -de and then try and use # "ad_strip" as the symbol for the entry point. diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 9069fba15601e..ce01db325cebe 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -59,11 +59,16 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then fi if [ "$DANGER_CI_ON_HOST_CCACHE_FOLDER" ]; then + # Temporary exclusion for https://github.com/bitcoin/bitcoin/issues/31108 + # to allow CI configs and envs generated in the past to work for a bit longer. + # Can be removed in March 2025. + if [ "${CCACHE_DIR}" != "/tmp/ccache_dir" ]; then if [ ! -d "${CCACHE_DIR}" ]; then echo "Error: Directory '${CCACHE_DIR}' must be created in advance." exit 1 fi CI_CCACHE_MOUNT="type=bind,src=${CCACHE_DIR},dst=${CCACHE_DIR}" + fi # End temporary exclusion fi docker network create --ipv6 --subnet 1111:1111::/112 ci-ip6net || true diff --git a/depends/README.md b/depends/README.md index eb868da957a36..1c4b6f24b29a3 100644 --- a/depends/README.md +++ b/depends/README.md @@ -22,15 +22,15 @@ created. To use it during configuring Groestlcoin Core: Common `host-platform-triplet`s for cross compilation are: -- `i686-pc-linux-gnu` for Linux 32 bit -- `x86_64-pc-linux-gnu` for x86 Linux +- `i686-pc-linux-gnu` for Linux x86 32 bit +- `x86_64-pc-linux-gnu` for Linux x86 64 bit - `x86_64-w64-mingw32` for Win64 - `x86_64-apple-darwin` for macOS - `arm64-apple-darwin` for ARM macOS - `arm-linux-gnueabihf` for Linux ARM 32 bit - `aarch64-linux-gnu` for Linux ARM 64 bit -- `powerpc64-linux-gnu` for Linux POWER 64-bit (big endian) -- `powerpc64le-linux-gnu` for Linux POWER 64-bit (little endian) +- `powerpc64-linux-gnu` for Linux POWER 64 bit (big endian) +- `powerpc64le-linux-gnu` for Linux POWER 64 bit (little endian) - `riscv32-linux-gnu` for Linux RISC-V 32 bit - `riscv64-linux-gnu` for Linux RISC-V 64 bit - `s390x-linux-gnu` for Linux S390X diff --git a/depends/packages/sqlite.mk b/depends/packages/sqlite.mk index 15bc0f4d7a3ca..d803a375a6ede 100644 --- a/depends/packages/sqlite.mk +++ b/depends/packages/sqlite.mk @@ -1,8 +1,8 @@ package=sqlite -$(package)_version=3380500 -$(package)_download_path=https://sqlite.org/2022/ +$(package)_version=3460100 +$(package)_download_path=https://sqlite.org/2024/ $(package)_file_name=sqlite-autoconf-$($(package)_version).tar.gz -$(package)_sha256_hash=5af07de982ba658fd91a03170c945f99c971f6955bc79df3266544373e39869c +$(package)_sha256_hash=67d3fe6d268e6eaddcae3727fce58fcc8e9c53869bdd07a0c61e38ddf2965071 define $(package)_set_vars $(package)_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking diff --git a/doc/benchmarking.md b/doc/benchmarking.md index 1cdfd3d96e38d..ddcc6d6491c84 100644 --- a/doc/benchmarking.md +++ b/doc/benchmarking.md @@ -40,7 +40,7 @@ The output will look similar to: Help --------------------- - build/src/bench/bench_groestlcoin -? + build/src/bench/bench_groestlcoin -h To print the various options, like listing the benchmarks without running them or using a regex filter to only run certain benchmarks. diff --git a/src/addrman.cpp b/src/addrman.cpp index 08db3aed4a37c..aa306bb9da2fc 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -825,6 +825,7 @@ std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct // gather a list of random nodes, skipping those of low quality const auto now{Now()}; std::vector addresses; + addresses.reserve(nNodes); for (unsigned int n = 0; n < vRandom.size(); n++) { if (addresses.size() >= nNodes) break; diff --git a/src/bench/CMakeLists.txt b/src/bench/CMakeLists.txt index 8a52980e072c8..45b6510044635 100644 --- a/src/bench/CMakeLists.txt +++ b/src/bench/CMakeLists.txt @@ -71,6 +71,7 @@ if(ENABLE_WALLET) wallet_create_tx.cpp wallet_loading.cpp wallet_ismine.cpp + wallet_migration.cpp ) target_link_libraries(bench_bitcoin bitcoin_wallet) endif() diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp new file mode 100644 index 0000000000000..eff6c6b526d75 --- /dev/null +++ b/src/bench/wallet_migration.cpp @@ -0,0 +1,80 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include // IWYU pragma: keep + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#if defined(USE_BDB) && defined(USE_SQLITE) // only enable benchmark when bdb and sqlite are enabled + +namespace wallet{ + +static void WalletMigration(benchmark::Bench& bench) +{ + const auto test_setup = MakeNoLogFileContext(); + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + // Number of imported watch only addresses + int NUM_WATCH_ONLY_ADDR = 20; + + // Setup legacy wallet + DatabaseOptions options; + options.use_unsafe_sync = true; + options.verify = false; + DatabaseStatus status; + bilingual_str error; + auto database = MakeWalletDatabase(fs::PathToString(test_setup->m_path_root / "legacy"), options, status, error); + uint64_t create_flags = 0; + auto wallet = TestLoadWallet(std::move(database), context, create_flags); + + // Add watch-only addresses + std::vector scripts_watch_only; + for (int w = 0; w < NUM_WATCH_ONLY_ADDR; ++w) { + CKey key = GenerateRandomKey(); + LOCK(wallet->cs_wallet); + const CScript& script = scripts_watch_only.emplace_back(GetScriptForDestination(GetDestinationForKey(key.GetPubKey(), OutputType::LEGACY))); + bool res = wallet->ImportScriptPubKeys(strprintf("watch_%d", w), {script}, + /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1); + assert(res); + } + + // Generate transactions and local addresses + for (int j = 0; j < 400; ++j) { + CMutableTransaction mtx; + mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, strprintf("bench_%d", j))))); + mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::LEGACY, strprintf("legacy_%d", j))))); + mtx.vout.emplace_back(COIN, scripts_watch_only.at(j % NUM_WATCH_ONLY_ADDR)); + mtx.vin.resize(2); + wallet->AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, /*rescanning_old_block=*/true); + } + + // Unload so the migration process loads it + TestUnloadWallet(std::move(wallet)); + + bench.epochs(/*numEpochs=*/1).run([&] { + util::Result res = MigrateLegacyToDescriptor(fs::PathToString(test_setup->m_path_root / "legacy"), "", context); + assert(res); + assert(res->wallet); + assert(res->watchonly_wallet); + }); +} + +BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW); + +} // namespace wallet + +#endif // end USE_SQLITE && USE_BDB diff --git a/src/coins.cpp b/src/coins.cpp index a47ab8063e609..cb09aa2e7aad5 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -9,7 +9,7 @@ #include #include -bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; } +std::optional CCoinsView::GetCoin(const COutPoint& outpoint) const { return std::nullopt; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } std::vector CCoinsView::GetHeadBlocks() const { return std::vector(); } bool CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlock) { return false; } @@ -17,12 +17,11 @@ std::unique_ptr CCoinsView::Cursor() const { return nullptr; } bool CCoinsView::HaveCoin(const COutPoint &outpoint) const { - Coin coin; - return GetCoin(outpoint, coin); + return GetCoin(outpoint).has_value(); } CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } -bool CCoinsViewBacked::GetCoin(const COutPoint &outpoint, Coin &coin) const { return base->GetCoin(outpoint, coin); } +std::optional CCoinsViewBacked::GetCoin(const COutPoint& outpoint) const { return base->GetCoin(outpoint); } bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base->HaveCoin(outpoint); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } std::vector CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); } @@ -45,26 +44,25 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const { const auto [ret, inserted] = cacheCoins.try_emplace(outpoint); if (inserted) { - if (!base->GetCoin(outpoint, ret->second.coin)) { + if (auto coin{base->GetCoin(outpoint)}) { + ret->second.coin = std::move(*coin); + cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); + if (ret->second.coin.IsSpent()) { // TODO GetCoin cannot return spent coins + // The parent only has an empty entry for this outpoint; we can consider our version as fresh. + ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel); + } + } else { cacheCoins.erase(ret); return cacheCoins.end(); } - if (ret->second.coin.IsSpent()) { - // The parent only has an empty entry for this outpoint; we can consider our version as fresh. - ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel); - } - cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); } return ret; } -bool CCoinsViewCache::GetCoin(const COutPoint &outpoint, Coin &coin) const { - CCoinsMap::const_iterator it = FetchCoin(outpoint); - if (it != cacheCoins.end()) { - coin = it->second.coin; - return !coin.IsSpent(); - } - return false; +std::optional CCoinsViewCache::GetCoin(const COutPoint& outpoint) const +{ + if (auto it{FetchCoin(outpoint)}; it != cacheCoins.end() && !it->second.coin.IsSpent()) return it->second.coin; + return std::nullopt; } void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possible_overwrite) { @@ -363,8 +361,8 @@ const Coin& AccessByTxid(const CCoinsViewCache& view, const Txid& txid) return coinEmpty; } -template -static bool ExecuteBackedWrapper(Func func, const std::vector>& err_callbacks) +template +static ReturnType ExecuteBackedWrapper(Func func, const std::vector>& err_callbacks) { try { return func(); @@ -381,10 +379,12 @@ static bool ExecuteBackedWrapper(Func func, const std::vector CCoinsViewErrorCatcher::GetCoin(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::GetCoin(outpoint); }, m_err_callbacks); } -bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint &outpoint) const { - return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); +bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); } diff --git a/src/coins.h b/src/coins.h index 78b8eddacd79b..a2449e1b81547 100644 --- a/src/coins.h +++ b/src/coins.h @@ -303,11 +303,8 @@ struct CoinsViewCacheCursor class CCoinsView { public: - /** Retrieve the Coin (unspent transaction output) for a given outpoint. - * Returns true only when an unspent coin was found, which is returned in coin. - * When false is returned, coin's value is unspecified. - */ - virtual bool GetCoin(const COutPoint &outpoint, Coin &coin) const; + //! Retrieve the Coin (unspent transaction output) for a given outpoint. + virtual std::optional GetCoin(const COutPoint& outpoint) const; //! Just check whether a given outpoint is unspent. virtual bool HaveCoin(const COutPoint &outpoint) const; @@ -344,7 +341,7 @@ class CCoinsViewBacked : public CCoinsView public: CCoinsViewBacked(CCoinsView *viewIn); - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; @@ -384,7 +381,7 @@ class CCoinsViewCache : public CCoinsViewBacked CCoinsViewCache(const CCoinsViewCache &) = delete; // Standard CCoinsView methods - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; void SetBestBlock(const uint256 &hashBlock); @@ -514,7 +511,7 @@ class CCoinsViewErrorCatcher final : public CCoinsViewBacked m_err_callbacks.emplace_back(std::move(f)); } - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; private: diff --git a/src/common/args.cpp b/src/common/args.cpp index 59088d681fd76..f97b49e51c9ce 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -184,7 +184,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin for (int i = 1; i < argc; i++) { std::string key(argv[i]); -#ifdef MAC_OSX +#ifdef __APPLE__ // At the first time when a user gets the "App downloaded from the // internet" warning, and clicks the Open button, macOS passes // a unique process serial number (PSN) as -psn_... command-line @@ -688,8 +688,8 @@ bool HelpRequested(const ArgsManager& args) void SetupHelpOptions(ArgsManager& args) { - args.AddArg("-?", "Print this help message and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - args.AddHiddenArgs({"-h", "-help"}); + args.AddArg("-help", "Print this help message and exit (also -h or -?)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + args.AddHiddenArgs({"-h", "-?"}); } static const int screenWidth = 79; @@ -741,7 +741,7 @@ fs::path GetDefaultDataDir() pathRet = fs::path("/"); else pathRet = fs::path(pszHome); -#ifdef MAC_OSX +#ifdef __APPLE__ // macOS return pathRet / "Library/Application Support/Groestlcoin"; #else diff --git a/src/common/system.cpp b/src/common/system.cpp index 6a9463a0a5153..7af792db44c35 100644 --- a/src/common/system.cpp +++ b/src/common/system.cpp @@ -70,7 +70,7 @@ void SetupEnvironment() #endif // On most POSIX systems (e.g. Linux, but not BSD) the environment's locale // may be invalid, in which case the "C.UTF-8" locale is used as fallback. -#if !defined(WIN32) && !defined(MAC_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__NetBSD__) +#if !defined(WIN32) && !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__NetBSD__) try { std::locale(""); // Raises a runtime error if current locale is invalid } catch (const std::runtime_error&) { diff --git a/src/init.cpp b/src/init.cpp index 6604d2733d29e..1f4552c5efb2a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1062,9 +1062,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (!blockman_result) { return InitError(util::ErrorString(blockman_result)); } - CTxMemPool::Options mempool_opts{ - .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, - }; + CTxMemPool::Options mempool_opts{}; auto mempool_result{ApplyArgsManOptions(args, chainparams, mempool_opts)}; if (!mempool_result) { return InitError(util::ErrorString(mempool_result)); @@ -1173,7 +1171,7 @@ bool CheckHostPortOptions(const ArgsManager& args) { return true; } -// A GUI user may opt to retry once if there is a failure during chainstate initialization. +// A GUI user may opt to retry once with do_reindex set if there is a failure during chainstate initialization. // The function therefore has to support re-entry. static ChainstateLoadResult InitAndLoadChainstate( NodeContext& node, @@ -1253,7 +1251,7 @@ static ChainstateLoadResult InitAndLoadChainstate( return f(); } catch (const std::exception& e) { LogError("%s\n", e.what()); - return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database")); + return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error loading databases")); } }; auto [status, error] = catch_exceptions([&] { return LoadChainstate(chainman, cache_sizes, options); }); @@ -1634,11 +1632,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (status == ChainstateLoadStatus::FAILURE && !do_reindex && !ShutdownRequested(node)) { // suggest a reindex bool do_retry = uiInterface.ThreadSafeQuestion( - error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"), + error + Untranslated(".\n\n") + _("Do you want to rebuild the databases now?"), error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.", "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT); if (!do_retry) { - LogError("Aborted block database rebuild. Exiting.\n"); return false; } do_reindex = true; @@ -1658,7 +1655,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // As LoadBlockIndex can take several minutes, it's possible the user // requested to kill the GUI during the last operation. If so, exit. - // As the program has not fully started yet, Shutdown() is possibly overkill. if (ShutdownRequested(node)) { LogPrintf("Shutdown requested. Exiting.\n"); return false; diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index ae5b28c481d8f..cc26042e3fcdc 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -123,18 +123,14 @@ if(NOT BUILD_SHARED_LIBS) set(all_kernel_static_link_libs "") get_target_static_link_libs(groestlcoinkernel all_kernel_static_link_libs) + install(TARGETS ${all_kernel_static_link_libs} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Kernel) + list(TRANSFORM all_kernel_static_link_libs PREPEND "-l") # LIBS_PRIVATE is substituted in the pkg-config file. - set(LIBS_PRIVATE "") - foreach(lib ${all_kernel_static_link_libs}) - install(TARGETS ${lib} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) - string(APPEND LIBS_PRIVATE " -l${lib}") - endforeach() - - string(STRIP "${LIBS_PRIVATE}" LIBS_PRIVATE) + list(JOIN all_kernel_static_link_libs " " LIBS_PRIVATE) endif() configure_file(${PROJECT_SOURCE_DIR}/libgroestlcoinkernel.pc.in ${PROJECT_BINARY_DIR}/libgroestlcoinkernel.pc @ONLY) -install(FILES ${PROJECT_BINARY_DIR}/libgroestlcoinkernel.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig") +install(FILES ${PROJECT_BINARY_DIR}/libgroestlcoinkernel.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig" COMPONENT Kernel) include(GNUInstallDirs) install(TARGETS groestlcoinkernel diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index d7e6176be1e96..fa8d7a386f6af 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -41,12 +41,17 @@ static ChainstateLoadResult CompleteChainstateInitialization( // new BlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); - pblocktree = std::make_unique(DBParams{ - .path = chainman.m_options.datadir / "blocks" / "index", - .cache_bytes = static_cast(cache_sizes.block_tree_db), - .memory_only = options.block_tree_db_in_memory, - .wipe_data = options.wipe_block_tree_db, - .options = chainman.m_options.block_tree_db}); + try { + pblocktree = std::make_unique(DBParams{ + .path = chainman.m_options.datadir / "blocks" / "index", + .cache_bytes = static_cast(cache_sizes.block_tree_db), + .memory_only = options.block_tree_db_in_memory, + .wipe_data = options.wipe_block_tree_db, + .options = chainman.m_options.block_tree_db}); + } catch (dbwrapper_error& err) { + LogError("%s\n", err.what()); + return {ChainstateLoadStatus::FAILURE, _("Error opening block database")}; + } if (options.wipe_block_tree_db) { pblocktree->WriteReindexing(true); @@ -107,10 +112,15 @@ static ChainstateLoadResult CompleteChainstateInitialization( for (Chainstate* chainstate : chainman.GetAll()) { LogPrintf("Initializing chainstate %s\n", chainstate->ToString()); - chainstate->InitCoinsDB( - /*cache_size_bytes=*/chainman.m_total_coinsdb_cache * init_cache_fraction, - /*in_memory=*/options.coins_db_in_memory, - /*should_wipe=*/options.wipe_chainstate_db); + try { + chainstate->InitCoinsDB( + /*cache_size_bytes=*/chainman.m_total_coinsdb_cache * init_cache_fraction, + /*in_memory=*/options.coins_db_in_memory, + /*should_wipe=*/options.wipe_chainstate_db); + } catch (dbwrapper_error& err) { + LogError("%s\n", err.what()); + return {ChainstateLoadStatus::FAILURE, _("Error opening coins database")}; + } if (options.coins_error_cb) { chainstate->CoinsErrorCatcher().AddReadErrCallback(options.coins_error_cb); @@ -236,7 +246,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize return {init_status, init_error}; } } else { - return {ChainstateLoadStatus::FAILURE, _( + return {ChainstateLoadStatus::FAILURE_FATAL, _( "UTXO snapshot failed to validate. " "Restart to resume normal initial block download, or try loading a different snapshot.")}; } diff --git a/src/node/coin.cpp b/src/node/coin.cpp index 221854c5f67d5..eba67f4cb50b9 100644 --- a/src/node/coin.cpp +++ b/src/node/coin.cpp @@ -16,10 +16,11 @@ void FindCoins(const NodeContext& node, std::map& coins) LOCK2(cs_main, node.mempool->cs); CCoinsViewCache& chain_view = node.chainman->ActiveChainstate().CoinsTip(); CCoinsViewMemPool mempool_view(&chain_view, *node.mempool); - for (auto& coin : coins) { - if (!mempool_view.GetCoin(coin.first, coin.second)) { - // Either the coin is not in the CCoinsViewCache or is spent. Clear it. - coin.second.Clear(); + for (auto& [outpoint, coin] : coins) { + if (auto c{mempool_view.GetCoin(outpoint)}) { + coin = std::move(*c); + } else { + coin.Clear(); // Either the coin is not in the CCoinsViewCache or is spent } } } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 0010c104a88b7..dd4eb229e853e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -358,9 +358,7 @@ class NodeImpl : public Node std::optional getUnspentOutput(const COutPoint& output) override { LOCK(::cs_main); - Coin coin; - if (chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin)) return coin; - return {}; + return chainman().ActiveChainstate().CoinsTip().GetCoin(output); } TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) override { diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index f58a694395b05..431dce3ff5d64 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -5,7 +5,6 @@ #include -#include #include #include #include @@ -32,6 +31,10 @@ #include #include +// The current format written, and the version required to read. Must be +// increased to at least 289900+1 on the next breaking change. +constexpr int CURRENT_FEES_FILE_VERSION{216000}; + static constexpr double INF_FEERATE = 1e99; std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon) @@ -168,7 +171,7 @@ class TxConfirmStats * Read saved state of estimation data from a file and replace all internal data structures and * variables with this state. */ - void Read(AutoFile& filein, int nFileVersion, size_t numBuckets); + void Read(AutoFile& filein, size_t numBuckets); }; @@ -414,7 +417,7 @@ void TxConfirmStats::Write(AutoFile& fileout) const fileout << Using>>(failAvg); } -void TxConfirmStats::Read(AutoFile& filein, int nFileVersion, size_t numBuckets) +void TxConfirmStats::Read(AutoFile& filein, size_t numBuckets) { // Read data file and do some very basic sanity checking // buckets and bucketMap are not updated yet, so don't access them @@ -961,8 +964,8 @@ bool CBlockPolicyEstimator::Write(AutoFile& fileout) const { try { LOCK(m_cs_fee_estimator); - fileout << 216000; // version required to read: 2.16.0 or later - fileout << CLIENT_VERSION; // version that wrote the file + fileout << CURRENT_FEES_FILE_VERSION; + fileout << int{0}; // Unused dummy field. Written files may contain any value in [0, 289900] fileout << nBestSeenHeight; if (BlockSpan() > HistoricalBlockSpan()/2) { fileout << firstRecordedHeight << nBestSeenHeight; @@ -976,7 +979,7 @@ bool CBlockPolicyEstimator::Write(AutoFile& fileout) const longStats->Write(fileout); } catch (const std::exception&) { - LogPrintf("CBlockPolicyEstimator::Write(): unable to write policy estimator data (non-fatal)\n"); + LogWarning("Unable to write policy estimator data (non-fatal)"); return false; } return true; @@ -986,10 +989,10 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein) { try { LOCK(m_cs_fee_estimator); - int nVersionRequired, nVersionThatWrote; - filein >> nVersionRequired >> nVersionThatWrote; - if (nVersionRequired > CLIENT_VERSION) { - throw std::runtime_error(strprintf("up-version (%d) fee estimate file", nVersionRequired)); + int nVersionRequired, dummy; + filein >> nVersionRequired >> dummy; + if (nVersionRequired > CURRENT_FEES_FILE_VERSION) { + throw std::runtime_error{strprintf("File version (%d) too high to be read.", nVersionRequired)}; } // Read fee estimates file into temporary variables so existing data @@ -997,9 +1000,9 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein) unsigned int nFileBestSeenHeight; filein >> nFileBestSeenHeight; - if (nVersionRequired < 216000) { - LogPrintf("%s: incompatible old fee estimation data (non-fatal). Version: %d\n", __func__, nVersionRequired); - } else { // New format introduced in 216000 + if (nVersionRequired < CURRENT_FEES_FILE_VERSION) { + LogWarning("Incompatible old fee estimation data (non-fatal). Version: %d", nVersionRequired); + } else { // nVersionRequired == CURRENT_FEES_FILE_VERSION unsigned int nFileHistoricalFirst, nFileHistoricalBest; filein >> nFileHistoricalFirst >> nFileHistoricalBest; if (nFileHistoricalFirst > nFileHistoricalBest || nFileHistoricalBest > nFileBestSeenHeight) { @@ -1015,9 +1018,9 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein) std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MED_BLOCK_PERIODS, MED_DECAY, MED_SCALE)); std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE)); std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE)); - fileFeeStats->Read(filein, nVersionThatWrote, numBuckets); - fileShortStats->Read(filein, nVersionThatWrote, numBuckets); - fileLongStats->Read(filein, nVersionThatWrote, numBuckets); + fileFeeStats->Read(filein, numBuckets); + fileShortStats->Read(filein, numBuckets); + fileLongStats->Read(filein, numBuckets); // Fee estimates file parsed correctly // Copy buckets from file and refresh our bucketmap @@ -1038,7 +1041,7 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein) } } catch (const std::exception& e) { - LogPrintf("CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal): %s\n",e.what()); + LogWarning("Unable to read policy estimator data (non-fatal): %s", e.what()); return false; } return true; diff --git a/src/qt/test/CMakeLists.txt b/src/qt/test/CMakeLists.txt index 58be04c4512c7..38f213d5da824 100644 --- a/src/qt/test/CMakeLists.txt +++ b/src/qt/test/CMakeLists.txt @@ -32,14 +32,6 @@ if(ENABLE_WALLET) ) endif() -if(NOT QT_IS_STATIC) - add_custom_command( - TARGET test_groestlcoin-qt POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different $>> $/plugins/platforms - VERBATIM - ) -endif() - add_test(NAME test_groestlcoin-qt COMMAND test_groestlcoin-qt ) diff --git a/src/random.cpp b/src/random.cpp index 163112585ac4e..9b5131023f764 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -34,7 +34,7 @@ #include #endif -#if defined(HAVE_GETRANDOM) || (defined(HAVE_GETENTROPY_RAND) && defined(MAC_OSX)) +#if defined(HAVE_GETRANDOM) || (defined(HAVE_GETENTROPY_RAND) && defined(__APPLE__)) #include #endif @@ -387,7 +387,7 @@ void GetOSRand(unsigned char *ent32) The function call is always successful. */ arc4random_buf(ent32, NUM_OS_RANDOM_BYTES); -#elif defined(HAVE_GETENTROPY_RAND) && defined(MAC_OSX) +#elif defined(HAVE_GETENTROPY_RAND) && defined(__APPLE__) if (getentropy(ent32, NUM_OS_RANDOM_BYTES) != 0) { RandFailure(); } @@ -599,7 +599,7 @@ void SeedPeriodic(CSHA512& hasher, RNGState& rng) noexcept // Add the events hasher into the mix rng.SeedEvents(hasher); - // Dynamic environment data (performance monitoring, ...) + // Dynamic environment data (clocks, resource usage, ...) auto old_size = hasher.Size(); RandAddDynamicEnv(hasher); LogDebug(BCLog::RAND, "Feeding %i bytes of dynamic environment data into RNG\n", hasher.Size() - old_size); @@ -616,7 +616,7 @@ void SeedStartup(CSHA512& hasher, RNGState& rng) noexcept // Everything that the 'slow' seeder includes. SeedSlow(hasher, rng); - // Dynamic environment data (performance monitoring, ...) + // Dynamic environment data (clocks, resource usage, ...) auto old_size = hasher.Size(); RandAddDynamicEnv(hasher); diff --git a/src/random.h b/src/random.h index 536e697ccaf01..2a26ca986be47 100644 --- a/src/random.h +++ b/src/random.h @@ -49,7 +49,7 @@ * * - RandAddPeriodic() seeds everything that fast seeding includes, but additionally: * - A high-precision timestamp - * - Dynamic environment data (performance monitoring, ...) + * - Dynamic environment data (clocks, resource usage, ...) * - Strengthen the entropy for 10 ms using repeated SHA512. * This is run once every minute. * diff --git a/src/randomenv.cpp b/src/randomenv.cpp index dee48481c5a47..7a46a5109bce2 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -28,7 +28,6 @@ #ifdef WIN32 #include -#include #else #include #include @@ -64,45 +63,6 @@ extern char** environ; // NOLINT(readability-redundant-declaration): Necessary o namespace { -void RandAddSeedPerfmon(CSHA512& hasher) -{ -#ifdef WIN32 - // Seed with the entire set of perfmon data - - // This can take up to 2 seconds, so only do it every 10 minutes. - // Initialize last_perfmon to 0 seconds, we don't skip the first call. - static std::atomic last_perfmon{SteadyClock::time_point{0s}}; - auto last_time = last_perfmon.load(); - auto current_time = SteadyClock::now(); - if (current_time < last_time + 10min) return; - last_perfmon = current_time; - - std::vector vData(250000, 0); - long ret = 0; - unsigned long nSize = 0; - const size_t nMaxSize = 10000000; // Bail out at more than 10MB of performance data - while (true) { - nSize = vData.size(); - ret = RegQueryValueExA(HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, vData.data(), &nSize); - if (ret != ERROR_MORE_DATA || vData.size() >= nMaxSize) - break; - vData.resize(std::min((vData.size() * 3) / 2, nMaxSize)); // Grow size of buffer exponentially - } - RegCloseKey(HKEY_PERFORMANCE_DATA); - if (ret == ERROR_SUCCESS) { - hasher.Write(vData.data(), nSize); - memory_cleanse(vData.data(), nSize); - } else { - // Performance data is only a best-effort attempt at improving the - // situation when the OS randomness (and other sources) aren't - // adequate. As a result, failure to read it is isn't considered critical, - // so we don't call RandFailure(). - // TODO: Add logging when the logger is made functional before global - // constructors have been invoked. - } -#endif -} - /** Helper to easily feed data into a CSHA512. * * Note that this does not serialize the passed object (like stream.h's << operators do). @@ -227,8 +187,6 @@ void AddAllCPUID(CSHA512& hasher) void RandAddDynamicEnv(CSHA512& hasher) { - RandAddSeedPerfmon(hasher); - // Various clocks #ifdef WIN32 FILETIME ftime; diff --git a/src/rest.cpp b/src/rest.cpp index ca26c699b5022..9b282dc484f99 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -870,10 +870,9 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: { auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) { for (const COutPoint& vOutPoint : vOutPoints) { - Coin coin; - bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin); - hits.push_back(hit); - if (hit) outs.emplace_back(std::move(coin)); + auto coin = !mempool || !mempool->isSpent(vOutPoint) ? view.GetCoin(vOutPoint) : std::nullopt; + hits.push_back(coin.has_value()); + if (coin) outs.emplace_back(std::move(*coin)); } active_height = chainman.ActiveHeight(); active_hash = chainman.ActiveTip()->GetBlockHash(); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8c48157f2eb26..5bf3ae622676e 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1133,35 +1133,32 @@ static RPCHelpMan gettxout() if (!request.params[2].isNull()) fMempool = request.params[2].get_bool(); - Coin coin; Chainstate& active_chainstate = chainman.ActiveChainstate(); CCoinsViewCache* coins_view = &active_chainstate.CoinsTip(); + std::optional coin; if (fMempool) { const CTxMemPool& mempool = EnsureMemPool(node); LOCK(mempool.cs); CCoinsViewMemPool view(coins_view, mempool); - if (!view.GetCoin(out, coin) || mempool.isSpent(out)) { - return UniValue::VNULL; - } + if (!mempool.isSpent(out)) coin = view.GetCoin(out); } else { - if (!coins_view->GetCoin(out, coin)) { - return UniValue::VNULL; - } + coin = coins_view->GetCoin(out); } + if (!coin) return UniValue::VNULL; const CBlockIndex* pindex = active_chainstate.m_blockman.LookupBlockIndex(coins_view->GetBestBlock()); ret.pushKV("bestblock", pindex->GetBlockHash().GetHex()); - if (coin.nHeight == MEMPOOL_HEIGHT) { + if (coin->nHeight == MEMPOOL_HEIGHT) { ret.pushKV("confirmations", 0); } else { - ret.pushKV("confirmations", (int64_t)(pindex->nHeight - coin.nHeight + 1)); + ret.pushKV("confirmations", (int64_t)(pindex->nHeight - coin->nHeight + 1)); } - ret.pushKV("value", ValueFromAmount(coin.out.nValue)); + ret.pushKV("value", ValueFromAmount(coin->out.nValue)); UniValue o(UniValue::VOBJ); - ScriptToUniv(coin.out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); + ScriptToUniv(coin->out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); ret.pushKV("scriptPubKey", std::move(o)); - ret.pushKV("coinbase", (bool)coin.fCoinBase); + ret.pushKV("coinbase", (bool)coin->fCoinBase); return ret; }, diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 0d18cd0c2b5cd..ec4720966b0d9 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -44,18 +44,14 @@ class CCoinsViewTest : public CCoinsView public: CCoinsViewTest(FastRandomContext& rng) : m_rng{rng} {} - [[nodiscard]] bool GetCoin(const COutPoint& outpoint, Coin& coin) const override + std::optional GetCoin(const COutPoint& outpoint) const override { - std::map::const_iterator it = map_.find(outpoint); - if (it == map_.end()) { - return false; - } - coin = it->second; - if (coin.IsSpent() && m_rng.randbool() == 0) { - // Randomly return false in case of an empty entry. - return false; + if (auto it{map_.find(outpoint)}; it != map_.end()) { + if (!it->second.IsSpent() || m_rng.randbool()) { + return it->second; // TODO spent coins shouldn't be returned + } } - return true; + return std::nullopt; } uint256 GetBestBlock() const override { return hashBestBlock_; } diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index bcc3dd3e14a69..a7e7f49d9f470 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -39,13 +39,6 @@ void initialize_addrman() g_setup = testing_setup.get(); } -[[nodiscard]] inline NetGroupManager ConsumeNetGroupManager(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap, 128)) asmap.clear(); - return NetGroupManager(asmap); -} - FUZZ_TARGET(data_stream_addr_man, .init = initialize_addrman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; @@ -118,121 +111,19 @@ void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider) } } -class AddrManDeterministic : public AddrMan -{ -public: - explicit AddrManDeterministic(const NetGroupManager& netgroupman, FuzzedDataProvider& fuzzed_data_provider) - : AddrMan(netgroupman, /*deterministic=*/true, GetCheckRatio()) - { - WITH_LOCK(m_impl->cs, m_impl->insecure_rand.Reseed(ConsumeUInt256(fuzzed_data_provider))); - } - - /** - * Compare with another AddrMan. - * This compares: - * - the values in `mapInfo` (the keys aka ids are ignored) - * - vvNew entries refer to the same addresses - * - vvTried entries refer to the same addresses - */ - bool operator==(const AddrManDeterministic& other) const - { - LOCK2(m_impl->cs, other.m_impl->cs); - - if (m_impl->mapInfo.size() != other.m_impl->mapInfo.size() || m_impl->nNew != other.m_impl->nNew || - m_impl->nTried != other.m_impl->nTried) { - return false; - } - - // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. - // Keys may be different. - - auto addrinfo_hasher = [](const AddrInfo& a) { - CSipHasher hasher(0, 0); - auto addr_key = a.GetKey(); - auto source_key = a.source.GetAddrBytes(); - hasher.Write(TicksSinceEpoch(a.m_last_success)); - hasher.Write(a.nAttempts); - hasher.Write(a.nRefCount); - hasher.Write(a.fInTried); - hasher.Write(a.GetNetwork()); - hasher.Write(a.source.GetNetwork()); - hasher.Write(addr_key.size()); - hasher.Write(source_key.size()); - hasher.Write(addr_key); - hasher.Write(source_key); - return (size_t)hasher.Finalize(); - }; - - auto addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) { - return std::tie(static_cast(lhs), lhs.source, lhs.m_last_success, lhs.nAttempts, lhs.nRefCount, lhs.fInTried) == - std::tie(static_cast(rhs), rhs.source, rhs.m_last_success, rhs.nAttempts, rhs.nRefCount, rhs.fInTried); - }; - - using Addresses = std::unordered_set; - - const size_t num_addresses{m_impl->mapInfo.size()}; - - Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; - for (const auto& [id, addr] : m_impl->mapInfo) { - addresses.insert(addr); - } - - Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; - for (const auto& [id, addr] : other.m_impl->mapInfo) { - other_addresses.insert(addr); - } - - if (addresses != other_addresses) { - return false; - } - - auto IdsReferToSameAddress = [&](nid_type id, nid_type other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) { - if (id == -1 && other_id == -1) { - return true; - } - if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { - return false; - } - return m_impl->mapInfo.at(id) == other.m_impl->mapInfo.at(other_id); - }; - - // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` - // contains just an id and the address is to be found in `mapInfo.at(id)`. The ids - // themselves may differ between `vvNew` and `other.vvNew`. - for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { - for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { - if (!IdsReferToSameAddress(m_impl->vvNew[i][j], other.m_impl->vvNew[i][j])) { - return false; - } - } - } - - // Same for `vvTried`. - for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { - for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { - if (!IdsReferToSameAddress(m_impl->vvTried[i][j], other.m_impl->vvTried[i][j])) { - return false; - } - } - } - - return true; - } -}; - FUZZ_TARGET(addrman, .init = initialize_addrman) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; - auto addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider); + auto addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio()); if (fuzzed_data_provider.ConsumeBool()) { const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; DataStream ds{serialized_data}; try { ds >> *addr_man_ptr; } catch (const std::ios_base::failure&) { - addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider); + addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio()); } } AddrManDeterministic& addr_man = *addr_man_ptr; @@ -310,8 +201,8 @@ FUZZ_TARGET(addrman_serdeser, .init = initialize_addrman) SetMockTime(ConsumeTime(fuzzed_data_provider)); NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; - AddrManDeterministic addr_man1{netgroupman, fuzzed_data_provider}; - AddrManDeterministic addr_man2{netgroupman, fuzzed_data_provider}; + AddrManDeterministic addr_man1{netgroupman, fuzzed_data_provider, GetCheckRatio()}; + AddrManDeterministic addr_man2{netgroupman, fuzzed_data_provider, GetCheckRatio()}; DataStream data_stream{}; diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 368c69819a0e8..54bfb93b4915e 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -162,22 +162,20 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) const bool exists_using_access_coin = !(coin_using_access_coin == EMPTY_COIN); const bool exists_using_have_coin = coins_view_cache.HaveCoin(random_out_point); const bool exists_using_have_coin_in_cache = coins_view_cache.HaveCoinInCache(random_out_point); - Coin coin_using_get_coin; - const bool exists_using_get_coin = coins_view_cache.GetCoin(random_out_point, coin_using_get_coin); - if (exists_using_get_coin) { - assert(coin_using_get_coin == coin_using_access_coin); + if (auto coin{coins_view_cache.GetCoin(random_out_point)}) { + assert(*coin == coin_using_access_coin); + assert(exists_using_access_coin && exists_using_have_coin_in_cache && exists_using_have_coin); + } else { + assert(!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin); } - assert((exists_using_access_coin && exists_using_have_coin_in_cache && exists_using_have_coin && exists_using_get_coin) || - (!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin && !exists_using_get_coin)); // If HaveCoin on the backend is true, it must also be on the cache if the coin wasn't spent. const bool exists_using_have_coin_in_backend = backend_coins_view.HaveCoin(random_out_point); if (!coin_using_access_coin.IsSpent() && exists_using_have_coin_in_backend) { assert(exists_using_have_coin); } - Coin coin_using_backend_get_coin; - if (backend_coins_view.GetCoin(random_out_point, coin_using_backend_get_coin)) { + if (auto coin{backend_coins_view.GetCoin(random_out_point)}) { assert(exists_using_have_coin_in_backend); - // Note we can't assert that `coin_using_get_coin == coin_using_backend_get_coin` because the coin in + // Note we can't assert that `coin_using_get_coin == *coin` because the coin in // the cache may have been modified but not yet flushed. } else { assert(!exists_using_have_coin_in_backend); diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index 8e717e96b4ed8..6000d52fc971b 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -137,9 +137,8 @@ struct CacheLevel /** Class for the base of the hierarchy (roughly simulating a memory-backed CCoinsViewDB). * - * The initial state consists of the empty UTXO set, though coins whose output index - * is 3 (mod 5) always have GetCoin() succeed (but returning an IsSpent() coin unless a UTXO - * exists). Coins whose output index is 4 (mod 5) have GetCoin() always succeed after being spent. + * The initial state consists of the empty UTXO set. + * Coins whose output index is 4 (mod 5) have GetCoin() always succeed after being spent. * This exercises code paths with spent, non-DIRTY cache entries. */ class CoinsViewBottom final : public CCoinsView @@ -147,19 +146,11 @@ class CoinsViewBottom final : public CCoinsView std::map m_data; public: - bool GetCoin(const COutPoint& outpoint, Coin& coin) const final + std::optional GetCoin(const COutPoint& outpoint) const final { - auto it = m_data.find(outpoint); - if (it == m_data.end()) { - if ((outpoint.n % 5) == 3) { - coin.Clear(); - return true; - } - return false; - } else { - coin = it->second; - return true; - } + // TODO GetCoin shouldn't return spent coins + if (auto it = m_data.find(outpoint); it != m_data.end()) return it->second; + return std::nullopt; } bool HaveCoin(const COutPoint& outpoint) const final @@ -270,17 +261,16 @@ FUZZ_TARGET(coinscache_sim) // Look up in simulation data. auto sim = lookup(outpointidx); // Look up in real caches. - Coin realcoin; - auto real = caches.back()->GetCoin(data.outpoints[outpointidx], realcoin); + auto realcoin = caches.back()->GetCoin(data.outpoints[outpointidx]); // Compare results. if (!sim.has_value()) { - assert(!real || realcoin.IsSpent()); + assert(!realcoin || realcoin->IsSpent()); } else { - assert(real && !realcoin.IsSpent()); + assert(realcoin && !realcoin->IsSpent()); const auto& simcoin = data.coins[sim->first]; - assert(realcoin.out == simcoin.out); - assert(realcoin.fCoinBase == simcoin.fCoinBase); - assert(realcoin.nHeight == sim->second); + assert(realcoin->out == simcoin.out); + assert(realcoin->fCoinBase == simcoin.fCoinBase); + assert(realcoin->nHeight == sim->second); } }, @@ -465,16 +455,15 @@ FUZZ_TARGET(coinscache_sim) // Compare the bottom coinsview (not a CCoinsViewCache) with sim_cache[0]. for (uint32_t outpointidx = 0; outpointidx < NUM_OUTPOINTS; ++outpointidx) { - Coin realcoin; - bool real = bottom.GetCoin(data.outpoints[outpointidx], realcoin); + auto realcoin = bottom.GetCoin(data.outpoints[outpointidx]); auto sim = lookup(outpointidx, 0); if (!sim.has_value()) { - assert(!real || realcoin.IsSpent()); + assert(!realcoin || realcoin->IsSpent()); } else { - assert(real && !realcoin.IsSpent()); - assert(realcoin.out == data.coins[sim->first].out); - assert(realcoin.fCoinBase == data.coins[sim->first].fCoinBase); - assert(realcoin.nHeight == sim->second); + assert(realcoin && !realcoin->IsSpent()); + assert(realcoin->out == data.coins[sim->first].out); + assert(realcoin->fCoinBase == data.coins[sim->first].fCoinBase); + assert(realcoin->nHeight == sim->second); } } } diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index beefc9d82edbe..f2bf44c76139e 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -20,6 +20,12 @@ namespace { const TestingSetup* g_setup; + +int32_t GetCheckRatio() +{ + return std::clamp(g_setup->m_node.args->GetIntArg("-checkaddrman", 0), 0, 1000000); +} + } // namespace void initialize_connman() @@ -32,10 +38,22 @@ FUZZ_TARGET(connman, .init = initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); + auto netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + auto addr_man_ptr{std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio())}; + if (fuzzed_data_provider.ConsumeBool()) { + const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; + DataStream ds{serialized_data}; + try { + ds >> *addr_man_ptr; + } catch (const std::ios_base::failure&) { + addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio()); + } + } + AddrManDeterministic& addr_man{*addr_man_ptr}; ConnmanTestMsg connman{fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral(), - *g_setup->m_node.addrman, - *g_setup->m_node.netgroupman, + addr_man, + netgroupman, Params(), fuzzed_data_provider.ConsumeBool()}; @@ -140,6 +158,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) (void)connman.GetTotalBytesSent(); (void)connman.GetTryNewOutboundPeer(); (void)connman.GetUseAddrmanOutgoing(); + (void)connman.ASMapHealthCheck(); connman.ClearTestNodes(); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 64861311dbdc2..39aa404484e2e 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -214,9 +214,8 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool) // Helper to query an amount const CCoinsViewMemPool amount_view{WITH_LOCK(::cs_main, return &chainstate.CoinsTip()), tx_pool}; const auto GetAmount = [&](const COutPoint& outpoint) { - Coin c; - Assert(amount_view.GetCoin(outpoint, c)); - return c.out.nValue; + auto coin{amount_view.GetCoin(outpoint).value()}; + return coin.out.nValue; }; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index 1a5902329e26a..cc73cdff4b795 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_TEST_FUZZ_UTIL_NET_H #define BITCOIN_TEST_FUZZ_UTIL_NET_H +#include +#include #include #include #include @@ -15,6 +17,7 @@ #include #include #include +#include #include #include @@ -34,6 +37,108 @@ */ CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext* rand = nullptr) noexcept; +class AddrManDeterministic : public AddrMan +{ +public: + explicit AddrManDeterministic(const NetGroupManager& netgroupman, FuzzedDataProvider& fuzzed_data_provider, int32_t check_ratio) + : AddrMan(netgroupman, /*deterministic=*/true, check_ratio) + { + WITH_LOCK(m_impl->cs, m_impl->insecure_rand.Reseed(ConsumeUInt256(fuzzed_data_provider))); + } + + /** + * Compare with another AddrMan. + * This compares: + * - the values in `mapInfo` (the keys aka ids are ignored) + * - vvNew entries refer to the same addresses + * - vvTried entries refer to the same addresses + */ + bool operator==(const AddrManDeterministic& other) const + { + LOCK2(m_impl->cs, other.m_impl->cs); + + if (m_impl->mapInfo.size() != other.m_impl->mapInfo.size() || m_impl->nNew != other.m_impl->nNew || + m_impl->nTried != other.m_impl->nTried) { + return false; + } + + // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. + // Keys may be different. + + auto addrinfo_hasher = [](const AddrInfo& a) { + CSipHasher hasher(0, 0); + auto addr_key = a.GetKey(); + auto source_key = a.source.GetAddrBytes(); + hasher.Write(TicksSinceEpoch(a.m_last_success)); + hasher.Write(a.nAttempts); + hasher.Write(a.nRefCount); + hasher.Write(a.fInTried); + hasher.Write(a.GetNetwork()); + hasher.Write(a.source.GetNetwork()); + hasher.Write(addr_key.size()); + hasher.Write(source_key.size()); + hasher.Write(addr_key); + hasher.Write(source_key); + return (size_t)hasher.Finalize(); + }; + + auto addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) { + return std::tie(static_cast(lhs), lhs.source, lhs.m_last_success, lhs.nAttempts, lhs.nRefCount, lhs.fInTried) == + std::tie(static_cast(rhs), rhs.source, rhs.m_last_success, rhs.nAttempts, rhs.nRefCount, rhs.fInTried); + }; + + using Addresses = std::unordered_set; + + const size_t num_addresses{m_impl->mapInfo.size()}; + + Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : m_impl->mapInfo) { + addresses.insert(addr); + } + + Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : other.m_impl->mapInfo) { + other_addresses.insert(addr); + } + + if (addresses != other_addresses) { + return false; + } + + auto IdsReferToSameAddress = [&](nid_type id, nid_type other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) { + if (id == -1 && other_id == -1) { + return true; + } + if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { + return false; + } + return m_impl->mapInfo.at(id) == other.m_impl->mapInfo.at(other_id); + }; + + // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` + // contains just an id and the address is to be found in `mapInfo.at(id)`. The ids + // themselves may differ between `vvNew` and `other.vvNew`. + for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(m_impl->vvNew[i][j], other.m_impl->vvNew[i][j])) { + return false; + } + } + } + + // Same for `vvTried`. + for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(m_impl->vvTried[i][j], other.m_impl->vvTried[i][j])) { + return false; + } + } + } + + return true; + } +}; + class FuzzedSock : public Sock { FuzzedDataProvider& m_fuzzed_data_provider; @@ -93,6 +198,13 @@ class FuzzedSock : public Sock return FuzzedSock{fuzzed_data_provider}; } +[[nodiscard]] inline NetGroupManager ConsumeNetGroupManager(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(asmap, 128)) asmap.clear(); + return NetGroupManager(asmap); +} + inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept { return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral()}; diff --git a/src/test/fuzz/util/wallet.h b/src/test/fuzz/util/wallet.h new file mode 100644 index 0000000000000..8b55b7a985aec --- /dev/null +++ b/src/test/fuzz/util/wallet.h @@ -0,0 +1,134 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_FUZZ_UTIL_WALLET_H +#define BITCOIN_TEST_FUZZ_UTIL_WALLET_H + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace wallet { + +/** + * Wraps a descriptor wallet for fuzzing. + */ +struct FuzzedWallet { + std::shared_ptr wallet; + FuzzedWallet(interfaces::Chain& chain, const std::string& name, const std::string& seed_insecure) + { + wallet = std::make_shared(&chain, name, CreateMockableWalletDatabase()); + { + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + auto height{*Assert(chain.getHeight())}; + wallet->SetLastBlockProcessed(height, chain.getBlockHash(height)); + } + wallet->m_keypool_size = 1; // Avoid timeout in TopUp() + assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + ImportDescriptors(seed_insecure); + } + void ImportDescriptors(const std::string& seed_insecure) + { + const std::vector DESCS{ + "pkh(%s/%s/*)", + "sh(wpkh(%s/%s/*))", + "tr(%s/%s/*)", + "wpkh(%s/%s/*)", + }; + + for (const std::string& desc_fmt : DESCS) { + for (bool internal : {true, false}) { + const auto descriptor{(strprintf)(desc_fmt, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})}; + + FlatSigningProvider keys; + std::string error; + auto parsed_desc = std::move(Parse(descriptor, keys, error, /*require_checksum=*/false).at(0)); + assert(parsed_desc); + assert(error.empty()); + assert(parsed_desc->IsRange()); + assert(parsed_desc->IsSingleType()); + assert(!keys.keys.empty()); + WalletDescriptor w_desc{std::move(parsed_desc), /*creation_time=*/0, /*range_start=*/0, /*range_end=*/1, /*next_index=*/0}; + assert(!wallet->GetDescriptorScriptPubKeyMan(w_desc)); + LOCK(wallet->cs_wallet); + auto spk_manager{wallet->AddWalletDescriptor(w_desc, keys, /*label=*/"", internal)}; + assert(spk_manager); + wallet->AddActiveScriptPubKeyMan(spk_manager->GetID(), *Assert(w_desc.descriptor->GetOutputType()), internal); + } + } + } + CTxDestination GetDestination(FuzzedDataProvider& fuzzed_data_provider) + { + auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; + if (fuzzed_data_provider.ConsumeBool()) { + return *Assert(wallet->GetNewDestination(type, "")); + } else { + return *Assert(wallet->GetNewChangeDestination(type)); + } + } + CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { return GetScriptForDestination(GetDestination(fuzzed_data_provider)); } + void FundTx(FuzzedDataProvider& fuzzed_data_provider, CMutableTransaction tx) + { + // The fee of "tx" is 0, so this is the total input and output amount + const CAmount total_amt{ + std::accumulate(tx.vout.begin(), tx.vout.end(), CAmount{}, [](CAmount t, const CTxOut& out) { return t + out.nValue; })}; + const uint32_t tx_size(GetVirtualTransactionSize(CTransaction{tx})); + std::set subtract_fee_from_outputs; + if (fuzzed_data_provider.ConsumeBool()) { + for (size_t i{}; i < tx.vout.size(); ++i) { + if (fuzzed_data_provider.ConsumeBool()) { + subtract_fee_from_outputs.insert(i); + } + } + } + std::vector recipients; + for (size_t idx = 0; idx < tx.vout.size(); idx++) { + const CTxOut& tx_out = tx.vout[idx]; + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + CRecipient recipient = {dest, tx_out.nValue, subtract_fee_from_outputs.count(idx) == 1}; + recipients.push_back(recipient); + } + CCoinControl coin_control; + coin_control.m_allow_other_inputs = fuzzed_data_provider.ConsumeBool(); + CallOneOf( + fuzzed_data_provider, [&] { coin_control.destChange = GetDestination(fuzzed_data_provider); }, + [&] { coin_control.m_change_type.emplace(fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)); }, + [&] { /* no op (leave uninitialized) */ }); + coin_control.fAllowWatchOnly = fuzzed_data_provider.ConsumeBool(); + coin_control.m_include_unsafe_inputs = fuzzed_data_provider.ConsumeBool(); + { + auto& r{coin_control.m_signal_bip125_rbf}; + CallOneOf( + fuzzed_data_provider, [&] { r = true; }, [&] { r = false; }, [&] { r = std::nullopt; }); + } + coin_control.m_feerate = CFeeRate{ + // A fee of this range should cover all cases + fuzzed_data_provider.ConsumeIntegralInRange(0, 2 * total_amt), + tx_size, + }; + if (fuzzed_data_provider.ConsumeBool()) { + *coin_control.m_feerate += GetMinimumFeeRate(*wallet, coin_control, nullptr); + } + coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool(); + // Add solving data (m_external_provider and SelectExternal)? + + int change_position{fuzzed_data_provider.ConsumeIntegralInRange(-1, tx.vout.size() - 1)}; + bilingual_str error; + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + tx.vout.clear(); + (void)FundTransaction(*wallet, tx, recipients, change_position, /*lockUnspents=*/false, coin_control); + } +}; +} + +#endif // BITCOIN_TEST_FUZZ_UTIL_WALLET_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 7465846356217..2de282dbf6d58 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -433,9 +433,8 @@ std::pair TestChain100Setup::CreateValidTransactio std::map input_coins; CAmount inputs_amount{0}; for (const auto& outpoint_to_spend : inputs) { - // - Use GetCoin to properly populate utxo_to_spend, - Coin utxo_to_spend; - assert(coins_cache.GetCoin(outpoint_to_spend, utxo_to_spend)); + // Use GetCoin to properly populate utxo_to_spend + auto utxo_to_spend{coins_cache.GetCoin(outpoint_to_spend).value()}; input_coins.insert({outpoint_to_spend, utxo_to_spend}); inputs_amount += utxo_to_spend.out.nValue; } diff --git a/src/txdb.cpp b/src/txdb.cpp index 9b43a2b03ef53..1622039d63b55 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -65,8 +65,10 @@ void CCoinsViewDB::ResizeCache(size_t new_cache_size) } } -bool CCoinsViewDB::GetCoin(const COutPoint &outpoint, Coin &coin) const { - return m_db->Read(CoinEntry(&outpoint), coin); +std::optional CCoinsViewDB::GetCoin(const COutPoint& outpoint) const +{ + if (Coin coin; m_db->Read(CoinEntry(&outpoint), coin)) return coin; + return std::nullopt; } bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { diff --git a/src/txdb.h b/src/txdb.h index 412d6c60090cd..565b060dbea23 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -57,7 +57,7 @@ class CCoinsViewDB final : public CCoinsView public: explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options); - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f8f5ec0360639..4756c2020968b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -988,12 +988,12 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { +std::optional CCoinsViewMemPool::GetCoin(const COutPoint& outpoint) const +{ // Check to see if the inputs are made available by another tx in the package. // These Coins would not be available in the underlying CoinsView. if (auto it = m_temp_added.find(outpoint); it != m_temp_added.end()) { - coin = it->second; - return true; + return it->second; } // If an entry in the mempool exists, always return that one, as it's guaranteed to never @@ -1002,14 +1002,13 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { CTransactionRef ptx = mempool.get(outpoint.hash); if (ptx) { if (outpoint.n < ptx->vout.size()) { - coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); + Coin coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); m_non_base_coins.emplace(outpoint); - return true; - } else { - return false; + return coin; } + return std::nullopt; } - return base->GetCoin(outpoint, coin); + return base->GetCoin(outpoint); } void CCoinsViewMemPool::PackageAddTransaction(const CTransactionRef& tx) diff --git a/src/txmempool.h b/src/txmempool.h index d0cb41a078ec2..f914cbd729e8f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -851,7 +851,7 @@ class CCoinsViewMemPool : public CCoinsViewBacked CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); /** GetCoin, returning whether it exists and is not spent. Also updates m_non_base_coins if the * coin is not fetched from base. */ - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; /** Add the coins created by this transaction. These coins are only temporarily stored in * m_temp_added and cannot be flushed to the back end. Only used for package validation. */ void PackageAddTransaction(const CTransactionRef& tx); diff --git a/src/util/check.h b/src/util/check.h index 8f28f5dc94127..187187e593ecb 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -42,9 +42,9 @@ void assertion_fail(std::string_view file, int line, std::string_view func, std: template constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { - if constexpr (IS_ASSERT + if (IS_ASSERT || std::is_constant_evaluated() #ifdef ABORT_ON_FAILED_ASSUME - || true + || true #endif ) { if (!val) { diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index 7ac7b829d8ef9..4d06afe144202 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -117,7 +117,7 @@ bool FileCommit(FILE* file) LogPrintf("FlushFileBuffers failed: %s\n", Win32ErrorString(GetLastError())); return false; } -#elif defined(MAC_OSX) && defined(F_FULLFSYNC) +#elif defined(__APPLE__) && defined(F_FULLFSYNC) if (fcntl(fileno(file), F_FULLFSYNC, 0) == -1) { // Manpage says "value other than -1" is returned on success LogPrintf("fcntl F_FULLFSYNC failed: %s\n", SysErrorString(errno)); return false; @@ -195,7 +195,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length) nFileSize.u.HighPart = nEndPos >> 32; SetFilePointerEx(hFile, nFileSize, 0, FILE_BEGIN); SetEndOfFile(hFile); -#elif defined(MAC_OSX) +#elif defined(__APPLE__) // OSX specific version // NOTE: Contrary to other OS versions, the OSX version assumes that // NOTE: offset is the size of the file. diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp index 432fe2d290b40..e820f96083050 100644 --- a/src/util/threadnames.cpp +++ b/src/util/threadnames.cpp @@ -29,7 +29,7 @@ static void SetThreadName(const char* name) ::prctl(PR_SET_NAME, name, 0, 0, 0); #elif (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)) pthread_set_name_np(pthread_self(), name); -#elif defined(MAC_OSX) +#elif defined(__APPLE__) pthread_setname_np(name); #else // Prevent warnings for unused parameters... diff --git a/src/validation.cpp b/src/validation.cpp index 1c97769a158e1..43366d01f0a7a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -176,18 +176,14 @@ std::optional> CalculatePrevHeights( std::vector prev_heights; prev_heights.resize(tx.vin.size()); for (size_t i = 0; i < tx.vin.size(); ++i) { - const CTxIn& txin = tx.vin[i]; - Coin coin; - if (!coins.GetCoin(txin.prevout, coin)) { + if (auto coin{coins.GetCoin(tx.vin[i].prevout)}) { + prev_heights[i] = coin->nHeight == MEMPOOL_HEIGHT + ? tip.nHeight + 1 // Assume all mempool transaction confirm in the next block. + : coin->nHeight; + } else { LogPrintf("ERROR: %s: Missing input %d in transaction \'%s\'\n", __func__, i, tx.GetHash().GetHex()); return std::nullopt; } - if (coin.nHeight == MEMPOOL_HEIGHT) { - // Assume all mempool transaction confirm in the next block. - prev_heights[i] = tip.nHeight + 1; - } else { - prev_heights[i] = coin.nHeight; - } } return prev_heights; } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index af0c78f0d9ef5..f3fe8a19c198a 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -208,6 +208,7 @@ class BerkeleyBatch : public DatabaseBatch bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; + bool HasActiveTxn() override { return activeTxn != nullptr; } DbTxn* txn() const { return activeTxn; } }; diff --git a/src/wallet/db.h b/src/wallet/db.h index 049af8dd19e36..e8790006a4d8b 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -122,6 +122,7 @@ class DatabaseBatch virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; + virtual bool HasActiveTxn() = 0; }; /** An instance of this class represents one database. diff --git a/src/wallet/migrate.h b/src/wallet/migrate.h index 58c8c0adf4e2f..16eadeb019d56 100644 --- a/src/wallet/migrate.h +++ b/src/wallet/migrate.h @@ -115,6 +115,7 @@ class BerkeleyROBatch : public DatabaseBatch bool TxnBegin() override { return false; } bool TxnCommit() override { return false; } bool TxnAbort() override { return false; } + bool HasActiveTxn() override { return false; } }; //! Return object giving access to Berkeley Read Only database at specified path. diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 04c02b0dcc677..0ac1b66897e7a 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -44,6 +44,7 @@ class DummyBatch : public DatabaseBatch bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } + bool HasActiveTxn() override { return false; } }; /** A dummy WalletDatabase that does nothing and never fails. Only used by salvage. diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 46ec5dc1acce0..62384056dc6e4 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1807,6 +1807,12 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() keyid_it++; } + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) { + LogPrintf("Error generating descriptors for migration, cannot initialize db transaction\n"); + return std::nullopt; + } + // keyids is now all non-HD keys. Each key will have its own combo descriptor for (const CKeyID& keyid : keyids) { CKey key; @@ -1837,8 +1843,8 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys auto desc_spk_man = std::make_unique(m_storage, w_desc, /*keypool_size=*/0); - desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); - desc_spk_man->TopUp(); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, key, key.GetPubKey())); + desc_spk_man->TopUpWithDB(batch); auto desc_spks = desc_spk_man->GetScriptPubKeys(); // Remove the scriptPubKeys from our current set @@ -1883,8 +1889,8 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys auto desc_spk_man = std::make_unique(m_storage, w_desc, /*keypool_size=*/0); - desc_spk_man->AddDescriptorKey(master_key.key, master_key.key.GetPubKey()); - desc_spk_man->TopUp(); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())); + desc_spk_man->TopUpWithDB(batch); auto desc_spks = desc_spk_man->GetScriptPubKeys(); // Remove the scriptPubKeys from our current set @@ -1950,9 +1956,9 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() if (!GetKey(keyid, key)) { continue; } - desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, key, key.GetPubKey())); } - desc_spk_man->TopUp(); + desc_spk_man->TopUpWithDB(batch); auto desc_spks_set = desc_spk_man->GetScriptPubKeys(); desc_spks.insert(desc_spks.end(), desc_spks_set.begin(), desc_spks_set.end()); @@ -2019,13 +2025,26 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make sure that we have accounted for all scriptPubKeys assert(spks.size() == 0); + + // Finalize transaction + if (!batch.TxnCommit()) { + LogPrintf("Error generating descriptors for migration, cannot commit db transaction\n"); + return std::nullopt; + } + return out; } bool LegacyDataSPKM::DeleteRecords() +{ + return RunWithinTxn(m_storage.GetDatabase(), /*process_desc=*/"delete legacy records", [&](WalletBatch& batch){ + return DeleteRecordsWithDB(batch); + }); +} + +bool LegacyDataSPKM::DeleteRecordsWithDB(WalletBatch& batch) { LOCK(cs_KeyStore); - WalletBatch batch(m_storage.GetDatabase()); return batch.EraseRecords(DBKeys::LEGACY_TYPES); } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index cf7b7eaf31de7..d8b6c90178a61 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -366,8 +366,9 @@ class LegacyDataSPKM : public ScriptPubKeyMan, public FillableSigningProvider /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. * Does not modify this ScriptPubKeyMan. */ std::optional MigrateToDescriptor(); - /** Delete all the records ofthis LegacyScriptPubKeyMan from disk*/ + /** Delete all the records of this LegacyScriptPubKeyMan from disk*/ bool DeleteRecords(); + bool DeleteRecordsWithDB(WalletBatch& batch); }; // Implements the full legacy wallet behavior @@ -582,6 +583,7 @@ class LegacySigningProvider : public SigningProvider class DescriptorScriptPubKeyMan : public ScriptPubKeyMan { + friend class LegacyDataSPKM; private: using ScriptPubKeyMap = std::map; // Map of scripts to descriptor range index using PubKeyMap = std::map; // Map of pubkeys involved in scripts to descriptor range index diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 6b84f34366f77..78a3accf890e7 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -95,6 +95,7 @@ class SQLiteBatch : public DatabaseBatch bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; + bool HasActiveTxn() override { return m_txn; } }; /** An instance of this class represents one SQLite3 database. diff --git a/src/wallet/test/fuzz/CMakeLists.txt b/src/wallet/test/fuzz/CMakeLists.txt index c30671db489ec..4e663977c2302 100644 --- a/src/wallet/test/fuzz/CMakeLists.txt +++ b/src/wallet/test/fuzz/CMakeLists.txt @@ -11,6 +11,7 @@ target_sources(fuzz $<$:${CMAKE_CURRENT_LIST_DIR}/notifications.cpp> parse_iso8601.cpp $<$:${CMAKE_CURRENT_LIST_DIR}/scriptpubkeyman.cpp> + spend.cpp wallet_bdb_parser.cpp ) target_link_libraries(fuzz bitcoin_wallet) diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index a7015f6685b27..1255218f3a0a4 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -53,121 +54,6 @@ void initialize_setup() g_setup = testing_setup.get(); } -void ImportDescriptors(CWallet& wallet, const std::string& seed_insecure) -{ - const std::vector DESCS{ - "pkh(%s/%s/*)", - "sh(wpkh(%s/%s/*))", - "tr(%s/%s/*)", - "wpkh(%s/%s/*)", - }; - - for (const std::string& desc_fmt : DESCS) { - for (bool internal : {true, false}) { - const auto descriptor{(strprintf)(desc_fmt, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})}; - - FlatSigningProvider keys; - std::string error; - auto parsed_desc = std::move(Parse(descriptor, keys, error, /*require_checksum=*/false).at(0)); - assert(parsed_desc); - assert(error.empty()); - assert(parsed_desc->IsRange()); - assert(parsed_desc->IsSingleType()); - assert(!keys.keys.empty()); - WalletDescriptor w_desc{std::move(parsed_desc), /*creation_time=*/0, /*range_start=*/0, /*range_end=*/1, /*next_index=*/0}; - assert(!wallet.GetDescriptorScriptPubKeyMan(w_desc)); - LOCK(wallet.cs_wallet); - auto spk_manager{wallet.AddWalletDescriptor(w_desc, keys, /*label=*/"", internal)}; - assert(spk_manager); - wallet.AddActiveScriptPubKeyMan(spk_manager->GetID(), *Assert(w_desc.descriptor->GetOutputType()), internal); - } - } -} - -/** - * Wraps a descriptor wallet for fuzzing. - */ -struct FuzzedWallet { - std::shared_ptr wallet; - FuzzedWallet(const std::string& name, const std::string& seed_insecure) - { - auto& chain{*Assert(g_setup->m_node.chain)}; - wallet = std::make_shared(&chain, name, CreateMockableWalletDatabase()); - { - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - auto height{*Assert(chain.getHeight())}; - wallet->SetLastBlockProcessed(height, chain.getBlockHash(height)); - } - wallet->m_keypool_size = 1; // Avoid timeout in TopUp() - assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); - ImportDescriptors(*wallet, seed_insecure); - } - CTxDestination GetDestination(FuzzedDataProvider& fuzzed_data_provider) - { - auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - if (fuzzed_data_provider.ConsumeBool()) { - return *Assert(wallet->GetNewDestination(type, "")); - } else { - return *Assert(wallet->GetNewChangeDestination(type)); - } - } - CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { return GetScriptForDestination(GetDestination(fuzzed_data_provider)); } - void FundTx(FuzzedDataProvider& fuzzed_data_provider, CMutableTransaction tx) - { - // The fee of "tx" is 0, so this is the total input and output amount - const CAmount total_amt{ - std::accumulate(tx.vout.begin(), tx.vout.end(), CAmount{}, [](CAmount t, const CTxOut& out) { return t + out.nValue; })}; - const uint32_t tx_size(GetVirtualTransactionSize(CTransaction{tx})); - std::set subtract_fee_from_outputs; - if (fuzzed_data_provider.ConsumeBool()) { - for (size_t i{}; i < tx.vout.size(); ++i) { - if (fuzzed_data_provider.ConsumeBool()) { - subtract_fee_from_outputs.insert(i); - } - } - } - std::vector recipients; - for (size_t idx = 0; idx < tx.vout.size(); idx++) { - const CTxOut& tx_out = tx.vout[idx]; - CTxDestination dest; - ExtractDestination(tx_out.scriptPubKey, dest); - CRecipient recipient = {dest, tx_out.nValue, subtract_fee_from_outputs.count(idx) == 1}; - recipients.push_back(recipient); - } - CCoinControl coin_control; - coin_control.m_allow_other_inputs = fuzzed_data_provider.ConsumeBool(); - CallOneOf( - fuzzed_data_provider, [&] { coin_control.destChange = GetDestination(fuzzed_data_provider); }, - [&] { coin_control.m_change_type.emplace(fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)); }, - [&] { /* no op (leave uninitialized) */ }); - coin_control.fAllowWatchOnly = fuzzed_data_provider.ConsumeBool(); - coin_control.m_include_unsafe_inputs = fuzzed_data_provider.ConsumeBool(); - { - auto& r{coin_control.m_signal_bip125_rbf}; - CallOneOf( - fuzzed_data_provider, [&] { r = true; }, [&] { r = false; }, [&] { r = std::nullopt; }); - } - coin_control.m_feerate = CFeeRate{ - // A fee of this range should cover all cases - fuzzed_data_provider.ConsumeIntegralInRange(0, 2 * total_amt), - tx_size, - }; - if (fuzzed_data_provider.ConsumeBool()) { - *coin_control.m_feerate += GetMinimumFeeRate(*wallet, coin_control, nullptr); - } - coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool(); - // Add solving data (m_external_provider and SelectExternal)? - - int change_position{fuzzed_data_provider.ConsumeIntegralInRange(-1, tx.vout.size() - 1)}; - bilingual_str error; - // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. - // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly - tx.vout.clear(); - (void)FundTransaction(*wallet, tx, recipients, change_position, /*lockUnspents=*/false, coin_control); - } -}; - FUZZ_TARGET(wallet_notifications, .init = initialize_setup) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; @@ -176,10 +62,12 @@ FUZZ_TARGET(wallet_notifications, .init = initialize_setup) // total amount. const auto total_amount{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY / 100000)}; FuzzedWallet a{ + *g_setup->m_node.chain, "fuzzed_wallet_a", "tprv8ZgxMBicQKsPd1QwsGgzfu2pcPYbBosZhJknqreRHgsWx32nNEhMjGQX2cgFL8n6wz9xdDYwLcs78N4nsCo32cxEX8RBtwGsEGgybLiQJfk", }; FuzzedWallet b{ + *g_setup->m_node.chain, "fuzzed_wallet_b", "tprv8ZgxMBicQKsPfCunYTF18sEmEyjz8TfhGnZ3BoVAhkqLv7PLkQgmoG2Ecsp4JuqciWnkopuEwShit7st743fdmB9cMD4tznUkcs33vK51K9", }; diff --git a/src/wallet/test/fuzz/spend.cpp b/src/wallet/test/fuzz/spend.cpp new file mode 100644 index 0000000000000..9abd9e402a5aa --- /dev/null +++ b/src/wallet/test/fuzz/spend.cpp @@ -0,0 +1,102 @@ +// Copyright (c) 2024-present 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using util::ToString; + +namespace wallet { +namespace { +const TestingSetup* g_setup; + +void initialize_setup() +{ + static const auto testing_setup = MakeNoLogFileContext(); + g_setup = testing_setup.get(); +} + +FUZZ_TARGET(wallet_create_transaction, .init = initialize_setup) +{ + SeedRandomStateForTest(SeedRand::ZEROS); + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const auto& node = g_setup->m_node; + Chainstate& chainstate{node.chainman->ActiveChainstate()}; + ArgsManager& args = *node.args; + args.ForceSetArg("-dustrelayfee", ToString(fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_MONEY))); + FuzzedWallet fuzzed_wallet{ + *g_setup->m_node.chain, + "fuzzed_wallet_a", + "tprv8ZgxMBicQKsPd1QwsGgzfu2pcPYbBosZhJknqreRHgsWx32nNEhMjGQX2cgFL8n6wz9xdDYwLcs78N4nsCo32cxEX8RBtwGsEGgybLiQJfk", + }; + + CCoinControl coin_control; + if (fuzzed_data_provider.ConsumeBool()) coin_control.m_version = fuzzed_data_provider.ConsumeIntegral(); + coin_control.m_avoid_partial_spends = fuzzed_data_provider.ConsumeBool(); + coin_control.m_include_unsafe_inputs = fuzzed_data_provider.ConsumeBool(); + if (fuzzed_data_provider.ConsumeBool()) coin_control.m_confirm_target = fuzzed_data_provider.ConsumeIntegral(); + coin_control.destChange = fuzzed_data_provider.ConsumeBool() ? fuzzed_wallet.GetDestination(fuzzed_data_provider) : ConsumeTxDestination(fuzzed_data_provider); + if (fuzzed_data_provider.ConsumeBool()) coin_control.m_change_type = fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES); + if (fuzzed_data_provider.ConsumeBool()) coin_control.m_feerate = CFeeRate(ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)); + coin_control.m_allow_other_inputs = fuzzed_data_provider.ConsumeBool(); + coin_control.m_locktime = fuzzed_data_provider.ConsumeIntegral(); + coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool(); + + int next_locktime{0}; + CAmount all_values{0}; + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) + { + CMutableTransaction tx; + tx.nLockTime = next_locktime++; + tx.vout.resize(1); + CAmount n_value{ConsumeMoney(fuzzed_data_provider)}; + all_values += n_value; + if (all_values > MAX_MONEY) return; + tx.vout[0].nValue = n_value; + tx.vout[0].scriptPubKey = GetScriptForDestination(fuzzed_wallet.GetDestination(fuzzed_data_provider)); + LOCK(fuzzed_wallet.wallet->cs_wallet); + auto txid{tx.GetHash()}; + auto ret{fuzzed_wallet.wallet->mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateConfirmed{chainstate.m_chain.Tip()->GetBlockHash(), chainstate.m_chain.Height(), /*index=*/0}))}; + assert(ret.second); + } + + std::vector recipients; + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100) { + CTxDestination destination; + CallOneOf( + fuzzed_data_provider, + [&] { + destination = fuzzed_wallet.GetDestination(fuzzed_data_provider); + }, + [&] { + CScript script; + script << OP_RETURN; + destination = CNoDestination{script}; + }, + [&] { + destination = ConsumeTxDestination(fuzzed_data_provider); + } + ); + recipients.push_back({destination, + /*nAmount=*/ConsumeMoney(fuzzed_data_provider), + /*fSubtractFeeFromAmount=*/fuzzed_data_provider.ConsumeBool()}); + } + + std::optional change_pos; + if (fuzzed_data_provider.ConsumeBool()) change_pos = fuzzed_data_provider.ConsumeIntegral(); + (void)CreateTransaction(*fuzzed_wallet.wallet, recipients, change_pos, coin_control); +} +} // namespace +} // namespace wallet diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index ba12f5f6bf157..c8a89c0e642ff 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -95,6 +95,7 @@ class MockableBatch : public DatabaseBatch bool TxnBegin() override { return m_pass; } bool TxnCommit() override { return m_pass; } bool TxnAbort() override { return m_pass; } + bool HasActiveTxn() override { return false; } }; /** A WalletDatabase whose contents and return values can be modified as needed for testing diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d509284632226..e1b4bd9cb5cf5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1701,10 +1701,16 @@ bool CWallet::CanGetAddresses(bool internal) const } void CWallet::SetWalletFlag(uint64_t flags) +{ + WalletBatch batch(GetDatabase()); + return SetWalletFlagWithDB(batch, flags); +} + +void CWallet::SetWalletFlagWithDB(WalletBatch& batch, uint64_t flags) { LOCK(cs_wallet); m_wallet_flags |= flags; - if (!WalletBatch(GetDatabase()).WriteWalletFlags(m_wallet_flags)) + if (!batch.WriteWalletFlags(m_wallet_flags)) throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } @@ -2382,8 +2388,21 @@ DBErrors CWallet::LoadWallet() util::Result CWallet::RemoveTxs(std::vector& txs_to_remove) { AssertLockHeld(cs_wallet); - WalletBatch batch(GetDatabase()); - if (!batch.TxnBegin()) return util::Error{_("Error starting db txn for wallet transactions removal")}; + bilingual_str str_err; // future: make RunWithinTxn return a util::Result + bool was_txn_committed = RunWithinTxn(GetDatabase(), /*process_desc=*/"remove transactions", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + util::Result result{RemoveTxs(batch, txs_to_remove)}; + if (!result) str_err = util::ErrorString(result); + return result.has_value(); + }); + if (!str_err.empty()) return util::Error{str_err}; + if (!was_txn_committed) return util::Error{_("Error starting/committing db txn for wallet transactions removal process")}; + return {}; // all good +} + +util::Result CWallet::RemoveTxs(WalletBatch& batch, std::vector& txs_to_remove) +{ + AssertLockHeld(cs_wallet); + if (!batch.HasActiveTxn()) return util::Error{strprintf(_("The transactions removal process can only be executed within a db txn"))}; // Check for transaction existence and remove entries from disk using TxIterator = std::unordered_map::const_iterator; @@ -2392,38 +2411,30 @@ util::Result CWallet::RemoveTxs(std::vector& txs_to_remove) for (const uint256& hash : txs_to_remove) { auto it_wtx = mapWallet.find(hash); if (it_wtx == mapWallet.end()) { - str_err = strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex()); - break; + return util::Error{strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex())}; } if (!batch.EraseTx(hash)) { - str_err = strprintf(_("Failure removing transaction: %s"), hash.GetHex()); - break; + return util::Error{strprintf(_("Failure removing transaction: %s"), hash.GetHex())}; } erased_txs.emplace_back(it_wtx); } - // Roll back removals in case of an error - if (!str_err.empty()) { - batch.TxnAbort(); - return util::Error{str_err}; - } - - // Dump changes to disk - if (!batch.TxnCommit()) return util::Error{_("Error committing db txn for wallet transactions removal")}; - - // Update the in-memory state and notify upper layers about the removals - for (const auto& it : erased_txs) { - const uint256 hash{it->first}; - wtxOrdered.erase(it->second.m_it_wtxOrdered); - for (const auto& txin : it->second.tx->vin) - mapTxSpends.erase(txin.prevout); - mapWallet.erase(it); - NotifyTransactionChanged(hash, CT_DELETED); - } + // Register callback to update the memory state only when the db txn is actually dumped to disk + batch.RegisterTxnListener({.on_commit=[&, erased_txs]() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + // Update the in-memory state and notify upper layers about the removals + for (const auto& it : erased_txs) { + const uint256 hash{it->first}; + wtxOrdered.erase(it->second.m_it_wtxOrdered); + for (const auto& txin : it->second.tx->vin) + mapTxSpends.erase(txin.prevout); + mapWallet.erase(it); + NotifyTransactionChanged(hash, CT_DELETED); + } - MarkDirty(); + MarkDirty(); + }, .on_abort={}}); - return {}; // all good + return {}; } bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::optional& new_purpose) @@ -3736,22 +3747,30 @@ DescriptorScriptPubKeyMan& CWallet::SetupDescriptorScriptPubKeyMan(WalletBatch& return *out; } -void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) +void CWallet::SetupDescriptorScriptPubKeyMans(WalletBatch& batch, const CExtKey& master_key) { AssertLockHeld(cs_wallet); - - // Create single batch txn - WalletBatch batch(GetDatabase()); - if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors setup"); - for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { SetupDescriptorScriptPubKeyMan(batch, master_key, t, internal); } } +} + +void CWallet::SetupOwnDescriptorScriptPubKeyMans(WalletBatch& batch) +{ + AssertLockHeld(cs_wallet); + assert(!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)); + // Make a seed + CKey seed_key = GenerateRandomKey(); + CPubKey seed = seed_key.GetPubKey(); + assert(seed_key.VerifyPubKey(seed)); - // Ensure information is committed to disk - if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors setup"); + // Get the extended key + CExtKey master_key; + master_key.SetSeed(seed_key); + + SetupDescriptorScriptPubKeyMans(batch, master_key); } void CWallet::SetupDescriptorScriptPubKeyMans() @@ -3759,16 +3778,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans() AssertLockHeld(cs_wallet); if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { - // Make a seed - CKey seed_key = GenerateRandomKey(); - CPubKey seed = seed_key.GetPubKey(); - assert(seed_key.VerifyPubKey(seed)); - - // Get the extended key - CExtKey master_key; - master_key.SetSeed(seed_key); - - SetupDescriptorScriptPubKeyMans(master_key); + if (!RunWithinTxn(GetDatabase(), /*process_desc=*/"setup descriptors", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet){ + SetupOwnDescriptorScriptPubKeyMans(batch); + return true; + })) throw std::runtime_error("Error: cannot process db transaction for descriptors setup"); } else { ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); @@ -4055,15 +4068,14 @@ std::optional CWallet::GetDescriptorsForLegacy(bilingual_str& err return res; } -bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) +util::Result CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch, MigrationData& data) { AssertLockHeld(cs_wallet); LegacyDataSPKM* legacy_spkm = GetLegacyDataSPKM(); if (!Assume(legacy_spkm)) { // This shouldn't happen - error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); - return false; + return util::Error{Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing"))}; } // Get all invalid or non-watched scripts that will not be migrated @@ -4077,16 +4089,15 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) for (auto& desc_spkm : data.desc_spkms) { if (m_spk_managers.count(desc_spkm->GetID()) > 0) { - error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted."); - return false; + return util::Error{_("Error: Duplicate descriptors created during migration. Your wallet may be corrupted.")}; } uint256 id = desc_spkm->GetID(); AddScriptPubKeyMan(id, std::move(desc_spkm)); } // Remove the LegacyScriptPubKeyMan from disk - if (!legacy_spkm->DeleteRecords()) { - return false; + if (!legacy_spkm->DeleteRecordsWithDB(local_wallet_batch)) { + return util::Error{_("Error: cannot remove legacy wallet records")}; } // Remove the LegacyScriptPubKeyMan from memory @@ -4095,22 +4106,21 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) m_internal_spk_managers.clear(); // Setup new descriptors - SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + SetWalletFlagWithDB(local_wallet_batch, WALLET_FLAG_DESCRIPTORS); if (!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { // Use the existing master key if we have it if (data.master_key.key.IsValid()) { - SetupDescriptorScriptPubKeyMans(data.master_key); + SetupDescriptorScriptPubKeyMans(local_wallet_batch, data.master_key); } else { // Setup with a new seed if we don't. - SetupDescriptorScriptPubKeyMans(); + SetupOwnDescriptorScriptPubKeyMans(local_wallet_batch); } } // Get best block locator so that we can copy it to the watchonly and solvables CBlockLocator best_block_locator; - if (!WalletBatch(GetDatabase()).ReadBestBlock(best_block_locator)) { - error = _("Error: Unable to read wallet's best block locator record"); - return false; + if (!local_wallet_batch.ReadBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to read wallet's best block locator record")}; } // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. @@ -4119,21 +4129,23 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) std::unique_ptr watchonly_batch; if (data.watchonly_wallet) { watchonly_batch = std::make_unique(data.watchonly_wallet->GetDatabase()); + if (!watchonly_batch->TxnBegin()) return util::Error{strprintf(_("Error: database transaction cannot be executed for wallet %s"), data.watchonly_wallet->GetName())}; // Copy the next tx order pos to the watchonly wallet LOCK(data.watchonly_wallet->cs_wallet); data.watchonly_wallet->nOrderPosNext = nOrderPosNext; watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext); // Write the best block locator to avoid rescanning on reload if (!watchonly_batch->WriteBestBlock(best_block_locator)) { - error = _("Error: Unable to write watchonly wallet best block locator record"); - return false; + return util::Error{_("Error: Unable to write watchonly wallet best block locator record")}; } } + std::unique_ptr solvables_batch; if (data.solvable_wallet) { + solvables_batch = std::make_unique(data.solvable_wallet->GetDatabase()); + if (!solvables_batch->TxnBegin()) return util::Error{strprintf(_("Error: database transaction cannot be executed for wallet %s"), data.solvable_wallet->GetName())}; // Write the best block locator to avoid rescanning on reload - if (!WalletBatch(data.solvable_wallet->GetDatabase()).WriteBestBlock(best_block_locator)) { - error = _("Error: Unable to write solvable wallet best block locator record"); - return false; + if (!solvables_batch->WriteBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to write solvable wallet best block locator record")}; } } for (const auto& [_pos, wtx] : wtxOrdered) { @@ -4152,8 +4164,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) ins_wtx.CopyFrom(to_copy_wtx); return true; })) { - error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); - return false; + return util::Error{strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex())}; } watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); // Mark as to remove from the migrated wallet only if it does not also belong to it @@ -4165,31 +4176,21 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) } if (!is_mine) { // Both not ours and not in the watchonly wallet - error = strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex()); - return false; + return util::Error{strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex())}; } } - watchonly_batch.reset(); // Flush + // Do the removes if (txids_to_delete.size() > 0) { - if (auto res = RemoveTxs(txids_to_delete); !res) { - error = _("Error: Could not delete watchonly transactions. ") + util::ErrorString(res); - return false; + if (auto res = RemoveTxs(local_wallet_batch, txids_to_delete); !res) { + return util::Error{_("Error: Could not delete watchonly transactions. ") + util::ErrorString(res)}; } } // Pair external wallets with their corresponding db handler std::vector, std::unique_ptr>> wallets_vec; - for (const auto& ext_wallet : {data.watchonly_wallet, data.solvable_wallet}) { - if (!ext_wallet) continue; - - std::unique_ptr batch = std::make_unique(ext_wallet->GetDatabase()); - if (!batch->TxnBegin()) { - error = strprintf(_("Error: database transaction cannot be executed for wallet %s"), ext_wallet->GetName()); - return false; - } - wallets_vec.emplace_back(ext_wallet, std::move(batch)); - } + if (data.watchonly_wallet) wallets_vec.emplace_back(data.watchonly_wallet, std::move(watchonly_batch)); + if (data.solvable_wallet) wallets_vec.emplace_back(data.solvable_wallet, std::move(solvables_batch)); // Write address book entry to disk auto func_store_addr = [](WalletBatch& batch, const CTxDestination& dest, const CAddressBookData& entry) { @@ -4236,39 +4237,27 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) continue; } - error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets"); - return false; + return util::Error{_("Error: Address book data in wallet cannot be identified to belong to migrated wallets")}; } } // Persist external wallets address book entries for (auto& [wallet, batch] : wallets_vec) { if (!batch->TxnCommit()) { - error = strprintf(_("Error: address book copy failed for wallet %s"), wallet->GetName()); - return false; + return util::Error{strprintf(_("Error: Unable to write data to disk for wallet %s"), wallet->GetName())}; } } // Remove the things to delete in this wallet - WalletBatch local_wallet_batch(GetDatabase()); - local_wallet_batch.TxnBegin(); if (dests_to_delete.size() > 0) { for (const auto& dest : dests_to_delete) { if (!DelAddressBookWithDB(local_wallet_batch, dest)) { - error = _("Error: Unable to remove watchonly address book data"); - return false; + return util::Error{_("Error: Unable to remove watchonly address book data")}; } } } - local_wallet_batch.TxnCommit(); - - // Connect the SPKM signals - ConnectScriptPubKeyManNotifiers(); - NotifyCanGetAddressesChanged(); - - WalletLogPrintf("Wallet migration complete.\n"); - return true; + return {}; // all good } bool CWallet::CanGrindR() const @@ -4379,10 +4368,14 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, } // Add the descriptors to wallet, remove LegacyScriptPubKeyMan, and cleanup txs and address book data - if (!wallet.ApplyMigrationData(*data, error)) { - return false; - } - return true; + return RunWithinTxn(wallet.GetDatabase(), /*process_desc=*/"apply migration process", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet){ + if (auto res_migration = wallet.ApplyMigrationData(batch, *data); !res_migration) { + error = util::ErrorString(res_migration); + return false; + } + wallet.WalletLogPrintf("Wallet migration complete.\n"); + return true; + }); } util::Result MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 85fa38f74c9c3..91220443cde95 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -422,6 +422,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Same as 'AddActiveScriptPubKeyMan' but designed for use within a batch transaction context void AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal); + /** Store wallet flags */ + void SetWalletFlagWithDB(WalletBatch& batch, uint64_t flags); + //! Cache of descriptor ScriptPubKeys used for IsMine. Maps ScriptPubKey to set of spkms std::unordered_map, SaltedSipHasher> m_cached_spks; @@ -791,6 +794,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati /** Erases the provided transactions from the wallet. */ util::Result RemoveTxs(std::vector& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + util::Result RemoveTxs(WalletBatch& batch, std::vector& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::optional& purpose); @@ -1018,9 +1022,12 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Create new DescriptorScriptPubKeyMan and add it to the wallet DescriptorScriptPubKeyMan& SetupDescriptorScriptPubKeyMan(WalletBatch& batch, const CExtKey& master_key, const OutputType& output_type, bool internal) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Create new DescriptorScriptPubKeyMans and add them to the wallet - void SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SetupDescriptorScriptPubKeyMans(WalletBatch& batch, const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + //! Create new seed and default DescriptorScriptPubKeyMans for this wallet + void SetupOwnDescriptorScriptPubKeyMans(WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const; @@ -1044,7 +1051,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Adds the ScriptPubKeyMans given in MigrationData to this wallet, removes LegacyScriptPubKeyMan, //! and where needed, moves tx and address book entries to watchonly_wallet or solvable_wallet - bool ApplyMigrationData(MigrationData& data, bilingual_str& error) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + util::Result ApplyMigrationData(WalletBatch& local_wallet_batch, MigrationData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Whether the (external) signer performs R-value signature grinding bool CanGrindR() const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0d4884be67bd7..83813c642e947 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1336,10 +1336,8 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) bool WalletBatch::EraseRecords(const std::unordered_set& types) { - return RunWithinTxn(*this, "erase records", [&types](WalletBatch& self) { - return std::all_of(types.begin(), types.end(), [&self](const std::string& type) { - return self.m_batch->ErasePrefix(DataStream() << type); - }); + return std::all_of(types.begin(), types.end(), [&](const std::string& type) { + return m_batch->ErasePrefix(DataStream() << type); }); } @@ -1350,12 +1348,34 @@ bool WalletBatch::TxnBegin() bool WalletBatch::TxnCommit() { - return m_batch->TxnCommit(); + bool res = m_batch->TxnCommit(); + if (res) { + for (const auto& listener : m_txn_listeners) { + listener.on_commit(); + } + // txn finished, clear listeners + m_txn_listeners.clear(); + } + return res; } bool WalletBatch::TxnAbort() { - return m_batch->TxnAbort(); + bool res = m_batch->TxnAbort(); + if (res) { + for (const auto& listener : m_txn_listeners) { + listener.on_abort(); + } + // txn finished, clear listeners + m_txn_listeners.clear(); + } + return res; +} + +void WalletBatch::RegisterTxnListener(const DbTxnListener& l) +{ + assert(m_batch->HasActiveTxn()); + m_txn_listeners.emplace_back(l); } std::unique_ptr MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index bffcc87202aa6..32c3c29b5e365 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -180,6 +180,11 @@ class CKeyMetadata } }; +struct DbTxnListener +{ + std::function on_commit, on_abort; +}; + /** Access to the wallet database. * Opens the database and provides read and write access to it. Each read and write is its own transaction. * Multiple operation transactions can be started using TxnBegin() and committed using TxnCommit() @@ -292,9 +297,18 @@ class WalletBatch bool TxnCommit(); //! Abort current transaction bool TxnAbort(); + bool HasActiveTxn() { return m_batch->HasActiveTxn(); } + + //! Registers db txn callback functions + void RegisterTxnListener(const DbTxnListener& l); + private: std::unique_ptr m_batch; WalletDatabase& m_database; + + // External functions listening to the current db txn outcome. + // Listeners are cleared at the end of the transaction. + std::vector m_txn_listeners; }; /** diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index e71871114d390..79d2f65164b2f 100644 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -7,7 +7,10 @@ import string from test_framework.test_framework import BitcoinTestFramework -from test_framework.test_node import ErrorMatch +from test_framework.test_node import ( + BITCOIN_PID_FILENAME_DEFAULT, + ErrorMatch, +) class FilelockTest(BitcoinTestFramework): def add_options(self, parser): @@ -33,7 +36,7 @@ def run_test(self): self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir") cookie_file = datadir / ".cookie" assert cookie_file.exists() # should not be deleted during the second bitcoind instance shutdown - pid_file = datadir / "bitcoind.pid" + pid_file = datadir / BITCOIN_PID_FILENAME_DEFAULT assert pid_file.exists() if self.is_wallet_compiled(): diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 659d33684e5cd..4a2f7ecf42f31 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -2,17 +2,20 @@ # Copyright (c) 2021-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Stress tests related to node initialization.""" +"""Tests related to node initialization.""" from pathlib import Path import platform import shutil from test_framework.test_framework import BitcoinTestFramework, SkipTest -from test_framework.test_node import ErrorMatch +from test_framework.test_node import ( + BITCOIN_PID_FILENAME_DEFAULT, + ErrorMatch, +) from test_framework.util import assert_equal -class InitStressTest(BitcoinTestFramework): +class InitTest(BitcoinTestFramework): """ Ensure that initialization can be interrupted at a number of points and not impair subsequent starts. @@ -25,7 +28,7 @@ def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 1 - def run_test(self): + def init_stress_test(self): """ - test terminating initialization after seeing a certain log line. - test removing certain essential files to test startup error paths. @@ -97,13 +100,13 @@ def check_clean_start(): files_to_delete = { 'blocks/index/*.ldb': 'Error opening block database.', - 'chainstate/*.ldb': 'Error opening block database.', + 'chainstate/*.ldb': 'Error opening coins database.', 'blocks/blk*.dat': 'Error loading block database.', } files_to_perturb = { 'blocks/index/*.ldb': 'Error loading block database.', - 'chainstate/*.ldb': 'Error opening block database.', + 'chainstate/*.ldb': 'Error opening coins database.', 'blocks/blk*.dat': 'Corrupted block database detected.', } @@ -147,6 +150,31 @@ def check_clean_start(): shutil.move(node.chain_path / "blocks_bak", node.chain_path / "blocks") shutil.move(node.chain_path / "chainstate_bak", node.chain_path / "chainstate") + def init_pid_test(self): + BITCOIN_PID_FILENAME_CUSTOM = "my_fancy_bitcoin_pid_file.foobar" + + self.log.info("Test specifying custom pid file via -pid command line option") + custom_pidfile_relative = BITCOIN_PID_FILENAME_CUSTOM + self.log.info(f"-> path relative to datadir ({custom_pidfile_relative})") + self.restart_node(0, [f"-pid={custom_pidfile_relative}"]) + datadir = self.nodes[0].chain_path + assert not (datadir / BITCOIN_PID_FILENAME_DEFAULT).exists() + assert (datadir / custom_pidfile_relative).exists() + self.stop_node(0) + assert not (datadir / custom_pidfile_relative).exists() + + custom_pidfile_absolute = Path(self.options.tmpdir) / BITCOIN_PID_FILENAME_CUSTOM + self.log.info(f"-> absolute path ({custom_pidfile_absolute})") + self.restart_node(0, [f"-pid={custom_pidfile_absolute}"]) + assert not (datadir / BITCOIN_PID_FILENAME_DEFAULT).exists() + assert custom_pidfile_absolute.exists() + self.stop_node(0) + assert not custom_pidfile_absolute.exists() + + def run_test(self): + self.init_pid_test() + self.init_stress_test() + if __name__ == '__main__': - InitStressTest(__file__).main() + InitTest(__file__).main() diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py index 22600bf8a4e92..9ec23e9d6eb25 100755 --- a/test/functional/p2p_orphan_handling.py +++ b/test/functional/p2p_orphan_handling.py @@ -5,6 +5,7 @@ import time +from test_framework.mempool_util import tx_in_orphanage from test_framework.messages import ( CInv, CTxInWitness, @@ -41,6 +42,8 @@ # for one peer and y seconds for another, use specific values instead. TXREQUEST_TIME_SKIP = NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY + OVERLOADED_PEER_TX_DELAY + 1 +DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100 + def cleanup(func): # Time to fastfoward (using setmocktime) in between subtests to ensure they do not interfere with # one another, in seconds. Equal to 12 hours, which is enough to expire anything that may exist @@ -566,6 +569,47 @@ def test_orphan_txid_inv(self): assert tx_child["txid"] in node_mempool assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"]) + @cleanup + def test_max_orphan_amount(self): + self.log.info("Check that we never exceed our storage limits for orphans") + + node = self.nodes[0] + self.generate(self.wallet, 1) + peer_1 = node.add_p2p_connection(P2PInterface()) + + self.log.info("Check that orphanage is empty on start of test") + assert len(node.getorphantxs()) == 0 + + self.log.info("Filling up orphanage with " + str(DEFAULT_MAX_ORPHAN_TRANSACTIONS) + "(DEFAULT_MAX_ORPHAN_TRANSACTIONS) orphans") + orphans = [] + parent_orphans = [] + for _ in range(DEFAULT_MAX_ORPHAN_TRANSACTIONS): + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + parent_orphans.append(tx_parent_1["tx"]) + orphans.append(tx_child_1["tx"]) + peer_1.send_message(msg_tx(tx_child_1["tx"])) + + peer_1.sync_with_ping() + orphanage = node.getorphantxs() + assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS) + + for orphan in orphans: + assert tx_in_orphanage(node, orphan) + + self.log.info("Check that we do not add more than the max orphan amount") + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + peer_1.send_and_ping(msg_tx(tx_child_1["tx"])) + parent_orphans.append(tx_parent_1["tx"]) + orphanage = node.getorphantxs() + assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS) + + self.log.info("Clearing the orphanage") + for index, parent_orphan in enumerate(parent_orphans): + peer_1.send_and_ping(msg_tx(parent_orphan)) + assert_equal(len(node.getorphantxs()),0) + def run_test(self): self.nodes[0].setmocktime(int(time.time())) @@ -582,6 +626,7 @@ def run_test(self): self.test_same_txid_orphan() self.test_same_txid_orphan_of_orphan() self.test_orphan_txid_inv() + self.test_max_orphan_amount() if __name__ == '__main__': diff --git a/test/functional/rpc_getorphantxs.py b/test/functional/rpc_getorphantxs.py index 8d32ce1638455..dfa1168b2ec95 100755 --- a/test/functional/rpc_getorphantxs.py +++ b/test/functional/rpc_getorphantxs.py @@ -125,6 +125,5 @@ def orphan_details_match(self, orphan, tx, verbosity): self.log.info("Check the transaction hex of orphan") assert_equal(orphan["hex"], tx["hex"]) - if __name__ == '__main__': GetOrphanTxsTest(__file__).main() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 60ca9269a5b9f..e59ff4e39254d 100644 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -48,6 +48,7 @@ NUM_XOR_BYTES = 8 # The null blocks key (all 0s) NULL_BLK_XOR_KEY = bytes([0] * NUM_XOR_BYTES) +BITCOIN_PID_FILENAME_DEFAULT = "bitcoind.pid" class FailedToStartError(Exception): diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index f55d0f8cb7b8c..3e9e5ba230a5e 100644 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb']) +SET_DOC_OPTIONAL = set(['-h', '-?', '-dbcrashratio', '-forcecompactdb']) def lint_missing_argument_documentation():