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

[paulmillr]

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

• web3.js: Investigate replacement for tweetnacl solana#26933
• [web3.js] Replace sha256 and secp256k1 impls solana#27390
• web3.js: Remove use of Buffer #1100
• [web3.js] Eliminate dependency on bn.js #1101
• [web3.js] Replace js-sha3 dependency for keccak-ing solana#27629

[paulmillr]

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

[steveluscher]

Paul! Thanks for this. This is great, worthwhile work that I'll start on now. I had started down this road in solana-labs/solana#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]

It also seems like tweetnacl has a security vulnerability…

The lockfile for @solana/web3.js specifies tweetnacl@1.0.3.

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]

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.

[paulmillr]

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]

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.

[steveluscher]

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.

[paulmillr]

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]

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]

@noble/ed25519 1.7.0 is out with sync support.

[paulmillr]

@steveluscher heads up, much smaller v2 of noble-secp, noble-ed is out. Many features have been removed, but should be fine for your case. The libraries are just 4kb gzipped now.

Another option is to use noble-curves which have improved security, are larger; but tree shaking is supported so all is cool.

[steveluscher]

Thanks, @paulmillr!

[steveluscher]

Closed by 5a20ed0.

[github-actions]

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.