Create wallet menu option (
gui) May 22, 2019
Bitcoin uses QT for its GUI. The minimum version is 5.5.
Online documentation. This PR is one of the last in a series of multiwallet PRs. The first of those was
#8694 which added multiwallet support through the RPC interface and was merged in June 2017. Adding the various multiwallet functions (create/open/close wallet) at runtime is tracked by
Issue 13059. We already have UI support for opening and closing wallets. This PR adds UI support for creating wallets.
Creating wallets dynamically is relatively new functionality (merged in
#13058 in June 2018). There have been a couple of additional options added to the
createwallet RPC since it was added:
Disable private keys, Blank wallets and born-encrypted wallets. This PR was opened before the born-encrypted wallets PR was merged, so potentially could be simplified now that the functionality exists within the wallet component.
For GUI PRs, I always like to build and manually test the PR to get a sense of what the new UI functionality feels like. If you’re testing on the same machine that you run your mainnet node on, REMEMBER TO MOVE OR BACKUP YOUR WALLET/DATADIR BEFORE TESTING!
Feel free to leave feedback on UI PRs about usability as well as comments on the code.
7 13:03 <pinheadmz> buenos dias
9 13:03 <michaelfolkson> What up. Head spinning trying to work out how all these PRs overlap lol
12 13:04 <jnewbery> So rather than me talking for 5 minutes now, let's just get straight into questions
13 13:04 <jonatack> is no testing for qt a thing
14 13:05 <jnewbery> Most qt PRs don't include tests
15 13:05 <jnewbery> But there are some in src/qt/test
16 13:06 <MarcoFalke> Yeah, and those are more like gui unit tests, not actual gui tests
17 13:07 <MarcoFalke> Currently the only way to test the GUI is by running it by hand
18 13:08 <michaelfolkson> So to confirm I understand what is going on. You can currently create a new independent HD wallet using bitcoind but until now you couldn't using Qt. Qt can't leverage bitcoind RPC so the C++ code needs to be written afresh for Qt in addition to editing the GUI?
20 13:10 <jnewbery> You can see that it's mostly just parsing arguments and then calling CWallet::CreateWalletFromFile() which does the heavy lifting
21 13:11 <jnewbery> This PR adds a new method to the node interface called createWallet() which basically does the same stuff
23 13:12 <jonatack> thanks! i suppose then that units are added for any new non-qt code. seem might still be good to add e2e tests for the user flows.
24 13:12 <jonatack> unit tests
25 13:12 <jnewbery> jonatack: Yes, I think that would be valuable contribution
26 13:12 <jonatack> end to end functional ones?
29 13:13 <jnewbery> I don't know anything about testing QT
30 13:13 <jnewbery> But I imagine there's documentation on the QT website
31 13:13 <MarcoFalke> Indeed, I'd welcome any progress in that direction
32 13:13 <jnewbery> michaelfolkson: to follow up on my comment in the review notes "This PR was opened before the born-encrypted wallets PR was merged, so potentially could be simplified now that the functionality exists within the wallet component.
33 13:13 <MarcoFalke> I don't know if there are existing framworks
36 13:16 <michaelfolkson> <jnewbery>: Thanks, that's useful. I wasn't expecting to see any C++ code for this PR, I was expecting to just be looking at GUI design. But I forgot that Qt wasn't just a front end
37 13:16 <jnewbery> I'm not entirely sure of that though. I haven't checked whether there are subtle differences in what logic is required for QT and RPC
38 13:17 <jnewbery> Yep, it's all C++
39 13:19 <jnewbery> Did everyone manage to build the PR and do some manual testing?
40 13:19 <jnewbery> Any questions about that?
41 13:20 <jnewbery> I guess that's it for today. Short meeting!
42 13:20 <jnewbery> Before we go, does anyone have requests for future PRs to cover?
43 13:21 <jonatack> i compiled but didn't run it yet because i didn't see it in root or source
44 13:21 <jonatack> any pointers ?
45 13:21 <jnewbery> jonatack: it should be at src/qt/bitcoin-qt
46 13:21 <jnewbery> if not, perhaps you're not configured to build the qt binary. Check the output from when you ran `configure`
47 13:22 <jnewbery> You should see: 'with gui / qt = yes'
48 13:22 <jonatack> all good
49 13:22 <emzy> I had no time to do it. Maybe I will check it later.
50 13:22 <michaelfolkson> For the creating blank empty wallets PR, what's the motivation for doing this? Importing a seed from elsewhere?
51 13:22 <jonatack> i never run qt, thanks for the tpi
53 13:22 <michaelfolkson> #15226
54 13:23 <jnewbery> michaelfolkson: by default, a new wallet will generate an HD seed
56 13:23 <jnewbery> if you want to import your own seed, you can create a blank wallet and then import
57 13:24 <jnewbery> another motivation: when you encrypt the wallet, it'll create a new seed, so if you created a wallet that wasn't blank, you'd have a seed from before encryption and then a seed from after encryption, which perhaps you don't want
58 13:25 <jnewbery> #15226 also made #15006 (Add option to create an encrypted wallet) much simpler
59 13:25 <jonatack> thanks marco
60 13:25 <ariard> I had a a more general question which is on travis build process, I often get a timeout error "Error! Initial build successful, but not enough time..." What's the process to do after that ? Force-push new branch tip?
62 13:26 <MarcoFalke> jonatack: et al ^
63 13:26 <ariard> Do this also mean things have been modified which make tests longer to run ?
64 13:26 <jonatack> MarcoFalke: nice
65 13:26 <jnewbery> ariard: Yes, I believe force-pushing the branch will trigger a rebuild on travis
66 13:27 <jnewbery> just run `git commit --amend` to update the timestamp and then `git force push`
67 13:27 <jonatack> today's pr has seen no activity since 3 months.
68 13:28 <ariard> jnewbery: thanks for tip!
69 13:28 <jonatack> does achow have too much on his plate, does he need more help
70 13:28 <jonatack> or is it not a priority
71 13:29 <jnewbery> It might just not be a priority for him. I know he has a PR for a descriptor-based wallet, which is large and more important
73 13:30 <jnewbery> In general, if you see PRs that haven't had attention from their author for a while, there's nothing wrong in offering to take over maintaining the PR
74 13:32 <jnewbery> any other questions?
75 13:32 <jonatack> MarcoFalke:
76 13:32 <jonatack> do you plan to resubmit a pr
77 13:32 <jonatack> for the f tests speedup
78 13:32 <jonatack> you had 2
79 13:32 <michaelfolkson> achow is overseeing the Bitcoin Core hardware wallet interface too isn't he? That is ongoing
80 13:33 <MarcoFalke> jonatack: Good question. Not sure if we are allowed to discuss other prs here
81 13:33 <MarcoFalke> jnewbery: ? ^
82 13:33 <jonatack> related to ariard's q
83 13:33 <jnewbery> Is it directly related to this week's PR? If not, I think #bitcoin-core-dev is the better place to discuss it (so all interested parties can see it)
85 13:35 <jonatack> qt is now running for me ty for the help jnewbery
86 13:36 <jnewbery> michaelfolkson: yes, achow maintains HWI
87 13:36 <jnewbery> jonatack: great!
88 13:36 <jnewbery> ok, unless there are any final questions, let's wrap it up.
89 13:37 <jnewbery> Thanks all! See you next week
91 13:37 <jonatack> cheers! thanks everyone
92 13:38 <michaelfolkson> A quick final question? :)
93 13:38 <jnewbery> go for it!
94 13:39 <jnewbery> michaelfolkson: was 'A quick final question?' the final question or is there a question after that question?
95 13:40 <michaelfolkson> So what are next steps for advancing this? Someone who understands the code for the GUI needs to oversee all the merge conflicts between these different PRs?
96 13:40 <michaelfolkson> Sorry yes
97 13:40 <jnewbery> Which different PRs? This one doesn't currently conflict with anything
98 13:40 <michaelfolkson> And there needs to be more feedback on the design. I saw Sjors gave some suggestions for what the UI should be
99 13:41 <michaelfolkson> I was seeing a bunch of conflicts on one of the PRs you linked to
100 13:42 <jnewbery> Yeah, i think it's still appropriate to give design feedback at this stage
101 13:42 <jonatack> michael: perhaps adding a recent review can kick the pr back into action
102 13:42 <jnewbery> I agree. achow will probably be more motivated to work on it if he knows there are people keen to review
103 13:44 <michaelfolkson> Ok cool. It is just design considerations now right?
104 13:44 <jnewbery> No, the code needs review too
105 13:44 <jnewbery> all aspects of the PR
106 13:45 <michaelfolkson> Ok understood, thanks
107 13:45 <michaelfolkson> Ok I'll let you go, many thanks
108 13:46 <jnewbery> ok, thanks again. See you all next time!