Replace use of ArgsManager with DatabaseOptions

Co-authored-by: Russell Yanofsky <[email protected]>

Author: kiminuo

src/wallet/bdb.cpp

@@ -60,12 +60,12 @@ bool WalletDatabaseFileId::operator==(const WalletDatabaseFileId& rhs) const
  * erases the weak pointer from the g_dbenvs map.
  * @post A new BerkeleyEnvironment weak pointer is inserted into g_dbenvs if the directory path key was not already in the map.
  */
-std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory)
+std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory, bool use_shared_memory)
 {
     LOCK(cs_db);
     auto inserted = g_dbenvs.emplace(fs::PathToString(env_directory), std::weak_ptr<BerkeleyEnvironment>());
     if (inserted.second) {
-        auto env = std::make_shared<BerkeleyEnvironment>(env_directory);
+        auto env = std::make_shared<BerkeleyEnvironment>(env_directory, use_shared_memory);
         inserted.first->second = env;
         return env;
     }
@@ -113,7 +113,7 @@ void BerkeleyEnvironment::Reset()
     fMockDb = false;
 }
 
-BerkeleyEnvironment::BerkeleyEnvironment(const fs::path& dir_path) : strPath(fs::PathToString(dir_path))
+BerkeleyEnvironment::BerkeleyEnvironment(const fs::path& dir_path, bool use_shared_memory) : strPath(fs::PathToString(dir_path)), m_use_shared_memory(use_shared_memory)
 {
     Reset();
 }
@@ -145,8 +145,9 @@ bool BerkeleyEnvironment::Open(bilingual_str& err)
     LogPrintf("BerkeleyEnvironment::Open: LogDir=%s ErrorFile=%s\n", fs::PathToString(pathLogDir), fs::PathToString(pathErrorFile));
 
     unsigned int nEnvFlags = 0;
-    if (gArgs.GetBoolArg("-privdb", DEFAULT_WALLET_PRIVDB))
+    if (!m_use_shared_memory) {
         nEnvFlags |= DB_PRIVATE;
+    }
 
     dbenv->set_lg_dir(fs::PathToString(pathLogDir).c_str());
     dbenv->set_cachesize(0, 0x100000, 1); // 1 MiB should be enough for just the wallet
@@ -188,7 +189,7 @@ bool BerkeleyEnvironment::Open(bilingual_str& err)
 }
 
 //! Construct an in-memory mock Berkeley environment for testing
-BerkeleyEnvironment::BerkeleyEnvironment()
+BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false)
 {
     Reset();
 
@@ -377,7 +378,7 @@ void BerkeleyBatch::Flush()
         nMinutes = 1;
 
     if (env) { // env is nullptr for dummy databases (i.e. in tests). Don't actually flush if env is nullptr so we don't segfault
-        env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetIntArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
+        env->dbenv->txn_checkpoint(nMinutes ? m_database.m_max_log_mb * 1024 : 0, nMinutes, 0);
     }
 }
 
@@ -831,13 +832,13 @@ std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, con
     {
         LOCK(cs_db); // Lock env.m_databases until insert in BerkeleyDatabase constructor
         std::string data_filename = fs::PathToString(data_file.filename());
-        std::shared_ptr<BerkeleyEnvironment> env = GetBerkeleyEnv(data_file.parent_path());
+        std::shared_ptr<BerkeleyEnvironment> env = GetBerkeleyEnv(data_file.parent_path(), options.use_shared_memory);
         if (env->m_databases.count(data_filename)) {
             error = Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", fs::PathToString(env->Directory() / data_filename)));
             status = DatabaseStatus::FAILED_ALREADY_LOADED;
             return nullptr;
         }
-        db = std::make_unique<BerkeleyDatabase>(std::move(env), std::move(data_filename));
+        db = std::make_unique<BerkeleyDatabase>(std::move(env), std::move(data_filename), options);
     }
 
     if (options.verify && !db->Verify(error)) {

src/wallet/bdb.h

@@ -32,8 +32,6 @@
 struct bilingual_str;
 
 namespace wallet {
-static const unsigned int DEFAULT_WALLET_DBLOGSIZE = 100;
-static const bool DEFAULT_WALLET_PRIVDB = true;
 
 struct WalletDatabaseFileId {
     u_int8_t value[DB_FILE_ID_LEN];
@@ -56,8 +54,9 @@ class BerkeleyEnvironment
     std::map<std::string, std::reference_wrapper<BerkeleyDatabase>> m_databases;
     std::unordered_map<std::string, WalletDatabaseFileId> m_fileids;
     std::condition_variable_any m_db_in_use;
+    bool m_use_shared_memory;
 
-    explicit BerkeleyEnvironment(const fs::path& env_directory);
+    explicit BerkeleyEnvironment(const fs::path& env_directory, bool use_shared_memory);
     BerkeleyEnvironment();
     ~BerkeleyEnvironment();
     void Reset();
@@ -85,7 +84,7 @@ class BerkeleyEnvironment
 };
 
 /** Get BerkeleyEnvironment given a directory path. */
-std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory);
+std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory, bool use_shared_memory);
 
 class BerkeleyBatch;
 
@@ -98,8 +97,8 @@ class BerkeleyDatabase : public WalletDatabase
     BerkeleyDatabase() = delete;
 
     /** Create DB handle to real database */
-    BerkeleyDatabase(std::shared_ptr<BerkeleyEnvironment> env, std::string filename) :
-        WalletDatabase(), env(std::move(env)), strFile(std::move(filename))
+    BerkeleyDatabase(std::shared_ptr<BerkeleyEnvironment> env, std::string filename, const DatabaseOptions& options) :
+        WalletDatabase(), env(std::move(env)), strFile(std::move(filename)), m_max_log_mb(options.max_log_mb)
     {
         auto inserted = this->env->m_databases.emplace(strFile, std::ref(*this));
         assert(inserted.second);
@@ -160,6 +159,7 @@ class BerkeleyDatabase : public WalletDatabase
     std::unique_ptr<Db> m_db;
 
     std::string strFile;
+    int64_t m_max_log_mb;
 
     /** Make a BerkeleyBatch connected to this database */
     std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override;

src/wallet/db.cpp

@@ -6,6 +6,7 @@
 #include <chainparams.h>
 #include <fs.h>
 #include <logging.h>
+#include <util/system.h>
 #include <wallet/db.h>
 
 #include <exception>
@@ -137,4 +138,13 @@ bool IsSQLiteFile(const fs::path& path)
     // Check the application id matches our network magic
     return memcmp(Params().MessageStart(), app_id, 4) == 0;
 }
+
+void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options)
+{
+    // Override current options with args values, if any were specified
+    options.use_unsafe_sync = args.GetBoolArg("-unsafesqlitesync", options.use_unsafe_sync);
+    options.use_shared_memory = !args.GetBoolArg("-privdb", !options.use_shared_memory);
+    options.max_log_mb = args.GetIntArg("-dblogsize", options.max_log_mb);
+}
+
 } // namespace wallet

src/wallet/db.h

@@ -16,6 +16,7 @@
 #include <optional>
 #include <string>
 
+class ArgsManager;
 struct bilingual_str;
 
 namespace wallet {
@@ -207,7 +208,12 @@ struct DatabaseOptions {
     std::optional<DatabaseFormat> require_format;
     uint64_t create_flags = 0;
     SecureString create_passphrase;
-    bool verify = true;
+
+    // Specialized options. Not every option is supported by every backend.
+    bool verify = true;             //!< Check data integrity on load.
+    bool use_unsafe_sync = false;   //!< Disable file sync for faster performance.
+    bool use_shared_memory = false; //!< Let other processes access the database.
+    int64_t max_log_mb = 100;       //!< Max log size to allow before consolidating.
 };
 
 enum class DatabaseStatus {
@@ -227,6 +233,7 @@ enum class DatabaseStatus {
 /** Recursively list database paths in directory. */
 std::vector<fs::path> ListDatabases(const fs::path& path);
 
+void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options);
 std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
 
 fs::path BDBDataFile(const fs::path& path);

src/wallet/dump.cpp

@@ -19,10 +19,10 @@ namespace wallet {
 static const std::string DUMP_MAGIC = "BITCOIN_CORE_WALLET_DUMP";
 uint32_t DUMP_VERSION = 1;
 
-bool DumpWallet(CWallet& wallet, bilingual_str& error)
+bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
 {
     // Get the dumpfile
-    std::string dump_filename = gArgs.GetArg("-dumpfile", "");
+    std::string dump_filename = args.GetArg("-dumpfile", "");
     if (dump_filename.empty()) {
         error = _("No dump file provided. To use dump, -dumpfile=<filename> must be provided.");
         return false;
@@ -114,10 +114,10 @@ static void WalletToolReleaseWallet(CWallet* wallet)
     delete wallet;
 }
 
-bool CreateFromDump(const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
+bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
 {
     // Get the dumpfile
-    std::string dump_filename = gArgs.GetArg("-dumpfile", "");
+    std::string dump_filename = args.GetArg("-dumpfile", "");
     if (dump_filename.empty()) {
         error = _("No dump file provided. To use createfromdump, -dumpfile=<filename> must be provided.");
         return false;
@@ -171,7 +171,7 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling
         return false;
     }
     // Get the data file format with format_value as the default
-    std::string file_format = gArgs.GetArg("-format", format_value);
+    std::string file_format = args.GetArg("-format", format_value);
     if (file_format.empty()) {
         error = _("No wallet file format provided. To use createfromdump, -format=<format> must be provided.");
         return false;
@@ -193,6 +193,7 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling
 
     DatabaseOptions options;
     DatabaseStatus status;
+    ReadDatabaseArgs(args, options);
     options.require_create = true;
     options.require_format = data_format;
     std::unique_ptr<WalletDatabase> database = MakeDatabase(wallet_path, options, status, error);

src/wallet/dump.h

@@ -11,11 +11,12 @@
 #include <vector>
 
 struct bilingual_str;
+class ArgsManager;
 
 namespace wallet {
 class CWallet;
-bool DumpWallet(CWallet& wallet, bilingual_str& error);
-bool CreateFromDump(const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
+bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error);
+bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_DUMP_H

src/wallet/init.cpp

@@ -80,9 +80,9 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 
 #ifdef USE_BDB
-    argsman.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
+    argsman.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DatabaseOptions().max_log_mb), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
     argsman.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
-    argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
+    argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", !DatabaseOptions().use_shared_memory), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
 #else
     argsman.AddHiddenArgs({"-dblogsize", "-flushwallet", "-privdb"});
 #endif

src/wallet/interfaces.cpp

@@ -544,6 +544,7 @@ class WalletLoaderImpl : public WalletLoader
         std::shared_ptr<CWallet> wallet;
         DatabaseOptions options;
         DatabaseStatus status;
+        ReadDatabaseArgs(*m_context.args, options);
         options.require_create = true;
         options.create_flags = wallet_creation_flags;
         options.create_passphrase = passphrase;
@@ -553,6 +554,7 @@ class WalletLoaderImpl : public WalletLoader
     {
         DatabaseOptions options;
         DatabaseStatus status;
+        ReadDatabaseArgs(*m_context.args, options);
         options.require_existing = true;
         return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
     }

src/wallet/load.cpp

@@ -57,6 +57,7 @@ bool VerifyWallets(WalletContext& context)
     if (!args.IsArgSet("wallet")) {
         DatabaseOptions options;
         DatabaseStatus status;
+        ReadDatabaseArgs(args, options);
         bilingual_str error_string;
         options.require_existing = true;
         options.verify = false;
@@ -84,6 +85,7 @@ bool VerifyWallets(WalletContext& context)
 
         DatabaseOptions options;
         DatabaseStatus status;
+        ReadDatabaseArgs(args, options);
         options.require_existing = true;
         options.verify = true;
         bilingual_str error_string;
@@ -112,6 +114,7 @@ bool LoadWallets(WalletContext& context)
             }
             DatabaseOptions options;
             DatabaseStatus status;
+            ReadDatabaseArgs(*context.args, options);
             options.require_existing = true;
             options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets()
             bilingual_str error;

src/wallet/rpc/wallet.cpp

@@ -8,6 +8,7 @@
 #include <rpc/server.h>
 #include <rpc/util.h>
 #include <util/translation.h>
+#include <wallet/context.h>
 #include <wallet/receive.h>
 #include <wallet/rpc/wallet.h>
 #include <wallet/rpc/util.h>
@@ -218,6 +219,7 @@ static RPCHelpMan loadwallet()
 
     DatabaseOptions options;
     DatabaseStatus status;
+    ReadDatabaseArgs(*context.args, options);
     options.require_existing = true;
     bilingual_str error;
     std::vector<bilingual_str> warnings;
@@ -377,6 +379,7 @@ static RPCHelpMan createwallet()
 
     DatabaseOptions options;
     DatabaseStatus status;
+    ReadDatabaseArgs(*context.args, options);
     options.require_create = true;
     options.create_flags = flags;
     options.create_passphrase = passphrase;

src/wallet/salvage.cpp

@@ -23,10 +23,11 @@ static bool KeyFilter(const std::string& type)
     return WalletBatch::IsKeyType(type) || type == DBKeys::HDCHAIN;
 }
 
-bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
+bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
 {
     DatabaseOptions options;
     DatabaseStatus status;
+    ReadDatabaseArgs(args, options);
     options.require_existing = true;
     options.verify = false;
     options.require_format = DatabaseFormat::BERKELEY;

src/wallet/salvage.h

@@ -9,10 +9,11 @@
 #include <fs.h>
 #include <streams.h>
 
+class ArgsManager;
 struct bilingual_str;
 
 namespace wallet {
-bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
+bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_SALVAGE_H

src/wallet/sqlite.cpp

@@ -67,8 +67,8 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va
     }
 }
 
-SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, bool mock)
-    : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path))
+SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock)
+    : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync)
 {
     {
         LOCK(g_sqlite_mutex);
@@ -239,7 +239,7 @@ void SQLiteDatabase::Open()
     // Enable fullfsync for the platforms that use it
     SetPragma(m_db, "fullfsync", "true", "Failed to enable fullfsync");
 
-    if (gArgs.GetBoolArg("-unsafesqlitesync", false)) {
+    if (m_use_unsafe_sync) {
         // Use normal synchronous mode for the journal
         LogPrintf("WARNING SQLite is configured to not wait for data to be flushed to disk. Data loss and corruption may occur.\n");
         SetPragma(m_db, "synchronous", "OFF", "Failed to set synchronous mode to OFF");
@@ -561,7 +561,7 @@ std::unique_ptr<SQLiteDatabase> MakeSQLiteDatabase(const fs::path& path, const D
 {
     try {
         fs::path data_file = SQLiteDataFile(path);
-        auto db = std::make_unique<SQLiteDatabase>(data_file.parent_path(), data_file);
+        auto db = std::make_unique<SQLiteDatabase>(data_file.parent_path(), data_file, options);
         if (options.verify && !db->Verify(error)) {
             status = DatabaseStatus::FAILED_VERIFY;
             return nullptr;

src/wallet/sqlite.h

@@ -69,7 +69,7 @@ class SQLiteDatabase : public WalletDatabase
     SQLiteDatabase() = delete;
 
     /** Create DB handle to real database */
-    SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, bool mock = false);
+    SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock = false);
 
     ~SQLiteDatabase();
 
@@ -113,6 +113,7 @@ class SQLiteDatabase : public WalletDatabase
     std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override;
 
     sqlite3* m_db{nullptr};
+    bool m_use_unsafe_sync;
 };
 
 std::unique_ptr<SQLiteDatabase> MakeSQLiteDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);