solana icon indicating copy to clipboard operation
solana copied to clipboard

web3.js: replace dependencies to lower supply-chain risk, improve tree-shakeability, and reduce bundle size

Open paulmillr opened this issue 3 years ago • 9 comments

You have quite a few dependencies in web3. All of those can be replaced by 4 deps:

"@ethersproject/sha2": "^5.5.0",
"bigint-buffer": "^1.1.5",
"bn.js": "^5.0.0",
"bs58": "^4.0.1",
"buffer": "6.0.1",
"js-sha3": "^0.8.0",
"secp256k1": "^4.0.2",
"tweetnacl": "^1.0.0"

I suggest switching to audited libraries that are faster, use less dependencies.

  • @noble/hashes for sha2, sha3. It's faster than ethersproject/sha2, js-sha3, and was audited
  • @noble/secp256k1 for secp256k1. It's faster than js implementations of secp, and was audited
  • @noble/ed25519 for ed25519 — which is 7x faster than tweetnacl, and was audited
  • @scure/base could be used for base58 — it was audited and has 0 deps
  • bn.js, bigint-buffer, buffer can be removed altogether

Supply chain attacks are very dangerous. Breaking any sub-dependency of your dependencies could leak user keys. So, the best option is to use projects that don't have sub-dependencies.


Individual tracking tasks

  • #26933

paulmillr avatar Aug 03 '22 10:08 paulmillr

It also seems like tweetnacl has a security vulnerability, which is a separate issue.

paulmillr avatar Aug 03 '22 16:08 paulmillr

Paul! Thanks for this. This is great, worthwhile work that I'll start on now. I had started down this road in #25014 - particularly eliminating @ethersproject/sha2 with a zero-dependency library – but never finished.

I'll start picking these off one by one. I have learned to expect significant challenges, such as:

  • Zero-dependency libraries often accomplish their work by leaning on built-in browser primitives (eg. SubtleCrypto). React Native does not offer a SubtleCrypto API. A ton of work needs to be done there – either polyfill what's missing on the React Native side, or employ some other strategy.
  • An impedance mismatch between a synchronous API in @solana/web3.js and a replacement module that's async only.
  • Replacements that are not tree-shakable because either they
    • have side effects
    • don't offer an ESM version
    • misconfiguration in package.json

I'll work through all these challenges as they come.

steveluscher avatar Aug 03 '22 17:08 steveluscher

It also seems like tweetnacl has a security vulnerability…

The lockfile for @solana/web3.js specifies [email protected].

https://github.com/solana-labs/solana/blob/403b2e4841ef7301370e86a443ebe51f8ae512e0/web3.js/yarn.lock#L7345-L7347

Nevertheless, I'll update the deps for avoidance of doubt.

steveluscher avatar Aug 03 '22 17:08 steveluscher

Immediately, when trying to replace tweetnacl with @noble/ed25519, I fell on my face in exactly the way I expected. All of our signing methods are synchronous, and the Noble library is async. There's simply no way to start with this:

class Transaction {
  sign(...signers: Array<Signer>): void;
}

Then replace the signing implementation

- const signature = nacl.sign.detached(signData, signer.secretKey);
+ const signature = await nobleEd25519.sign(signData, signer.secretKey);

…without having to change the signature to this:

   class Transaction {
-    sign(...signers: Array<Signer>): void;
+    sign(...signers: Array<Signer>): Promise<void>;
   }

You can always start with an async API and remove the genuinely async parts of it, but you can't go the other way without breaking the entire universe. Everyone who currently does this in @solana/web3.js:

function handleClick(): void {
  const t = new Transaction().add(/* ... */).sign(signer);
  /* ... */
}

Would have to change to this:

   function handleClick(): void {
-    const t = new Transaction().add(/* ... */).sign(signer);
+    const t = new Transaction().add(/* ... */);
+    await t.sign(signer);
     /* ... */
   }

…which in turn would force the outer method to become async…

- function handleClick(): void {
+ async function handleClick(): Promise<void> {
     const t = new Transaction().add(/* ... */);
     await t.sign(signer);
     /* ... */
  }

…which in turn would cause folks to have to completely rethink the lifecycle of their event handlers (eg. what happens if the event handler is halfway done but then the UI it's handling an event for unmounts?).

This is part of @noble/ed25519's genius; it achieves speed and small size by delegating hashing to the runtime's implementation of sha512 (ie. SubtleCrypto). This is incompatible with @solana/web3.js as it stands now for two reasons:

  1. SubtleCrypto is async. We're sync.
  2. SubtleCrypto is not available in React Native, and we need @solana/web3.js to run there.

steveluscher avatar Aug 04 '22 23:08 steveluscher

It is the simplest and most reasonable way to implement no-dependency ed25519. Current consumers are fine with using it as-is. If you need synchronous api, see how noble-secp256k1 implemented both async and sync versions — it is still possible.

What happens if you'll decide to sign sol TX with hardware wallet? The procedure cannot be sync — since that can take indefinite amount of time.

paulmillr avatar Aug 05 '22 00:08 paulmillr

As for

This is part of @noble/ed25519's genius; it achieves speed and small size by delegating hashing

it achieves speed by delegating hashing

False: sha512 is not a bottleneck — scalar multiplication is. Native sha512 is 2x slower than noble-hashes implementation. Noble-hashes on Apple M1 can still do 463,606 sha512 hashings of 32-byte input per second — one hashing every 2 microseconds, or ~4000 hashings per 1 120fps frame.

For comparison, noble-ed25519 sign() takes 261μs — 100x longer than sha512(). Your current tweetnacl's signing takes 8x longer than noble-hashes: 2ms per operation.

it achieves small size by delegating hashing

False: sha512 takes about 350 tree-shaken lines of code in noble-hashes. tweetnacl-fast is 2.3KLOC, while noble ed+sha512 is still 1.5KLOC.

paulmillr avatar Aug 05 '22 00:08 paulmillr

It is the simplest and most reasonable way to implement no-dependency ed25519.

100% agree!

…see how noble-secp256k1 implemented both async and sync versions — it is still possible.

You're right! It's possible to inject a sync implementation with a modification like that. But then, we have to go find a userspace implementation of sha512. But then we lose all the benefits that @noble/ed255519 offers: we're slower again (because we're not using the native browser binding), and bigger (because we had to bundle a new dependency to do what SubtleCrypto does ‘for free’).

What happens if you'll decide to sign sol TX with hardware wallet?

This isn't something that @solana/web3.js is responsible for. The signing methods in web3.js are only useful where you have access to a private key. A hardware wallet never exfiltrates those keys, and it certainly doesn't run web3.js.

steveluscher avatar Aug 05 '22 00:08 steveluscher

But then, we have to go find a userspace implementation of sha512

You don't have to find anything. noble-hashes is audited and it is there. You will need it in any case, to replace js-sha3, and other projects.

But then we lose all the benefits that @noble/ed255519 offers: we're slower again

secp with sha256 from noble-hashes is 15 microseconds slower than a native version. Proof:

sign x 4,974 ops/sec @ 201μs/op
signSync x 4,642 ops/sec @ 215μs/op

It is still 215μs, which is 9x faster than your current tweetnacl ed25519-sign.

and bigger (because we had to bundle a new dependency

You will need noble-hashes in any case. Or you can bundle its implementation of sha512, which is about 350 lines of code.

paulmillr avatar Aug 05 '22 00:08 paulmillr

Want to reiterate: @noble/ed25519 could be easily made synchronous without noticeable performance penalty and without massive increase in bundle size. This just requires a few hours of work. If enough users commit to this, we could even do it upstream.

We have made the same thing in @noble/secp256k1 and it's up and running: see README (search for signSync).

paulmillr avatar Aug 24 '22 03:08 paulmillr

@noble/ed25519 1.7.0 is out with sync support.

paulmillr avatar Aug 26 '22 03:08 paulmillr