Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Bitwise Builtin #395

Open
wants to merge 1 commit into
base: gali/integrate_verify_bitwise_xor
Choose a base branch
from

Conversation

Gali-StarkWare
Copy link
Contributor

@Gali-StarkWare Gali-StarkWare commented Jan 28, 2025

This change is Reviewable

Copy link
Contributor Author

Gali-StarkWare commented Jan 28, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@Gali-StarkWare Gali-StarkWare marked this pull request as ready for review January 28, 2025 12:58
@Gali-StarkWare Gali-StarkWare self-assigned this Jan 28, 2025
@Gali-StarkWare Gali-StarkWare changed the base branch from gali/bitwise to gali/integrate_verify_bitwise_xor January 28, 2025 15:50
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from bcde69c to 227eca8 Compare January 28, 2025 15:55
Copy link
Contributor

@ohad-starkware ohad-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @Gali-StarkWare and @shaharsamocha7)


stwo_cairo_prover/crates/prover/src/cairo_air/air.rs line 70 at r1 (raw file):

    pub verify_instruction: verify_instruction::Claim,
    pub builtins: BuiltinsClaim,
    pub verify_bitwise_xor_9: verify_bitwise_xor_9::Claim,

reorder after range checks ( v > r)


stwo_cairo_prover/crates/prover/src/cairo_air/builtins_air.rs line 72 at r1 (raw file):

                (bitwise.stop_ptr - bitwise.begin_addr).is_power_of_two(),
                "bitwise segment length is not a power of two"
            );

try using inspect here, I don't know if it's much nicer. just something I saw while on leave.

Code quote:

        let bitwise_builtin_trace_generator = builtin_segments.bitwise.map(|bitwise| {
            assert!(
                (bitwise.stop_ptr - bitwise.begin_addr).is_power_of_two(),
                "bitwise segment length is not a power of two"
            );

stwo_cairo_prover/crates/prover/src/cairo_air/builtins_air.rs line 109 at r1 (raw file):

            };
        let (bitwise_builtin_claim, bitwise_builtin_interaction_gen) =
            if let Some(bitwise_builtin_trace_generator) = self.bitwise_builtin_trace_generator {

map?

@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 227eca8 to c2b33a6 Compare January 29, 2025 13:17
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from c2b33a6 to bc21129 Compare January 30, 2025 09:11
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from bc21129 to b2bb977 Compare January 30, 2025 09:14
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 0175ecd to 29fe89c Compare January 30, 2025 15:13
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 29fe89c to 1b5772f Compare January 30, 2025 15:18
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 1b5772f to 3cf483f Compare January 30, 2025 15:23
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 3cf483f to 41ebc06 Compare January 30, 2025 15:29
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 41ebc06 to 1bd9a66 Compare January 30, 2025 15:37
Copy link
Contributor Author

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

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


stwo_cairo_prover/crates/prover/src/cairo_air/air.rs line 70 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

reorder after range checks ( v > r)

Done.


stwo_cairo_prover/crates/prover/src/cairo_air/builtins_air.rs line 72 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

try using inspect here, I don't know if it's much nicer. just something I saw while on leave.

Looks nice but runs on an iterator


stwo_cairo_prover/crates/prover/src/cairo_air/builtins_air.rs line 109 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

map?

I need a tuple of options here so it doesn't work

@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 1bd9a66 to 936db7d Compare January 30, 2025 15:56
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 936db7d to afb4699 Compare January 30, 2025 17:48
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from afb4699 to daa2b18 Compare February 2, 2025 08:28
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from daa2b18 to 9ba0339 Compare February 2, 2025 08:52
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 9ba0339 to 1bd00a1 Compare February 2, 2025 12:31
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 1bd00a1 to 6f29c03 Compare February 2, 2025 12:42
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 6f29c03 to 8679c4f Compare February 2, 2025 14:47
@Gali-StarkWare Gali-StarkWare force-pushed the gali/integrate_verify_bitwise_xor branch from 8679c4f to 4da0272 Compare February 2, 2025 14:51
Copy link
Contributor

@ohad-starkware ohad-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r5.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @Gali-StarkWare and @shaharsamocha7)


stwo_cairo_prover/crates/prover/src/cairo_air/builtins_air.rs line 109 at r1 (raw file):

Previously, Gali-StarkWare wrote…

I need a tuple of options here so it doesn't work

you can have this return an option of a tuple and then do the map from outside

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.

2 participants