UX: Add warning to "Create account"

[mryellow]

We see endless threads here from this UX failure causing people to lose funds.

It's not them, it's MetaMask.

In the Bitcoin world people are used to the paradigm of a seed phrase being used to generate multiple addresses. With a total lack of any explaination Metamask leaves people with the impression they are generating a new address they will be able to recover. edit: They are!!

Later MetaMask bugs and they reinstall, only to discover they have lost their funds edit: to re-create a derived key.

• "You have generated a new Private/Public Key pair. Please write down this Private Key for your records. Failure to do so MAY RESULT IN LOST FUNDS and is your sole responsibility!!"

[mryellow]

I note responses in these threads saying "Reinstall and hit the Create Account button again", while some appear claim that hasn't helped them.

It seems mnemonic phrase is designed to be a seed for "Create Account"

• A keychain, or keyring, controls many accounts with a single backup and signing strategy.
  • For example, a mnemonic phrase can generate many accounts, and is a keyring.

https://github.com/MetaMask/metamask-extension/blob/a45cb754358ff798dce25fa0b44d6b182abc7692/app/scripts/metamask-controller.js#L412-L419

https://github.com/MetaMask/metamask-extension/blob/a45cb754358ff798dce25fa0b44d6b182abc7692/app/scripts/metamask-controller.js#L494-L513

https://github.com/MetaMask/metamask-extension/blob/a45cb754358ff798dce25fa0b44d6b182abc7692/app/scripts/lib/seed-phrase-verifier.js#L27-L53

https://github.com/MetaMask/eth-simple-keyring/blob/4103f7f26a9d72f75cb4ef520abb6c0146b37772/index.js#L38-L46

Note the use of Wallet.generate() rather than new Wallet(priv, pub) which may have been intended, assuming we're "generating" based on a seed phrase. https://github.com/ethereumjs/ethereumjs-wallet/blob/d483e5ee3146aab5aeb60afbb5e2040e4c3b3bb8/index.js#L18

Using ethereumjs-wallet:

https://github.com/ethereumjs/ethereumjs-wallet/blob/d483e5ee3146aab5aeb60afbb5e2040e4c3b3bb8/index.js#L51-L63

Looks to be using crypto.randomBytes(32) rather than anything associated with a "mnemonic phrase".

Am I reading this right? Documented as all being based on the seed/mnemonic phrase but then generated randomly?

[mryellow]

MetaMask/eth-simple-keyring doesn't appear to be the code I should be looking at.

metamask-controller.addNewAccount() -> const keyState = await keyringController.addNewAccount(primaryKeyring)

const KeyringController = require('eth-keyring-controller')

https://github.com/MetaMask/KeyringController

https://github.com/MetaMask/KeyringController/blob/168c576ddc5bd7c33a449aed856fc62a4a849aaa/index.js#L85

Which is calling addAccounts() on either:

const SimpleKeyring = require('eth-simple-keyring')
const HdKeyring = require('eth-hd-keyring')

So this works as intended?

How can it be improved?

The suggested warning is incorrect wording, although they should probably backup the private key.

[bdresser]

@mryellow the hd wallet will generate the same accounts in the same order every time.

the basic experience is this: (1) User installs metamask, automatically starts with Account 1, gets seed phrase (2) User creates more accounts (Account 2, Account 3, etc) from within Metamask (3) User's local data gets wiped (4) User restores their accounts from seed phrase. After doing so, they only see Account 1 in the account list. (5) User "creates" accounts from within Metamask again. The accounts they create this time will the same ones they created before. That is, the Account 2 that is created after restore has the same address (and therefore same ETH and tokens) as the Account 2 created before the restore.

you're totally right though that we should add a warning to the "Create account" screen to help make this clear.

[mryellow]

I had to delve deep into the code to approach understand this basic functionality.

It needs explaining somewhere, perhaps different wording in the UI. It's very hard to come to the realisation that these are "Derived keys".

[mryellow]

"Derived keys" is so accurate but also so clunky and technical. Brain-storming some semantics which may or may not work.

• "derived"
• "from seed".
• "sub-"
• account vs address (this is a semantic issue more broadly which cross sometimes and is mentioned in another issue here I can't find)

Perhaps something like "from seed" is doable, though "Create Account from Seed" is long-winded and that sort of language would have to wait and appear on the next screen.

[bdresser]

also related to #3034 – if we auto-detect tokens, we can auto-detect if the user has generated any accounts with value on them

[mryellow]

if the user has generated any accounts with value on them

That was an issue with the other day with that tweet where someone thought they had generated a key held by someone else, but it was a derived key of their own they had used previously.