bitcoinjs / bip38

BIP38 is a standard process to encrypt Bitcoin and crypto currency private keys that is less susceptible to brute force attacks thus protecting the user.
http://cryptocoinjs.com/modules/currency/bip38/
MIT License
206 stars 100 forks source link

Add network params support #44

Closed sondreb closed 5 years ago

sondreb commented 5 years ago

Closes #20

Test result: image

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1aafca6f04b8a2f91e477b11cf2d3c6a1373517f on sondreb:feature/network-params-support into 8e3a2cc6f7391782f3012129924a73bb632a3d4d on bitcoinjs:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1aafca6f04b8a2f91e477b11cf2d3c6a1373517f on sondreb:feature/network-params-support into 8e3a2cc6f7391782f3012129924a73bb632a3d4d on bitcoinjs:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1aafca6f04b8a2f91e477b11cf2d3c6a1373517f on sondreb:feature/network-params-support into 8e3a2cc6f7391782f3012129924a73bb632a3d4d on bitcoinjs:master.

sondreb commented 5 years ago

A package that supports alternative networks is now published under the name "bip38-network": https://www.npmjs.com/package/bip38-network

The package will be unpublished if and when this PR is accepted and published to bip38, but if anyone else needs to use bip38 for other networks than Bitcoin, feel free to try it out.

junderw commented 5 years ago

I think the reasoning behind being Bitcoin only was because the BIP consistently refers to "Bitcoin Addresses" etc. explicitly stating "Bitcoin" before any sort of noun, and therefore was taken as "BIP38 is a Bitcoin protocol unlike BIP44 etc. which make mentions of using it for other altcoins"

That being said, this BIP was originally drafted in late 2013, when the number of altcoins was less than 10 or so... so in this context it could be reasonable to assume that had this BIP been written later they might have made more concessions for altcoins.

I would be hesitant to merge these changes for only one reason: there is no mention in the BIP of what to do with alternative network parameters.

Your pull request makes some logical assumptions, but a small difference in "standards" can cause people to lose access to funds unless they go out and grab a crypto expert. (though many will just assume they self-goxxed and give up)

So my recommendation would be to create a standard somewhere (SLIP, BIP, LIP, or whatever you want to call it for your coin / organization) that specifies in exact terms what to do and how network parameters are used explicitly.

Or propose a modification to the BIP38 draft (remember, it's a draft)

However, I doubt a modification would be accepted, as it seems unanimously discouraged for implementation. https://github.com/bitcoin/bips/wiki/Comments:BIP-0038

That being said, this pull request is not consistent with the standard (while it is a good argument that it is a logical extension) so I will reject this PR.

You are free to maintain your bip38-network fork, create a new standard document for the protocol, etc. and if you have any questions or want to seek advice down the road feel free to ask on our channels.