Skip to content

Conversation

@pcw109550
Copy link
Contributor

@pcw109550 pcw109550 commented Apr 2, 2024

TL; DR: Implemented op-challenger for asterisc. Only unit test is passing. E2E tests will be implemented at asterisc repo. Need to work on challenger abstraction to avoid code duplication.

Description

op-challenger provides a wrapper with FPVM binary, and it only supported cannon to play dispute game, providing traces. This PR adds a wrapper with yet another FPVM, asterisc.

Although code diff is quite large, they do not add fancy ideas. They are almost same with cannon's structure. I mirrored the structure of cannon's trace directory. There are few building blocks to build the challenger for asterisc. I will list up the components and their needs.

  • asterisc_state.go, asterisc_memory.go, asterisc_page.go : It reads the asterisc state, and calculate witness or statehash. Cannot be shared with cannon because they have different state formats.
    • Most of the codes are copied from the asterisc repo to avoid importing asterisc.
    • Asterisc repo already imports the monorepo, so if we import asterisc here again, we get a circular dependency.
    • I temporarily copied the partial code of asterisc repo here. Space for abstraction: Please refer to next paragraph.
  • prestate.go: It reads the prestate using asterisc_state.go. Mirroring cannon_state.go.
  • executor.go: Wrapper of asterisc binary, supplying flags.
  • provider.go: Trace provider for asterisc.
  • preimage.go, local.go: These were imported from cannon because these are not cannon specific. No new files added for these.

Asterisc specific flags where added to op-challenger. Copied cannon flags and configs, but changed their name to contain asterisc instead of cannon. Tweaked op-challenger to support asterisc game type = 2.

dispute-mon will now accept asterisc game type as valid.

Space for abstraction

op-challenger must import the state/memory/page related code to interact. For example, lets compare below code segments:
At trace/cannon/prestate.go:

func (p *CannonPrestateProvider) AbsolutePreStateCommitment(_ context.Context) (common.Hash, error) {
	...
	hash, err := mipsevm.StateWitness(state).StateHash()
	...
}

At trace/asterisc/prestate.go:

func (p *AsteriscPreStateProvider) AbsolutePreStateCommitment(_ context.Context) (common.Hash, error) {
	...
	hash, err := StateWitness(state).StateHash()
	...
}

Each code segment requires to compute state hash from the state. For cannon, this job is relatively simple because cannon is directly embedded in the monorepo. For asterisc, different story. We cannot import asterisc to the monorepo because of circular dependency. Therefore we need to duplicate the codes, as seen in this PR.

We need better abstraction to hide these details, and this can be redesigned in the follow up PR.

Tests

Unit tests for asterisc tracer is added. Basic flag sanity checking unit test is added.

There are no e2e tests added! These will be added to asterisc. Will refer to output_cannon_test.go.

To add more details about why e2e is implemented at asterisc, asterisc is expected to be maintained at separate repository. Asterisc pulls the monorepo(here!)

@pcw109550 pcw109550 force-pushed the tip/pcw109550/op-challenger-astersic branch 2 times, most recently from a14b8f4 to ee317ee Compare April 2, 2024 23:34
@ImTei
Copy link
Collaborator

ImTei commented Apr 3, 2024

What I understand is that the only thing we need to make is TraceProvider for Asterisc. So we don't have to follow how the CannonTraceProvider implements methods of TraceProvider like GetStepData(). That means we just can make Asterisc binary return the state hash with state and proof, other than the challenger calculates the hash by itself. Then we can remove all duplicated code for Asterisc state hash calculation - such as memory implementation. (or we can also leave a minimal type definition) Let's figure out what we should change!

@pcw109550 pcw109550 marked this pull request as ready for review April 3, 2024 05:15
@pcw109550 pcw109550 requested a review from a team as a code owner April 3, 2024 05:15
@pcw109550 pcw109550 requested a review from Inphi April 3, 2024 05:15
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 3, 2024

Walkthrough

Walkthrough

The recent updates bring significant enhancements to the system by introducing the "Asterisc" trace type. These changes include new variables, error handling, configuration settings, and testing tailored specifically for the "Asterisc" configuration. The system now supports Asterisc-specific game types, metrics recording for Asterisc execution time, and optimizations in existing structures to align better with the Ethereum Optimism project requirements.

Changes

Files Change Summaries
op-challenger/cmd/main_test.go
op-challenger/config/config.go
op-challenger/config/config_test.go
op-challenger/flags/flags.go
Introduced and integrated the "Asterisc" trace type with new variables, configuration parameters, error handling, and testing capabilities.
op-challenger/game/fault/register.go
op-challenger/game/fault/types/types.go
op-dispute-mon/mon/extract/caller.go
op-dispute-mon/mon/extract/caller_test.go
Updated game registration and contract creation logic to incorporate the "Asterisc" game type with specific configurations and metadata handling.
op-challenger/game/fault/trace/asterisc/...
op-challenger/game/fault/trace/cannon/...
Enhanced trace management and proof generation functionalities for the "Asterisc" trace type while aligning naming conventions in the "Cannon" trace type files.
op-challenger/metrics/metrics.go
op-challenger/metrics/noop.go
Added metrics recording for the execution time of "Asterisc" operations and made adjustments to the existing metrics implementation for better tracking.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@pcw109550 pcw109550 force-pushed the tip/pcw109550/op-challenger-astersic branch from 8cefe3c to b6dd670 Compare April 3, 2024 17:58
@pcw109550 pcw109550 force-pushed the tip/pcw109550/op-challenger-astersic branch from b6dd670 to f25a705 Compare April 3, 2024 21:35
@pcw109550 pcw109550 marked this pull request as draft April 5, 2024 17:51
@pcw109550
Copy link
Contributor Author

Closing this PR; Refactored implementation is at #10094.

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