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

pull achow101 wants to merge 4 commits into bitcoin-core:master from achow101:gui-migrate-path changing 5 files +93 −22
  1. 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

  2. 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.

    Type Reviewers
    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:

    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.

  3. achow101 force-pushed on May 26, 2025
  4. DrahtBot added the label CI failed on May 26, 2025
  5. 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.

  6. DrahtBot removed the label CI failed on May 26, 2025
  7. 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.

  8. 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.

  9. 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.

  10. achow101 force-pushed on Jun 4, 2025
  11. 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.
  12. DrahtBot added the label CI failed on Jun 13, 2025
  13. DrahtBot removed the label CI failed on Jun 13, 2025
  14. pablomartin4btc commented at 7:38 pm on June 20, 2025: contributor

    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),

    If you copy the .dat file or the backup file into the wallets dir (default: datadir/wallets) you will see the option in the “Migrate Wallet” menu item (you can check also with bitcoin-cli or the RPC console and if you call listwalletdir would show it in the output).

    but I also couldn’t access the wallet in the migrate menu because it hadn’t successfully been loaded yet!

    It’s no longer possible to load a legacy wallet on the master branch and future releases, but you don’t need to load the wallet in order to migrate it as I explained above, however you could do it in previous versions of QT or bircoind (v29 or earlier).

    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.

    Just tried and it works, if you launched QT with a test chain you need to add that into the path (eg if you used -regtest, the wallets dir path would be /datadir/regtest/wallets).

  15. pablomartin4btc commented at 7:55 pm on June 20, 2025: contributor

    tACK 56407ca69e34778cfd0d054fbaa94ab0b28ffb6d

    I’ll try to review the code later (first commit is not longer needed, perhaps rebase?).

    Regarding the approach IMHO:

    integrate this functionality to automatically detect if it is a legacy wallet and offer the migration option in the same flow.

    I thought the same at first glance as an additional to the current PR logic, if the user knows that has the backup to restore maybe goes directly there first.

    I’d update the menu item title to “Wallet file…”, since you could restore a .dat file or migrate a backup file directly - checked with CLI.

    Perhaps @GBKS could add his view here…

    image

  16. 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.
    2acf773800
  17. 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.
    c13f1fbffb
  18. 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.
    114b52d605
  19. 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.
    c02e8a376d
  20. achow101 force-pushed on Jun 20, 2025
  21. achow101 marked this as ready for review on Jun 20, 2025
  22. achow101 commented at 8:56 pm on June 20, 2025: member
    Rebased and ready for review

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-09 13:20 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me