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

Use common.Hash in VMState - Update Monorepo submodule to 457f33f #68

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

ImTei
Copy link
Collaborator

@ImTei ImTei commented Jun 19, 2024

Description

Use common.Hash type in VMState instead of bytes array. it would improve readability of output files and we don't need to make proof file to get prestate.

Updated:

  • We need the same change in the monorepo Asterisc VMState. The PR has been opened in the monorepo: op-challenger: Use common.Hash type in Asterisc VMState optimism#10967
  • This PR temporarily pinned the tip fork as the monorepo version. Once monorepo PR is merged, this PR must updates the monorepo dependency version.
  • This PR has some changes to make it compatible with the new monorepo version.
    • To avoid solidity import path issues between rvsol and monorepo contracts, I have moved library files in rvsol/scripts/lib directory, and changed forge-std/ to @forge-std/.

@ImTei ImTei requested a review from pcw109550 June 19, 2024 23:36
@ajsutton
Copy link
Collaborator

I think we'll need to update op-challenger for this as well since it reads that field right?

@ImTei
Copy link
Collaborator Author

ImTei commented Jun 19, 2024

@ajsutton I naively guessed it works.. but it seems something's broken in the CI. Let me check what should I fix more, including op-challenger.

@ajsutton
Copy link
Collaborator

My guess is we'll have to change https://github.com/ethereum-optimism/optimism/blob/8447d1653a32d2210d8aa2f12c43889de4c3b1c7/op-challenger/game/fault/trace/asterisc/state.go#L21 to a common.Hash as well. Will obviously have to be a separate PR anyway since its a different repo.

@ImTei
Copy link
Collaborator Author

ImTei commented Jun 20, 2024

@ajsutton right. I'll update it in the monorepo and check via e2e to confirm everything works.

@ImTei
Copy link
Collaborator Author

ImTei commented Jun 20, 2024

Update: opened a monorepo PR: ethereum-optimism/optimism#10967, and added some details in the PR description.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@9c2c49a). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master      #68   +/-   ##
=========================================
  Coverage          ?   72.25%           
=========================================
  Files             ?       16           
  Lines             ?     2725           
  Branches          ?        0           
=========================================
  Hits              ?     1969           
  Misses            ?      671           
  Partials          ?       85           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pcw109550 pcw109550 added this pull request to the merge queue Jun 21, 2024
Merged via the queue into master with commit 4860406 Jun 21, 2024
8 checks passed
@pcw109550 pcw109550 changed the title Use common.Hash in VMState Use common.Hash in VMState - Update Monorepo submodule to 457f33f Jun 21, 2024
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.

4 participants