-
Notifications
You must be signed in to change notification settings - Fork 996
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
In batch cell verification, take commitment for each cell #3834
Conversation
This will make verification slower, right? If you know which commitments are duplicates, then the commitment MSM will be smaller. |
It shouldn't make much of a difference. The public method will create a set of commitments and indices to each. The core verification method ( |
Instead of doing a review, I pushed two commits as you suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
Maybe we should see if @b-wagn can also review this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, API change seems reasonable to me.
In
verify_cell_kzg_proof_batch()
, we used to take as input a list of deduplicated commitments. This PR changes it so that it takes as input all the commitments. This is done for UI reasons since it's more natural for clients who do batch verification across multiple sidecars, and also for security reasons because it forces us to validate all the received sidecar commitments.Specifically, to verify all cells from column 0 and column 1 we would pass something like this to
verify_cell_kzg_proof_batch()
:whereas after this PR we would give inputs:
Keep in mind that the helper function
verify_cell_kzg_proof_batch_impl()
stays the same as before (using the old input format), and we have the public functionverify_cell_kzg_proof_batch()
move from the new input format to the old input format, by checking the commitments provided and recreatingrow_indices
.