Skip to content
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

Remove Wallet Name from early setup, move to just before "wallet encryption info" #45

Closed
docalb opened this issue Jul 24, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers priority: high issues raised or encountered by 2 or more testers

Comments

@docalb
Copy link

docalb commented Jul 24, 2023

During set up it asks to name wallet. 'Wallet' is already filled in. Any reason not to use 'Wallet'? If not, suggest new user keep 'wallet'

@BenWestgate
Copy link
Owner

BenWestgate commented Jul 24, 2023

Because you can make and restore multiple wallets on the same Bails USB and need a unique name to disambiguate them in Bitcoin Core.

Your first wallet named wallet is fine. But my plan for L2 is to have your wallet created by the current Bails be your "spending wallet" and another watch-only wallet be your "savings wallet".

Also not everyone speaks English. I see a lot of reasons to not force a name choice.

The final reason is your wallet name chosen is added to the user entropy that's securely mixed into the private key, along with threshold, locations, passphrase and their "add randomness text"

Anywhere the user could choose something it gets collected as entropy to strengthen the seed.

@BenWestgate BenWestgate added enhancement New feature or request wontfix This will not be worked on labels Jul 24, 2023
@BenWestgate
Copy link
Owner

After the identifier is changed to public masterkey fingerprint this question can be moved to the last step of Restore wallet before the info about encryption.

It still needs to be a pre-backup step of 'Create wallet'

@BenWestgate
Copy link
Owner

Naming the wallet should come after the mentioned dialog. The flow should be as follows:

Show the dialog with the choices: "Create a new seed," "I already have a seed," and "Import descriptors."

Based on the user's choice in the dialog, proceed accordingly:
a. If the user selects "Create a new seed," continue to the next step to ask for the wallet name.
b. If the user selects "I already have a seed" or "Import descriptors," proceed with the respective actions.

@BenWestgate
Copy link
Owner

Remove naming the wallet from "Create a new seed"

Because none of it has anything to do with a wallet and it doesn't matter what the wallet is named in that function block

The new wallet will get named after 'Restore wallet' has recovered seed and unique id

That way a user can name the wallet something meaningful based on the seed they created or restored.

And the default name can include the identifier if it needs to helping the user remember which shares are to their own wallet and which are to someone elses.

Cons to asking for wallet name first

it felt weird being asked to update the name due to collision, like minutes later after setting the name.

Grabbing the wallet name first for extra entropy in seed creation isn't that helpful, either they add enough entropy in the 'Add Randomness' step, or they don't really get enough from "name, threshold, locations, identifier" to be secure against a backdoor unless they rolled dice to make their passphrase and most people leave it at "Wallet".

Another reason it doesn't belong in 'Create a seed' is a custom wallet name cannot be recovered from the codex32 backup.

if you name it Purple Donkey, you can't have that name be automatically restored when you later choose "I have a seed"

meaning: wallet name is NOT part of "creating a seed backup"

Contextual decision: Users are more likely to make an informed decision about naming their wallet after understanding the options for seed creation. By providing them with the information about creating a new seed or using an existing one first, they can better comprehend the implications of their choices and how it may relate to the wallet name.

Reduced ambiguity: Naming a wallet without knowing whether it will be a new seed or an existing one can lead to confusion. Users might unintentionally give a name that doesn't match the type of seed they eventually use, which can cause confusion and make it harder to manage multiple wallets.

Streamlined flow: By first asking about seed creation, the flow becomes more streamlined and intuitive. Users will first decide on the core aspect of the wallet creation process (seed choice) and then move on to the secondary aspect (naming the wallet).

@BenWestgate BenWestgate added documentation Improvements or additions to documentation good first issue Good for newcomers priority: high issues raised or encountered by 2 or more testers and removed wontfix This will not be worked on labels Jul 27, 2023
@BenWestgate BenWestgate changed the title Wallet Name Remove Wallet Name from early setup, move to just before "wallet encryption info" Jul 27, 2023
@BenWestgate
Copy link
Owner

This has been resolved. It asks for the wallet name right after the seed is successfully restored. I'm not currently defaulting the identifier into the name unless there is a collision with the wallet name chosen. But I'm open to difference of opinion about this. Just open a new issue about it with your argument to make it default to say

"Wallet 7jsn" so you know exactly what shares belong to your wallet. That said, the information is in listdescriptors so it is unnecessary to to put it in the wallet name if that makes the shoulder surfing privacy better. Wallet names are not masked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers priority: high issues raised or encountered by 2 or more testers
Projects
None yet
Development

No branches or pull requests

2 participants