gui: Add a menu action to restore then migrate a legacy wallet

achow101 commented at 10:09 pm on May 26, 2025: member

Some users will have a backup of their legacy wallet. These cannot be restored since the “Restore Wallet” action expects to be able to load the wallet after restoring, and this fails for legacy wallets now that they are deleted. Furthermore, the “Migrate Wallet” action only allows users to migrate wallets that are in the wallets directory, so such backups cannot be migrated from the GUI.

This PR resolves this issue by adding a menu item in the “Migrate Wallet” menu which allows users to select their backup file so that it will first be restored but not loaded, and then migrated.

Depends on https://github.com/bitcoin/bitcoin/pull/32620

DrahtBot commented at 10:09 pm on May 26, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin/bitcoin/26022 (Add util::ResultPtr class by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

achow101 force-pushed on May 26, 2025

DrahtBot added the label CI failed on May 26, 2025

DrahtBot commented at 10:18 pm on May 26, 2025: contributor

🚧 At least one of the CI tasks failed. Task ARM, unit tests, no functional tests: https://github.com/bitcoin-core/gui/runs/42923129558 LLM reason (✨ experimental): The CI failure is caused by a compilation error in bitcoingui.cpp due to incorrect usage of the Qt connect() function.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot removed the label CI failed on May 26, 2025

Krellan commented at 3:17 am on May 27, 2025: contributor

Good idea. I had this happen to me just recently. I encountered a Catch-22: couldn’t restore an old wallet file because it wasn’t in a currently recognized format (it told me to use the migrate wallet option instead), but I also couldn’t access the wallet in the migrate menu because it hadn’t successfully been loaded yet!

I tried manually copying the wallet.dat file into a new directory created in my Bitcoin datadir, but that just produced an error at startup saying the wallet was not in a recognized format. So, it’s clear we need an all-in-one “restore the old wallet.dat file AND migrate it simultaneously” feature.

luke-jr commented at 5:47 pm on May 27, 2025: member

by adding a menu item in the “Migrate Wallet” menu

IMO this is bad UX. Users will be looking to restore the wallet, and the details of how it does that shouldn’t require unintuitive steps.

achow101 commented at 6:43 pm on May 27, 2025: member

IMO this is bad UX. Users will be looking to restore the wallet, and the details of how it does that shouldn’t require unintuitive steps.

While it is an extra click, I don’t think it is unintuitive.

When the user goes to restore a legacy wallet, they will be informed that the restore failed and that they will need to migrate the wallet via the corresponding menu. When the user goes there, this option to restore and migrate will be presented to them.

wallet, interfaces: Use BERKELEY_RO in isEncrypted

Since isEncrypted opens a file to search for a particular record, it
needs to use BERKELEY_RO when opening a legacy wallet.

130a922980

achow101 force-pushed on Jun 4, 2025

wallet, interfaces, gui: Expose load_after_restore parameter

RestoreWallet has a load_after_restore parameter, expose this to callers
using it through the wallet interface as well.

b8da4fc5d1

gui: Move actual migration part of migrate() to its own function

We will need to use the same migration code in a later commit, so first
move it to a separate function.

8db890a285

gui: Add restore_and_migrate function to restore then migrate a wallet

restore_and_migrate first restores a wallet file to the wallets
directory in the expected layout, then it performs legacy to descriptor
wallet migration on the restored wallet.

2e78881cd9

gui: Add a menu item to restore then migrate a wallet file

Some users will have backups of a legacy wallet which cannot be restored
due to being a legacy wallet, and therefore cannot be migrated from the
GUI. This menu item allows such users to restore and migrate their
wallets in a single action.

56407ca69e

liamperfil commented at 1:34 pm on June 5, 2025: none

I support the proposal, but I also consider that this extra click is a bad experience for the user. The ideal would be to directly integrate this functionality to automatically detect if it is a legacy wallet and offer the migration option in the same flow. If you need someone on your team I am available.

DrahtBot added the label CI failed on Jun 13, 2025

DrahtBot removed the label CI failed on Jun 13, 2025

gui: Add a menu action to restore then migrate a legacy wallet #877

Reviews

Conflicts