Merge bitcoin/bitcoin#23732: refactor: Remove `gArgs` from `bdb.h` and `sqlite.h`

39b1763730177cd7d6a32fd9321da640b0d65e0e Replace use of `ArgsManager` with `DatabaseOptions` (Kiminuo)

Pull request description:

  Contributes to #21005.

  The goal of this PR is to remove `gArgs` from database classes (i.e. `bdb.h` and `sqlite.h`) so that they can be tested without relying on `gArgs` in tests.

  Notes:

  * My goal is to enable unit-testing without relying on `gArgs` as much as possible. Global variables are hard to reason about which in turn makes it slightly harder to contribute to this codebase. When the compiler does the heavy lifting for us and allows us only to construct an object (or call a method) with valid parameters, we may also save some time in code reviews. The cost for this is passing an argument which is not for free but the cost is very miniscule compared to benefits, I think.
      * GUI code is an exception because it seems fine to have `gArgs` there so I don't plan to make changes in `src/qt` folder, for example.
  * My approach to removal of `gArgs` uses is moving from lower levels to upper ones and pass `ArgsManager` as an argument as needed. The approach is very similar to what #20158.

ACKs for top commit:
  achow101:
    ACK 39b1763730177cd7d6a32fd9321da640b0d65e0e
  ryanofsky:
    Code review ACK 39b1763730177cd7d6a32fd9321da640b0d65e0e. Just the two small ReadDatabaseArgs and Berkeley open changes that were discussed since the last review

Tree-SHA512: aa066b314db593e46c18698fe8cdd500f558b405dc04e4a9a3ff57b52b5b3a81a6cb090e0e661785d1d02c1bf18958c1f4cd715ff233aab63381e3f80960622d

4 files changed, 21 insertions, 24 deletions

diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp
index 35ae3707f8..fbf1e0efd3 100644
--- a/src/wallet/test/db_tests.cpp
+++ b/src/wallet/test/db_tests.cpp
@@ -19,13 +19,13 @@ static std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& path, s
 {
     fs::path data_file = BDBDataFile(path);
     database_filename = fs::PathToString(data_file.filename());
-    return GetBerkeleyEnv(data_file.parent_path());
+    return GetBerkeleyEnv(data_file.parent_path(), false);
 }
 
 BOOST_AUTO_TEST_CASE(getwalletenv_file)
 {
     std::string test_name = "test_name.dat";
-    const fs::path datadir = gArgs.GetDataDirNet();
+    const fs::path datadir = m_args.GetDataDirNet();
     fs::path file_path = datadir / test_name;
     std::ofstream f{file_path};
     f.close();
@@ -39,7 +39,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file)
 BOOST_AUTO_TEST_CASE(getwalletenv_directory)
 {
     std::string expected_name = "wallet.dat";
-    const fs::path datadir = gArgs.GetDataDirNet();
+    const fs::path datadir = m_args.GetDataDirNet();
 
     std::string filename;
     std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(datadir, filename);
@@ -49,8 +49,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_directory)
 
 BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple)
 {
-    fs::path datadir = gArgs.GetDataDirNet() / "1";
-    fs::path datadir_2 = gArgs.GetDataDirNet() / "2";
+    fs::path datadir = m_args.GetDataDirNet() / "1";
+    fs::path datadir_2 = m_args.GetDataDirNet() / "2";
     std::string filename;
 
     std::shared_ptr<BerkeleyEnvironment> env_1 = GetWalletEnv(datadir, filename);
diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp
index be38cebafd..34c22a9c0d 100644
--- a/src/wallet/test/init_test_fixture.cpp
+++ b/src/wallet/test/init_test_fixture.cpp
@@ -15,12 +15,12 @@
 namespace wallet {
 InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName)
 {
-    m_wallet_loader = MakeWalletLoader(*m_node.chain, *Assert(m_node.args));
+    m_wallet_loader = MakeWalletLoader(*m_node.chain, m_args);
 
     std::string sep;
     sep += fs::path::preferred_separator;
 
-    m_datadir = gArgs.GetDataDirNet();
+    m_datadir = m_args.GetDataDirNet();
     m_cwd = fs::current_path();
 
     m_walletdir_path_cases["default"] = m_datadir / "wallets";
@@ -42,14 +42,11 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam
 
 InitWalletDirTestingSetup::~InitWalletDirTestingSetup()
 {
-    gArgs.LockSettings([&](util::Settings& settings) {
-        settings.forced_settings.erase("walletdir");
-    });
     fs::current_path(m_cwd);
 }
 
 void InitWalletDirTestingSetup::SetWalletDir(const fs::path& walletdir_path)
 {
-    gArgs.ForceSetArg("-walletdir", fs::PathToString(walletdir_path));
+    m_args.ForceSetArg("-walletdir", fs::PathToString(walletdir_path));
 }
 } // namespace wallet
diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp
index 7fdecc5642..fb0a07e181 100644
--- a/src/wallet/test/init_tests.cpp
+++ b/src/wallet/test/init_tests.cpp
@@ -18,7 +18,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
     SetWalletDir(m_walletdir_path_cases["default"]);
     bool result = m_wallet_loader->verify();
     BOOST_CHECK(result == true);
-    fs::path walletdir = gArgs.GetPathArg("-walletdir");
+    fs::path walletdir = m_args.GetPathArg("-walletdir");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
     BOOST_CHECK_EQUAL(walletdir, expected_path);
 }
@@ -28,7 +28,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom)
     SetWalletDir(m_walletdir_path_cases["custom"]);
     bool result = m_wallet_loader->verify();
     BOOST_CHECK(result == true);
-    fs::path walletdir = gArgs.GetPathArg("-walletdir");
+    fs::path walletdir = m_args.GetPathArg("-walletdir");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]);
     BOOST_CHECK_EQUAL(walletdir, expected_path);
 }
@@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
     SetWalletDir(m_walletdir_path_cases["trailing"]);
     bool result = m_wallet_loader->verify();
     BOOST_CHECK(result == true);
-    fs::path walletdir = gArgs.GetPathArg("-walletdir");
+    fs::path walletdir = m_args.GetPathArg("-walletdir");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
     BOOST_CHECK_EQUAL(walletdir, expected_path);
 }
@@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
     SetWalletDir(m_walletdir_path_cases["trailing2"]);
     bool result = m_wallet_loader->verify();
     BOOST_CHECK(result == true);
-    fs::path walletdir = gArgs.GetPathArg("-walletdir");
+    fs::path walletdir = m_args.GetPathArg("-walletdir");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
     BOOST_CHECK_EQUAL(walletdir, expected_path);
 }
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index c59f7e6f05..7d2b769ae6 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -221,7 +221,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
         wallet->SetupLegacyScriptPubKeyMan();
         WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));
         WalletContext context;
-        context.args = &gArgs;
+        context.args = &m_args;
         AddWallet(context, wallet);
         UniValue keys;
         keys.setArray();
@@ -277,12 +277,12 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
     SetMockTime(KEY_TIME);
     m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
 
-    std::string backup_file = fs::PathToString(gArgs.GetDataDirNet() / "wallet.backup");
+    std::string backup_file = fs::PathToString(m_args.GetDataDirNet() / "wallet.backup");
 
     // Import key into wallet and call dumpwallet to create backup file.
     {
         WalletContext context;
-        context.args = &gArgs;
+        context.args = &m_args;
         const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase());
         {
             auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
@@ -310,7 +310,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
         wallet->SetupLegacyScriptPubKeyMan();
 
         WalletContext context;
-        context.args = &gArgs;
+        context.args = &m_args;
         JSONRPCRequest request;
         request.context = &context;
         request.params.setArray();
@@ -716,10 +716,10 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
 //! rescanning where new transactions in new blocks could be lost.
 BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
 {
-    gArgs.ForceSetArg("-unsafesqlitesync", "1");
+    m_args.ForceSetArg("-unsafesqlitesync", "1");
     // Create new wallet with known key and unload it.
     WalletContext context;
-    context.args = &gArgs;
+    context.args = &m_args;
     context.chain = m_node.chain.get();
     auto wallet = TestLoadWallet(context);
     CKey key;
@@ -812,7 +812,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
 BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup)
 {
     WalletContext context;
-    context.args = &gArgs;
+    context.args = &m_args;
     auto wallet = TestLoadWallet(context);
     BOOST_CHECK(wallet);
     UnloadWallet(std::move(wallet));
@@ -820,9 +820,9 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup)
 
 BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
 {
-    gArgs.ForceSetArg("-unsafesqlitesync", "1");
+    m_args.ForceSetArg("-unsafesqlitesync", "1");
     WalletContext context;
-    context.args = &gArgs;
+    context.args = &m_args;
     context.chain = m_node.chain.get();
     auto wallet = TestLoadWallet(context);
     CKey key;