refactor: store wallet names to MigrationResult
What the developer wrote
Authored by Pol Espinasa
refactor: store wallet names to MigrationResult
Store wallet names into MigrationResult struct when migrating a wallet.
Also refactor the RPC and the wallet interface to rely on them instead of pointers to shared_ptr<CWallet> objects.
This allows in a future commit migrate wallet without loading them.
What changed, and why it matters
This is a straightforward internal code cleanup in Bitcoin Core's wallet migration feature. It changes how wallet names are passed around so that the code stores names directly in a result structure rather than fetching names from live wallet objects. There is no security-relevant change visible in the diff, and the commit message explicitly calls it a refactor.
No security action needed; treat as normal code maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors MigrationResult to carry optional watchonly_wallet_name and solvables_wallet_name strings, and updates the RPC and wallet interface consumers to read those strings instead of calling GetName() on shared_ptr
Changed components
src/wallet/wallet.hsrc/wallet/wallet.cppsrc/wallet/rpc/wallet.cppsrc/wallet/interfaces.cppInspect captured patch +17 / −8
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 4eee155c..0237c795 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -605,8 +605,8 @@ public:
if (!res) return util::Error{util::ErrorString(res)};
WalletMigrationResult out{
.wallet = MakeWallet(m_context, res->wallet),
- .watchonly_wallet_name = res->watchonly_wallet ? std::make_optional(res->watchonly_wallet->GetName()) : std::nullopt,
- .solvables_wallet_name = res->solvables_wallet ? std::make_optional(res->solvables_wallet->GetName()) : std::nullopt,
+ .watchonly_wallet_name = res->watchonly_wallet_name,
+ .solvables_wallet_name = res->solvables_wallet_name,
.backup_path = res->backup_path,
};
return out;
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index b6c9179d..df40e51e 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -625,11 +625,11 @@ static RPCMethod migratewallet()
UniValue r{UniValue::VOBJ};
r.pushKV("wallet_name", res->wallet_name);
- if (res->watchonly_wallet) {
- r.pushKV("watchonly_name", res->watchonly_wallet->GetName());
+ if (res->watchonly_wallet_name.has_value()) {
+ r.pushKV("watchonly_name", res->watchonly_wallet_name.value());
}
- if (res->solvables_wallet) {
- r.pushKV("solvables_name", res->solvables_wallet->GetName());
+ if (res->solvables_wallet_name.has_value()) {
+ r.pushKV("solvables_name", res->solvables_wallet_name.value());
}
r.pushKV("backup_path", res->backup_path.utf8string());
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index fcc1969d..c84effe7 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4449,6 +4449,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
LogInfo("Loading new wallets after migration...\n");
// Migration successful, load all the migrated wallets.
+ bool main_wallet_set{false};
for (std::shared_ptr<CWallet>* wallet_ptr : {&local_wallet, &res.watchonly_wallet, &res.solvables_wallet}) {
if (success && *wallet_ptr) {
std::shared_ptr<CWallet>& wallet = *wallet_ptr;
@@ -4467,9 +4468,15 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
// Set the first successfully loaded wallet as the main one.
// The loop order is intentional and must always start with the local wallet.
- if (!res.wallet) {
- res.wallet_name = wallet->GetName();
+ if (!main_wallet_set) {
+ res.wallet_name = wallet_name;
res.wallet = std::move(wallet);
+ main_wallet_set = true;
+ }
+ if (wallet_ptr == &res.watchonly_wallet) {
+ res.watchonly_wallet_name = wallet_name;
+ } else if (wallet_ptr == &res.solvables_wallet) {
+ res.solvables_wallet_name = wallet_name;
}
}
}
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 32ef9c8e..b964846f 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1134,6 +1134,8 @@ bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_nam
struct MigrationResult {
std::string wallet_name;
+ std::optional<std::string> watchonly_wallet_name;
+ std::optional<std::string> solvables_wallet_name;
std::shared_ptr<CWallet> wallet;
std::shared_ptr<CWallet> watchonly_wallet;
std::shared_ptr<CWallet> solvables_wallet;
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.