Add FRI layer 0 commitment

[andrewmilson]

This change is 

[andrewmilson]

• Optimize FRI quotient constants (Work In Progress) #205
• Add FRI layer 0 commitment #186 👈 (View in Graphite)
• Change stwo_cairo_verifier:: to crate:: #185 : 1 other dependent PR (#191 )
• Remove pow function #184
• Implement Serde on StarkProof #164
• Update Cairo to version 2.8.4 #174
• Optimize hash_node #163
• Optimize poly folding #162
• Implement batch_inverse on M31,CM31 #137 : 1 other dependent PR (#138 )
• Optimize Coset::at #135
• Refactor FriVerifier::decommit_inner_layers #134
• Add README for cairo verifier #153
• main

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

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/pcs/quotients.cairo line 30 at r1 (raw file):

    mut query_positions_per_log_size: Felt252Dict<Nullable<Span<usize>>>,
    queried_values_per_column: @Array<Array<M31>>,
    mut missing_values: Span<QM31>,

Please document the arguments.

Code quote:

    log_size_per_column: @Array<u32>,
    samples_per_column: @Array<Array<PointSample>>,
    random_coeff: QM31,
    mut query_positions_per_log_size: Felt252Dict<Nullable<Span<usize>>>,
    queried_values_per_column: @Array<Array<M31>>,
    mut missing_values: Span<QM31>,

[andrewmilson]

Reviewable status: 0 of 29 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware and @shaharsamocha7)

---

stwo_cairo_verifier/src/pcs/quotients.cairo line 30 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

Please document the arguments.

Done.

---

stwo_cairo_verifier/src/utils.cairo line 14 at r3 (raw file):

/// Look up table where index `i` stores value `2^i`.
const POW_2: [u32; 32] = [
    0b1, // 

Formatter in new version wants to squash multiple into the same line. The empty comment stops that from happening

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/queries.cairo line 47 at r5 (raw file):

    fn len(self: @Queries) -> usize {
        (*self.positions).len()

doesn't span have len?

Suggestion:

 self.positions.len()

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/queries.cairo line 55 at r5 (raw file):

        Queries {
            positions: get_folded_query_positions(self.positions, n_folds),
            // TODO(andrew): Check Cairo does checked sub in all profiles.

it does, sierra is type safe.

only felt252 can overflow.

Code quote:

 // TODO(andrew): Check Cairo does checked sub in all profiles.

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/queries.cairo line 75 at r5 (raw file):

    for position in query_positions {
        folded_positions.append(*position / folding_factor);
    };

Consider deduping inside this loop.

Suggestion:

    for position in query_positions {
        new_position = *position / folding_factor;
        if new_position != prev_position
            folded_positions.append(*position / folding_factor);
        prev_position = new_position.
    };

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/utils.cairo line 166 at r5 (raw file):

    fn first(mut self: Span<T>) -> Option<@T> {
        self.pop_front()
    }

Does this work?

Suggestion:

    fn first(self: @Span<T>) -> Option<@T> {
        *self.pop_front()
    }

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/utils.cairo line 184 at r5 (raw file):

            if value == other {
                self = self_copy;
                return Option::Some(other);

nice!

Code quote:

                self = self_copy;
                return Option::Some(other);

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/pcs/quotients.cairo line 242 at r5 (raw file):

/// * `sampled_batches`: OOD column samples grouped by eval point.
/// * `queried_values_by_column`: Sampled query evals by trace column.
/// * `row`: The index of the query to compute the quotients for.

Suggestion:

/// * `query_idx`: The index of the query to compute the quotients for

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/pcs/quotients.cairo line 262 at r5 (raw file):

    let mut row_accumulator: QM31 = Zero::zero();

    for batch_i in 0..n_batches {

You have 4 loops here.

I'd consider refactoring it in the future as

1. compute an array if (numerator, denumerator)
2. batch_inverse + row_accumulator.

you can add a todo.

Code quote:

  for batch_i in 0..n_batches {

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/vcs/verifier.cairo line 300 at r5 (raw file):

    use core::nullable::NullableTrait;
    use core::result::ResultTrait;
    use stwo_cairo_verifier::fields::m31::m31;

revert

Code quote:

  use stwo_cairo_verifier::fields::m31::m31;

[ilyalesokhin-starkware]

Reviewed 2 of 13 files at r1, 4 of 17 files at r3, 9 of 24 files at r5, all commit messages.
Reviewable status: 13 of 29 files reviewed, 7 unresolved discussions (waiting on @andrewmilson and @shaharsamocha7)

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/pcs/quotients.cairo line 38 at r5 (raw file):

    // Group columns by log size.
    // TODO(andrew): Refactor. Use a dict.

If the columns are stored by size is the dict still nessiary?

Code quote:

 // TODO(andrew): Refactor. Use a dict.

[andrewmilson]

Reviewable status: 13 of 29 files reviewed, 7 unresolved discussions (waiting on @ilyalesokhin-starkware and @shaharsamocha7)

---

stwo_cairo_verifier/src/queries.cairo line 47 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

doesn't span have len?

Span does but @Span doesn't

---

stwo_cairo_verifier/src/queries.cairo line 55 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

it does, sierra is type safe.

only felt252 can overflow.

Great. Removed

---

stwo_cairo_verifier/src/queries.cairo line 75 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

Consider deduping inside this loop.

Done.

---

stwo_cairo_verifier/src/utils.cairo line 166 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

Does this work?

Complains "ref argument must be a variable".
It's possible with

    fn first(self: @Span<T>) -> Option<@T> {
        let mut self_copy = *self;
        self_copy.pop_front()
    }

but leaving as is seems more consistent with the other Span impls like .len() being implemented on Span not @Span. WDYT?

---

stwo_cairo_verifier/src/pcs/quotients.cairo line 38 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

If the columns are stored by size is the dict still nessiary?

Ahh nice. True

---

stwo_cairo_verifier/src/pcs/quotients.cairo line 262 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

You have 4 loops here.

I'd consider refactoring it in the future as

1. compute an array if (numerator, denumerator)
2. batch_inverse + row_accumulator.

you can add a todo.

Discussed offline.

---

stwo_cairo_verifier/src/vcs/verifier.cairo line 300 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

revert

Done.

---

stwo_cairo_verifier/src/pcs/quotients.cairo line 242 at r5 (raw file):

/// * `sampled_batches`: OOD column samples grouped by eval point.
/// * `queried_values_by_column`: Sampled query evals by trace column.
/// * `row`: The index of the query to compute the quotients for.

Done.

[ilyalesokhin-starkware]

stwo_cairo_verifier/src/utils.cairo line 166 at r5 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

Complains "ref argument must be a variable".
It's possible with

    fn first(self: @Span<T>) -> Option<@T> {
        let mut self_copy = *self;
        self_copy.pop_front()
    }

but leaving as is seems more consistent with the other Span impls like .len() being implemented on Span not @Span. WDYT?

I find it strange that you must consume the span to get the first element.

Maybe:

fn first(self: @span) -> Option<@t> {
self.iter().next()
}

[ilyalesokhin-starkware]

stwo_cairo_verifier/tests/verifier.cairo line 20 at r6 (raw file):

//     let test_proof = proofs::test_proof::proof();
//     println!("TEST PROOF WORKED");
// }

?

Code quote:

// #[test]
// fn test_deserialise() {
//     let test_proof = proofs::test_proof::proof();
//     println!("TEST PROOF WORKED");
// }

[ilyalesokhin-starkware]

Reviewed 11 of 24 files at r5, 6 of 6 files at r6.
Reviewable status: 27 of 29 files reviewed, 2 unresolved discussions (waiting on @andrewmilson, @shaharsamocha7, @span, and @t)

[andrewmilson]

Reviewable status: 26 of 29 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware, @shaharsamocha7, @span, and @t)

---

stwo_cairo_verifier/src/utils.cairo line 166 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

I find it strange that you must consume the span to get the first element.

Maybe:

fn first(self: @span) -> Option<@t> {
self.iter().next()
}

No iter. Only into_iter() which is also implemented on Span

---

stwo_cairo_verifier/tests/verifier.cairo line 20 at r6 (raw file):

Previously, ilyalesokhin-starkware wrote…

?

Done.

[ilyalesokhin-starkware]

Reviewed 2 of 24 files at r5, 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)