Merge bitcoin/bitcoin#27576: kernel: Remove args, settings, chainparams, chainparamsbase from kernel library

db77f87 scripted-diff: move settings to common namespace (TheCharlatan)
c27e4bd move-only: Move settings to the common library (TheCharlatan)
c2dae5d kernel: Remove chainparams, chainparamsbase, args, settings from kernel library (TheCharlatan)
05870b1 refactor: Remove gArgs access from validation.cpp (TheCharlatan)
8789b11 refactor: Add path argument to FindSnapshotChainstateDir (TheCharlatan)
ef95be3 refactor: Add stop_at_height option in ChainstateManager (TheCharlatan)

Pull request description:

  This pull request is part of the `libbitcoinkernel` project bitcoin/bitcoin#27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel".

  ---

  This completes the removal of the node's chainparams, chainparamsbase, args and settings files and their respective classes from the kernel library. This is the last pull request in a long series working towards decoupling the `ArgsManager` and the `gArgs` global from kernel code. These prior pull requests are: bitcoin/bitcoin#26177 bitcoin/bitcoin#27125 bitcoin/bitcoin#25527 bitcoin/bitcoin#25487 bitcoin/bitcoin#25290

ACKs for top commit:
  MarcoFalke:
    lgtm ACK db77f87 🍄
  hebasto:
    ACK db77f87, I have reviewed the code and it looks OK.
  ryanofsky:
    Code review ACK db77f87. Looks great!

Tree-SHA512: cbfbd705d056f2f10f16810d4f869eb152362fff2c5ddae5e1ac6785deae095588e52ad48b29d921962b085e51de1e0ecab6e50f46149ffe3c16250608a2c93a

Author: ryanofsky

src/Makefile.am

@@ -143,6 +143,7 @@ BITCOIN_CORE_H = \
   compat/compat.h \
   compat/cpuid.h \
   compat/endian.h \
+  common/settings.h \
   common/system.h \
   compressor.h \
   consensus/consensus.h \
@@ -309,7 +310,6 @@ BITCOIN_CORE_H = \
   util/readwritefile.h \
   util/result.h \
   util/serfloat.h \
-  util/settings.h \
   util/sock.h \
   util/spanparsing.h \
   util/string.h \
@@ -663,6 +663,7 @@ libbitcoin_common_a_SOURCES = \
   common/init.cpp \
   common/interfaces.cpp \
   common/run_command.cpp \
+  common/settings.cpp \
   common/system.cpp \
   compressor.cpp \
   core_read.cpp \
@@ -733,7 +734,6 @@ libbitcoin_util_a_SOURCES = \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/readwritefile.cpp \
-  util/settings.cpp \
   util/thread.cpp \
   util/threadinterrupt.cpp \
   util/threadnames.cpp \
@@ -912,12 +912,8 @@ libbitcoinkernel_la_SOURCES = \
   kernel/bitcoinkernel.cpp \
   arith_uint256.cpp \
   chain.cpp \
-  chainparamsbase.cpp \
-  chainparams.cpp \
   clientversion.cpp \
   coins.cpp \
-  common/args.cpp \
-  common/config.cpp \
   compressor.cpp \
   consensus/merkle.cpp \
   consensus/tx_check.cpp \
@@ -978,7 +974,6 @@ libbitcoinkernel_la_SOURCES = \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/serfloat.cpp \
-  util/settings.cpp \
   util/strencodings.cpp \
   util/string.cpp \
   util/syscall_sandbox.cpp \

src/addrdb.cpp

@@ -9,6 +9,7 @@
 #include <chainparams.h>
 #include <clientversion.h>
 #include <common/args.h>
+#include <common/settings.h>
 #include <cstdint>
 #include <hash.h>
 #include <logging.h>
@@ -21,7 +22,6 @@
 #include <univalue.h>
 #include <util/fs.h>
 #include <util/fs_helpers.h>
-#include <util/settings.h>
 #include <util/translation.h>
 
 namespace {
@@ -132,7 +132,7 @@ CBanDB::CBanDB(fs::path ban_list_path)
 bool CBanDB::Write(const banmap_t& banSet)
 {
     std::vector<std::string> errors;
-    if (util::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) {
+    if (common::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) {
         return true;
     }
 
@@ -152,10 +152,10 @@ bool CBanDB::Read(banmap_t& banSet)
         return false;
     }
 
-    std::map<std::string, util::SettingsValue> settings;
+    std::map<std::string, common::SettingsValue> settings;
     std::vector<std::string> errors;
 
-    if (!util::ReadSettings(m_banlist_json, settings, errors)) {
+    if (!common::ReadSettings(m_banlist_json, settings, errors)) {
         for (const auto& err : errors) {
             LogPrintf("Cannot load banlist %s: %s\n", fs::PathToString(m_banlist_json), err);
         }

src/bitcoin-chainstate.cpp

@@ -17,8 +17,6 @@
 #include <kernel/context.h>
 #include <kernel/validation_cache_sizes.h>
 
-#include <chainparams.h>
-#include <common/args.h>
 #include <consensus/validation.h>
 #include <core_io.h>
 #include <node/blockstorage.h>
@@ -53,13 +51,9 @@ int main(int argc, char* argv[])
     }
     std::filesystem::path abs_datadir = std::filesystem::absolute(argv[1]);
     std::filesystem::create_directories(abs_datadir);
-    gArgs.ForceSetArg("-datadir", abs_datadir.string());
 
 
-    // SETUP: Misc Globals
-    SelectParams(ChainType::MAIN);
-    auto chainparams = CChainParams::Main();
-
+    // SETUP: Context
     kernel::Context kernel_context{};
     // We can't use a goto here, but we can use an assert since none of the
     // things instantiated so far requires running the epilogue to be torn down
@@ -106,16 +100,18 @@ int main(int argc, char* argv[])
     };
     auto notifications = std::make_unique<KernelNotifications>();
 
+
     // SETUP: Chainstate
+    auto chainparams = CChainParams::Main();
     const ChainstateManager::Options chainman_opts{
         .chainparams = *chainparams,
-        .datadir = gArgs.GetDataDirNet(),
+        .datadir = abs_datadir,
         .adjusted_time_callback = NodeClock::now,
         .notifications = *notifications,
     };
     const node::BlockManager::Options blockman_opts{
         .chainparams = chainman_opts.chainparams,
-        .blocks_dir = gArgs.GetBlocksDirPath(),
+        .blocks_dir = abs_datadir / "blocks",
     };
     ChainstateManager chainman{chainman_opts, blockman_opts};
 
@@ -148,7 +144,8 @@ int main(int argc, char* argv[])
     // Main program logic starts here
     std::cout
         << "Hello! I'm going to print out some information about your datadir." << std::endl
-        << "\t" << "Path: " << gArgs.GetDataDirNet() << std::endl;
+        << "\t"
+        << "Path: " << abs_datadir << std::endl;
     {
         LOCK(chainman.GetMutex());
         std::cout

src/common/args.cpp

@@ -6,6 +6,7 @@
 #include <common/args.h>
 
 #include <chainparamsbase.h>
+#include <common/settings.h>
 #include <logging.h>
 #include <sync.h>
 #include <tinyformat.h>
@@ -14,7 +15,6 @@
 #include <util/check.h>
 #include <util/fs.h>
 #include <util/fs_helpers.h>
-#include <util/settings.h>
 #include <util/strencodings.h>
 
 #ifdef WIN32
@@ -104,7 +104,7 @@ KeyInfo InterpretKey(std::string key)
  * @return parsed settings value if it is valid, otherwise nullopt accompanied
  * by a descriptive error string
  */
-std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value,
+std::optional<common::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value,
                                                   unsigned int flags, std::string& error)
 {
     // Return negated settings as false values.
@@ -238,15 +238,15 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
             return false;
         }
 
-        std::optional<util::SettingsValue> value = InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error);
+        std::optional<common::SettingsValue> value = InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error);
         if (!value) return false;
 
         m_settings.command_line_options[keyinfo.name].push_back(*value);
     }
 
     // we do not allow -includeconf from command line, only -noincludeconf
-    if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) {
-        const util::SettingsSpan values{*includes};
+    if (auto* includes = common::FindKey(m_settings.command_line_options, "includeconf")) {
+        const common::SettingsSpan values{*includes};
         // Range may be empty if -noincludeconf was passed
         if (!values.empty()) {
             error = "-includeconf cannot be used from commandline; -includeconf=" + values.begin()->write();
@@ -361,7 +361,7 @@ std::optional<const ArgsManager::Command> ArgsManager::GetCommand() const
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 {
     std::vector<std::string> result;
-    for (const util::SettingsValue& value : GetSettingsList(strArg)) {
+    for (const common::SettingsValue& value : GetSettingsList(strArg)) {
         result.push_back(value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str());
     }
     return result;
@@ -408,7 +408,7 @@ bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors)
     LOCK(cs_args);
     m_settings.rw_settings.clear();
     std::vector<std::string> read_errors;
-    if (!util::ReadSettings(path, m_settings.rw_settings, read_errors)) {
+    if (!common::ReadSettings(path, m_settings.rw_settings, read_errors)) {
         SaveErrors(read_errors, errors);
         return false;
     }
@@ -430,7 +430,7 @@ bool ArgsManager::WriteSettingsFile(std::vector<std::string>* errors, bool backu
 
     LOCK(cs_args);
     std::vector<std::string> write_errors;
-    if (!util::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) {
+    if (!common::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) {
         SaveErrors(write_errors, errors);
         return false;
     }
@@ -441,10 +441,10 @@ bool ArgsManager::WriteSettingsFile(std::vector<std::string>* errors, bool backu
     return true;
 }
 
-util::SettingsValue ArgsManager::GetPersistentSetting(const std::string& name) const
+common::SettingsValue ArgsManager::GetPersistentSetting(const std::string& name) const
 {
     LOCK(cs_args);
-    return util::GetSetting(m_settings, m_network, name, !UseDefaultSection("-" + name),
+    return common::GetSetting(m_settings, m_network, name, !UseDefaultSection("-" + name),
         /*ignore_nonpersistent=*/true, /*get_chain_type=*/false);
 }
 
@@ -460,11 +460,11 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st
 
 std::optional<std::string> ArgsManager::GetArg(const std::string& strArg) const
 {
-    const util::SettingsValue value = GetSetting(strArg);
+    const common::SettingsValue value = GetSetting(strArg);
     return SettingToString(value);
 }
 
-std::optional<std::string> SettingToString(const util::SettingsValue& value)
+std::optional<std::string> SettingToString(const common::SettingsValue& value)
 {
     if (value.isNull()) return std::nullopt;
     if (value.isFalse()) return "0";
@@ -473,7 +473,7 @@ std::optional<std::string> SettingToString(const util::SettingsValue& value)
     return value.get_str();
 }
 
-std::string SettingToString(const util::SettingsValue& value, const std::string& strDefault)
+std::string SettingToString(const common::SettingsValue& value, const std::string& strDefault)
 {
     return SettingToString(value).value_or(strDefault);
 }
@@ -485,11 +485,11 @@ int64_t ArgsManager::GetIntArg(const std::string& strArg, int64_t nDefault) cons
 
 std::optional<int64_t> ArgsManager::GetIntArg(const std::string& strArg) const
 {
-    const util::SettingsValue value = GetSetting(strArg);
+    const common::SettingsValue value = GetSetting(strArg);
     return SettingToInt(value);
 }
 
-std::optional<int64_t> SettingToInt(const util::SettingsValue& value)
+std::optional<int64_t> SettingToInt(const common::SettingsValue& value)
 {
     if (value.isNull()) return std::nullopt;
     if (value.isFalse()) return 0;
@@ -498,7 +498,7 @@ std::optional<int64_t> SettingToInt(const util::SettingsValue& value)
     return LocaleIndependentAtoi<int64_t>(value.get_str());
 }
 
-int64_t SettingToInt(const util::SettingsValue& value, int64_t nDefault)
+int64_t SettingToInt(const common::SettingsValue& value, int64_t nDefault)
 {
     return SettingToInt(value).value_or(nDefault);
 }
@@ -510,18 +510,18 @@ bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 
 std::optional<bool> ArgsManager::GetBoolArg(const std::string& strArg) const
 {
-    const util::SettingsValue value = GetSetting(strArg);
+    const common::SettingsValue value = GetSetting(strArg);
     return SettingToBool(value);
 }
 
-std::optional<bool> SettingToBool(const util::SettingsValue& value)
+std::optional<bool> SettingToBool(const common::SettingsValue& value)
 {
     if (value.isNull()) return std::nullopt;
     if (value.isBool()) return value.get_bool();
     return InterpretBool(value.get_str());
 }
 
-bool SettingToBool(const util::SettingsValue& value, bool fDefault)
+bool SettingToBool(const common::SettingsValue& value, bool fDefault)
 {
     return SettingToBool(value).value_or(fDefault);
 }
@@ -738,7 +738,7 @@ std::variant<ChainType, std::string> ArgsManager::GetChainArg() const
 {
     auto get_net = [&](const std::string& arg) {
         LOCK(cs_args);
-        util::SettingsValue value = util::GetSetting(m_settings, /* section= */ "", SettingName(arg),
+        common::SettingsValue value = common::GetSetting(m_settings, /* section= */ "", SettingName(arg),
             /* ignore_default_section_config= */ false,
             /*ignore_nonpersistent=*/false,
             /* get_chain_type= */ true);
@@ -769,24 +769,24 @@ bool ArgsManager::UseDefaultSection(const std::string& arg) const
     return m_network == ChainTypeToString(ChainType::MAIN) || m_network_only_args.count(arg) == 0;
 }
 
-util::SettingsValue ArgsManager::GetSetting(const std::string& arg) const
+common::SettingsValue ArgsManager::GetSetting(const std::string& arg) const
 {
     LOCK(cs_args);
-    return util::GetSetting(
+    return common::GetSetting(
         m_settings, m_network, SettingName(arg), !UseDefaultSection(arg),
         /*ignore_nonpersistent=*/false, /*get_chain_type=*/false);
 }
 
-std::vector<util::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const
+std::vector<common::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const
 {
     LOCK(cs_args);
-    return util::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg));
+    return common::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg));
 }
 
 void ArgsManager::logArgsPrefix(
     const std::string& prefix,
     const std::string& section,
-    const std::map<std::string, std::vector<util::SettingsValue>>& args) const
+    const std::map<std::string, std::vector<common::SettingsValue>>& args) const
 {
     std::string section_str = section.empty() ? "" : "[" + section + "] ";
     for (const auto& arg : args) {