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

Add Decider impl for Nova onchain #66

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Add Decider impl for Nova onchain #66

merged 2 commits into from
Feb 8, 2024

Conversation

arnaucube
Copy link
Collaborator

@arnaucube arnaucube commented Feb 5, 2024

  • Add Decider impl for Nova onchain's circuit.
  • Update also the Decider trait.

Nova onchain decider: (compressed SNARK / final proof), in order to later verify the Nova+CycleFold proofs onchain (in Ethereum’s EVM).

As a sidenote info, these past days I've been benchmarking the decider circuit, and next thing I'll do is to work on the step 6 of the decider circuit (src/folding/nova/decider_eth_circuit.rs#L418) because the current approach takes too many constraints.

@arnaucube arnaucube added Nova decider decider/compressed snark labels Feb 5, 2024
Add Decider impl for Nova onchain.
Update also the Decider trait.

Nova onchain decider: (compressed SNARK / final proof), in order to
later verify the Nova+CycleFold proofs onchain (in Ethereum’s EVM).
@arnaucube arnaucube force-pushed the feature/decider-eth branch from 32d3e28 to 6b19c2a Compare February 5, 2024 20:00
@arnaucube arnaucube marked this pull request as draft February 5, 2024 20:04
@arnaucube arnaucube marked this pull request as ready for review February 5, 2024 20:09
@arnaucube arnaucube requested review from CPerezz and han0110 February 5, 2024 20:10
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.

Unsure why do we add the MNT curves here. To avoid Foreign Field ops in-circuit?
If not can you explain?

Cargo.toml Outdated Show resolved Hide resolved
src/folding/circuits/nonnative.rs Outdated Show resolved Hide resolved
src/folding/circuits/nonnative.rs Outdated Show resolved Hide resolved
src/folding/circuits/nonnative.rs Outdated Show resolved Hide resolved
src/folding/circuits/nonnative.rs Outdated Show resolved Hide resolved
src/folding/nova/decider_eth.rs Outdated Show resolved Hide resolved
@arnaucube
Copy link
Collaborator Author

Unsure why do we add the MNT curves here. To avoid Foreign Field ops in-circuit? If not can you explain?

Ideally I would have avoided the use of the MNT curves, and would have used directly BN254 - Grumpkin cycle, but at the latest release of arkworks (v0.4.0) the BN254 does not have the constraints implemented, which was added recently (by you here hehe), so once we migrate to the new arkworks version (which is not yet released) we can enjoy the BN254 R1CS constraints. But for the moment for this test needed a cycle of curves with pairings, so used the MNT cycle for the test.

Regarding arkworks versions, there have been substantial improvements and fixes on many arkworks crates over this past year (since v0.4.0) release, just sent a msg in arkworks chat asking if there are plans to do a new release with all those changes that we would benefit from (eg. the mentioned bn254 constraints, some fixes, and maybe also the new configurable nonnativefield gadgets).

@CPerezz
Copy link
Member

CPerezz commented Feb 6, 2024

Ohhh I see!

Let's leave an issue filled to remember to update this once we move to the next version?

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!

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!

@arnaucube arnaucube added this pull request to the merge queue Feb 8, 2024
Merged via the queue into main with commit 97e973a Feb 8, 2024
3 checks passed
@arnaucube arnaucube deleted the feature/decider-eth branch February 14, 2024 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decider decider/compressed snark Nova
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants