Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Add Optional? WebAssembly Support #24

Open
josh-hemphill opened this issue Mar 2, 2020 · 7 comments
Open

[Feature Request] Add Optional? WebAssembly Support #24

josh-hemphill opened this issue Mar 2, 2020 · 7 comments

Comments

@josh-hemphill
Copy link

josh-hemphill commented Mar 2, 2020

As far as I can tell, multithreading support for WASM is coming and could provide comparable performance to the Node.js libraries.
Since this is the only library I could find that had browser support, I think it would be ideally suited to provide both a WASM solution and pure js fallbacks to maintain cross compatibility between Node.js and Browsers.

I've seen a few repos that seem to have attempted this, but they are all either not finished or not actively maintained/have no usage.

An additional benefit might be adding the SCrypt source/core C code as a dependency for tracking any issues with the function itself.

I might try building/making a pull request if it's something others agree might belong in this repo/library.

@ricmoo
Copy link
Owner

ricmoo commented May 19, 2020

This is probably still quite complicated to do in a way that is portable, simple and doesn't explode the library size. It is certainly a candidate for the future, but not anytime soon as I use this in ethers and am still supporting back to ES3-ish.

@webmaster128
Copy link

webmaster128 commented May 30, 2020

@josh-hemphill did you make some benchmarks to compare the pure JS implementation and a Wasm implementation, e.g. https://github.com/MyEtherWallet/scrypt-wasm or https://github.com/indutny/dumb-scrypt-wasm?

It should not be too hard to create a wrapper around this lib and a Wasm implementation, using the better one depending on the environment.

@webmaster128
Copy link

There is also an implementation of scryptsalsa208sha256 in libsodium, which is wasm compiled and wrapped in JS as libsodium.js and distributed as libsodium-wrappers-sumo.

@webmaster128
Copy link

In cosmos/cosmjs#181 I played around with the JS implementation from here and the Wasm implementation from MyEtherWallet/scrypt-wasm#2. The benchmark results on Node.js 12 are not that convincing:

Jasmine started

  1 scrypt (js)
    ✓ works for keplr params (16 secs)
    ✓ conforms to test vectors from RFC 7914 (14 secs)

  2 scrypt (wasm)
    ✓ works for keplr params (12 secs)
    ✓ conforms to test vectors from RFC 7914 (10 secs)

I did not run this in browsers yet as the logistics to ship the Wasm file into the test runner are not trivial in combination with Webpack and TypeScript.

If you need to run this very very often, deploying and loading the 40 KB Wasm file might makes sense. Maybe the scrypt implementation can benefit from upcoming multithreading or SIMD support, I don't know.

@webmaster128
Copy link

After further investigation it turned out that for browsers the speedup of Wasm is between 1.5x and 4x: cosmos/cosmjs#181 (comment) Wasm execution is very stable across different environments. JS execution speed varies a lot.

@ryancdotorg
Copy link

Did you add progress callbacks to the wasm implementation?

@webmaster128
Copy link

Did you add progress callbacks to the wasm implementation?

Nope.

The main issue I see with a Wasm implementation is that you can hardly use it on the main thread since a call to Wasm blocks until it is fully executed. This makes the UI freeze for any meaningful scrypt parameters. The JS implementation here avoids this by using some callback tricks. In order to use a Wasm implementation you'd need to execute it in a worker or divide the process into multiple small steps that are executed one after another. For this reason I did not follow this any further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants