Switch to the selected wallet after loading #665

pull w0xlt wants to merge 1 commits into bitcoin-core:master from w0xlt:load_wallet_signal changing 1 files +1 −1

w0xlt commented at 2:33 AM on September 8, 2022: contributor

Currently, the user loads a wallet and the screen does not switch to the selected wallet after loading (File -> Open Wallet -> wallet name).

This PR changes that by making the OpenWalletActivity::opened signal connection a Qt::QueuedConnection type.
gui: update the screen after loading wallet b8b59ff9fe
hebasto commented at 5:14 AM on September 9, 2022: member

Maybe amend a PR title/commit message to mention "switch to the selected wallet after loading"?
in src/qt/bitcoingui.cpp:413 in b8b59ff9fe
```
 409 | @@ -410,7 +410,7 @@ void BitcoinGUI::createActions()
 410 |  
 411 |                  connect(action, &QAction::triggered, [this, path] {
 412 |                      auto activity = new OpenWalletActivity(m_wallet_controller, this);
 413 | -                    connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet);
 414 | +                    connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet, Qt::QueuedConnection);
```
hebasto commented at 5:49 AM on September 9, 2022:

Why this solution works?

w0xlt commented at 7:05 PM on September 9, 2022:

There's a good explanation here: #471#pullrequestreview-989631968 as this is the same case found in Restore Wallet.

As the default value for this parameter is Qt::AutoConnection, it appears to be using Qt::DirectConnection, in which the slot is invoked immediately.

https://doc.qt.io/qt-6/qt.html#ConnectionType-enum

luke-jr commented at 11:47 PM on September 10, 2022:

Feels like a race that would be better being explicitly handled in the correct order?

w0xlt commented at 3:21 PM on September 14, 2022:

I think Qt::DirectConnection guarantees the correct order in this case, as BitcoinGUI::setCurrentWallet will only run after control returns to the event loop of the receiver's thread.
hebasto added the label UX on Sep 9, 2022
w0xlt renamed this:
~~Update the screen after loading wallet~~
Switch to the selected wallet after loading
on Sep 9, 2022
jarolrod commented at 9:47 PM on September 13, 2022: member

ACK b8b59ff9fea69f4f25d98005e3ac9172a7a11c12

Can confirm that this fixes the issue. It appears safe to explicitly specify what kind of ConnectionType we want, there isn't an issue with the parameters given to the meta system and queuing this. We probably still want to continue to flesh this out and determine if we can refactor to remove explicitly specifying the ConnectionType
pablomartin4btc commented at 5:15 PM on September 21, 2022: contributor

Tested ACK on ubuntu 20.04.

Loading a wallet now sets it on the "Wallet:" combobox at the top right hand corner.

So far I couldn't find if there's an alternative to avoid specifying the ConnectionType on the params. I see the different types will depend on whether the sender and receiver belong to the same thread or not.
pablomartin4btc commented at 5:32 PM on September 21, 2022: contributor

There's a kind of similar issue when there's a selected wallet on the main window and opening the rpc console sets a different one. I didn't check yet (rpcconsole.cpp?) if there's a call to select a wallet and it's not working properly.
Sjors commented at 1:01 PM on October 6, 2022: member

Concept ACK. It'd be nice if Console switches too; wouldn't be the first time I import descriptors into the wrong wallet :-)
hebasto approved
hebasto commented at 12:49 PM on October 27, 2022: member

ACK b8b59ff9fea69f4f25d98005e3ac9172a7a11c12, tested on Ubuntu 22.04.

Feels like a race that would be better being explicitly handled in the correct order?

Yes, a PR with a better solution is welcome.
hebasto merged this on Oct 27, 2022
hebasto closed this on Oct 27, 2022
sidhujag referenced this in commit 961c8ce93b on Oct 28, 2022
john-moffett cross-referenced this on Jan 10, 2023 from issue Switch RPCConsole wallet selection to the one most recently opened/restored/created by john-moffett
hebasto referenced this in commit c71a96c431 on Jul 4, 2023
bitcoin-core locked this on Oct 27, 2023

Contributors

Labels

Linked (view graph)

#471 Add Wallet Restore in the GUI #696 Switch RPCConsole wallet selection to the one most recently opened/restored/created