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

[Fix] Declare public variable 1field once #2268

Merged
merged 4 commits into from
Jan 19, 2024
Merged

Conversation

vicsn
Copy link
Collaborator

@vicsn vicsn commented Dec 28, 2023

Motivation

Closes https://github.com/AleoHQ/snarkVM/issues/2266

We were declaring two hardcoded public One variables (in R1CS and ConstraintSystem), and adding them twice (in to_transition_verifier_inputs and in verify_batch. This PR:

  • skips adding the hardcoded variable from R1CS onto the ConstraintSystem
  • removes adding the hardcoded input in verify_batch
  • adds logic to remove LinearCombination terms with value 0. Similar to https://github.com/AleoHQ/snarkVM/pull/1796 but for a different object doing the same thing.

Test Plan

No new tests were added.

Requires resampling credits and inclusion.

Related PRs

Previous attempt: https://github.com/AleoHQ/snarkVM/pull/2110/

@vicsn vicsn changed the title WIP: Declare public once Declare public variable 1field once Dec 28, 2023
@vicsn vicsn marked this pull request as ready for review December 28, 2023 13:10
@vicsn vicsn requested a review from Pratyush December 28, 2023 13:10
@Pratyush
Copy link
Contributor

Pratyush commented Jan 3, 2024

This looks good to me, modulo an additional check in generate_constraints that ConstraintSystem allocates a one variable.

@vicsn
Copy link
Collaborator Author

vicsn commented Jan 4, 2024

This looks good to me, modulo an additional check in generate_constraints that ConstraintSystem allocates a one variable.

Thank you for the suggestion! I looked into it again and think we have a good setup already (avoiding further refactors):

  1. Verifier: Most importantly, the verifier already checks that the first public variable is a one
  2. Prover: Indeed clearly pushes the one input variable into the converter. Meanwhile the cs: ConstraintSystem trait doesn't give access to its variables barring other checks on that.

There is a lot of complexity, one day, I hope we can cut out some parts from the R1CS->Assignment->Converter->ConstraintSystem->[Matrix;3] chain.

@howardwu howardwu merged commit 6133fce into mainnet Jan 19, 2024
77 of 78 checks passed
@howardwu howardwu deleted the declare_public_once branch January 19, 2024 02:36
@howardwu howardwu changed the title Declare public variable 1field once [Fix] Declare public variable 1field once Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Only declare the public variable 1field once
4 participants