Merge bitcoin/bitcoin#31241: wallet: remove BDB dependency from wallet migration benchmark

18619b473255786b80f27dd33b46eb26a63fffc7 wallet: remove BDB dependency from wallet migration benchmark (furszy)

Pull request description:

  Part of the legacy wallet removal working path #20160.

  Stops creating a bdb database in the wallet migration benchmark.
  Instead, the benchmark now creates the db in memory and re-uses it for the migration process.

ACKs for top commit:
  achow101:
    ACK 18619b473255786b80f27dd33b46eb26a63fffc7
  brunoerg:
    code review ACK 18619b473255786b80f27dd33b46eb26a63fffc7
  theStack:
    Code-review ACK 18619b473255786b80f27dd33b46eb26a63fffc7

Tree-SHA512: a107deee3d2c00b980e3606be07d038ca524b98251442956d702a7996e2ac5e2901f656482018cacbac8ef6a628ac1fb03f677d1658aeaded4036d834a95d7e0

src/bench/wallet_migration.cpp

@@ -5,6 +5,7 @@
 #include <bitcoin-build-config.h> // IWYU pragma: keep
 
 #include <bench/bench.h>
+#include <kernel/chain.h>
 #include <interfaces/chain.h>
 #include <node/context.h>
 #include <test/util/mining.h>
@@ -16,7 +17,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,41 +33,39 @@ 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);
+    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase());
+    wallet->chainStateFlushed(ChainstateRole::NORMAL, CBlockLocator{});
+    LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM();
 
     // Add watch-only addresses
     std::vector<CScript> scripts_watch_only;
     for (int w = 0; w < NUM_WATCH_ONLY_ADDR; ++w) {
         CKey key = GenerateRandomKey();
         LOCK(wallet->cs_wallet);
-        const CScript& script = scripts_watch_only.emplace_back(GetScriptForDestination(GetDestinationForKey(key.GetPubKey(), OutputType::LEGACY)));
-        bool res = wallet->ImportScriptPubKeys(strprintf("watch_%d", w), {script},
-                                    /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
-        assert(res);
+        const auto& dest = GetDestinationForKey(key.GetPubKey(), OutputType::LEGACY);
+        const CScript& script = scripts_watch_only.emplace_back(GetScriptForDestination(dest));
+        assert(legacy_spkm->LoadWatchOnly(script));
+        assert(wallet->SetAddressBook(dest, strprintf("watch_%d", w), /*purpose=*/std::nullopt));
     }
 
     // Generate transactions and local addresses
-    for (int j = 0; j < 400; ++j) {
+    for (int j = 0; j < 500; ++j) {
+        CKey key = GenerateRandomKey();
+        CPubKey pubkey = key.GetPubKey();
+        // Load key, scripts and create address book record
+        Assert(legacy_spkm->LoadKey(key, pubkey));
+        CTxDestination dest{PKHash(pubkey)};
+        Assert(wallet->SetAddressBook(dest, strprintf("legacy_%d", j), /*purpose=*/std::nullopt));
+
         CMutableTransaction mtx;
-        mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, strprintf("bench_%d", j)))));
-        mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::LEGACY, strprintf("legacy_%d", j)))));
+        mtx.vout.emplace_back(COIN, GetScriptForDestination(dest));
         mtx.vout.emplace_back(COIN, scripts_watch_only.at(j % NUM_WATCH_ONLY_ADDR));
         mtx.vin.resize(2);
         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));
-
-    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);

src/wallet/wallet.cpp

@@ -4398,9 +4398,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;
@@ -4452,10 +4451,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();

src/wallet/wallet.h

@@ -1135,6 +1135,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 attachment 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