search

erhant / circomkit

A testing & development environment for Circom.
https://www.npmjs.com/package/circomkit
MIT License
98 stars 6 forks source link

Default optimization level matches circom compiler #91

Closed numtel closed 1 month ago

numtel commented 1 month ago

Hi Erhan,

I've been compiling Anon Aadhaar and ZK-p2p circuits using their final Zkeys from DefinitelySetup and have found that I have to set the optimization level to 2 for each of these, the default value when invoking circom without any optimization flag set. I propose aligning circomkit with the default used by the compiler.

This is just a draft because it breaks the tests. If you agree, I'll take a look at fixing the tests.

Ben

erhant commented 1 month ago

heya Ben, thanks for the issue!

Im kind of outside these two days, I will try to check this and your comment on C tester issue on Sunday 🙏🏻

erhant commented 1 month ago

Helo helo

I propose aligning circomkit with the default used by the compiler.

I agree Circomkit should follow the default settings with Circom. Lets just also add a comment like this at the optimization field's docstring within configs/index.ts

Defaults to `2` as per the Circom defaults, see [`circom/src/input_user.rs`](https://github.com/iden3/circom/blob/master/circom/src/input_user.rs#L249).

If you agree, I'll take a look at fixing the tests.

I tried the test, indeed a weird error, usual Groth16 SnarkJS shit hahah. Using plonk fixes it, or setting version 1 explicitly passes as well. Probably fixable with some public-input trick that usually fixes these ("Scalar size does not match" stuff). I'm leaving it to you if you want to work the fix, but Im ok with a workaround like I mentioned, setting version: 1 would be really fine with me, would still want to keep it groth16 due to speed.

Afterwards, I will merge this 🙏🏻

numtel commented 1 month ago

Ok so the tests fail for the Fibonacci circuit with optimization level 2 because at that level, circom removes addition constraints so it does make sense to just not test those circuits at that level of optimization. I tried doing the semaphore "dummySquare" fix but this causes other problems. Nonetheless, that previously failing test is now easily extendable to other optimization levels.

Also, I added a link on the readme to the default circomkit.json definition since there wasn't any other breakdown of the options. This should make it easier for new users to configure their projects. I don't feel like it's worth putting more details about the options on the readme since that typescript file explains it so clearly.