Make CALLs Revert Properly by willmeister · Pull Request #28 · ethereum-optimism/optimism

willmeister · 2020-03-03T13:39:22Z

Description

Making CALLs revert while transactions log a revert log when execution fails.

Metadata

Fixes

Fixes # YAS-195

Contributing Agreement

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

…y-support

masonforest

Suggested removal of a couple onlys and some commented code. LGTM! 📚

packages/ovm/test/contracts/execution-manager.executeCall.spec.ts

masonforest · 2020-03-03T14:15:10Z

packages/rollup-dev-tools/src/tools/transpiler/transpiler.ts

+  // private static enoughBytesLeft(
+  //   opcode: EVMOpcode,
+  //   bytecodeLength: number,
+  //   pc: number,
+  //   errors: TranspilationError[]
+  // ): boolean {
+  //   if (pc + opcode.programBytesConsumed >= bytecodeLength) {
+  //     return false
+  //   }
+  //   return true
+  // }


Suggested change

// private static enoughBytesLeft(

// opcode: EVMOpcode,

// bytecodeLength: number,

// pc: number,

// errors: TranspilationError[]

// ): boolean {

// if (pc + opcode.programBytesConsumed >= bytecodeLength) {

// return false

// }

// return true

// }

I agree with this, but this is from another PR that I merged into this one, so would rather handle it there.

masonforest · 2020-03-03T14:15:55Z

packages/rollup-dev-tools/test/contracts/library-transpilation/SimpleSafeMath.sol

+
+        uint c = a + b;
+
+        require(c >= a);   // Makre sure the right computation was made


Suggested change

require(c >= a); // Makre sure the right computation was made

require(c >= a); // Make sure the right computation was made

I agree with this, but this is from another PR that I merged into this one, so would rather handle it there.

masonforest · 2020-03-03T14:16:10Z

packages/rollup-dev-tools/test/transpiler/constructor-transpilation.spec.ts

 const log = getLogger(`constructor-transpilation`)

-describe('Solitity contracts with constructors that take inputs should be correctly deployed', () => {
+describe.only('Solitity contracts with constructors that take inputs should be correctly deployed', () => {


Suggested change

describe.only('Solitity contracts with constructors that take inputs should be correctly deployed', () => {

describe('Solitity contracts with constructors that take inputs should be correctly deployed', () => {

I agree with this, but this is from another PR that I merged into this one, so would rather handle it there.

- @eth-optimism/core-db@0.0.1-alpha.13 - @eth-optimism/core-utils@0.0.1-alpha.13 - @eth-optimism/docs@0.0.1-alpha.13 - @eth-optimism/optimistic-game-semantics@0.0.1-alpha.13 - @eth-optimism/ovm-truffle-provider-wrapper@0.0.1-alpha.13 - @eth-optimism/ovm@0.0.1-alpha.13 - @eth-optimism/rollup-contracts@0.0.1-alpha.13 - @eth-optimism/rollup-core@0.0.1-alpha.13 - @eth-optimism/rollup-dev-tools@0.0.1-alpha.13 - @eth-optimism/rollup-full-node@0.0.1-alpha.13 - @eth-optimism/solc-transpiler@0.0.1-alpha.13

* patch(l2geth): revert back to latest upstream optimism master * chore: disable android tests

use zeppelin safe transfer stuff

- Add infrastructure stability after v0.5 regenesis - Simplify the separation of concerns for v1.0 - Add some "future optimizations" to reflect a more accurate list of what we might want to do to push the chain to the next level. - Update the terminology in the fraud proof track. - Moved the boxes around so they are more chronologically ordered

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

* chore: add forge-std * feat: add forge testdata dir * feat: simple testing tool * feat: command line args * refactor: split test into test and testCmd * refactor: return passes and fails * fix: use testCmd in cmd/test * refactor: set gas limit to 1e7 * fix: add gasLimit to genesis * fix: set gasLimit to 2e7 * refactor: assert test path count in test

* Fix register validator and add e2e tests

…r-dash L2 builder dashboard

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

* fix install err * comments

This reverts commit e2e006a.

…nd oversized payloads Add two bounds checks to `read_tx_data` before indexing into the slice: 1. Reject payloads exceeding `MAX_SPAN_BATCH_ELEMENTS` with `TooBigSpanBatchSize`, matching op-node's `rlp.NewStream(r, MaxSpanBatchElementCount)` behavior. Without this, kona accepts oversized transactions that op-node rejects, causing a consensus divergence. (Cantina finding #28) 2. Reject truncated payloads where `payload_length_with_header` exceeds the remaining buffer, preventing a panic on malformed input. (Cantina finding #25, item 4) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…nd oversized payloads (#19904) Add two bounds checks to `read_tx_data` before indexing into the slice: 1. Reject payloads exceeding `MAX_SPAN_BATCH_ELEMENTS` with `TooBigSpanBatchSize`, matching op-node's `rlp.NewStream(r, MaxSpanBatchElementCount)` behavior. Without this, kona accepts oversized transactions that op-node rejects, causing a consensus divergence. (Cantina finding #28) 2. Reject truncated payloads where `payload_length_with_header` exceeds the remaining buffer, preventing a panic on malformed input. (Cantina finding #25, item 4) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ben-chain and others added 10 commits February 26, 2020 14:26

transpile library deployment

f21cd21

initial library test passing

327ef3e

add missing closeOVM

8831697

library test passing

1e6271d

Merge remote-tracking branch 'origin/master' into feat/YAS-163/librar…

198a271

…y-support

lint

13fe2dc

add multi-lib test

2953931

lint, test timeout

10f6881

commit just transpiler hotfix

4e69113

Making calls that revert revert

18b5f7b

masonforest approved these changes Mar 3, 2020

View reviewed changes

willmeister added 10 commits March 3, 2020 11:49

PR feedback

babf500

fixing merge conflicts

a3c218a

Removing unused circular dependency

856a0a7

re-adding necessary dependency

5e1c8ad

Merge branch 'master' into YAS-195/eth_call-error-handling

d4a41b9

removing circular dependency

9a5b530

changing libraries tests to unit tests instead of integration tests

03ef2cc

fixing libraries

107d031

bumping timeout

6e80d26

willmeister merged commit 430ce36 into master Mar 5, 2020

willmeister deleted the YAS-195/eth_call-error-handling branch March 5, 2020 02:16

gakonst added a commit that referenced this pull request Apr 12, 2021

patch(l2geth): revert back to latest upstream optimism master (#28)

021d955

* patch(l2geth): revert back to latest upstream optimism master * chore: disable android tests

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

Remove unused byte, for real this time (#28)

96db49b

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

patch(l2geth): revert back to latest upstream optimism master (#28)

264c72e

* patch(l2geth): revert back to latest upstream optimism master * chore: disable android tests

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

Merge pull request #28 from enyalabs/inomurko/safe_transfer_erc20

76dd67b

use zeppelin safe transfer stuff

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

Merge pull request #28 from enyalabs/inomurko/safe_transfer_erc20

b2706aa

use zeppelin safe transfer stuff

mslipper mentioned this pull request May 16, 2022

Bedrock -> Develop #2563

Merged

github-actions bot mentioned this pull request Sep 20, 2022

Version Packages #3509

Merged

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

Work on registration (ethereum-optimism#28)

9312b78

* Fix register validator and add e2e tests

SozinM pushed a commit to NethermindEth/optimism that referenced this pull request Feb 10, 2025

Merge pull request ethereum-optimism#28 from flashbots/feat/l2-builde…

6e5811c

…r-dash L2 builder dashboard

blockchaindevsh pushed a commit to blockchaindevsh/optimism that referenced this pull request Jun 30, 2025

CircleCI: fix sanitize-op-program: sudo (ethereum-optimism#28)

8fe9785

* fix install err * comments

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

Revert "feat: release-plz release pipeline (#27)" (#28)

88c1ed5

This reverts commit e2e006a.

sebastianst mentioned this pull request Apr 2, 2026

fix(kona/protocol): bounds checks in read_tx_data for truncated/oversized payloads #19904

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make CALLs Revert Properly#28

Make CALLs Revert Properly#28
willmeister merged 20 commits intomasterfrom
YAS-195/eth_call-error-handling

willmeister commented Mar 3, 2020

Uh oh!

masonforest left a comment

Uh oh!

Uh oh!

masonforest Mar 3, 2020

Uh oh!

willmeister Mar 4, 2020

Uh oh!

masonforest Mar 3, 2020

Uh oh!

willmeister Mar 4, 2020

Uh oh!

masonforest Mar 3, 2020

Uh oh!

willmeister Mar 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		uint c = a + b;

		require(c >= a); // Makre sure the right computation was made

	require(c >= a); // Makre sure the right computation was made
	require(c >= a); // Make sure the right computation was made

	describe.only('Solitity contracts with constructors that take inputs should be correctly deployed', () => {
	describe('Solitity contracts with constructors that take inputs should be correctly deployed', () => {

Conversation

willmeister commented Mar 3, 2020

Description

Metadata

Fixes

Contributing Agreement

Uh oh!

masonforest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

masonforest Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

masonforest Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

masonforest Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants