bitcoinjs / bip39

JavaScript implementation of Bitcoin BIP39: Mnemonic code for generating deterministic keys
ISC License
1.09k stars 444 forks source link

Version 3 #48

Closed dcousens closed 6 years ago

dcousens commented 7 years ago

Thoughts any/everyone?

prayerslayer commented 7 years ago

I don't know what your goals are, so it's a bit hard to assess these proposals. From top of my head:

Buffer APIs only

There is no Buffer in the browser, only ArrayBuffer and typed arrays (views on ArrayBuffers). Could be that it can be shimmed cheaply, I'm not sure.

safe-buffer

Depends on the Node versions you want to support. Personally I'd not use it.

Removal of mnemonicToEntropy and entropyToMnemonic from the public API

What is the reasoning behind this?

dcousens commented 7 years ago

Could be that it can be shimmed cheaply, I'm not sure.

Buffer is ArrayBuffer/UInt8Array in the browser. It simply has a few methods tacked on for encoding, decoding, serialization and other Buffer operations.

Depends on the Node versions you want to support. Personally I'd not use it.

I suppose we could drop node <4.5.0... probably simplest.

What is the reasoning behind [Removal of mnemonicToEntropy and entropyToMnemonic]

Simplification - I don't know if they are being used - if anyone feels otherwise we can keep them.

prayerslayer commented 7 years ago

Buffer is ArrayBuffer/UInt8Array in the browser

Sure, I'm just saying that you can't expect new Buffer() to work in the browser, unless you include shims. Typed arrays however are supported on both platforms and you could use them directly.

I suppose we could drop node <4.5.0... probably simplest.

0.x is EOL and in between there was iojs :) So yeah, definitely drop everything < 4 https://github.com/nodejs/LTS

Simplification - I don't know if they are being used - if anyone feels otherwise we can keep them.

My company has a use case for mnemonic -> entropy, so I'd be happy if you keep it around :grin:

dcousens commented 7 years ago

My company has a use case for mnemonic -> entropy, so I'd be happy if you keep it around

@prayerslayer you probably should be using mnemonicToSeed, not mnemonicToEntropy?

0.x is EOL and in between there was iojs :) So yeah, definitely drop everything < 4 https://github.com/nodejs/LTS

4.5.0 isn't < 4, specifically

@prayerslayer could you elaborate on why you use mnemonicToEntropy?

@fanatid I think we need to specify \u300 in the Japanese WORDLIST as the default delimiter for us to be able to drop the wordlists from the default export

fanatid commented 7 years ago

@dcousens sorry, I do not understand, what's wrong with Japanese wordlist?

dcousens commented 7 years ago

@fanatid it requires the \u300 separator instead of ' '.

fanatid commented 7 years ago

agrh, I see... we can store seprator with wordlist...

dcousens commented 6 years ago

@prayerslayer could you elaborate on why you use mnemonicToEntropy?

Added a checkbox that we should change to an async API to support high performance pbkdf2, as recommended in https://github.com/crypto-browserify/pbkdf2/pull/75

AFAIK, only mnemonicToSeed would need to be async...

@fanatid

agrh, I see... we can store seprator with wordlist...

Any thoughts on how we should?

dcousens commented 6 years ago

@fanatid

agrh, I see... we can store seprator with wordlist...

Any thoughts on how we should?

fanatid commented 6 years ago

Any thoughts on how we should?

{
  separator: '...',
  words: [...]
}

?

dcousens commented 6 years ago

@fanatid we will need to re-write the download script

dcousens commented 6 years ago

ping @prayerslayer ?

dcousens commented 6 years ago

@jkandzi @oed I noticed you use mnemonicToEntropy, can you elaborate on why it was required compared to using the seed bytes?

oed commented 6 years ago

@dcousens seed bytes is double the size which annoying if you want to keep size down. Last time I played around with it mnemonicToSeed behaved weirdly when going back and forth between seed and mnemonic.

dcousens commented 6 years ago

@oed was truncating the seed not an option?

oed commented 6 years ago

Right, now I remeber. There is no way to go from seed -> mnemonic. The reason I want this is because I want to split the entropy into multiple parts using shamirs secret sharing. Then I can give these parts to some delegates. When I recreate the entropy I want to be able to get the mnemonic again.

dcousens commented 6 years ago

@oed do you ever need mnemonicToEntropy? Or only entropyToMnemonic?

(thanks for this information btw)

oed commented 6 years ago

I need both. In case a user want's to recover with mnemonic and then split this and give to delegates. Thank you for asking for feedback! :+1:

dcousens commented 6 years ago

See https://github.com/bitcoinjs/bip39/pull/74