-
Notifications
You must be signed in to change notification settings - Fork 446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use more lightweight sha256 implementation #170
Comments
If I get a 👍 for such a change, I'm happy to create a PR. |
I'd love to see this. I'm facing the Webpack 5 compatibility issue today, it would be great not to depend on |
As briefly mentioned in #173, we still pull in both md5 and ripemd160 as well as helper dependencies through pbkdf2 because this package supports those non-SHA2 algorithms. Ideally we had a pbkdf2 implementation which uses dependency injection to get the hash algorihm. pbkdf2 in @noble/hashes works like that. However, it does not use the SubtleCrypto API in browsers and modern Node for pbkdf2. But I'd be surprised if this was really necessary for the usage in BIP39. |
Here are noble crypro benchmarks from my machine. The 3rd block with 2048 iterations is what matters to this use case. So as long as we are happy with those timings, we can build this lib on @noble/hashes for both sha256 and pbkdf2.
|
for anyone interested, there is scure-bip39 (audited) which is much, much lighter weight, and by the author of noble-hashes |
the issue is not relevant anymore Line 38 in a7ecbfe
|
Using sh256 via create-hash pulls in a bunch of dependencies we don't need, like cipher-base, md5.js and ripemd160.
cipher-base
is particularly problematic for everyone switching from Webpack 4 to Webpack 5 or using other reasonably modern bundlers because it requires node'sstream
: https://github.com/crypto-browserify/cipher-base/blob/v1.0.4/index.js#L7. This causes hard to debug and support problems downstream, like cosmos/cosmjs#925.I think it would be much nicer to use a dependency that only implements sha256. This could be sha.js or something else, but sha.js is used already anyways.
The text was updated successfully, but these errors were encountered: