Merge bitcoin/bitcoin#21850: Remove `GetDataDir(net_specific)` function

aca0e5dcdb174ef7e88b76910d6fffd633688235 Remove `GetDataDir(bool fNetSpecific = true)` function (Kiminuo)
b3e67f20a02bcf43f20873029ce30617c95eb9da scripted-diff: Replace `GetDataDir(true)` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
4c3a5dcbfc3010965332ad568c3a70618c930ef3 scripted-diff: Replace `GetDataDir()` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
13bd8bb0536f008118b1de921654925ed9ce1da7 Make `ArgsManager.GetDataDirPath` private and drop needless suffix (Kiminuo)
4d8189f62076d47be182e709857774bc3921b334 scripted-diff: Change `ArgsManager.GetDataDirPath()` to `ArgsManager.GetDataDirBase()` in tests (Kiminuo)
0f53df47d5b8319fc71f3b87c15a115ff797fb50 Add `ArgsManager.GetDataDirBase()` and `ArgsManager.GetDataDirNet()` as an intended replacement for `ArgsManager.GetDataDirPath(net_identifier)` (Kiminuo)
716de29dd8672d28bfe0a08424e9958fe61054d2 Make `m_cached_blocks_path` mutable. Make `ArgsManager::GetBlocksDirPath()` const. (Kiminuo)

Pull request description:

  This PR is a follow up PR to #21244. The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by removing `GetDataDir(net_specific)` and replacing it by `gArgs.GetDataDir(net_specific)` calls.

  The approach of this PR attempts to be similar to the one chosen in "De-globalize ChainstateManager" (#20158). The goal is to pass `ArgsManager` to functions (or ideally to have `ArgsManager` as a member of a class where needed; inspiration from here: #21789) instead of having it as a global variable (i.e. `gArgs`).

  **Notes:**
  * First commit makes `m_cached_blocks_path` `mutable` as was suggested [here](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615274095) but not fully applied in #21244. (`m_cached_datadir_path` and `m_cached_network_datadir_path` were marked as `mutable` in #21244) This commit can be in a separate PR too.
  * Other commits deal with removing of `GetDataDir(net_specific)` function.
      * This was originally part of #21244 but it was [left]((https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-633779754)) for a follow up PR.
  * I think that the proposed changes show nicely where there is reliance on `gArgs` which is IMO a good thing.

  If you know about a better approach how to do this, please share it here.

ACKs for top commit:
  hebasto:
    ACK aca0e5dcdb174ef7e88b76910d6fffd633688235
  MarcoFalke:
    re-ACK aca0e5dcdb174ef7e88b76910d6fffd633688235 👃

Tree-SHA512: deec4d88edb32d7f4c818c3a74ffbb64709685819b88242dcf5dbaa1fb611f3ce2b29d2576ddb9e0dc5e75288e43538968224008c0a80e7149fc81c309f7c9da

2 files changed, 26 insertions, 16 deletions

diff --git a/src/util/system.cpp b/src/util/system.cpp
index 76d63074e4..5b87806a45 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -386,7 +386,7 @@ std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) co
     return std::nullopt;
 }
 
-const fs::path& ArgsManager::GetBlocksDirPath()
+const fs::path& ArgsManager::GetBlocksDirPath() const
 {
     LOCK(cs_args);
     fs::path& path = m_cached_blocks_path;
@@ -402,7 +402,7 @@ const fs::path& ArgsManager::GetBlocksDirPath()
             return path;
         }
     } else {
-        path = GetDataDirPath(false);
+        path = GetDataDirBase();
     }
 
     path /= BaseParams().DataDir();
@@ -412,7 +412,7 @@ const fs::path& ArgsManager::GetBlocksDirPath()
     return path;
 }
 
-const fs::path& ArgsManager::GetDataDirPath(bool net_specific) const
+const fs::path& ArgsManager::GetDataDir(bool net_specific) const
 {
     LOCK(cs_args);
     fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path;
@@ -511,7 +511,7 @@ bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp) const
     }
     if (filepath) {
         std::string settings = GetArg("-settings", BITCOIN_SETTINGS_FILENAME);
-        *filepath = fsbridge::AbsPathJoin(GetDataDirPath(/* net_specific= */ true), temp ? settings + ".tmp" : settings);
+        *filepath = fsbridge::AbsPathJoin(GetDataDirNet(), temp ? settings + ".tmp" : settings);
     }
     return true;
 }
@@ -800,11 +800,6 @@ fs::path GetDefaultDataDir()
 #endif
 }
 
-const fs::path &GetDataDir(bool fNetSpecific)
-{
-    return gArgs.GetDataDirPath(fNetSpecific);
-}
-
 bool CheckDataDirOption()
 {
     std::string datadir = gArgs.GetArg("-datadir", "");
@@ -1359,7 +1354,7 @@ fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific)
     if (path.is_absolute()) {
         return path;
     }
-    return fsbridge::AbsPathJoin(GetDataDir(net_specific), path);
+    return fsbridge::AbsPathJoin(net_specific ? gArgs.GetDataDirNet() : gArgs.GetDataDirBase(), path);
 }
 
 void ScheduleBatchPriority()
diff --git a/src/util/system.h b/src/util/system.h
index f68975ffa3..c4317c62d0 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -90,7 +90,6 @@ void ReleaseDirectoryLocks();
 
 bool TryCreateDirectories(const fs::path& p);
 fs::path GetDefaultDataDir();
-const fs::path &GetDataDir(bool fNetSpecific = true);
 // Return true if -datadir option points to a valid directory or is not specified.
 bool CheckDataDirOption();
 fs::path GetConfigFile(const std::string& confPath);
@@ -119,7 +118,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string&
  * the datadir if they are not absolute.
  *
  * @param path The path to be conditionally prefixed with datadir.
- * @param net_specific Forwarded to GetDataDir().
+ * @param net_specific Use network specific datadir variant
  * @return The normalized path.
  */
 fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific = true);
@@ -195,7 +194,7 @@ protected:
     std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
     bool m_accept_any_command GUARDED_BY(cs_args){true};
     std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
-    fs::path m_cached_blocks_path GUARDED_BY(cs_args);
+    mutable fs::path m_cached_blocks_path GUARDED_BY(cs_args);
     mutable fs::path m_cached_datadir_path GUARDED_BY(cs_args);
     mutable fs::path m_cached_network_datadir_path GUARDED_BY(cs_args);
 
@@ -266,16 +265,23 @@ public:
      *
      * @return Blocks path which is network specific
      */
-    const fs::path& GetBlocksDirPath();
+    const fs::path& GetBlocksDirPath() const;
 
     /**
      * Get data directory path
      *
-     * @param net_specific Append network identifier to the returned path
      * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
      * @post Returned directory path is created unless it is empty
      */
-    const fs::path& GetDataDirPath(bool net_specific = true) const;
+    const fs::path& GetDataDirBase() const { return GetDataDir(false); }
+
+    /**
+     * Get data directory path with appended network identifier
+     *
+     * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
+     * @post Returned directory path is created unless it is empty
+     */
+    const fs::path& GetDataDirNet() const { return GetDataDir(true); }
 
     /**
      * Clear cached directory paths
@@ -437,6 +443,15 @@ public:
     void LogArgs() const;
 
 private:
+    /**
+     * Get data directory path
+     *
+     * @param net_specific Append network identifier to the returned path
+     * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
+     * @post Returned directory path is created unless it is empty
+     */
+    const fs::path& GetDataDir(bool net_specific) const;
+
     // Helper function for LogArgs().
     void logArgsPrefix(
         const std::string& prefix,