Skip to content

Conversation

@AvivYossef-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Jan 8, 2026

Copy link
Contributor Author

AvivYossef-starkware commented Jan 8, 2026

Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

@Yoni-Starkware reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @AvivYossef-starkware, and @noaov1).


crates/starknet_os_runner/src/classes_provider_test.rs line 49 at r1 (raw file):

        round_tripped_casm.entry_points_by_type, original_casm.entry_points_by_type,
        "Entry points by type should match exactly"
    );

Do the opposite - erase irrelevant fields and do a single comparision

Code quote:

    // Verify bytecode matches exactly.
    assert_eq!(
        round_tripped_casm.bytecode, original_casm.bytecode,
        "Bytecode should match exactly"
    );

    // Verify bytecode_segment_lengths matches.
    assert_eq!(
        round_tripped_casm.bytecode_segment_lengths, original_casm.bytecode_segment_lengths,
        "Bytecode segment lengths should match"
    );

    // Verify hints match (as verified in existing test_hints_round_trip).
    assert_eq!(round_tripped_casm.hints, original_casm.hints, "Hints should match exactly");

    // Verify entry_points_by_type matches.
    assert_eq!(
        round_tripped_casm.entry_points_by_type, original_casm.entry_points_by_type,
        "Entry points by type should match exactly"
    );

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_hints_to_casm_from_contact_class branch 2 times, most recently from cac1bfc to c567eec Compare January 8, 2026 14:55
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_hints_to_casm_from_contact_class branch from c567eec to 10e42a9 Compare January 8, 2026 15:17
Copy link
Contributor Author

@AvivYossef-starkware AvivYossef-starkware left a comment

Choose a reason for hiding this comment

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

@AvivYossef-starkware made 1 comment.
Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware).


crates/starknet_os_runner/src/classes_provider_test.rs line 49 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Do the opposite - erase irrelevant fields and do a single comparision

Done.

Copy link
Collaborator

@avi-starkware avi-starkware left a comment

Choose a reason for hiding this comment

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

@avi-starkware reviewed 4 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noaov1 and @Yoni-Starkware).

Copy link
Collaborator

@avi-starkware avi-starkware left a comment

Choose a reason for hiding this comment

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

@avi-starkware made 2 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware, @noaov1, and @Yoni-Starkware).


crates/starknet_os_runner/src/classes_provider.rs line 27 at r2 (raw file):

/// Note: Some fields are not preserved in `CompiledClassV1` and are set to default values:
/// - `compiler_version`: Set to empty string
/// - `hints`: Set to empty (OS doesn't use them from this struct for Cairo 1 contracts)

Outdated comment

Hints are now set to
program_hints_to_casm_hints(&class.program.shared_program_data.hints_collection)?

Code quote:

/// - `hints`: Set to empty (OS doesn't use them from this struct for Cairo 1 contracts)

crates/starknet_os_runner/src/errors.rs line 54 at r2 (raw file):

    StateError(#[from] StateError),
    #[error(transparent)]
    ContractClassConversionError(#[from] ProgramError),

Consider naming this HintsConversionError or CasmHintsExtractionError

Code quote:

    ContractClassConversionError(#[from] ProgramError),

Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@Yoni-Starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @noaov1).

@AvivYossef-starkware AvivYossef-starkware changed the base branch from aviv/round_trip_hints to main January 11, 2026 08:01
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_hints_to_casm_from_contact_class branch from 10e42a9 to 497a582 Compare January 11, 2026 09:02
Copy link
Contributor Author

@AvivYossef-starkware AvivYossef-starkware left a comment

Choose a reason for hiding this comment

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

@AvivYossef-starkware made 2 comments.
Reviewable status: 1 of 6 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware).


crates/starknet_os_runner/src/classes_provider.rs line 27 at r2 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Outdated comment

Hints are now set to
program_hints_to_casm_hints(&class.program.shared_program_data.hints_collection)?

Thanks


crates/starknet_os_runner/src/errors.rs line 54 at r2 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Consider naming this HintsConversionError or CasmHintsExtractionError

Done.

Copy link
Collaborator

@avi-starkware avi-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@avi-starkware reviewed 5 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noaov1).

@AvivYossef-starkware AvivYossef-starkware added this pull request to the merge queue Jan 11, 2026
Merged via the queue into main with commit 64b42a9 Jan 11, 2026
32 of 37 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2026
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.

5 participants