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

Update circuit paths + add a new flag #90

Merged
merged 14 commits into from
Aug 10, 2023
Merged

Conversation

saleel
Copy link
Member

@saleel saleel commented Jul 25, 2023

  • Update circom deps to not use relative paths. This was causing trouble for consuming projects not having deps in same path. Builds can be made using -l flag to including necessary (node_modules) folders.
  • Add ignore_body_hash_check flag to EmailVerifier circuit

@@ -68,33 +69,34 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k) {
signal shifted_bh_out[LEN_SHA_B64] <== VarShiftLeft(max_header_bytes, LEN_SHA_B64)(bh_reveal, body_hash_idx);
// log(body_hash_regex.out);

if (ignore_body_hash_check != 1) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This diff look messed up but the only change is move the whole block below under this if condition.

@saleel saleel changed the title Update circuits Update circuit paths + add a new flag Jul 25, 2023
@saleel saleel requested a review from Divide-By-0 July 25, 2023 15:03
@Divide-By-0
Copy link
Member

Divide-By-0 commented Jul 25, 2023

I don't have visibility on if this breaks the Twitter demo since we don't have e2e tests, do you mind adding a test for something quick like witness generation that allows us to ensure things like paths and inputs are formatted correctly? We don't currently have a seperate button for that in the UI but you can probably just throw a testing wit-gen only flag onto the called function!

Also while we are bumping packages, we should update to circom 2.1.6 for faster array queries (might change constraint counts? i'm curious by how much).

@saleel saleel force-pushed the fix/update-circom-imports branch from a2d60be to 2199213 Compare July 26, 2023 05:39
@saleel saleel force-pushed the fix/update-circom-imports branch from a0d24f6 to 4d8b2dd Compare July 26, 2023 20:49
@saleel saleel force-pushed the fix/update-circom-imports branch from 4d8b2dd to 8e22bb6 Compare July 27, 2023 07:33
@saleel
Copy link
Member Author

saleel commented Jul 27, 2023

I don't have visibility on if this breaks the Twitter demo since we don't have e2e tests, do you mind adding a test for something quick like witness generation that allows us to ensure things like paths and inputs are formatted correctly? We don't currently have a seperate button for that in the UI but you can probably just throw a testing wit-gen only flag onto the called function!

There are unit tests in both circuits and twitter-verifier-circuits that run circom tests (input + witness generation and constraint check on circuit). I have added circuits to CircleCI, but twitter tests are failing in the docker container for some unknown reason (circuit build fails) so its disabled for now.

Also while we are bumping packages, we should update to circom 2.1.6 for faster array queries (might change constraint counts? i'm curious by how much).

Yea, I think we can do this in another PR. ?

@Divide-By-0 Divide-By-0 merged commit 9f9ef46 into main Aug 10, 2023
5 checks passed
This was referenced Aug 10, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants