diff --git a/ci/dash/lint-tidy.sh b/ci/dash/lint-tidy.sh index 159591d81ed2..f12b1ebbe2b7 100755 --- a/ci/dash/lint-tidy.sh +++ b/ci/dash/lint-tidy.sh @@ -83,6 +83,7 @@ echo "==========================" cd "${BASE_ROOT_DIR}/build-ci/dashcore-${BUILD_TARGET}" iwyu_tool.py \ + "src/common/init.cpp" \ "src/common/url.cpp" \ "src/compat" \ "src/dbwrapper.cpp" \ diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index ac78c511139e..94e01cf906d4 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -12,4 +12,6 @@ export TEST_RUNNER_EXTRA="--timeout-factor=4" # Increase timeout because saniti export FUNCTIONAL_TESTS_CONFIG="--exclude wallet_multiwallet.py" # Temporarily suppress ASan heap-use-after-free (see issue #14163) export RUN_BENCH=true export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=address,integer,undefined CC=clang CXX=clang++" +export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 \ +CPPFLAGS=-DDEBUG_LOCKORDER \ +--with-sanitizers=address,float-divide-by-zero,integer,undefined CC=clang CXX=clang++" diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index 6d405af70d47..dcb9fb8dbf24 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -15,4 +15,5 @@ export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=true export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer CC='clang-19 -ftrivial-auto-var-init=pattern' CXX='clang++-19 -ftrivial-auto-var-init=pattern'" +export BITCOIN_CONFIG="--enable-zmq --enable-fuzz --with-sanitizers=fuzzer,address,undefined,float-divide-by-zero,integer \ +CC='clang-19 -ftrivial-auto-var-init=pattern' CXX='clang++-19 -ftrivial-auto-var-init=pattern'" diff --git a/src/Makefile.am b/src/Makefile.am index 3545be271f89..9acbf13ac338 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -194,6 +194,7 @@ BITCOIN_CORE_H = \ coinjoin/walletman.h \ coins.h \ common/bloom.h \ + common/init.h \ common/run_command.h \ common/url.h \ compat/assumptions.h \ @@ -948,6 +949,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ common/bloom.cpp \ + common/init.cpp \ common/run_command.cpp \ compressor.cpp \ core_read.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index c789b8c35bd8..451c3e9e94fe 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -195,6 +195,7 @@ BITCOIN_TESTS =\ test/timedata_tests.cpp \ test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ + test/translation_tests.cpp \ test/txindex_tests.cpp \ test/txpackage_tests.cpp \ test/txreconciliation_tests.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 5778242ca5aa..26a871bcda51 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -126,7 +126,7 @@ int main(int argc, char* argv[]) std::cerr << "Failed to load Chain state from your datadir." << std::endl; goto epilogue; } else { - std::tie(status, error) = node::VerifyLoadedChainstate(std::ref(chainman), *evodb, options); + std::tie(status, error) = node::VerifyLoadedChainstate(chainman, *evodb, options); if (status != node::ChainstateLoadStatus::SUCCESS) { std::cerr << "Failed to verify loaded Chain state from your datadir." << std::endl; goto epilogue; diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 3b0873970e9f..073a1305dddd 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -170,7 +170,7 @@ static int AppInitRPC(int argc, char* argv[]) } return EXIT_SUCCESS; } - if (!CheckDataDirOption()) { + if (!CheckDataDirOption(gArgs)) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")); return EXIT_FAILURE; } @@ -837,7 +837,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co if (failedToGetAuthCookie) { throw std::runtime_error(strprintf( "Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)", - fs::PathToString(GetConfigFile(gArgs.GetPathArg("-conf", BITCOIN_CONF_FILENAME))))); + fs::PathToString(gArgs.GetConfigFilePath()))); } else { throw std::runtime_error("Authorization failed: Incorrect rpcuser or rpcpassword"); } diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index be9ba957ec4b..09ade315457f 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -85,7 +85,7 @@ static std::optional WalletAppInit(ArgsManager& args, int argc, char* argv[ // check for printtoconsole, allow -debug LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", args.GetBoolArg("-debug", false)); - if (!CheckDataDirOption()) { + if (!CheckDataDirOption(args)) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", args.GetArg("-datadir", "")); return EXIT_FAILURE; } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index da628f0c51b8..7e6ebbcd3aaa 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -109,20 +110,32 @@ int fork_daemon(bool nochdir, bool noclose, TokenPipeEnd& endpoint) #endif -static bool AppInit(NodeContext& node, int argc, char* argv[]) +static bool ParseArgs(ArgsManager& args, int argc, char* argv[]) { - bool fRet = false; - - util::ThreadSetInternalName("init"); - // If Qt is used, parameters/dash.conf are parsed in qt/bitcoin.cpp's main() - ArgsManager& args = *Assert(node.args); SetupServerArgs(args); std::string error; if (!args.ParseParameters(argc, argv, error)) { - return InitError(Untranslated(strprintf("Error parsing command line arguments: %s\n", error))); + return InitError(Untranslated(strprintf("Error parsing command line arguments: %s", error))); + } + + if (args.IsArgSet("-printcrashinfo")) return true; + + if (auto error = common::InitConfig(args)) { + return InitError(error->message, error->details); + } + + // Error out when loose non-argument tokens are encountered on command line + for (int i = 1; i < argc; i++) { + if (!IsSwitchChar(argv[i][0])) { + return InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see dashd -h for a list of options.", argv[i]))); + } } + return true; +} +static bool ProcessInitCommands(ArgsManager& args) +{ if (args.IsArgSet("-printcrashinfo")) { std::cout << GetCrashInfoStrFromSerializedStr(args.GetArg("-printcrashinfo", "")) << std::endl; return true; @@ -144,6 +157,14 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) return true; } + return false; +} + +static bool AppInit(NodeContext& node) +{ + bool fRet = false; + ArgsManager& args = *Assert(node.args); + CoreContext context{node}; #if HAVE_DECL_FORK // Communication with parent after daemonizing. This is used for signalling in the following ways: @@ -155,37 +176,12 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) #endif try { - if (!CheckDataDirOption()) { - return InitError(Untranslated(strprintf("Specified data directory \"%s\" does not exist.\n", args.GetArg("-datadir", "")))); - } - if (!args.ReadConfigFiles(error, true)) { - return InitError(Untranslated(strprintf("Error reading configuration file: %s\n", error))); - } - // Check for chain settings (Params() calls are only valid after this clause) - try { - SelectParams(args.GetChainName()); - } catch (const std::exception& e) { - return InitError(Untranslated(strprintf("%s\n", e.what()))); - } - - // Error out when loose non-argument tokens are encountered on command line - for (int i = 1; i < argc; i++) { - if (!IsSwitchChar(argv[i][0])) { - return InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see dashd -h for a list of options.\n", argv[i]))); - } - } - - if (!gArgs.InitSettings(error)) { - InitError(Untranslated(error)); - return false; - } - // -server defaults to true for dashd but not for the GUI so do this here args.SoftSetBoolArg("-server", true); // Set this early so that parameter interactions go to console InitLogging(args); InitParameterInteraction(args); - if (!AppInitBasicSetup(args)) { + if (!AppInitBasicSetup(args, node.exit_status)) { // InitError will have been called with detailed error, which ends up on console return false; } @@ -215,7 +211,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } break; case -1: // Error happened. - return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", SysErrorString(errno)))); + return InitError(Untranslated(strprintf("fork_daemon() failed: %s", SysErrorString(errno)))); default: { // Parent: wait and exit. int token = daemon_ep.TokenRead(); if (token) { // Success @@ -227,7 +223,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } } #else - return InitError(Untranslated("-daemon is not supported on this operating system\n")); + return InitError(Untranslated("-daemon is not supported on this operating system")); #endif // HAVE_DECL_FORK } // Lock data directory after daemonization @@ -248,12 +244,6 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) daemon_ep.Close(); } #endif - if (fRet) { - WaitForShutdown(); - } - Interrupt(node); - Shutdown(node); - return fRet; } @@ -279,5 +269,22 @@ MAIN_FUNCTION // Connect dashd signal handlers noui_connect(); - return (AppInit(node, argc, argv) ? EXIT_SUCCESS : EXIT_FAILURE); + util::ThreadSetInternalName("init"); + + // Interpret command line arguments + ArgsManager& args = *Assert(node.args); + if (!ParseArgs(args, argc, argv)) return EXIT_FAILURE; + // Process early info return commands such as -help or -version + if (ProcessInitCommands(args)) return EXIT_SUCCESS; + + // Start application + if (AppInit(node)) { + WaitForShutdown(); + } else { + node.exit_status = EXIT_FAILURE; + } + Interrupt(node); + Shutdown(node); + + return node.exit_status; } diff --git a/src/common/init.cpp b/src/common/init.cpp new file mode 100644 index 000000000000..fe75b0d9a2d9 --- /dev/null +++ b/src/common/init.cpp @@ -0,0 +1,74 @@ +// Copyright (c) 2023 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 + +namespace common { +std::optional InitConfig(ArgsManager& args, SettingsAbortFn settings_abort_fn) +{ + try { + if (!CheckDataDirOption(args)) { + return ConfigError{ConfigStatus::FAILED, strprintf(_("Specified data directory \"%s\" does not exist."), args.GetArg("-datadir", ""))}; + } + std::string error; + if (!args.ReadConfigFiles(error, true)) { + return ConfigError{ConfigStatus::FAILED, strprintf(_("Error reading configuration file: %s"), error)}; + } + + // Check for chain settings (Params() calls are only valid after this clause) + SelectParams(args.GetChainName()); + + // Create datadir if it does not exist. + const auto base_path{args.GetDataDirBase()}; + if (!fs::exists(base_path)) { + // When creating a *new* datadir, also create a "wallets" subdirectory, + // whether or not the wallet is enabled now, so if the wallet is enabled + // in the future, it will use the "wallets" subdirectory for creating + // and listing wallets, rather than the top-level directory where + // wallets could be mixed up with other files. For backwards + // compatibility, wallet code will use the "wallets" subdirectory only + // if it already exists, but never create it itself. There is discussion + // in https://github.com/bitcoin/bitcoin/issues/16220 about ways to + // change wallet code so it would no longer be necessary to create + // "wallets" subdirectories here. + fs::create_directories(base_path / "wallets"); + } + const auto net_path{args.GetDataDirNet()}; + if (!fs::exists(net_path)) { + fs::create_directories(net_path / "wallets"); + } + + // Create settings.json if -nosettings was not specified. + if (args.GetSettingsPath()) { + std::vector details; + if (!args.ReadSettingsFile(&details)) { + const bilingual_str& message = _("Settings file could not be read"); + if (!settings_abort_fn) { + return ConfigError{ConfigStatus::FAILED, message, details}; + } else if (settings_abort_fn(message, details)) { + return ConfigError{ConfigStatus::ABORTED, message, details}; + } else { + details.clear(); // User chose to ignore the error and proceed. + } + } + if (!args.WriteSettingsFile(&details)) { + const bilingual_str& message = _("Settings file could not be written"); + return ConfigError{ConfigStatus::FAILED_WRITE, message, details}; + } + } + } catch (const std::exception& e) { + return ConfigError{ConfigStatus::FAILED, Untranslated(e.what())}; + } + return {}; +} +} // namespace common diff --git a/src/common/init.h b/src/common/init.h new file mode 100644 index 000000000000..380ac3ac7e70 --- /dev/null +++ b/src/common/init.h @@ -0,0 +1,39 @@ +// Copyright (c) 2023 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_COMMON_INIT_H +#define BITCOIN_COMMON_INIT_H + +#include + +#include +#include +#include +#include + +class ArgsManager; + +namespace common { +enum class ConfigStatus { + FAILED, //!< Failed generically. + FAILED_WRITE, //!< Failed to write settings.json + ABORTED, //!< Aborted by user +}; + +struct ConfigError { + ConfigStatus status; + bilingual_str message{}; + std::vector details{}; +}; + +//! Callback function to let the user decide whether to abort loading if +//! settings.json file exists and can't be parsed, or to ignore the error and +//! overwrite the file. +using SettingsAbortFn = std::function& details)>; + +/* Read config files, and create datadir and settings.json if they don't exist. */ +std::optional InitConfig(ArgsManager& args, SettingsAbortFn settings_abort_fn = nullptr); +} // namespace common + +#endif // BITCOIN_COMMON_INIT_H diff --git a/src/index/base.cpp b/src/index/base.cpp index 4c4a865ed747..139deb129f2d 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -28,10 +28,7 @@ constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; void BaseIndex::FatalErrorImpl(const std::string& message) { - SetMiscWarning(Untranslated(message)); - LogPrintf("*** %s\n", message); - AbortError(_("A fatal internal error occurred, see debug.log for details")); - StartShutdown(); + AbortNode(message); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/init.cpp b/src/init.cpp index e116d9cfa4b3..34b43d4f8096 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -196,7 +196,7 @@ static const char* BITCOIN_PID_FILENAME = "dashd.pid"; static fs::path GetPidFile(const ArgsManager& args) { - return AbsPathForConfigVal(args.GetPathArg("-pid", BITCOIN_PID_FILENAME)); + return AbsPathForConfigVal(args, args.GetPathArg("-pid", BITCOIN_PID_FILENAME)); } [[nodiscard]] static bool CreatePidFile(const ArgsManager& args) @@ -1110,7 +1110,7 @@ std::set g_enabled_filter_types; std::terminate(); }; -bool AppInitBasicSetup(const ArgsManager& args) +bool AppInitBasicSetup(const ArgsManager& args, std::atomic& exit_status) { // ********************************************************* Step 1: setup #ifdef _MSC_VER @@ -1124,7 +1124,7 @@ bool AppInitBasicSetup(const ArgsManager& args) // Enable heap terminate-on-corruption HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0); #endif - if (!InitShutdownState()) { + if (!InitShutdownState(exit_status)) { return InitError(Untranslated("Initializing wait-for-shutdown state failed.")); } diff --git a/src/init.h b/src/init.h index f1030756469e..47e813bc8937 100644 --- a/src/init.h +++ b/src/init.h @@ -8,6 +8,7 @@ #include +#include #include #include @@ -39,7 +40,7 @@ void InitParameterInteraction(ArgsManager& args); * @note This can be done before daemonization. Do not call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read. */ -bool AppInitBasicSetup(const ArgsManager& args); +bool AppInitBasicSetup(const ArgsManager& args, std::atomic& exit_status); /** * Initialization: parameter interaction. * @note This can be done before daemonization. Do not call Shutdown() if this function fails. diff --git a/src/init/common.cpp b/src/init/common.cpp index 5dfad4719c66..52123a11eef5 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -44,7 +44,7 @@ void AddLoggingArgs(ArgsManager& argsman) void SetLoggingOptions(const ArgsManager& args) { LogInstance().m_print_to_file = !args.IsArgNegated("-debuglogfile"); - LogInstance().m_file_path = AbsPathForConfigVal(args.GetPathArg("-debuglogfile", DEFAULT_DEBUGLOGFILE)); + LogInstance().m_file_path = AbsPathForConfigVal(args, args.GetPathArg("-debuglogfile", DEFAULT_DEBUGLOGFILE)); LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", !args.GetBoolArg("-daemon", false)); LogInstance().m_log_timestamps = args.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS); LogInstance().m_log_time_micros = args.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS); @@ -119,7 +119,7 @@ bool StartLogging(const ArgsManager& args) LogPrintf("Using data directory %s\n", fs::PathToString(gArgs.GetDataDirNet())); // Only log conf file usage message if conf file actually exists. - fs::path config_file_path = GetConfigFile(args.GetPathArg("-conf", BITCOIN_CONF_FILENAME)); + fs::path config_file_path = args.GetConfigFilePath(); if (fs::exists(config_file_path)) { LogPrintf("Config file: %s\n", fs::PathToString(config_file_path)); } else if (args.IsArgSet("-conf")) { diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 21bab8a95ec3..ae968124f039 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -298,6 +298,9 @@ class Node //! Get warnings. virtual bilingual_str getWarnings() = 0; + //! Get exit status. + virtual int getExitStatus() = 0; + // Get log flags. virtual uint64_t getLogCategories() = 0; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index d66c8e472b97..2a1253bf56b5 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -926,8 +926,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) { BlockValidationState state; if (!chainstate->ActivateBestChain(state, nullptr)) { - LogPrintf("Failed to connect best block (%s)\n", state.ToString()); - StartShutdown(); + AbortNode(strprintf("Failed to connect best block (%s)", state.ToString())); return; } } diff --git a/src/node/context.h b/src/node/context.h index 43f36be32eca..5d1f1f00d02f 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -7,7 +7,9 @@ #include +#include #include +#include #include #include #include @@ -89,6 +91,7 @@ struct NodeContext { std::unique_ptr coinjoin_loader{nullptr}; std::unique_ptr scheduler; std::function rpc_interruption_point = [] {}; + std::atomic exit_status{EXIT_SUCCESS}; //! Dash managers std::unique_ptr cj_walletman; std::unique_ptr dstxman; diff --git a/src/node/interface_ui.cpp b/src/node/interface_ui.cpp index 6fa66ba5f770..18d16a4654cb 100644 --- a/src/node/interface_ui.cpp +++ b/src/node/interface_ui.cpp @@ -4,6 +4,7 @@ #include +#include #include #include @@ -77,6 +78,18 @@ bool InitError(const bilingual_str& str) return false; } +bool InitError(const bilingual_str& str, const std::vector& details) +{ + // For now just flatten the list of error details into a string to pass to + // the base InitError overload. In the future, if more init code provides + // error details, the details could be passed separately from the main + // message for rich display in the GUI. But currently the only init + // functions which provide error details are ones that run during early init + // before the GUI uiInterface is registered, so there's no point passing + // main messages and details separately to uiInterface yet. + return InitError(details.empty() ? str : strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details))); +} + void InitWarning(const bilingual_str& str) { uiInterface.ThreadSafeMessageBox(str, "", CClientUIInterface::MSG_WARNING); diff --git a/src/node/interface_ui.h b/src/node/interface_ui.h index c4b36ef07cfb..07a7c230ce96 100644 --- a/src/node/interface_ui.h +++ b/src/node/interface_ui.h @@ -134,7 +134,7 @@ void InitWarning(const bilingual_str& str); /** Show error message **/ bool InitError(const bilingual_str& str); -constexpr auto AbortError = InitError; +bool InitError(const bilingual_str& str, const std::vector& details); extern CClientUIInterface uiInterface; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 3633fc3805ef..5f2fb8b8e6bc 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -702,10 +702,11 @@ class NodeImpl : public Node void initLogging() override { InitLogging(*Assert(m_context->args)); } void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); } bilingual_str getWarnings() override { return GetWarnings(true); } + int getExitStatus() override { return Assert(m_context)->exit_status.load(); } uint64_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override { - if (!AppInitBasicSetup(gArgs)) return false; + if (!AppInitBasicSetup(gArgs, Assert(m_context)->exit_status)) return false; if (!AppInitParameterInteraction(gArgs)) return false; m_context->kernel = std::make_unique(); @@ -718,7 +719,10 @@ class NodeImpl : public Node } bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override { - return AppInitMain(*m_context, tip_info); + if (AppInitMain(*m_context, tip_info)) return true; + // Error during initialization, set exit status before continue + m_context->exit_status.store(EXIT_FAILURE); + return false; } void appShutdown() override { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index d19d27c4107d..90d092c15706 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -167,54 +168,36 @@ static void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTrans } } -static bool InitSettings() +static bool ErrorSettingsRead(const bilingual_str& error, const std::vector& details) { - gArgs.EnsureDataDir(); - if (!gArgs.GetSettingsPath()) { - return true; // Do nothing if settings file disabled. - } - - std::vector errors; - if (!gArgs.ReadSettingsFile(&errors)) { - std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be read"); - std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Reset | QMessageBox::Abort); - /*: Explanatory text shown on startup when the settings file cannot be read. - Prompts user to make a choice between resetting or aborting. */ - messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); - messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); - messagebox.setTextFormat(Qt::PlainText); - messagebox.setDefaultButton(QMessageBox::Reset); - switch (messagebox.exec()) { - case QMessageBox::Reset: - break; - case QMessageBox::Abort: - return false; - default: - assert(false); - } - } - - errors.clear(); - if (!gArgs.WriteSettingsFile(&errors)) { - std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be written"); - std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Ok); - /*: Explanatory text shown on startup when the settings file could not be written. - Prompts user to check that we have the ability to write to the file. - Explains that the user has the option of running without a settings file.*/ - messagebox.setInformativeText(QObject::tr("A fatal error occurred. Check that settings file is writable, or try running with -nosettings.")); - messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); - messagebox.setTextFormat(Qt::PlainText); - messagebox.setDefaultButton(QMessageBox::Ok); - messagebox.exec(); + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort); + /*: Explanatory text shown on startup when the settings file cannot be read. + Prompts user to make a choice between resetting or aborting. */ + messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(details))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Reset); + switch (messagebox.exec()) { + case QMessageBox::Reset: return false; + case QMessageBox::Abort: + return true; + default: + assert(false); } - return true; +} + +static void ErrorSettingsWrite(const bilingual_str& error, const std::vector& details) +{ + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok); + /*: Explanatory text shown on startup when the settings file could not be written. + Prompts user to check that we have the ability to write to the file. + Explains that the user has the option of running without a settings file.*/ + messagebox.setInformativeText(QObject::tr("A fatal error occurred. Check that settings file is writable, or try running with -nosettings.")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(details))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Ok); + messagebox.exec(); } /* qDebug() message handler --> debug.log */ @@ -405,9 +388,7 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead { qDebug() << __func__ << ": Initialization result: " << success; - // Set exit result. - returnValue = success ? EXIT_SUCCESS : EXIT_FAILURE; - if(success) { + if (success) { delete m_splash; m_splash = nullptr; @@ -557,7 +538,7 @@ int GuiMain(int argc, char* argv[]) SetupUIArgs(gArgs); std::string error; if (!gArgs.ParseParameters(argc, argv, error)) { - InitError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error)); + InitError(strprintf(Untranslated("Error parsing command line arguments: %s"), error)); // Create a message box, because the gui has neither been created nor has subscribed to core signals QMessageBox::critical(nullptr, PACKAGE_NAME, // message cannot be translated because translations have not been initialized @@ -629,34 +610,23 @@ int GuiMain(int argc, char* argv[]) // Gracefully exit if the user cancels if (!Intro::showIfNeeded(did_show_intro, prune_MiB)) return EXIT_SUCCESS; - /// 6a. Determine availability of data directory - if (!CheckDataDirOption()) { - InitError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", ""))); - QMessageBox::critical(nullptr, PACKAGE_NAME, - QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); - return EXIT_FAILURE; - } - try { - /// 6b. Parse dash.conf - /// - Do not call gArgs.GetDataDirNet() before this step finishes - if (!gArgs.ReadConfigFiles(error, true)) { - InitError(strprintf(Untranslated("Error reading configuration file: %s\n"), error)); - QMessageBox::critical(nullptr, PACKAGE_NAME, - QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error))); - return EXIT_FAILURE; + /// 6-7. Parse dash.conf, determine network, switch to network specific + /// options, and create datadir and settings.json. + // - Do not call gArgs.GetDataDirNet() before this step finishes + // - Do not call Params() before this step + // - QSettings() will use the new application name after this, resulting in network-specific settings + // - Needs to be done before createOptionsModel + if (auto error = common::InitConfig(gArgs, ErrorSettingsRead)) { + InitError(error->message, error->details); + if (error->status == common::ConfigStatus::FAILED_WRITE) { + // Show a custom error message to provide more information in the + // case of a datadir write error. + ErrorSettingsWrite(error->message, error->details); + } else if (error->status != common::ConfigStatus::ABORTED) { + // Show a generic message in other cases, and no additional error + // message in the case of a read error if the user decided to abort. + QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(QString::fromStdString(error->message.translated))); } - - /// 7. Determine network (and switch to network specific options) - // - Do not call Params() before this step - // - Do this after parsing the configuration file, as the network can be switched there - // - QSettings() will use the new application name after this, resulting in network-specific settings - // - Needs to be done before createOptionsModel - - // Check for chain settings (Params() calls are only valid after this clause) - SelectParams(gArgs.GetChainName()); - } catch(std::exception &e) { - InitError(Untranslated(strprintf("%s\n", e.what()))); - QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what())); return EXIT_FAILURE; } #ifdef ENABLE_WALLET @@ -664,10 +634,6 @@ int GuiMain(int argc, char* argv[]) PaymentServer::ipcParseCommandLine(argc, argv); #endif - if (!InitSettings()) { - return EXIT_FAILURE; - } - QScopedPointer networkStyle(NetworkStyle::instantiate(Params().NetworkIDString())); assert(!networkStyle.isNull()); // Allow for separate UI settings for testnets @@ -806,7 +772,6 @@ int GuiMain(int argc, char* argv[]) app.InitPruneSetting(prune_MiB); } - int rv = EXIT_SUCCESS; try { app.createWindow(networkStyle.data()); @@ -819,14 +784,13 @@ int GuiMain(int argc, char* argv[]) WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely…").arg(PACKAGE_NAME), (HWND)app.getMainWinId()); #endif app.exec(); - rv = app.getReturnValue(); } else { // A dialog with detailed error will have been shown by InitError() - rv = EXIT_FAILURE; + return EXIT_FAILURE; } } catch (...) { PrintExceptionContinue(std::current_exception(), "Runaway exception"); app.handleRunawayException(QString::fromStdString(app.node().getWarnings().translated)); } - return rv; + return app.node().getExitStatus(); } diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 077253c3bd91..bc6898e101f4 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -61,9 +61,6 @@ class BitcoinApplication: public QApplication /// Request core initialization void requestInitialize(); - /// Get process return value - int getReturnValue() const { return returnValue; } - /// Get window identifier of QMainWindow (BitcoinGUI) WId getMainWinId() const; @@ -101,7 +98,6 @@ public Q_SLOTS: PaymentServer* paymentServer{nullptr}; WalletController* m_wallet_controller{nullptr}; #endif - int returnValue{0}; std::unique_ptr shutdownWindow; SplashScreen* m_splash = nullptr; std::unique_ptr m_node; diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 15e65c8bfb13..59d90824c52e 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -592,7 +592,7 @@ void openDebugLogfile() void openConfigfile() { - fs::path pathConfig = GetConfigFile(gArgs.GetPathArg("-conf", BITCOIN_CONF_FILENAME)); + fs::path pathConfig = gArgs.GetConfigFilePath(); /* Open dash.conf with the associated application */ if (fs::exists(pathConfig)) { diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 087ebf950eee..379de379c335 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -77,7 +77,7 @@ static fs::path GetAuthCookieFile(bool temp=false) if (temp) { arg += ".tmp"; } - return AbsPathForConfigVal(arg); + return AbsPathForConfigVal(gArgs, arg); } static bool g_generated_cookie = false; diff --git a/src/shutdown.cpp b/src/shutdown.cpp index b9aafeee0ab9..3a6bfe00afd9 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -10,6 +10,7 @@ #endif #include +#include #include #include @@ -21,6 +22,8 @@ #include #endif +static std::atomic* g_exit_status{nullptr}; + bool AbortNode(const std::string& strMessage, bilingual_str user_message) { SetMiscWarning(Untranslated(strMessage)); @@ -28,7 +31,8 @@ bool AbortNode(const std::string& strMessage, bilingual_str user_message) if (user_message.empty()) { user_message = _("A fatal internal error occurred, see debug.log for details"); } - AbortError(user_message); + InitError(user_message); + Assert(g_exit_status)->store(EXIT_FAILURE); StartShutdown(); return false; } @@ -47,8 +51,9 @@ static TokenPipeEnd g_shutdown_r; static TokenPipeEnd g_shutdown_w; #endif -bool InitShutdownState() +bool InitShutdownState(std::atomic& exit_status) { + g_exit_status = &exit_status; #ifndef WIN32 std::optional pipe = TokenPipe::Make(); if (!pipe) return false; diff --git a/src/shutdown.h b/src/shutdown.h index 4c10e0368719..068558c80092 100644 --- a/src/shutdown.h +++ b/src/shutdown.h @@ -8,13 +8,15 @@ #include // For bilingual_str +#include + /** Abort with a message */ bool AbortNode(const std::string& strMessage, bilingual_str user_message = bilingual_str{}); /** Initialize shutdown state. This must be called before using either StartShutdown(), * AbortShutdown() or WaitForShutdown(). Calling ShutdownRequested() is always safe. */ -bool InitShutdownState(); +bool InitShutdownState(std::atomic& exit_status); /** Request shutdown of the application. */ void StartShutdown(); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index ef3927c7b0a7..e304d383f94a 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -122,7 +122,9 @@ FUZZ_TARGET(utxo_total_supply) current_block = PrepareNextBlock(); StoreLastTxo(); - LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 100'000) + // Limit to avoid timeout, but enough to cover duplicate_coinbase_height + // and CVE-2018-17144. + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 2'000) { CallOneOf( fuzzed_data_provider, @@ -147,12 +149,12 @@ FUZZ_TARGET(utxo_total_supply) const auto prev_utxo_stats = utxo_stats; if (was_valid) { - circulation += GetBlockSubsidy(ActiveTip(), Params().GetConsensus()); - if (duplicate_coinbase_height == ActiveHeight()) { // we mined the duplicate coinbase assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); } + + circulation += GetBlockSubsidy(ActiveTip(), Params().GetConsensus()); } UpdateUtxoStats(); diff --git a/src/test/translation_tests.cpp b/src/test/translation_tests.cpp new file mode 100644 index 000000000000..bda5dfd09943 --- /dev/null +++ b/src/test/translation_tests.cpp @@ -0,0 +1,21 @@ +// Copyright (c) 2023 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 + +BOOST_AUTO_TEST_SUITE(translation_tests) + +BOOST_AUTO_TEST_CASE(translation_namedparams) +{ + bilingual_str arg{"original", "translated"}; + bilingual_str format{"original [%s]", "translated [%s]"}; + bilingual_str result{strprintf(format, arg)}; + BOOST_CHECK_EQUAL(result.original, "original [original]"); + BOOST_CHECK_EQUAL(result.translated, "translated [translated]"); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/system.cpp b/src/util/system.cpp index dc792681c354..541cbbc56689 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -387,28 +387,6 @@ fs::path ArgsManager::GetBackupsDirPath() return fs::absolute(GetPathArg("-walletbackupsdir")); } - -void ArgsManager::EnsureDataDir() const -{ - /** - * "/wallets" subdirectories are created in all **new** - * datadirs, because wallet code will create new wallets in the "wallets" - * subdirectory only if exists already, otherwise it will create them in - * the top-level datadir where they could interfere with other files. - * Wallet init code currently avoids creating "wallets" directories itself - * for backwards compatibility, but this be changed in the future and - * wallet code here could go away (#16220). - */ - auto path{GetDataDir(false)}; - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } - path = GetDataDir(true); - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } -} - void ArgsManager::ClearPathCache() { LOCK(cs_args); @@ -452,25 +430,6 @@ bool ArgsManager::IsArgSet(const std::string& strArg) const return !GetSetting(strArg).isNull(); } -bool ArgsManager::InitSettings(std::string& error) -{ - EnsureDataDir(); - if (!GetSettingsPath()) { - return true; // Do nothing if settings file disabled. - } - - std::vector errors; - if (!ReadSettingsFile(&errors)) { - error = strprintf("Failed loading settings file:\n%s\n", MakeUnorderedList(errors)); - return false; - } - if (!WriteSettingsFile(&errors)) { - error = strprintf("Failed saving settings file:\n%s\n", MakeUnorderedList(errors)); - return false; - } - return true; -} - bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp, bool backup) const { fs::path settings = GetPathArg("-settings", BITCOIN_SETTINGS_FILENAME); @@ -854,15 +813,15 @@ fs::path GetBackupsDir() return gArgs.GetBackupsDirPath(); } -bool CheckDataDirOption() +bool CheckDataDirOption(const ArgsManager& args) { - const fs::path datadir{gArgs.GetPathArg("-datadir")}; + const fs::path datadir{args.GetPathArg("-datadir")}; return datadir.empty() || fs::is_directory(fs::absolute(datadir)); } -fs::path GetConfigFile(const fs::path& configuration_file_path) +fs::path GetConfigFile(const ArgsManager& args, const fs::path& configuration_file_path) { - return AbsPathForConfigVal(configuration_file_path, /*net_specific=*/false); + return AbsPathForConfigVal(args, configuration_file_path, /*net_specific=*/false); } static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector>& options, std::list& sections) @@ -955,7 +914,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file fs::path ArgsManager::GetConfigFilePath() const { - return GetConfigFile(GetPathArg("-conf", BITCOIN_CONF_FILENAME)); + return GetConfigFile(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME)); } bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) @@ -1014,7 +973,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) const size_t default_includes = add_includes({}); for (const std::string& conf_file_name : conf_file_names) { - std::ifstream conf_file_stream{GetConfigFile(fs::PathFromString(conf_file_name))}; + std::ifstream conf_file_stream{GetConfigFile(*this, fs::PathFromString(conf_file_name))}; if (conf_file_stream.good()) { if (!ReadConfigStream(conf_file_stream, conf_file_name, error, ignore_invalid_keys)) { return false; @@ -1041,7 +1000,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) } } else { // Create an empty dash.conf if it does not exist - std::ofstream configFile{GetConfigFile(conf_path), std::ios_base::app}; + std::ofstream configFile{GetConfigFile(*this, conf_path), std::ios_base::app}; if (!configFile.good()) return false; configFile.close(); @@ -1049,8 +1008,8 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) } // If datadir is changed in .conf file: - gArgs.ClearPathCache(); - if (!CheckDataDirOption()) { + ClearPathCache(); + if (!CheckDataDirOption(*this)) { error = strprintf("specified data directory \"%s\" does not exist.", GetArg("-datadir", "")); return false; } @@ -1259,12 +1218,12 @@ int64_t GetStartupTime() return nStartupTime; } -fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific) +fs::path AbsPathForConfigVal(const ArgsManager& args, const fs::path& path, bool net_specific) { if (path.is_absolute()) { return path; } - return fsbridge::AbsPathJoin(net_specific ? gArgs.GetDataDirNet() : gArgs.GetDataDirBase(), path); + return fsbridge::AbsPathJoin(net_specific ? args.GetDataDirNet() : args.GetDataDirBase(), path); } void ScheduleBatchPriority() diff --git a/src/util/system.h b/src/util/system.h index 5ef62c06a28d..96661b3eb122 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -38,6 +38,7 @@ extern int nWalletBackups; extern const std::string gCoinJoinName; +class ArgsManager; class UniValue; // Application startup time (used for uptime calculation) @@ -59,8 +60,8 @@ bool error(const char* fmt, const Args&... args) void PrintExceptionContinue(const std::exception_ptr pex, const char* pszExceptionOrigin); // Return true if -datadir option points to a valid directory or is not specified. -bool CheckDataDirOption(); -fs::path GetConfigFile(const fs::path& configuration_file_path); +bool CheckDataDirOption(const ArgsManager& args); +fs::path GetConfigFile(const ArgsManager& args, const fs::path& configuration_file_path); #ifndef WIN32 std::string ShellEscape(const std::string& arg); #endif @@ -72,11 +73,12 @@ void runCommand(const std::string& strCommand); * Most paths passed as configuration arguments are treated as relative to * the datadir if they are not absolute. * + * @param args Parsed arguments and settings. * @param path The path to be conditionally prefixed with datadir. * @param net_specific Use network specific datadir variant * @return The normalized path. */ -fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific = true); +fs::path AbsPathForConfigVal(const ArgsManager& args, const fs::path& path, bool net_specific = true); inline bool IsSwitchChar(char c) { @@ -413,13 +415,6 @@ class ArgsManager */ std::optional GetArgFlags(const std::string& name) const; - /** - * Read and update settings file with saved settings. This needs to be - * called after SelectParams() because the settings file location is - * network-specific. - */ - bool InitSettings(std::string& error); - /** * Get settings file path, or return false if read-write settings were * disabled with -nosettings. @@ -459,12 +454,6 @@ class ArgsManager */ void LogArgs() const; - /** - * If datadir does not exist, create it along with wallets/ - * subdirectory(s). - */ - void EnsureDataDir() const; - private: /** * Get data directory path diff --git a/src/util/translation.h b/src/util/translation.h index 07b7f43c8a87..baf1e7719d74 100644 --- a/src/util/translation.h +++ b/src/util/translation.h @@ -47,11 +47,24 @@ inline bilingual_str operator+(bilingual_str lhs, const bilingual_str& rhs) /** Mark a bilingual_str as untranslated */ inline bilingual_str Untranslated(std::string original) { return {original, original}; } +// Provide an overload of tinyformat::format which can take bilingual_str arguments. namespace tinyformat { +inline std::string TranslateArg(const bilingual_str& arg, bool translated) +{ + return translated ? arg.translated : arg.original; +} + +template +inline T const& TranslateArg(const T& arg, bool translated) +{ + return arg; +} + template bilingual_str format(const bilingual_str& fmt, const Args&... args) { - return bilingual_str{format(fmt.original, args...), format(fmt.translated, args...)}; + return bilingual_str{tfm::format(fmt.original, TranslateArg(args, false)...), + tfm::format(fmt.translated, TranslateArg(args, true)...)}; } } // namespace tinyformat diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index 3c4e03bb05ee..b843176015f6 100755 --- a/test/functional/feature_abortnode.py +++ b/test/functional/feature_abortnode.py @@ -40,7 +40,7 @@ def run_test(self): # Check that node0 aborted self.log.info("Waiting for crash") - self.nodes[0].wait_until_stopped(timeout=5) + self.nodes[0].wait_until_stopped(timeout=5, expect_error=True) self.log.info("Node crashed - now verifying restart fails") self.nodes[0].assert_start_raises_init_error() diff --git a/test/functional/feature_help.py b/test/functional/feature_help.py index 88bf53481937..5c437a40645f 100755 --- a/test/functional/feature_help.py +++ b/test/functional/feature_help.py @@ -4,6 +4,8 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Verify that starting dashd with -h works as expected.""" +from pathlib import Path + from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal @@ -49,6 +51,22 @@ def run_test(self): assert b'version' in output self.log.info(f"Version text received: {output[0:60]} (...)") + missing_datadir = Path(self.options.tmpdir) / "missing_crashinfo_datadir" + assert not missing_datadir.exists() + + self.log.info("Start dashd with -printcrashinfo and a nonexistent datadir") + self.nodes[0].start(extra_args=["-printcrashinfo=invalid", f"-datadir={missing_datadir}"]) + output, error = self.get_node_output(ret_code_expected=0) + assert b'Error while deserializing crash info' in output + assert_equal(error, b'') + assert not missing_datadir.exists() + + self.log.info("Start dashd with a nonexistent datadir and no -printcrashinfo") + self.nodes[0].start(extra_args=[f"-datadir={missing_datadir}"]) + _, error = self.get_node_output(ret_code_expected=1) + assert b'Specified data directory' in error + assert not missing_datadir.exists() + # Test that arguments not in the help results in an error self.log.info("Start dashdd with -fakearg to make sure it does not start") self.nodes[0].start(extra_args=['-fakearg']) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index dad1a802a2b9..c43bd0b3f815 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -406,7 +406,7 @@ def stop_node(self, expected_stderr='', *, wait=0, wait_until_stopped=True): if wait_until_stopped: self.wait_until_stopped() - def is_node_stopped(self): + def is_node_stopped(self, expected_ret_code=None): """Checks whether the node has stopped. Returns True if the node has stopped. False otherwise. @@ -418,8 +418,13 @@ def is_node_stopped(self): return False # process has stopped. Assert that it didn't return an error code. - assert return_code == 0, self._node_msg( - "Node returned non-zero exit code (%d) when stopping" % return_code) + # unless 'expected_ret_code' is provided. + if expected_ret_code is not None: + assert return_code == expected_ret_code, self._node_msg( + "Node returned unexpected exit code (%d) vs (%d) when stopping" % (return_code, expected_ret_code)) + else: + assert return_code == 0, self._node_msg( + "Node returned non-zero exit code (%d) when stopping" % return_code) self.running = False self.process = None self.rpc_connected = False @@ -427,8 +432,9 @@ def is_node_stopped(self): self.log.debug("Node stopped") return True - def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT): - wait_until_helper(self.is_node_stopped, timeout=timeout, timeout_factor=self.timeout_factor) + def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT, expect_error=False): + expected_ret_code = 1 if expect_error else None # Whether node shutdown return EXIT_FAILURE or EXIT_SUCCESS + wait_until_helper(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code), timeout=timeout, timeout_factor=self.timeout_factor) def replace_in_config(self, replacements): """ diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 07b0c86979d6..caf0ecb82777 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -22,6 +22,7 @@ ("src/rpc/evo.cpp", "strprintf(it->second, nParamNum)"), ("src/stacktraces.cpp", "strprintf(fmtStr, i, si.pc, lstr, fstr)"), ("src/util/system.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), + ("src/test/translation_tests.cpp", "strprintf(format, arg)"), ("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), ("src/wallet/wallet.h", "LogPrintf((\"%s \" + fmt).c_str(), GetDisplayName(), parameters...)"),