merge #15935: Add <datadir>/settings.json persistent settings storage

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

doc/files.md

@@ -52,7 +52,7 @@ Subdirectory       | File(s)               | Description
 `evodb/`         |                       |special txes and quorums database
 `llmq/`          |                       |quorum signatures database
 `./`               | `banlist.dat`         | Stores the IPs/subnets of banned nodes
-`./`               | `dash.conf`        | Contains [configuration settings](dash-conf.md) for `dashd` or `dash-qt`; can be specified by `-conf` option
+`./`               | `dash.conf`        | User-defined [configuration settings](dash-conf.md) for `dashd` or `dash-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option
 `./`               | `dashd.pid`        | Stores the process ID (PID) of `dashd` or `dash-qt` while running; created at start and deleted on shutdown; can be specified by `-pid` option
 `./`               | `debug.log`           | Contains debug information and general logging generated by `dashd` or `dash-qt`; can be specified by `-debuglogfile` option
 `./`               | `governance.dat`      | stores data for governance objects
@@ -63,6 +63,7 @@ Subdirectory       | File(s)               | Description
 `./`               | `mempool.dat`         | Dump of the mempool's transactions
 `./`               | `onion_v3_private_key` | Cached Tor hidden service private key for `-listenonion` option
 `./`               | `peers.dat`           | Peer IP address database (custom format)
+`./`               | `settings.json`       | Read-write settings set through GUI or RPC interfaces, augmenting manual settings from [dash.conf](dash-conf.md). File is created automatically if read-write settings storage is not disabled with `-nosettings` option. Path can be specified with `-settings` option
 `./`               | `.cookie`             | Session RPC authentication cookie; if used, created at start and deleted on shutdown; can be specified by `-rpccookiefile` option
 `./`               | `.lock`               | Data directory lock file
 

src/dashd.cpp

@@ -102,6 +102,11 @@ static bool AppInit(int argc, char* argv[])
             }
         }
 
+        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

src/init.cpp

@@ -503,7 +503,7 @@ void SetupServerArgs(NodeContext& node)
 #endif
     argsman.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless '-whitelistforcerelay' is '1', in which case whitelisted peers' transactions will be relayed. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-conf=<file>", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (%d to %d, default: %d). In addition, unused mempool memory is shared for this cache (see -maxmempool).", nMinDbCache, nMaxDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -522,6 +522,7 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -rescan and -disablegovernance=false. "
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
             "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-settings=<file>", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-syncmempool", strprintf("Sync mempool from other nodes on start (default: %u)", DEFAULT_SYNC_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 #ifndef WIN32
     argsman.AddArg("-sysperms", "Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

src/interfaces/node.cpp

@@ -187,6 +187,7 @@ public:
     bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
     bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
     void selectParams(const std::string& network) override { SelectParams(network); }
+    bool initSettings(std::string& error) override { return gArgs.InitSettings(error); }
     uint64_t getAssumedBlockchainSize() override { return Params().AssumedBlockchainSize(); }
     uint64_t getAssumedChainStateSize() override { return Params().AssumedChainStateSize(); }
     std::string getNetwork() override { return Params().NetworkIDString(); }

src/interfaces/node.h

@@ -136,6 +136,11 @@ public:
     //! Choose network parameters.
     virtual void selectParams(const std::string& network) = 0;
 
+    //! Read and update <datadir>/settings.json file with saved settings. This
+    //! needs to be called after selectParams() because the settings file
+    //! location is network-specific.
+    virtual bool initSettings(std::string& error) = 0;
+
     //! Get the (assumed) blockchain size.
     virtual uint64_t getAssumedBlockchainSize() = 0;
 

src/qt/dash.cpp

@@ -551,6 +551,11 @@ int GuiMain(int argc, char* argv[])
     // Parse URIs on command line -- this can affect Params()
     PaymentServer::ipcParseCommandLine(*node, argc, argv);
 #endif
+    if (!node->initSettings(error)) {
+        node->initError(Untranslated(error));
+        QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error initializing settings: %1").arg(QString::fromStdString(error)));
+        return EXIT_FAILURE;
+    }
 
     QScopedPointer<const NetworkStyle> networkStyle(NetworkStyle::instantiate(QString::fromStdString(Params().NetworkIDString())));
     assert(!networkStyle.isNull());

src/test/settings_tests.cpp

@@ -10,10 +10,90 @@
 #include <boost/test/unit_test.hpp>
 #include <univalue.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 #include <vector>
 
+inline bool operator==(const util::SettingsValue& a, const util::SettingsValue& b)
+{
+    return a.write() == b.write();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const util::SettingsValue& value)
+{
+    os << value.write();
+    return os;
+}
+
+inline std::ostream& operator<<(std::ostream& os, const std::pair<std::string, util::SettingsValue>& kv)
+{
+    util::SettingsValue out(util::SettingsValue::VOBJ);
+    out.__pushKV(kv.first, kv.second);
+    os << out.write();
+    return os;
+}
+
+inline void WriteText(const fs::path& path, const std::string& text)
+{
+    fsbridge::ofstream file;
+    file.open(path);
+    file << text;
+}
+
 BOOST_FIXTURE_TEST_SUITE(settings_tests, BasicTestingSetup)
 
+BOOST_AUTO_TEST_CASE(ReadWrite)
+{
+    fs::path path = GetDataDir() / "settings.json";
+
+    WriteText(path, R"({
+        "string": "string",
+        "num": 5,
+        "bool": true,
+        "null": null
+    })");
+
+    std::map<std::string, util::SettingsValue> expected{
+        {"string", "string"},
+        {"num", 5},
+        {"bool", true},
+        {"null", {}},
+    };
+
+    // Check file read.
+    std::map<std::string, util::SettingsValue> values;
+    std::vector<std::string> errors;
+    BOOST_CHECK(util::ReadSettings(path, values, errors));
+    BOOST_CHECK_EQUAL_COLLECTIONS(values.begin(), values.end(), expected.begin(), expected.end());
+    BOOST_CHECK(errors.empty());
+
+    // Check no errors if file doesn't exist.
+    fs::remove(path);
+    BOOST_CHECK(util::ReadSettings(path, values, errors));
+    BOOST_CHECK(values.empty());
+    BOOST_CHECK(errors.empty());
+
+    // Check duplicate keys not allowed
+    WriteText(path, R"({
+        "dupe": "string",
+        "dupe": "dupe"
+    })");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> dup_keys = {strprintf("Found duplicate key dupe in settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), dup_keys.begin(), dup_keys.end());
+
+    // Check non-kv json files not allowed
+    WriteText(path, R"("non-kv")");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> non_kv = {strprintf("Found non-object value \"non-kv\" in settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), non_kv.begin(), non_kv.end());
+
+    // Check invalid json not allowed
+    WriteText(path, R"(invalid json)");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> fail_parse = {strprintf("Unable to parse settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end());
+}
+
 //! Check settings struct contents against expected json strings.
 static void CheckValues(const util::Settings& settings, const std::string& single_val, const std::string& list_val)
 {

src/test/util_tests.cpp

@@ -6,6 +6,7 @@
 
 #include <clientversion.h>
 #include <sync.h>
+#include <test/util/logging.h>
 #include <test/util.h>
 #include <util/getuniquepath.h>
 #include <util/strencodings.h>
@@ -972,6 +973,28 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup)
     BOOST_CHECK_EQUAL(out_sha_hex, "3e70723862e346ed6e9b48d8efa13d4d56334c0b73fbf3c3a6ac8b8f4d914f65");
 }
 
+BOOST_AUTO_TEST_CASE(util_ReadWriteSettings)
+{
+    // Test writing setting.
+    TestArgsManager args1;
+    args1.LockSettings([&](util::Settings& settings) { settings.rw_settings["name"] = "value"; });
+    args1.WriteSettingsFile();
+
+    // Test reading setting.
+    TestArgsManager args2;
+    args2.ReadSettingsFile();
+    args2.LockSettings([&](util::Settings& settings) { BOOST_CHECK_EQUAL(settings.rw_settings["name"].get_str(), "value"); });
+
+    // Test error logging, and remove previously written setting.
+    {
+        ASSERT_DEBUG_LOG("Failed renaming settings file");
+        fs::remove(GetDataDir() / "settings.json");
+        fs::create_directory(GetDataDir() / "settings.json");
+        args2.WriteSettingsFile();
+        fs::remove(GetDataDir() / "settings.json");
+    }
+}
+
 BOOST_AUTO_TEST_CASE(util_FormatMoney)
 {
     BOOST_CHECK_EQUAL(FormatMoney(0), "0.00");

src/util/settings.cpp

@@ -4,6 +4,7 @@
 
 #include <util/settings.h>
 
+#include <tinyformat.h>
 #include <univalue.h>
 
 namespace util {
@@ -12,12 +13,13 @@ namespace {
 enum class Source {
    FORCED,
    COMMAND_LINE,
+   RW_SETTINGS,
    CONFIG_FILE_NETWORK_SECTION,
    CONFIG_FILE_DEFAULT_SECTION
 };
 
 //! Merge settings from multiple sources in precedence order:
-//! Forced config > command line > config file network-specific section > config file default section
+//! Forced config > command line > read-write settings file > config file network-specific section > config file default section
 //!
 //! This function is provided with a callback function fn that contains
 //! specific logic for how to merge the sources.
@@ -32,6 +34,10 @@ static void MergeSettings(const Settings& settings, const std::string& section,
     if (auto* values = FindKey(settings.command_line_options, name)) {
         fn(SettingsSpan(*values), Source::COMMAND_LINE);
     }
+    // Merge in the read-write settings
+    if (const SettingsValue* value = FindKey(settings.rw_settings, name)) {
+        fn(SettingsSpan(*value), Source::RW_SETTINGS);
+    }
     // Merge in the network-specific section of the config file
     if (!section.empty()) {
         if (auto* map = FindKey(settings.ro_config, section)) {
@@ -49,6 +55,62 @@ static void MergeSettings(const Settings& settings, const std::string& section,
 }
 } // namespace
 
+bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& values, std::vector<std::string>& errors)
+{
+    values.clear();
+    errors.clear();
+
+    fsbridge::ifstream file;
+    file.open(path);
+    if (!file.is_open()) return true; // Ok for file not to exist.
+
+    SettingsValue in;
+    if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) {
+        errors.emplace_back(strprintf("Unable to parse settings file %s", path.string()));
+        return false;
+    }
+
+    if (file.fail()) {
+        errors.emplace_back(strprintf("Failed reading settings file %s", path.string()));
+        return false;
+    }
+    file.close(); // Done with file descriptor. Release while copying data.
+
+    if (!in.isObject()) {
+        errors.emplace_back(strprintf("Found non-object value %s in settings file %s", in.write(), path.string()));
+        return false;
+    }
+
+    const std::vector<std::string>& in_keys = in.getKeys();
+    const std::vector<SettingsValue>& in_values = in.getValues();
+    for (size_t i = 0; i < in_keys.size(); ++i) {
+        auto inserted = values.emplace(in_keys[i], in_values[i]);
+        if (!inserted.second) {
+            errors.emplace_back(strprintf("Found duplicate key %s in settings file %s", in_keys[i], path.string()));
+        }
+    }
+    return errors.empty();
+}
+
+bool WriteSettings(const fs::path& path,
+    const std::map<std::string, SettingsValue>& values,
+    std::vector<std::string>& errors)
+{
+    SettingsValue out(SettingsValue::VOBJ);
+    for (const auto& value : values) {
+        out.__pushKV(value.first, value.second);
+    }
+    fsbridge::ofstream file;
+    file.open(path);
+    if (file.fail()) {
+        errors.emplace_back(strprintf("Error: Unable to open settings file %s for writing", path.string()));
+        return false;
+    }
+    file << out.write(/* prettyIndent= */ 1, /* indentLevel= */ 4) << std::endl;
+    file.close();
+    return true;
+}
+
 SettingsValue GetSetting(const Settings& settings,
     const std::string& section,
     const std::string& name,

src/util/settings.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_UTIL_SETTINGS_H
 #define BITCOIN_UTIL_SETTINGS_H
 
+#include <fs.h>
+
 #include <map>
 #include <string>
 #include <vector>
@@ -24,19 +26,31 @@ namespace util {
 //!       https://github.com/bitcoin/bitcoin/pull/15934/files#r337691812)
 using SettingsValue = UniValue;
 
-//! Stored bitcoin settings. This struct combines settings from the command line
+//! Stored settings. This struct combines settings from the command line, a
-//! and a read-only configuration file.
+//! read-only configuration file, and a read-write runtime settings file.
 struct Settings {
     //! Map of setting name to forced setting value.
     std::map<std::string, SettingsValue> forced_settings;
     //! Map of setting name to list of command line values.
     std::map<std::string, std::vector<SettingsValue>> command_line_options;
+    //! Map of setting name to read-write file setting value.
+    std::map<std::string, SettingsValue> rw_settings;
     //! Map of config section name and setting name to list of config file values.
     std::map<std::string, std::map<std::string, std::vector<SettingsValue>>> ro_config;
 };
 
+//! Read settings file.
+bool ReadSettings(const fs::path& path,
+    std::map<std::string, SettingsValue>& values,
+    std::vector<std::string>& errors);
+
+//! Write settings file.
+bool WriteSettings(const fs::path& path,
+    const std::map<std::string, SettingsValue>& values,
+    std::vector<std::string>& errors);
+
 //! Get settings value from combined sources: forced settings, command line
-//! arguments and the read-only config file.
+//! arguments, runtime read-write settings, and the read-only config file.
 //!
 //! @param ignore_default_section_config - ignore values in the default section
 //!                                        of the config file (part before any

src/util/system.cpp

@@ -89,6 +89,7 @@ const std::string gCoinJoinName = "CoinJoin";
 int nWalletBackups = 10;
 
 const char * const BITCOIN_CONF_FILENAME = "dash.conf";
+const char * const BITCOIN_SETTINGS_FILENAME = "settings.json";
 
 ArgsManager gArgs;
 
@@ -412,6 +413,84 @@ bool ArgsManager::IsArgSet(const std::string& strArg) const
     return !ArgsManagerHelper::Get(*this, strArg).isNull();
 }
 
+bool ArgsManager::InitSettings(std::string& error)
+{
+    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", Join(errors, "\n- "));
+        return false;
+    }
+    if (!WriteSettingsFile(&errors)) {
+        error = strprintf("Failed saving settings file:\n- %s\n", Join(errors, "\n- "));
+        return false;
+    }
+    return true;
+}
+
+bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp) const
+{
+    if (IsArgNegated("-settings")) {
+        return false;
+    }
+    if (filepath) {
+        std::string settings = GetArg("-settings", BITCOIN_SETTINGS_FILENAME);
+        *filepath = fs::absolute(temp ? settings + ".tmp" : settings, GetDataDir(/* net_specific= */ true));
+    }
+    return true;
+}
+
+static void SaveErrors(const std::vector<std::string> errors, std::vector<std::string>* error_out)
+{
+    for (const auto& error : errors) {
+        if (error_out) {
+            error_out->emplace_back(error);
+        } else {
+            LogPrintf("%s\n", error);
+        }
+    }
+}
+
+bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors)
+{
+    fs::path path;
+    if (!GetSettingsPath(&path, /* temp= */ false)) {
+        return true; // Do nothing if settings file disabled.
+    }
+
+    LOCK(cs_args);
+    m_settings.rw_settings.clear();
+    std::vector<std::string> read_errors;
+    if (!util::ReadSettings(path, m_settings.rw_settings, read_errors)) {
+        SaveErrors(read_errors, errors);
+        return false;
+    }
+    return true;
+}
+
+bool ArgsManager::WriteSettingsFile(std::vector<std::string>* errors) const
+{
+    fs::path path, path_tmp;
+    if (!GetSettingsPath(&path, /* temp= */ false) || !GetSettingsPath(&path_tmp, /* temp= */ true)) {
+        throw std::logic_error("Attempt to write settings file when dynamic settings are disabled.");
+    }
+
+    LOCK(cs_args);
+    std::vector<std::string> write_errors;
+    if (!util::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) {
+        SaveErrors(write_errors, errors);
+        return false;
+    }
+    if (!RenameOver(path_tmp, path)) {
+        SaveErrors({strprintf("Failed renaming settings file %s to %s\n", path_tmp.string(), path.string())}, errors);
+        return false;
+    }
+    return true;
+}
+
 bool ArgsManager::IsArgNegated(const std::string& strArg) const
 {
     return ArgsManagerHelper::Get(*this, strArg).isFalse();
@@ -958,6 +1037,9 @@ void ArgsManager::LogArgs() const
     for (const auto& section : m_settings.ro_config) {
         logArgsPrefix("Config file arg:", section.first, section.second);
     }
+    for (const auto& setting : m_settings.rw_settings) {
+        LogPrintf("Setting file arg: %s = %s\n", setting.first, setting.second.write());
+    }
     logArgsPrefix("Command-line arg:", "", m_settings.command_line_options);
 }
 

src/util/system.h

@@ -61,6 +61,7 @@ extern const std::string gCoinJoinName;
 int64_t GetStartupTime();
 
 extern const char * const BITCOIN_CONF_FILENAME;
+extern const char * const BITCOIN_SETTINGS_FILENAME;
 
 void SetupEnvironment();
 bool SetupNetworking();
@@ -343,6 +344,39 @@ public:
      */
     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.
+     */
+    bool GetSettingsPath(fs::path* filepath = nullptr, bool temp = false) const;
+
+    /**
+     * Read settings file. Push errors to vector, or log them if null.
+     */
+    bool ReadSettingsFile(std::vector<std::string>* errors = nullptr);
+
+    /**
+     * Write settings file. Push errors to vector, or log them if null.
+     */
+    bool WriteSettingsFile(std::vector<std::string>* errors = nullptr) const;
+
+    /**
+     * Access settings with lock held.
+     */
+    template <typename Fn>
+    void LockSettings(Fn&& fn)
+    {
+        LOCK(cs_args);
+        fn(m_settings);
+    }
+
     /**
      * Log the config file options and the command line arguments,
      * useful for troubleshooting.

test/functional/feature_settings.py

@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017-2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test various command line arguments and configuration file parameters."""
+
+import json
+
+from pathlib import Path
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_node import ErrorMatch
+from test_framework.util import assert_equal
+
+
+class SettingsTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+
+    def run_test(self):
+        node, = self.nodes
+        settings = Path(node.datadir, self.chain, "settings.json")
+        conf = Path(node.datadir, "dash.conf")
+
+        # Assert empty settings file was created
+        self.stop_node(0)
+        with settings.open() as fp:
+            assert_equal(json.load(fp), {})
+
+        # Assert settings are parsed and logged
+        with settings.open("w") as fp:
+            json.dump({"string": "string", "num": 5, "bool": True, "null": None, "list": [6,7]}, fp)
+        with node.assert_debug_log(expected_msgs=[
+                'Setting file arg: string = "string"',
+                'Setting file arg: num = 5',
+                'Setting file arg: bool = true',
+                'Setting file arg: null = null',
+                'Setting file arg: list = [6,7]']):
+            self.start_node(0)
+            self.stop_node(0)
+
+        # Assert settings are unchanged after shutdown
+        with settings.open() as fp:
+            assert_equal(json.load(fp), {"string": "string", "num": 5, "bool": True, "null": None, "list": [6,7]})
+
+        # Test invalid json
+        with settings.open("w") as fp:
+            fp.write("invalid json")
+        node.assert_start_raises_init_error(expected_msg='Unable to parse settings file', match=ErrorMatch.PARTIAL_REGEX)
+
+        # Test invalid json object
+        with settings.open("w") as fp:
+            fp.write('"string"')
+        node.assert_start_raises_init_error(expected_msg='Found non-object value "string" in settings file', match=ErrorMatch.PARTIAL_REGEX)
+
+        # Test invalid settings file containing duplicate keys
+        with settings.open("w") as fp:
+            fp.write('{"key": 1, "key": 2}')
+        node.assert_start_raises_init_error(expected_msg='Found duplicate key key in settings file', match=ErrorMatch.PARTIAL_REGEX)
+
+        # Test invalid settings file is ignored with command line -nosettings
+        with node.assert_debug_log(expected_msgs=['Command-line arg: settings=false']):
+            self.start_node(0, extra_args=["-nosettings"])
+            self.stop_node(0)
+
+        # Test invalid settings file is ignored with config file -nosettings
+        with conf.open('a') as conf:
+            conf.write('nosettings=1\n')
+        with node.assert_debug_log(expected_msgs=['Config file arg: [regtest] settings=false']):
+            self.start_node(0)
+            self.stop_node(0)
+
+        # Test alternate settings path
+        altsettings = Path(node.datadir, "altsettings.json")
+        with altsettings.open("w") as fp:
+            fp.write('{"key": "value"}')
+        with node.assert_debug_log(expected_msgs=['Setting file arg: key = "value"']):
+            self.start_node(0, extra_args=["-settings={}".format(altsettings)])
+            self.stop_node(0)
+
+
+if __name__ == '__main__':
+    SettingsTest().main()

test/functional/test_runner.py

@@ -248,6 +248,7 @@ BASE_SCRIPTS = [
     'p2p_permissions.py',
     'feature_blocksdir.py',
     'feature_config_args.py',
+    'feature_settings.py',
     'rpc_help.py',
     'feature_help.py',
     # Don't append tests at the end to avoid merge conflicts