Skip to content

Adds option to not load the wallet after migration - #953

Merged
hebasto merged 2 commits into
bitcoin-core:masterfrom
polespinasa:2026-07-27-noloadwalletaftermigration
Jul 29, 2026
Merged

Adds option to not load the wallet after migration#953
hebasto merged 2 commits into
bitcoin-core:masterfrom
polespinasa:2026-07-27-noloadwalletaftermigration

Conversation

@polespinasa

@polespinasa polespinasa commented Jul 27, 2026

Copy link
Copy Markdown
Member

Following bitcoin/bitcoin#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:

imagen

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:
imagen

@DrahtBot

DrahtBot commented Jul 27, 2026

Copy link
Copy Markdown
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 <!--meta-tag:bot-skip--> 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.

@pablomartin4btc pablomartin4btc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/qt/walletcontroller.h Outdated
Comment thread src/qt/walletcontroller.cpp Outdated
@polespinasa
polespinasa force-pushed the 2026-07-27-noloadwalletaftermigration branch from 45f6171 to e40c1e3 Compare July 27, 2026 17:50
@DrahtBot

Copy link
Copy Markdown
Contributor

🚧 At least one of the CI tasks failed.
Task riscv32 bare metal, static libbitcoin_consensus: https://github.com/bitcoin-core/gui/actions/runs/30286650882/job/90045998601
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).

Hints

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.

@achow101

Copy link
Copy Markdown
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
polespinasa force-pushed the 2026-07-27-noloadwalletaftermigration branch from e40c1e3 to 2b4f57c Compare July 28, 2026 09:11
@polespinasa

Copy link
Copy Markdown
Member Author

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 polespinasa reopened this Jul 28, 2026

@pablomartin4btc pablomartin4btc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 2b4f57c

Comment thread src/qt/walletcontroller.cpp
Comment thread src/wallet/interfaces.cpp Outdated
@w0xlt

w0xlt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Concept ACK

@pablomartin4btc pablomartin4btc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4cea595

@DrahtBot
DrahtBot requested a review from w0xlt July 29, 2026 13:54
@hebasto hebasto changed the title gui: Adds option to not load the wallet after migration Adds option to not load the wallet after migration Jul 29, 2026

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@achow101

Copy link
Copy Markdown
Member

ACK 4cea595

@hebasto
hebasto merged commit 9b38d07 into bitcoin-core:master Jul 29, 2026
28 of 29 checks passed
@polespinasa
polespinasa deleted the 2026-07-27-noloadwalletaftermigration branch July 30, 2026 09:15
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Aug 2, 2026
…migration

3bfd600 add release notes (Pol Espinasa)
e0f5830 gui: Adds option to not load the wallet after migration (Pol Espinasa)

Pull request description:

  Following bitcoin/bitcoin#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:

  <img width="497" height="388" alt="imagen" src="https://github.com/user-attachments/assets/79c76f9b-9b06-4fcb-88fe-3b5db3eaf14f" />

  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" />

ACKs for top commit:
  achow101:
    ACK 3bfd600
  pablomartin4btc:
    ACK 3bfd600
  hebasto:
    ACK 3bfd600, I have reviewed the code and it looks OK.

Tree-SHA512: 6256849ecca3888fe24866ed539b262be6275c6192d29d272e7ad1475943c2947c04a3fadaae94e1843b3fcfa43a1f84edb02365933bf954b8ba7d0ae53eda32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants