Fix handling of load wallet in controller#447
Merged
hebasto merged 1 commit intobitcoin-core:mainfrom Mar 11, 2025
Merged
Conversation
Previous implementation was loading the wallet twice
Member
What scenarios are you referring to? |
Collaborator
Author
This is the QList member of the walletqmlcontroller. Previously it wasn't properly checking if we had the wallet in the list already before calling loadWallet and storing the unique_ptr in this list. You'll see the check at the top of setSelectedWallet now. The other change is that we don't use the unique_ptr after loadWallet, instead the handleLoadWallet method is used to store the resulting wallet. handleLoadWallet gets called anytime a wallet is loaded either from calling that interface method, rpc load, or at startup. |
johnny9
changed the title
johnny9
commented
Mar 10, 2025
Collaborator
Author
johnny9
left a comment
johnny9
left a comment
There was a problem hiding this comment.
Tested on Ubuntu desktop.
- With multiple wallets created and all of them unloaded, selected each wallet to test that it doesn't crash.
- With the wallets previously loaded, start the application and select each wallet in the select to test that it doesn't crash
- Started the application with a datadir that contains no wallets to validate initialization handles the empty case properly
Collaborator
Author
jarolrod
reviewed
Mar 11, 2025
johnny9
pushed a commit
to johnny9/bitcoin-core-app
that referenced
this pull request
Jul 4, 2025
…ller c3c63a9 qml: Fix handling of load wallet in controller (johnny9) Pull request description: Don't load wallet if we already have them loaded. There are 3 changes to the original code. In setSelectedWallet there is a proper check to see if there's already an instance of the wallet loaded in our m_wallets list and sets selected_wallet to that if true, It only loads the wallet into the list once (handleLoadWallet gets called when doing loadWallet), and it fixes the initialize to not call handlLoadWallet and emit selectedWalletChanged a bunch of times. ACKs for top commit: jarolrod: ACK c3c63a9 Tree-SHA512: daba6fb019ba78f1c83105e34a1d724408ba83a4564942dd127d1e0edd0fc5abc63e3080b9b621cce9ae27b2dca96070300caaa8250c4c031836f34b5b678a07
tx-signer450
added a commit
to tx-signer450/gui-qml
that referenced
this pull request
Oct 20, 2025
…ller
c3c63a98cf42cc22e3423cacb66dd56b9caeea5e qml: Fix handling of load wallet in controller (johnny9)
Pull request description:
Don't load wallet if we already have them loaded.
There are 3 changes to the original code. In setSelectedWallet there is a proper check to see if there's already an instance of the wallet loaded in our m_wallets list and sets selected_wallet to that if true, It only loads the wallet into the list once (handleLoadWallet gets called when doing loadWallet), and it fixes the initialize to not call handlLoadWallet and emit selectedWalletChanged a bunch of times.
ACKs for top commit:
jarolrod:
ACK c3c63a98cf42cc22e3423cacb66dd56b9caeea5e
Tree-SHA512: daba6fb019ba78f1c83105e34a1d724408ba83a4564942dd127d1e0edd0fc5abc63e3080b9b621cce9ae27b2dca96070300caaa8250c4c031836f34b5b678a07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't load wallet if we already have them loaded.
There are 3 changes to the original code. In setSelectedWallet there is a proper check to see if there's already an instance of the wallet loaded in our m_wallets list and sets selected_wallet to that if true, It only loads the wallet into the list once (handleLoadWallet gets called when doing loadWallet), and it fixes the initialize to not call handlLoadWallet and emit selectedWalletChanged a bunch of times.