Deletes /ovm and moves its contents into /rollup-contracts #164

smartcontracts · 2020-06-24T18:17:55Z

Description

We have various files (mostly tests) sitting in /ovm. This PR moves these files into /rollup-contracts and cleans up /rollup-contracts in general. I've kept some of the older files from /ovm/src for the sake of simplicity, but it's likely they'll need to either be deleted or moved into a different package. I figured we could handle that as part of a separate ticket in the future in order to keep this PR as tightly scoped as possible.

Tests are passing, though we have some linting errors that I need to fix.

Metadata

Fixes

Fixes YAS-443

Contributing Agreement

I have read and understood the Optimism Contributing Guide and Code of Conduct and am following those guidelines in this pull request.

karlfloersch

Nice I really like this separation. Some of my opinions would be:

feels like if the package is called rollup-contracts then the rollup package is redundant. I actually think that there are some arguments to call this package contracts and then that 2nd directory could be more reasonable. That said, a 2nd note would be I like not overusing the term rollup and actually replace it with optimistic-ethereum, as that is technically the only unifying concept behind these contracts (they are all a part of OE). Only the chain and queue packages technically have any "rollup"-like features. And btw this package was named rollup-contracts because it only had those two sets of contract in it, and then we started co-locating all of our contracts and so that's why this package is now poorly named.
- TLDR, rename package to contracts? rename rollup directory to optimistic-ethereum?
instead of execution (even though I find it to be a very descriptive name) we could consider calling that directory ovm to help people find the contracts that they're looking for if they come having heard about this ovm thing that we've built.
moving SafetyChecker.sol into the ovm package (currently called execution) could be good because it feels the ExecutionManager is sooo closely tied to the SafetyChecker, and I can't really see anywhere other in our crazy use-case where anyone would be trying to use our safety checker. That said, I guess technically it could be generally useful as we did steal the idea of safety checking from the original 'purity checker' in Casper FFG. Ok so I don't actually have a strong opinion here.
precompiles could actually be a directory under ovm (ovm/precompiles) because they are really only used in the context of the ExecutionManager.

Anyway overall this is looking way cleaner already!!! Very excited! Leaving this as just a comment because I'm curious what everyone thinks. That's my 2¢

karlfloersch · 2020-06-24T18:22:13Z

packages/ovm/.soliumrc.json

-      "unix"
-    ]
-  }
-}


Oh should we add this to the rollup-contracts package?

Ohhh yes we should. Good catch.

karlfloersch · 2020-06-25T22:03:03Z

packages/ovm/test/bridge/L1ToL2TransactionPasser.spec.ts

-  const entryPoint = '0x0000000000000000000000000000000000001234'
-  const callData = '0xdeadbeefee5555'
-  it('should emit the correct L1ToL2Transaction L1 event when an L1->L2 tx is sent', async () => {
-    await l1ToL2TransactionPasser


Are these tests reflected anywhere in these changes or just generally deleted? Do these contracts impose a circular dependencies? If they do maybe putting stuff in rollup-full-node makes sense? Although ideally these tests are just in the rollup-contracts package

These tests have all been transferred over to the rollup-contracts package.

smartcontracts · 2020-06-26T18:01:58Z

I'm going to go ahead and close this PR since it's mostly covered in #166

smartcontracts · 2020-06-26T18:02:30Z

@karlfloersch I'll address your comments in the other PR.

smartcontracts · 2020-06-26T18:07:09Z

TLDR, rename package to contracts? rename rollup directory to optimistic-ethereum?

Yeah, I think renaming this package is a good idea. I'll go ahead and make that change.

instead of execution (even though I find it to be a very descriptive name) we could consider calling that directory ovm to help people find the contracts that they're looking for if they come having heard about this ovm thing that we've built.

moving SafetyChecker.sol into the ovm package (currently called execution) could be good because it feels the ExecutionManager is sooo closely tied to the SafetyChecker, and I can't really see anywhere other in our crazy use-case where anyone would be trying to use our safety checker. That said, I guess technically it could be generally useful as we did steal the idea of safety checking from the original 'purity checker' in Casper FFG. Ok so I don't actually have a strong opinion here.

precompiles could actually be a directory under ovm (ovm/precompiles) because they are really only used in the context of the ExecutionManager.

I like all of these!

* state dump: allow for config * lint: fix * add build:kovan command Co-authored-by: Kevin Ho <[email protected]>

https://github.com/sherlock-audit/2023-01-optimism-judging/blob/b7921698c1c537714c5f2021aa46ce0a1935ba39/1-specs-findings/1-processed/1-true/message-passer-interface/164.md#L1

c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1

* feat: use superchain config lockbox in portal * test: add new sharedlockbox test

* remove useless diff * another

* feat: add shared lockbox (#126) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * chore: update compiler version * feat: integrate portal to lockbox (#139) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert * feat: add liquidity migrator contract with its unit test and interface (#128) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * feat: add liqudity migrator contract with its unit test and interface * chore: remove underscore on stack var * chore: add todo * chore: run pre-pr * chore: add contract title natspec and proxied * refactor: integrate testing suite with common test * chore: pre-pr * chore: add spec test * feat: integrate system config with superchain config (#140) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert * feat: integrate system config with superchain config * fix: remove OPCM interop * test: add dependency counter test * feat: manage dependency set on superchain config (#138) * chore: add zero dependencies check (#142) * fix: pre pr * feat: Add pause check (#145) * feat: Add pause check Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> * test: add tests natspecs --------- Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> * fix: pre pr and interfaces imports * feat: add upgrader role to superchain config (#163) * feat: use superchain config lockbox in portal (#164) * feat: use superchain config lockbox in portal * test: add new sharedlockbox test * fix: pre pr * feat: liquidity migrator deployment (#166) * feat: liquidity migrator deployment * test: fix comment * test: fix internal variables names * feat: dependency set refactor (#170) * feat: dependency set refactor * fix: deploy script variable name * fix: pr * fix: pr * fix: pre pr * fix: semgrep * fix: merge conflict * [WIP] feat: new lockbox (#192) * chore: partial implementation comments * feat: new lockbox * feat: introduce dependency manager predeploy * feat: remove timestamp check from CrossL2Inbox * feat: introduce cluster manager role and remove immutables * fix: remove unnecessary code, fix tests and setup * feat: use unstructured storage and OZ v5 * fix: L2ToL2CDM dependency set check * fix: dependency manager gas limit * feat: refactor interop feature contracts (#200) * feat: refactor interop feature contracts * fix: add noops comment * feat: adds OptimismPortal migrated flag * test: add missing tests * fix: portal interop storage naming --------- Co-authored-by: Skeletor Spaceman <[email protected]> * fix: pre pr, setup and tests * fix: remove system config interop and add interop portal target check (#205) * fix: remove system config interop and add interop portal check * fix: interop portal target check order * fix: remove wrong comment * fix: refactor portal noops function (#206) * test: add dependency manager and portal interop tests (#209) * feat: initialize shared lockbox in interop portal (#211) * feat: initialize shared lockbox in interop portal * fix: refactor shared lockbox storage getter * fix: lockbox pr fixes (#214) * fix: pre pr * fix: semver lock * fix: semver lock * feat: descope dependency manager (#242) * feat: descope dependency manager * test: fix tests * test: fix tests * chore: improve eth liquidity test (#248) * fix: internal review fixes (#243) * fix: I-0 * fix: I-1 * fix: I-2 * fix: I-3 * fix: I-6 * fix: I-7 * fix: I-9 * fix: pre pr * fix: pre pr * fix: portal withdrawal checks (#255) * fix: portal withdrawal checks * fix: include current withdrawal check * fix: remove unused interop contracts (#256) * test: fix flake tests (#257) * fix: adjust op-deployer interop scripts (#262) * fix: pre pr * fix op-deployer tests * remove dependency code * fix lint * use Bob account --------- Co-authored-by: Disco <[email protected]> Co-authored-by: AgusDuha <[email protected]> Co-authored-by: agusduha <[email protected]> Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> Co-authored-by: Skeletor Spaceman <[email protected]>

c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1

* fix(plasma): cleanup the plasma data source * fix(plasma): generic bound rename

…hereum-optimism#165 ethereum-optimism#166

### Description Closes #150

Moved all OVM files and deleted the package

207cbe8

smartcontracts added enhancement labels Jun 24, 2020

smartcontracts mentioned this pull request Jun 25, 2020

Clean up contracts and tests #166

Merged

1 task

karlfloersch reviewed Jun 25, 2020

View reviewed changes

smartcontracts closed this Jun 26, 2020

gakonst deleted the YAS-443/ovm/refactor branch March 18, 2021 15:09

snario pushed a commit that referenced this pull request Apr 14, 2021

state dump: allow for config (#164)

0cb732b

* state dump: allow for config * lint: fix * add build:kovan command Co-authored-by: Kevin Ho <[email protected]>

Lredhdx pushed a commit to node-real/combo-optimism that referenced this pull request Jun 17, 2024

chore: add sequencer step metrics (ethereum-optimism#164)

3f4904f

bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024

chore: Replace logrus with zap as logger (ethereum-optimism#164)

eebea03

OptimismBot pushed a commit that referenced this pull request Jan 16, 2025

feat: use superchain config lockbox in portal (#164)

85d49dc

* feat: use superchain config lockbox in portal * test: add new sharedlockbox test

blockchaindevsh added a commit to blockchaindevsh/optimism that referenced this pull request Jan 25, 2025

remove useless diff (ethereum-optimism#164)

f5cad37

* remove useless diff * another

theochap pushed a commit that referenced this pull request Dec 10, 2025

fix(plasma): Plasma Data Source Cleanup (#164)

4147919

* fix(plasma): cleanup the plasma data source * fix(plasma): generic bound rename

Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025

[ACC-01C] Duplicate Function Implementations ethereum-optimism#164 et…

3285a48

…hereum-optimism#165 ethereum-optimism#166

Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025

[ACC-01C] Duplicate Function Implementations ethereum-optimism#164

74d679c

theochap pushed a commit that referenced this pull request Jan 15, 2026

fix: Op Prefix (#164)

16d4ac5

### Description Closes #150

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deletes /ovm and moves its contents into /rollup-contracts #164

Deletes /ovm and moves its contents into /rollup-contracts #164

Uh oh!

smartcontracts commented Jun 24, 2020

Uh oh!

karlfloersch left a comment •

edited

Loading

Uh oh!

karlfloersch Jun 24, 2020

Uh oh!

smartcontracts Jun 26, 2020

Uh oh!

karlfloersch Jun 25, 2020

Uh oh!

smartcontracts Jun 26, 2020 •

edited

Loading

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Deletes /ovm and moves its contents into /rollup-contracts #164

Deletes /ovm and moves its contents into /rollup-contracts #164

Uh oh!

Conversation

smartcontracts commented Jun 24, 2020

Description

Metadata

Fixes

Contributing Agreement

Uh oh!

karlfloersch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karlfloersch Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

smartcontracts Jun 26, 2020

Choose a reason for hiding this comment

Uh oh!

karlfloersch Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

smartcontracts Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

smartcontracts commented Jun 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

karlfloersch left a comment •

edited

Loading

smartcontracts Jun 26, 2020 •

edited

Loading