Skip to content

op-challenger: Deduplicate executor#10785

Merged
ajsutton merged 10 commits intodevelopfrom
aj/vm-dedupe
Jun 12, 2024
Merged

op-challenger: Deduplicate executor#10785
ajsutton merged 10 commits intodevelopfrom
aj/vm-dedupe

Conversation

@ajsutton
Copy link
Contributor

Description

Deduplicate the executor code for cannon and asterisc. Previously they only varied by the Asterisc or Cannon config items. Now there's a vm.Config which can be reused for both VMs and still allow having different config for each.

Tests

Updated unit tests.

Metadata

@ajsutton ajsutton requested a review from a team as a code owner June 10, 2024 14:18
@ajsutton ajsutton requested review from Inphi and mbaxter June 10, 2024 14:18
@codecov
Copy link

codecov bot commented Jun 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.96%. Comparing base (4826337) to head (9b335c1).

Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #10785       +/-   ##
============================================
+ Coverage    59.46%   79.96%   +20.49%     
============================================
  Files           19       11        -8     
  Lines         1658     1123      -535     
  Branches        71        0       -71     
============================================
- Hits           986      898       -88     
+ Misses         640      193      -447     
  Partials        32       32               
Flag Coverage Δ
cannon-go-tests 79.96% <ø> (ø)
chain-mon-tests ?
sdk-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 8 files with indirect coverage changes

Copy link
Contributor

@mbaxter mbaxter left a comment

Choose a reason for hiding this comment

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

lgtm - just a few questions + looks like there's a merge conflict

@ajsutton ajsutton enabled auto-merge June 12, 2024 12:02
@ajsutton ajsutton disabled auto-merge June 12, 2024 12:02
@ajsutton ajsutton enabled auto-merge June 12, 2024 12:03
@ajsutton ajsutton added this pull request to the merge queue Jun 12, 2024
Merged via the queue into develop with commit 8d9287b Jun 12, 2024
@ajsutton ajsutton deleted the aj/vm-dedupe branch June 12, 2024 12:38
rdovgan pushed a commit to rdovgan/optimism that referenced this pull request Jun 24, 2024
* challenger: Create generic vm executor and use for asterisc

* challenger: Move executor utils to vm package

* challenger: Switch cannon to use common vm executor.

* challenger: Use vm.Config to encapsulate Cannon config items.

* challenger: Rename to include Path

* challenger: Use vm.Config to encapsulate Asterisc config items.

* challenger: Remove unused interface

* challenger: Reduce amount of config passed to cannon/asterisc trace providers

* challenger: Remove Config from names

* challenger: Remove replaced cannon metrics
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.

3 participants