Never revert on ovmCREATE* #10

willmeister · 2020-02-19T17:12:11Z

Description

Makes ExecutionManager.sol always return an address from ovmCREATE and ovmCREATE2. It will be the 0 address if the create failed. This is to make it work how the EVM works currently.

Metadata

Fixes

Fixes # YAS-171

Contributing Agreement

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

…l be 0 address on failure

packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts

masonforest

LGTM

packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts

karlfloersch

I'd approve if I didn't feel self concious for only taking approx. 7 mins to review this!

Looks good so far! I did ask one question here in the comments though that I'm curious about

karlfloersch · 2020-02-19T21:51:00Z

packages/ovm/src/contracts/ExecutionManager.sol

+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }
+        }


Do we know that this is what is returned during a failed STATIC_CALL? I personally don't so just want to make sure

Yep! https://github.com/ethereum-optimism/optimism-monorepo/pull/10/files#diff-712fcb88c95eed4ca84335dd59b7c435R440

I suppose we had tests for all failures result in 0 address and that static calls to create fail, but I added some tests to make sure that static calls actually return 0 address 👍

karlfloersch · 2020-02-19T21:51:19Z

packages/ovm/src/contracts/ExecutionManager.sol

+                let returnData := mload(0x40)
+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }


Previously, the Watcher was a separate package. It's currently consumed by: * our integration tests * our UI * synthetix’s tests and UI Given core-utils is a package meant to ship Optimism related utilities to consumers, it makes more sense to bundle the Watcher in core-utils than have it as a separate package. Co-authored-by: Kelvin Fichter <[email protected]>

Fix ctc events

Previously, the Watcher was a separate package. It's currently consumed by: * our integration tests * our UI * synthetix’s tests and UI Given core-utils is a package meant to ship Optimism related utilities to consumers, it makes more sense to bundle the Watcher in core-utils than have it as a separate package. Co-authored-by: Kelvin Fichter <[email protected]>

Add chain picker

…um-optimism#11) * ci: add the ci code used to package and release docker images (#7) * ci: add the ci code used to package and release docker images Co-authored-by: Welkin <[email protected]> * fix: add latest tag for docker image (ethereum-optimism#9) Co-authored-by: Welkin <[email protected]> * try to use cache for docker build (ethereum-optimism#10) Co-authored-by: Welkin <[email protected]> --------- Co-authored-by: Welkin <[email protected]>

Provide an option for users to invalidate nonces by increasing their nonces to (higher) new values

…um-optimism#11) * ci: add the ci code used to package and release docker images (#7) * ci: add the ci code used to package and release docker images Co-authored-by: Welkin <[email protected]> * fix: add latest tag for docker image (ethereum-optimism#9) Co-authored-by: Welkin <[email protected]> * try to use cache for docker build (ethereum-optimism#10) Co-authored-by: Welkin <[email protected]> --------- Co-authored-by: Welkin <[email protected]>

* op-batcher: Add metrics for pending L2 transaction data size (ethereum-optimism#5797) * feat(op-node): Finalize Mainnet Rollup Config [release branch] (ethereum-optimism#5905) * copy over develop chainsgo * stage rollup config changes * final rollup config values * fix(op-batcher): solve race condition of BatchSubmitter publishTxToL1 and handleReceipt access state concurrently (#5) * chore: update readme, add testnet assets (ethereum-optimism#9) * chore: update readme, add testnet assets * doc: clarify readme * ci: add the ci code used to package and release docker images (ethereum-optimism#11) * ci: add the ci code used to package and release docker images (#7) * ci: add the ci code used to package and release docker images Co-authored-by: Welkin <[email protected]> * fix: add latest tag for docker image (ethereum-optimism#9) Co-authored-by: Welkin <[email protected]> * try to use cache for docker build (ethereum-optimism#10) Co-authored-by: Welkin <[email protected]> --------- Co-authored-by: Welkin <[email protected]> * feat(ci): add ci workflow * fix * fix * update * update * skip fail test temporarily * add batcher/proposer * add e2e * skip fail e2e case temporary * add op-node-lint * fix lint * add batcher/proposer lint * test junit-report * add junit report for all * adjust parallel to 2 for avoiding fail * add needs for job serial execution * use testname format to simplify result --------- Co-authored-by: Joshua Gutow <[email protected]> Co-authored-by: refcell.eth <[email protected]> Co-authored-by: bnoieh <[email protected]> Co-authored-by: Owen <[email protected]> Co-authored-by: Welkin <[email protected]>

* blob encoding version 2 * - cosmetic cleanup - use testify/require for testing - improve big blob test case - some performance improvements * op-service: blob encoding nits (#10) --------- Co-authored-by: Roberto Bayardo <[email protected]> Co-authored-by: protolambda <[email protected]>

88047e707 Add tbcd, a small bitcoin daemon that participates on bitcoin p2p (ethereum-optimism#50) 1146a08b5 localnet reorg fix (ethereum-optimism#76) 87f18a191 build(deps): bump github.com/docker/docker (ethereum-optimism#64) 9073baeaf localnet (ethereum-optimism#37) 1588cbf04 Add common user-specific files to gitignore (ethereum-optimism#51) 210aabe7a Update popm.go, fix typo (ethereum-optimism#40) a5e689493 make: automate copyright headers (ethereum-optimism#31) 1c3bfc9bc Use `maps.Clone(m)` to copy returned map in `APICommands()` (ethereum-optimism#33) 1be4df2a3 Use 'errors.Is' to compare errors (ethereum-optimism#32) 3f6bc5f8e e2e: sync ElectrumX environment variables with infra (ethereum-optimism#36) c5b0fea01 electrumx: add connection reuse and pooling (ethereum-optimism#26) cfc1293e9 Update README.md (ethereum-optimism#29) 8896259f0 retry mine keystone on failure (ethereum-optimism#18) a10e3bb29 Use '%w' verb in fmt.Errorf to wrap errors (fixes ethereum-optimism#13) (ethereum-optimism#27) 6cd677611 deps: update google.golang.org/protobuf to v1.33.0 (ethereum-optimism#28) ed7eb8e97 ci: fix concurrency cancel-in-progress for pull requests (ethereum-optimism#16) ac3b7eacb docker: update golang image to v1.22.1 (ethereum-optimism#25) d6b0ac8af returning response errors if they exist from bfg -> popm (ethereum-optimism#24) d450b787a Network test start height + no panic (ethereum-optimism#22) b390805c5 allowing BTC Block and L2 Keystone generation rates to be configurable in local network (ethereum-optimism#19) bfd3b1dc0 make: add -local flag to goimports (ethereum-optimism#9) e0e8964fc Move internal error into protocol package (ethereum-optimism#10) 7875a897c l2 keystone mining fixes (#3) git-subtree-dir: heminetwork git-subtree-split: 88047e707e2db8522e2ad77c5f849e55bc94cd10

Add integration test setup

willmeister added 2 commits February 19, 2020 11:10

Making ExecutionManager always return an address on ovmCREATE* -- wil…

7e0ebc3

…l be 0 address on failure

Adding tests, removing unwatned it.only

8d1cd81

masonforest reviewed Feb 19, 2020

View reviewed changes

packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts Outdated Show resolved Hide resolved

masonforest approved these changes Feb 19, 2020

View reviewed changes

packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts Outdated Show resolved Hide resolved

packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts Outdated Show resolved Hide resolved

karlfloersch reviewed Feb 19, 2020

View reviewed changes

PR feedback

0c5a321

willmeister merged commit 0df35d9 into master Feb 19, 2020

willmeister deleted the YAS-171/ContractCreationShouldNotRevert branch February 19, 2020 22:41

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

Merge pull request #10 from ethereum-optimism/fix/ctc_events

2afd195

Fix ctc events

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

Merge pull request #10 from enyalabs/add-chain-picker

4ad5962

Add chain picker

refcell pushed a commit that referenced this pull request Sep 6, 2023

Merge pull request #10 from lbeder/fix18b

9794daf

Provide an option for users to invalidate nonces by increasing their nonces to (higher) new values

bap2pecs referenced this pull request in babylonlabs-io/optimism Jul 31, 2024

Merge pull request #10 from babylonchain/integration-tests

52ea27b

Add integration test setup

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Never revert on ovmCREATE* #10

Never revert on ovmCREATE* #10

willmeister commented Feb 19, 2020

masonforest left a comment

karlfloersch left a comment

karlfloersch Feb 19, 2020

willmeister Feb 19, 2020

willmeister Feb 19, 2020

karlfloersch Feb 19, 2020

Never revert on ovmCREATE* #10

Never revert on ovmCREATE* #10

Conversation

willmeister commented Feb 19, 2020

Description

Metadata

Fixes

Contributing Agreement

masonforest left a comment

Choose a reason for hiding this comment

karlfloersch left a comment

Choose a reason for hiding this comment

karlfloersch Feb 19, 2020

Choose a reason for hiding this comment

willmeister Feb 19, 2020

Choose a reason for hiding this comment

willmeister Feb 19, 2020

Choose a reason for hiding this comment

karlfloersch Feb 19, 2020

Choose a reason for hiding this comment