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

Add binary deserialization to asterisc's state converter trace function #12238

Conversation

mininny
Copy link
Collaborator

@mininny mininny commented Oct 1, 2024

Description

Adds support for binary serialization of VMState for Asterisc, while preserving compatibility with JSON snapshots. We infer the type of the snapshot by looking at the file type, and dynamically choose whether to use binary representation of json representation.

This is dependency PR of Asterisc PR here: ethereum-optimism/asterisc#82

This is used in Asterisc's op-e2e tests, where a prestate file is converted into VMState instance in op-challenger trace provider

@mininny mininny marked this pull request as ready for review October 2, 2024 00:03
@mininny mininny requested review from a team as code owners October 2, 2024 00:03
@mininny mininny requested review from clabby and blmalone October 2, 2024 00:03
@mininny mininny force-pushed the feature/mininny/asterisc-binary-serialization branch 3 times, most recently from a9e250e to d5e9984 Compare October 2, 2024 19:48
@mininny
Copy link
Collaborator Author

mininny commented Oct 2, 2024

Currently includes diff for #12265, but will be rebase this PR to develop once it's merged.

@mininny mininny force-pushed the feature/mininny/asterisc-binary-serialization branch from d5e9984 to 9a92763 Compare October 2, 2024 20:00
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So good to see knowledge of the specific snapshot format moving out of challenger. Just need to make sure the witness subcommand in asterisc is giving back what you expect.

op-challenger/game/fault/trace/asterisc/state_converter.go Outdated Show resolved Hide resolved
@mininny mininny requested review from a team as code owners October 3, 2024 22:53
@mininny mininny requested review from ajsutton and geoknee October 3, 2024 22:53
@mininny mininny force-pushed the feature/mininny/asterisc-binary-serialization branch from 55d7533 to a55aea4 Compare October 3, 2024 23:00
@mininny mininny force-pushed the feature/mininny/asterisc-binary-serialization branch from a55aea4 to 9d908cc Compare October 3, 2024 23:10
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ajsutton ajsutton added this pull request to the merge queue Oct 4, 2024
Merged via the queue into ethereum-optimism:develop with commit b02dba0 Oct 4, 2024
55 checks passed
protolambda pushed a commit that referenced this pull request Oct 7, 2024
…on (#12238)

* Move serialize from cannon to op-service

* Add binary deserialization to asterisc's state converter trace function

* Use Asterisc's witness cmd to generate state data

* Fix op-challenger tests

* remove unnecessary logic from state_converter, and only test witness subcommand
samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
…on (ethereum-optimism#12238)

* Move serialize from cannon to op-service

* Add binary deserialization to asterisc's state converter trace function

* Use Asterisc's witness cmd to generate state data

* Fix op-challenger tests

* remove unnecessary logic from state_converter, and only test witness subcommand
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.

2 participants