Never revert on ovmCREATE* by willmeister · Pull Request #10 · ethereum-optimism/optimism

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 <kelvinfichter@gmail.com>

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 <kelvinfichter@gmail.com>

Add chain picker

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

* 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 <roberto.bayardo@coinbase.com> Co-authored-by: protolambda <proto@protolambda.com>

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

Add integration test setup

cannon: Ensure FP tests are runnable in CI

…eum-optimism#10) * feat(scripts): support rootless docker in 3-op-init.sh * feat(scripts): rename scripts for consistency * fix(scripts): fix rootless docker command * refactor(scrits): rename script name for consistency

Switch from docker-bake.yaml to the new unified docker.yaml workflow with mode: bake to test the merged workflow from factory PR #10.

feat(`kona-common`): Bootstrap

Send ETH using the call function.

There are two fixes in this commit: 1. add: reentrancy modifier to updateSeigniorage function 2. fix: Fixed bug when settling to layer2 sequencer in updateSeigniorage function

feat(`kona-common`): Bootstrap

Add onlyDelegateCall enforcement to upgradeSuperchain, upgrade, and migrate functions (#17). Include msg.sender in deploy salt to prevent cross-caller CREATE2 collisions (#17). Add duplicate instruction key detection in upgrade validation (#9). Validate startingRespectedGameType against enabled game configs (#10). Add code-existence check in loadBytes (#18). Add tests covering all audit fix behaviors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add onlyDelegateCall enforcement to upgradeSuperchain, upgrade, and migrate functions (#17). Include msg.sender in deploy salt to prevent cross-caller CREATE2 collisions (#17). Add duplicate instruction key detection in upgrade validation (#9). Validate startingRespectedGameType against enabled game configs (#10). Add code-existence check in loadBytes (#18). Add setUp guard to VerifyOPCM.runSingle (#4). Remove unused _findChar function (#5). Pass real AddressManager in migrator proxy deploy args (#11). Add tests covering all audit fix behaviors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…19272) * contracts: implement audit code fixes and add tests Add onlyDelegateCall enforcement to upgradeSuperchain, upgrade, and migrate functions (#17). Include msg.sender in deploy salt to prevent cross-caller CREATE2 collisions (#17). Add duplicate instruction key detection in upgrade validation (#9). Validate startingRespectedGameType against enabled game configs (#10). Add code-existence check in loadBytes (#18). Add setUp guard to VerifyOPCM.runSingle (#4). Remove unused _findChar function (#5). Pass real AddressManager in migrator proxy deploy args (#11). Add tests covering all audit fix behaviors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * contracts: regenerate semver-lock.json for OPContractsManagerV2 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * contracts: bump OPContractsManagerV2 version to 7.0.10 Semver-diff requires a patch version bump when bytecode changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

…edStaker Cantina audit finding #10: when a beneficiary removes a staker from their allowlist, the staker's lastUpdate is reset via _increasePeData, losing accumulated staking weight. Document this as an inherent trust assumption of the delegation model. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(staking): document pause bypass behavior in setAllowedStaker * fix(staking): only reset lastUpdate on full unstake in _decreasePeData Cantina audit finding #7: _decreasePeData unconditionally reset lastUpdate on every decrease, penalizing partial unstakers by resetting their staking weight. Now lastUpdate is only reset when effectiveStake reaches zero. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(staking): implement two-step ownership transfer Cantina audit finding #8: transferOwnership now nominates a pending owner who must call acceptOwnership() to finalize the transfer, preventing irrevocable ownership loss from incorrect addresses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(staking): document lastUpdate reset trust assumption in setAllowedStaker Cantina audit finding #10: when a beneficiary removes a staker from their allowlist, the staker's lastUpdate is reset via _increasePeData, losing accumulated staking weight. Document this as an inherent trust assumption of the delegation model. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: undo version change * chore: upgrade natspec * chore: update event emission order * fix: wrong event args * chore: add interface comments * fix: pre-pr * refactor: make ownable private functions public * fix: pre-pr * feat(ownable): reset pending owner on zeroaddress ownership transfer * refactor: inherit OZ's ownable and use helper contract for mapping storage slot * fix: ci --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

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

BlocksOnAChain mentioned this pull request Apr 3, 2025

op-program: Incorrect error type when timestamp invariant is broken #15214

Closed

OptimismBot pushed a commit that referenced this pull request Aug 18, 2025

Merge pull request #10 from ethereum-optimism/inphi/ec2/alloc-preheap

ce9dc7e

cannon: Ensure FP tests are runnable in CI

falcorocks added a commit that referenced this pull request Dec 4, 2025

chore(ci): use merged docker.yaml workflow with mode: bake

8b57996

Switch from docker-bake.yaml to the new unified docker.yaml workflow with mode: bake to test the merged workflow from factory PR #10.

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

Merge pull request #10 from anton-rs/cl/common-traits

d389c64

feat(`kona-common`): Bootstrap

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

fix: should not use the transfer function ethereum-optimism#10

fe5812a

Send ETH using the call function.

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

Merge pull request #10 from anton-rs/cl/common-traits

ad837ba

feat(`kona-common`): Bootstrap

smartcontracts mentioned this pull request Feb 22, 2026

contracts: implement onlyDelegateCall and add tests for audit fixes #19272

Merged

0xChin mentioned this pull request Mar 9, 2026

fix: staking audit fixes #19449

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Never revert on ovmCREATE*#10

Never revert on ovmCREATE*#10
willmeister merged 3 commits intomasterfrom
YAS-171/ContractCreationShouldNotRevert

willmeister commented Feb 19, 2020

Uh oh!

Uh oh!

masonforest left a comment

Uh oh!

Uh oh!

Uh oh!

karlfloersch left a comment

Uh oh!

karlfloersch Feb 19, 2020

Uh oh!

willmeister Feb 19, 2020

Uh oh!

willmeister Feb 19, 2020

Uh oh!

karlfloersch Feb 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

willmeister commented Feb 19, 2020

Description

Metadata

Fixes

Contributing Agreement

Uh oh!

Uh oh!

masonforest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

karlfloersch left a comment

Choose a reason for hiding this comment

Uh oh!

karlfloersch Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

karlfloersch Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants