Never revert on ovmCREATE*

packages/ovm/src/contracts/ExecutionManager.sol

@@ -511,10 +511,17 @@ contract ExecutionManager is FullStateManager {
      * calldata: variable-length bytes:
      *       [methodID (bytes4)]
      *       [ovmInitcode (bytes (variable length))]
-     * returndata: [newOvmContractAddress (as bytes32)]
+     * returndata: [newOvmContractAddress (as bytes32)] -- will be all 0s if this create failed.
      */
     function ovmCREATE() public {
-        require(!executionContext.inStaticContext, "Cannot create new contracts from a STATICCALL.");
+        if (executionContext.inStaticContext) {
+            // Cannot create new contracts from a STATICCALL -- return 0 address
+            assembly {
+                let returnData := mload(0x40)
+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }
+        }
 
         bytes memory _ovmInitcode;
         assembly {
@@ -534,7 +541,14 @@ contract ExecutionManager is FullStateManager {
         uint creatorNonce = getOvmContractNonce(creator);
         address _newOvmContractAddress = contractAddressGenerator.getAddressFromCREATE(creator, creatorNonce);
         // Next we need to actually create the contract in our state at that address
-        createNewContract(_newOvmContractAddress, _ovmInitcode);
+        if (!createNewContract(_newOvmContractAddress, _ovmInitcode)) {
+            // Failure: Return 0 address
+            assembly {
+                let returnData := mload(0x40)
+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }
+        }
         // We also need to increment the contract nonce
         incrementOvmContractNonce(creator);
 
@@ -557,10 +571,17 @@ contract ExecutionManager is FullStateManager {
      *       [methodID (bytes4)]
      *       [salt (bytes32)]
      *       [ovmInitcode (bytes (variable length))]
-     * returndata: [newOvmContractAddress (as bytes32)]
+     * returndata: [newOvmContractAddress (as bytes32)] -- will be all 0s if this create failed.
      */
     function ovmCREATE2() public {
-        require(!executionContext.inStaticContext, "Cannot create new contracts from a STATICCALL.");
+        if (executionContext.inStaticContext) {
+            // Cannot create new contracts from a STATICCALL -- return 0 address
+            assembly {
+                let returnData := mload(0x40)
+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }
+        }
 
         bytes memory _ovmInitcode;
         bytes32 _salt;
@@ -582,7 +603,14 @@ contract ExecutionManager is FullStateManager {
         address creator = executionContext.ovmActiveContract;
         address _newOvmContractAddress = contractAddressGenerator.getAddressFromCREATE2(creator, _salt, _ovmInitcode);
         // Next we need to actually create the contract in our state at that address
-        createNewContract(_newOvmContractAddress, _ovmInitcode);
+        if (!createNewContract(_newOvmContractAddress, _ovmInitcode)) {
+            // Failure: Return 0 address
+            assembly {
+                let returnData := mload(0x40)
+                mstore(returnData, 0)
+                return(returnData, 0x20)
+            }
+        }
 
         // Shifting so that it is left-padded, big-endian ('00'x12 + 20 bytes of address)
         bytes32 newOvmContractAddressBytes32 = bytes32(bytes20(_newOvmContractAddress)) >> 96;
@@ -601,21 +629,25 @@ contract ExecutionManager is FullStateManager {
      * @notice Create a new contract at some OVM contract address.
      * @param _newOvmContractAddress The desired OVM contract address for this new contract we will deploy.
      * @param _ovmInitcode The initcode for our new contract
+     * @return True if this succeeded, false otherwise.
      */
-    function createNewContract(address _newOvmContractAddress, bytes memory _ovmInitcode) internal {
+    function createNewContract(address _newOvmContractAddress, bytes memory _ovmInitcode) internal returns (bool){
         // Purity check the initcode -- unless the overridePurityChecker flag is set to true
-        require(overridePurityChecker || purityChecker.isBytecodePure(_ovmInitcode), "createNewContract: Contract init code is not pure.");
+        if (!overridePurityChecker && !purityChecker.isBytecodePure(_ovmInitcode)) {
+            // Contract init code is not pure.
+            return false;
+        }
         // Switch the context to be the new contract
         (address oldMsgSender, address oldActiveContract) = switchActiveContract(_newOvmContractAddress);
         // Deploy the _ovmInitcode as a code contract -- Note the init script will run in the newly set context
         address codeContractAddress = deployCodeContract(_ovmInitcode);
         // Get the runtime bytecode
         bytes memory codeContractBytecode = getCodeContractBytecode(codeContractAddress);
         // Purity check the runtime bytecode -- unless the overridePurityChecker flag is set to true
-        require(
-            overridePurityChecker || purityChecker.isBytecodePure(codeContractBytecode),
-            "createNewContract: Contract runtime bytecode is not pure."
-        );
+        if (!overridePurityChecker && !purityChecker.isBytecodePure(codeContractBytecode)) {
+            // Contract runtime bytecode is not pure.
+            return false;
+        }
         // Associate the code contract with our ovm contract
         associateCodeContract(_newOvmContractAddress, codeContractAddress);
         // Get the code contract address to be emitted by a CreatedContract event
@@ -624,6 +656,7 @@ contract ExecutionManager is FullStateManager {
         restoreContractContext(oldMsgSender, oldActiveContract);
         // Emit CreatedContract event! We've created a new contract!
         emit CreatedContract(_newOvmContractAddress, codeContractAddress, codeContractHash);
+        return true;
     }
 
     /**

packages/ovm/src/contracts/testing-contracts/InvalidOpcodes.sol

@@ -0,0 +1,16 @@
+pragma solidity ^0.5.0;
+pragma experimental ABIEncoderV2;
+
+contract InvalidOpcodes {
+    function getCoinbase() public returns (address){
+        return block.coinbase;
+    }
+
+    function getDifficulty() public returns (uint){
+        return block.difficulty;
+    }
+
+    function getBlockNumber() public returns (uint) {
+        return block.number;
+    }
+}

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

@@ -24,8 +24,11 @@ import {
   manuallyDeployOvmContract,
   addressToBytes32Address,
   DEFAULT_ETHNODE_GAS_LIMIT,
+  didCreateSucceed,
+  gasLimit,
 } from '../helpers'
 import { GAS_LIMIT, OPCODE_WHITELIST_MASK } from '../../src/app'
+import { TransactionReceipt } from 'ethers/providers'
 
 export const abi = new ethers.utils.AbiCoder()
 
@@ -172,7 +175,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${result}]`)
@@ -187,15 +190,15 @@ describe('Execution Manager -- Call opcodes', () => {
       await wallet.sendTransaction({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       const fetchData: string = `${executeCallToCallContractData}${callMethodId}${callContract2Address32}${sloadMethodIdAndParams}`
 
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data: fetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${result}]`)
@@ -210,14 +213,17 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`RESULT: ${result}`)
 
-      result
-        .substr(2)
-        .length.should.equal(64, 'Should have got a bytes32 address back')
+      const address = remove0x(result)
+      address.length.should.equal(64, 'Should have got a bytes32 address back!')
+      address.length.should.not.equal(
+        '00'.repeat(32),
+        'Should not be 0 address!'
+      )
     })
 
     it('properly executes ovmCALL to CREATE2', async () => {
@@ -226,14 +232,17 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`RESULT: ${result}`)
 
-      result
-        .substr(2)
-        .length.should.equal(64, 'Should have got a bytes32 address back')
+      const address = remove0x(result)
+      address.length.should.equal(64, 'Should have got a bytes32 address back!')
+      address.length.should.not.equal(
+        '00'.repeat(32),
+        'Should not be 0 address!'
+      )
     })
   })
 
@@ -253,7 +262,7 @@ describe('Execution Manager -- Call opcodes', () => {
       await wallet.sendTransaction({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       // Stored in contract 2 via delegate call but accessed via contract 1
@@ -262,7 +271,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data: fetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${result}]`)
@@ -276,7 +285,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const contract2Result = await executionManager.provider.call({
         to: executionManager.address,
         data: contract2FetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${contract2Result}]`)
@@ -296,14 +305,14 @@ describe('Execution Manager -- Call opcodes', () => {
       await wallet.sendTransaction({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       const contract1FetchData: string = `${executeCallToCallContractData}${callMethodId}${callContractAddress32}${sloadMethodIdAndParams}`
       const contract1Result = await executionManager.provider.call({
         to: executionManager.address,
         data: contract1FetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result 1: [${contract1Result}]`)
@@ -318,7 +327,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const contract2Result = await executionManager.provider.call({
         to: executionManager.address,
         data: contract2FetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result 2: [${contract2Result}]`)
@@ -333,7 +342,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const contract3Result = await executionManager.provider.call({
         to: executionManager.address,
         data: contract3FetchData,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result 3: [${contract3Result}]`)
@@ -357,7 +366,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${result}]`)
@@ -371,7 +380,7 @@ describe('Execution Manager -- Call opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
 
       log.debug(`Result: [${result}]`)
@@ -386,7 +395,7 @@ describe('Execution Manager -- Call opcodes', () => {
       await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit: 6_700_000,
+        gasLimit,
       })
     })
 
@@ -397,7 +406,7 @@ describe('Execution Manager -- Call opcodes', () => {
         const res = await executionManager.provider.call({
           to: executionManager.address,
           data,
-          gasLimit: 6_700_000,
+          gasLimit,
         })
       })
     })
@@ -410,35 +419,69 @@ describe('Execution Manager -- Call opcodes', () => {
         await wallet.sendTransaction({
           to: executionManager.address,
           data,
-          gasLimit: 6_700_000,
+          gasLimit,
         })
       })
     })
 
-    it('fails on ovmSTATICCALL to CREATE', async () => {
+    it('Fails to create on ovmSTATICCALL to CREATE -- tx', async () => {
       const data: string = `${executeCallToCallContractData}${staticCallMethodId}${callContract2Address32}${createMethodIdAndData}`
 
-      await TestUtils.assertThrowsAsync(async () => {
-        // Note: Send transaction vs call so it is persisted
-        await wallet.sendTransaction({
-          to: executionManager.address,
-          data,
-          gasLimit: 6_700_000,
-        })
+      // Note: Send transaction vs call so it is persisted
+      const receipt = await wallet.sendTransaction({
+        to: executionManager.address,
+        data,
+        gasLimit,
+      })
+
+      const createSucceeded = await didCreateSucceed(
+        executionManager,
+        receipt.hash
+      )
+      createSucceeded.should.equal(false, 'Create should have failed!')
+    })
+
+    it('Fails to create on ovmSTATICCALL to CREATE -- call', async () => {
+      const data: string = `${executeCallToCallContractData}${staticCallMethodId}${callContract2Address32}${createMethodIdAndData}`
+
+      const res = await wallet.provider.call({
+        to: executionManager.address,
+        data,
+        gasLimit,
       })
+
+      const address = remove0x(res)
+      address.should.equal('00'.repeat(32), 'Should be 0 address!')
     })
 
-    it('fails on ovmSTATICCALL to CREATE2', async () => {
+    it('fails on ovmSTATICCALL to CREATE2 -- tx', async () => {
       const data: string = `${executeCallToCallContractData}${staticCallMethodId}${callContract2Address32}${create2MethodIdAndData}`
 
-      await TestUtils.assertThrowsAsync(async () => {
-        // Note: Send transaction vs call so it is persisted
-        await wallet.sendTransaction({
-          to: executionManager.address,
-          data,
-          gasLimit: 6_700_000,
-        })
+      // Note: Send transaction vs call so it is persisted
+      const receipt = await wallet.sendTransaction({
+        to: executionManager.address,
+        data,
+        gasLimit,
       })
+
+      const createSucceeded = await didCreateSucceed(
+        executionManager,
+        receipt.hash
+      )
+      createSucceeded.should.equal(false, 'Create should have failed!')
+    })
+
+    it('fails on ovmSTATICCALL to CREATE2 -- call', async () => {
+      const data: string = `${executeCallToCallContractData}${staticCallMethodId}${callContract2Address32}${create2MethodIdAndData}`
+
+      const res = await wallet.provider.call({
+        to: executionManager.address,
+        data,
+        gasLimit,
+      })
+
+      const address = remove0x(res)
+      address.should.equal('00'.repeat(32), 'Should be 0 address!')
     })
   })
 })