Network: BIP44 indexes

[iancoleman]

See http://doc.satoshilabs.com/slips/slip-0044.html

[coveralls]

Coverage remained the same when pulling eec3ac2 on iancoleman:bip44_indexes into c96a1bc on bitcoinjs:master.

[dcousens]

Thoughts on using:

...
bip44: {
  index: 0
},
...

It is a bit superfluous, but, it does mean it'll be a kind of standard way of introducing these constants.

Otherwise, I'm +1.

[dcousens]

Also, @weilu, thoughts on including anything related to BIP44 considering it still isn't an accepted BIP?

[iancoleman]

Probably the object is more consistent. I tossed it over for a while and figured since the object was only going to have one property it should just be a property without the nesting. The nesting does has a certain sense of consistency about it with bip32 above. Thing is, there's no correlation between the two bips so the bip44 object 'looking consistent' is really just a fluke. Either way...

[dcousens]

You also get the advantage of checking for network compatibility by simply using if (network.bip44) rather than if (network.bip44Index !== undefined).
I personally prefer the consistency, even if it is a superfluous object.

[coveralls]

Coverage remained the same when pulling 37dbfd2 on iancoleman:bip44_indexes into c96a1bc on bitcoinjs:master.

[dcousens]

LGTM, squash and when weilu +1, good to merge.

[coveralls]

Coverage remained the same when pulling b18257d on iancoleman:bip44_indexes into c96a1bc on bitcoinjs:master.

[weilu]

We are not using the added constants anywhere. As @dcousens mentioned the BIP is still in draft, so I'd err on the side of putting everything bip44 in its own module (e.g. https://github.com/weilu/bip39).

[iancoleman]

@weilu now that you say it, I can see how these constants are better in their own module. At first it seemed to me that bip44 was too simple to need its own module, but given the various options for derivation paths, I can see how it could/should be a separate module. I'll close this since I agree these variables are better off elsewhere (albeit no current module exists). This possibly makes a case for moving the bip32 variables also, but that's a different thing to this pull request.

[dcousens]

@iancoleman you raise a valid point though.
Is BIP32 suitable to be in this module? Or should it be moved out?

Perhaps we could take a master repo approach npm install bips, then:

var bip32 = require('bips/bip32')
var bip39 = require('bips/bip39')
var bip44 = require('bips/bip44')
... etc

Where each dependency in that module is what we feel is the most standardized and well implemented (and naturally consistent with other modules) of the BIP modules?
Thoughts @weilu ?

[dcousens]

Thoughts @jprichardson

[weilu]

BIP32 is accepted, BIP39 and 44 aren't

[jprichardson]

1. BIP32 has a lot of complexity that it makes sense to stay in this module. And as @weilu stated, it's accepted.
2. BIP44 in its own module? Meh. There's not much to it.
3. Is it acceptable to have BIP44 constants in this module? I think so. As long as it's the bitcoin and bitcoin testnet only.
4. Having a master repo for all of the bips? No way. That's insanity. So many bips are unrelated. What would be cool though is a curated listed (one repo - sort of like the "awesome" repos) with links pointing to JavaScript implementations of the various bips.

[dcousens]

Having a master repo for all of the bips? No way. That's insanity. So many bips are unrelated. What would be cool though is a curated listed (one repo - sort of like the "awesome" repos) with links pointing to JavaScript implementations of the various bips.

That is exactly what I was proposing, a repo that would allow you to require them easily, but didn't actually contain any code (beyond the index anyway). A curated list, as it were.

But otherwise, I'm OK with the awesome- style too.