Reviewed 4 of 6 files at r1, all commit messages.
Reviewable status: 4 of 6 files reviewed, 6 unresolved discussions (waiting on @alonh5)

stwo_cairo_prover/rustfmt.toml line 1 at r1 (raw file):

# See: https://rust-lang.github.io/rustfmt

Can you merge it in a separate pr?

stwo_cairo_prover/src/components/mod.rs line 1 at r1 (raw file):

pub mod range_check;

Why?
Do you think we'll put here the other range checks?

Code quote:

pub mod range_check;

stwo_cairo_prover/src/components/range_check/component_prover.rs line 24 at r1 (raw file):

        accum: &mut ColumnAccumulator<'_, CpuBackend>,
        interaction_elements: &InteractionElements,
        _lookup_values: &LookupValues,

Remove

Code quote:

        _lookup_values: &LookupValues,

stwo_cairo_prover/src/components/range_check/mod.rs line 29 at r1 (raw file):

    pub fn register_test_rc() -> ComponentGenerationRegistry {
        let mut registry = ComponentGenerationRegistry::default();

Consider to get a mut registry as input and register to it.

Code quote:

let mut registry = ComponentGenerationRegistry::default();

stwo_cairo_prover/src/components/range_check/mod.rs line 139 at r1 (raw file):

        // let verifier_channel =
        //     &mut Blake2sChannel::new(Blake2sHasher::hash(BaseField::into_slice(&[])));
        // verify(proof, &air.to_air_prover(), verifier_channel).unwrap();

Verify doesn't work?

Code quote:

        // verify(proof, &air.to_air_prover(), verifier_channel).unwrap();

stwo_cairo_prover/src/components/range_check/component.rs line 77 at r1 (raw file):

    ) -> TreeVec<ColumnVec<Vec<CirclePoint<SecureField>>>> {
        TreeVec::new(vec![
            fixed_mask_points(&vec![vec![0_usize]; 2], point),

Was it a bug? the 2 change

Code quote:

fixed_mask_points(&vec![vec![0_usize]; 2], point),

Reviewable status: 4 of 6 files reviewed, 6 unresolved discussions (waiting on @shaharsamocha7)

stwo_cairo_prover/rustfmt.toml line 1 at r1 (raw file):

Done.

stwo_cairo_prover/src/components/mod.rs line 1 at r1 (raw file):

This is where we'll declare each component model. I'm not sure if that answers the question.

stwo_cairo_prover/src/components/range_check/component_prover.rs line 24 at r1 (raw file):

Done.

stwo_cairo_prover/src/components/range_check/mod.rs line 29 at r1 (raw file):

Done.

stwo_cairo_prover/src/components/range_check/mod.rs line 139 at r1 (raw file):

It does, forgot to uncomment.

stwo_cairo_prover/src/components/range_check/component.rs line 77 at r1 (raw file):

Yes there are 2 columns and not 1.

Reviewed 1 of 5 files at r2, all commit messages.
Reviewable status: 2 of 6 files reviewed, all discussions resolved

Reviewed 1 of 6 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alonh5)

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 16 at r2 (raw file):

use super::component::{RangeCheckUnitComponent, RC_Z};

impl RangeCheckUnitComponent {

Let's try to put impls on structs only near the struct definition. Here, you can make this a function instead of a method (i.e., not inside an impl).

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 17 at r2 (raw file):

impl RangeCheckUnitComponent {
    fn evaluate_lookup_boundary_constraints(

Bring this function down, after its use. This is not the first thing the reader should see in this file.

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

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 16 at r2 (raw file):

I removed this impl in a later PR (18).

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 17 at r2 (raw file):

I removed this function altogether in said PR (18).

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

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 16 at r2 (raw file):

But right now it's not near the component, and I will surely forget to check it by then.
Can you juse make it a function here instead? Shouldn't eb a lot of work

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

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 17 at r2 (raw file):

Can you do it now?

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

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 16 at r2 (raw file):

Done.

stwo_cairo_prover/src/components/range_check_unit/component_prover.rs line 17 at r2 (raw file):

Done.

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