refactor[contracts]: Port OVM_ProxyEOA to use ovm-solc by smartcontracts · Pull Request #545 · ethereum-optimism/optimism

smartcontracts · 2021-04-21T20:29:36Z

Description
Continuation of the work started in #475. Ports OVM_ProxyEOA to use ovm-solc. This introduces an interesting problem because OVM_ExecutionManager uses the standard solc and can therefore no longer reference OVM_ProxyEOA directly in order to create a new version of it during ovmCREATEEOA. Instead, we deploy a template version of OVM_ProxyEOA that can be duplicated during ovmCREATEEOA. This is a bit less efficient, not exactly sure how much less efficient but I doubt it'll matter very much.

changeset-bot · 2021-04-21T20:29:38Z

🦋 Changeset detected

Latest commit: 2e405df

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@eth-optimism/contracts	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

smartcontracts · 2021-04-21T20:31:49Z

packages/contracts/test/helpers/test-runner/test-runner.ts


    this.contracts.OVM_DeployerWhitelist = DeployerWhitelist

+    this.contracts.OVM_ProxyEOA = await getContractFactory(


Here we deploy the reference version of the OVM_ProxyEOA which is duplicated by the OVM_ExecutionManager.

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

karlfloersch

I didn't catch anything so seeeeemed to look good. However, definitely would like another set of eyes on this! Especially with raw opcode writing -- cc @ben-chain

karlfloersch · 2021-04-21T22:51:06Z

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

+        // PUSH1 0x00
+        // CODECOPY     # copy everything after prefix into memory at pos 0
+        // PUSH1 0x00
+        // RETURN       # return the copied code


@ben-chain would love to have your eyes here

karlfloersch · 2021-04-21T22:52:24Z

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

+        // PUSH1 0x00
+        // RETURN       # return the copied code
+        address proxyEOA = Lib_EthUtils.createContract(abi.encodePacked(
+            hex"600D380380600D6000396000f3",


Confirmed that this does indeed decompile to the opcodes in the comment above

This bytestring looks good; it would also be super obvious from integration tests if it weren't looking. Since we do not have constructors, then this code could obviously also be put in place without a constructor. I assume we are going this way for ease of implementation in geth, right @smartcontracts? if so, we could use the ovmSETCODE upgrade path in the future.

Good call on using SETCODE for this in the future

ben-chain

This LGTM!

ben-chain · 2021-04-21T23:16:02Z

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol

     *************/

+    address constant DEFAULT_IMPLEMENTATION = 0x4200000000000000000000000000000000000003;
    bytes32 constant IMPLEMENTATION_KEY = 0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddead;


Maybe for a separate PR, but harping again that this should really use this EIP here.

I agree that we should use that EIP, note that this will also break our regenesis script
https://github.com/ethereum-optimism/scripts/blob/main/scripts/state-surgery.js
Will add an issue over there

ethereum-optimism/scripts#12

Created an issue here: #550

ben-chain · 2021-04-21T23:19:42Z

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol

-            IMPLEMENTATION_KEY,
-            Lib_Bytes32Utils.fromAddress(_implementation)
-        );
+        bytes32 addr32 = Lib_Bytes32Utils.fromAddress(_implementation);


I double checked the test files to make sure these are aligned correctly, looks good.

ben-chain · 2021-04-21T23:20:43Z

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

+        address proxyEOA = Lib_EthUtils.createContract(abi.encodePacked(
+            hex"600D380380600D6000396000f3",
+            ovmEXTCODECOPY(
+                0x4200000000000000000000000000000000000009,


Recommend to resolve these addresses

ben-chain · 2021-04-21T23:25:47Z

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

+        // PUSH1 0x00
+        // RETURN       # return the copied code
+        address proxyEOA = Lib_EthUtils.createContract(abi.encodePacked(
+            hex"600D380380600D6000396000f3",


This bytestring looks good; it would also be super obvious from integration tests if it weren't looking. Since we do not have constructors, then this code could obviously also be put in place without a constructor. I assume we are going this way for ease of implementation in geth, right @smartcontracts? if so, we could use the ovmSETCODE upgrade path in the future.

tynes · 2021-04-21T23:35:56Z

packages/contracts/src/contract-deployment/config.ts

      factory: getContractFactory('ERC1820Registry'),
    },
+    OVM_ProxyEOA: {
+      factory: getContractFactory('OVM_ProxyEOA', undefined, true),


What are these additional arguments? undefined is probably a provider, what is true? imo Python kwargs are handy for this kind of thing which can be replicated by making the second argument an object

Agree that we should make these additional arguments an object. Although probably deserves its own issue.

tynes

Please add a changeset and is this safe to merge to master?

smartcontracts · 2021-04-21T23:45:57Z

Yes, safe to merge. Will add a changeset.

smartcontracts · 2021-04-21T23:48:13Z

Changeset added.

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol

* Port OVM_ProxyEOA to use ovm-solc * Remove all constructor logic from ProxyEOA * chore[contracts]: add changeset

…e` and `Safe`->`CrossSafe` (#545) Updates `SafetyLevel` variant names to match Go impl Ref ethereum-optimism/specs#717

Port OVM_ProxyEOA to use ovm-solc

5e0abf8

smartcontracts requested review from ben-chain and maurelian as code owners April 21, 2021 20:29

smartcontracts marked this pull request as draft April 21, 2021 20:29

smartcontracts commented Apr 21, 2021

View reviewed changes

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol Show resolved Hide resolved

Remove all constructor logic from ProxyEOA

09029cd

smartcontracts marked this pull request as ready for review April 21, 2021 21:35

karlfloersch reviewed Apr 21, 2021

View reviewed changes

smartcontracts changed the title ~~Port OVM_ProxyEOA to use ovm-solc~~ refactor[contracts]: Port OVM_ProxyEOA to use ovm-solc Apr 21, 2021

ben-chain approved these changes Apr 21, 2021

View reviewed changes

tynes mentioned this pull request Apr 21, 2021

Broken state-surgery script ethereum-optimism/scripts#12

Open

tynes reviewed Apr 21, 2021

View reviewed changes

tynes requested changes Apr 21, 2021

View reviewed changes

chore[contracts]: add changeset

2e405df

tynes approved these changes Apr 22, 2021

View reviewed changes

tynes mentioned this pull request Apr 22, 2021

OVM_ProxyEOA should use EIP 1967 (standard proxy storage slots) #550

Closed

tynes added the A-contracts label Apr 22, 2021

maurelian reviewed Apr 22, 2021

View reviewed changes

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol Show resolved Hide resolved

smartcontracts merged commit edb4346 into master Apr 22, 2021

smartcontracts deleted the ovm-solc/proxy-eoa branch April 22, 2021 18:00

ben-chain mentioned this pull request Apr 23, 2021

Upgrade the Solidity compiler to simplify pre-deploy contract code #495

Closed

2 tasks

InoMurko referenced this pull request in omgnetwork/optimism May 25, 2021

refactor[contracts]: Port OVM_ProxyEOA to use ovm-solc (#545)

70cf1d7

* Port OVM_ProxyEOA to use ovm-solc * Remove all constructor logic from ProxyEOA * chore[contracts]: add changeset

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

fix(client): Continue derivation on execution failure (#545)

d61fd23

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

chore(consensus): Rename SafetyLevel variants Unsafe->`LocalUnsaf…

5b8a7d8

…e` and `Safe`->`CrossSafe` (#545) Updates `SafetyLevel` variant names to match Go impl Ref ethereum-optimism/specs#717


		this.contracts.OVM_DeployerWhitelist = DeployerWhitelist

		this.contracts.OVM_ProxyEOA = await getContractFactory(

Conversation

smartcontracts commented Apr 21, 2021

Uh oh!

changeset-bot bot commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

karlfloersch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-chain left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes left a comment

Choose a reason for hiding this comment

Uh oh!

smartcontracts commented Apr 21, 2021

Uh oh!

smartcontracts commented Apr 21, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

changeset-bot bot commented Apr 21, 2021 •

edited

Loading