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

ark-r1cs-std interpolate_and_evaluate failing for n>11 #80

Closed
arnaucube opened this issue Mar 23, 2024 · 7 comments · Fixed by #166
Closed

ark-r1cs-std interpolate_and_evaluate failing for n>11 #80

arnaucube opened this issue Mar 23, 2024 · 7 comments · Fixed by #166
Labels
arkworks bug Something isn't working circuits

Comments

@arnaucube
Copy link
Collaborator

In the r1cs-std lib, using the poly/evaluations/univariate:

Let $n$ be such that the evaluation domain is of size $2^n$. Then, when trying to use EvaluationsVar::interpolate_and_evaluate, it leads to stack overflow when $n>11$, while for $n<=11$ seems to work fine.
A temporary isolated sample to illustrate the issue can be found at: https://github.com/arnaucube/r1cs-interpolate-poly-test/blob/main/src/lib.rs
where different $n$ values can be tried, getting stack overflow for $n>11$.

This $n>11$ is needed for interpolating and evaluating at the challenge the polynomials from the $W_{i+1}.W,~ W_{i+1}.E$ vectors in the DeciderEthCircuit.

arnaucube added a commit that referenced this issue Mar 23, 2024
The check is temporary disabled due
#80,
but the public inputs and logic are there, to be able to continue the
other parts development while issue #80 is solved.
arnaucube added a commit that referenced this issue Mar 23, 2024
The check is temporary disabled due
#80,
but the public inputs and logic are there, to be able to continue the
other parts development while issue #80 is solved.
github-merge-queue bot pushed a commit that referenced this issue Mar 26, 2024
…oofs in Onchain Decider, refactor CommitmentScheme trait (#79)

* Compute Decider's CM challenges in Groth16 circuit, link G16 & KZG proofs in Onchain Decider, refactor CommitmentScheme trait

- Refactor commitment package
  - Refactor `Commitment` trait and the kzg, ipa, pedersen impls
  - Add methods to prove & verify given challenges (not computing them in-method)
- Add KZG challenges computation in decider_eth_circuit
- Add cmE & cmW KZG proving & verification in DeciderEth
- Link Decider's Groth16 proof & KZG proofs data
- Fix point to bytes arkworks inconsistency
- Patch ark_curves to use a cherry-picked version with bn254::constraints & grumpkin for v0.4.0 (once arkworks v0.5.0 is released this will no longer be needed)

* DeciderEthCircuit: Add check eval=p(c) for E & W

The check is temporary disabled due
#80,
but the public inputs and logic are there, to be able to continue the
other parts development while issue #80 is solved.
@winderica
Copy link
Collaborator

This PR should fix this issue :)

@CPerezz
Copy link
Member

CPerezz commented Oct 2, 2024

@winderica I think this issue can be closed right?

@arnaucube arnaucube added the bug Something isn't working label Oct 2, 2024
@winderica
Copy link
Collaborator

I guess no? Although arkworks-rs/r1cs-std#145 has already been merged, we need to depend on the latest r1cs-std to fix this issue, but this will result in some dependency hell... I tried to port to the latest arkworks in my fork several months ago, but at that time I wanted to wait for the release of arkworks 0.5 before submitting a PR.

I think for now we can cherry-pick the changes in arkworks-rs/r1cs-std#145 while preparing for the next release of arkworks at the same time.

@arnaucube
Copy link
Collaborator Author

arnaucube commented Oct 2, 2024

I was just working on this, made a cherrypick of your commit fixing the poly eval, and porting it to the cherry-pick branch of your fork r1cs-std: winderica/r1cs-std#1
it also adapts some other stuff to make it work with the branch arkworks version (v0.4.0).

I think that with this you (@winderica ) should be able to uncomment the current poly-evals in the various decider's circuits and open a PR in Sonobe with the fix, which would then include the fix you did in arkworks and would make the poly evals work ^^

@CPerezz
Copy link
Member

CPerezz commented Oct 2, 2024

I was just working on this, made a cherrypick of your commit fixing the poly eval, and porting it to the cherry-pick branch of your fork r1cs-std: winderica/r1cs-std#1 it also adapts some other stuff to make it work with the branch arkworks version (v0.4.0).

I think that with this you (@winderica ) should be able to uncomment the current poly-evals in the various decider's circuits and open a PR in Sonobe with the fix, which would then include the fix you did in arkworks and would make the poly evals work ^^

Awesome! @winderica will you make a PR then tagging this such that we can close?

@winderica
Copy link
Collaborator

Sure! I am now working on this, but it takes more time than expected because I ran into another issue with r1cs-std. While everything should work correctly in proof generation (i.e., when the mode of cs is SynthesisMode::Prove), the interpolation will panic during trusted setup (i.e., when cs has mode SynthesisMode::Setup), because it internally unwraps .value(), which is None during setup.

@CPerezz
Copy link
Member

CPerezz commented Oct 2, 2024

GL with this madness @winderica <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arkworks bug Something isn't working circuits
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants