Modify ts in compiler and add tsc compile.

zkemail / zk-regex

A library to do regex verification in circom, adapted from the original zk-email. It additionally generates lookup tables for halo2-regex.

GNU General Public License v3.0

76 stars 27 forks source link

Modify ts in compiler and add tsc compile. #31

Closed SoraSuegami closed 9 months ago

SoraSuegami commented 10 months ago

I found one problem in regex.ts. You modified the following parts to use a flat string instead of [string]. https://github.com/zkemail/zk-regex/blob/main/packages/compiler/src/regex.ts#L208-L297 However, each character can be represented as [string] if the character is an escape character.

@SneakY-NickY Could you modify the type definitions without changing the original logic in my code?

I modified some of your code to fix that problem, but it still has a compile error. https://github.com/zkemail/zk-regex/tree/feat/ts_to_js/packages/compiler

SneakY-NickY commented 10 months ago

I'll certainly have a look. I apologize for the mistakes 😅

SneakY-NickY commented 10 months ago

I've forked this branch and created a PR with a possible fix: https://github.com/zkemail/zk-regex/pull/32 Hope this helps 😄

SneakY-NickY commented 10 months ago

Hi, just wanted to check on the state of this PR if it's okay to be merged into the main branch 😄

SoraSuegami commented 10 months ago

Thank you for updating the codes! Sorry for the late review.

I just removed the console.log and println.
I tested circuits in packages/circom with the new compiler, and all tests passed.
I added the following optimizations on the main branch. Could you support them in ts as well? https://github.com/zkemail/zk-regex/commit/69eb7d0fd86b574cbb366c12a340271aac202013#diff-6db1bcc6a237c8cf4b73f1932a6666cf8a9a1df3db30fe56a012affed28a45a9L57-R198 https://github.com/zkemail/zk-regex/commit/9cd365d199dec29eb5e19b9f91913ba5723e4de8#diff-6db1bcc6a237c8cf4b73f1932a6666cf8a9a1df3db30fe56a012affed28a45a9R62-R246

Divide-By-0 commented 10 months ago

@SneakY-NickY let us know if you can get this updated! We'd love to merge and send you a small grant for your efforts.

SneakY-NickY commented 10 months ago

Got a bit busy this week 😅 I'll try to get this one done asap 😄

@SneakY-NickY let us know if you can get this updated! We'd love to merge and send you a small grant for your efforts.

SneakY-NickY commented 10 months ago

Here's a PR that supports the optimizations done on the main branch 😄 https://github.com/zkemail/zk-regex/pull/34

SoraSuegami commented 9 months ago

@SneakY-NickY Thank you for supporting the optimizations! I checked that all tests in packages/circom/tests passed with the circom circuits generated from your ts script. However, the constraints in some circuits such as email_addr_regex.circom are different from those on the main branch. Could you debug why they are different?

SneakY-NickY commented 9 months ago

@SneakY-NickY Thank you for supporting the optimizations! I checked that all tests in packages/circom/tests passed with the circom circuits generated from your ts script. However, the constraints in some circuits such as email_addr_regex.circom are different from those on the main branch. Could you debug why they are different?

I tried running fresh "yarn install" from main branch and ran tests both on feat/ts_to_js and main afterwards. The results were exactly the same and identical to current main branch.

However, when I tried running fresh "yarn install" from feat/ts_to_js branch and ran tests both on feat/ts_to_js and main again, the results were still exactly the same, but different from current main branch.

Perhaps this is caused by some dependency difference in this branch? As far as I can tell, this is not an issue caused by conversion from .js to .ts.