gui
gui copied to clipboard
bitcoin-core
Migrate legacy wallets that are not loaded
Currently the Migrate Wallet menu item can only be used to migrate the currently loaded wallet. This is not suitable for the future when legacy wallets can no longer be loaded at all, but should still be able to be migrated. This PR changes that menu item into a menu list like Open Wallet and lets users migrate any legacy wallet in their wallet directory regardless of the wallets loaded.
One issue I ran into was dealing with encrypted wallets. Ideally, we would detect whether a wallet is encrypted, and prompt the user for their passphrase at that time. However, that's actually difficult to do in the GUI since migration will unload the wallet if it was already loaded, and reload it without connecting it to any signals or interfaces. Only then can it detect whether a wallet is encrypted, but then there is no WalletModel or even an interfaces::Wallet that the GUI could use to unlock it via a callback.
To deal with this, I've opted to just add a button to the migration dialog box that has the user enter their passphrase first, along with instructional text to use that button if their wallet was encrypted. If the user enters the wrong passphrase or clicked the other button that does not prompt for the passphrase, migration will fail with a message indicating that the passphrase was incorrect.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviews
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hebasto, furszy |
| Stale ACK | pablomartin4btc |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #753 (Add new "address type" column to the "receiving tab" address book page by pablomartin4btc)
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.
To deal with this, I've opted to just add a button to the migration dialog box that has the user enter their passphrase first, along with instructional text to use that button if their wallet was encrypted.
If we're preemptively making UX changes just to avoid changing UX in the future, it seems like we shouldn't take shortcuts like this... :/
To deal with this, I've opted to just add a button to the migration dialog box that has the user enter their passphrase first, along with instructional text to use that button if their wallet was encrypted.
If we're preemptively making UX changes just to avoid changing UX in the future, it seems like we shouldn't take shortcuts like this... :/
We already do this in the RPC too, for exactly the same reason.
This PR changes that menu item into a menu list like Open Wallet and lets users migrate any legacy wallet in their wallet directory regardless of the wallets loaded.
Tested d45ac03a89cc500cd461f878f092cf8ec99e7760. The "Migrate Wallet" menu item is still disabled when no wallet is loaded.
Tested d45ac03. The "Migrate Wallet" menu item is still disabled when no wallet is loaded.
Should be fixed now.
"Migrate Wallet" is shown when there are no wallets available at all.
I think that's ok.
"Migrate Wallet" is shown when there are no wallets available at all.
I think that's ok.
Yeah, it was just an observation (mainly to compare when the menu is disabled), thanks!
Could replace it directly for furszy/bitcoin-core@e8d147e. It detects the wallet encryption status automatically regardless of its loaded/unloaded state.
Cherry picked.
Also fixed test failure.