Use a single set of lookup elements for all relations

[az-starkware]

Separation between relations is done by prepending a unique "relation ID" M31 to each tuple used / yielded.

---

This change is 

[anatgstarkware]

@anatgstarkware reviewed 7 of 431 files at r1, 28 of 196 files at r3, 1 of 8 files at r4, 5 of 18 files at r5, 1 of 1 files at r6.
Reviewable status: 40 of 462 files reviewed, 7 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 4 at r5 (raw file):

use stwo::core::fields::m31::M31;
use stwo_constraint_framework::relation;

Please document - why we need these and how they are computed

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 18 at r5 (raw file):

pub const VERIFY_BITWISE_XOR_12_RELATION_ID: M31 = M31::from_u32_unchecked(648362599);

relation!(CommonLookupElements, 128);

Can we have a documented constant for that as well? Shouldn't that be the max of the list we just erased? Isn't 64 enough?

Code quote:

128

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 308 at r4 (raw file):

#[derive(Drop)]
pub struct CairoInteractionElements {

Why do we still need the rest of the elements?

---

stwo_cairo_prover/crates/cairo-air/src/air.rs line 599 at r4 (raw file):

pub struct CairoInteractionElements {
    pub common: relations::CommonLookupElements,
}

Can we remove this struct?:

Code quote:

pub struct CairoInteractionElements {
    pub common: relations::CommonLookupElements,
}

---

stwo_cairo_verifier/crates/cairo_air/src/components/poseidon_aggregator.cairo line 1 at r4 (raw file):

// This file was created by the AIR team.

please remove

Code quote:

// This file was created by the AIR team.

---

stwo_cairo_verifier/crates/constraint_framework/src/lib.cairo line 49 at r6 (raw file):

    /// both cases.
    fn combine_qm31<impl IntoSpan: ToSpanTrait<[QM31; N], QM31>>(
        self: @LookupElements<N>, values: Span<QM31>,

Suggestion:

mut values: Span<QM31>

---

stwo_cairo_verifier/crates/cairo_air/src/components/cube_252.cairo line 1 at r6 (raw file):

// This file was created by the AIR team.

please remove

Code quote:

// This file was created by the AIR team.

[anatgstarkware]

Reviewable status: 40 of 462 files reviewed, 8 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 112 at r6 (raw file):

pub type VerifyBitwiseXor_12Elements = LookupElements<3>;

pub type CommonElements = LookupElements<128>;

Can we remove the struct CairoInteractionElements and use only
pub type CommonLookupElements = LookupElements<64>;?

Code quote:

pub type CommonElements = LookupElements<128>;

[anatgstarkware]

@anatgstarkware reviewed 1 of 18 files at r5.
Reviewable status: 41 of 462 files reviewed, 8 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

[gilbens-starkware]

@gilbens-starkware reviewed 13 of 431 files at r1, 22 of 196 files at r3, 8 of 18 files at r5, 1 of 1 files at r6.
Reviewable status: 68 of 464 files reviewed, 9 unresolved discussions (waiting on @anatgstarkware and @az-starkware)

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 4 at r5 (raw file):

Previously, anatgstarkware (anatg) wrote…

Please document - why we need these and how they are computed

+1

---

stwo_cairo_verifier/crates/constraint_framework/src/lib.cairo line 48 at r6 (raw file):

    /// We use horner evaluation here regardless of the qm31_opcode feature flag as it is faster in
    /// both cases.
    fn combine_qm31<impl IntoSpan: ToSpanTrait<[QM31; N], QM31>>(

You can remove the generic now.

[anatgstarkware]

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 4 at r5 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

+1

You can write that these are not autogenerated ;)

[az-starkware]

Reviewable status: 35 of 476 files reviewed, 9 unresolved discussions (waiting on @anatgstarkware and @gilbens-starkware)

---

stwo_cairo_prover/crates/cairo-air/src/air.rs line 599 at r4 (raw file):

Previously, anatgstarkware (anatg) wrote…

Can we remove this struct?:

Done.

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 4 at r5 (raw file):

Previously, anatgstarkware (anatg) wrote…

You can write that these are not autogenerated ;)

Done.

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 18 at r5 (raw file):

Previously, anatgstarkware (anatg) wrote…

Can we have a documented constant for that as well? Shouldn't that be the max of the list we just erased? Isn't 64 enough?

Done. PartialEcMul requires 73, so I rounded up to 128.

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 308 at r4 (raw file):

Previously, anatgstarkware (anatg) wrote…

Why do we still need the rest of the elements?

Done.

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 112 at r6 (raw file):

Previously, anatgstarkware (anatg) wrote…

Can we remove the struct CairoInteractionElements and use only
pub type CommonLookupElements = LookupElements<64>;?

Done.

---

stwo_cairo_verifier/crates/cairo_air/src/components/cube_252.cairo line 1 at r6 (raw file):

Previously, anatgstarkware (anatg) wrote…

please remove

Done.

---

stwo_cairo_verifier/crates/cairo_air/src/components/poseidon_aggregator.cairo line 1 at r4 (raw file):

Previously, anatgstarkware (anatg) wrote…

please remove

Done.

---

stwo_cairo_verifier/crates/constraint_framework/src/lib.cairo line 49 at r6 (raw file):

    /// both cases.
    fn combine_qm31<impl IntoSpan: ToSpanTrait<[QM31; N], QM31>>(
        self: @LookupElements<N>, values: Span<QM31>,

It is an error to do so:

error: Parameter of trait function `LookupElementsTrait::combine_qm31` can't be defined as mutable.
 --> /work/git/stwo-cairo/stwo_cairo_verifier/crates/constraint_framework/src/lib.cairo:49:35
        self: @LookupElements<N>, mut values: Span<QM31>,

[anatgstarkware]

@anatgstarkware reviewed 6 of 18 files at r5, 29 of 110 files at r8, 1 of 1 files at r9, 4 of 4 files at r10, all commit messages.
Reviewable status: 74 of 476 files reviewed, 4 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

---

stwo_cairo_prover/crates/cairo-air/src/relations.rs line 18 at r5 (raw file):

Previously, az-starkware wrote…

Done. PartialEcMul requires 73, so I rounded up to 128.

As we talked, it's fine :)

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 65 at r10 (raw file):

use stwo_verifier_core::{ColumnSpan, TreeArray, TreeSpan};

pub type CommonElements = LookupElements<128>;

Please use a const here too
I prefer to call it CommonLookupElements - not a must

Code quote:

128

---

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 787 at r10 (raw file):

#[cfg(feature: "qm31_opcode")]
fn sum_public_memory_entries(

@gilbens-starkware can you please also check this part?

[anatgstarkware]

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 65 at r10 (raw file):

Previously, anatgstarkware (anatg) wrote…

Please use a const here too
I prefer to call it CommonLookupElements - not a must

Can we call LookupElements CommonLookupElements and remove CommonElements type?

[anatgstarkware]

stwo_cairo_verifier/crates/cairo_air/src/test_utils.cairo line 45 at r11 (raw file):

}

pub fn make_lookup_elements(z: QM31, alpha: QM31) -> LookupElements {

please remove this function

Code quote:

pub fn make_lookup_elements(z: QM31, alpha: QM31) -> LookupElements 

[anatgstarkware]

@anatgstarkware reviewed 4 of 5 files at r11, all commit messages.
Reviewable status: 73 of 476 files reviewed, 4 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

[anatgstarkware]

@anatgstarkware reviewed 1 of 5 files at r11, 5 of 44 files at r12, all commit messages.
Reviewable status: 72 of 476 files reviewed, 3 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

[az-starkware]

Reviewable status: 53 of 476 files reviewed, 3 unresolved discussions (waiting on @anatgstarkware and @gilbens-starkware)

---

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 65 at r10 (raw file):

Previously, anatgstarkware (anatg) wrote…

Can we call LookupElements CommonLookupElements and remove CommonElements type?

Done.

---

stwo_cairo_verifier/crates/cairo_air/src/test_utils.cairo line 45 at r11 (raw file):

Previously, anatgstarkware (anatg) wrote…

please remove this function

Done.

---

stwo_cairo_verifier/crates/constraint_framework/src/lib.cairo line 48 at r6 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

You can remove the generic now.

Done.

[anatgstarkware]

@anatgstarkware reviewed 20 of 190 files at r13, all commit messages.
Reviewable status: 73 of 476 files reviewed, 2 unresolved discussions (waiting on @az-starkware and @gilbens-starkware)

[gilbens-starkware]

@gilbens-starkware reviewed 1 of 1 files at r9, 3 of 190 files at r13, all commit messages.
Reviewable status: 73 of 476 files reviewed, 1 unresolved discussion (waiting on @anatgstarkware and @az-starkware)

---

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 787 at r10 (raw file):

Previously, anatgstarkware (anatg) wrote…

@gilbens-starkware can you please also check this part?

lgtm

---

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 832 at r13 (raw file):

    let mut alpha_powers = common_lookup_elements.alpha_powers.span();
    // Remove the first two elements (1 and alpha).

Can you add an explanation of why we need to also remove alpha now? (non blocking)

Code quote:

// Remove the first two elements (1 and alpha).

[anatgstarkware]

@anatgstarkware reviewed 1 of 1 files at r14, all commit messages.
Reviewable status: 73 of 476 files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[anatgstarkware]

@anatgstarkware reviewed 479 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @az-starkware).