Skip to content
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

v2 ideas #88

Open
paulmillr opened this issue May 15, 2024 · 21 comments
Open

v2 ideas #88

paulmillr opened this issue May 15, 2024 · 21 comments

Comments

@paulmillr
Copy link
Owner

paulmillr commented May 15, 2024

  • Remove cryptoNode import! Requires to ditch support for nodejs v18 (EOL 30 Apr 2025)
  • Remove import maps? Change @noble/hashes/sha3 to @noble/hashes/sha3.js
    • Import maps are not browser-friendly etc
    • Before doing this, actually test in real browser
  • Add root imports
  • Make typescript source work natively with node.js 23+ type-stripping https://2ality.com/2025/01/nodejs-strip-type.html#local-imports-must-refer-to-typescript-files
  • Test in bundler-less browser
  • Remove support for hashing strings, only allow uint8arrays
  • Rename / refactor
    • crypto / cryptoNode => webcrypto (to match noble-ciphers)
    • sha256, sha512 => sha2? not sure yet
    • blake2b, blake2s, blake3 => blake?
    • ripemd160 => ripemd?
    • sha1 => legacy? and add md5??

Maybe also:

  • Make package ESM-only
  • Re-audit (need $ funds)
@imcotton
Copy link

imcotton commented Jun 5, 2024

Any thoughts on adding non-cryptographic hashes? e.g.:

  • CRC32
  • Murmur3
  • xxHash
  • City
  • etc...

@paulmillr
Copy link
Owner Author

What would be the use case?

@imcotton
Copy link

imcotton commented Jun 5, 2024

In general non-cryptographic hashes give synchronize call which widely applicable.

CRC32 is good for quick integrity check.

Both Murmur3 and xxHash commonly used in Bloom Filter, however existing widely used NPM packages all using old syntax (ES5, CJS) and have no updates in 5-6 years.

In case of Murmur3, rarely handle utf8 correctly1. Jumping rabbit hole forks lead to inactive non-resolved issue2

Footnotes

  1. https://github.com/cimi/murmurhash3js-revisited

  2. https://github.com/karanlyons/murmurHash3.js/issues/13

@wasd171
Copy link

wasd171 commented Aug 5, 2024

Would be great to have a properly supported murmur3 hash for Kafka-like projects!

@imcotton
Copy link

imcotton commented Aug 5, 2024

Btw, I'm currently settle on fnv1a as alternative to Murmur3, changeable via:

  • npm:@sindresorhus/fnv1a
  • jsr:@std/crypto crypto.subtle.digestSync('FNV32A')

@paulmillr
Copy link
Owner Author

paulmillr commented Sep 9, 2024

Quote from contributor about ESM transition:


Got it, thank you for explaining the rationale! Might be worth considering that extended support for Node.js v18 will be provided for users of among others:

So for these users, Node.js v18 will not actually be deprecated during 2025 and some users will expect to keep using their supported runtime versions beyond the upstream maintenance window.

Not meaning to imply that ethereum-cryptography should match all of these (or even strictly needs to consider any of it, though it would be nice1), just that "when nodejs v18 is deprecated" is not an entirely black-and-white question and hopefully bring some nuance to the ESM-only timeline consideration.

Footnotes

  1. If it were me, I think it could be a good target to aim at having if not mainline then at least critical and security-related bugfixes backported to a major version runtime compatible during lifetime of Debian bookworm (2028-06) if not also Standard Support for Ubuntu 24.04 LTS (2029-04). Or if not that, at least until both distros have new stable/LTS versions providing newer Node.js versions (~2026?). This is considering users who are comfortable using the vendor-provided versions but where either nvm, or "compile it from source" are non-starters (for compliance reasons or otherwise). And if we're doing that, I imagine that maintaining two separate build systems will not be worth the headache....

@mahnunchik
Copy link
Contributor

+1 to add md5 😉

@paulmillr
Copy link
Owner Author

paulmillr commented Oct 3, 2024

md5 is available in test/misc/md5.ts

@paulmillr paulmillr changed the title Ideas for next major update v2 ideas Feb 1, 2025
@kewde
Copy link

kewde commented Feb 5, 2025

Unrolled loops for SHA3 resulted in a 3.5x improvement on Hermes v0.12.0 on an Android Samsung A52.
Perhaps adding unrolled loop variants for the various hash functions could be an interesting feature?

I assume the performance improvement wouldn't be quite as impactful on modern JS engines, but for older engines it might be worth having.

@paulmillr
Copy link
Owner Author

@kewde is SHA3 a bottleneck for you? How are the other parts of your app? Do you do many sha3-s per second?

@kewde
Copy link

kewde commented Feb 5, 2025

@paulmillr

is SHA3 a bottleneck for you?

We've been spending a bit of efforts on performance profiling our application. We do a lot of cryptographic operations, which is currently bottlenecking our startup time. The reality is that the underlying JavaScript engine (Hermes) is to blame for a lot of the bad performance but we can't simply swap out the engine for a more performant one at this moment in time.

We moved from this unrolled keccak implementation: https://github.com/ExodusMovement/keccak/blob/exodus/lib/keccak-state-unroll.js to the one provided by noble hashes, the SHA3 operations used to take 109ms, with noble it is taking 350ms.

How are the other parts of your app?

Mostly battling other cryptographic operations, one of the other things that has been a slowdown for us on this older version of Hermes are the noble elliptic libraries, the default window size of 8 results in a lot of points being precomputed. About 500ms for each curve on older Android phones. Reducing the window size to 2 - resulted in a much faster startup time for those devices.

Do you do many sha3-s per second?

Yes, we parse a bunch of solidity contracts, in order to compute the id of the methods, we need to SHA3 hash some bits and bobs.

@paulmillr
Copy link
Owner Author

I've published unrolled version of noble-hashes sha3 in this package: https://www.npmjs.com/package/unrolled-nbl-hashes-sha3

You can use it for now from NPM (make sure to lock down dep version).

@paulmillr
Copy link
Owner Author

the default window size of 8 results in a lot of points being precomputed

Good to know, we should adjust docs.

@mahnunchik
Copy link
Contributor

What about SipHash?

@paulmillr
Copy link
Owner Author

Who uses it?

@Vexcited
Copy link

Vexcited commented Feb 13, 2025

md5 is available in test/misc/md5.ts

I think he meant +1 to have it exported in the package so we can use it directly instead of copying the code from test/misc/md5.ts and make our own function.

@paulmillr
Copy link
Owner Author

@Vexcited what do you use md5 for?

@Vexcited
Copy link

what do you use md5 for?

Whenever the API I'm writing a library for requires an MD5 hash at some point.

For example, today I wrote the following code to retrieve the key to decrypt an AES-CBC ciphertext from a service I want to write a wrapper for:

const keychain: Array<Uint8Array> = [];
const saltedKey = concatBytes(this.key, hexToBytes(salt));
keychain.push(hexToBytes(md5(saltedKey)));

for (let i = 0; i < 3; i++) {
  keychain.push(
    hexToBytes(md5(concatBytes(
      keychain.at(-1)!,
      saltedKey
    )))
  );
}

and had to find another library that also uses Uint8Array to create md5 while I was using your library everywhere. As you can see I had to hexToBytes every md5 call because for some reason it was only returning HEX strings and not another Uint8Array unlike your library...

That's why I'd love to have this implemented in noble-hashes.

@mahnunchik
Copy link
Contributor

I need md5 only for gravatar but it seems it became support sha256 now

@paulmillr
Copy link
Owner Author

Understood. Maybe we should add it.

@mahnunchik
Copy link
Contributor

Gravatar switched to sha256 https://docs.gravatar.com/api/avatars/hash/ 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants