removed circular import dependencies

overview:
  can be verified by running "make check_vm2 X=0" in the nimbus directory
  (be patient when running it.) the X=0 flag is necessary if there is a
  native NIM compiler which may bail out at some vendor imports.

details:
  when compiling state_transaction.nim, the nim flag vm2_enabled must
  be set in order to avoid implicit import of native VM definitions.

nimbus/makefile

@@ -51,6 +51,12 @@ NIMDOC_FLAGS := --verbosity:0 --hints:off --warnings:off
 NIMDOC_FLAGS += -d:debug -d:disable_libbacktrace
 NIMDOC_FLAGS += $(NIMFLAGS)
 
+# Nim check flags
+NIMCHK_FLAGS := c -r --verbosity:0 --hints:off --warnings:off
+
+# Flags vor compiling VM2
+VM2ENABLED   := -d:vm2_enabled
+
 # Markdown compiler (test for discount tool with tables support)
 MD_CMD     := markdown
 MD_TEST    := $(MD_CMD) -VV 2>/dev/null|grep -q TABLES
@@ -74,6 +80,9 @@ help::
 	echo "          docs-update  -- process missing doc pages"
 	echo "          docs-index   -- index collected docs"
 	echo
+	echo "          check_vm     -- run \"nim c -r ..\" on each native VM source file"
+	echo "          check_vm2    -- run \"nim c -r ..\" on each VM2 source file"
+	echo
 	echo "          clean        -- clean up generated and backup files (not docs)"
 	echo "          clean-exe    -- clean up generated executables"
 	echo "          clean-docs   -- clean up generated docs and extra files"
@@ -83,7 +92,7 @@ help::
 	echo
 
 # ------------------------------------------------------------------------------
-# Build indexed dox
+# Build indexed docs
 # ------------------------------------------------------------------------------
 
 # Automatic rule for updating single html/idx file
@@ -91,9 +100,10 @@ docs/%.html : %.nim
 #	use compat option if it works with the nim compiler
 	@mkdir -p docs
 	@nim=$(NIM_EXE); doc_root="$(DOC_ROOT)"; \
+	unset vm2flag; case "$<" in *vm2/*)vm2flag="$(VM2ENABLED)";; esac; \
 	export NIMBLE_DIR=$(NIMBLE_DIR); \
 	(set -x; $$nim doc --outdir:docs --docRoot:"$$doc_root" --index:on \
-		--errorMax:0 $(NIMDOC_FLAGS) \
+		$$vm2flag --errorMax:0 $(NIMDOC_FLAGS) \
 		"$<" $(MUFFLE)) || true
 
 # Automatic rule for updating markdown files
@@ -163,6 +173,42 @@ docs-index:: docs-index-helper
 docs:: docs-update
 docs:: docs-index
 
+# ------------------------------------------------------------------------------
+# Run local compilation by source file
+# ------------------------------------------------------------------------------
+
+# Compile & run individual VM2 files. Note that the source file
+# "state_transactions.nim" needs the flag -d:vm2_enabled set in order
+# to compile.
+check_vm2:
+	@vm2exe=`echo $(EXE_FILES)|tr ' ' '\n'|sed '/vm2\//!d'`;\
+	 nim=$(NIM_EXE); \
+	 export NIMBLE_DIR=$(NIMBLE_DIR); \
+	 $$nim --version | sed q; \
+	 for path in `echo "$$vm2exe"`; do ( \
+	   dir=`dirname "$$path"`; \
+	   src=`basename "$$path"`; \
+	   unset vm2flag; case "$$src" in \
+	      state_transactions) vm2flag="$(VM2ENABLED)";\
+           esac; \
+	   cd "$$dir" ; \
+	   (set -x;$$nim $(NIMCHK_FLAGS) $$vm2flag "$$src.nim" $(MUFFLE)) || \
+	      echo "*** FAIL $$path"; \
+	); done || true
+
+check_vm:
+	@vmexe=`echo $(EXE_FILES)|tr ' ' '\n'|sed '/vm\//!d'`;\
+	 nim=$(NIM_EXE); \
+	 export NIMBLE_DIR=$(NIMBLE_DIR); \
+	 $$nim --version | sed q; \
+	 for path in `echo "$$vmexe"`; do ( \
+	   dir=`dirname "$$path"`; \
+	   src=`basename "$$path"`; \
+	   cd "$$dir" ; \
+	   (set -x;$$nim $(NIMCHK_FLAGS) "$$src.nim" $(MUFFLE)) || \
+	      echo "*** FAIL $$path"; \
+	); done || true
+
 # ------------------------------------------------------------------------------
 # Clean up etc.
 # ------------------------------------------------------------------------------

nimbus/vm2/computation.nim

@@ -14,6 +14,7 @@ import
   ../constants,
   ./compu_helper,
   ./interpreter/forks_list,
+  ./interpreter_dispatch,
   ./message, ./types, ./state,
   ../db/accounts_cache,
   ./precompiles
@@ -21,9 +22,12 @@ import
 logScope:
   topics = "vm computation"
 
-proc initAddress(x: int): EthAddress {.compileTime.} = result[19] = x.byte
+const
-const ripemdAddr = initAddress(3)
+  ripemdAddr = block:
-proc executeOpcodes*(c: Computation) {.gcsafe.}
+    proc initAddress(x: int): EthAddress {.compileTime.} =
+      result[19] = x.byte
+    initAddress(3)
+
 
 proc beforeExecCall(c: Computation) =
   c.snapshot()
@@ -100,6 +104,26 @@ proc afterExec(c: Computation) =
   else:
     c.afterExecCreate()
 
+proc executeOpcodes*(c: Computation) =
+  let fork = c.fork
+
+  block:
+    if not c.continuation.isNil:
+      c.continuation = nil
+    elif c.execPrecompiles(fork):
+      break
+
+    try:
+      c.selectVM(fork)
+    except CatchableError as e:
+      c.setError(
+        &"Opcode Dispatch Error msg={e.msg}, depth={c.msg.depth}", true)
+
+  if c.isError() and c.continuation.isNil:
+    if c.tracingEnabled: c.traceError()
+    debug "executeOpcodes error", msg=c.error.info
+
+
 proc execCallOrCreate*(cParam: Computation) =
   var (c, before) = (cParam, true)
   defer:
@@ -122,25 +146,3 @@ proc execCallOrCreate*(cParam: Computation) =
     c.dispose()
     (before, c.parent, c) = (false, nil.Computation, c.parent)
     (c.continuation)()
-
-
-import interpreter_dispatch
-
-proc executeOpcodes(c: Computation) =
-  let fork = c.fork
-
-  block:
-    if not c.continuation.isNil:
-      c.continuation = nil
-    elif c.execPrecompiles(fork):
-      break
-
-    try:
-      c.selectVM(fork)
-    except CatchableError as e:
-      c.setError(
-        &"Opcode Dispatch Error msg={e.msg}, depth={c.msg.depth}", true)
-
-  if c.isError() and c.continuation.isNil:
-    if c.tracingEnabled: c.traceError()
-    debug "executeOpcodes error", msg=c.error.info

nimbus/vm2/interpreter/op_handlers/oph_defs.nim

@@ -12,18 +12,8 @@
 ## ========================
 ##
 
-# Including types.nim needed unless included (not imported) into
-# oph_kludge.nim
-#
-# Note that the nim compiler will distinguish <Vm2Ctx> tuples defined
-# here when imported and from oph_kludge.nim. This is so due to the
-# duplicate/different Computation definitions.
-#
-when not declared(Computation):
-  import
-    ../../types
-
 import
+  ../../types,
   ../forks_list,
   ../op_codes,
   eth/common/eth_types

nimbus/vm2/interpreter/op_handlers/oph_helpers.nim

@@ -12,20 +12,14 @@
 ## ============================================
 ##
 
-# Including types.nim and others unless included (not imported) into
+import
-# oph_kludge.nim
-#
-when not declared(Computation):
-  import
   ../../../db/accounts_cache,
+  ../../../errors,
   ../../state,
   ../../types,
   ../gas_costs,
   ../gas_meter,
-    eth/common
+  eth/common,
-
-import
-  ../../../errors,
   eth/common/eth_types,
   macros,
   stint

nimbus/vm2/interpreter/op_handlers/oph_kludge.nim

@@ -1,193 +0,0 @@
-# Nimbus
-# Copyright (c) 2018 Status Research & Development GmbH
-# Licensed under either of
-#  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
-#    http://www.apache.org/licenses/LICENSE-2.0)
-#  * MIT license ([LICENSE-MIT](LICENSE-MIT) or
-#    http://opensource.org/licenses/MIT)
-# at your option. This file may not be copied, modified, or distributed except
-# according to those terms.
-
-## EVM Opcodes, Definitions -- Kludge Version
-## ==========================================
-##
-
-{.warning: "Circular dependency breaker kludge -- no production code".}
-
-import
-  ../../../errors,
-  ../../memory,
-  ../../stack,
-  ../forks_list,
-  ../op_codes,
-  eth/common/eth_types,
-  macros,
-  sets,
-  stint
-
-# ------------------------------------------------------------------------------
-# Kludge BEGIN
-# ------------------------------------------------------------------------------
-
-type
-  MsgFlags* = enum
-    emvcNoFlags  = 0
-    emvcStatic   = 1
-
-  CallKind* = enum
-    evmcCall         = 0 # CALL
-    evmcDelegateCall = 1 # DELEGATECALL
-    evmcCallCode     = 2 # CALLCODE
-    evmcCreate       = 3 # CREATE
-    evmcCreate2      = 4 # CREATE2
-
-  ReadOnlyStateDB* =
-    seq[byte]
-
-  GasMeter* = object
-    gasRefunded*: GasInt
-    gasRemaining*: GasInt
-
-  CodeStream* = ref object
-    bytes*: seq[byte]
-    pc*: int
-
-  ChainId* = uint # distinct uint
-
-  ChainConfig* = object
-    chainId*: ChainId
-    homesteadBlock*: BlockNumber
-    daoForkBlock*: BlockNumber
-    daoForkSupport*: bool
-
-  AccountsCache* = ref object
-    isDirty: bool
-
-  BaseChainDB* = ref object
-    pruneTrie*: bool
-    config*: ChainConfig
-
-  GasCost* = object
-    opaq: int
-
-  GasCosts* = array[Op, GasCost]
-
-  BaseVMState* = ref object of RootObj
-    chaindb*       : BaseChainDB
-    blockHeader*   : BlockHeader
-    logEntries*    : seq[Log]
-    accountDb*     : AccountsCache
-    touchedAccounts*: HashSet[EthAddress]
-    suicides*      : HashSet[EthAddress]
-    txOrigin*      : EthAddress
-    txGasPrice*    : GasInt
-    gasCosts*      : GasCosts
-    fork*          : Fork
-
-  Message* = ref object
-    kind*: CallKind
-    depth*: int
-    gas*: GasInt
-    contractAddress*: EthAddress
-    codeAddress*: EthAddress
-    sender*: EthAddress
-    value*: UInt256
-    data*: seq[byte]
-    flags*: MsgFlags
-
-  Computation* = ref object
-    returnStack*: seq[int]
-    output*: seq[byte]
-    vmState*: BaseVMState
-    gasMeter*: GasMeter
-    stack*: Stack
-    memory*: Memory
-    msg*: Message
-    code*: CodeStream
-    returnData*: seq[byte]
-    fork*: Fork
-    parent*, child*: Computation
-    continuation*: proc() {.gcsafe.}
-    touchedAccounts*: HashSet[EthAddress]
-    suicides*: HashSet[EthAddress]
-    logEntries*: seq[Log]
-
-const
-  ColdAccountAccessCost* = 2
-  WarmStorageReadCost* = 3
-  MaxCallDepth*     = 42
-
-# function stubs from state.nim
-proc `status=`*(v: BaseVMState; status: bool) = discard
-template mutateStateDB*(vmState: BaseVMState, body: untyped) =
-  block:
-    var db {.inject.} = vmState.accountDb
-    body
-
-# function stubs from compu_helper.nim (to satisfy compiler logic)
-proc gasCosts*(c: Computation): array[Op,int] = result
-proc getBalance*[T](c: Computation, address: T): Uint256 = result
-proc accountExists*(c: Computation, address: EthAddress): bool = result
-
-# function stubs from computation.nim (to satisfy compiler logic)
-proc execCallOrCreate*(cParam: Computation) = discard
-proc refundSelfDestruct*(c: Computation) = discard
-func shouldBurnGas*(c: Computation): bool = result
-proc getGasRefund*(c: Computation): GasInt = result
-proc newComputation*[A,B](v:A, m:B, salt = 0.u256): Computation = new result
-proc isSuccess*(c: Computation): bool = result
-proc isOriginComputation*(c: Computation): bool = result
-proc merge*(c, child: Computation) = discard
-template chainTo*(c, d: Computation, e: untyped) =
-  c.child = d; c.continuation = proc() = e
-
-# function stubs from accounts_cache.nim (some also match state_db.nim):
-func inAccessList*[A,B](ac: A; address: B): bool = false
-proc accessList*[A,B](ac: var A, address: B) = discard
-proc incNonce*[A,B](ac: var A, address: B) = discard
-proc addBalance*[A,B](ac: var A, address: B, delta: UInt256) = discard
-
-# function stubs from gas_meter.nim
-proc consumeGas*(gasMeter: var GasMeter;amount: GasInt;reason: string) = discard
-proc returnGas*(gasMeter: var GasMeter; amount: GasInt) = discard
-
-# function stubs from gas_costs.nim
-type
-  GasResult* =
-     tuple[gasCost, gasRefund: GasInt]
-
-  GasParams* = object
-    case kind*: Op
-    of Create:
-      cr_currentMemSize*, cr_memOffset*, cr_memLength*: int64
-    of Call, CallCode, DelegateCall, StaticCall:
-      c_isNewAccount*: bool
-      c_contractGas*: Uint256
-      c_gasBalance*, c_currentMemSize*, c_memOffset*, c_memLength*: int64
-    else:
-      discard
-
-proc c_handler*(x: int; y: Uint256, z: GasParams): GasResult = result
-proc m_handler*(x: int; curMemSize, memOffset, memLen: int64): int = result
-proc forkToSchedule*(fork: Fork): GasCosts = result
-
-# function stubs from config.nim
-proc toFork*[T](c: T; number: BlockNumber): Fork = result
-
-# function stubs from transaction.nim
-proc intrinsicGas*(tx: Transaction, fork: Fork): GasInt = result
-
-# function stubs from message.nim
-proc isCreate*(message: Message): bool = result
-
-# ------------------------------------------------------------------------------
-# Kludge END
-# ------------------------------------------------------------------------------
-
-include
-  ./oph_defs,
-  ./oph_helpers
-
-# ------------------------------------------------------------------------------
-# End
-# ------------------------------------------------------------------------------

nimbus/vm2/state_transactions.nim

@@ -8,41 +8,32 @@
 # at your option. This file may not be copied, modified, or distributed except
 # according to those terms.
 
-const
+# This source must have the <vm2_enabled> compiler flag set.
-  # needed for compiling locally
+#
-  kludge {.intdefine.}: int = 0
+# why:
-  breakCircularDependency {.used.} = kludge > 0
+#   ../config, ../transaction,  etc include ../vm_* interface files which in
+#   turn will refer to the ../vm/* definitions rather than ../vm2/* unless the
+#    <vm2_enabled> compiler flag is set.
+#
+when not defined(vm2_enabled):
+  {.error: "NIM flag must be set: -d:vm2_enabled".}
 
 import
-  ../transaction,
-  ./interpreter,
-  chronicles,
-  eth/common/eth_types,
-  options,
-  sets
-
-# ------------------------------------------------------------------------------
-# Kludge BEGIN
-# ------------------------------------------------------------------------------
-
-when not breakCircularDependency:
-  import
   ../config,
   ../constants,
   ../db/accounts_cache,
+  ../transaction,
+  ./compu_helper,
   ./computation,
-    ./interpreter/gas_costs,
+  ./interpreter,
+  ./interpreter/[forks_list, gas_costs, gas_meter],
   ./state,
   ./types,
-    eth/common
+  chronicles,
-
+  eth/common,
-else:
+  eth/common/eth_types,
-  import
+  options,
-    ./interpreter/op_handlers/oph_kludge
+  sets
-
-# ------------------------------------------------------------------------------
-# Kludge END
-# ------------------------------------------------------------------------------
 
 proc setupTxContext*(vmState: BaseVMState, origin: EthAddress, gasPrice: GasInt, forkOverride=none(Fork)) =
   ## this proc will be called each time a new transaction
@@ -81,6 +72,14 @@ proc setupComputation*(vmState: BaseVMState, tx: Transaction, sender: EthAddress
   result = newComputation(vmState, msg)
   doAssert result.isOriginComputation
 
+
+proc refundGas*(c: Computation, tx: Transaction, sender: EthAddress) =
+  let maxRefund = (tx.gasLimit - c.gasMeter.gasRemaining) div 2
+  c.gasMeter.returnGas min(c.getGasRefund(), maxRefund)
+  c.vmState.mutateStateDB:
+    db.addBalance(sender, c.gasMeter.gasRemaining.u256 * tx.gasPrice.u256)
+
+
 proc execComputation*(c: Computation) =
   if not c.msg.isCreate:
     c.vmState.mutateStateDB:
@@ -95,9 +94,3 @@ proc execComputation*(c: Computation) =
     c.vmState.touchedAccounts.incl c.touchedAccounts
 
   c.vmstate.status = c.isSuccess
-
-proc refundGas*(c: Computation, tx: Transaction, sender: EthAddress) =
-  let maxRefund = (tx.gasLimit - c.gasMeter.gasRemaining) div 2
-  c.gasMeter.returnGas min(c.getGasRefund(), maxRefund)
-  c.vmState.mutateStateDB:
-    db.addBalance(sender, c.gasMeter.gasRemaining.u256 * tx.gasPrice.u256)