search

iancoleman / bip39

A web tool for converting BIP39 mnemonic codes
https://iancoleman.io/bip39/
MIT License
3.57k stars 1.45k forks source link

Add zcash support #212

Closed xarakas closed 6 years ago

xarakas commented 6 years ago

A potential solution to issue #205

iancoleman commented 6 years ago

I really appreciate this PR being submitted.

Accepting this is not too clear to me since any upgrades to bitcoinjs-lib in the future will cause breakages / require modifications due to this. I don't like modifying libraries, especially key libraries such as bitcoinjs-lib. Extending them is one thing, modifying is perhaps a step too far.

On the other hand the maintenance is pretty minimal, especially compared to the significant changes proposed in bitcoinjslib-4.0.0, so maybe merging isn't such a big issue.

I'll have a think about how to approach this, but thanks again for submitting the PR.

iancoleman commented 6 years ago

@dcousens can I please get a second set of eyes on the relatively small change in bitcoinjs-lib in this PR? Can you see any obvious problems with the change? I'm not familiar enough with the internals of bitcoinjs-lib to make that judgement.

@xarakas I'm ok with managing the technical debt this imposes but want a second opinion on the technical aspects of the change.

I've confirmed the zcash parameters in this PR are valid by generating an xprv with this tool and importing it into https://guarda.co/web-wallet - it gives the same address, so looks good as far as I can tell.

dcousens commented 6 years ago

It doesn't look to be malicious.

It appears to be adding support for a 16-bit integer version support to the generalized network object type and toBase58Check.

I'd generally recommend just making another function toBase58CheckZcash or something.

Generalizing all of your networks to support 16-bit integers, shouldn't hurt, but it could, depending on how you use them.

manichandra commented 2 years ago

@iancoleman Hi, I am working to add zcash support in xchainjs and running into similar issue. I would really appreciate your help if you are available. https://github.com/xchainjs/xchainjs-lib/pull/454

Thank you