feat: Replace @stablelib/ with noble-crypto

[ukstv]

This PR replaces @StableLib dependencies with noble-crypto wherever it makes sense, i.e. only @stablelib/xchacha20poly1305 is still used.

Code difference:

1. Jest now treats test files as ES modules
2. @stablelib/sha256 -> @noble/hashes
3. js-sha3 -> @noble/hashes
4. @stablelib/random -> @noble/hashes as @noble/hashes/utils
5. elliptic, @stablelib/ed25519, @stablelib/x25519 -> @noble/curves
6. Inhouse Ripemd160 implementation -> @noble/hashes
7. jwe-vectors.js turned into TS jwe-vectors.ts

Important note regarding different signatures previously created by elliptic package you could see in the test cases. Apparently, elliptic created signature with high value "s". @noble/curves signature gets lower "s" value, being the same fully valid signature.

Another issue: for EdDSASigner a private key is expected to contain 64 bytes. Really, private key is only 32 bytes. The second 32 bytes are a public key. Maybe at some point, a constraint should be relieved.

One benefit apart relying on a solid cryptography libraries, is speed. Tests in the upstream take 22s on my machine. After changes in PR, the same test suite takes 4s.

Re: @scure/base for text encoding/decoding: I am conflicted. uint8arrays and multiformats are quite often used alongside did-jwt, so adding @scure/base would add to the end application size, rather than reduce it. And, @paulmillr names you use like base64url and base64 are quite misleading if compared to multiformats analogs because of padding.

[oed]

That sounds like pretty significant performance gains!

Is impact on package size also positive?

[ukstv]

Is impact on package size also positive?

The best way to gauge size is a bundle prepared by webpack. It contains all the dependencies included as far as I can see. According to the bundle size, current upstream is 236K, changes in the PR reduce it to 133K. Looks like ~40% reduction.

[kdenhartog]

I can give a solid +1 for these noble libraries. They're well tested and audited crypto libraries that weren't around when a lot of this was originally implemented. Noting I've not reviewed the code throughly just providing approval on behalf of the dependency changes. It's worth noting that this change bumps a lot of unrelated packages as well because of the usage of ^. To minimize the number of updates to the yarn.lock file it's probably worth version locking those to see only the affected changes from replacing the dependencies.

[paulmillr]

Re: @scure/base for text encoding/decoding: I am conflicted. uint8arrays and multiformats are quite often used alongside did-jwt

multiformat libraries are using unaudited garbage dependencies

so adding @scure/base would add to the end application size, rather than reduce it.

true, so what's needed is switching multiformats to noble - unfortunately they don't seem to care much, i've tried an issue one time and they mentioned it ain't really needed: multiformats/js-sha3#48 but open to pull requests

And, @paulmillr names you use like base64url and base64 are quite misleading if compared to multiformats analogs because of padding.

padding is easily adjustable in code. we have an open issue for the native features paulmillr/scure-base#4

[ukstv]

To minimize the number of updates to the yarn.lock file it's probably worth version locking those to see only the affected changes from replacing the dependencies.

Okay, I am going to try minimising changes to yarn.lock.

[ukstv]

padding is easily adjustable in code. we have an open issue for the native features

@paulmillr I am aware I can create proper base64url using combinators provided in @scure/base/utils. This seems far from optimal to me. Are you open to rename exported names according to multibase table ? As far as I know, the table is the most comprehensive list of various encoding combinations. It gives you no surprises with regards to padding and casing.

Anyway, encoding/decoding I believe is a slightly separate topic. Let's do with crypto libs first.

[paulmillr]

Are you open to rename

if it ain't breaking backwards compat, sure thing. breaking will need a new breaking release, not optimal rn

[nklomp]

Very nice work!

I haven't done a proper review, but one thing I did notice is that the e256kSigner test is producing different signatures for the same inputs/keys. What is the reason for those?

[ukstv]

@nklomp Copied from the PR description :)

Important note regarding different signatures previously created by elliptic package you could see in the test cases. Apparently, elliptic created signature with high value "s". @noble/curves signature gets lower "s" value, being the same fully valid signature.

If you look closer to the changed signatures in the tests, you'll see that they begin the same. The second part encoding "s" is different.

[nklomp]

Hehehe, thanks. Guess I was too excited about these changes, to read that ;)

[paulmillr]

There is a lowS: false option that will bring the old behavior if you need it.

However, the old behavior (your current) is not safe and introduces signature malleability.

[ukstv]

@kdenhartog Now yarn.lock should be fine. One could easily see scope of the changes there.

[ukstv]

@mirceanis Hey, is there anything I can do to move the PR forward?

The gains are pretty significant:

• proper audited dependencies for cryptography,
• improved performance (5x reduction of test time on Apple M1),
• reduced bundle size by ~40%.

[mirceanis]

@mirceanis Hey, is there anything I can do to move the PR forward?

I'm reviewing it now. This looks great so far 🚀

[mirceanis]

Looks wonderful, thank you!

I'll mark this as a BREAKING CHANGE since the weierstrass signer outputs are changing and it's likely going to cause test failures for dependents.

[uport-automation-bot]

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ukstv]

Yahoo!!