-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Design] New 'Create Wallet' user flow. #77
Comments
Seems reasonable, but a few comments:
|
General Concept ACK the "Encryption problem"Since it doesn't really "encrypt the wallet", we should use the term "encrypt private keys". Other than that, people think their transactions and additional client side comments are also encrypted. Users might think their wallet privacy is protected when storing it ("encrypted") in an unsafe place. |
Concept ACK. Agree with @jonasschnelli . The warning "You have not entered a passphrase ..." should probably not be added. See also bitcoin/bitcoin#18085 |
Everything looks good however I noticed that confirming wallet encryption form mentioned "bitcoins". Recently saw one video and also read at few places in past that "bitcoin" is used for both singular and plural. Then I realized its not used only in one form and used at lot of places: https://github.com/search?l=&q=BITCOINS+repo%3Abitcoin-core%2Fgui&type=Code One video in which Andreas mentioned about it: https://youtu.be/FYo5E7zT-vM?t=640 Not sure if it needs to be changed. Just an observation. |
I like the larger layout this design implements. It's much more clear than the tooltips. It also leaves room for an Advanced section at the bottom, where stuff like descriptor wallets (and perhaps an early HWI integration) could be handled. Agree with @achow101's point (2), but I would just skip the "encrypt private keys" step without explaining. It also needs to be easier to skip this step, e.g. by adding a "Skip" button between "Back" and "Create". I don't like using a blank input field as the method of indicating not wanting a password. Later on you could add a Recover / Import dialog (or option inside the dialog), where people can restore from a backup file, BIP39 mnemonic, bunch of xpubs. |
yes I should have mentioned this currently exists, though I don't think this method of explaining the wallet types is the best UX and the current tooltips info isn't as informative as they could be. Does this mean blank only wallets can also not be encrypted (because no pkeys are generated)? It would be handy if you could encrypt the watch only / blank wallets for privacy reasons though. I'll get the user flows up for the other wallet creation types soon. Instead of having descriptor wallets as an option in the first modal I think it would be better to have it in subsequent wallet creation steps as an advanced option as @Sjors mentioned, or just something that can be turned on after the wallet is created. I prefer this second method as to not clutter the wallet creation process / leave this to advanced users / avoid scaring beginners with more new terminology.
I agree with not having that modal, it isn't entirely unnecessary, my main motivation behind adding it was to push users towards encrypting as well prevent accidentally not encrypting (unlikely but could happen). Though I think the more descriptive encrypt wallet modal educates the user enough.
My mistake, I usually use Andreas terminology, similar to other commodities you wouldn't call gold golds or wheat wheats.
If watch only wallets are unable to be encrypted then yeah just skipping this step would be ideal, though giving watch only wallets the ability to be encrypted is good for privacy. I'll do a watch only and blank wallet user flow soon. I'll make it easier to skip this step and just have this as the 'no password option' rather than leaving the field blank. I'd rather leave the skip option as a tertiary button as to still lean towards users encrypting to encourage good OPSEC.
Yeah I would like to eventually have all of this as part of the flows here, ideally having the first modal as having the options to either create a new wallet or import existing (with xpub / pkeys). I'll get design mockups done for these in the coming weeks for feedback. |
Wallet encryption only encrypts private keys. No transactions, watch only scripts, or literally anything else in the wallet is encrypted. There are no privacy gains.
Absolutely not. Descriptor wallets must be a thing for wallet creation. In fact, I will give an automatic Concept NACK if it is not an option. Descriptor wallets is not a feature that can just be turned on and it is not reasonable to make people migrate a newly created legacy wallet to a descriptor wallet if they just wanted a descriptor wallet. Furthermore, migration will probably have some bugs and be horribly inefficient. |
I'm not too familiar with how the technical details work here, but what I had in mind is how say in Electrum you can encrypt your watch only wallet. How do they achieve this? Why is not possible on the core GUI? I know there is no keys generated to encrypt, but is there another way to conceal that information being easily accessible to someone simply opening core on your device?
Apologies, I am not too familiar with the technical details of how descriptor wallets work. I will include it in the flow and upload a new design - is this something you would be comfortable with under a advanced options dialogue in a modal after choosing your wallet type? |
Electrum encrypts more information, IIRC they do encrypt the entire wallet file. Bitcoin Core only encrypts the private keys.
Nope.
I guess advanced options is fine. I prefer that it is an option up front just so that we can promote people to use it more, but the option just needs to exist in a reasonably accessible place. |
I imagine at some point we make descriptor wallets the default and it's nice if that doesn't lead to a confusing UI change. A check box hidden in an Advanced section helps with that. In #4 I automatically detect if a hardware wallet is connected, in which case descriptor wallet is selected automatically. |
So what happens currently if you select 'blank only wallet' and 'encrypt wallet'? In this new modal, should it ask the user if they wish to encrypt private keys when (if) private keys are imported? |
An encryption key is added to the wallet. Any keys that are added to the wallet (either via import of |
What are the UI hurdles when descriptor wallets are a default for all wallets created with core? From my understanding it would actually improve UI/UX a bit (for example allowing xpub imports during the wallet creation process for watch only / blank wallets). |
Awesome to see this issue, and I like this direction a lot. A few thoughts on copy:
I think you can avoid repeating the wallet type from the beginning of these description sentences and just write "Generates a set of private keys...".
Encrypting the wallet is what the app does, not me. I only choose a passphrase. How about changing the name of this screen to "Choose a passphrase"?
How about "... you will lose access to..."?
A pretty long first sentence and lots of negation that makes both sentences hard to parse. How about something like "Do you want to continue with low security and accept the resulting risks of potentially losing your bitcoin?" Overall I prefer simple, clear, short sentences. Should be much easier to parse and understand, particularly by non-native English speakers. Good copy goes a long way. |
Cheers @GBKS I have integrated these changes into the figma file, agree with having as simple explanations as possible. |
I made a few modifications to the create dialog based on the above, but nowhere near a full implementation: #96 |
@achow101 Some people are using this to sell wallet.dat files Example: https://cointelegraph.com/news/1b-of-bitcoin-from-silk-road-wallet-moves-for-first-time-since-2015 The So I am assuming most of the wallet.dat files with screenshots sold at https://allprivatekeys.com/wallet.dat are created like this:
|
Nope, that's not how those wallets are made. If it were, you would see the balance as a watch only balance. Typically scammers make wallets like that by directly modifying the wallet.dat file to include a bogus |
Yes. Just noticed that it shows as "watch-only" if you import address in a blank wallet and "available" in a wallet with private keys disabled.
Interesting |
What if the ckey record included a signature of the encrypted data with the private key it claims to have? That would prevent scammers from inserting public keys they don't have, though it would leave the option open for them to insert a public key for which they have the private key (but don't insert it in the wallet file). |
Anything that we do now won't change the fact that prior wallets did not have the check, and because we have to maintain backwards compatibility, scammers would just use an old version of the wallet format which did not include signatures with the ckey. |
Backwards compatibility just requires support for opening these wallets, not for displaying the balance as if it were spendable. It could show the balance as unconfirmed or something, until the wallet is upgraded |
ac64cec gui: create wallet: add advanced section (Sjors Provoost) c99d6f6 gui: create wallet: name placeholder (Sjors Provoost) 5bff825 [gui] create wallet: smarter checkbox toggling (Sjors Provoost) Pull request description: Previously only users who needed a second wallet had to use to the create wallet dialog. With the merge of bitcoin/bitcoin#15454 now all new users have to. I don't think it was user-friendly enough for that. <img width="403" alt="Schermafbeelding 2020-09-18 om 09 41 44" src="https://user-images.githubusercontent.com/10217/93574129-52ef9680-f998-11ea-9a6f-31144f66d3bf.png"> This PR makes a few simple improvements so that new users don't have to think too much: <img width="369" alt="Schermafbeelding 2020-10-15 om 16 45 22" src="https://user-images.githubusercontent.com/10217/96145959-0c914700-0f06-11eb-9526-cf447d841d7a.png"> It's lightly inspired by #77. It would be better if those changes made it into the upcoming release, but this PR is a good start imo. * wallet encryption is no longer checked by default, because such a change in the default needs a separate discussion (fwiw, I suspect it increases the number of users losing access to coins) * watch-only and descriptor wallet stuff is moved to advanced, so new users know they can safely ignore these check boxes * bonus: when you click on "disable private keys" it disables encrypt wallet and checks blank wallet * label changes: see screenshot * tooltip changes: see code diff Note that a blank wallet name isn't allowed in the dialog; I haven't addressed that. _Update 2020-10-30_, dropped the new strings for now: <img width="450" alt="Schermafbeelding 2020-10-30 om 11 26 55" src="https://user-images.githubusercontent.com/10217/97694591-1b99fc80-1aa3-11eb-8b85-e19f1ad5add4.png"> ACKs for top commit: fjahr: Tested ACK ac64cec jonatack: re-ACK ac64cec, per `git diff d393708 ac64cec` only change since my last review is improving the placeholder from "MyWallet" to "Wallet" and dropping the last commit. Tested creating a dozen wallets in signet with different combinations of options and then verifying/comparing their characteristics in the console with getwalletinfo. My remaining caveats are (1) the need for less user surprise by either (a) improving the user info or (b) with less auto-(un)selecting as mentioned in #96 (comment) and (2) I prefer the "Encrypt private keys" and "Watch-only" wording and descriptions below over the current ones; hopefully these can be addressed in a follow-up. hebasto: re-ACK ac64cec ryanofsky: Code review ACK ac64cec. Only changes since last review are tweaking placeholder text and dropping "allow nameless" commit Tree-SHA512: a25f84eb66ee4f99af441d73e33928df9d9cf592177398ef48f0037f5913699e47a162cf1301c83b34501546d43ff4ae12607fd078c5c03b92f573bf7604a9f2
Overview of current and new create wallet user flow
1.0 Who is this being designed for?
I am under the assumption that currently the Bitcoin core GUI primarily caters to Bitcoin power users and or developers who are okay with usability trade offs for improved security and more robustly audited code. With the GUI being one of the most secure, simple ways to run a full node, and if a goal of the Bitcoin community is to have as many full nodes as possible (which I presume it is), then we need the GUI to cater to a wider, less technical audience.
The aim of my designs is to remove friction for beginner and power users without having any security and or privacy trade offs, as well as have as minimal technical hurdles. Primarily, however, my goal is to make the GUI more beginner friendly. The Bitcoin core GUI in my view should be a benchmark / reference implementation and should not aim to be a flashy node software like umbrel or mynode – but this does not mean usability improvements cant be made. A well designed GUI will also further improve the security and privacy of its users - for this reason core should put more emphasis on design implementations. With good design, both the beginners and power users will be able to send and receive bitcoin backed by their own full node using the GUI without trade offs.
2.0 Problems being solved
The current user flow for creating a new wallet does not explain to users the types of wallets you can create or how each wallet type is different. Furthermore, the types of wallets available to create use terminology that is likely unfamiliar to many users (e.g. ‘Disable private keys’ could just be named ‘Watch Only Wallet’ which is more common and describes the same kind of wallet) - Luke Jr brang up this concern in bitcoin/bitcoin#17879.
The encrypt wallet option should be part of the create new wallet user flow and not optional (but also not mandatory – see below). The current initial modal that is presented does nothing to educate the user of the importance and pitfalls (not recoverable) of encrypting your wallet. This information could be included in the first modal but to keep things minimal and clean this should focus on ‘Choose your wallet type’ options whilst ‘Encrypt your wallet’ should have its on modal to fully explain why/why not to encrypt. Having encrypt wallet as part of the user flow by default will likely lead to better security of users bitcoin – An example being users will be more likely encrypt their wallets if informed why they should, this will lead to less stolen (but possibly more lost) bitcoins. If we want beginners using the GUI (meaning many more nodes) simple content changes such as this are important. Some discussions around this occurred in bitcoin/bitcoin#17879.
A general clean up of the UI without any drastic changes was also made. The design scope is quite limited due to Qt Widgets inheriting most components from the host OS. A cleaner UI generally leads to a smoother user experience. Nothing too fancy was done here, mostly some hierarchical separation of text, cleaner inputs and more consistent padding.
From a wider GUI perspective I chose to focus on the create new wallet feature first as creating a wallet and creating it securely is one of the most important things a bitcoin user can do so I figured this was a top priority.
3.0 Solutions
Below are the designs I have made with comments, along with the current screens for comparison. You can also view the figma file (opens in your desktop fine) if you want to view the source files and more specific details of the design. You can leave comments on my figma file by clicking the speech bubble in the top left and clicking the area of the design you want to comment on. Designs are done using the WindowsOS Qt widgets inherited styling. Linux and Mac versions will inherit their own styling but the primary elements will be consistent.
3.1 Create Wallet Meta
This is the first modal that pops up when creating a wallet, it is quite vague what it is the user needs to do here #69. Here are some questions that users likely have but are unanswered of which I aimed to answer or avoid the user having in the above new design – Why do I encrypt my wallet? It is selected by default so it must be important, but why is it important? What does disable private keys mean and why would I want that? Why is it disabled by default? What is a blank wallet and why would I want that? What are these options anyways? When I click create what exactly am I creating?
For a cleaner more practical flow, the first modal should focus on the meta of wallet creation such as the name / wallet type and move secondary steps like encrypting to a subsequent modal. Several wallet types can be created which is not clearly communicated, nor is what each of those wallet types can do. I included a standard wallet option which should be selected by default as this is likely the most common wallet type users will create. The encrypt wallet option has been moved to its own modal and should be a default option for creating any type of wallet (details in design below).
Minor Changes
The wallet name input should be pre-filled in with text such as ‘MyWallet1’ for users who want to quickly make a wallet. If the user already has say one wallet then the pre-filled text should be ‘MyWallet2,’ they have two wallets ‘MyWallet3,’ etc. This creates a quicker user flow and better user experience. Electrum wallet does this nicely when creating a new wallet.
Removed the help button ‘?’ at the top – currently does nothing and the new design should clarify any confusion the user may have- #74.
3.2 Create Wallet Encryption
Currently the encrypt wallet option is a toggled feature in the first modal, but as explained above this modal should focus on wallet meta options like wallet type/name. The encrypt wallet modal should be presented to all users after picking a wallet type regardless of the wallet type chosen and should clearly emphasize the pros and cons of encrypting. It should also still be optional to not encrypt your wallet which can be done by leaving the passphrase input blank – though the warnings should push users more towards encrypting for better security / privacy. A modal should pop up (last frame shown above) confirming the user does not want to encrypt if they leave the passphrase input blank. bitcoin/bitcoin#17879.
Much of this emphasis on the pros and cons of encrypting is placed AFTER the user encrypts their wallet in subsequent modals which does not make sense – the emphasis should be placed before they decide to encrypt or not. I did this by including the details from the modals that usually pop up after entering a passphrase at the header of this modal. I also added a warning in red to clearly indicate that cons of encrypting. Lastly, I added a checkbox input that must be clicked before the ‘create’ button can be clicked which finishes the wallet creation process.
Minor Changes:
Moved the passphrase visibility toggle to icons on each passphrase input. This is a cleaner, more user friendly approach. Also made each input have their own toggle rather than a toggle that just shows both inputs when clicked like in the current design. These are two new icons not currently in the GUI that will need to be added to implement this design.
3.3 Create Wallet extra redundant modals
With a more detailed encrypt wallet modal the two subsequent modals (shown above) can be removed making the create wallet process more seamless and involve less steps.
Next Steps
This was mostly a re-modelling of what currently exists - no extra frames / steps have been added. However, going forward I would also like to setup a user flow in this create wallet section of the GUI for watch only wallets and blank wallets. Such as having the ability to enter a xpub for watch only wallets in the creation process rather than in the settings after the wallet is created like it currently is.
Looking forward to some feedback!
The text was updated successfully, but these errors were encountered: