ExecResult simplification (ethereumjs#551)

* Always use VMError and not directly ERROR

* Remove PrecompileResult and move OOGResult

* Rename EVMResult#vm to EVMResult#execResult

* Replace runValue for return

* Fix TxReceipt's status doc

* Simplify exceptions handling in results values

* Remove unnecessary re-export

* Rename return to returnValue

Author: alcuadrado

README.md

@@ -71,7 +71,7 @@ vm.runCode({
   gasLimit: new BN(0xffff),
 })
   .then(results => {
-    console.log('Returned : ' + results.return.toString('hex'))
+    console.log('Returned : ' + results.returnValue.toString('hex'))
     console.log('gasUsed  : ' + results.gasUsed.toString())
   })
   .catch(err => console.log('Error    : ' + err))

examples/run-solidity-contract/index.ts

@@ -6,7 +6,7 @@ import * as fs from 'fs'
 import { promisify } from 'util'
 import * as util from 'ethereumjs-util'
 import Account from 'ethereumjs-account'
-const Transaction = require('ethereumjs-tx') // Change when https://github.com/ethereumjs/ethereumjs-vm/pull/541 gets merged
+import { Transaction } from 'ethereumjs-tx'
 const abi = require('ethereumjs-abi')
 const solc = require('solc')
 
@@ -95,9 +95,8 @@ async function deployContract(
   const params = abi.rawEncode(['string'], [greeting])
 
   const tx = new Transaction({
-    to: null,
     value: 0,
-    gas: 2000000, // We assume that 2M is enough,
+    gasLimit: 2000000, // We assume that 2M is enough,
     gasPrice: 1,
     data: '0x' + deploymentBytecode + params.toString('hex'),
     nonce: await getAccountNonce(vm, senderPrivateKey),
@@ -107,8 +106,8 @@ async function deployContract(
 
   const deploymentResult = await vm.runTx({ tx })
 
-  if (deploymentResult.vm.exception === 0) {
-    throw deploymentResult.vm.exceptionError
+  if (deploymentResult.execResult.exceptionError) {
+    throw deploymentResult.execResult.exceptionError
   }
 
   return deploymentResult.createdAddress!
@@ -125,7 +124,7 @@ async function setGreeting(
   const tx = new Transaction({
     to: contractAddress,
     value: 0,
-    gas: 2000000, // We assume that 2M is enough,
+    gasLimit: 2000000, // We assume that 2M is enough,
     gasPrice: 1,
     data: '0x' + abi.methodID('setGreeting', ['string']).toString('hex') + params.toString('hex'),
     nonce: await getAccountNonce(vm, senderPrivateKey),
@@ -135,8 +134,8 @@ async function setGreeting(
 
   const setGreetingResult = await vm.runTx({ tx })
 
-  if (setGreetingResult.vm.exception === 0) {
-    throw setGreetingResult.vm.exceptionError
+  if (setGreetingResult.execResult.exceptionError) {
+    throw setGreetingResult.execResult.exceptionError
   }
 }
 
@@ -148,11 +147,11 @@ async function getGreeting(vm: VM, contractAddress: Buffer, caller: Buffer) {
     data: abi.methodID('greet', []),
   })
 
-  if (greetResult.vm.exception === 0) {
-    throw greetResult.vm.exceptionError
+  if (greetResult.execResult.exceptionError) {
+    throw greetResult.execResult.exceptionError
   }
 
-  const results = abi.rawDecode(['string'], greetResult.vm.return)
+  const results = abi.rawDecode(['string'], greetResult.execResult.return)
 
   return results[0]
 }

examples/run-transactions-complete/index.ts

@@ -2,8 +2,7 @@ import VM from '../..'
 import Account from 'ethereumjs-account'
 import * as utils from 'ethereumjs-util'
 import PStateManager from '../../lib/state/promisified'
-
-const Transaction = require('ethereumjs-tx') // Change when https://github.com/ethereumjs/ethereumjs-vm/pull/541 gets merged
+import { Transaction } from 'ethereumjs-tx'
 
 async function main() {
   const vm = new VM()
@@ -62,7 +61,7 @@ async function runTx(vm: VM, rawTx: any, privateKey: Buffer) {
   })
 
   console.log('gas used: ' + results.gasUsed.toString())
-  console.log('returned: ' + results.vm.return.toString('hex'))
+  console.log('returned: ' + results.execResult.return.toString('hex'))
 
   const createdAddress = results.createdAddress
 

lib/evm/eei.ts

@@ -6,7 +6,7 @@ import Common from 'ethereumjs-common'
 import PStateManager from '../state/promisified'
 import { VmError, ERROR } from '../exceptions'
 import Message from './message'
-import EVM from './evm'
+import EVM, { EVMResult } from './evm'
 const promisify = require('util.promisify')
 
 /**
@@ -460,34 +460,35 @@ export default class EEI {
 
     const results = await this._evm.executeMessage(msg)
 
-    if (results.vm.logs) {
-      this._result.logs = this._result.logs.concat(results.vm.logs)
+    if (results.execResult.logs) {
+      this._result.logs = this._result.logs.concat(results.execResult.logs)
     }
 
     // add gasRefund
-    if (results.vm.gasRefund) {
-      this._result.gasRefund = this._result.gasRefund.add(results.vm.gasRefund)
+    if (results.execResult.gasRefund) {
+      this._result.gasRefund = this._result.gasRefund.add(results.execResult.gasRefund)
     }
 
     // this should always be safe
     this.useGas(results.gasUsed)
 
     // Set return value
     if (
-      results.vm.return &&
-      (!results.vm.exceptionError || (results.vm.exceptionError as VmError).error === ERROR.REVERT)
+      results.execResult.returnValue &&
+      (!results.execResult.exceptionError ||
+        results.execResult.exceptionError.error === ERROR.REVERT)
     ) {
-      this._lastReturned = results.vm.return
+      this._lastReturned = results.execResult.returnValue
     }
 
-    if (!results.vm.exceptionError) {
+    if (!results.execResult.exceptionError) {
       Object.assign(this._result.selfdestruct, selfdestruct)
       // update stateRoot on current contract
       const account = await this._state.getAccount(this._env.address)
       this._env.contract = account
     }
 
-    return new BN(results.vm.exception)
+    return this._getReturnCode(results)
   }
 
   /**
@@ -521,27 +522,27 @@ export default class EEI {
 
     const results = await this._evm.executeMessage(msg)
 
-    if (results.vm.logs) {
-      this._result.logs = this._result.logs.concat(results.vm.logs)
+    if (results.execResult.logs) {
+      this._result.logs = this._result.logs.concat(results.execResult.logs)
     }
 
     // add gasRefund
-    if (results.vm.gasRefund) {
-      this._result.gasRefund = this._result.gasRefund.add(results.vm.gasRefund)
+    if (results.execResult.gasRefund) {
+      this._result.gasRefund = this._result.gasRefund.add(results.execResult.gasRefund)
     }
 
     // this should always be safe
     this.useGas(results.gasUsed)
 
     // Set return buffer in case revert happened
     if (
-      results.vm.exceptionError &&
-      (results.vm.exceptionError as VmError).error === ERROR.REVERT
+      results.execResult.exceptionError &&
+      results.execResult.exceptionError.error === ERROR.REVERT
     ) {
-      this._lastReturned = results.vm.return
+      this._lastReturned = results.execResult.returnValue
     }
 
-    if (!results.vm.exceptionError) {
+    if (!results.execResult.exceptionError) {
       Object.assign(this._result.selfdestruct, selfdestruct)
       // update stateRoot on current contract
       const account = await this._state.getAccount(this._env.address)
@@ -552,7 +553,7 @@ export default class EEI {
       }
     }
 
-    return new BN(results.vm.exception)
+    return this._getReturnCode(results)
   }
 
   /**
@@ -570,6 +571,16 @@ export default class EEI {
   async isAccountEmpty(address: Buffer): Promise<boolean> {
     return this._state.accountIsEmpty(address)
   }
+
+  private _getReturnCode(results: EVMResult) {
+    // This preserves the previous logic, but seems to contradict the EEI spec
+    // https://github.com/ewasm/design/blob/38eeded28765f3e193e12881ea72a6ab807a3371/eth_interface.md
+    if (results.execResult.exceptionError) {
+      return new BN(0)
+    } else {
+      return new BN(1)
+    }
+  }
 }
 
 function trap(err: ERROR) {