← Watch feed
bench: Use setup() in WalletMigration to prepare the legacy wallet
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Ava Chow
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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a small internal cleanup of a Bitcoin Core benchmark test. It moves the creation of a fake legacy wallet from the main benchmark body into a setup() routine so the benchmark can be repeated cleanly. It does not change any production wallet code, network rules, or user-facing behavior, and it introduces no security issue.
Recommended action
No action required. This is a benign benchmark refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors src/bench/wallet_migration.cpp so that WalletMigration uses the benchmark framework’s setup() callback to create a fresh legacy wallet for each epoch, instead of constructing the wallet once before the benchmark loop. The same records (best block, watch-only addresses, keys, transactions) are still written; only the timing boundaries and wallet name (now unique per setup via an incrementing counter) change. This is a test-only change.
Changed components
src/bench/wallet_migration.cppInspect captured patch +52 / −41
diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp
index 7d7aca11..578fdb50 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<CWallet> wallet = std::make_unique<CWallet>(test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase());
- {
- LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM();
- WalletBatch batch{wallet->GetDatabase()};
+ // Add watch-only addresses
+ std::vector<std::pair<CScript, CTxDestination>> 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);
+ }
+
+ // Generate transactions and local addresses
+ std::vector<CKey> keys(500);
+ std::ranges::generate(keys, []{ return GenerateRandomKey(); });
- // Write a best block record as migration expects one to exist
- CBlockLocator loc;
- batch.WriteBestBlock(loc);
+ std::unique_ptr<CWallet> wallet;
+ size_t i = 0;
+ bench.epochs(/*numEpochs=*/1) // run the migration exactly once
+ .setup([&] {
+ // Setup legacy wallet
+ wallet = std::make_unique<CWallet>(test_setup->m_node.chain.get(), std::string(i++, 'A'), CreateMockableWalletDatabase());
+ LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM();
+ WalletBatch batch{wallet->GetDatabase()};
- // 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 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));
+ // Write a best block record as migration expects one to exist
+ CBlockLocator loc;
+ batch.WriteBestBlock(loc);
- 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
+ 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));
- 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);
- });
+ 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);
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.