From 1da11dbc441790773502ffd5f60dc05191514a83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C5=91rinc?= <pap.lorinc@gmail.com>
Date: Sat, 19 Apr 2025 13:35:21 +0200
Subject: [PATCH 1/3] bench: clean up migrated descriptor wallets via loader
 teardown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`MigrateLegacyToDescriptor` returns both a spendable descriptor wallet and a watch‑only wallet.
If these remain attached, their files stay open and on Windows this can hang CI when removing the test directory.

By constructing them via `MakeWalletLoader` (which owns the `WalletContext`), both wallets are automatically unloaded when the loader is destroyed at the end.
This ensures no lingering handles or resource leaks when running the benchmark on CI with `-sanity-check`.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
---
 src/bench/wallet_migration.cpp | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp
index ca28ab0448..87710d41d8 100644
--- a/src/bench/wallet_migration.cpp
+++ b/src/bench/wallet_migration.cpp
@@ -3,14 +3,15 @@
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
 #include <bench/bench.h>
-#include <kernel/chain.h>
 #include <interfaces/chain.h>
+#include <interfaces/wallet.h>
+#include <kernel/chain.h>
 #include <node/context.h>
 #include <test/util/mining.h>
 #include <test/util/setup_common.h>
-#include <wallet/test/util.h>
 #include <wallet/context.h>
 #include <wallet/receive.h>
+#include <wallet/test/util.h>
 #include <wallet/wallet.h>
 
 #include <optional>
@@ -19,11 +20,8 @@ namespace wallet{
 
 static void WalletMigration(benchmark::Bench& bench)
 {
-    const auto test_setup = MakeNoLogFileContext<TestingSetup>();
-
-    WalletContext context;
-    context.args = &test_setup->m_args;
-    context.chain = test_setup->m_node.chain.get();
+    const auto test_setup{MakeNoLogFileContext<TestingSetup>()};
+    const auto loader{MakeWalletLoader(*test_setup->m_node.chain, test_setup->m_args)};
 
     // Number of imported watch only addresses
     int NUM_WATCH_ONLY_ADDR = 20;
@@ -63,12 +61,13 @@ static void WalletMigration(benchmark::Bench& bench)
         batch.WriteKey(pubkey, key.GetPrivKey(), CKeyMetadata());
     }
 
-    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);
-    });
+    bench.epochs(/*numEpochs=*/1)
+         .run([&] {
+             auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context(), /*was_loaded=*/false)};
+             assert(res);
+             assert(res->wallet);
+             assert(res->watchonly_wallet);
+         });
 }
 
 BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW);

From c1f458aaa06d3e23f40b695dba07fb772b51fd58 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C5=91rinc?= <pap.lorinc@gmail.com>
Date: Sat, 19 Apr 2025 14:05:38 +0200
Subject: [PATCH 2/3] ci: re-enable all benchmark runs

Drop the temporary `-filter` that excluded the `WalletMigration` benchmark.
---
 .github/workflows/ci.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 1dff87ad23..9d8fdb4a16 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -358,8 +358,7 @@ jobs:
           ./src/univalue/unitester.exe
 
       - name: Run benchmarks
-        # TODO: Fix the `WalletMigration` benchmark and re-enable it.
-        run: ./bin/bench_bitcoin.exe -sanity-check -filter='^(?!WalletMigration$).+$'
+        run: ./bin/bench_bitcoin.exe -sanity-check
 
       - name: Adjust paths in test/config.ini
         shell: pwsh

From cad39f86fb5a81f0e3b5116e8e989bab8af89718 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C5=91rinc?= <pap.lorinc@gmail.com>
Date: Sat, 19 Apr 2025 20:09:11 +0200
Subject: [PATCH 3/3] bench: ensure wallet migration benchmark runs exactly
 once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The migration benchmark crashes if run more than once, because of `std::move(wallet)` and leaves subsequent iterations in an undefined state - avoiding `UndefinedBehaviorSanitizer` null‑dereference error.
---
 src/bench/wallet_migration.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp
index 87710d41d8..afb79d98af 100644
--- a/src/bench/wallet_migration.cpp
+++ b/src/bench/wallet_migration.cpp
@@ -61,7 +61,7 @@ static void WalletMigration(benchmark::Bench& bench)
         batch.WriteKey(pubkey, key.GetPrivKey(), CKeyMetadata());
     }
 
-    bench.epochs(/*numEpochs=*/1)
+    bench.epochs(/*numEpochs=*/1).epochIterations(/*numIters=*/1) // run the migration exactly once
          .run([&] {
              auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context(), /*was_loaded=*/false)};
              assert(res);