feat: use async await

[alanshaw]

This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the callback parameter.

Notes

async keyword

I've labeled API methods that are asynchronous with the async keyword even if they don't actually use await. I'm interested to hear views on this but I think it communicates the intention of the function better even if it is not required. A contrived example:

async function publicApiFn (type) {
  type = type.toLowerCase()
  return _privateFn(type) // an async function / returns a promise
}

publicApiFn doesn't actually use await, so async is not strictly required. However the async keyword helps communicate the fact that it returns a promise so you can use it without having to track down _privateFn to see what that returns.

The other reason for doing this is if a consumer is using then/catch and type.toLowerCase() throws, then it'll be caught in the catch block. Without the async keyword it'll be an uncaught exception.

Mostly synchronous API

There's a lot of functions labelled as async that don't actually do anything async! This is largely because the Node.js crypto API is synchronous. However the webcrypto API is async so I assume this has been done to present a consistent interface to developers in both Node.js and the browser. Maybe someone can verify that for me?

---

depends on

• feat: change api to async/await multiformats/js-multihashing-async#37
• feat: use async/await js-libp2p-crypto-secp256k1#9

refs ipfs/js-ipfs#1670

---

Benchmarks

Before

$ node benchmarks/ephemeral-keys.js 
ephemeral key with secrect P-256 x 5,825 ops/sec ±2.25% (74 runs sampled)
ephemeral key with secrect P-384 x 5,938 ops/sec ±3.78% (77 runs sampled)
ephemeral key with secrect P-521 x 6,044 ops/sec ±2.72% (77 runs sampled)

$ node benchmarks/key-stretcher.js 
keyStretcher AES-128 SHA1 x 9,810 ops/sec ±3.23% (72 runs sampled)
keyStretcher AES-128 SHA256 x 13,401 ops/sec ±4.01% (74 runs sampled)
keyStretcher AES-128 SHA512 x 22,072 ops/sec ±4.63% (70 runs sampled)
keyStretcher AES-256 SHA1 x 8,631 ops/sec ±2.72% (76 runs sampled)
keyStretcher AES-256 SHA256 x 10,794 ops/sec ±5.49% (71 runs sampled)
keyStretcher AES-256 SHA512 x 16,027 ops/sec ±6.36% (74 runs sampled)
keyStretcher Blowfish SHA1 x 81,146 ops/sec ±9.82% (71 runs sampled)
keyStretcher Blowfish SHA256 x 75,919 ops/sec ±13.66% (72 runs sampled)
keyStretcher Blowfish SHA512 x 70,505 ops/sec ±20.75% (67 runs sampled)

$ node benchmarks/rsa.js 
generateKeyPair 1024bits x 73.18 ops/sec ±6.67% (53 runs sampled)
generateKeyPair 2048bits x 16.51 ops/sec ±15.23% (30 runs sampled)
generateKeyPair 4096bits x 1.89 ops/sec ±47.81% (14 runs sampled)
sign and verify x 1,887 ops/sec ±1.77% (73 runs sampled)

After

$ node benchmarks/ephemeral-keys.js 
ephemeral key with secrect P-256 x 6,275 ops/sec ±2.17% (76 runs sampled)
ephemeral key with secrect P-384 x 6,291 ops/sec ±3.32% (78 runs sampled)
ephemeral key with secrect P-521 x 6,272 ops/sec ±3.35% (76 runs sampled)

$ node benchmarks/key-stretcher.js 
keyStretcher AES-128 SHA1 x 16,948 ops/sec ±1.23% (80 runs sampled)
keyStretcher AES-128 SHA256 x 22,188 ops/sec ±1.48% (76 runs sampled)
keyStretcher AES-128 SHA512 x 34,204 ops/sec ±2.26% (77 runs sampled)
keyStretcher AES-256 SHA1 x 14,910 ops/sec ±0.85% (82 runs sampled)
keyStretcher AES-256 SHA256 x 17,841 ops/sec ±1.75% (77 runs sampled)
keyStretcher AES-256 SHA512 x 26,028 ops/sec ±1.57% (73 runs sampled)
keyStretcher Blowfish SHA1 x 113,697 ops/sec ±8.02% (75 runs sampled)
keyStretcher Blowfish SHA256 x 93,089 ops/sec ±28.10% (55 runs sampled)
keyStretcher Blowfish SHA512 x 34,725 ops/sec ±50.76% (29 runs sampled)

$ node benchmarks/rsa.js 
generateKeyPair 1024bits x 71.01 ops/sec ±7.37% (48 runs sampled)
generateKeyPair 2048bits x 13.97 ops/sec ±10.97% (34 runs sampled)
generateKeyPair 4096bits x 2.59 ops/sec ±29.13% (17 runs sampled)
sign and verify x 1,989 ops/sec ±5.39% (79 runs sampled)

[daviddias]

This is largely because the Node.js crypto API is synchronous. However the webcrypto API is async so I assume this has been done to present a consistent interface to developers in both Node.js and the browser. Maybe someone can verify that for me?

This is exactly correct. We need to keep things async otherwise we would have to interact with both the Node.js Crypto and WebCrypto differently changing a bunch of modules upstream.

[dignifiedquire]

@alanshaw what is the state of this?

[alanshaw]

LGTM - will need to be released as 0.17 and needs a "BREAKING CHANGE: API refactored to use async/await" message in the squashed commit.

[achingbrain]

I've updated the commit message

[vasco-santos]

This is looking great! The code is much more readable now!

Just left a comment regarding bundle optimization.
Other than that, can we create an issue for adding err-code to the errors? I don’t want to block this PR from getting merged since its goal is not that, but a second pass to add the codes would be great! I am available to create the issue and add them!

[vasco-santos]

I believe @hugomrdias added this for reducing the bundle size: 8d8294d#diff-17ddd891dd4a459b2c549aebf399e5edR10

Should we really remove this? If not, we should document why we have this

[achingbrain]

Good catch, removing this increased the bundle size by 400kb(!)(!!!!!!!). I have put it back in.

[vasco-santos]

LGTM! 🎉

[dirkmc]

I'm not sure if this if block is necessary?

[achingbrain]

Certainly removing it doesn't make any tests fail, though it's in the callback version too.

[jacobheun]

This looks good.

[jacobheun]

@dignifiedquire I am going to merge this in so that we can incorporate #153 as well before the release. If there are any issues you see with this we can do a follow up PR before the release.