iden3 / circom

zkSnark circuit compiler
GNU General Public License v3.0
1.25k stars 232 forks source link

Possible bug in cpp witness generation #230

Closed Stentonian closed 1 month ago

Stentonian commented 6 months ago

Hi I'm getting this error when trying to generate the witness using C++:

fr.cpp:166: void Fr_fail(): Assertion `false' failed.

But witness generation works for wasm. It's happening in a library that I'm using and I'm hesitant to spend time debugging because it seems to be an issue with the finite field code in circom.

I'm using circom CLI version 2.1.7

You can reproduce this error by running the script here: https://github.com/puma314/batch-ecdsa/blob/ff080ca1bcae3dedab9de46a0a4fa8ad6248a039/scripts/batch_ecdsa/fast_build.sh

I see this has happened before https://github.com/iden3/circom/issues/121

Any advice?

Stentonian commented 6 months ago

I've tried changing all the pragmas in the circuits to 2.1.7 but the failure still happens

Stentonian commented 6 months ago

I also tried using version 2.0.4 of circom CLI, but I got the same error

clararod9 commented 6 months ago

Hi! Thanks for reporting this issue.

We have located the lines of code in the circuit that generate the unexpected behavior (lines 275-277 in file batch_ecdsa.circom) and we are working to fix it, we hope to be able to do it soon.

Meanwhile, a possible way to prevent the unexpected behavior is to replace the lines 275- 277:

adders[coord_idx][batch_idx-1].a[x_or_y][reg_idx] <==
     are_points_equal[coord_idx][batch_idx-1].out * (dummy2[x_or_y][reg_idx] - partial[coord_idx][batch_idx-1][x_or_y][reg_idx])
      + partial[coord_idx][batch_idx-1][x_or_y][reg_idx];

by

var aux1 = dummy2[x_or_y][reg_idx] - partial[coord_idx][batch_idx-1][x_or_y][reg_idx];
var aux2 = are_points_equal[coord_idx][batch_idx-1].out * aux1
          + partial[coord_idx][batch_idx-1][x_or_y][reg_idx];
adders[coord_idx][batch_idx-1].a[x_or_y][reg_idx] <== aux2;

We have tried this solution for the circuit test_batch_ecdsa_verify_2.circom and it does not crash and produces the same witness as the original circuit when executed via WASM.

Stentonian commented 6 months ago

@clararod9 thanks so much for digging into this! Really appreciate it. Can confirm that the fix works.

I will test that the fix works for larger circuits (greater number of ecdsa sigs) and then post a PR on the repo (unless you'd like to that?).

clararod9 commented 2 months ago

Hi!

We solved this issue of the compiler in our last release (circom 2. 1. 9). Now the original code works as expected.

Thank you again for reporting this bug and for your help :-)