bitcoinjs / bip39

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

feat: defer/lazy load word lists #95

Open sesam opened 5 years ago

sesam commented 5 years ago

currently word lists are loaded all at start-up, while realistically the user only ever needs to use one at a time. The suggestion is to make word-lists initialize lazily, and only after the language choice has been made. This abstraction could be hidden in a separate js file to make the main file less verbose. (Imho 154 lines is verbose. You might have taller screens / bigger heads than me, YMMW :-p)

junderw commented 5 years ago

We've been throwing around similar ideas for a while, but it's low priority right now. Pull Requests welcome, though!

I was thinking of making it more generic and making an npm package for each wordlist.

junderw commented 5 years ago

@dcousens I wonder if a lazy loading Proxy might be good...

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy

  1. When initializing BIP39, no wordlist is loaded.
  2. When any attribute of bip39.wordlists is accessed a getter Proxy takes the name of the thing being gotten and then attempts to require('@bip39/' + name) and return it.
  3. If the parent app has name wordlist installed, require will find it and load it.
  4. Maybe add hash values for each wordlist to check against known values.

That way, a person looking to add bip39 with English only to their app will do

npm install --save bip39 @bip39/english

Then in their app:

const bip39 = require('bip39')
// bip39 currently has not loaded any wordlists into memory
bip39.mnemonicToSeed(mn, pass)
// doesn't throw an error and still no wordlist in bip39.wordlists. since it doesn't use wordlists.
bip39.entropyToMnemonic(crypto.randomBytes(16))
// this will perform wordlist = wordlist || bip39.wordlists.english
// which will have the Proxy of wordlists do require('@bip39/english') and return that. storing it within bip39.wordlists
// if the require fails, return undefined.

hmmm... I'll have to test it out.

dcousens commented 5 years ago

I don't think many packages is necessary. The only issue is the bundle burden, not npm.

junderw commented 5 years ago

I made a PR with one idea...