keybase / warpwallet

A brain wallet generator that uses scrypt.
BSD 3-Clause "New" or "Revised" License
187 stars 61 forks source link

Output bip39 mnemonic suitable for import into HD wallets. #22

Open TechyShishy opened 7 years ago

TechyShishy commented 7 years ago

I find a need to generate a high quality HD wallet key for Electrum. The key stretching in warpwallet is perfect for this, but it doesn't output a seed suitable for importing into Electrum.

This fix should a suitable first measure in solving #13. Admittedly, using a mnemonic as the intermediate mechanism isn't great, but it is pretty portable. Most systems that accept bip32 and bip44 will accept bip39 as an input method. Additionally, tools like [https://iancoleman.github.io/bip39/ (https://iancoleman.github.io/bip39/) make interoperability between various systems easy.

dabura667 commented 7 years ago

FYI Electrum doesn't use BIP39. Only the word list.

TechyShishy commented 7 years ago

It has a toggle for BIP39 hidden in the advanced settings.

The eventual goal is to output BIP44 keys, but I haven't figured out how to do that yet. Outputting the seed is the super important part that can then be munged externally to make a useful address.

dabura667 commented 7 years ago

high quality HD wallet key

generate 128 bits of randomness from /u/random

The BIP39 clearly states that a minimum of 128 bits of entropy is to be used, but with the current way warp wallet is set up there is no way to guarantee the user's passphrase is 128 bits strong.

So while these phrases might be able to import into BIP39 wallets fine, they are not BIP39 compliant, which defeats the whole purpose of BIPs.

Don't call it BIP39 or advertise it as compatible with BIP39 wallets if you're going to keep it as is.

Possible fix: Add an entropy estimation of the user's password and forbid generation of BIP39 until the user's passphrase is greater than or equal to 128 bits. (you could take a few bits off to account for the hashing that warpwallet adds.)

lirazsiri commented 7 years ago

This is a great idea. Generating BIP39 mnemonic codes is one of the things that would make Warpwallet a lot more useful. The flip side is that we still need to maintain backwards compatibility with old Warpwallet versions so that future users don't lose access to their funds. Supporting the generation of both a single keypair and the seed for an HD wallet would increase tool complexity a bit.

While Electrum doesn't support BIP39 directly, another way to import into Electrum would be to convert the BIP39 into BIP32 using Ian Coleman's tool, with or without an additional passphrase: https://iancoleman.github.io/bip39/

Adding HD wallet support would also help mitigate privacy issues with address re-use, that are currently encouraged with Brainwallet.

casey commented 7 years ago

I checked a mnemonic generated by this PR against the online mnemonic tool here:

https://iancoleman.github.io/bip39/

The BIP 39 seed was: b7d0ec26414e1061a3c05546d7efbe675cb740270d0f1df836e46c227a1e8e9a

And the mnemonic was: result manual another little three cotton monitor apple egg satoshi usage sorry

However, when entering the mnemonic, the seed from the tool didn't match that from the PR.

Now, I'm not sure if the tool I used is correct, but the discrepancy should be accounted for.

Also, this PR should come with an update to the test vectors to include both the mnemonic and BIP 39 seed, to make sure that the derivations are correct and don't change.

dabura667 commented 7 years ago

@casey it's a difference in naming. the tool you linked is using "BIP39 seed" to refer to the 64 byte hash of the phrase. This PR is using the term to refer to the raw data that is to be encoded into the phrase.

Logicwax commented 7 years ago

I did this exact thing two years ago, take a look: https://github.com/Logicwax/PortalWallet

I also allowed the QR for import of the seed, as some wallets (like Jaxx) support this.

TechyShishy commented 7 years ago

@Logicwax Your implementation was the inspiration for my code. Unfortunately, because your variant compressed and rewrote a lot of the original warpwallet code, I was unable to update it with the new features and bugfixes included in the parent warpwallet. As a result, I adapted the idea directly into a PR against warpwallet.

lattice0 commented 6 years ago

Came here to suggest this. I wanted to create a shared wallet where one of the signers would be a warp generated private key, but Copay only accepts bip39 mnemonics to join shared wallets (or just create normal wallets). Please do it :)