Create twitter_reset_regex.circom from zk-regex

[foolo]

Use zk-regex to create twitter_reset_regex.circom (and add regex_to_circom.sh, with which it can be repeated)
Remove libs/regex_to_circom/

Fixes #116

[Divide-By-0]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

[foolo]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

[Divide-By-0]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

Well it should fall with new inputs for sure, but I'm worried it's just ignoring the verifier you uploaded for some reason.

[Divide-By-0]

Also #65 is an easy issue to also address while we make these changes! We can add another test for a generic twitter notification for this.

[Divide-By-0]

Also, we are mid-move for these examples to be in a different folder (not packages), so you may have to reproduce your changes on https://github.com/zkemail/proof-of-twitter/ as well.

[foolo]

Also, we are mid-move for these examples to be in a different folder (not packages), so you may have to reproduce your changes on https://github.com/zkemail/proof-of-twitter/ as well.

@Divide-By-0 OK! zkemail/proof-of-twitter#2

[foolo]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

Well it should fall with new inputs for sure, but I'm worried it's just ignoring the verifier you uploaded for some reason.

@Divide-By-0 I assume that the tests should fail if i change proof_a, proof_b and proof_c in TestTwitter.t.sol to some junk data, right?
I tried changing this on latest main branch (see foolo@9375e8d )
but the test still passes...!

path:
~/checkout/zk/zk-email-verify/packages/twitter-verifier-circuits

➤ yarn test
 PASS  tests/twitter.test.ts (679.627 s)
  Twitter email test
    ✓ should verify twitter email (177670 ms)
    ✓ should fail if the twitter_username_idx is invalid (7707 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        679.749 s
Ran all test suites.

[Divide-By-0]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

Well it should fall with new inputs for sure, but I'm worried it's just ignoring the verifier you uploaded for some reason.

@Divide-By-0 I assume that the tests should fail if i change proof_a, proof_b and proof_c in TestTwitter.t.sol to some junk data, right? I tried changing this on latest main branch (see foolo@9375e8d ) but the test still passes...!

path:
~/checkout/zk/zk-email-verify/packages/twitter-verifier-circuits

➤ yarn test
 PASS  tests/twitter.test.ts (679.627 s)
  Twitter email test
    ✓ should verify twitter email (177670 ms)
    ✓ should fail if the twitter_username_idx is invalid (7707 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        679.749 s
Ran all test suites.

Oops lol. Do you mind debugging this and figuring out what's wrong? I'll have a minute to check later today as well.

[foolo]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

Well it should fall with new inputs for sure, but I'm worried it's just ignoring the verifier you uploaded for some reason.

@Divide-By-0 I assume that the tests should fail if i change proof_a, proof_b and proof_c in TestTwitter.t.sol to some junk data, right? I tried changing this on latest main branch (see foolo@9375e8d ) but the test still passes...!

path:
~/checkout/zk/zk-email-verify/packages/twitter-verifier-circuits

➤ yarn test
 PASS  tests/twitter.test.ts (679.627 s)
  Twitter email test
    ✓ should verify twitter email (177670 ms)
    ✓ should fail if the twitter_username_idx is invalid (7707 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        679.749 s
Ran all test suites.

Oops lol. Do you mind debugging this and figuring out what's wrong? I'll have a minute to check later today as well.

@Divide-By-0 Ok, now I have at least managed to make the tests fail. I was running the wrong tests... Btw, I updated the README instructions for setting up the forge tests:
https://github.com/zkemail/zk-email-verify/pull/143/files#diff-0cb9111809d1955cb6f9fcb074915d1c0cd60a7b4c3ca835f30f1770c5b2fe0b
Need to install an older version, v4.9.3, of openzeppelin to be able to compile the tests.

Regarding the actual test suite, I'm probably missing some very vital step :) maybe I need to deploy a contract or upload new keys somewhere.. I'll continue investigating it.

[Divide-By-0]

How is it possible that twitter contract tests pass if you updated the verifier and not the test with a new sample proof in https://github.com/zkemail/zk-email-verify/blob/main/packages/twitter-verifier-contracts/src/test/TestTwitter.t.sol -- do you mind double checking that the test is testing your code correctly?

@Divide-By-0 Sure! I did do some basic sanity checks, that I could actually make the unit tests fail if i changed some other inputs, because I too was a bit surprised that they passed :) But I'll take a closer look at it.

Well it should fall with new inputs for sure, but I'm worried it's just ignoring the verifier you uploaded for some reason.

@Divide-By-0 I assume that the tests should fail if i change proof_a, proof_b and proof_c in TestTwitter.t.sol to some junk data, right? I tried changing this on latest main branch (see foolo@9375e8d ) but the test still passes...!

path:
~/checkout/zk/zk-email-verify/packages/twitter-verifier-circuits

➤ yarn test
 PASS  tests/twitter.test.ts (679.627 s)
  Twitter email test
    ✓ should verify twitter email (177670 ms)
    ✓ should fail if the twitter_username_idx is invalid (7707 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        679.749 s
Ran all test suites.

Oops lol. Do you mind debugging this and figuring out what's wrong? I'll have a minute to check later today as well.

@Divide-By-0 Ok, now I have at least managed to make the tests fail. I was running the wrong tests... Btw, I updated the README instructions for setting up the forge tests: https://github.com/zkemail/zk-email-verify/pull/143/files#diff-0cb9111809d1955cb6f9fcb074915d1c0cd60a7b4c3ca835f30f1770c5b2fe0b Need to install an older version, v4.9.3, of openzeppelin to be able to compile the tests.

Regarding the actual test suite, I'm probably missing some very vital step :) maybe I need to deploy a contract or upload new keys somewhere.. I'll continue investigating it.

Tests seem to pass in CI? Also, we take versions from the remapping, not the docs so might want to put that there too! Odd that tests don't run with new Oz versions, we should be up to date on that for security reasons.

[foolo]

Odd that tests don't run with new Oz versions, we should be up to date on that for security reasons.

@Divide-By-0 Yeah, for example, the first error I encountered was that contracts/utils/Counters.sol (which we are using in a few places), has been removed in OZ 5, so there will be some code changes needed to move to OZ 5

Tests seem to pass in CI?

Yep, it passes in CI. I was a big unclear when I said that i could make the tests fail. I meant that I can make them fail if I change the proofs in TestTwitter.t.sol (before, I claimed that they pass even if i changed the proofs, but that was just a misunderstanding, sorry about that :) ).
So yes I agree it is strange they pass, but I have not figured it why yet.

Also, we take versions from the remapping, not the docs so might want to put that there too!

Ok, I'll look at that.

[Divide-By-0]

It looks like testverifytestemail is failing; do you know what might be causing that?

[foolo]

It looks like testverifytestemail is failing; do you know what might be causing that?

@Divide-By-0
Yeah, I'm kind of stuck on this one tbh.
At least I found the reason that the tests passed before. It was because I had modified verifier.sol (which wasn't used at all) instead of Groth16VerifierTwitter.sol.
I have updated testVerifyTestEmail with new input and proofs, but for some I reason i get
"[FAIL. Reason: EvmError: Revert] testVerifyTestEmail() (gas: 9079256848778899365)",
and the gas cost seems ridiculously high

When I trace the execution of the tests, it seems that it halts on
success := staticcall(sub(gas(), 2000), 8, add(input, 0x20), mul(inputSize, 0x20), out, 0x20)
in paring(…) in Groth16VerifierTwitter.sol, and that's where I ran out of ideas.

I converted this PR to a draft again. I fear that it can take a while for me to solve it, so if you need it more urgently, maybe it's better to assign the issue to someone with more knowledge about this.

[saleel]

Hey @foolo ,

As we are moving Twitter example to https://github.com/zkemail/proof-of-twitter/ would be able to make the PR there?
That repo is already based on the circuits generated by the zk-regex, but I think we can add the sh scripts and doc changes you have created here.

The tests are working fine there, but there is an issue (zkemail/proof-of-twitter#9) to improve them. I will add more info there.

[foolo]

@saleel Sure, i can look at it and see if there is something from this PR that should be updated in the new repo, such as the docs and scripts.

[saleel]

@saleel Sure, i can look at it and see if there is something from this PR that should be updated in the new repo, such as the docs and scripts.

Thank you 🙏
Please close this PR once you move.

[foolo]

i checked the new repo and these changes are no longer relevant

[saleel]

Ah ok. Sorry about that. I should have looked at this PR earlier.
We are improving process on contribution so things like these don't happen.

[foolo]

@saleel No problem, I was kind of stuck on this one anyway :)