Make CALLs Revert Properly (#28)

* Making calls that revert revert * Fixing circlular dependency Released: - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected] - @eth-optimism/[email protected]
ethereum-optimism · Mar 5, 2020 · 430ce36 · 430ce36
1 parent 00581e3
commit 430ce36
Show file tree

Hide file tree

Showing 20 changed files with 1,563 additions and 153 deletions.
diff --git a/packages/core-db/package.json b/packages/core-db/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/core-db",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimism DB Utils",
   "main": "build/index.js",
   "files": [
@@ -29,7 +29,7 @@
     "url": "https://github.com/ethereum-optimism/optimism-monorepo.git"
   },
   "dependencies": {
-    "@eth-optimism/core-utils": "0.0.1-alpha.12",
+    "@eth-optimism/core-utils": "^0.0.1-alpha.13",
     "abstract-leveldown": "^6.2.2",
     "async-lock": "^1.2.2",
     "chai": "^4.2.0",

diff --git a/packages/core-utils/package.json b/packages/core-utils/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/core-utils",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimism Core Utils",
   "main": "build/index.js",
   "files": [

diff --git a/packages/docs/package.json b/packages/docs/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@eth-optimism/docs",
   "private": true,
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimism docs",
   "author": "Optimism",
   "license": "MIT",

diff --git a/packages/optimistic-game-semantics/package.json b/packages/optimistic-game-semantics/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/optimistic-game-semantics",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimism Optimistic Game Semantics",
   "main": "build/index.js",
   "files": [
@@ -29,8 +29,8 @@
     "url": "https://github.com/ethereum-optimism/optimism-monorepo.git"
   },
   "dependencies": {
-    "@eth-optimism/core-db": "0.0.1-alpha.12",
-    "@eth-optimism/core-utils": "0.0.1-alpha.12",
+    "@eth-optimism/core-db": "^0.0.1-alpha.13",
+    "@eth-optimism/core-utils": "^0.0.1-alpha.13",
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
     "debug": "^4.1.1",

diff --git a/packages/ovm-truffle-provider-wrapper/package.json b/packages/ovm-truffle-provider-wrapper/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/ovm-truffle-provider-wrapper",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimism Truffle Provider Wrapper",
   "main": "build/index.js",
   "files": [
@@ -28,7 +28,7 @@
     "url": "https://github.com/ethereum-optimism/optimism-monorepo.git"
   },
   "dependencies": {
-    "@eth-optimism/rollup-full-node": "0.0.1-alpha.12"
+    "@eth-optimism/rollup-full-node": "^0.0.1-alpha.13"
   },
   "devDependencies": {
     "@types/node": "^12.0.7",

diff --git a/packages/ovm/package.json b/packages/ovm/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/ovm",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "An optimistic execution-compatible EVM",
   "main": "build/index.js",
   "files": [
@@ -50,9 +50,9 @@
     "typescript": "^3.3.3333"
   },
   "dependencies": {
-    "@eth-optimism/core-db": "0.0.1-alpha.12",
-    "@eth-optimism/core-utils": "0.0.1-alpha.12",
-    "@eth-optimism/rollup-core": "0.0.1-alpha.12",
+    "@eth-optimism/core-db": "^0.0.1-alpha.13",
+    "@eth-optimism/core-utils": "^0.0.1-alpha.13",
+    "@eth-optimism/rollup-core": "^0.0.1-alpha.13",
     "@types/sinon-chai": "^3.2.2",
     "chai": "^4.2.0",
     "ethereum-waffle": "2.1.0",

diff --git a/packages/ovm/src/contracts/ExecutionManager.sol b/packages/ovm/src/contracts/ExecutionManager.sol
@@ -248,6 +248,7 @@ contract ExecutionManager is FullStateManager {
             mstore8(add(_callBytes, 3), methodId)
         }
 
+        bool isCall = executionContext.ovmTxOrigin == ZERO_ADDRESS;
         bool success = false;
         address addr = address(this);
         assembly {
@@ -258,6 +259,9 @@ contract ExecutionManager is FullStateManager {
             if eq(success, 1) {
                 return(result, size)
             }
+            if eq(isCall, 1) {
+                revert(result, size)
+            }
         }
 
         if (!success) {

diff --git a/packages/ovm/src/contracts/testing-contracts/DummyContract.sol b/packages/ovm/src/contracts/testing-contracts/DummyContract.sol
@@ -13,4 +13,12 @@ contract DummyContract {
         success = testInt != 0;
         output = testBytes;
     }
+
+    function dummyRevert() public {
+        revert("This is a test revert");
+    }
+
+    function dummyFailingRequire() public {
+        require(false, "This is a test revert");
+    }
 }
diff --git a/packages/ovm/test/contracts/execution-manager.executeCall.spec.ts b/packages/ovm/test/contracts/execution-manager.executeCall.spec.ts
@@ -17,6 +17,7 @@ import {
   manuallyDeployOvmContract,
   getUnsignedTransactionCalldata,
   DEFAULT_ETHNODE_GAS_LIMIT,
+  ZERO_UINT,
 } from '../helpers'
 import { GAS_LIMIT, CHAIN_ID, OPCODE_WHITELIST_MASK } from '../../src/app'
 
@@ -28,8 +29,8 @@ const log = getLogger('execution-manager-calls', true)
  * TESTS *
  *********/
 
-const methodId: string = ethereumjsAbi
-  .methodID('executeCall', [])
+const unsignedCallMethodId: string = ethereumjsAbi
+  .methodID('executeUnsignedEOACall', [])
   .toString('hex')
 
 describe('Execution Manager -- Call opcodes', () => {
@@ -217,5 +218,85 @@ describe('Execution Manager -- Call opcodes', () => {
       )
       await provider.waitForTransaction(tx.hash)
     })
+
+    it('reverts when it makes a call that reverts', async () => {
+      // Generate our tx internalCalldata
+      const internalCalldata = getUnsignedTransactionCalldata(
+        dummyContract,
+        'dummyRevert',
+        []
+      )
+
+      const calldata = getUnsignedTransactionCalldata(
+        executionManager,
+        'executeUnsignedEOACall',
+        [
+          ZERO_UINT,
+          ZERO_UINT,
+          dummyContractAddress,
+          internalCalldata,
+          wallet.address,
+        ]
+      )
+      const nonce = await wallet.getTransactionCount()
+
+      let failed = false
+      try {
+        await wallet.provider.call({
+          nonce,
+          gasLimit: GAS_LIMIT,
+          gasPrice: 0,
+          to: executionManager.address,
+          value: 0,
+          data: calldata,
+          chainId: CHAIN_ID,
+        })
+      } catch (e) {
+        log.debug(JSON.stringify(e) + '  ' + e.stack)
+        failed = true
+      }
+
+      failed.should.equal(true, `This call should have reverted!`)
+    })
+
+    it('reverts when it makes a call that fails a require', async () => {
+      // Generate our tx internalCalldata
+      const internalCalldata = getUnsignedTransactionCalldata(
+        dummyContract,
+        'dummyFailingRequire',
+        []
+      )
+
+      const calldata = getUnsignedTransactionCalldata(
+        executionManager,
+        'executeUnsignedEOACall',
+        [
+          ZERO_UINT,
+          ZERO_UINT,
+          dummyContractAddress,
+          internalCalldata,
+          wallet.address,
+        ]
+      )
+      const nonce = await wallet.getTransactionCount()
+
+      let failed = false
+      try {
+        await wallet.provider.call({
+          nonce,
+          gasLimit: GAS_LIMIT,
+          gasPrice: 0,
+          to: executionManager.address,
+          value: 0,
+          data: calldata,
+          chainId: CHAIN_ID,
+        })
+      } catch (e) {
+        log.debug(JSON.stringify(e) + '  ' + e.stack)
+        failed = true
+      }
+
+      failed.should.equal(true, `This call should have reverted!`)
+    })
   })
 })
diff --git a/packages/ovm/test/helpers.ts b/packages/ovm/test/helpers.ts
@@ -12,7 +12,6 @@ import {
   bufToHexString,
   bufferUtils,
 } from '@eth-optimism/core-utils'
-import * as ethereumjsAbi from 'ethereumjs-abi'
 import { Contract, ContractFactory, Wallet, ethers } from 'ethers'
 import {
   Provider,
@@ -21,6 +20,7 @@ import {
   Log,
 } from 'ethers/providers'
 import { Transaction } from 'ethers/utils'
+import * as ethereumjsAbi from 'ethereumjs-abi'
 
 /* Contract Imports */
 import {
@@ -31,6 +31,8 @@ import {
 
 type Signature = [string, string, string]
 
+export const ZERO_UINT = '00'.repeat(32)
+
 export const DEFAULT_ETHNODE_GAS_LIMIT = 9_000_000
 export const gasLimit = 6_700_000
 const log = getLogger('helpers', true)

diff --git a/packages/rollup-contracts/package.json b/packages/rollup-contracts/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/rollup-contracts",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "Optimistic Rollup smart contracts",
   "main": "build/index.js",
   "files": [
@@ -43,9 +43,9 @@
     "typescript": "^3.3.3333"
   },
   "dependencies": {
-    "@eth-optimism/core-db": "0.0.1-alpha.12",
-    "@eth-optimism/core-utils": "0.0.1-alpha.12",
-    "@eth-optimism/rollup-core": "0.0.1-alpha.12",
+    "@eth-optimism/core-db": "^0.0.1-alpha.13",
+    "@eth-optimism/core-utils": "^0.0.1-alpha.13",
+    "@eth-optimism/rollup-core": "^0.0.1-alpha.13",
     "@types/sinon-chai": "^3.2.2",
     "chai": "^4.2.0",
     "ethereum-waffle": "2.1.0",

diff --git a/packages/rollup-core/package.json b/packages/rollup-core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eth-optimism/rollup-core",
-  "version": "0.0.1-alpha.12",
+  "version": "0.0.1-alpha.13",
   "description": "[Optimism] Optimistic Rollup Core Library",
   "main": "build/index.js",
   "files": [
@@ -29,8 +29,8 @@
     "url": "https://github.com/ethereum-optimism/optimism-monorepo.git"
   },
   "dependencies": {
-    "@eth-optimism/core-db": "0.0.1-alpha.12",
-    "@eth-optimism/core-utils": "0.0.1-alpha.12",
+    "@eth-optimism/core-db": "^0.0.1-alpha.13",
+    "@eth-optimism/core-utils": "^0.0.1-alpha.13",
     "async-lock": "^1.2.2",
     "ethers": "^4.0.39"
   },