feat: use noble libs for cryptography

[sublimator]

Authors

This PR is a collab with @ckniffen / @sublimator

Use @noble/{curbes,hashes}

Replace elliptic/hash.js/bn.js with noble suite (incl native bigint implementations of ecc)

See: repo
See: most recent audit

TODO

• Check: paulmillr/noble-curves@42de620 (ed25519 pertinent?)
• For wider compatibility set ZIP 215 setting to false to override zcash friendly defaults (stricter than NIST)
• @noble/hashes iso wrapper Simple isomorphic wrapper ?  paulmillr/noble-hashes#56
  • @noble/xxx not interested
  • @xrpl/crypto (was ripple-iso-crypto)
    • isomorphic support potentially overkill given @noble speed (see @ckniffen' bench results)
      • noble sha512 support isn't as good (relative to native) as its sha256/ripemd
    • seems like a library that could be useful in many contexts (i.e. @xrpl/xxx potentially overly XRPL specific ? on the other hand XRPLF can "own" it with no other pesky use cases/users popping up)
• Some issues with brorand replacement with @noble/hashes/utils#randomBytes
• Look at bundle size
  • Take first look
  • Remove crypto-browserify which brings in an unused elliptic due to no tree shaking
    • Update xrpl sha512Half util to use @noble/hashes
    • Browser tests are failing for some reason (see Tests are flakey? #2275 )
  • Investigate possibility of reducing use of crypto-browserify super package
    • ES6 is more amenable to tree shaking/static analysis {optimizations.usedExports}
• ripple-binary-codec uses createHash, could replace with tiny noble/hashes/sha512 ?
• ripple-keypairs is using only @noble/hashes (and before only hash.js), never node native
• Replace brorand with @nobles/hashes/utils#randomBytes (see feat: use noble libs for cryptography #2273 (comment))
  • Node 14 is EOL soon, so EOL support in this repo too ? (v14 doesn't support webcrypto )
    • @xrpl/crypto, uses crypto.randomBytes on node
• Tidy up
  • eslint overrides
  • Remove use of Buffer in ripple-keypairs in favor of Uint8Array
  • Update ripple-address-codec to be able to encode any Sequence as per baseX
• random.ts probably should just be inlined
• Check RFC6979 is enabled
  • seems so
  • there seems to be an extraEntropy option? deterministic nonce + entropy ?
    • tests would fail if signatures aren't deterministic and there's no API surface to expose this so setting extraEntropy: false
• Check fully canonical signatures
  • ed25519 matches rippled: feat: use noble libs for cryptography #2273 (comment)
  • seems so

[socket-security]

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

[sublimator]

@intelliot
What you think ?

[ckniffen]

What is the impact on bundle size? I know the existing libs are a substantial source of the size.

[sublimator]

Will have a look tomorrow

[mvadari]

What is the impact on bundle size? I know the existing libs are a substantial source of the size.

One improvement is that we'll be able to take out some of the custom dedupe code for bn.js in the webpack files

[sublimator]

Unfortunately, crypto-browserify seems to include elliptic (and in turn bn.js) regardless (I looked in the webpack output)

      alias: {
        ws: './dist/npm/client/WSWrapper.js',
        'https-proxy-agent': false,
      },
      extensions: ['.js', '.json'],
      // We don't want to webpack any of the local dependencies:
      // ripple-address-codec, ripple-binary-codec, ripple-keypairs, which are
      // symlinked together via lerna
      symlinks: false,
      fallback: {
        buffer: require.resolve('buffer/'),
        assert: require.resolve('assert/'),
        url: require.resolve('url/'),
        stream: require.resolve('stream-browserify'),
        crypto: require.resolve('crypto-browserify'),
        https: require.resolve('https-browserify'),
        http: require.resolve('stream-http'),
      },

With change:

➜  xrpl.js.sublimator git:(nd-use-noble-libs-for-ripple-keypairs-2023-04-13) ✗ ls -lah packages/xrpl/build 
total 21504
drwxr-xr-x   7 nicholasdudfield  staff   224B Apr 14 10:47 .
drwxr-xr-x  25 nicholasdudfield  staff   800B Apr 14 10:47 ..
-rw-r--r--   1 nicholasdudfield  staff   913K Apr 14 10:47 xrpl-latest-min.js
-rw-r--r--   1 nicholasdudfield  staff   1.2K Apr 14 10:47 xrpl-latest-min.js.LICENSE.txt
-rw-r--r--   1 nicholasdudfield  staff   3.7M Apr 14 10:47 xrpl-latest-min.js.map
-rw-r--r--   1 nicholasdudfield  staff   2.6M Apr 14 10:47 xrpl-latest.js
-rw-r--r--   1 nicholasdudfield  staff   3.3M Apr 14 10:47 xrpl-latest.js.map

As before:

lerna success - xrpl
➜  xrpl.js.sublimator git:(main) ls -lah packages/xrpl/build
total 19704
drwxr-xr-x   7 nicholasdudfield  staff   224B Apr 14 10:47 .
drwxr-xr-x  25 nicholasdudfield  staff   800B Apr 14 10:47 ..
-rw-r--r--   1 nicholasdudfield  staff   847K Apr 14 10:49 xrpl-latest-min.js
-rw-r--r--   1 nicholasdudfield  staff   1.0K Apr 14 10:49 xrpl-latest-min.js.LICENSE.txt
-rw-r--r--   1 nicholasdudfield  staff   3.4M Apr 14 10:49 xrpl-latest-min.js.map
-rw-r--r--   1 nicholasdudfield  staff   2.4M Apr 14 10:48 xrpl-latest.js
-rw-r--r--   1 nicholasdudfield  staff   2.9M Apr 14 10:48 xrpl-latest.js.map

It seems it will require some further investigations, perhaps updating some of the other packages?

[ckniffen]

If this file is updated would browserify crypto shim still be required?

xrpl.js/packages/xrpl/src/utils/hashes/sha512Half.ts

Line 1 in 4ffc51a

import { createHash } from 'crypto'

[sublimator]

Yep, can remove the crypto resolve.fallback entry after replacing that with @noble/hashes:

➜  xrpl git:(nd-use-noble-libs-for-ripple-keypairs-2023-04-13) ✗ ls -lah build
total 19304
drwxr-xr-x   7 nicholasdudfield  staff   224B Apr 14 10:47 .
drwxr-xr-x  26 nicholasdudfield  staff   832B Apr 14 11:54 ..
-rw-r--r--   1 nicholasdudfield  staff   784K Apr 14 11:53 xrpl-latest-min.js
-rw-r--r--   1 nicholasdudfield  staff   1.2K Apr 14 10:53 xrpl-latest-min.js.LICENSE.txt
-rw-r--r--   1 nicholasdudfield  staff   3.3M Apr 14 11:53 xrpl-latest-min.js.map
-rw-r--r--   1 nicholasdudfield  staff   2.4M Apr 14 11:54 xrpl-latest.js
-rw-r--r--   1 nicholasdudfield  staff   3.0M Apr 14 11:53 xrpl-latest.js.map

[sublimator]

@ckniffen
How do you usually work with this monorepo in IDE/editor?
Open each individual project ? or the whole thing ?

I generally open the monorepo root, but I noticed the eslint config files have root: true soIntelliJ seems to only be applying the root configuration and would only find out about issues when the CI complained.

So I opened up the leaves, but then found the typing doesn't seem to work very well.
Maybe vscode fares better?

[ckniffen]

Awesome. A 7.5% decrease is really good and removing an extra shim will be really nice. I'm inclined to count this as a major release change.

[sublimator]

Seems to be some issues with the browser tests.
I'll have to look into it at some point later

[ckniffen]

@ckniffen How do you usually work with this monorepo in IDE/editor? Open each individual project ? or the whole thing ?

I also use IntelliJ and open and the top level. I dont dig into xrpl.js much as I mostly work on the the explorer.

@JST5000 you use VSCode. How do you work on the project?

[JST5000]

@JST5000 you use VSCode. How do you work on the project?

I usually open the monorepo with all the packages and then just search within it if I need something specific. Lerna makes it ok to install, build, and run the tests from the top, so I usually do that. (Although if you want fewer tests you can cd into the individual package and run npm test there)

[sublimator]

Open whole repo ftw! :) Takes some work, but if you get all the packages using the same eslint/typescript config (tricky refactoring financial code such as this), you can also set up tsconfig paths options so the IDE can understand everything without building, and refactors work across package boundaries

[sublimator]

Are these tests a bit flakey at times ?
This last time it was "integration" that failed but the time prior it was the "browser" ?

[mvadari]

Love this comment

[sublimator]

Back in the day a hash was passed as an array of numbers iirc, so it's probably left over from that. I will tidy that up in the followup PR we discussed to address the TODO re: SigningScheme interfaces. (on mobile)

[ckniffen]

Suggested commit message

feat: use @noble and @scure libraries for cryptography

Switch to using @noble/hashes, @noble/curves, @scure/base, @scure/bip32, and @scure/bip39. This replaces `crypto` polyfills (such as `crypto-browserify`), `create-hash`, `elliptic`, `hash.js`, 'bn.js' (both versions), and their many dependencies.  This also means there are 33 less dependencies downloaded when running a fresh  `npm install` and will make the project much easier to maintain.

This reduces the bundle size by 44% (82kb minified and gzipped) over the current 3.0 branch as well as reducing the amount of configuration required to bundle.

Closes #1814, #1817, #2272, and #2306

[mvadari]

Left one nit that I'd like fixed before merge but ✅

[sublimator]

wow! we did it! thanks @ckniffen