tiero commented 3 years ago

This is a substantial refactor in the secp256k1-zkp JS bindings module.

You can find comparisons for node and browser environments in the following comments in this PR thread. I don't see wht holding back on this refactor, we should move to WASM as soon as possible.

secp256k1-zkp submodule has been checked out to latest commit of master
The building pipeline has been simplified (non-docker procedure has been dropped): npm run compile will call scripts/compile_wasm_docker wich will inject the build_wasm script, the secp256k1-zkp folder along with the main.c wrapper and will build with emscripten inside the docker container and move out the artifacts at the end
artifacts now live in dist, not src which seems much more semantically correct
main.c goes to lib folder
we use -s SINGLE_FILE which bundles .wasm and the .js "glue" together to allow easy exporting for browser and node. This targets both node and the web, so no need for browser switch.
Browser suite has been added (only for ecdh at the moment, because this it needs to rewrite the test for browsers specifically)

Breaking changes

In order to instantiate the method in a safe way (wasm need to be compiled by the runtime either browser or node) we now return a promise out of the secp256k1-zkp. This is equivalent of passing the callback onRuntimeInitialized Ref: Emscripten Doc

As documented in test and README we must do as the following:

const secp256k1 = require('secp256k1-zkp');

// secp256k1 returns a Promise that must be resolved before using the exported methods
const { rangeproof, surjectionproof } = await secp256k1(); 

rangeproof.rewind(...)
surjectionproof.verify(...)

This adds a little overhead from 30 to 80 milliseconds in browsers (depending if Chorme or Firefox, Chrome's V8 runtime is always faster), negligible ~0-1 ms in node

ecdh method is not wrapped in an object, but exposed directly as function in the root exported object. Ie. const { ecdh } = await secp256k1(); => ecdh(...)

Improvements for the future

Use embind instead of custom C wrapper main.c
The lib/index.js should be rewritten to use cross-platform data structures as much as possible (between node and browser)
Write more *.browser.js tests

tiero commented 3 years ago

I can go to bed now :) That's only node, hope the gains will be the same for the browser tho