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: remove storage_logs field from ExecutionState #1799

Merged
merged 1 commit into from Apr 30, 2024

Conversation

joonazan
Copy link
Contributor

Before this change CurrentExecutionState contained deduplicated storage logs and the raw storage logs. However, the fast VM cannot produce the raw storage logs, as those are an implementation detail. Luckily it seems that they aren't needed, at least for out-of-circuit use.

@joonazan
Copy link
Contributor Author

I expect that compiling prover will fail but let's see.

@joonazan
Copy link
Contributor Author

CI for prover was skipped, so prover doesn't depend on multivm. In that case this change shouldn't cause any problems.

@perekopskiy
Copy link
Contributor

@joonazan we need storage logs for each L2 block (deduplicated too but within each L2 block), is new VM capable of producing those?

@joonazan joonazan added this pull request to the merge queue Apr 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 29, 2024
@joonazan joonazan added this pull request to the merge queue Apr 30, 2024
Merged via the queue into main with commit 33b39bd Apr 30, 2024
37 checks passed
@joonazan joonazan deleted the remove-duplicated-storage-logs branch April 30, 2024 19:07
Deniallugo pushed a commit that referenced this pull request May 7, 2024
Before this change CurrentExecutionState contained deduplicated storage
logs and the raw storage logs. However, the fast VM cannot produce the
raw storage logs, as those are an implementation detail. Luckily it
seems that they aren't needed, at least for out-of-circuit use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants