Skip to content

Conversation

@AvivYossef-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/class_provider branch 2 times, most recently from 042c72c to ef84d39 Compare December 23, 2025 08:41
@AvivYossef-starkware AvivYossef-starkware changed the base branch from aviv/class_provider to graphite-base/10940 December 24, 2025 12:49
@AvivYossef-starkware AvivYossef-starkware changed the base branch from graphite-base/10940 to aviv/class_provider December 24, 2025 12:53
Copy link
Collaborator

@meship-starkware meship-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:

@meship-starkware partially reviewed 7 files and made 1 comment.
Reviewable status: 4 of 7 files reviewed, all discussions resolved (waiting on @avi-starkware and @noaov1).

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 3 files and made 4 comments.
Reviewable status: 5 of 7 files reviewed, 4 unresolved discussions (waiting on @avi-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1).


crates/starknet_os_runner/src/runner.rs line 31 at r3 (raw file):

    V: VirtualBlockExecutor,
{
    pub fn new(classes_provider: C, storage_proofs_provider: S, virtual_block_executor: V) -> Self {

Note: I hope to use this code for general block proofs, not just virtual ones. Not for our first milestone, but keep in mind.

Code quote:

virtual_block_executor

crates/starknet_os_runner/src/runner.rs line 35 at r3 (raw file):

    }

    /// Creates OS input from execution data and storage proofs.

Suggestion:

    /// Creates the OS input hint required to run the given transactions virtually on top of the given block number.

crates/starknet_os_runner/src/runner.rs line 42 at r3 (raw file):

        txs: Vec<(InvokeTransaction, TransactionHash)>,
    ) -> Result<StarknetOsInput, RunnerError> {
        // 1. Execute virtual block and get execution data.

Remove the numbers from the documentation (annoying to maintain)

Code quote:

 // 1.

crates/starknet_os_runner/src/runner.rs line 94 at r3 (raw file):

            // We assume that no classes are migrated.
            class_hashes_to_migrate: HashMap::new(),
            // Stored block hash buffer skipped for now.

Suggestion:

// Virtual OS does not update the block hash mapping

@AvivYossef-starkware AvivYossef-starkware changed the base branch from aviv/class_provider to graphite-base/10940 December 25, 2025 06:47
@AvivYossef-starkware AvivYossef-starkware changed the base branch from graphite-base/10940 to aviv/class_provider December 25, 2025 08:21
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 and resolved 2 discussions.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @meship-starkware, @noaov1, and @Yoni-Starkware).


crates/starknet_os_runner/src/runner.rs line 31 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Note: I hope to use this code for general block proofs, not just virtual ones. Not for our first milestone, but keep in mind.

The differences are
preprocess,
finalize,
writing to state
and new block hash right?


crates/starknet_os_runner/src/runner.rs line 35 at r3 (raw file):

    }

    /// Creates OS input from execution data and storage proofs.

Done.

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 6 files and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @AvivYossef-starkware, and @noaov1).


crates/starknet_os_runner/src/runner.rs line 31 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

The differences are
preprocess,
finalize,
writing to state
and new block hash right?

Yes

@AvivYossef-starkware AvivYossef-starkware changed the base branch from aviv/class_provider to main December 25, 2025 15:47
@graphite-app
Copy link

graphite-app bot commented Dec 25, 2025

Merge activity

  • Dec 25, 3:47 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

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 resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @avi-starkware and @noaov1).

@AvivYossef-starkware AvivYossef-starkware added this pull request to the merge queue Dec 25, 2025
Merged via the queue into main with commit d2731a7 Dec 25, 2025
13 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 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.

5 participants