Merge bitcoin/bitcoin#27150: Deduplicate bitcoind and bitcoin-qt init code

802cc1ef53 Deduplicate bitcoind and bitcoin-qt init code (Ryan Ofsky)
d172b5c671 Add InitError(error, details) overload (Ryan Ofsky)
3db2874bd7 Extend bilingual_str support for tinyformat (Ryan Ofsky)
c361df90b9 scripted-diff: Remove double newlines after some init errors (Ryan Ofsky)

Pull request description:

  Add common InitConfig function to deduplicate bitcoind and bitcoin-qt code reading config files and creating the datadir.

  Noticed the duplicate code while reviewing #27073 and want to remove it because difference in bitcoin-qt and bitcoind behavior make it hard to evaluate changes like #27073

  There are a few minor changes in behavior:

  - In bitcoin-qt, when there is a problem reading the configuration file, the GUI error text has changed from "Error: Cannot parse configuration file:" to "Error reading configuration file:" to be consistent with bitcoind.
  - In bitcoind, when there is a problem reading the settings.json file, the error text has changed from "Failed loading settings file" to "Settings file could not be read" to be consistent with bitcoin-qt.
  - In bitcoind, when there is a problem writing the settings.json file, the error text has changed from "Failed saving settings file" to "Settings file could not be written" to be consistent with bitcoin-qt.
  - In bitcoin-qt, if there datadir is not accessible (e.g. no permission to read), there is an normal error dialog showing "Error: filesystem error: status: Permission denied [.../settings.json]", instead of an uncaught exception.

ACKs for top commit:
  Sjors:
    Light review ACK 802cc1ef53
  TheCharlatan:
    ACK 802cc1ef53
  achow101:
    ACK 802cc1ef53

Tree-SHA512: 9c78d277e9ed595fa8ce286b97d2806e1ec06ddbbe7bd3434bd9dd7b456faf8d989f71231e97311f36edb9caaec645a50c730bd7514b8e0fe6e6f7741b13d981

ci/test/06_script_b.sh

@@ -42,6 +42,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
   ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error"
   export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/"
   CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\
+          " src/common/init.cpp"\
           " src/common/url.cpp"\
           " src/compat"\
           " src/dbwrapper.cpp"\

src/Makefile.am

@@ -134,6 +134,7 @@ BITCOIN_CORE_H = \
   clientversion.h \
   coins.h \
   common/bloom.h \
+  common/init.h \
   common/run_command.h \
   common/url.h \
   compat/assumptions.h \
@@ -640,6 +641,7 @@ libbitcoin_common_a_SOURCES = \
   chainparams.cpp \
   coins.cpp \
   common/bloom.cpp \
+  common/init.cpp \
   common/interfaces.cpp \
   common/run_command.cpp \
   compressor.cpp \

src/Makefile.test.include

@@ -147,6 +147,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 \

src/bitcoind.cpp

@@ -9,6 +9,7 @@
 
 #include <chainparams.h>
 #include <clientversion.h>
+#include <common/init.h>
 #include <common/url.h>
 #include <compat/compat.h>
 #include <init.h>
@@ -120,7 +121,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[])
     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)));
     }
 
     // Process help and version before taking care about datadir
@@ -150,31 +151,17 @@ static bool AppInit(NodeContext& node, int argc, char* argv[])
     std::any context{&node};
     try
     {
-        if (!CheckDataDirOption(args)) {
-            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())));
+        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 bitcoind -h for a list of options.\n", argv[i])));
+                return InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see bitcoind -h for a list of options.", argv[i])));
             }
         }
 
-        if (!args.InitSettings(error)) {
-            InitError(Untranslated(error));
-            return false;
-        }
-
         // -server defaults to true for bitcoind but not for the GUI so do this here
         args.SoftSetBoolArg("-server", true);
         // Set this early so that parameter interactions go to console
@@ -210,7 +197,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
@@ -222,7 +209,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

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 <common/init.h>
+#include <chainparams.h>
+#include <fs.h>
+#include <tinyformat.h>
+#include <util/system.h>
+#include <util/translation.h>
+
+#include <algorithm>
+#include <exception>
+#include <optional>
+
+namespace common {
+std::optional<ConfigError> 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<std::string> 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

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 <util/translation.h>
+
+#include <functional>
+#include <optional>
+#include <string>
+#include <vector>
+
+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<std::string> 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<bool(const bilingual_str& message, const std::vector<std::string>& details)>;
+
+/* Read config files, and create datadir and settings.json if they don't exist. */
+std::optional<ConfigError> InitConfig(ArgsManager& args, SettingsAbortFn settings_abort_fn = nullptr);
+} // namespace common
+
+#endif // BITCOIN_COMMON_INIT_H

src/index/base.cpp

@@ -35,7 +35,7 @@ static void FatalError(const char* fmt, const Args&... args)
     std::string strMessage = tfm::format(fmt, args...);
     SetMiscWarning(Untranslated(strMessage));
     LogPrintf("*** %s\n", strMessage);
-    AbortError(_("A fatal internal error occurred, see debug.log for details"));
+    InitError(_("A fatal internal error occurred, see debug.log for details"));
     StartShutdown();
 }
 

src/node/interface_ui.cpp

@@ -4,6 +4,7 @@
 
 #include <node/interface_ui.h>
 
+#include <util/string.h>
 #include <util/translation.h>
 
 #include <boost/signals2/optional_last_value.hpp>
@@ -62,6 +63,18 @@ bool InitError(const bilingual_str& str)
     return false;
 }
 
+bool InitError(const bilingual_str& str, const std::vector<std::string>& 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);

src/node/interface_ui.h

@@ -116,7 +116,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<std::string>& details);
 
 extern CClientUIInterface uiInterface;
 

src/qt/bitcoin.cpp

@@ -9,6 +9,7 @@
 #include <qt/bitcoin.h>
 
 #include <chainparams.h>
+#include <common/init.h>
 #include <init.h>
 #include <interfaces/handler.h>
 #include <interfaces/init.h>
@@ -165,54 +166,36 @@ static void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTrans
     }
 }
 
-static bool InitSettings()
+static bool ErrorSettingsRead(const bilingual_str& error, const std::vector<std::string>& details)
 {
-    gArgs.EnsureDataDir();
-    if (!gArgs.GetSettingsPath()) {
-        return true; // Do nothing if settings file disabled.
-    }
-
-    std::vector<std::string> errors;
-    if (!gArgs.ReadSettingsFile(&errors)) {
-        std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read");
-        std::string error_translated = QCoreApplication::translate("bitcoin-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("bitcoin-core", "Settings file could not be written");
-        std::string error_translated = QCoreApplication::translate("bitcoin-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<std::string>& 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 */
@@ -546,7 +529,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
@@ -587,34 +570,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(gArgs)) {
-        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 bitcoin.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 bitcoin.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
@@ -622,10 +594,6 @@ int GuiMain(int argc, char* argv[])
     PaymentServer::ipcParseCommandLine(argc, argv);
 #endif
 
-    if (!InitSettings()) {
-        return EXIT_FAILURE;
-    }
-
     QScopedPointer<const NetworkStyle> networkStyle(NetworkStyle::instantiate(Params().NetworkIDString()));
     assert(!networkStyle.isNull());
     // Allow for separate UI settings for testnets

src/shutdown.cpp

@@ -27,7 +27,7 @@ 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);
     StartShutdown();
     return false;
 }

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 <tinyformat.h>
+#include <util/translation.h>
+
+#include <boost/test/unit_test.hpp>
+
+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()

src/util/system.cpp

@@ -438,27 +438,6 @@ const fs::path& ArgsManager::GetDataDir(bool net_specific) const
     return path;
 }
 
-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);
@@ -502,25 +481,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<std::string> 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);

src/util/system.h

@@ -434,13 +434,6 @@ protected:
      */
     std::optional<unsigned int> 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.
@@ -480,12 +473,6 @@ protected:
      */
     void LogArgs() const;
 
-    /**
-     * If datadir does not exist, create it along with wallets/
-     * subdirectory(s).
-     */
-    void EnsureDataDir() const;
-
 private:
     /**
      * Get data directory path

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 <typename T>
+inline T const& TranslateArg(const T& arg, bool translated)
+{
+    return arg;
+}
+
 template <typename... Args>
 bilingual_str format(const bilingual_str& fmt, const Args&... args)
 {
-    return bilingual_str{format(fmt.original, args...), format(fmt.translated, args...)};
+    return bilingual_str{format(fmt.original, TranslateArg(args, false)...),
+                         format(fmt.translated, TranslateArg(args, true)...)};
 }
 } // namespace tinyformat
 

test/lint/run-lint-format-strings.py

@@ -17,6 +17,7 @@ FALSE_POSITIVES = [
     ("src/index/base.cpp", "FatalError(const char* fmt, const Args&... args)"),
     ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"),
     ("src/clientversion.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...)"),