Skip to content

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

Merged
avivg-starkware merged 1 commit intomain-v0.13.4from
avivg/blockifier/execution_tast_output_rename_writes_to_state_diff
Feb 2, 2025
Merged

chore(blockifier): rename execution_tast_output feild 'writes' to state_diff'#3871
avivg-starkware merged 1 commit intomain-v0.13.4from
avivg/blockifier/execution_tast_output_rename_writes_to_state_diff

Conversation

@avivg-starkware
Copy link
Contributor

state_diff'

@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Feb 2, 2025

Copy link
Contributor Author

avivg-starkware 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
9 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