ui: Support wallets loaded dynamically #13097

promag · 2018-04-26T17:31:28Z

Add support in the UI for wallets loaded dynamically.

promag · 2018-04-26T17:35:44Z

@ryanofsky had to convert interfaces::Wallet to shared pointer because of QueuedConnection. Do you have an alternative? Like instancing WalletModel on RPC thread and then make that the slot argument?

promag · 2018-04-26T18:22:58Z

Previous version in promag/2018-04-ui-wallet-loaded.1, now WalletModel* is the argument in the queued connection.

promag · 2018-04-26T18:23:19Z

@jnewbery mind testing this?

jnewbery · 2018-04-26T21:26:10Z

@jnewbery mind testing this?

Will do! (next week)

promag · 2018-04-26T21:27:01Z

In order to test, suppose there are wallets w1 and w2:

launch bitcoin-qt -regtest -nowallet
open console and run loadwallet w1 (should show wallet)
run loadwallet w2 (should show multi-wallet selector)

promag · 2018-04-26T23:04:42Z

src/qt/bitcoin.cpp

@@ -250,7 +251,8 @@ public Q_SLOTS:
    QTimer *pollShutdownTimer;
 #ifdef ENABLE_WALLET
    PaymentServer* paymentServer;
-    std::vector<WalletModel*> m_wallet_models;
+    std::list<WalletModel*> m_wallet_models;


Revert to vector.

promag · 2018-04-26T23:05:03Z

src/qt/bitcoin.cpp

+        window->setCurrentWallet(walletModel->getWalletName());
+    }
+
+    connect(walletModel, SIGNAL(coinsSent(WalletModel*,SendCoinsRecipient,QByteArray)),


Improve format.

jnewbery · 2018-04-30T16:00:57Z

Please update title to indicate that this PR is for GUI support for wallets loaded at runtime.

Please rebase on #10740 (which I just rebased on master) to pick up the wallet: Make vpwallets usage thread safe commit

promag · 2018-04-30T16:06:51Z

@jnewbery done.

jnewbery · 2018-04-30T18:08:15Z

Tested ACK c9048b9213f7e9dc9b8da9b06c2caccd27f8c3cc

jnewbery · 2018-04-30T20:56:41Z

This (and #13100) need to update doc/release-notes-pr10740.md to state that dynamic wallet loading is supported in the GUI.

promag · 2018-05-01T21:14:06Z

@jnewbery done (here).

meshcollider · 2018-05-02T09:13:11Z

Concept ACK

jnewbery · 2018-05-04T21:53:30Z

@promag - this requires a rebase for one of my commits. I've rebased #10740. I suggest you reset to that branch and then cherrypick your commits.

promag · 2018-05-04T22:01:13Z

@jnewbery rebased.

jonasschnelli · 2018-05-17T20:01:25Z

utACK 0e674ba though I would prefer if the glitches would be fixed before merge

jnewbery · 2018-05-17T20:13:25Z

the initial "default" wallet did had no name in the dropdown.

Yes - this is a result of #11687 (comment) , and is also visible in the listwallets and getwalletinfo RPC calls.

This PR does introduce that behaviour, but does make it more visible when loading a second wallet.

promag · 2018-05-17T23:27:23Z

@jonasschnelli launching with bitcoin-qt -regtest -wallet -wallet=w1 also adds an empty item to the checkbox.

promag · 2018-05-17T23:48:08Z

@jonasschnelli care to test again? Will squash after that.

jonasschnelli · 2018-05-18T08:10:07Z

Tested ACK 2e75134

2e75134 fixup! ui: Support wallets loaded dynamically (João Barbosa) 0e674ba ui: Support wallets loaded dynamically (João Barbosa) 1c8fe0b ui: Remove unnecessary variable fFirstWallet (João Barbosa) Pull request description: Add support in the UI for wallets loaded dynamically. Tree-SHA512: 4016d61580b31e28c49861b1cb0e77fac5417f9676a6ce6156be28cb6059fdf3d3dd4d57dbbc22a574ad428c2a4a3702aedca596a84e644ce148e1084feb29c9

ken2812221 · 2018-06-19T15:41:29Z

If wallet failed to load because of pruned mode, the dropdown menu shows the wallet.

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in #13063 (80b4910) where #13097 made possible to get "hit" by that bug. Reported by @ken2812221 (#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

Summary: 2e75134 fixup! ui: Support wallets loaded dynamically (João Barbosa) 0e674ba ui: Support wallets loaded dynamically (João Barbosa) 1c8fe0b ui: Remove unnecessary variable fFirstWallet (João Barbosa) Pull request description: Add support in the UI for wallets loaded dynamically. Tree-SHA512: 4016d61580b31e28c49861b1cb0e77fac5417f9676a6ce6156be28cb6059fdf3d3dd4d57dbbc22a574ad428c2a4a3702aedca596a84e644ce148e1084feb29c9 Backport of Core PR13097 bitcoin/bitcoin#13097 Depends on D4245 This dependency is a bug fix. Test Plan: make check Create a new wallet first ./bitcoind ./bitcoin-cli createwallet "testwallet" ./bitcoin-cli -rpcwallet="testwallet" getwalletinfo kill bitcoind The `getwalletinfo` rpc should output something like this: { "walletname": "testwallet", "walletversion": 200300, "balance": 0.00000000, "unconfirmed_balance": 0.00000000, "immature_balance": 0.00000000, "txcount": 0, "keypoololdest": 1570729970, "keypoolsize": 1000, "keypoolsize_hd_internal": 1000, "paytxfee": 0.00000000, "hdmasterkeyid": "e0c38684642d0301c5db7f5e2a43ea3a84764fec" } Open bitcoin-qt and then load the new wallet ./bitcoin-qt Help -> Debug -> Console loadwallet "testwallet" In the upper left corner under the menu bar, use the drop-down menu labeled `wallets` to select `testwallet`. In the console input: getwalletinfo This should output the same as above Close the console window. In the upper right corner of the main menu, use the drop-down menu labeled `wallets` to select `testwallet`. The window should now display the information for `testwallet`. Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D4236

@ken2812221

Summary: 3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in #13063 (80b4910) where #13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin/bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f Backport of Core PR13506 bitcoin/bitcoin#13506 Depends on D4236 Test Plan: make check bitcoin-qt --regtest --nowallet -usehd=0 Help -> Debug -> Console loadwallet wallet.dat Before this patch, the gui would throw an error window, but the wallet would still be selectable from the multiwallet drop down menu and the wallet information would still be displayed on the main window. After this patch, the gui should throw an error window and there should still be no wallet loaded and/or displayed in the main window. In both cases, the console will output `Wallet loading failed. (error code -4).` Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4238

2e75134 fixup! ui: Support wallets loaded dynamically (João Barbosa) 0e674ba ui: Support wallets loaded dynamically (João Barbosa) 1c8fe0b ui: Remove unnecessary variable fFirstWallet (João Barbosa) Pull request description: Add support in the UI for wallets loaded dynamically. Tree-SHA512: 4016d61580b31e28c49861b1cb0e77fac5417f9676a6ce6156be28cb6059fdf3d3dd4d57dbbc22a574ad428c2a4a3702aedca596a84e644ce148e1084feb29c9

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

2e75134 fixup! ui: Support wallets loaded dynamically (João Barbosa) 0e674ba ui: Support wallets loaded dynamically (João Barbosa) 1c8fe0b ui: Remove unnecessary variable fFirstWallet (João Barbosa) Pull request description: Add support in the UI for wallets loaded dynamically. Tree-SHA512: 4016d61580b31e28c49861b1cb0e77fac5417f9676a6ce6156be28cb6059fdf3d3dd4d57dbbc22a574ad428c2a4a3702aedca596a84e644ce148e1084feb29c9

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

@ken2812221

3a03d2a Qt: load wallet in UI after possible init aborts (Jonas Schnelli) Pull request description: Bug was introduced in bitcoin#13063 (80b4910) where bitcoin#13097 made possible to get "hit" by that bug. Reported by @ken2812221 (bitcoin#13097 (comment)). Dynamically loading a wallet informs the UI (and therefore makes the instance accessible) about the new wallet before all possible error cases where handled. Easy to reproduce by starting `bitcoin-qt --regtest --nowallet -usehd=0` then in the console enter `loadwallet wallet.dat`. This PR will make sure only correctly initialised (loaded) wallets will appear in the UI. Tree-SHA512: 3139545e852d53b117182b579f45259c198d1c25c1a6fa4e0108f942d45f6fe2691e6bfcbbae2e18c33ad0174a520f379c17867b1eb87f950d830a5f519fec4f

fanquake added the Wallet label Apr 26, 2018

promag force-pushed the 2018-04-ui-wallet-loaded branch from a06c0ee to 11b76ed Compare April 26, 2018 17:33

fanquake added the GUI label Apr 26, 2018

promag force-pushed the 2018-04-ui-wallet-loaded branch from 11b76ed to 4f3c063 Compare April 26, 2018 18:19

promag force-pushed the 2018-04-ui-wallet-loaded branch 2 times, most recently from eb495bb to e15724a Compare April 26, 2018 20:53

promag mentioned this pull request Apr 26, 2018

gui: Add dynamic wallets support #13100

Closed

promag commented Apr 26, 2018

View reviewed changes

promag force-pushed the 2018-04-ui-wallet-loaded branch from e15724a to aee425d Compare April 27, 2018 08:30

promag changed the title ~~Support wallets loaded dynamically~~ ui: Support wallets loaded dynamically Apr 30, 2018

promag force-pushed the 2018-04-ui-wallet-loaded branch from aee425d to c9048b9 Compare April 30, 2018 16:06

jnewbery mentioned this pull request Apr 30, 2018

Dynamic wallet load / create / unload #13059

Closed

10 tasks

promag force-pushed the 2018-04-ui-wallet-loaded branch from c9048b9 to 92f27ed Compare May 1, 2018 21:12

laanwj added this to Blockers in High-priority for review May 3, 2018

jnewbery added this to In progress in Multiwallet support May 3, 2018

promag force-pushed the 2018-04-ui-wallet-loaded branch from 92f27ed to ab8b66c Compare May 4, 2018 21:58

ui: Remove unnecessary variable fFirstWallet

1c8fe0b

fixup! ui: Support wallets loaded dynamically

2e75134

jonasschnelli merged commit 2e75134 into bitcoin:master May 18, 2018

laanwj removed this from Blockers in High-priority for review May 18, 2018

ryanofsky mentioned this pull request May 18, 2018

Add BitcoinApplication & RPCConsole tests #11625

Merged

promag deleted the 2018-04-ui-wallet-loaded branch May 20, 2018 17:29

Bushstar mentioned this pull request May 21, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#334

Merged

fanquake moved this from In progress to Done in Multiwallet support Jun 1, 2018

jonasschnelli mentioned this pull request Jun 19, 2018

Qt: load wallet in UI after possible init aborts #13506

Merged

PastaPastaPasta mentioned this pull request Jul 15, 2020

Backport 12610 + PrivateSend multiwallet support. dashpay/dash#3604

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: Support wallets loaded dynamically #13097

ui: Support wallets loaded dynamically #13097

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018

jnewbery commented Apr 26, 2018

promag commented Apr 26, 2018 •

edited

promag Apr 26, 2018

promag Apr 26, 2018

jnewbery commented Apr 30, 2018

promag commented Apr 30, 2018

jnewbery commented Apr 30, 2018

jnewbery commented Apr 30, 2018

promag commented May 1, 2018 •

edited

meshcollider commented May 2, 2018

jnewbery commented May 4, 2018

promag commented May 4, 2018

jonasschnelli commented May 17, 2018

jnewbery commented May 17, 2018

promag commented May 17, 2018

promag commented May 17, 2018 •

edited by jonasschnelli

jonasschnelli commented May 18, 2018

ken2812221 commented Jun 19, 2018

ui: Support wallets loaded dynamically #13097

ui: Support wallets loaded dynamically #13097

Conversation

promag commented Apr 26, 2018 • edited

promag commented Apr 26, 2018 • edited

promag commented Apr 26, 2018 • edited

promag commented Apr 26, 2018

jnewbery commented Apr 26, 2018

promag commented Apr 26, 2018 • edited

promag Apr 26, 2018

Choose a reason for hiding this comment

promag Apr 26, 2018

Choose a reason for hiding this comment

jnewbery commented Apr 30, 2018

promag commented Apr 30, 2018

jnewbery commented Apr 30, 2018

jnewbery commented Apr 30, 2018

promag commented May 1, 2018 • edited

meshcollider commented May 2, 2018

jnewbery commented May 4, 2018

promag commented May 4, 2018

jonasschnelli commented May 17, 2018

jnewbery commented May 17, 2018

promag commented May 17, 2018

promag commented May 17, 2018 • edited by jonasschnelli

jonasschnelli commented May 18, 2018

ken2812221 commented Jun 19, 2018

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018 •

edited

promag commented Apr 26, 2018 •

edited

promag commented May 1, 2018 •

edited

promag commented May 17, 2018 •

edited by jonasschnelli