wasm patch

[Okm165]

The primary focus of these changes is to enhance Web-Assembly(WASM) support

---

This change is 

[ohad-starkware]

what is the serialize/deserialize input used for?
I'd like to review those changes separately, the hashbrown thing and the wasm fix are looking good

[Okm165]

input serialize and deserialize is useful when separating trace_gen and prove stages
so after trace gen prover_input can be returned saved (to file or in JS runtime) etc. then it can be proven.
refs:
https://github.com/Okm165/stwo-web-stark/blob/master/backend/src/lib.rs#L45
https://github.com/Okm165/stwo-web-stark/blob/master/backend/src/lib.rs#L56

[ohad-starkware]

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

---

stwo_cairo_prover/crates/prover/src/input/builtin_segments.rs line 83 at r1 (raw file):

            .unwrap_or_else(|_| panic!("Unable to read file: {}", path.display()));
        let pub_data: PublicInput<'_> =
            serde_json::from_str(&pub_data_string).expect("Unable to parse JSON");

sonic is much faster, I see it has some no no-std dependencies, but did you make sure it can't be used with no std?

---

stwo_cairo_prover/Cargo.toml line 41 at r1 (raw file):

    "parallel",
], default-features = false}
thiserror-no-std = "2.0.2"

can this be thiserror-core with default-features = false?

[Okm165]

sonic-rs will support wasm in the future cloudwego/sonic-rs#70

[ohad-starkware]

alright, looks good.I triggered the CI, please rebase and see if we broke wasm support in the last few days, then ill run CI again

[Okm165]

default thiserror used with default-features = false

[Okm165]

Testing full flow on my side will ping u when ready

[Okm165]

@ohad-starkware All good! I merged latest main resolved conflicts and tested WASM build&functionality
Ready to merge imo

[ohad-starkware]

Reviewed 1 of 10 files at r3, all commit messages.
Reviewable status: 2 of 18 files reviewed, 1 unresolved discussion (waiting on @Okm165)

---

stwo_cairo_prover/crates/prover/src/input/vm_import/mod.rs line 49 at r3 (raw file):

        .max()
        .ok_or(VmImportError::NoMemorySegments)?;
    assert!(end_addr < 1 << (usize::BITS - 1));

can use P here?

[ohad-starkware]

stwo_cairo_prover/crates/prover/src/input/vm_import/mod.rs line 49 at r3 (raw file):

Previously, ohad-starkware (Ohad) wrote…

can use P here?

sorry about the nitpick, all looks good

[Okm165]

can use P here?

wdym P?

[ohad-starkware]

M31 prime

[Okm165]

Those are checks for MemorySegmentAddress space I personally don't see connection with M31 prime here too much but i may be wrong

[ohad-starkware]

im not sure why you changed it to 2^31, but after discussing,
the real memory bound for stwo proofs is 2^27 ((P + 1) >> 4) , you can revert your changes or fix it to 2^27.

Reviewable status: 2 of 18 files reviewed, 1 unresolved discussion (waiting on @Okm165)

[ohad-starkware]

stwo_cairo_prover/crates/prover/src/input/vm_import/mod.rs line 20 at r5 (raw file):

use crate::input::memory::MemoryBuilder;

pub const REAL_MEMORY_BOUND: usize = 1 << 27; // 2^27 ((M31 + 1) >> 4)

Suggestion:

pub const MEMORY_ADDRESS_BOUND: usize = 1 << 27; // ((P + 1) >> 4).

[ohad-starkware]

I hate that this change got coupled with the wasm patch.
we usually do small changes in prs and have a 1:1 relation between prs and commits
we already have this constant here

stwo-cairo/stwo_cairo_prover/crates/prover/src/components/memory/mod.rs

Line 6 in e30c584

pub const MEMORY_ADDRESS_BOUND: usize = 1 << LOG_MEMORY_ADDRESS_BOUND;

it's best not to define it again. why was the change here to begin with? did you get address at that range?
as for this pr: sorry about the ping -pong, use the defined constant or revert to the original state, and squash your commits so each represents a change, I think you should arrive at 4 commits.

Reviewed 4 of 13 files at r2, 3 of 10 files at r3, 1 of 1 files at r5.
Reviewable status: 9 of 18 files reviewed, 1 unresolved discussion (waiting on @Okm165)

[Okm165]

the memory bound was addressed in this WASM pr coz wasm has usize = u32 and original 1 << 32 was failing to compile due to overflow

[ohad-starkware]

I see, just squash the merge commits and ill lgtm

Reviewed 1 of 1 files at r8.
Reviewable status: 10 of 18 files reviewed, all discussions resolved

[Okm165]

@ohad-starkware tested full flow works on my end

[ohad-starkware]

you didnt squash the commits

Reviewable status: 1 of 61 files reviewed, all discussions resolved

[ohad-starkware]

Reviewed 1 of 18 files at r1, 5 of 13 files at r10, 5 of 53 files at r11, 46 of 47 files at r12, 3 of 3 files at r13, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Okm165)

[mellowcroc]

Is there a reason the PIE support was removed in this PR?

[Okm165]

@mellowcroc Dependencies required to run PIEs directly could not be compiled or executed within a WebAssembly environment.
https://github.com/starkware-libs/stwo-cairo/pull/304/files#diff-82a73df0651d83b95b00e3b98571860cc3837d3ee02fbb1470248bb89693e44bL80

[Okm165]

I think the runner of PIE should be externalized or enabled by additional feature flag, because it conflicts with multi-arch design.