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.

Sign up for GitHub

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

Jump to bottom

Feat/clean compute pi #795

Merged
merged 13 commits into from
Jul 28, 2023
Merged

Feat/clean compute pi #795

merged 13 commits into from
Jul 28, 2023

Conversation

ThomasPiellard
Copy link
Collaborator

Plonk verifier modifs:

  • Verify is now in a single assembly block -> compute_pi and derive_gamma_beta_alpha_zeta are now in the same block
  • Sanity checks are now in the main block in Verify

@ThomasPiellard ThomasPiellard added consolidate strengthen an existing feature zk-evm labels Jul 27, 2023
ivokub
ivokub approved these changes Jul 28, 2023
View reviewed changes
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

LGTM. It is mostly refactoring, but with slight changes (some functions return non-reduced values for challenge computation).

backend/plonk/bn254/solidity.go Outdated

let mPtr := mload(0x40)
let state := mload(0x40)
let mPtr := add(mload(0x40), state_last_mem)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't we use state as the first argument here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep 👍

@ThomasPiellard ThomasPiellard merged commit 716f7df into develop Jul 28, 2023
1 check passed
@ThomasPiellard ThomasPiellard deleted the feat/clean_compute_pi branch July 28, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivokub ivokub ivokub approved these changes

@thedarkjester thedarkjester Awaiting requested review from thedarkjester

@julien-marchand julien-marchand Awaiting requested review from julien-marchand

Assignees
No one assigned
Labels
consolidate strengthen an existing feature zk-evm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ThomasPiellard @ivokub