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

refactor(rln): improve circuit loading performances and tests fix #26

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

s1fr0
Copy link
Contributor

@s1fr0 s1fr0 commented Jul 26, 2022

This PR implements a different circuit loading mechanism -inspired by semaphore-rs one's- in order to address some performance issues described in #23.

In particular, we replace the previous ark-circom CircomBuilder object used to load circuit and generate witnesses, with a witness generator obtained directly from the circuit .WASM file. As a result the .R1C1 is no more needed. These changes result in a more efficient zkSNARK proof generation.

This PR further fixes some tests for tree height equal 20 that were previously failing, due to missing/wrong hard-coded assert values.

@s1fr0 s1fr0 added the track:zerokit Zerokit track (Applied ZK/Explorations) label Jul 26, 2022
Copy link
Contributor

@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.

Nice one! Only question is regarding debug information being lost when not allowing usage of r1cs file. Seems like a trade-offs that might look different in testing and production? Not sure

const ZKEY_FILENAME: &str = "rln_final.zkey";
const VK_FILENAME: &str = "verifying_key.json";
const R1CS_FILENAME: &str = "rln.r1cs";
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expect most of the (circuit) debugging to be done in circom, not in zerokit. Here we just plug a verified/tested circuit compiled in circom and use it to generate proofs. If I get your concerns correctly, the debugging phase should happen before the compiled circuit is embedded in zerokit. Of course you might find some unexpected behaviors while using zerokit, but then it would most probably be a bad circuit implementation, hence you're back to circom. Does this address your concern?

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, not sure

I think you are correct most debugging will happen at circom level, but I also see the need for debugging generally as well as for perf etc in Zerokit

but let's cross that bridge when we get there and ignore for now

@oskarth oskarth merged commit c1db14d into master Jul 29, 2022
@s1fr0 s1fr0 deleted the rln-performance-tests branch October 17, 2022 10:42
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
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants