Adds option to not load the wallet after migration #953

polespinasa commented at 4:51 PM on July 27, 2026: member

Following https://github.com/bitcoin/bitcoin/pull/35266 this PR adds the option to not load the wallet after migrating to the GUI.

It is only added for the migrate option, not for the restore_and_migrate. I guess if we are restoring the wallet we always want to load it. In any case, it's pretty straightforward to implement it there too.

Inside the original migration pop-up box it appears a checkbox that allows the user to choose if want to load the wallet or not, it is checked by default:

If yes, the wallet is loaded and shown, if not the wallet gets migrated and the GUI returns to it's state.

The checkbox has a tooltip that informs when not loading a migrated wallet can be useful: <img width="503" height="494" alt="imagen" src="https://github.com/user-attachments/assets/3a3404ff-1fd3-44ef-8f9d-db526c9f032f" />

DrahtBot commented at 4:52 PM on July 27, 2026: contributor

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

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	pablomartin4btc, hebasto, achow101
Concept ACK	w0xlt

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#943 (qt: warn about rescan in wallet migration dialogs by ShauryaaSharma)
#bitcoin/bitcoin/25722 (refactor: Use util::Result class for wallet loading 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.

polespinasa requested review from achow101 on Jul 27, 2026

in src/qt/walletcontroller.h:199 in 45f61713fc

 195 | @@ -196,7 +196,7 @@ class MigrateWalletActivity : public WalletControllerActivity
 196 |  private:
 197 |      QString m_success_message;
 198 |  
 199 | -    void do_migrate(const std::string& name);
 200 | +    void do_migrate(const std::string& name, const bool load_wallet);

pablomartin4btc commented at 5:29 PM on July 27, 2026:

nit: I think const on a by-value parameter is redundant here, only protects the local copy and doesn't affect callers. Same applies to the other occurrences in walletcontroller.cpp, interfaces/wallet.h and interfaces.cpp.

    void do_migrate(const std::string& name, bool load_wallet);

polespinasa commented at 5:46 PM on July 27, 2026:

done

in src/qt/walletcontroller.cpp:498 in 45f61713fc

 490 | @@ -486,7 +491,16 @@ void MigrateWalletActivity::migrate(const std::string& name)
 491 |      box.setDefaultButton(QMessageBox::Yes);
 492 |      if (box.exec() != QMessageBox::Yes) return;
 493 |  
 494 | -    do_migrate(name);
 495 | +    // Warn the user about loading the wallet
 496 | +    QMessageBox box_loadwallet(m_parent_widget);
 497 | +    box_loadwallet.setWindowTitle(tr("Migrate wallet"));
 498 | +    box_loadwallet.setText(tr("Do you want to load the wallet <i>%1</i> after migrating?").arg(GUIUtil::HtmlEscape(GUIUtil::WalletDisplayName(name))));
 499 | +    box_loadwallet.setInformativeText(tr("If the node is pruned, loading the wallet after migrating can fail the migration if the wallet was created before the pruned height."));

pablomartin4btc commented at 5:39 PM on July 27, 2026:

nit (perhaps cleaner...):

    box_loadwallet.setInformativeText(tr("If the node is pruned and the wallet was created before the pruned height, the migration process may fail trying to load the migrated wallet."));

polespinasa commented at 5:46 PM on July 27, 2026:

done

pablomartin4btc commented at 5:40 PM on July 27, 2026: contributor

Concept ACK

It'd be nice to have the screenshots in the description (perhaps collapsed with markup <details>) so reviewers that don't want to test it can also give their feedback.

Left a couple of nits.

polespinasa force-pushed on Jul 27, 2026

DrahtBot added the label CI failed on Jul 27, 2026

DrahtBot commented at 5:51 PM on July 27, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task riscv32 bare metal, static libbitcoin_consensus: https://github.com/bitcoin-core/gui/actions/runs/30286650882/job/90045998601</sub> <sub>LLM reason (✨ experimental): CI failed because ./ci/test/01_base_install.sh couldn’t clone the newlib-cygwin submodule from sourceware.org due to HTTP 429 (rate limited).</sub>

<details><summary>Hints</summary>

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.

</details>

achow101 commented at 10:46 PM on July 27, 2026: member

Concept ACK

In terms of UX, I think it would be better to have this be a checkbox option in the initial dialog rather than a second dialog that the user has to click through.

polespinasa force-pushed on Jul 28, 2026

polespinasa commented at 9:13 AM on July 28, 2026: member

In terms of UX, I think it would be better to have this be a checkbox option in the initial dialog rather than a second dialog that the user has to click through.

Done, modified the description to show the current UI.

polespinasa closed this on Jul 28, 2026

polespinasa reopened this on Jul 28, 2026

in src/qt/walletcontroller.cpp:491 in 2b4f57cad6 outdated

 487 | @@ -482,11 +488,15 @@ void MigrateWalletActivity::migrate(const std::string& name)
 488 |                  "The migration process will create a backup of the wallet before migrating. This backup file will be named "
 489 |                  "<wallet name>-<timestamp>.legacy.bak and can be found in the directory for this wallet. In the event of "
 490 |                  "an incorrect migration, the backup can be restored with the \"Restore Wallet\" functionality."));
 491 | +    auto* load_wallet_checkbox = new QCheckBox(tr("Load wallet after migration"), &box);

pablomartin4btc commented at 12:41 PM on July 28, 2026:

usually we include the widget into the .ui form, but this is ok I think.

polespinasa commented at 2:01 PM on July 28, 2026:

I will resolve it for now, unless someone requires that too, I also think this is ok as is, easier to modify and remove if ever have to

in src/wallet/interfaces.cpp:604 in 2b4f57cad6

 600 | @@ -601,9 +601,9 @@ class WalletLoaderImpl : public WalletLoader
 601 |          }
 602 |          return wallet;
 603 |      }
 604 | -    util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase) override
 605 | +    util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase, const bool load_wallet) override

pablomartin4btc commented at 12:43 PM on July 28, 2026:

nit: I think this one was missed from my previous suggestion...

    util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase, bool load_wallet) override

polespinasa commented at 2:01 PM on July 28, 2026:

wopps I missed that one, thanks!

pablomartin4btc commented at 12:43 PM on July 28, 2026: contributor

ACK 2b4f57cad66ad2e58c51ec0f351a5c112d59b534

w0xlt commented at 1:17 PM on July 28, 2026: contributor

Concept ACK

gui: Adds option to not load the wallet after migration 492a715d78

add release notes 4cea59573c

polespinasa force-pushed on Jul 28, 2026

pablomartin4btc commented at 1:54 PM on July 29, 2026: contributor

ACK 4cea59573c8310fd182d7b4f8ecf04fd0bbc3897

DrahtBot requested review from w0xlt on Jul 29, 2026

hebasto renamed this:
~~gui: Adds option to not load the wallet after migration~~
Adds option to not load the wallet after migration
on Jul 29, 2026

hebasto approved

hebasto commented at 6:11 PM on July 29, 2026: member

ACK 4cea59573c8310fd182d7b4f8ecf04fd0bbc3897, I have reviewed the code and it looks OK.

achow101 commented at 6:33 PM on July 29, 2026: member

ACK 4cea59573c8310fd182d7b4f8ecf04fd0bbc3897

hebasto merged this on Jul 29, 2026

hebasto closed this on Jul 29, 2026

polespinasa deleted the branch on Jul 30, 2026