feat(apollo_starknet_os_program): define and implement static Program #5873

dorimedini-starkware · 2025-04-15T14:13:15Z

No description provided.

reviewable-StarkWare · 2025-04-15T14:13:21Z

This change is

dorimedini-starkware · 2025-04-15T14:13:27Z

feat(apollo_starknet_os_program): add command to dump aggregator program #5943 : 2 dependent PRs (#5944 , #5945 )
feat(apollo_starknet_os_program): add aggregator program + program hash(es) #5937
test(ci): prepare apollo_starknet_os_program for publishing #5933
feat(starknet_committer_and_os_cli): add command to dump the OS compiled program to file #5876
feat(apollo_starknet_os_program): add fixer for program hash test #5875
feat(apollo_starknet_os_program): define and implement static ProgramHash #5874
feat(apollo_starknet_os_program): define and implement static Program #5873 👈 (View in Graphite)
fix(ci): properly install python deps, in CI and docker #5902 : 1 other dependent PR (#6266 )
feat(apollo_infra_utils): better cairo0 compiler checks #5921
chore(apollo_infra_utils): explicit instructions in verify_cairo0_compiler_deps #5872
chore(apollo_infra_utils,blockifier_test_utils): move verify_cairo0_compiler_deps to infra_utils #5871
main-v0.14.0

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

github-actions · 2025-04-15T14:14:42Z

Artifacts upload workflows:

github-actions · 2025-04-15T14:47:20Z

Benchmark movements: No major performance changes detected.

amosStarkware

Reviewed 2 of 4 files at r2, 8 of 10 files at r3, 2 of 2 files at r4.
Reviewable status: 11 of 12 files reviewed, 8 unresolved discussions (waiting on @nimrod-starkware and @TzahiTaub)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 44 at r4 (raw file):

#[derive(thiserror::Error, Debug)]
pub enum Cairo0CompilerError {

Since there are two cairo0 compilers, WYT of adding an error enum for each one?
and each enum can have Cairo0CompilerError as a variant.
or any other way of distinguishing which compiler failed

Code quote:

pub enum Cairo0CompilerError {

crates/apollo_infra_utils/src/cairo0_compiler.rs line 89 at r4 (raw file):

    cairo_root_path: PathBuf,
) -> Result<Vec<u8>, Cairo0CompilerError> {
    cairo0_compilers_correct_version()?;

Why verify the Starknet compiler is the correct version, if it's not used?

Code quote:

cairo0_compilers_correct_version()?;

crates/apollo_infra_utils/src/cairo0_compiler.rs line 92 at r4 (raw file):

    if !path_to_main.exists() {
        return Err(Cairo0CompilerError::SourceFileNotFound(path_to_main));
    }

Consider verifying it's a cairo file

Code quote:

    if !path_to_main.exists() {
        return Err(Cairo0CompilerError::SourceFileNotFound(path_to_main));
    }

crates/apollo_infra_utils/src/cairo0_compiler.rs line 103 at r4 (raw file):

        cairo_root_path
            .to_str()
            .ok_or(Cairo0CompilerError::InvalidPath(cairo_root_path.clone()))?,

--cairo_path tells the compiler what the 'root' dir is? i.e., the path the cairo imports are relative to?
what's --debug_info_with_source?

Code quote:

        "--debug_info_with_source",
        "--cairo_path",
        cairo_root_path
            .to_str()
            .ok_or(Cairo0CompilerError::InvalidPath(cairo_root_path.clone()))?,

crates/apollo_starknet_os_program/src/lib.rs line 15 at r4 (raw file):

pub static OS_PROGRAM: LazyLock<Program> = LazyLock::new(|| {
    Program::from_bytes(
        include_bytes!(concat!(env!("OUT_DIR"), "/starknet_os_bytes")),

Maybe add a get_output_dir() function, instead of using the OUT_DIR variable directly?

Code quote:

env!("OUT_DIR")

scripts/install_build_tools.sh line 16 at r4 (raw file):

            libzstd-dev \
            python3-dev \
            python3-venv \

you're installing this?
this is from std Python & should be safe, right?

Code quote:

python3-venv

.github/workflows/committer_ci.yml line 88 at r4 (raw file):

          LD_LIBRARY_PATH: ${{ env.Python3_ROOT_DIR }}/bin
        run: echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" >> $GITHUB_ENV
      - run: pip install -r scripts/requirements.txt

Why does the committer CI need these?

Code quote:

      # Setup pypy and link to the location expected by .cargo/config.toml.
      # Python + requirements are needed to compile the OS.
      - uses: actions/setup-python@v5
        id: setup-pypy
        with:
          python-version: "pypy3.9"
          cache: 'pip'
      - run: ln -s '${{ steps.setup-pypy.outputs.python-path }}' /usr/local/bin/pypy3.9
      - env:
          LD_LIBRARY_PATH: ${{ env.Python3_ROOT_DIR }}/bin
        run: echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" >> $GITHUB_ENV
      - run: pip install -r scripts/requirements.txt

      - id: auth
        uses: "google-github-actions/auth@v2"
        with:
          credentials_json: ${{ secrets.COMMITER_PRODUCTS_EXT_WRITER_JSON }}
      - uses: 'google-github-actions/setup-gcloud@v2'
      - run: echo "BENCH_INPUT_FILES_PREFIX=$(cat ./crates/starknet_committer_and_os_cli/src/committer_cli/tests/flow_test_files_prefix)" >> $GITHUB_ENV
      - run: gcloud storage cp -r gs://committer-testing-artifacts/$BENCH_INPUT_FILES_PREFIX/* ./crates/starknet_committer_and_os_cli/test_inputs
      - run: cargo test -p starknet_committer_and_os_cli --release -- --include-ignored test_regression

  benchmarking:
    runs-on: starkware-ubuntu-24.04-medium
    if: ${{ github.event_name == 'pull_request' }}
    steps:
      # Checkout the base branch to get the old code.
      - uses: actions/checkout@v4
        with:
          ref: ${{ github.base_ref }}
      - uses: ./.github/actions/bootstrap
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}

      # Setup pypy and link to the location expected by .cargo/config.toml.
      # Python + requirements are needed to compile the OS.
      - uses: actions/setup-python@v5
        id: setup-pypy
        with:
          python-version: "pypy3.9"
          cache: 'pip'
      - run: ln -s '${{ steps.setup-pypy.outputs.python-path }}' /usr/local/bin/pypy3.9
      - env:
          LD_LIBRARY_PATH: ${{ env.Python3_ROOT_DIR }}/bin
        run: echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" >> $GITHUB_ENV
      - run: pip install -r scripts/requirements.txt

crates/apollo_starknet_os_program/build/compile_program.rs line 17 at r5 (raw file):

                 not have a conflicting installation in your {}/bin directory.\nOriginal error: \
                 {error:?}.",
                std::env::var("CARGO_HOME").unwrap_or("${CARGO_HOME}".to_string())

Is this better?
https://docs.rs/home/latest/home/fn.cargo_home.html

Code quote:

std::env::var("CARGO_HOME").unwrap_or("${CARGO_HOME}".to_string())

dorimedini-starkware

Reviewable status: 11 of 12 files reviewed, 8 unresolved discussions (waiting on @amosStarkware, @nimrod-starkware, and @TzahiTaub)

.github/workflows/committer_ci.yml line 88 at r4 (raw file):

Previously, amosStarkware wrote…

Why does the committer CI need these?

because now the CLI depends on the OS program crate, so building the CLI requires building this crate, which requires cairo-lang (and python)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 44 at r4 (raw file):

Previously, amosStarkware wrote…

Since there are two cairo0 compilers, WYT of adding an error enum for each one?
and each enum can have Cairo0CompilerError as a variant.
or any other way of distinguishing which compiler failed

I am kind of against this, because you never really use both compilers in the same context - you are either compiling contracts, or OS/aggregator code. if you can think of a use case, PLMK

crates/apollo_infra_utils/src/cairo0_compiler.rs line 89 at r4 (raw file):

Previously, amosStarkware wrote…

Why verify the Starknet compiler is the correct version, if it's not used?

both compilers (and in the near future: the two compilers + the formatter) are all from cairo-lang and should be of identical version

crates/apollo_infra_utils/src/cairo0_compiler.rs line 92 at r4 (raw file):

Previously, amosStarkware wrote…

Consider verifying it's a cairo file

this will be implicit - the compiler will fail to compile :)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 103 at r4 (raw file):

Previously, amosStarkware wrote…

--cairo_path tells the compiler what the 'root' dir is? i.e., the path the cairo imports are relative to?
what's --debug_info_with_source?

yes
from the docs: Include debug information with a copy of the source code.; this flag is passed where we currently compile the OS

crates/apollo_starknet_os_program/build/compile_program.rs line 17 at r5 (raw file):

Previously, amosStarkware wrote…

Is this better?
https://docs.rs/home/latest/home/fn.cargo_home.html

this requires a new crate (home), not worth it; it's only a suggestion. happened to me during development of this feature, I just thought to document it

crates/apollo_starknet_os_program/src/lib.rs line 15 at r4 (raw file):

Previously, amosStarkware wrote…

Maybe add a get_output_dir() function, instead of using the OUT_DIR variable directly?

include_bytes! must accept a string literal, so I can't use functions here

scripts/install_build_tools.sh line 16 at r4 (raw file):

Previously, amosStarkware wrote…

you're installing this?
this is from std Python & should be safe, right?

yes; required for the Dockerfile change

dorimedini-starkware

Reviewable status: 11 of 12 files reviewed, 8 unresolved discussions (waiting on @amosStarkware, @nimrod-starkware, and @TzahiTaub)

.github/workflows/committer_ci.yml line 88 at r4 (raw file):

Previously, dorimedini-starkware wrote…

because now the CLI depends on the OS program crate, so building the CLI requires building this crate, which requires cairo-lang (and python)

ah... actually, this looks like a rebase issue... the workflows are updated here, where it's needed... I'll fix

amosStarkware

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware, @nimrod-starkware, and @TzahiTaub)

.github/workflows/committer_ci.yml line 88 at r4 (raw file):

Previously, dorimedini-starkware wrote…

ah... actually, this looks like a rebase issue... the workflows are updated here, where it's needed... I'll fix

This is the committer CI, not committer and os CLI . why does the committer need to load the OS?

crates/apollo_infra_utils/src/cairo0_compiler.rs line 89 at r4 (raw file):

Previously, dorimedini-starkware wrote…

both compilers (and in the near future: the two compilers + the formatter) are all from cairo-lang and should be of identical version

Still weird to verify the version for a compiler that isn't used, but not important

crates/apollo_infra_utils/src/cairo0_compiler.rs line 92 at r4 (raw file):

Previously, dorimedini-starkware wrote…

this will be implicit - the compiler will fail to compile :)

then why verify the path exists?
but not important

crates/apollo_starknet_os_program/build/compile_program.rs line 17 at r5 (raw file):

Previously, dorimedini-starkware wrote…

this requires a new crate (home), not worth it; it's only a suggestion. happened to me during development of this feature, I just thought to document it

Removed block
I don't understand the second part of the message? what did you document?

dorimedini-starkware

Reviewable status: 10 of 12 files reviewed, 3 unresolved discussions (waiting on @amosStarkware, @nimrod-starkware, and @TzahiTaub)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 92 at r4 (raw file):

Previously, amosStarkware wrote…

then why verify the path exists?
but not important

because otherwise you get an uninformative OS error, "path does not exist", without the path in the output

crates/apollo_starknet_os_program/build/compile_program.rs line 17 at r5 (raw file):

Previously, amosStarkware wrote…

Removed block
I don't understand the second part of the message? what did you document?

in one of the iterations, I tried running locally, and noticed cargo test prepended ${CARGO_HOME}/bin to my PATH variable. I had a cairo-compile executable there for some reason, and it took precedent over the installed executable from cairo-lang... so I added this error message to warn devs that this can happen

.github/workflows/committer_ci.yml line 88 at r4 (raw file):

Previously, amosStarkware wrote…

This is the committer CI, not committer and os CLI . why does the committer need to load the OS?

this workflow runs benchmarks for the committer via the starknet_committer_and_os_cli crate

amosStarkware

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware and @TzahiTaub)

amosStarkware

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware and @TzahiTaub)

.github/workflows/committer_ci.yml line 41 at r8 (raw file):

      # Setup pypy and link to the location expected by .cargo/config.toml.
      # Python + requirements are needed to compile the OS.

Can you explain in comment why this is needed? in both places in this file

Code quote:

# Python + requirements are needed to compile the OS.

amosStarkware

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware and @TzahiTaub)

dorimedini-starkware requested review from TzahiTaub and nimrod-starkware April 15, 2025 14:15

dorimedini-starkware self-assigned this Apr 15, 2025

dorimedini-starkware marked this pull request as ready for review April 15, 2025 14:16

dorimedini-starkware force-pushed the 04-13-chore_apollo_infra_utils_explicit_instructions_in_verify_cairo0_compiler_deps branch from 3ce0665 to 186d311 Compare April 15, 2025 14:31

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 97349df to 608a692 Compare April 15, 2025 14:31

dorimedini-starkware marked this pull request as draft April 15, 2025 14:32

dorimedini-starkware force-pushed the 04-13-chore_apollo_infra_utils_explicit_instructions_in_verify_cairo0_compiler_deps branch from 186d311 to 01f24e5 Compare April 15, 2025 15:08

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch 2 times, most recently from 7c2ee53 to f5f2e37 Compare April 15, 2025 15:48

dorimedini-starkware force-pushed the 04-13-chore_apollo_infra_utils_explicit_instructions_in_verify_cairo0_compiler_deps branch from 01f24e5 to ed9e2c1 Compare April 15, 2025 16:15

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch 6 times, most recently from e0bf542 to de9f216 Compare April 16, 2025 10:55

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from 1ebf665 to d31d2a7 Compare May 19, 2025 22:10

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 5954e4d to 84eeaec Compare May 19, 2025 22:10

dorimedini-starkware mentioned this pull request May 21, 2025

test(apollo_starknet_os_program): test all non-deprecated syscalls have gas costs #6639

Closed

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from d31d2a7 to 8ca6af6 Compare May 25, 2025 14:54

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 84eeaec to 3265e5d Compare May 25, 2025 14:54

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from 8ca6af6 to 969db30 Compare May 25, 2025 15:26

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 3265e5d to 8ad4e5b Compare May 25, 2025 15:26

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from 969db30 to 9bd6376 Compare May 25, 2025 17:09

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 8ad4e5b to 1500f5b Compare May 25, 2025 17:09

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from 9bd6376 to 09bb755 Compare May 26, 2025 07:55

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 1500f5b to eedaf6d Compare May 26, 2025 08:01

dorimedini-starkware force-pushed the 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker branch from 09bb755 to c9f149c Compare May 26, 2025 09:50

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from eedaf6d to 7c19baf Compare May 26, 2025 09:50

amosStarkware requested changes May 26, 2025

View reviewed changes

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 7c19baf to 1216f60 Compare May 26, 2025 11:51

dorimedini-starkware changed the base branch from 04-16-fix_ci_properly_install_python_deps_in_ci_and_docker to main-v0.14.0 May 26, 2025 11:51

dorimedini-starkware commented May 26, 2025

View reviewed changes

amosStarkware requested changes May 26, 2025

View reviewed changes

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 1216f60 to 30e8ec6 Compare May 26, 2025 13:10

dorimedini-starkware commented May 26, 2025

View reviewed changes

feat(apollo_starknet_os_program): define and implement static Program

ea3a188

amosStarkware approved these changes May 26, 2025

View reviewed changes

dorimedini-starkware force-pushed the 04-13-feat_apollo_starknet_os_program_define_and_implement_static_program branch from 30e8ec6 to ea3a188 Compare May 26, 2025 13:43

amosStarkware requested changes May 26, 2025

View reviewed changes

amosStarkware approved these changes May 26, 2025

View reviewed changes

dorimedini-starkware added this pull request to the merge queue May 26, 2025

Merged via the queue into main-v0.14.0 with commit 18c9bc7 May 26, 2025
25 checks passed

github-actions bot locked and limited conversation to collaborators May 28, 2025

feat(apollo_starknet_os_program): define and implement static Program #5873

feat(apollo_starknet_os_program): define and implement static Program #5873

Uh oh!

Conversation

dorimedini-starkware commented Apr 15, 2025

Uh oh!

reviewable-StarkWare commented Apr 15, 2025

Uh oh!

dorimedini-starkware commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amosStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

dorimedini-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

dorimedini-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

amosStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

dorimedini-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

amosStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

amosStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

amosStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dorimedini-starkware commented Apr 15, 2025 •

edited

Loading

github-actions bot commented Apr 15, 2025 •

edited

Loading

github-actions bot commented Apr 15, 2025 •

edited

Loading