Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(blockifier): rename execution_tast_output feild 'writes' to state_diff' #3871

Conversation

avivg-starkware
Copy link
Contributor

state_diff'

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link

github-actions bot commented Feb 2, 2025

@avivg-starkware avivg-starkware marked this pull request as ready for review February 2, 2025 12:25
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/execute_return_stetediff branch from 69a927b to b445c22 Compare February 2, 2025 12:52
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/execution_tast_output_rename_writes_to_state_diff branch from 21089c7 to 936c0cd Compare February 2, 2025 12:52
Copy link
Contributor Author

@avivg-starkware avivg-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: 0 of 2 files reviewed, 1 unresolved discussion


crates/blockifier/src/concurrency/worker_logic.rs line 146 at r1 (raw file):

                reads: transactional_state.cache.take().initial_reads,
                // Failed transaction - ignore the writes.
                state_diff: StateMaps::default(),

@Yoni-Starkware, I wasn't surre whether to edit the wording in the comment or not (to use 'state_diff' vs 'writes').

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.

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


crates/blockifier/src/concurrency/worker_logic_test.rs line 355 at r1 (raw file):

    // writes and reads to and from the sequencer balance (to avoid the inevitable dependency
    // between all the transactions).
    let state_diff = StateMaps {

This is the set of writes, there might be tirival updates here.

Suggestion:

et writes 

@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/execution_tast_output_rename_writes_to_state_diff branch from 936c0cd to 6012bb1 Compare February 2, 2025 14:02
Copy link
Contributor Author

@avivg-starkware avivg-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: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)


crates/blockifier/src/concurrency/worker_logic_test.rs line 355 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

This is the set of writes, there might be tirival updates here.

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:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

@avivg-starkware avivg-starkware changed the base branch from avivg/blockifier/execute_return_stetediff to graphite-base/3871 February 2, 2025 17:34
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/execution_tast_output_rename_writes_to_state_diff branch from 6012bb1 to 7a7c52f Compare February 2, 2025 17:34
@avivg-starkware avivg-starkware changed the base branch from graphite-base/3871 to main-v0.13.4 February 2, 2025 17:34
@avivg-starkware avivg-starkware merged commit 2165f34 into main-v0.13.4 Feb 2, 2025
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 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.

3 participants