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

feat: wasm #38

Merged
merged 5 commits into from Sep 20, 2022
Merged

feat: wasm #38

merged 5 commits into from Sep 20, 2022

Conversation

richard-ramos
Copy link
Member

@richard-ramos richard-ramos commented Aug 30, 2022

How to compile zerokit for wasm and see example code:

  1. Make sure you have nodejs installed and the build-essential package if using ubuntu.
  2. Install wasm-pack
curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh
  1. Compile zerokit for wasm32-unknown-unknown:
cd rln
wasm-pack build --release
  1. Launch example app
cd www
npm install
npm start
  1. Browse http://localhost:8080 and open the developer tools to see console logs

@richard-ramos richard-ramos changed the base branch from master to release-v0.1 August 30, 2022 23:08
@s1fr0
Copy link
Contributor

s1fr0 commented Aug 31, 2022

Maybe it can be of help here to have a look to how we computed the circuit builder before switching to the current witness generator logic from semaphore-rs:

pub fn CIRCOM(resources_folder: &str) -> Option<CircomBuilder<Bn254>> {
let wasm_path = format!("{resources_folder}{WASM_FILENAME}");
let r1cs_path = format!("{resources_folder}{R1CS_FILENAME}");
// Load the WASM and R1CS for witness and proof generation
let cfg = CircomConfig::<Bn254>::new(&wasm_path, &r1cs_path).unwrap();
// We build and return the circuit
Some(CircomBuilder::new(cfg))

We had performance issues at that time, but those were mainly due to not compiling in release mode. So I expect that even that was a valid and efficient solution, although we never actually tested it. If this helps sorting out the wasm problem and there we measure no big regression in circuit performances, we can move back to that implementation.

rln/Cargo.toml Show resolved Hide resolved
rln/Cargo.toml Outdated
ark-relations = { version = "0.3.0", default-features = false, features = [ "std" ] }
ark-serialize = { version = "0.3.0", default-features = false }
ark-circom = { git = "https://github.com/gakonst/ark-circom", features = ["circom-2"] }
wasmer = { version = "2.0" }
ark-circom = { git = "https://github.com/vacp2p/ark-circom", branch = "wasm", features = ["circom-2"] }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fork I created so (most of) this dependency can be run in wasm

rln/Cargo.toml Outdated Show resolved Hide resolved
rln/Cargo.toml Outdated Show resolved Hide resolved
rln/www/index.js Outdated

rln.init_panic_hook();

function _base64ToArrayBuffer(base64) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper function is for converting the circom, zkey and vk files from base64 to an Uint8Array

rln/src/ffi.rs Outdated Show resolved Hide resolved
rln/src/ffi.rs Outdated Show resolved Hide resolved
rln/src/public.rs Outdated Show resolved Hide resolved
Comment on lines 373 to 396
fn calculate_witness_element<E: ark_ec::PairingEngine>(
witness: Vec<BigInt>,
sanity_check: bool,
) -> Result<Vec<E::Fr>> {
use ark_ff::{FpParameters, PrimeField};
let modulus = <<E::Fr as PrimeField>::Params as FpParameters>::MODULUS;

// convert it to field elements
use num_traits::Signed;
let witness = witness
.into_iter()
.map(|w| {
let w = if w.sign() == num_bigint::Sign::Minus {
// Need to negate the witness element if negative
modulus.into() - w.abs().to_biguint().unwrap()
} else {
w.to_biguint().unwrap()
};
E::Fr::from(w)
})
.collect::<Vec<_>>();

Ok(witness)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was extracted from ark-circom, It's main difference is that it will receive the witness instead of calculating it inside the function.

rln/src/protocol.rs Show resolved Hide resolved
@richard-ramos
Copy link
Member Author

@s1fr0 , @staheri14
I wasn't able to make zerokit load the wasm file (I believe it's due to reasons explained in this issue: arkworks-rs/circom-compat#4), so I ended up doing the calculation on the JS side, hence why I have two functions for generating the proof instead of one (to obtain the json input to pass to the witness calculator, and another function that receives the calculated witness).

I'm currently having trouble with create_proof_with_reduction_and_matrices because it tries to use threads even after disabling the parallel feature. (I'm not sure what's going on here). The error I'm receiving is

panicked at 'The global thread pool has not been initialized.: ThreadPoolBuildError { kind: IOError(Error { kind: Unsupported, message: "operation not supported on this platform" }) }

I'll try cloning arkworks-rs/groth16 and adding some logs to that function to try to find out what's going on.

rln/www/index.js Outdated
Comment on lines 34 to 39
const msgLen = Buffer.allocUnsafe(8);
msgLen.writeUIntLE(uint8Msg.length, 0, 8);

// Converting index to LE bytes
const memIndexBytes = Buffer.allocUnsafe(8)
memIndexBytes.writeUIntLE(memIndex, 0, 8);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fryorcraken I had to use a buffer here because i need the function writeUintLE

rln/www/index.js Outdated
Comment on lines 45 to 47
const verifKeyUint8Array = _base64ToArrayBuffer(files.verification_key);
const zkeyUint8Array = _base64ToArrayBuffer(files.zkey);
const circomUint8Array = _base64ToArrayBuffer(files.rln_wasm);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a dev be able to specify the depth and the zkey and verification key to use? or should these be embedded in the code?

@richard-ramos
Copy link
Member Author

Good news! I managed to generate proofs now! I had to make a change in vacp2p/ark-circom@dcde9ba so the parallels feature can be disabled, and this allows generating the proof in the browser without running into the error:

panicked at 'The global thread pool has not been initialized.: ThreadPoolBuildError { kind: IOError(Error { kind: Unsupported, message: "operation not supported on this platform" }) }

However, the proof generation is very slow. It takes ~9s. Perhaps there's a way to enable threads on wasm?
For now, I'm happy that I was able to move past this blocker, so I can continue with verifying the proof, extracting the JS code to a separate repository and reverting a lot of the changes I made that are unrelated to the task of running zerokit on the browser.

@richard-ramos
Copy link
Member Author

I just noticed that if I don't open the devtools console immediatly, the proof generations is faster, i.e:

proof_gen_timer: 2492.31396484375 ms

~2.5s

🤔

@s1fr0
Copy link
Contributor

s1fr0 commented Sep 4, 2022

@richard-ramos this is great! So as we suspected, it was rayon enabled on dependencies that was creating the issue. Maybe this could be of help in the future for enabling multithreading: https://github.com/GoogleChromeLabs/wasm-bindgen-rayon

Regarding performance: does the delay happen only for the first proof generated or it takes 2.5s for all of them? When we struggled with the performance issue, those were affecting mainly the circom builder/witness calculator initialization (i.e., read the circuit wasm and load it) while proof generation and verification remained relatively fast. In other words we had a big delay ~60s (but in debug mode) for generating the first proof (since the witness generator had to be load first), while successive proofs using the same RLN object were generated fast (since the witness generator was ready). If that's the case we might able to manage it by init the witness generator while intializing the node.

Can you replicate execution of test_groth16_proofs_performance_ffi ?

@richard-ramos
Copy link
Member Author

richard-ramos commented Sep 6, 2022

I think the code is now ready for a review. Please do not focus on the javascript code, as it is done only to see interop between js and rust: it is going to be removed from the repository, and is kept there so PR reviewers can see/test the wasm output.
Check previous comments regarding some changes done in rln/

@richard-ramos
Copy link
Member Author

I will close the PR as I learned today that compiling zerokit to wasm is not needed.
It was a nice learning exercise tho :-/

@oskarth oskarth reopened this Sep 13, 2022
@oskarth
Copy link
Member

oskarth commented Sep 13, 2022

@richard-ramos First of all, amazing work on getting WASM support working!

I think there was some miscommunication here. The first/easiest thing to do here was to use existing JS RLN libraries, like zk-kit and other things from PSE. This seems like a riskier and harder path, but if it works that's great. Sanaz and Franck have some more context from PM call(s) about this too.

Regardless of if this is this the main path we choose or not we should definitely try to get it merged, so re-opening this. Will have a look at the PR a bit later.

Copy link
Member

@oskarth oskarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying this out, having some problems getting it to run.

    Compile zerokit for wasm32-unknown-unknown:

cd rln
wasm-pack build --release

I assume this should be rln-wasm? When I run in rln I get:

error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support

But when I run in rln-wasm the wasm-pack command succeeds.

npm install also works. However, when I run npm start I get:

ERROR in ../pkg/rln_wasm_bg.wasm
Import "__wbg_BigInt_1fab4952b6c4a499" from "./rln_wasm_bg.js" with Non-JS-compatible Func Sigurature (i64 as parameter) can only be used for direct wasm to wasm dependencies
 @ ../pkg/rln_wasm.js
 @ ./index.js
 @ ./bootstrap.js

ERROR in ../pkg/rln_wasm_bg.wasm
Import "__wbg_BigInt_d0c7d465bfa30d3b" from "./rln_wasm_bg.js" with Non-JS-compatible Func Sigurature (i64 as parameter) can only be used for direct wasm to wasm dependencies
 @ ../pkg/rln_wasm.js
 @ ./index.js
 @ ./bootstrap.js
ℹ 「wdm」: Failed to compile.

Any idea what could be wrong?

Env:

# Running on MBP M1
node --version: v18.9.0
wasm-pack --version: wasm-pack 0.10.3
rustc --version: rustc 1.63.0 (4b91a6ea7 2022-08-08)
npm --version: 8.19.1

@oskarth
Copy link
Member

oskarth commented Sep 14, 2022

Few general comments:

  1. Can we target master on this instead of release branch? And resolve conflicts
  2. Can we add a simple README in crate with instructions like you wrote in OP, and then a brief sentence in RLN crate about WASM support existing in separate crate?

}

false
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests to this module? Super basic one

@@ -0,0 +1,20 @@
# RLN WASM EXAMPLE APP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see there's a README here already! I missed it because I expected it to be in the rln-wasm dir

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example app will also be removed, and will exist in https://github.com/waku-org/js-rln/ :)

ark-relations = { version = "0.3.0", default-features = false, features = [ "std" ] }
ark-serialize = { version = "0.3.0", default-features = false }
ark-circom = { git = "https://github.com/gakonst/ark-circom", features = ["circom-2"] }
wasmer = { version = "2.0" }
ark-circom = { git = "https://github.com/vacp2p/ark-circom", branch = "wasm", default-features = false, features = ["circom-2"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we upstream this as a PR to ark-circom too? Are all things in arkworks-rs/circom-compat#4 supported?

I think even if just partial support for our case it'd be a welcome PR!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see in vacp2p/ark-circom@0e58714 that it requires disabling parallels. I wonder if it is possible to get the feature flag to be disabled when wasm flag is enabled? Perhaps in a conditional package list or something, WDYT?

Also any idea why parallels doesn't work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we upstream this as a PR to ark-circom too?

I will create a PR for ark-circom, that way we don't have to maintain a separate fork!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any idea why parallels doesn't work?

While running with that feature enabled, the following error was generated:

The global thread pool has not been initialized.: ThreadPoolBuildError { kind: IOError(Error { kind: Unsupported, message: "operation not supported on this platform"

I was looking at https://github.com/GoogleChromeLabs/wasm-bindgen-rayon , which should enable concurrency using js web workers, but I feel like the effort of adding multithreading to zerokit-wasm should be done in a separate PR.

@oskarth oskarth mentioned this pull request Sep 14, 2022
@oskarth oskarth added the track:zerokit Zerokit track (Applied ZK/Explorations) label Sep 14, 2022
@richard-ramos
Copy link
Member Author

Trying this out, having some problems getting it to run.

    Compile zerokit for wasm32-unknown-unknown:

cd rln
wasm-pack build --release

I assume this should be rln-wasm? When I run in rln I get:

error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support

But when I run in rln-wasm the wasm-pack command succeeds.

npm install also works. However, when I run npm start I get:

ERROR in ../pkg/rln_wasm_bg.wasm
Import "__wbg_BigInt_1fab4952b6c4a499" from "./rln_wasm_bg.js" with Non-JS-compatible Func Sigurature (i64 as parameter) can only be used for direct wasm to wasm dependencies
 @ ../pkg/rln_wasm.js
 @ ./index.js
 @ ./bootstrap.js

ERROR in ../pkg/rln_wasm_bg.wasm
Import "__wbg_BigInt_d0c7d465bfa30d3b" from "./rln_wasm_bg.js" with Non-JS-compatible Func Sigurature (i64 as parameter) can only be used for direct wasm to wasm dependencies
 @ ../pkg/rln_wasm.js
 @ ./index.js
 @ ./bootstrap.js
ℹ 「wdm」: Failed to compile.

Any idea what could be wrong?

Env:

# Running on MBP M1
node --version: v18.9.0
wasm-pack --version: wasm-pack 0.10.3
rustc --version: rustc 1.63.0 (4b91a6ea7 2022-08-08)
npm --version: 8.19.1

Please try again with the latest commit. I updated some js dependency versions, so it's necessary to do a npm i in www to download them.

@oskarth
Copy link
Member

oskarth commented Sep 19, 2022

Works now! I see the proof verification in the console :)

@richard-ramos
Copy link
Member Author

I added a couple of tests, and removed the example app since it's supposed to live in js-rln anyways. The only task pending is creating PRs for ark-circom

Copy link
Member

@oskarth oskarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Feel free to merge, can do ark-circom upstream update in separate PR

```
cd rln-wasm
wasm-pack build --release
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a pointer to how https://github.com/waku-org/js-rln/? Maybe this belongs more in that repo, but looking at README not obvious to me how to run something similar to the example app that existed here before

@richard-ramos richard-ramos merged commit c401c0b into master Sep 20, 2022
@richard-ramos richard-ramos deleted the wasm branch October 3, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
track:zerokit Zerokit track (Applied ZK/Explorations)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants