cryptocoinjs / hdkey

JavaScript component for Bitcoin hierarchical deterministic keys (BIP32)
MIT License
201 stars 74 forks source link

Replacing the module crypto #40

Open alcuadrado opened 4 years ago

alcuadrado commented 4 years ago

Hi,

I'm opening this issue to know if you are open to replacing the use of crypto for more browser-friendly libraries like create-hmac and randombytes.

I've had a hard time using this library with Rollup, which led me to bundling my own version here: https://github.com/ethereum/js-ethereum-cryptography/tree/master/hdkey-without-crypto-config

I'd love not to have to do that, and the needed change is really small. I can prepare a PR for this, making sure that the builtin implementations are used on node.

junderw commented 4 years ago

bip32 library works with browsers.

Is there any functionality that bip32 doesn't offer that you need? Besides not having bitcoin in the name... lol

alcuadrado commented 4 years ago

My main issue with bip32 is that it depends on tiny-secp256k1 which needs to be recompiled every time you install it. I used it in my main project until very recently.

Besides that, I don't see how your comment is related. I'm proposing a small change that would improve this lib's portability.

junderw commented 4 years ago

I don't see how your comment is related.

I don't see how tiny-secp256k1 compilation step is related to browsers, since we fallback to pure JS when packaging for browsers. If the compilation fails you will use pure JS in node environment too.


Since you were talking about browsers, I suggested a bip32 library that works in browsers out of the box. Asking why you don't use that helps me understand exactly what you are aiming for with this suggestion to hdkey library.

I don't know why hdkey doesn't use browser friendly dependencies, but by asking you why you didn't use one that did (bip32), it gives us more context and information for considering your request.

alcuadrado commented 4 years ago

I don't see how tiny-secp256k1 compilation step is related to browsers, since we fallback to pure JS when packaging for browsers. If the compilation fails you will use pure JS in node environment too.

I'm leading an effort to remove native dependencies (except N-API based ones) from the Ethereum js ecosystem. Why?

  1. They are degrade the development experience by being slow to install.
  2. They tend to break with new Node.js major versions
  3. Many people, especially Windows users, don't have a proper setup to use them. This is incredibly common in educational contexts (e.g. a workshop at a conference). In those cases, many people just desist.

As part of this process, I'm also making sure that the dependencies being used work well with web bundlers and don't produce unnecessarily large bundles.

The problems with the crypto module are:

  1. It is normally be replaced by crypto-browserify, which is HUGE.
  2. It doesn't work with Rollup.

Why hdkey and not bip32? hdkey is more popular in the Ethereum ecosystem, so migrating to a portable version of it is easier than migrating to bip32. This is especially important because this effort requires the buy-in of multiple open source projects.

coolaj86 commented 2 years ago

In package.json you can specify a broswer key and then put your replacements in it. This is the de facto solution for this problem and supported by virtually every bundler system.

Example:

{
  "main": "hdkey.js",
  "browser": {
    "./lib/thingy.js": "./lib-browser/thingy.js",
    "foo-dep": "foo-browser-dep"
  }
}

Anything that installs this as a dependency would get the correct bundle for that bundle system.

junderw commented 2 years ago

@alcuadrado Also FYI tiny-secp256k1 has since moved to WASM, no more compilation, but now you have to deal with bundlers WASM support. (webpack works pretty well in my experience)

alcuadrado commented 2 years ago

Thanks for the heads up, @junderw!

coolaj86 commented 1 year ago

New Breaking Version 3.0

I'm rewriting this today, in about 10 minutes (for probably 4-6 hours).

$DASH Code Hangout #6: WebCrypto & ArrayBuffers for HD Keys. Full Send.

The stream will be live at https://twitch.tv/dashincubator and stored at https://youtube.com/@dashincubator.

Changes I'm making include:

Node v18-only changes

These defineProperty / prototype changes are necessary because they don't work with async functions, which are required by the current-gen APIs.

I have the prerequisite PR up at https://github.com/dashhive/hdkey/pull/4.

I'm considering wether to try to get the old mocha tests going or to migrate them to modern table-driven tests.

I will submit the PRs here too, but we'll be maintaining our own fork either way.