search

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
208 stars 100 forks source link

Add addressPrefix parameter, merging with progressCallback, scryptParams and network into options argument #20

Closed dcousens closed 4 years ago

dcousens commented 7 years ago

Thoughts @jprichardson ?

If we lost progressCallback, it would be a breaking change.

encrypt(buffer, compressed, passphrase[, options])
decrypt(encryptedKey, passhprase[, options])

Where options is

{
  progressCallback: () => {},
  addressPrefix: 0x00,
  N: 16384,
  r: 8,
  p: 8
}
jprichardson commented 7 years ago

I'm indifferent... but why?

dcousens commented 7 years ago

They are rarely used positional arguments, and we don't actually have an addressPrefix argument at the moment.

dcousens commented 7 years ago

Someone else can re-open this if they need it - I don't have the time to follow up, and the API is great as is :+1:

jprichardson commented 7 years ago

These seems solid now that I think about it. What about moving compressed to options as well? This really simplifies things especially if we have sane defaults IMHO.

dcousens commented 7 years ago

@jprichardson agreed, compressed by default :+1:

youssefgh commented 7 years ago

Is this supposed to add network to params ? because right now decrypting a testnet encrypted key result in an error Thinks for your efforts ! you rock guys :+1:

dalitsairio commented 6 years ago

@youssefgh yes, this would solve your problem. If this feature were developed, you would just have to pass addressPrefix: 0x6F and you'd be fine for the testnet. I'd appreciate an options parameter a lot :muscle:

sondreb commented 5 years ago

What is the status on this?

CryptoCoinJS did implement a .version field that supported specifying the required network details, while BitcoinJS does not support alternative networks.

Is there a way to get hold of the old CryptoCoinJS repo?

Example from this URL: http://cryptocoinjs.com/modules/currency/bip38/

var bip38 = new Bip38(); bip38.version = {private: 0x80, public: 0x0};

There is no such support in BitcoinJS bip38?

sondreb commented 5 years ago

I made an implementation using function parameter, as oppose to the old 1.4.0 version which had a version field on the object instance. Please review, and it should close this issue.

Ashtonulliac commented 5 years ago

bc1qchl0dursnqqjwzma38kqfkhumtju5jjnzqwgx4

junderw commented 4 years ago

Re: AddressPrefix, I think no. My reasoning is here: https://github.com/bitcoinjs/bip38/pull/44#issuecomment-471817247

Re: switching from positional to an opts object argument I am for. It would be a breaking change.

However, at this point since BIP38 is discouraged for use by most core devs, This library is mostly here for legacy purposes.

I will go ahead and close this issue.