From be79bc8740f19b95dc4be741aa628a16fd6dc8be Mon Sep 17 00:00:00 2001 From: andri lim Date: Tue, 19 Mar 2019 23:30:35 +0700 Subject: [PATCH] remove opCodeExec, use executeOpcodes --- nimbus/p2p/chain.nim | 2 +- nimbus/p2p/executor.nim | 2 +- nimbus/rpc/p2p.nim | 2 +- nimbus/vm/computation.nim | 41 ++++++++++---------------- nimbus/vm/interpreter.nim | 4 +-- nimbus/vm/interpreter/opcodes_impl.nim | 13 ++++++-- nimbus/vm/interpreter_dispatch.nim | 24 ++++++--------- nimbus/vm_state_transactions.nim | 5 +++- nimbus/vm_types.nim | 10 ------- 9 files changed, 44 insertions(+), 59 deletions(-) diff --git a/nimbus/p2p/chain.nim b/nimbus/p2p/chain.nim index 3088ee798..9ac64390c 100644 --- a/nimbus/p2p/chain.nim +++ b/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 diff --git a/nimbus/p2p/executor.nim b/nimbus/p2p/executor.nim index 3a87c06a1..fe8719729 100644 --- a/nimbus/p2p/executor.nim +++ b/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 = diff --git a/nimbus/rpc/p2p.nim b/nimbus/rpc/p2p.nim index 230b00e19..905b7e6ee 100644 --- a/nimbus/rpc/p2p.nim +++ b/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: diff --git a/nimbus/vm/computation.nim b/nimbus/vm/computation.nim index 81cad49ff..377c1bb8d 100644 --- a/nimbus/vm/computation.nim +++ b/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 diff --git a/nimbus/vm/interpreter.nim b/nimbus/vm/interpreter.nim index 88db95d00..61675d077 100644 --- a/nimbus/vm/interpreter.nim +++ b/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 diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index b5339ba41..8bd461a59 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/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: diff --git a/nimbus/vm/interpreter_dispatch.nim b/nimbus/vm/interpreter_dispatch.nim index b13b472c5..1b3453ff0 100644 --- a/nimbus/vm/interpreter_dispatch.nim +++ b/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) diff --git a/nimbus/vm_state_transactions.nim b/nimbus/vm_state_transactions.nim index 2363c16aa..c36de0ff6 100644 --- a/nimbus/vm_state_transactions.nim +++ b/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 diff --git a/nimbus/vm_types.nim b/nimbus/vm_types.nim index 0f02ade05..82e5bbddb 100644 --- a/nimbus/vm_types.nim +++ b/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