refactor!: remove unnecessary async from crypto methods

[tabcat]

Removed async keyword from function which don't:

1. do any async work
2. return a promise
3. have an async browser equivalent

Closes: #1949

[maschad]

Left a comment. I think there are some other functions such as encrypt and create which could also be made synchronous.

Majority of the functions addressed in 78b008c would have been async without an await so I think many could be made synchronous

[tabcat]

The create function is already made sync. I chose to keep the encrypt function returning a promise because that is what the browser equivalent does. Can change if it's fine to do this.

The other places where the require-await comments were removed still return promises. With current linter rules any function returning a promise must be an async function.

[maschad]

The create function is already made sync.

My apologies, I had actually intended to link to create function in HMAC

I chose to keep the encrypt function returning a promise because that is what the browser equivalent does. Can change if it's fine to do this.

My understanding is that given they run in different environments that should be inconsequential, but @achingbrain can comment further.

The other places where the require-await comments were removed still return promises. With current linter rules any function returning a promise must be an async function.

The premise was that given the functions are not doing asynchronous work, they should not return a promise. So they need to be refactored to not create promises.

[tabcat]

The premise was that given the functions are not doing asynchronous work, they should not return a promise. So they need to be refactored to not create promises.

There may be some situations where the function being returned could be refactored to not return a promise. I will check for this.

[tabcat]

I've made the hmac create function synchronous. I looked at whether some of the keys methods can be made synchronous and they can, although mainly just for nodejs. Ed25519 can be made synchronous in both the browser and nodejs.

[tabcat]

Ah, hmac's create function for the browser cannot be made sync. Did you talk with @achingbrain yet about if they have to match? Seems like they should, especially if part of public API.

[tabcat]

I think would be best for the node/browser mirror methods to keep parity wrt their interfaces. One returning a value while the other returns a Promise looks like trouble.

There are cases (Ed25519 keys) where both node and browser methods could be synchronous. Since this is not a public API/not exported from package and gets turned into a promise via the public API methods, changing these seems ok.
Same thing with Secp256k1 keys except that there is no node/browser separation here.

[tabcat]

i did some benchmarking with synchronous ed25519 in node and the async api is much faster.
also checked over all the async function and i think i covered them all.

[maschad]

i did some benchmarking with synchronous ed25519 in node and the async api is much faster.
also checked over all the async function and i think i covered them all.

Could you share the benchmark results here, I find it strange that's the case.

[tabcat]

https://gist.github.com/tabcat/871bca3b48fb23c30ab3ba4dc243b8fa i fixed the benchmarks and the results are:

synchronous x 31,708 ops/sec ±0.81% (96 runs sampled)
asynchronous x 24,510 ops/sec ±1.44% (81 runs sampled)
Fastest is synchronous

I made a mistake with how concurrency is handled in the benchmark

[tabcat]

I've made the ed25519 methods all synchronous, this doesn't change public api since they are later wrapped in a promise inside of keys/index.ts

use the refactor semantic commit tag but maybe chore would be better?

[achingbrain]

Thanks!