Skip to content

integrate triple_xor gate#452

Open
Stavbe wants to merge 1 commit intomainfrom
stav/integare_triple_xor_gate
Open

integrate triple_xor gate#452
Stavbe wants to merge 1 commit intomainfrom
stav/integare_triple_xor_gate

Conversation

@Stavbe
Copy link
Copy Markdown
Collaborator

@Stavbe Stavbe commented Apr 14, 2026

No description provided.

@reviewable-StarkWare
Copy link
Copy Markdown
Collaborator

This change is Reviewable

Copy link
Copy Markdown
Collaborator Author

Stavbe commented Apr 14, 2026

@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch 2 times, most recently from 9cdddb1 to 715acf5 Compare April 14, 2026 13:42
@Stavbe Stavbe force-pushed the stav/integare_m31_to_u32 branch from 183f28a to b8c5cdd Compare April 14, 2026 13:49
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 715acf5 to 1f60ab1 Compare April 14, 2026 13:49
@Stavbe Stavbe force-pushed the stav/integare_m31_to_u32 branch from b8c5cdd to f8a0e30 Compare April 15, 2026 08:08
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 1f60ab1 to 4e84cce Compare April 15, 2026 08:09
@Stavbe Stavbe force-pushed the stav/integare_m31_to_u32 branch from f8a0e30 to 990ecca Compare April 15, 2026 10:22
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 4e84cce to 6fb04fe Compare April 15, 2026 10:22
@Stavbe Stavbe force-pushed the stav/integare_m31_to_u32 branch from 990ecca to f53b07b Compare April 15, 2026 13:09
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 6fb04fe to 3a0af98 Compare April 15, 2026 13:09
@Stavbe Stavbe force-pushed the stav/integare_m31_to_u32 branch 2 times, most recently from 77ca0f5 to 08adf4a Compare April 15, 2026 13:20
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch 2 times, most recently from a23d909 to 88d221c Compare April 15, 2026 14:18
@Stavbe Stavbe changed the base branch from stav/integare_m31_to_u32 to main April 15, 2026 14:18
@Stavbe Stavbe marked this pull request as ready for review April 15, 2026 14:33
@Stavbe Stavbe self-assigned this Apr 15, 2026
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 88d221c to 7b83cac Compare April 16, 2026 09:01
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

_typos.toml line 8 at r3 (raw file):

[default.extend-words]
consts = "consts"
ba = "ba"

?

Code quote:

ba = "ba"

Copy link
Copy Markdown
Collaborator

@az-starkware az-starkware left a comment

Choose a reason for hiding this comment

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

@az-starkware reviewed 22 files and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on alon-f, ilyalesokhin-starkware, and Stavbe).


crates/circuits/src/ivalue.rs line 55 at r3 (raw file):

    /// Packs a `u32` value into a QM31 limb representation `(low_u16, high_u16, 0, 0)`.
    fn pack_u32(value: u32) -> Self;

Not sure if "pack" is the best word here. Pack usually means we take a few things and put them back-to-back in some container, while here we take one thing and split it to fit the constraints of the container (that each limb can only store 31 bits).

Maybe "serialize" and "deserialize"?

Non-blocking.


crates/circuits/src/ivalue.rs line 100 at r3 (raw file):

    fn unpack_u32(&self) -> u32 {
        let [low, high, _, _] = self.to_m31_array().map(|m| m.0);

Assert that the two "unused" felts are zeros. If it hurts performace, you can only assert when running in debug mode.


crates/circuit_common/src/preprocessed.rs line 142 at r3 (raw file):

        mul,
        pointwise_mul,
        eq: _,

Replace ignored fields with ..


crates/circuit_common/src/preprocessed.rs line 330 at r3 (raw file):

        pointwise_mul: _,
        eq: _,
        blake,

Here too


crates/circuit_common/src/preprocessed.rs line 190 at r3 (raw file):

        mul: _,
        pointwise_mul: _,
        eq,

Here too


crates/circuit_common/src/preprocessed.rs line 371 at r3 (raw file):

/// Adds TripleXor gates to preprocessed trace columns.
/// | input_a_address | input_b_address | input_c_address | output_address | multiplicity |
fn fill_triple_xor_columns(

Don't use numeric column indices (e.g. columns[3]). One possible solution is to change this function to create the columns, and return a dictionary of id -> values. This way won't need the list of column names in add_triple_xor_to_preprocessed_trace.
Other solutions are also OK as long as we don't have a list of string IDs and a list of numeric IDs that have to be kept in sync.

@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 7b83cac to 5b904d3 Compare April 26, 2026 12:01
@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from 5b904d3 to fcb07cb Compare April 26, 2026 12:03
Copy link
Copy Markdown
Collaborator Author

@Stavbe Stavbe left a comment

Choose a reason for hiding this comment

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

@Stavbe made 7 comments.
Reviewable status: 16 of 22 files reviewed, 6 unresolved discussions (waiting on alon-f, az-starkware, and ilyalesokhin-starkware).


_typos.toml line 8 at r3 (raw file):

Previously, ilyalesokhin-starkware wrote…

?

Typos fail on the function name (which is auto-gen hash) in: crates/circuit_air/src/circuit_eval_components/triple_xor.rs


crates/circuit_common/src/preprocessed.rs line 142 at r3 (raw file):

Previously, az-starkware wrote…

Replace ignored fields with ..

Done.


crates/circuit_common/src/preprocessed.rs line 190 at r3 (raw file):

Previously, az-starkware wrote…

Here too

Done.


crates/circuit_common/src/preprocessed.rs line 330 at r3 (raw file):

Previously, az-starkware wrote…

Here too

Done.


crates/circuit_common/src/preprocessed.rs line 371 at r3 (raw file):

Previously, az-starkware wrote…

Don't use numeric column indices (e.g. columns[3]). One possible solution is to change this function to create the columns, and return a dictionary of id -> values. This way won't need the list of column names in add_triple_xor_to_preprocessed_trace.
Other solutions are also OK as long as we don't have a list of string IDs and a list of numeric IDs that have to be kept in sync.

Since literally all the other gates do this, I think this should be a TODO, and one should update all of them together once we decide on the chosen solution.


crates/circuits/src/ivalue.rs line 55 at r3 (raw file):

Previously, az-starkware wrote…

Not sure if "pack" is the best word here. Pack usually means we take a few things and put them back-to-back in some container, while here we take one thing and split it to fit the constraints of the container (that each limb can only store 31 bits).

Maybe "serialize" and "deserialize"?

Non-blocking.

I prefer 'pack' and this is the term I used in the DR.
@ilyalesokhin-starkware WDYT?


crates/circuits/src/ivalue.rs line 100 at r3 (raw file):

Previously, az-starkware wrote…

Assert that the two "unused" felts are zeros. If it hurts performace, you can only assert when running in debug mode.

Done.

@Stavbe Stavbe force-pushed the stav/integare_triple_xor_gate branch from fcb07cb to ae5d50d Compare April 26, 2026 12:54
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

crates/circuits/src/ivalue.rs line 55 at r3 (raw file):

Previously, Stavbe wrote…

I prefer 'pack' and this is the term I used in the DR.
@ilyalesokhin-starkware WDYT?

maybe from_u32:
"Takes a u32 and encodes it in an IVALUE, for QM31 the representation is `(low_u16, high_u16, 0, 0)."

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

crates/circuit_common/src/preprocessed.rs line 371 at r3 (raw file):

Previously, Stavbe wrote…

Since literally all the other gates do this, I think this should be a TODO, and one should update all of them together once we decide on the chosen solution.

We just want to write the first 5 columns here, and unfortunately, Rust doesn't have a nice syntax for that.

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.

4 participants