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

decaf377: arkworks v0.5 #109

Closed
wants to merge 8 commits into from
Closed

decaf377: arkworks v0.5 #109

wants to merge 8 commits into from

Conversation

TalDerei
Copy link
Collaborator

@TalDerei TalDerei commented Nov 18, 2024

Upgrade arkworks dependencies to v0.5 in decaf377

enabling the r1cs flag as default tested that the feature-gated r1cs tests pass as well.

@TalDerei TalDerei self-assigned this Nov 18, 2024
@TalDerei TalDerei marked this pull request as draft November 18, 2024 07:38
@redshiftzero
Copy link
Member

redshiftzero commented Nov 19, 2024

Running this on the test vectors generated in #110 does result in failures, still investigating what the cause is..

@TalDerei
Copy link
Collaborator Author

TalDerei commented Nov 19, 2024

Running this on the test vectors generated in #110 does result in failures, still investigating what the cause is..

We should cross-reference the R1CS matrices for each circuit to validate R1CS synthesis.

Strangely, regenerating the keys and reducing the number of proptest iterations fails initially, but passes the test suite in subsequent runs.

@redshiftzero
Copy link
Member

Investigating the constraints for the test circuit DiscreteLogCircuit, and the number of constraints goes down by 28 R1CS constraints - from 2910 to 2882.

Digging deeper, the equality check between ElementVars no longer produces the same constraints..

We do the Decaf equality check X_1 * Y_2 = X_2 * Y_1 and there is a relevant circuit-breaking change introduced in 0.5 in ark-r1cs-std for FqVar (via AllocatedFp) equality reducing the number of constraints from 3 to 2 per FqVar equality check: arkworks-rs/r1cs-std@v0.4.0...v0.5.0#diff-fbbc484f9167af78a4164ae804e0c52adc5feb0f50790a2b0834f4918271ec34R356-R362

There are probably other circuit-breaking changes, this is the first I came across relevant to us.

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.

2 participants