Merge #17473: refactor: Settings code cleanups

e9fd366044e271632dc0e4f96e1c14f8e87213ae refactor: Remove null setting check in GetSetting() (Russell Yanofsky)
cba2710220d76bbe790b04088839cbbd410436de scripted-diff: Remove unused ArgsManager type flags in tests (Russell Yanofsky)
425bb307252cf4dec9b3ef6426e6548b2be7a303 refactor: Add util_CheckValue test (Russell Yanofsky)
0fa54358b06b58f4d17073bcc8a959eb9498aadc refactor: Add ArgsManager::GetSettingsList method (Russell Yanofsky)
3e185522ace1678e0a25b9cf8a5553a4bc279bea refactor: Get rid of ArgsManagerHelper class (Russell Yanofsky)
dc0f1480746b34aa3ca2d9c0f1ec764083026b40 refactor: Replace FlagsOfKnownArg with GetArgFlags (Russell Yanofsky)
57e8b7a7273567aa4a4aee87cce18e9bff8f3196 refactor: Clean up includeconf comments (Russell Yanofsky)
3f7dc9b808316c1e5d677af8d9a99112568c8ccb refactor: Clean up long lines in settings code (Russell Yanofsky)

Pull request description:

  This PR doesn't change behavior. It just implements some suggestions from #15934 and #16545 and few other small cleanups.

ACKs for top commit:
  jnewbery:
    Code review ACK e9fd366044e271632dc0e4f96e1c14f8e87213ae
  MarcoFalke:
    ACK e9fd366044 🚟

Tree-SHA512: 6e100d92c72f72bc39567187ab97a3547b3c06e5fcf1a1b74023358b8bca552124ca6a53c0ab53179b7f1329c03d9a73faaef6d73d2cd1a2321568a0286525e2

7 files changed, 235 insertions, 81 deletions

diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
index 5abd1087ec..4c64d8c833 100644
--- a/src/test/getarg_tests.cpp
+++ b/src/test/getarg_tests.cpp
@@ -43,7 +43,7 @@ static void SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& a
 
 BOOST_AUTO_TEST_CASE(boolarg)
 {
-    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL);
+    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY);
     SetupArgs({foo});
     ResetArgs("-foo");
     BOOST_CHECK(gArgs.GetBoolArg("-foo", false));
@@ -97,8 +97,8 @@ BOOST_AUTO_TEST_CASE(boolarg)
 
 BOOST_AUTO_TEST_CASE(stringarg)
 {
-    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_STRING);
-    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_STRING);
+    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY);
+    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY);
     SetupArgs({foo, bar});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "");
@@ -124,8 +124,8 @@ BOOST_AUTO_TEST_CASE(stringarg)
 
 BOOST_AUTO_TEST_CASE(intarg)
 {
-    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_INT);
-    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_INT);
+    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY);
+    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY);
     SetupArgs({foo, bar});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11);
@@ -159,8 +159,8 @@ BOOST_AUTO_TEST_CASE(doubledash)
 
 BOOST_AUTO_TEST_CASE(boolargno)
 {
-    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL);
-    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_BOOL);
+    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY);
+    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY);
     SetupArgs({foo, bar});
     ResetArgs("-nofoo");
     BOOST_CHECK(!gArgs.GetBoolArg("-foo", true));
diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp
index 235420e6ac..45644834a5 100644
--- a/src/test/settings_tests.cpp
+++ b/src/test/settings_tests.cpp
@@ -46,6 +46,19 @@ BOOST_AUTO_TEST_CASE(Simple)
     CheckValues(settings2, R"("val2")", R"(["val2","val3"])");
 }
 
+// Confirm that a high priority setting overrides a lower priority setting even
+// if the high priority setting is null. This behavior is useful for a high
+// priority setting source to be able to effectively reset any setting back to
+// its default value.
+BOOST_AUTO_TEST_CASE(NullOverride)
+{
+    util::Settings settings;
+    settings.command_line_options["name"].push_back("value");
+    BOOST_CHECK_EQUAL(R"("value")", GetSetting(settings, "section", "name", false, false).write().c_str());
+    settings.forced_settings["name"] = {};
+    BOOST_CHECK_EQUAL(R"(null)", GetSetting(settings, "section", "name", false, false).write().c_str());
+}
+
 // Test different ways settings can be merged, and verify results. This test can
 // be used to confirm that updates to settings code don't change behavior
 // unintentionally.
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index c45426a631..86ea56ff36 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -5,6 +5,7 @@
 #include <util/system.h>
 
 #include <clientversion.h>
+#include <optional.h>
 #include <sync.h>
 #include <test/util/setup_common.h>
 #include <test/util/str.h>
@@ -189,12 +190,119 @@ struct TestArgsManager : public ArgsManager
             AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS);
         }
     }
+    using ArgsManager::GetSetting;
+    using ArgsManager::GetSettingsList;
     using ArgsManager::ReadConfigStream;
     using ArgsManager::cs_args;
     using ArgsManager::m_network;
     using ArgsManager::m_settings;
 };
 
+//! Test GetSetting and GetArg type coercion, negation, and default value handling.
+class CheckValueTest : public TestChain100Setup
+{
+public:
+    struct Expect {
+        util::SettingsValue setting;
+        bool default_string = false;
+        bool default_int = false;
+        bool default_bool = false;
+        const char* string_value = nullptr;
+        Optional<int64_t> int_value;
+        Optional<bool> bool_value;
+        Optional<std::vector<std::string>> list_value;
+        const char* error = nullptr;
+
+        Expect(util::SettingsValue s) : setting(std::move(s)) {}
+        Expect& DefaultString() { default_string = true; return *this; }
+        Expect& DefaultInt() { default_int = true; return *this; }
+        Expect& DefaultBool() { default_bool = true; return *this; }
+        Expect& String(const char* s) { string_value = s; return *this; }
+        Expect& Int(int64_t i) { int_value = i; return *this; }
+        Expect& Bool(bool b) { bool_value = b; return *this; }
+        Expect& List(std::vector<std::string> m) { list_value = std::move(m); return *this; }
+        Expect& Error(const char* e) { error = e; return *this; }
+    };
+
+    void CheckValue(unsigned int flags, const char* arg, const Expect& expect)
+    {
+        TestArgsManager test;
+        test.SetupArgs({{"-value", flags}});
+        const char* argv[] = {"ignored", arg};
+        std::string error;
+        bool success = test.ParseParameters(arg ? 2 : 1, (char**)argv, error);
+
+        BOOST_CHECK_EQUAL(test.GetSetting("-value").write(), expect.setting.write());
+        auto settings_list = test.GetSettingsList("-value");
+        if (expect.setting.isNull() || expect.setting.isFalse()) {
+            BOOST_CHECK_EQUAL(settings_list.size(), 0);
+        } else {
+            BOOST_CHECK_EQUAL(settings_list.size(), 1);
+            BOOST_CHECK_EQUAL(settings_list[0].write(), expect.setting.write());
+        }
+
+        if (expect.error) {
+            BOOST_CHECK(!success);
+            BOOST_CHECK_NE(error.find(expect.error), std::string::npos);
+        } else {
+            BOOST_CHECK(success);
+            BOOST_CHECK_EQUAL(error, "");
+        }
+
+        if (expect.default_string) {
+            BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), "zzzzz");
+        } else if (expect.string_value) {
+            BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), expect.string_value);
+        } else {
+            BOOST_CHECK(!success);
+        }
+
+        if (expect.default_int) {
+            BOOST_CHECK_EQUAL(test.GetArg("-value", 99999), 99999);
+        } else if (expect.int_value) {
+            BOOST_CHECK_EQUAL(test.GetArg("-value", 99999), *expect.int_value);
+        } else {
+            BOOST_CHECK(!success);
+        }
+
+        if (expect.default_bool) {
+            BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), false);
+            BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), true);
+        } else if (expect.bool_value) {
+            BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), *expect.bool_value);
+            BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), *expect.bool_value);
+        } else {
+            BOOST_CHECK(!success);
+        }
+
+        if (expect.list_value) {
+            auto l = test.GetArgs("-value");
+            BOOST_CHECK_EQUAL_COLLECTIONS(l.begin(), l.end(), expect.list_value->begin(), expect.list_value->end());
+        } else {
+            BOOST_CHECK(!success);
+        }
+    }
+};
+
+BOOST_FIXTURE_TEST_CASE(util_CheckValue, CheckValueTest)
+{
+    using M = ArgsManager;
+
+    CheckValue(M::ALLOW_ANY, nullptr, Expect{{}}.DefaultString().DefaultInt().DefaultBool().List({}));
+    CheckValue(M::ALLOW_ANY, "-novalue", Expect{false}.String("0").Int(0).Bool(false).List({}));
+    CheckValue(M::ALLOW_ANY, "-novalue=", Expect{false}.String("0").Int(0).Bool(false).List({}));
+    CheckValue(M::ALLOW_ANY, "-novalue=0", Expect{true}.String("1").Int(1).Bool(true).List({"1"}));
+    CheckValue(M::ALLOW_ANY, "-novalue=1", Expect{false}.String("0").Int(0).Bool(false).List({}));
+    CheckValue(M::ALLOW_ANY, "-novalue=2", Expect{false}.String("0").Int(0).Bool(false).List({}));
+    CheckValue(M::ALLOW_ANY, "-novalue=abc", Expect{true}.String("1").Int(1).Bool(true).List({"1"}));
+    CheckValue(M::ALLOW_ANY, "-value", Expect{""}.String("").Int(0).Bool(true).List({""}));
+    CheckValue(M::ALLOW_ANY, "-value=", Expect{""}.String("").Int(0).Bool(true).List({""}));
+    CheckValue(M::ALLOW_ANY, "-value=0", Expect{"0"}.String("0").Int(0).Bool(false).List({"0"}));
+    CheckValue(M::ALLOW_ANY, "-value=1", Expect{"1"}.String("1").Int(1).Bool(true).List({"1"}));
+    CheckValue(M::ALLOW_ANY, "-value=2", Expect{"2"}.String("2").Int(2).Bool(true).List({"2"}));
+    CheckValue(M::ALLOW_ANY, "-value=abc", Expect{"abc"}.String("abc").Int(0).Bool(false).List({"abc"}));
+}
+
 BOOST_AUTO_TEST_CASE(util_ParseParameters)
 {
     TestArgsManager testArgs;
@@ -289,12 +397,12 @@ BOOST_AUTO_TEST_CASE(util_ArgParsing)
 BOOST_AUTO_TEST_CASE(util_GetBoolArg)
 {
     TestArgsManager testArgs;
-    const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL);
-    const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL);
-    const auto c = std::make_pair("-c", ArgsManager::ALLOW_BOOL);
-    const auto d = std::make_pair("-d", ArgsManager::ALLOW_BOOL);
-    const auto e = std::make_pair("-e", ArgsManager::ALLOW_BOOL);
-    const auto f = std::make_pair("-f", ArgsManager::ALLOW_BOOL);
+    const auto a = std::make_pair("-a", ArgsManager::ALLOW_ANY);
+    const auto b = std::make_pair("-b", ArgsManager::ALLOW_ANY);
+    const auto c = std::make_pair("-c", ArgsManager::ALLOW_ANY);
+    const auto d = std::make_pair("-d", ArgsManager::ALLOW_ANY);
+    const auto e = std::make_pair("-e", ArgsManager::ALLOW_ANY);
+    const auto f = std::make_pair("-f", ArgsManager::ALLOW_ANY);
 
     const char *argv_test[] = {
         "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
@@ -333,8 +441,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
     TestArgsManager testArgs;
 
     // Params test
-    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL);
-    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_BOOL);
+    const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY);
+    const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY);
     const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"};
     testArgs.SetupArgs({foo, bar});
     std::string error;
@@ -406,16 +514,16 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
 
     TestArgsManager test_args;
     LOCK(test_args.cs_args);
-    const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL);
-    const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL);
-    const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_STRING);
-    const auto d = std::make_pair("-d", ArgsManager::ALLOW_STRING);
+    const auto a = std::make_pair("-a", ArgsManager::ALLOW_ANY);
+    const auto b = std::make_pair("-b", ArgsManager::ALLOW_ANY);
+    const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_ANY);
+    const auto d = std::make_pair("-d", ArgsManager::ALLOW_ANY);
     const auto e = std::make_pair("-e", ArgsManager::ALLOW_ANY);
-    const auto fff = std::make_pair("-fff", ArgsManager::ALLOW_BOOL);
-    const auto ggg = std::make_pair("-ggg", ArgsManager::ALLOW_BOOL);
-    const auto h = std::make_pair("-h", ArgsManager::ALLOW_BOOL);
-    const auto i = std::make_pair("-i", ArgsManager::ALLOW_BOOL);
-    const auto iii = std::make_pair("-iii", ArgsManager::ALLOW_INT);
+    const auto fff = std::make_pair("-fff", ArgsManager::ALLOW_ANY);
+    const auto ggg = std::make_pair("-ggg", ArgsManager::ALLOW_ANY);
+    const auto h = std::make_pair("-h", ArgsManager::ALLOW_ANY);
+    const auto i = std::make_pair("-i", ArgsManager::ALLOW_ANY);
+    const auto iii = std::make_pair("-iii", ArgsManager::ALLOW_ANY);
     test_args.SetupArgs({a, b, ccc, d, e, fff, ggg, h, i, iii});
 
     test_args.ReadConfigString(str_config);
@@ -618,8 +726,8 @@ BOOST_AUTO_TEST_CASE(util_GetArg)
 BOOST_AUTO_TEST_CASE(util_GetChainName)
 {
     TestArgsManager test_args;
-    const auto testnet = std::make_pair("-testnet", ArgsManager::ALLOW_BOOL);
-    const auto regtest = std::make_pair("-regtest", ArgsManager::ALLOW_BOOL);
+    const auto testnet = std::make_pair("-testnet", ArgsManager::ALLOW_ANY);
+    const auto regtest = std::make_pair("-regtest", ArgsManager::ALLOW_ANY);
     test_args.SetupArgs({testnet, regtest});
 
     const char* argv_testnet[] = {"cmd", "-testnet"};
diff --git a/src/util/settings.cpp b/src/util/settings.cpp
index af75fef310..e4979df755 100644
--- a/src/util/settings.cpp
+++ b/src/util/settings.cpp
@@ -56,6 +56,7 @@ SettingsValue GetSetting(const Settings& settings,
     bool get_chain_name)
 {
     SettingsValue result;
+    bool done = false; // Done merging any more settings sources.
     MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) {
         // Weird behavior preserved for backwards compatibility: Apply negated
         // setting even if non-negated setting would be ignored. A negated
@@ -68,7 +69,9 @@ SettingsValue GetSetting(const Settings& settings,
         // precedence over early settings, but for backwards compatibility in
         // the config file the precedence is reversed for all settings except
         // chain name settings.
-        const bool reverse_precedence = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !get_chain_name;
+        const bool reverse_precedence =
+            (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) &&
+            !get_chain_name;
 
         // Weird behavior preserved for backwards compatibility: Negated
         // -regtest and -testnet arguments which you would expect to override
@@ -77,19 +80,23 @@ SettingsValue GetSetting(const Settings& settings,
         // negated values, or at least warn they are ignored.
         const bool skip_negated_command_line = get_chain_name;
 
+        if (done) return;
+
         // Ignore settings in default config section if requested.
-        if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION && !never_ignore_negated_setting) return;
+        if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION &&
+            !never_ignore_negated_setting) {
+            return;
+        }
 
         // Skip negated command line settings.
         if (skip_negated_command_line && span.last_negated()) return;
 
-        // Stick with highest priority value, keeping result if already set.
-        if (!result.isNull()) return;
-
         if (!span.empty()) {
             result = reverse_precedence ? span.begin()[0] : span.end()[-1];
+            done = true;
         } else if (span.last_negated()) {
             result = false;
+            done = true;
         }
     });
     return result;
@@ -101,7 +108,7 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
     bool ignore_default_section_config)
 {
     std::vector<SettingsValue> result;
-    bool result_complete = false;
+    bool done = false; // Done merging any more settings sources.
     bool prev_negated_empty = false;
     MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) {
         // Weird behavior preserved for backwards compatibility: Apply config
@@ -111,14 +118,16 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
         // value is followed by non-negated value, in which case config file
         // settings will be brought back from the dead (but earlier command
         // line settings will still be ignored).
-        const bool add_zombie_config_values = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !prev_negated_empty;
+        const bool add_zombie_config_values =
+            (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) &&
+            !prev_negated_empty;
 
         // Ignore settings in default config section if requested.
         if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION) return;
 
         // Add new settings to the result if isn't already complete, or if the
         // values are zombies.
-        if (!result_complete || add_zombie_config_values) {
+        if (!done || add_zombie_config_values) {
             for (const auto& value : span) {
                 if (value.isArray()) {
                     result.insert(result.end(), value.getValues().begin(), value.getValues().end());
@@ -129,8 +138,8 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
         }
 
         // If a setting was negated, or if a setting was forced, set
-        // result_complete to true to ignore any later lower priority settings.
-        result_complete |= span.negated() > 0 || source == Source::FORCED;
+        // done to true to ignore any later lower priority settings.
+        done |= span.negated() > 0 || source == Source::FORCED;
 
         // Update the negated and empty state used for the zombie values check.
         prev_negated_empty |= span.last_negated() && result.empty();
diff --git a/src/util/settings.h b/src/util/settings.h
index 17832e4d2c..9ca581109d 100644
--- a/src/util/settings.h
+++ b/src/util/settings.h
@@ -43,11 +43,18 @@ struct Settings {
 //!                                        [section] keywords)
 //! @param get_chain_name - enable special backwards compatible behavior
 //!                         for GetChainName
-SettingsValue GetSetting(const Settings& settings, const std::string& section, const std::string& name, bool ignore_default_section_config, bool get_chain_name);
+SettingsValue GetSetting(const Settings& settings,
+    const std::string& section,
+    const std::string& name,
+    bool ignore_default_section_config,
+    bool get_chain_name);
 
 //! Get combined setting value similar to GetSetting(), except if setting was
 //! specified multiple times, return a list of all the values specified.
-std::vector<SettingsValue> GetSettingsList(const Settings& settings, const std::string& section, const std::string& name, bool ignore_default_section_config);
+std::vector<SettingsValue> GetSettingsList(const Settings& settings,
+    const std::string& section,
+    const std::string& name,
+    bool ignore_default_section_config);
 
 //! Return true if a setting is set in the default config file section, and not
 //! overridden by a higher priority command-line or network section value.
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 95458672ed..d99a87a9f2 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -168,23 +168,6 @@ static std::string SettingName(const std::string& arg)
     return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg;
 }
 
-/** Internal helper functions for ArgsManager */
-class ArgsManagerHelper {
-public:
-    /** Determine whether to use config settings in the default section,
-     *  See also comments around ArgsManager::ArgsManager() below. */
-    static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args)
-    {
-        return (am.m_network == CBaseChainParams::MAIN || am.m_network_only_args.count(arg) == 0);
-    }
-
-    static util::SettingsValue Get(const ArgsManager& am, const std::string& arg)
-    {
-        LOCK(am.cs_args);
-        return GetSetting(am.m_settings, am.m_network, SettingName(arg), !UseDefaultSection(am, arg), /* get_chain_name= */ false);
-    }
-};
-
 /**
  * Interpret -nofoo as if the user supplied -foo=0.
  *
@@ -327,9 +310,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
         key.erase(0, 1);
         std::string section;
         util::SettingsValue value = InterpretOption(section, key, val);
-        const unsigned int flags = FlagsOfKnownArg(key);
+        Optional<unsigned int> flags = GetArgFlags('-' + key);
         if (flags) {
-            if (!CheckValid(key, value, flags, error)) {
+            if (!CheckValid(key, value, *flags, error)) {
                 return false;
             }
             // Weird behavior preserved for backwards compatibility: command
@@ -345,7 +328,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
         }
     }
 
-    // we do not allow -includeconf from command line, so we clear it here
+    // we do not allow -includeconf from command line
     bool success = true;
     if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) {
         for (const auto& include : util::SettingsSpan(*includes)) {
@@ -356,25 +339,22 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
     return success;
 }
 
-unsigned int ArgsManager::FlagsOfKnownArg(const std::string& key) const
+Optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const
 {
     LOCK(cs_args);
     for (const auto& arg_map : m_available_args) {
-        const auto search = arg_map.second.find('-' + key);
+        const auto search = arg_map.second.find(name);
         if (search != arg_map.second.end()) {
             return search->second.m_flags;
         }
     }
-    return ArgsManager::NONE;
+    return nullopt;
 }
 
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 {
-    LOCK(cs_args);
-    bool ignore_default_section_config = !ArgsManagerHelper::UseDefaultSection(*this, strArg);
     std::vector<std::string> result;
-    for (const util::SettingsValue& value :
-        util::GetSettingsList(m_settings, m_network, SettingName(strArg), ignore_default_section_config)) {
+    for (const util::SettingsValue& value : GetSettingsList(strArg)) {
         result.push_back(value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str());
     }
     return result;
@@ -382,29 +362,29 @@ std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 
 bool ArgsManager::IsArgSet(const std::string& strArg) const
 {
-    return !ArgsManagerHelper::Get(*this, strArg).isNull();
+    return !GetSetting(strArg).isNull();
 }
 
 bool ArgsManager::IsArgNegated(const std::string& strArg) const
 {
-    return ArgsManagerHelper::Get(*this, strArg).isFalse();
+    return GetSetting(strArg).isFalse();
 }
 
 std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const
 {
-    const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg);
+    const util::SettingsValue value = GetSetting(strArg);
     return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str();
 }
 
 int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
 {
-    const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg);
+    const util::SettingsValue value = GetSetting(strArg);
     return value.isNull() ? nDefault : value.isFalse() ? 0 : value.isTrue() ? 1 : value.isNum() ? value.get_int64() : atoi64(value.get_str());
 }
 
 bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 {
-    const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg);
+    const util::SettingsValue value = GetSetting(strArg);
     return value.isNull() ? fDefault : value.isBool() ? value.get_bool() : InterpretBool(value.get_str());
 }
 
@@ -736,9 +716,9 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
         std::string section;
         std::string key = option.first;
         util::SettingsValue value = InterpretOption(section, key, option.second);
-        const unsigned int flags = FlagsOfKnownArg(key);
+        Optional<unsigned int> flags = GetArgFlags('-' + key);
         if (flags) {
-            if (!CheckValid(key, value, flags, error)) {
+            if (!CheckValid(key, value, *flags, error)) {
                 return false;
             }
             m_settings.ro_config[section][key].push_back(value);
@@ -771,7 +751,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
             return false;
         }
         // `-includeconf` cannot be included in the command line arguments except
-        // as `-noincludeconf` (which indicates that no conf file should be used).
+        // as `-noincludeconf` (which indicates that no included conf file should be used).
         bool use_conf_file{true};
         {
             LOCK(cs_args);
@@ -845,9 +825,9 @@ std::string ArgsManager::GetChainName() const
 {
     auto get_net = [&](const std::string& arg) {
         LOCK(cs_args);
-        util::SettingsValue value = GetSetting(m_settings, /* section= */ "", SettingName(arg),
-                                               /* ignore_default_section_config= */ false,
-                                               /* get_chain_name= */ true);
+        util::SettingsValue value = util::GetSetting(m_settings, /* section= */ "", SettingName(arg),
+            /* ignore_default_section_config= */ false,
+            /* get_chain_name= */ true);
         return value.isNull() ? false : value.isBool() ? value.get_bool() : InterpretBool(value.get_str());
     };
 
@@ -865,6 +845,24 @@ std::string ArgsManager::GetChainName() const
     return GetArg("-chain", CBaseChainParams::MAIN);
 }
 
+bool ArgsManager::UseDefaultSection(const std::string& arg) const
+{
+    return m_network == CBaseChainParams::MAIN || m_network_only_args.count(arg) == 0;
+}
+
+util::SettingsValue ArgsManager::GetSetting(const std::string& arg) const
+{
+    LOCK(cs_args);
+    return util::GetSetting(
+        m_settings, m_network, SettingName(arg), !UseDefaultSection(arg), /* get_chain_name= */ false);
+}
+
+std::vector<util::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const
+{
+    LOCK(cs_args);
+    return util::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg));
+}
+
 bool RenameOver(fs::path src, fs::path dest)
 {
 #ifdef WIN32
diff --git a/src/util/system.h b/src/util/system.h
index 82903b5187..394adb9555 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -19,6 +19,7 @@
 #include <compat/assumptions.h>
 #include <fs.h>
 #include <logging.h>
+#include <optional.h>
 #include <sync.h>
 #include <tinyformat.h>
 #include <util/memory.h>
@@ -132,7 +133,6 @@ class ArgsManager
 {
 public:
     enum Flags {
-        NONE = 0x00,
         // Boolean options can accept negation syntax -noOPTION or -noOPTION=1
         ALLOW_BOOL = 0x01,
         ALLOW_INT = 0x02,
@@ -148,8 +148,6 @@ public:
     };
 
 protected:
-    friend class ArgsManagerHelper;
-
     struct Arg
     {
         std::string m_help_param;
@@ -166,6 +164,27 @@ protected:
 
     NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
 
+    /**
+     * Returns true if settings values from the default section should be used,
+     * depending on the current network and whether the setting is
+     * network-specific.
+     */
+    bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
+
+    /**
+     * Get setting value.
+     *
+     * Result will be null if setting was unset, true if "-setting" argument was passed
+     * false if "-nosetting" argument was passed, and a string if a "-setting=value"
+     * argument was passed.
+     */
+    util::SettingsValue GetSetting(const std::string& arg) const;
+
+    /**
+     * Get list of setting values.
+     */
+    std::vector<util::SettingsValue> GetSettingsList(const std::string& arg) const;
+
 public:
     ArgsManager();
 
@@ -296,9 +315,9 @@ public:
 
     /**
      * Return Flags for known arg.
-     * Return ArgsManager::NONE for unknown arg.
+     * Return nullopt for unknown arg.
      */
-    unsigned int FlagsOfKnownArg(const std::string& key) const;
+    Optional<unsigned int> GetArgFlags(const std::string& name) const;
 };
 
 extern ArgsManager gArgs;