wallet: decouple bdb dependency from wallet migration benchmark

furszy · Nov 7, 2024 · 60cc977 · 60cc977
1 parent 2c90f8e
commit 60cc977
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 16 deletions.
diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp
@@ -16,7 +16,7 @@
 
 #include <optional>
 
-#if defined(USE_BDB) && defined(USE_SQLITE) // only enable benchmark when bdb and sqlite are enabled
+#if defined(USE_SQLITE) // only enable benchmark when sqlite is enabled
 
 namespace wallet{
 
@@ -32,14 +32,8 @@ static void WalletMigration(benchmark::Bench& bench)
     int NUM_WATCH_ONLY_ADDR = 20;
 
     // Setup legacy wallet
-    DatabaseOptions options;
-    options.use_unsafe_sync = true;
-    options.verify = false;
-    DatabaseStatus status;
-    bilingual_str error;
-    auto database = MakeWalletDatabase(fs::PathToString(test_setup->m_path_root / "legacy"), options, status, error);
     uint64_t create_flags = 0;
-    auto wallet = TestLoadWallet(std::move(database), context, create_flags);
+    auto wallet = TestLoadWallet(CreateMockableWalletDatabase(), context, create_flags);
 
     // Add watch-only addresses
     std::vector<CScript> scripts_watch_only;
@@ -62,11 +56,14 @@ static void WalletMigration(benchmark::Bench& bench)
         wallet->AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, /*rescanning_old_block=*/true);
     }
 
-    // Unload so the migration process loads it
-    TestUnloadWallet(std::move(wallet));
+    // As we don't support legacy wallets anymore, remove wallet from context to make it look like it was externally loaded.
+    RemoveWallet(context, wallet, false);
 
-    bench.epochs(/*numEpochs=*/1).run([&] {
-        util::Result<MigrationResult> res = MigrateLegacyToDescriptor(fs::PathToString(test_setup->m_path_root / "legacy"), "", context);
+    bench.epochs(/*numEpochs=*/1).run([&context, &wallet] {
+        util::Result<MigrationResult> res = MigrateLegacyToDescriptor(std::move(wallet),
+                                                                      /*passphrase=*/"",
+                                                                      context,
+                                                                      /*was_loaded=*/false);
         assert(res);
         assert(res->wallet);
         assert(res->watchonly_wallet);

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -4380,9 +4380,8 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
 
 util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context)
 {
-    MigrationResult res;
-    bilingual_str error;
     std::vector<bilingual_str> warnings;
+    bilingual_str error;
 
     // If the wallet is still loaded, unload it so that nothing else tries to use it while we're changing it
     bool was_loaded = false;
@@ -4431,10 +4430,23 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error};
     }
 
+    return MigrateLegacyToDescriptor(std::move(local_wallet), passphrase, context, was_loaded);
+}
+
+util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded)
+{
+    MigrationResult res;
+    bilingual_str error;
+    std::vector<bilingual_str> warnings;
+
+    DatabaseOptions options;
+    options.require_existing = true;
+    DatabaseStatus status;
+
+    const std::string& wallet_name = local_wallet->GetName();
+
     // Helper to reload as normal for some of our exit scenarios
     const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) {
-        // Reset options.require_format as wallets of any format may be reloaded.
-        options.require_format = std::nullopt;
         assert(to_reload.use_count() == 1);
         std::string name = to_reload->GetName();
         to_reload.reset();

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -1134,6 +1134,8 @@ struct MigrationResult {
 
 //! Do all steps to migrate a legacy wallet to a descriptor wallet
 [[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context);
+//! Requirement: The wallet provided to this function must be isolated with no attachement to the node's context.
+[[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_WALLET_H