What characters may a Recovery Phrase contain?

The latest Parity Signer and Parity Fether generates recovery words that just contain lowercase letters.

But based on feedback here it seems we need to allow for capital letters in recovery phrases, or perhaps any character at all.

@ltfschoen Thank you for bringing this up

I'm not 100% certain about this but from testing out recovery phrases from existing accounts, I found the recovery phrases are definitely case sensitive and considering the raw wordlists themselves are strictly lowercase as well, I would say the ideal behavior is to strictly allow only lowercase words.

If a user types any characters with capitals in it, we would automatically convert it to lowercase, as you said in this issue.

Considering the wordlists have never included uppercase letters (as far as I've seen), I would conjecture that they never have and never will though someone who's been on top of this for longer @amaurymartiny @jacogr @Tbaut would surely have a more definite answer.

We are using the bip39 NPM package to generate the recovery words.

If you want to use it then just install Node.js, clone Fether and install its dependencies with yarn install so the BIP39 package is installed into Fether's node_modules/ directory, then run node to start the REPL, then require BIP39 with const bip39 = require('./node_modules/bip39/index.js');,
then enter bip39. and press tab a couple of times and it will show you the different methods that are available

Note: If you install the BIP39 package globally (i.e. npm install -g bip39) you just require it with const bip39 = require('bip39');

Then you can check if a given recovery phrase (mnemonic) is valid, and whether making the first letter a capital letter makes a difference (see Example):

If you run bip39.wordlists then it will display all the different words that it may generate (including non-English word lists). It defaults to the English word list, and it appears that that's the list we're using, and it just contains all lowercase letters.

We're generating the recovery phrase in here in Fether by running bip39.generateMnemonic(),

We also need to support novasamatech/parity-signer#154 (so not just recovery phrases generated by Parity Signer or Parity Fether, which may include non-English word lists), support Substrate/Polkadot novasamatech/parity-signer#113

Note that in Substrate/Polkadot the mnemonic is an editable field

@ltfschoen Is that a greenlight regarding automatically converting words typed into recovery phrase to lowercase?

I have added the BIP English wordlist next to the Parity Wordlist as how I interpreted the requested feature is to alert the user if the last word they typed is invalid, as they type it.

validatemnemonic() will validate the entire mnemonic, but I am adding a feature to check if the last word is contained in either wordlist. If the first word they typed is a BIP39 word, then all words thereafter are expected to be BIP39 as well, otherwise it will pop up a warning, and same for if the first word is a Parity word.

Regarding paritytech/parity-signer#154 I suppose the least effort way would be to do the same thing and import the other language wordlists as well.

@ltfschoen Is that a greenlight regarding automatically converting words typed into recovery phrase to lowercase?

I have added the BIP English wordlist next to the Parity Wordlist as how I interpreted the requested feature is to alert the user if the last word they typed is invalid, as they type it.

validatemnemonic() will validate the entire mnemonic, but I am adding a feature to check if the last word is contained in either wordlist. If the first word they typed is a BIP39 word, then all words thereafter are expected to be BIP39 as well, otherwise it will pop up a warning, and same for if the first word is a Parity word.

Regarding paritytech/parity-signer#154 I suppose the least effort way would be to do the same thing and import the other language wordlists as well.

No it's not a green light. We should ask someone with more experience to help us answer those questions

Hey, so there are two ways of generating an address (in Fether's case, I mean for "recovering" an address) :

• Using BIP39 (requires BIP39 wordlist)
• Or using parity_phraseToAddress (legacy), which accepts any string input, but for Fether we want to restrict it to phrases from the Parity Wordlist (generated with parity_generateSecretPhrase). If a user generated his account using a custom phrase, we're ready to provide personal support and guidance for them to import the account as a JSON keyfile instead.

Accounts generated inside Fether use BIP39.

Right now, when the user enters a recovery phrase, we check if it's a BIP39 phrase, and if not we check if it's a Parity wordlist phrase.
BIP39 uses the index of the word in the wordlist, so in the check for BIP39 we can lowercase it before checking (assuming the wordlist file has the words in lowercase).
parity_phraseToAddress is case-sensitive, but since for it we only accept phrases from the Parity wordlist in Fether (which is lowercase), it's safe to lowercase it before checking.

If the first word they typed is a BIP39 word, then all words thereafter are expected to be BIP39 as well, otherwise it will pop up a warning, and same for if the first word is a Parity word.

There is overlap in the two wordlists, so I think it'd be better to pop up a warning if the words aren't all contained in the BIP39 English wordlist, nor are they all contained in the Parity wordlist.

I've just boarded a flight on route back to Berlin :) I've cloned the branch to review offline and will provide feedback tomorrow.

i have a presentation on Thurs that i need to urgently start preparing for so i won't be able to look at this until this Fri at the earliest

* User types in his phrase

At this point you expect the users to have copied their recovery-phrase in the first place whereas they have skipped the verification step most probably because they didn't want to copy it anywhere.

Also I think it's important to be able to let users view their recovery phrase in case they lose their backup. JSON are more complex to handle.

they have skipped the verification step most probably because they didn't want to copy it anywhere

Ah, then I think we disagree on this. My initial thought was: you can skip the rewrite phrase step, because we're not your teachers here to make sure you've done your homework right. Just skip, but we'll show your account as not secured, because we can't prove that you've backed up your account.

I.e. for me, on account creation, we expect you to copy your phrase.

Also I think it's important to be able to let users view their recovery phrase in case they lose their backup

I agree, but I would do that via RPC (related to #148). I'm just not convinced we should ever store the private keys inside localStorage.

Thinking about it, I agree security-wise that it makes sense not to store the (albeit encrypted) private keys in the localstorage. I guess that means not being able to show the user their phrase ? Not sure how you're thinking of doing that with RPC, since the node doesn't know the phrase of the account, only the derived private key (or did you mean using the RPCs to encrypt the passphrase with the account's private key? seems a bit complex and might introduce vulnerabilities)

Right, thanks guys. We need to find a secure way to do this or not do it at all.

Not sure how you're thinking of doing that with RPC, since the node doesn't know the phrase of the account, only the derived private key

Hmm yes, I wasn't aware of that. So that's kind of problematic.

Just to clarify, I think it's a bad idea to store the (albeit encrypted) seed phrase on the client side in general (whether it be localStorage, filesystem or something else). fether is a UI to the node, the latter securely managing the accounts, private keys, etc. I'd say let's keep this model of Fether not managing anything sensible and being (to an extent) pure UI. If we store the encrypted seed phrase in Fether then Fether isn't just a UI anymore, and we introduce a second point of vulnerability

What about we do what Status does and we keep the RP in localstorage up until the user confirms they copied it, then we remove it entirely? This adds more meaning to having the Warning too. I agree it is not ideal to have the phrase in local storage at all but that's the risk the user opts into by not copying it somewhere safe on account creation.

https://user-images.githubusercontent.com/6559881/41660909-b12273bc-749d-11e8-9b60-731f0a703a45.jpg

I'm talking with Kirill to see what he thinks. I'm not sure it'd be a good idea because the user are not aware of the fact that their wallet is less secure. All they are aware of, is that they could lose their money if they don't backup the phrase, which a totally different story. Recovery phrases could very well be stored for a long period of time, be stolen, then no matter what the users do, such as backing it up, they are still at risk of stolen funds..

We need a RPC that gives a BIP39 phrase from an account and password, but this doesn't exist afaik.

What should we do with this PR then? Apart from the localStorage, it's a great PR UI- and UX-wise.

I'd propose again the flow I described in #400 (review). imo it's a better UX than the current one (=forcing user to copy phrase), and we get to keep all the React components YJ added.

What you proposed is an interesting yet disturbing flow IMHO. The only difference with the current flow is that we bother the users later on to make sure they have originally written down the recovery phrase correctly.

If we oblige them to correctly write it down in the first place anyway, no need to wait for the verification. I don't see the added value in delaying the timing of this verification because we can't provide the phrase anymore.

Also what happens if they haven't correctly written down the phrase and only realize later on? They're left in an situation where they have to move their funds, that's frustrating.

If we oblige him to correctly write it down in the first place anyway

That's actually the step I want to avoid. I remember hating MetaMask with their "click on the words in the right order" step which you couldn't skip, thinking that I didn't need them to babysit me.

If we don't add a "Skip" button and disable copy/pasting on the field, then I believe that'll be a super annoying step for most Fether users, which imo are already semi-experienced crypto-enthusiasts who know what they're doing.

I'm okay with only the Skip button (+ warning popup), and removing the "Backup later" step, which I agree can be disturbing and often quite useless.

I'm okay with only the Skip button (+ warning popup), and removing the "Backup later" step, which I agree can be disturbing and often quite useless.

Right, I'm also fine with that, given the warning screen.

Closing as we won't implement it any time soon.

😿