Skip to content

Conversation

@nimrod-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

nimrod-starkware commented Jul 3, 2025

@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from d1698db to e3ebcf7 Compare July 3, 2025 09:14
@nimrod-starkware nimrod-starkware self-assigned this Jul 3, 2025
@nimrod-starkware nimrod-starkware marked this pull request as ready for review July 3, 2025 09:15
@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 3, 2025 13:07
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to nimrod/cairo_runner/pointers_to_maybe July 3, 2025 13:08
@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 3, 2025 13:10
@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from e3ebcf7 to 67903f4 Compare July 6, 2025 06:39
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to nimrod/cairo_runner/pointers_to_maybe July 6, 2025 06:40
@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 6, 2025 09:02
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to nimrod/cairo_runner/pointers_to_maybe July 6, 2025 11:22
Copy link
Collaborator

@amosStarkware amosStarkware left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @nimrod-starkware)


crates/starknet_os/src/test_utils/cairo_runner.rs line 571 at r2 (raw file):

    state_reader: Option<DictStateReader>,
) -> Cairo0EntryPointRunnerResult<(Vec<EndpointArg>, Vec<EndpointArg>, CairoRunner)> {
    let (mut cairo_runner, program, entrypoint) = initialize_cairo_runner(

Suggestion:

/// This function is split into to sub-functions to allow advanced use cases,
/// which require access to the cairo runner before running the entrypoint.
let (mut cairo_runner, program, entrypoint) = initialize_cairo_runner(

crates/starknet_os/src/test_utils/cairo_runner.rs line 595 at r2 (raw file):

    program_bytes: &[u8],
    entrypoint: &str,
    implicit_args: &[ImplicitArg],

Suggestion:

implicit_args: &[ImplicitArg], // Used to infer the builtins the program uses.

@nimrod-starkware nimrod-starkware force-pushed the nimrod/cairo_runner/pointers_to_maybe branch from 414735b to c51a852 Compare July 6, 2025 12:33
@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from 67903f4 to 295c5e9 Compare July 6, 2025 12:33
Copy link
Contributor Author

@nimrod-starkware nimrod-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: all files reviewed, 2 unresolved discussions (waiting on @amosStarkware)


crates/starknet_os/src/test_utils/cairo_runner.rs line 571 at r2 (raw file):

    state_reader: Option<DictStateReader>,
) -> Cairo0EntryPointRunnerResult<(Vec<EndpointArg>, Vec<EndpointArg>, CairoRunner)> {
    let (mut cairo_runner, program, entrypoint) = initialize_cairo_runner(

Done.
you mean add as a comment, right? (with two slashes)


crates/starknet_os/src/test_utils/cairo_runner.rs line 595 at r2 (raw file):

    program_bytes: &[u8],
    entrypoint: &str,
    implicit_args: &[ImplicitArg],

Done.

Copy link
Collaborator

@amosStarkware amosStarkware 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 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware)


crates/starknet_os/src/test_utils/cairo_runner.rs line 571 at r2 (raw file):

Previously, nimrod-starkware wrote…

Done.
you mean add as a comment, right? (with two slashes)

Yes, thanks.
non blocking - please change to -> two, my bad

@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 6, 2025 14:19
@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from 295c5e9 to 29ccbce Compare July 7, 2025 07:00
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to nimrod/cairo_runner/pointers_to_maybe July 7, 2025 07:00
@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 7, 2025 10:32
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to nimrod/cairo_runner/pointers_to_maybe July 7, 2025 10:36
@nimrod-starkware nimrod-starkware force-pushed the nimrod/cairo_runner/pointers_to_maybe branch from 92c0134 to fb900d4 Compare July 7, 2025 15:24
@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from 29ccbce to 6684907 Compare July 7, 2025 15:24
@nimrod-starkware nimrod-starkware changed the base branch from nimrod/cairo_runner/pointers_to_maybe to graphite-base/7744 July 7, 2025 15:59
@nimrod-starkware nimrod-starkware force-pushed the nimrod/aliases_test/refactor_runner branch from 6684907 to 778e7d8 Compare July 7, 2025 15:59
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/7744 to main-v0.14.0 July 7, 2025 16:00
Copy link
Contributor Author

@nimrod-starkware nimrod-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 4 of 4 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware)

@nimrod-starkware nimrod-starkware added this pull request to the merge queue Jul 7, 2025
Merged via the queue into main-v0.14.0 with commit f305794 Jul 7, 2025
15 of 25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants