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

Separate commitment prover and verifier to modules #482

Conversation

spapinistarkware
Copy link
Contributor

@spapinistarkware spapinistarkware commented Mar 15, 2024

This change is Reviewable

Copy link

graphite-app bot commented Mar 15, 2024

Your org has enabled the Graphite merge queue for merging into dev

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

This was referenced Mar 15, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 98.67550% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 95.09%. Comparing base (20412e4) to head (c69d9da).

Files Patch % Lines
src/core/commitment_scheme/verifier.rs 97.84% 2 Missing and 1 partial ⚠️
src/core/commitment_scheme/prover.rs 99.38% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #482   +/-   ##
=======================================
  Coverage   95.09%   95.09%           
=======================================
  Files          62       63    +1     
  Lines        9195     9195           
  Branches     9195     9195           
=======================================
  Hits         8744     8744           
  Misses        393      393           
  Partials       58       58           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 8692b73 to 55070e9 Compare March 19, 2024 14:27
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 55070e9 to 3ff5bbb Compare March 19, 2024 14:38
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 3ff5bbb to 46d3ad1 Compare March 19, 2024 14:48
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 46d3ad1 to 62672e7 Compare March 19, 2024 14:53
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 62672e7 to 8fe5ecd Compare March 19, 2024 15:22
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 8fe5ecd to ab770f3 Compare March 21, 2024 09:15
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from ab770f3 to 67c557c Compare March 21, 2024 10:43
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 67c557c to 59252fc Compare March 21, 2024 11:17
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 59252fc to f9ab969 Compare March 21, 2024 11:20
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-quotientops branch 3 times, most recently from fed4457 to 229a2f9 Compare March 24, 2024 13:10
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from f9ab969 to b658169 Compare March 24, 2024 13:10
@spapinistarkware spapinistarkware changed the base branch from spapini/03-15-quotientops to dev March 24, 2024 13:30
@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from b658169 to 095d7a8 Compare March 24, 2024 13:30
Copy link
Contributor

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)


src/core/commitment_scheme/prover.rs line 6 at r2 (raw file):

//! Note: This implementation is not really a polynomial commitment scheme, because we are not in
//! the unique decoding regime. This is enough for a STARK proof though, where we onyl want to imply
//! the existence of such polynomials, and re ok with having a small decoding list.

Move to mod.rs?

Code quote:

//! Implements a FRI polynomial commitment scheme.
//! This is a protocol where the prover can commit on a set of polynomials and then prove their
//! opening on a set of points.
//! Note: This implementation is not really a polynomial commitment scheme, because we are not in
//! the unique decoding regime. This is enough for a STARK proof though, where we onyl want to imply
//! the existence of such polynomials, and re ok with having a small decoding list.

src/core/commitment_scheme/verifier.rs line 27 at r2 (raw file):

type ProofChannel = Blake2sChannel;

/// The verifier side of a FRI polynomial commitment scheme. See [self].

Modify this.

Code quote:

See [self]

@spapinistarkware spapinistarkware force-pushed the spapini/03-15-separate_commitment_prover_and_verifier_to_modules branch from 095d7a8 to c69d9da Compare March 24, 2024 13:55
Copy link
Contributor Author

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alonh5)


src/core/commitment_scheme/prover.rs line 6 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Move to mod.rs?

Done.


src/core/commitment_scheme/verifier.rs line 27 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Modify this.

Done.

Copy link
Contributor

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @spapinistarkware)

@spapinistarkware spapinistarkware merged commit bcb5732 into dev Mar 25, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants