remove opCodeExec, use executeOpcodes

nimbus/p2p/chain.nim

@@ -1,5 +1,5 @@
 import ../db/[db_chain, state_db], eth/common, chronicles, ../vm_state, ../vm_types, ../transaction, ranges,
-  ../vm/[computation, interpreter_dispatch, message], ../constants, stint, nimcrypto,
+  ../vm/[computation, message], ../constants, stint, nimcrypto,
   ../vm_state_transactions, sugar, ../utils, eth/trie/db, ../tracer, ./executor
 
 type

nimbus/p2p/executor.nim

@@ -3,7 +3,7 @@ import options,
   ../db/[db_chain, state_db],
   ../utils, ../constants, ../transaction,
   ../vm_state, ../vm_types, ../vm_state_transactions,
-  ../vm/[computation, interpreter_dispatch, message],
+  ../vm/[computation, message],
   ../vm/interpreter/vm_forks
 
 proc processTransaction*(tx: Transaction, sender: EthAddress, vmState: BaseVMState, forkOverride=none(Fork)): GasInt =

nimbus/rpc/p2p.nim

@@ -14,7 +14,7 @@ import
   ../transaction, ../config, ../vm_state, ../constants, ../vm_types,
   ../vm_state_transactions, ../utils,
   ../db/[db_chain, state_db, storage_types],
-  rpc_types, rpc_utils, ../vm/[message, computation, interpreter_dispatch]
+  rpc_types, rpc_utils, ../vm/[message, computation]
 
 #[
   Note:

nimbus/vm/computation.nim

@@ -143,48 +143,47 @@ proc writeContract*(computation: var BaseComputation, fork: Fork): bool =
     if fork < FkHomestead: computation.output = @[]
     result = false
 
-proc transferBalance(computation: var BaseComputation, opCode: static[Op]) =
+proc transferBalance(computation: var BaseComputation, opCode: static[Op]): bool =
   if computation.msg.depth >= MaxCallDepth:
-    raise newException(StackDepthError, "Stack depth limit reached")
+    debug "Stack depth limit reached", depth=computation.msg.depth
+    return false
 
   let senderBalance = computation.vmState.readOnlyStateDb().
                       getBalance(computation.msg.sender)
 
   if senderBalance < computation.msg.value:
-    raise newException(InsufficientFunds,
-      &"Insufficient funds: {senderBalance} < {computation.msg.value}")
+    debug "insufficient funds", available=senderBalance, needed=computation.msg.value
+    return false
 
   when opCode in {Call, Create}:
     computation.vmState.mutateStateDb:
       db.subBalance(computation.msg.sender, computation.msg.value)
       db.addBalance(computation.msg.storageAddress, computation.msg.value)
 
+  result = true
+
+proc executeOpcodes*(computation: var BaseComputation)
+
 proc applyMessage(fork: Fork, computation: var BaseComputation, opCode: static[Op]) =
   var snapshot = computation.snapshot()
   defer: snapshot.dispose()
 
   when opCode in {CallCode, Call, Create}:
-    try:
-      computation.transferBalance(opCode)
-    except VMError:
+    if not computation.transferBalance(opCode): 
       snapshot.revert()
-      debug "transferBalance failed", msg = computation.error.info
       return
-
+  
   if computation.gasMeter.gasRemaining < 0:
     snapshot.commit()
     return
 
   var success = true
   try:
-    # Run code
-    # We cannot use the normal dispatching function `executeOpcodes`
-    # within `interpreter_dispatch.nim` due to a cyclic dependency.
-    computation.opcodeExec(computation)
+    executeOpcodes(computation)
     success = not computation.isError
   except VMError:
     success = false
-    debug "applyMessage failed", 
+    debug "applyMessage failed",
       msg = getCurrentExceptionMsg(),
       depth = computation.msg.depth
 
@@ -197,18 +196,6 @@ proc applyMessage(fork: Fork, computation: var BaseComputation, opCode: static[O
   else:
     snapshot.revert(true)
 
-proc generateChildComputation*(fork: Fork, computation: var BaseComputation, childMsg: Message): BaseComputation =
-  var childComp = newBaseComputation(
-      computation.vmState,
-      computation.vmState.blockNumber,
-      childMsg,
-      some(fork))
-
-  # Copy the fork op code executor proc (assumes child computation is in the same fork)
-  childComp.opCodeExec = computation.opCodeExec
-
-  return childComp
-
 proc addChildComputation(fork: Fork, computation: var BaseComputation, child: BaseComputation) =
   if child.isError:
     if child.msg.isCreate:
@@ -286,3 +273,5 @@ proc traceError*(c: BaseComputation) =
 
 proc prepareTracer*(c: BaseComputation) =
   c.vmState.tracer.prepare(c.msg.depth)
+
+include interpreter_dispatch

nimbus/vm/interpreter.nim

@@ -10,11 +10,11 @@ import
   ./interpreter/vm_forks
 
 import # Used in vm_types. Beware of recursive dependencies
-  ./code_stream, ./computation, ./stack, ./message, interpreter_dispatch
+  ./code_stream, ./computation, ./stack, ./message
 
 export
   opcode_values, gas_meter,
   vm_forks
 
 export
-  code_stream, computation, stack, message, interpreter_dispatch
+  code_stream, computation, stack, message

nimbus/vm/interpreter/opcodes_impl.nim

@@ -572,7 +572,11 @@ proc setupCreate(computation: var BaseComputation, memPos, len: int, value: Uint
     )
 
   childMsg.sender = computation.msg.storageAddress
-  result = generateChildComputation(computation.getFork, computation, childMsg)
+  result = newBaseComputation(
+    computation.vmState,
+    computation.vmState.blockNumber,
+    childMsg,
+    some(computation.getFork))
 
 op create, inline = false, value, startPosition, size:
   ## 0xf0, Create a new account with associated code.
@@ -739,7 +743,12 @@ template genCall(callName: untyped, opCode: Op): untyped =
     when opCode == CallCode:
       childMsg.storageAddress = computation.msg.storageAddress
 
-    var childComp = generateChildComputation(computation.getFork, computation, childMsg)
+    var childComp = newBaseComputation(
+      computation.vmState,
+      computation.vmState.blockNumber,
+      childMsg,
+      some(computation.getFork))
+
     result = (childComp, memOutPos, memOutLen)
 
   op callName, inline = false:

nimbus/vm/interpreter_dispatch.nim

@@ -11,7 +11,7 @@ import
   ./interpreter/[opcode_values, opcodes_impl, vm_forks, gas_costs, gas_meter, utils/macros_gen_opcodes],
   ./code_stream,
   ../vm_types, ../errors, precompiles,
-  ./stack, ./computation, terminal # Those are only needed for logging
+  ./stack, terminal # Those are only needed for logging
 
 logScope:
   topics = "vm opcode"
@@ -256,20 +256,14 @@ proc homesteadVM(computation: var BaseComputation) =
   if not computation.execPrecompiles:
     genHomesteadDispatch(computation)
 
-proc executeOpcodes*(computation: var BaseComputation) =
+proc executeOpcodes(computation: var BaseComputation) =
   # TODO: Optimise getting fork and updating opCodeExec only when necessary
   let fork = computation.getFork
 
-  try:
-    case fork
-    of FkFrontier..FkThawing:
-      computation.opCodeExec = frontierVM
-      computation.frontierVM()
-    of FkHomestead..FkSpurious:
-      computation.opCodeExec = homesteadVM
-      computation.homesteadVM()
-    else:
-      raise newException(VMError, "Unknown or not implemented fork: " & $fork)
-  except VMError:
-    computation.error = Error(info: getCurrentExceptionMsg())
-    debug "VM Error executeOpcodes() failed", error = getCurrentExceptionMsg()
+  case fork
+  of FkFrontier..FkThawing:
+    computation.frontierVM()
+  of FkHomestead..FkSpurious:
+    computation.homesteadVM()
+  else:
+    raise newException(VMError, "Unknown or not implemented fork: " & $fork)

nimbus/vm_state_transactions.nim

@@ -77,9 +77,12 @@ proc execComputation*(computation: var BaseComputation): bool =
       for deletedAccount in computation.getAccountsForDeletion:
         db.deleteAccount deletedAccount
     result = not computation.isError
+  except VMError:
+    result = false
+    debug "execComputation() VM Error", error = getCurrentExceptionMsg()
   except ValueError:
     result = false
-    debug "execComputation() error", msg = getCurrentExceptionMsg()
+    debug "execComputation() Value Error", msg = getCurrentExceptionMsg()
 
   if result and computation.msg.isCreate:
     let fork = computation.getFork

nimbus/vm_types.nim

@@ -44,8 +44,6 @@ type
     accounts*: HashSet[EthAddress]
     storageKeys*: seq[HashSet[Uint256]]
 
-  OpcodeExecutor* = proc(computation: var BaseComputation)
-
   BaseComputation* = ref object of RootObj
     # The execution computation
     vmState*:               BaseVMState
@@ -62,7 +60,6 @@ type
     accountsToDelete*:      Table[EthAddress, EthAddress]
     opcodes*:               Table[Op, proc(computation: var BaseComputation){.nimcall.}]
     gasCosts*:              GasCosts # TODO - will be hidden at a lower layer
-    opCodeExec*:            OpcodeExecutor
     forkOverride*:          Option[Fork]
     logEntries*:            seq[Log]
 
@@ -71,13 +68,6 @@ type
     burnsGas*:              bool
     erasesReturnData*:      bool
 
-  Opcode* = ref object of RootObj
-    # TODO can't use a stack-allocated object because
-    # "BaseComputation is not a concrete type"
-    # TODO: We can probably remove this.
-    kind*: Op
-    runLogic*:  proc(computation: var BaseComputation)
-
   GasMeter* = object
     gasRefunded*: GasInt
     startGas*: GasInt