Switch from indutny/elliptic to noble-secp256k1.

[paulmillr]

I've been developing a highly auditable self-contained zero-dependency secp256k1 library @ https://github.com/paulmillr/noble-secp256k1 & https://paulmillr.com/posts/noble-secp256k1-fast-ecc/

The PR should also resolve #666, since noble-secp256k1 is algorithmically protected against timing attack.

The library will increase security of ethers, since all Noble releases are signed, don't contain dependencies and a user is able to verify the source code in a simpler way versus indutny/elliptic, which uses BigNum library with thousands of LOC.

It also has very simple API and is fun to play with.

A hacker could break into any dependency of elliptic and steal important user data. This has happened before:

• https://medium.com/intrinsic/compromised-npm-package-event-stream-d47d08605502
• https://blog.npmjs.org/post/185397814280/plot-to-steal-cryptocurrency-foiled-by-the-npm

[ricmoo]

Looks awesome!

Unfortunately it looks like it is using the JavaScript bigint which I don’t plan to switch to until v6. There are still a lot of platforms which do not support it yet. :(

[paulmillr]

@ricmoo which platforms specifically are you targetting?

[paulmillr]

All modern platforms except for Safari support bigints. So, the userbase is quite high today.

• Chrome, Firefox, Edge, nodejs 10, nodejs12 support bigints
• Safari doesn't support bigint, that's sad. It should have the feature in a couple months.
• nodejs 8 doesn't support bigint. The version is officially unsupported today. We don't want folks to use nodejs which doesn't receive security updates, do we?

Even if someone wants to use nodejs6-8, they could use v4 instead of v5.

[ricmoo]

Here is a useful table to track the feature, but here is a quick list of things that matter more to me:

• DukTape
• Node 8 (which affects a lot of other environments, like electron, react-native, expo)
• Safari iOS (so no iPhones would support it; eek! That's me!)
• Safari macOS (so I won't be able to develop it ;))
• Apparently Edge doesn't support it; while I don't target it directly, it seems abrasive to drop support for them
• Otto; doesn't matter to me a lot, but I know it is used by ethers users, because I hear about it when I use any look-ahead regex (which it doesn't support)

This also only represents the direct versions, keep in mind that many applications may be using even slightly out-of-date versions of react-native or electron, which might be using slightly more out-of-date node, and so on, which forces these other projects to update.

I plan to have the next major version move to more modern things (BigInt is still undecided), but Proxy and WeakMap are things I'm very excited to add support for.

Make sense?

[paulmillr]

@ricmoo I agree we should delay this until Safari supports bigints, but for officially unsupported nodejs v8: users need to upgrade, period. It's a security issue, like, for real. There are electron versions with modern / supported nodejs bundle.

[ricmoo]

As someone who maintains a lot of random apps and projects, dropping support is a huge hassle. Often you can't just upgrade a single library. If a library stops working on node 8, and you are based on an older react-native app, you then upgrade node, which breaks other libraries which have other incompatibilities, and possibly have to re-write part of your main app, because now the react-native API has changed and so on. Node 8 only ended maintenance mode a month ago, and there is a lot of catch-up that is always needed.

I will probably make that decision in v6, that in order to support older platforms you will need to continue using v5 (which means I need to continue bug fixes in a legacy version :s). But v5 is too close to release and I did not give any advanced warning that I would drop the user base at all, certainly not "all apple devices" (including, I'm guessing JavaScripCore, which I'm planning to use in ethers wallet in the next version).

Again, I speak as someone who has had to deal with these events often. I stopped using Cocoa Pods as a result of the packages becoming so legacy, so quickly and instantly dissolving the entire dependency tree to the point there is no spanning tree that produces a working set of libraries, but C has additional complications of symbol collisions and weird mutually-exclusive compile time flags different people depend on. :p

Luckily (?), this is more of a headache for me than anyone else, since consumers of ethers don't need to know what is happening inside. This is one reason why ethers wraps BN.js, so that it is easy to swap it out if I ever change secp256k1 libraries.

I also apologize if I sounded a bit defensive, but this is an discussion I've had many times for a lot of various language features and design patterns for both JavaScript and TypeScript.

Oh! But I should point out, if you are on a platform that has BigInt, and you pass it into v5, it will accept it fine as an input value. :)

[paulmillr]

Sounds great!

[paulmillr]

I've wrote a blog post about the library. Basically during march i've made it faster than indutny/elliptic. Check it out: https://paulmillr.com/posts/noble-secp256k1-fast-ecc/

[ricmoo]

Awesome! I’d love to use it, but it looks like it still uses JavaScript BigInt?

Also great article. I read through it quickly, but am looking forward to reading through it more closely when I get a moment. :)

[paulmillr]

Yup, I don't see any other option that doesn't requires thousands of lines of code re-implementing the BigInt functionality for Apple Safari.

After all, it's just safari that stops us from merging this. all major platforms have supported this for at least a couple releases. All stable node.js versions also.

[paulmillr]

Kudos -- Safari update would support BigInt this year, in a few months. Webkit already merged necessary changes. This would allow us to support all browsers.

[ricmoo]

I will still probably wait a bit longer to make sure there is fairly substantial support, but I’ve written a Babel plugin that can convert your TypeScript library into BigNumber-object-based TypeScript at build-time, so I’m hoping I can switch to it earlier and have extra dist files for es2020, which gains pretty substantial file size improvements for anyone who doesn’t mind dropping support for slightly older browsers. :)

The plugin right now is fairly specific to your secp256k1 library, but I’m trying to make it a bit more generic to support your bls library too. :)

[paulmillr]

Wow, sounds awesome!

With the plugin, besides native support @ Chrome+Firefox+Safari+Edge+Node, we'll cover all other browsers!

[paulmillr]

Let me know if there are some hard points which could be changed in bls12-381 to make babel parsing effortless.

[paulmillr]

@ricmoo heads up — all major browsers including iOS / Macos safari support bigints out-of-box.

[paulmillr]

Bigints are 88.72%-supported right now: https://caniuse.com/bigint

[paulmillr]

I see you've changed directory structure. Here's new signing-key/src.ts/index.ts converted to noble: https://gist.github.com/paulmillr/b37bbf3c2ab19f610862fd398f5597e9

[nissoh]

will be nice to see a switch from BigNumber into BigInt as its more readable, compatible and future proof. thank you for pushing this @paulmillr

[ajb]

Thank you both for working on this. Any plans to still implement?

[ricmoo]

Definitely! But it likely cannot go into v5.

This library depends on BigInt, which is a fairly recent addition to the JavaScript language. It was only adopted by JavaScriptCore last September, which means any dependency based on JavaScriptCore would instantly abort if trying to load it (as it is a syntax error), not to mention it wouldn't work anyways.

Safari for iOS and macOS both use JavaScriptCore, which means on those platforms it would fail. Most users of iOS and macOS remain up to date, so that is of less concern than React Native, which is built on-top of JavaScriptCore, along with Expo and many other "forks".

This means that for RN and all those forks and tweaks of RN, any developer that depends on ethers would be forced to upgrade their underlying framework, which is always a huge undertaking. As a maintainer of several legacy packages, it is debilitating when a dependency requires such a dramatic change. I have suffered at the hand of many such changes, so don't plan to inflict that on anyone myself. :)

In v6, I have plans to use Noble by default, but to also have the build scripts dump out a dist/ethers.compat.umd.js and dist/ethers.compat.umd.min.js which will shim in an elliptic-based (probably?) implementation for those requiring a higher level of compatibility with older systems.

I've been spending a lot of time on v6, and it's coming along. :)

[ajb]

Richard, you are a gem, thank you for the comprehensive explanation :)

[paulmillr]

I've just pushed _syncSign() support via paulmillr/noble-secp256k1@5ce349b, released in 1.2.8.

That was a requirement for ethers to include noble, see twitter thread how we've been resolving it: https://twitter.com/ricmoo/status/1417050092759101440

[paulmillr]

Just rebased this 15month-old pull request with master and it's ready to be merged @ricmoo

I did not remove elliptic and bn.

[ajb]

I just want thank you both again prematurely, as I checked out the code and can confirm that for my implementation, it indeed speeds things up by a factor of 2x. :)

[paulmillr]

I've changed the API to syncSign and utils.hmacSha256Sync

[paulmillr]

The official ethereum-cryptography package is now using noble-secp256k1; 15 times smaller; depending on 3 packages instead of 38.

Makes sense to ditch those in favor of it:

    "bech32": "1.1.4",
    "bn.js": "^4.11.9",
    "elliptic": "6.5.4",
    "hash.js": "1.1.7",
    "js-sha3": "0.8.0",
    "scrypt-js": "3.0.1",

[ricmoo]

I am using them in v6. But I'm making as few and as small changes to v5 as possible. :)

I was glad to see you used a similar API for your scrypt library as I do in js-scrypt. That was a hard requirement, and I'll likely use your solution over mine. ;)

[paulmillr]

@ricmoo onProgress is awesome, thanks for the inspiration!

[ricmoo]

FYI. This is used in v6. Thanks for the awesome libraries!! :)

Closing this PR now. :)