Skip to content

refactor(blockifier): storage access tracker#3582

Merged
aner-starkware merged 1 commit intomainfrom
aner/storage_access_tracker
Jan 28, 2025
Merged

refactor(blockifier): storage access tracker#3582
aner-starkware merged 1 commit intomainfrom
aner/storage_access_tracker

Conversation

@aner-starkware
Copy link
Contributor

No description provided.

@aner-starkware aner-starkware self-assigned this Jan 21, 2025
@reviewable-StarkWare
Copy link

This change is Reviewable

@aner-starkware aner-starkware force-pushed the aner/storage_access_tracker branch 3 times, most recently from b572293 to 63de7a7 Compare January 22, 2025 09:17
Copy link
Collaborator

@dorimedini-starkware dorimedini-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 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @Yoni-Starkware)


a discussion (no related file):
need a py side PR here, this is a breaking change

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.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)


a discussion (no related file):

Previously, dorimedini-starkware wrote…

need a py side PR here, this is a breaking change

Why are we doing this? nice to have? we might end up moving the sets out of this class

Copy link
Collaborator

@dorimedini-starkware dorimedini-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, 1 unresolved discussion (waiting on @aner-starkware and @Yoni-Starkware)


a discussion (no related file):

Previously, Yoni-Starkware (Yoni) wrote…

Why are we doing this? nice to have? we might end up moving the sets out of this class

nice to have. makes implementing any later decision easier

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.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)


a discussion (no related file):

Previously, dorimedini-starkware wrote…

nice to have. makes implementing any later decision easier

Cool

@aner-starkware aner-starkware force-pushed the aner/storage_access_tracker branch from 63de7a7 to c3adb90 Compare January 23, 2025 16:20
@aner-starkware
Copy link
Contributor Author

Previously, Yoni-Starkware (Yoni) wrote…

Cool

Python side:
https://reviewable.io/reviews/starkware-industries/starkware/36812

@aner-starkware aner-starkware force-pushed the aner/storage_access_tracker branch 2 times, most recently from b1cc191 to 7925e34 Compare January 23, 2025 18:07
Copy link
Collaborator

@dorimedini-starkware dorimedini-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 3 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware)


crates/starknet_consensus_orchestrator/resources/central_transaction_execution_info.json line 0 at r3 (raw file):
what's this diff...?

@aner-starkware aner-starkware force-pushed the aner/storage_access_tracker branch from 7925e34 to 1a448e1 Compare January 27, 2025 20:56
Copy link
Collaborator

@dorimedini-starkware dorimedini-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: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware)

@aner-starkware aner-starkware force-pushed the aner/storage_access_tracker branch from 1a448e1 to bc2bbac Compare January 28, 2025 07:41
@aner-starkware
Copy link
Contributor Author

crates/starknet_consensus_orchestrator/resources/central_transaction_execution_info.json line at r3 (raw file):

Previously, dorimedini-starkware wrote…

what's this diff...?

nesting

Copy link
Collaborator

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

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.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aner-starkware)

@aner-starkware aner-starkware added this pull request to the merge queue Jan 28, 2025
Merged via the queue into main with commit 1e592b2 Jan 28, 2025
11 checks passed
Itay-Tsabary-Starkware pushed a commit that referenced this pull request Jan 30, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2025
@aner-starkware aner-starkware deleted the aner/storage_access_tracker branch February 9, 2025 12:30
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