From 61412ef887f3061e32b27fe1415b369d18277276 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 29 Apr 2026 14:27:30 -0700 Subject: [PATCH 1/6] bench: Utilize setup() in WalletCreate to cleanup previous wallets The WalletCreate benchmark should only be for creating a wallet and exclude the unloading of the newly created wallet. Instead, unloading can be done in setup() and after the benchmark completes. --- src/bench/wallet_create.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index d29ef339d12..11244fe8444 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -47,17 +47,21 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) const auto wallet_path = test_setup->m_path_root / "test_wallet"; const auto wallet_name = fs::PathToString(wallet_path); - bench.run([&] { - auto wallet = CreateWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error_string, warnings); - assert(status == DatabaseStatus::SUCCESS); - assert(wallet != nullptr); - + std::shared_ptr wallet; + auto cleanup{[&] { + if (!wallet) return; // Release wallet - RemoveWallet(context, wallet, /*load_on_start=*/ std::nullopt); + RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt); WaitForDeleteWallet(std::move(wallet)); fs::remove(wallet_path / "wallet.dat"); fs::remove(wallet_path); + }}; + bench.setup(cleanup).run([&] { + wallet = CreateWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + assert(status == DatabaseStatus::SUCCESS); + assert(wallet != nullptr); }); + cleanup(); } static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } From d672455d201583dbc66f03b831910028633da434 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 29 Apr 2026 14:29:07 -0700 Subject: [PATCH 2/6] bench: Utilitze setup() in WalletBalance for marking caches dirty WalletBalance benchmarks the balance computation function and should exclude the setup step of (optionally) marking caches as dirty. Instead, that is moved into setup(). --- src/bench/wallet_balance.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index d9cd4bbb952..07fd46a9fca 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -53,11 +53,14 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b auto bal = GetBalance(wallet); // Cache - bench.run([&] { - if (set_dirty) wallet.MarkDirty(); - bal = GetBalance(wallet); - if (add_mine) assert(bal.m_mine_trusted > 0); - }); + bench.setup([&] { + if (set_dirty) wallet.MarkDirty(); + }) + .run([&] { + bal = GetBalance(wallet); + ankerl::nanobench::doNotOptimizeAway(bal); + assert(add_mine == (bal.m_mine_trusted > 0)); + }); } static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/true, /*add_mine=*/true); } From 426a94e7bd7ec31213506f6e5663e74e4118e48d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 29 Apr 2026 14:32:35 -0700 Subject: [PATCH 3/6] bench: Utilize setup() in WalletEncrypt to create the encryption wallet WalletEncrypt needs an unencrypted wallet in order for the benchmark to encrypt a wallet. This was previously achieved by duplicating the contents of an initial wallet for each run of the benchmark. We can instead use setup() to unload the previously loaded wallet and then create a new wallet with unencrypted keys. --- src/bench/wallet_encrypt.cpp | 63 ++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/bench/wallet_encrypt.cpp b/src/bench/wallet_encrypt.cpp index 3d73081c7cc..1f4db614dfc 100644 --- a/src/bench/wallet_encrypt.cpp +++ b/src/bench/wallet_encrypt.cpp @@ -30,48 +30,47 @@ static void WalletEncrypt(benchmark::Bench& bench, unsigned int key_count) context.chain = test_setup->m_node.chain.get(); uint64_t create_flags = WALLET_FLAG_DESCRIPTORS; - auto database = CreateMockableWalletDatabase(); - auto wallet = TestCreateWallet(std::move(database), context, create_flags); - { - LOCK(wallet->cs_wallet); - for (unsigned int i = 0; i < key_count; i++) { - CKey key = GenerateRandomKey(); - FlatSigningProvider keys; - std::string error; - std::vector> desc = Parse("combo(" + EncodeSecret(key) + ")", keys, error, /*require_checksum=*/false); - WalletDescriptor w_desc(std::move(desc.at(0)), /*creation_time=*/0, /*range_start=*/0, /*range_end=*/0, /*next_index=*/0); - Assert(wallet->AddWalletDescriptor(w_desc, keys, /*label=*/"", /*internal=*/false)); - } + std::vector> descs; + descs.reserve(key_count); + for (unsigned int i = 0; i < key_count; i++) { + CKey key = GenerateRandomKey(); + FlatSigningProvider keys; + std::string error; + std::vector> desc = Parse("combo(" + EncodeSecret(key) + ")", keys, error, /*require_checksum=*/false); + WalletDescriptor w_desc(std::move(desc.at(0)), /*creation_time=*/0, /*range_start=*/0, /*range_end=*/0, /*next_index=*/0); + descs.emplace_back(w_desc, keys); } - database = DuplicateMockDatabase(wallet->GetDatabase()); - - // reload the wallet for the actual benchmark - TestUnloadWallet(std::move(wallet)); - // Setting a mock time is necessary to force default derive iteration count during // wallet encryption. SetMockTime(1); - // This benchmark has a lot of overhead, this should be good enough to catch - // any regressions, but for an accurate measurement of how long wallet - // encryption takes, this should be reworked after something like - // https://github.com/bitcoin/bitcoin/pull/34208 is merged. - bench.batch(key_count).unit("key").run([&] { - wallet = TestLoadWallet(std::move(database), context); + std::unique_ptr database; + std::shared_ptr wallet; + bench.batch(key_count).unit("key").setup([&] { + if (wallet) { + TestUnloadWallet(std::move(wallet)); + } - // Save a copy of the db before encrypting - database = DuplicateMockDatabase(wallet->GetDatabase()); + std::unique_ptr database = CreateMockableWalletDatabase(); + wallet = TestCreateWallet(std::move(database), context, create_flags); - wallet->EncryptWallet(secure_pass); + { + LOCK(wallet->cs_wallet); + for (auto& [desc, keys] : descs) { + Assert(wallet->AddWalletDescriptor(desc, keys, /*label=*/"", /*internal=*/false)); + } + } + }) + .run([&] { + wallet->EncryptWallet(secure_pass); - for (const auto& [_, key] : wallet->mapMasterKeys){ - assert(key.nDeriveIterations == CMasterKey::DEFAULT_DERIVE_ITERATIONS); - } - - TestUnloadWallet(std::move(wallet)); - }); + for (const auto& [_, key] : wallet->mapMasterKeys){ + assert(key.nDeriveIterations == CMasterKey::DEFAULT_DERIVE_ITERATIONS); + } + }); + TestUnloadWallet(std::move(wallet)); } constexpr unsigned int KEY_COUNT = 2000; From 9a7604fd257c4a549d0f22f28a36fef607c0dcf4 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 29 Apr 2026 14:34:39 -0700 Subject: [PATCH 4/6] bench: Use setup() in WalletMigration to prepare the legacy wallet WalletMigration needs a new wallet with legacy records for each run of the benchmark. This can be done in setup() rather than duplicating the records of an initial wallet. --- src/bench/wallet_migration.cpp | 101 ++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 45 deletions(-) diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp index 7d7aca113d1..578fdb50d6b 100644 --- a/src/bench/wallet_migration.cpp +++ b/src/bench/wallet_migration.cpp @@ -27,53 +27,64 @@ static void WalletMigration(benchmark::Bench& bench) // Number of imported watch only addresses int NUM_WATCH_ONLY_ADDR = 20; - // Setup legacy wallet - std::unique_ptr wallet = std::make_unique(test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()); - { - LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM(); - WalletBatch batch{wallet->GetDatabase()}; - - // Write a best block record as migration expects one to exist - CBlockLocator loc; - batch.WriteBestBlock(loc); - - // Add watch-only addresses - std::vector scripts_watch_only; - for (int w = 0; w < NUM_WATCH_ONLY_ADDR; ++w) { - CKey key = GenerateRandomKey(); - LOCK(wallet->cs_wallet); - const PKHash dest{key.GetPubKey()}; - 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)); - batch.WriteWatchOnly(script, CKeyMetadata()); - } - - // Generate transactions and local addresses - 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(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, /*rescanning_old_block=*/true); - batch.WriteKey(pubkey, key.GetPrivKey(), CKeyMetadata()); - } + // Add watch-only addresses + std::vector> scripts_watch_only; + for (int w = 0; w < NUM_WATCH_ONLY_ADDR; ++w) { + CKey key = GenerateRandomKey(); + const PKHash dest{key.GetPubKey()}; + scripts_watch_only.emplace_back(GetScriptForDestination(dest), dest); } - bench.epochs(/*numEpochs=*/1).epochIterations(/*numIters=*/1) // run the migration exactly once - .run([&] { - auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context())}; - assert(res); - assert(res->wallet); - assert(res->watchonly_wallet); - }); + // Generate transactions and local addresses + std::vector keys(500); + std::ranges::generate(keys, []{ return GenerateRandomKey(); }); + + std::unique_ptr wallet; + size_t i = 0; + bench.epochs(/*numEpochs=*/1) // run the migration exactly once + .setup([&] { + // Setup legacy wallet + wallet = std::make_unique(test_setup->m_node.chain.get(), std::string(i++, 'A'), CreateMockableWalletDatabase()); + LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM(); + WalletBatch batch{wallet->GetDatabase()}; + + LOCK(wallet->cs_wallet); + + // Write a best block record as migration expects one to exist + CBlockLocator loc; + batch.WriteBestBlock(loc); + + // Add watch-only addresses + for (size_t w = 0; w < scripts_watch_only.size(); ++w) { + const auto& [script, dest] = scripts_watch_only.at(w); + assert(legacy_spkm->LoadWatchOnly(script)); + assert(wallet->SetAddressBook(dest, strprintf("watch_%d", w), /*purpose=*/std::nullopt)); + batch.WriteWatchOnly(script, CKeyMetadata()); + } + + // Generate transactions and local addresses + for (size_t j = 0; j < keys.size(); ++j) { + const CKey& key = keys.at(j); + // Load key, scripts and create address book record + CPubKey pubkey = key.GetPubKey(); + 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(dest)); + mtx.vout.emplace_back(COIN, scripts_watch_only.at(j % NUM_WATCH_ONLY_ADDR).first); + mtx.vin.resize(2); + wallet->AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*rescanning_old_block=*/true); + batch.WriteKey(pubkey, key.GetPrivKey(), CKeyMetadata()); + } + }) + .run([&] { + auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context())}; + assert(res); + assert(res->wallet); + assert(res->watchonly_wallet); + }); } BENCHMARK(WalletMigration); From 57820c472b4e1b10fc0d2cc5b60c4783338edc7c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 6 Apr 2026 14:58:53 -0700 Subject: [PATCH 5/6] bench: Utilize setup() for WalletLoading and use a real database Instead of making a mock database and duplicating it for the benchmark, use a real database. Also use setup() to avoid measuring the overhead in the benchmark. --- src/bench/wallet_loading.cpp | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 20997ff8927..09028acdb03 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -42,7 +42,12 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench) // Setup the wallet // Loading the wallet will also create it uint64_t create_flags = WALLET_FLAG_DESCRIPTORS; - auto database = CreateMockableWalletDatabase(); + DatabaseStatus status; + DatabaseOptions options; + options.require_format = DatabaseFormat::SQLITE; + options.require_create = true; + bilingual_str error; + auto database = MakeWalletDatabase("", options, status, error); auto wallet = TestCreateWallet(std::move(database), context, create_flags); // Generate a bunch of transactions and addresses to put into the wallet @@ -50,18 +55,20 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench) AddTx(*wallet); } - database = DuplicateMockDatabase(wallet->GetDatabase()); + options.require_create = false; + options.require_existing = true; - // reload the wallet for the actual benchmark + bench.epochs(5) + .setup([&] { + TestUnloadWallet(std::move(wallet)); + database = MakeWalletDatabase("", options, status, error); + }) + .run([&] { + wallet = TestLoadWallet(std::move(database), context); + }); + + // Cleanup TestUnloadWallet(std::move(wallet)); - - bench.epochs(5).run([&] { - wallet = TestLoadWallet(std::move(database), context); - - // Cleanup - database = DuplicateMockDatabase(wallet->GetDatabase()); - TestUnloadWallet(std::move(wallet)); - }); } BENCHMARK(WalletLoadingDescriptors); From 1d1ae6f0c4f2f60f5ceb2d60e8a52662e542e21d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 6 Apr 2026 15:04:00 -0700 Subject: [PATCH 6/6] wallet, test: Remove DuplicateMockDatabase DuplicateMockDatabase is no longer used. Furthermore, as SQLite gets used more as a database and less as a key value store, this function gets more complicated and more bug prone. As the benchmarks now run equivalently quickly with a real database, retaining this duplication function is no longer necessary. --- src/wallet/test/util.cpp | 22 ---------------------- src/wallet/test/util.h | 3 --- 2 files changed, 25 deletions(-) diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index b21c96f17ab..fda8cb1c3a9 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -105,27 +104,6 @@ void TestUnloadWallet(std::shared_ptr&& wallet) WaitForDeleteWallet(std::move(wallet)); } -std::unique_ptr DuplicateMockDatabase(WalletDatabase& database) -{ - std::unique_ptr batch_orig = database.MakeBatch(); - std::unique_ptr cursor_orig = batch_orig->GetNewCursor(); - - std::unique_ptr new_db = CreateMockableWalletDatabase(); - std::unique_ptr new_db_batch = new_db->MakeBatch(); - MockableSQLiteBatch* batch_new = dynamic_cast(new_db_batch.get()); - Assert(batch_new); - - while (true) { - DataStream key, value; - DatabaseCursor::Status status = cursor_orig->Next(key, value); - Assert(status != DatabaseCursor::Status::FAIL); - if (status != DatabaseCursor::Status::MORE) break; - batch_new->WriteKey(std::move(key), std::move(value)); - } - - return new_db; -} - std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index fbc5188fc14..9a407d31025 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -39,9 +39,6 @@ std::shared_ptr TestLoadWallet(WalletContext& context); std::shared_ptr TestLoadWallet(std::unique_ptr database, WalletContext& context); void TestUnloadWallet(std::shared_ptr&& wallet); -// Creates a copy of the provided database -std::unique_ptr DuplicateMockDatabase(WalletDatabase& database); - /** Returns a new encoded destination from the wallet (hardcoded to BECH32) */ std::string getnewaddress(CWallet& w); /** Returns a new destination, of an specific type, from the wallet */