FRI in Commitment Scheme

[spapinistarkware]

This change is 

[graphite-app]

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.

[spapinistarkware]

• parallel wide fib test #511
• Evaluation oods once #496
• Reuse commitment evaluation #495
• Use precomputed twiddles #494
• Add measurement logs #493
• WideFib test with AVX Backend #492
• Implement Backend for AVX512Backend #491
• Make prove() generic in Backend #490
• CommitmentSchemeProver generic in backend #489
• AVX quotients #488
• Clean up fri quotients #486
• Save inverse on quotient numerator #484
• Commitment Scheme evaluation per size #483
• Separate commitment prover and verifier to modules #482
• QuotientOps #481
• DomainAccumulator allows accumulating a row at once #480
• Rename evalaution.rs to accumulaion.rs #479
• Move SecureColumn #478
• Use only canonic domains #463
• Revert "integrated batch inverse in fib cpu (#448)" #477
• FRI in Commitment Scheme #476 👈
• Field debug implementations #501
• CommitmentScheme utils #500
• Air with dyn component #487
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @spapinistarkware and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

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

Project coverage is 94.49%. Comparing base (4c48ae4) to head (7fa8661).

Files	Patch %	Lines
src/core/commitment_scheme/mod.rs	98.03%	3 Missing and 1 partial ⚠️
src/core/mod.rs	60.00%	2 Missing ⚠️
src/core/prover/mod.rs	98.61%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #476      +/-   ##
==========================================
+ Coverage   94.16%   94.49%   +0.32%     
==========================================
  Files          60       60              
  Lines        9240     9260      +20     
  Branches     9240     9260      +20     
==========================================
+ Hits         8701     8750      +49     
+ Misses        485      458      -27     
+ Partials       54       52       -2     

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

[alonh5]

Reviewed 1 of 3 files at r4, 4 of 7 files at r6, 1 of 5 files at r8, 2 of 2 files at r10, all commit messages.
Reviewable status: 8 of 10 files reviewed, 15 unresolved discussions (waiting on @spapinistarkware)

---

src/core/mod.rs line 27 at r9 (raw file):

impl<T: Clone> ComponentVec<T> {
    pub fn flatten(&self) -> ColumnVec<T> {
        self.iter().flatten().cloned().collect()

Are we okay with cloning things? What if it's a circle evaluation for example?

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

type ProofChannel = Blake2sChannel;

/// The prover size of a FRI polynomial commitment scheme. See [self].

Suggestion:

side

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

type ProofChannel = Blake2sChannel;

/// The prover size of a FRI polynomial commitment scheme. See [self].

What is this supposed to point at?
Same below.

Code quote:

See [self]

---

src/core/commitment_scheme/mod.rs line 65 at r9 (raw file):

    }

    pub fn polys(&self) -> TreeVec<ColumnVec<&CPUCirclePoly>> {

Suggestion:

polynomials(&self)

---

src/core/commitment_scheme/mod.rs line 77 at r9 (raw file):

    }

    pub fn open_values(

Suggestion:

prove_values

---

src/core/commitment_scheme/mod.rs line 6 at r10 (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.

Suggestion:

//! the unique decoding regime. This is enough for a STARK proof though, where we only want to imply
//! the existence of such polynomials, and are ok with having a small decoding list.

---

src/core/commitment_scheme/mod.rs line 152 at r10 (raw file):

/// Prover data for a single commitment tree in a commitment scheme. The commitment scheme allows to
/// commit on a set polynomials at a time. This corresponds to such a set.

Suggestion:

set of polynomials

---

src/core/commitment_scheme/mod.rs line 227 at r10 (raw file):

    }

    /// A [TreeVec<ColumnVec] of the log sizes of each column in each commitment tree.

Suggestion:

TreeVec<ColumnVec>

---

src/core/commitment_scheme/mod.rs line 243 at r10 (raw file):

    }

    pub fn verify_opening(

Suggestion:

verify_values

---

src/core/commitment_scheme/mod.rs line 251 at r10 (raw file):

        channel.mix_felts(&proof.opened_values.clone().flatten_cols());

        // Compute degree bounds for oods quotients without looking at the proof.

Same below.

Suggestion:

OODS

---

src/core/commitment_scheme/mod.rs line 253 at r10 (raw file):

        // Compute degree bounds for oods quotients without looking at the proof.
        let bounds = self
            .column_log_sizes()

Are you assuming these are ordered?
TAL at how it's done in quotient_log_bounds(), and after that it can be removed.

Code quote:

.column_log_sizes()

---

src/core/commitment_scheme/mod.rs line 280 at r10 (raw file):

                tree.verify(
                    decommitment,
                    &fri_query_domains[&(tree.log_sizes[0] + 1)].flatten(),

Same below.

Suggestion:

 LOG_BLOWUP_FACTOR

---

src/core/commitment_scheme/mod.rs line 328 at r10 (raw file):

    value: SecureField,
) -> SparseCircleEvaluation<SecureField> {
    let queried_values = &mut queried_values.clone().into_iter();

Is this necessary?

Code quote:

.clone()

---

src/core/commitment_scheme/mod.rs line 341 at r10 (raw file):

            .collect(),
    );
    assert!(queried_values.is_empty());

Add message.

Code quote:

assert!(queried_values.is_empty());

---

src/core/prover/mod.rs line 68 at r10 (raw file):

    let mut open_points = TreeVec::new(vec![open_points.flatten()]);
    // Second tree - composition polynomial.
    open_points.0.push(vec![vec![oods_point]; 4]);

Remove wherever possible.

Code quote:

.0

[spapinistarkware]

Reviewable status: 8 of 10 files reviewed, 10 unresolved discussions (waiting on @alonh5)

---

src/core/mod.rs line 27 at r9 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Are we okay with cloning things? What if it's a circle evaluation for example?

We're not ok with this, no.

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

What is this supposed to point at?
Same below.

The module doc.

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

type ProofChannel = Blake2sChannel;

/// The prover size of a FRI polynomial commitment scheme. See [self].

Done.

---

src/core/commitment_scheme/mod.rs line 6 at r10 (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.

Done.

---

src/core/commitment_scheme/mod.rs line 152 at r10 (raw file):

/// Prover data for a single commitment tree in a commitment scheme. The commitment scheme allows to
/// commit on a set polynomials at a time. This corresponds to such a set.

Done.

---

src/core/commitment_scheme/mod.rs line 251 at r10 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Same below.

Done.

---

src/core/commitment_scheme/mod.rs line 253 at r10 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Are you assuming these are ordered?
TAL at how it's done in quotient_log_bounds(), and after that it can be removed.

It's gonna change in a few PRs anyway. I'll gather them up by size.

---

src/core/commitment_scheme/mod.rs line 280 at r10 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Same below.

Done.

---

src/core/commitment_scheme/mod.rs line 341 at r10 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Add message.

Done.

---

src/core/prover/mod.rs line 68 at r10 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Remove wherever possible.

Done.

[shaharsamocha7]

Reviewed 1 of 3 files at r4, 4 of 7 files at r6.
Reviewable status: 6 of 11 files reviewed, 16 unresolved discussions (waiting on @alonh5 and @spapinistarkware)

---

src/core/commitment_scheme/mod.rs line 3 at r11 (raw file):

//! 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.

Mention that we can open points from the domain

Code quote:

//! This is a protocol where the prover can commit on a set of polynomials and then prove their
//! opening on a set of points.

---

src/core/commitment_scheme/mod.rs line 43 at r11 (raw file):

/// The prover side of a FRI polynomial commitment scheme. See [self].
pub struct CommitmentSchemeProver {

Document.
Commitment of multiple polynomials (could be of different degrees?)

Code quote:

pub struct CommitmentSchemeProver {

---

src/core/commitment_scheme/mod.rs line 105 at r11 (raw file):

                    })
                    .collect_vec()
            });

Should this use batch_inverse?

Code quote:

        let quotients = self
            .evaluations()
            .zip_cols(&opened_values)
            .zip_cols(&open_points)
            .map_cols(|((evaluation, values), points)| {
                zip(points, values)
                    .map(|(&point, &value)| {
                        get_pair_oods_quotient(point, value, evaluation).bit_reverse()
                    })
                    .collect_vec()
            });

---

src/core/commitment_scheme/mod.rs line 108 at r11 (raw file):

        // Run FRI commitment phase on the oods quotients.
        let fri_config = FriConfig::new(LOG_LAST_LAYER_DEGREE_BOUND, LOG_BLOWUP_FACTOR, N_QUERIES);

Consider moving configs to the top of the function

Code quote:

        // Run FRI commitment phase on the oods quotients.
        let fri_config = FriConfig::new(LOG_LAST_LAYER_DEGREE_BOUND, LOG_BLOWUP_FACTOR, N_QUERIES);

---

src/core/commitment_scheme/mod.rs line 248 at r11 (raw file):

        proof: CommitmentSchemeProof,
        channel: &mut ProofChannel,
    ) -> bool {

This should return an error?

Code quote:

) -> bool {

---

src/core/commitment_scheme/mod.rs line 321 at r11 (raw file):

}

fn eval_quotients_on_sparse_domain(

Document

Code quote:

fn eval_quotients_on_sparse_domain(

[alonh5]

Reviewed 3 of 4 files at r11, all commit messages.
Reviewable status: 9 of 11 files reviewed, 10 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

src/core/mod.rs line 32 at r11 (raw file):

impl<T> ComponentVec<ColumnVec<T>> {
    pub fn flatten_all(self) -> Vec<T> {

For consistency.

Suggestion:

flatten_cols

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

The module doc.

Got it, thanks.

---

src/core/commitment_scheme/mod.rs line 144 at r11 (raw file):

pub struct CommitmentSchemeProof {
    pub opened_values: TreeVec<ColumnVec<Vec<SecureField>>>,

The open notion is a bit confusing to me, can we change it to prove everywhere related?
Also maybe add documentation for the fields here.

Suggestion:

pub proved_values: TreeVec<ColumnVec<Vec<SecureField>>>,

---

src/core/commitment_scheme/mod.rs line 283 at r11 (raw file):

                )
            })
            .0

[alonh5]

Reviewable status: 9 of 11 files reviewed, 10 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

src/core/commitment_scheme/mod.rs line 3 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Mention that we can open points from the domain

Can't?

[spapinistarkware]

Reviewable status: 9 of 11 files reviewed, 9 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

src/core/mod.rs line 32 at r11 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

For consistency.

Done.

---

src/core/commitment_scheme/mod.rs line 42 at r9 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Got it, thanks.

Done.

---

src/core/commitment_scheme/mod.rs line 3 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Mention that we can open points from the domain

Done.

---

src/core/commitment_scheme/mod.rs line 43 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Document.
Commitment of multiple polynomials (could be of different degrees?)

I doced in the module level, and referenced form here

---

src/core/commitment_scheme/mod.rs line 105 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Should this use batch_inverse?

Later.

---

src/core/commitment_scheme/mod.rs line 108 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Consider moving configs to the top of the function

Noted. This is constructed right before using it => in FRI.

---

src/core/commitment_scheme/mod.rs line 144 at r11 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

The open notion is a bit confusing to me, can we change it to prove everywhere related?
Also maybe add documentation for the fields here.

Done.

---

src/core/commitment_scheme/mod.rs line 248 at r11 (raw file):

Previously, shaharsamocha7 wrote…

This should return an error?

Yes.

---

src/core/commitment_scheme/mod.rs line 321 at r11 (raw file):

Previously, shaharsamocha7 wrote…

Document

Done.

[spapinistarkware]

Reviewable status: 7 of 11 files reviewed, 9 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

src/core/commitment_scheme/mod.rs line 3 at r11 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Done.

Done.

[alonh5]

Reviewed 3 of 4 files at r12, all commit messages.
Reviewable status: 10 of 11 files reviewed, 8 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

src/core/commitment_scheme/mod.rs line 81 at r12 (raw file):

    pub fn prove_values(
        &self,
        open_points: TreeVec<ColumnVec<Vec<CirclePoint<SecureField>>>>,

prove_points? points_to_prove?
Same below (everywhere that uses open).

Code quote:

open_points

---

src/core/commitment_scheme/mod.rs line 293 at r12 (raw file):

        // Answer FRI queries.
        let mut fri_answers = self

Do you think all these collect()s (in zip_cols, map_cols etc.) are going be costly? If so maybe add a TODO to consider a refactor of this code chunk?

[spapinistarkware]

Reviewable status: 10 of 11 files reviewed, 8 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

src/core/commitment_scheme/mod.rs line 293 at r12 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Do you think all these collect()s (in zip_cols, map_cols etc.) are going be costly? If so maybe add a TODO to consider a refactor of this code chunk?

No.

[alonh5]

Non-blocking comments on tests, for next PR.

Reviewed 2 of 2 files at r13, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

src/fibonacci/mod.rs line 165 at r13 (raw file):

    #[test]
    fn test_oods_quotients_are_low_degree() {

This test was to check the oods quotients are low degree (because when we only excluded one point and not a pair it wasn't), not that the CP is low degree.

---

src/fibonacci/mod.rs line 226 at r13 (raw file):

        let trace = fib.get_trace();
        let trace_poly = trace.interpolate();
        let _trace = ComponentTrace::new(vec![&trace_poly]);

You can remove these.

---

src/fibonacci/mod.rs line 254 at r13 (raw file):

            .commitment_scheme_proof
            .proved_values
            .0

You can now remove these too.

Code quote:

.0

[shaharsamocha7]

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

[spapinistarkware]

Reviewable status: 10 of 12 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @shaharsamocha7)

---

src/fibonacci/mod.rs line 165 at r13 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

This test was to check the oods quotients are low degree (because when we only excluded one point and not a pair it wasn't), not that the CP is low degree.

So it should be in oods.rs. I put one there.

[spapinistarkware]

Merge activity

• Mar 21, 3:27 AM EDT: @spapinistarkware merged this pull request with Graphite.