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

Feature/nova ivc #36

Merged
merged 10 commits into from
Nov 24, 2023
Merged

Feature/nova ivc #36

merged 10 commits into from
Nov 24, 2023

Conversation

arnaucube
Copy link
Collaborator

This PR implements Nova's IVC (new, prove, verify methods) (without the CycleFold part yet).

Copy link
Collaborator

@han0110 han0110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just got some small qs

src/constants.rs Outdated Show resolved Hide resolved
src/folding/nova/circuits.rs Outdated Show resolved Hide resolved
src/folding/nova/ivc.rs Outdated Show resolved Hide resolved
…onstraints in AugmentedFCircuit

(constraint count went down ~6k)
Comment on lines +1 to +3
// used for committed instances hash, so when going to the other curve of the cycle it does not
// overflow the scalar field
pub const N_BITS_HASH: usize = 250;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we tie this somehow to F::NUM_BITS or something similar?

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just some suggestions & comments

src/folding/nova/circuits.rs Outdated Show resolved Hide resolved
src/folding/nova/ivc.rs Outdated Show resolved Hide resolved
src/folding/nova/ivc.rs Outdated Show resolved Hide resolved
src/folding/nova/ivc.rs Show resolved Hide resolved
src/folding/nova/ivc.rs Outdated Show resolved Hide resolved
src/folding/nova/mod.rs Outdated Show resolved Hide resolved
src/folding/nova/nifs.rs Outdated Show resolved Hide resolved
src/folding/nova/nifs.rs Show resolved Hide resolved
src/folding/nova/nifs.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
…ld, and rm transcript from nova/IVC.

- Updates from PR suggestions
- Additionally updated:
  - in nova/nifs.rs: reuse folded_committed_instance for verify_folded_instance, computationally is the same, but reusing the same code so avoiding duplication and having an error on one of the two versions.
  - in nova/ivc.rs: remove transcript from IVC (not needed, it uses the RO)
@arnaucube arnaucube added this pull request to the merge queue Nov 24, 2023
Merged via the queue into main with commit 905ba44 Nov 24, 2023
3 checks passed
@CPerezz CPerezz deleted the feature/nova-ivc branch November 24, 2023 11:17
@arnaucube arnaucube restored the feature/nova-ivc branch November 24, 2023 18:21
@arnaucube
Copy link
Collaborator Author

note: restored the branch of this PR temporarily just to rebase to main some other branches that were depending on this one (as I did also on #35 (comment) )

@arnaucube arnaucube deleted the feature/nova-ivc branch November 24, 2023 18:36
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.

3 participants