Gas refactoring - decouple opcode logic and gas

[mratsim]

This PR isolates gas computation in a self-contained file gas_costs:

• easier to read and reason about gas
• declutter opcode logic
• easier to debug gas related issue

Furthermore apart for the selfDestruct instruction, all gas computation logics are implemented.

Design choices

All gas handling is stateless:

• easier to test and fuzz-test gas proc
• easier to add a gas-evaluation/simulation feature to Nimbus (which seems to be lacking in the Ethereum world)
• can be multi-threaded (avoid race condition on the computation state)
• results can be cached/memoized
• useful if we want to remove inheritance of Ethereum forks (VM and VM_STATE) from the codebase
• extendMemory function that was hiding gas cost for memory expansion has been removed.
  For opcodes like MStore, gas cost is now explicitly GasVeryLow + MemoryExpansionCost.

Most gas computation can be done at compile-time:

• Faster!
• Less memory allocated at runtime.
• At the cost of compilation time. (Note that there is an overhaul to add cached incremental compilation in Nim).
• It should be possible to inline all fixed costs, only keep the dynamic gas procs in the final binary and avoid compiling in the gas tables to reduce binary size. all gas computations (fixed, dynamic, memExpansion, complex) with a consumeGas template that dispatches to prefix proc {.inline.} instead of dispatching through an array of procs (Non-inlinable even though most are very short)

Fork-specific code is easy to add, maintain and has no runtime cost:

• Derive from a Fee schedule and update only what is needed at compile-time
• add a when Fork: fork-specific else: previous-fork-code
  to create compile-time defined codepath.

Note that now Op is a dense enum (with helper macros to reduce boilerplate). This was introduced to workaround with const table bug - nim-lang/Nim#8007.
It has the added benefit of being compatible with computed gotos, the fastest way to implement an interpreter short of implementing a JIT.

• See research notes (from Lua, Python, ANTLR, the Bosch x86 interpreter, game emulators, ...): https://github.com/status-im/nimbus/wiki/Interpreter-optimization-resources
• computedGoto bad codegen with enum with holes: ComputedGoto: bad codegen with enum with holes nim-lang/Nim#7699

Test results

After refactoring, nimbus still passes the default test suite.

Regarding the official JSON test suite

Regressions (several fixed since PR opening):

• addmod1_overflow2.json
• div1.json
• fibbonacci_unrolled.json
• mul7.json
• mulmod1_overflow.json
• not1.json

Progressions:

• signextend_Overflow_dj42.json
• blockhash257Block.json
• blockhash258Block.json
• blockhashInRange.json
• blockhashMyBlock.json
• blockhashNotExistingBlock.json
• blockhashOutOfRange.json
• coinbase.json

Upstream bugs & Todos

Skipped

Note: This PR was supposed to address #47 workaround:
https://github.com/status-im/nimbus/blob/8528f1b70474f4fb77c57bdbd63da56828da2b25/nimbus/opcode.nim#L20-L28

but this will be done separately, the PR is complex enough.

Bugs uncovered

compile-time object variant are reset at runtime:

• const table are losing everything const Table losing object variant data at runtime nim-lang/Nim#8007
• const arrays with proc field const array indexing zeroes data in object variants at runtime nim-lang/Nim#8015

TODO:

• fix regressions
• Change let to const once bug const array indexing zeroes data in object variants at runtime nim-lang/Nim#8015 is fixed https://github.com/status-im/nimbus/blob/b27996ef905553235a4cb0bdab4bfff3ce2a6502/nimbus/vm/forks/gas_costs.nim#L320-L323
  Alternatively dispatch directly to "prefix gasOp" in a consumeGas/refundGas template that avoids the gasCosts array of procs (pointer indirection, harder debug due to anonymous function) and allows inlining gas cost functions.
• Remove the special handling of Call, Create, SelfDestruct now that gas has been decoupled.
• Showcase how to easily add fork-specific code path for gas (with zero runtime cost) on EIP158 hardfork - https://github.com/status-im/nimbus/blob/b27996ef905553235a4cb0bdab4bfff3ce2a6502/nimbus/vm/forks/gas_costs.nim#L276-L288
• Op are decoded into an Opcode which is a type that only contain a handler now
  Opcode is probably an unnecessary step and we can dispatch directly Op to its handler.
  Assuming we take a similar approach to op handling as what we’ve done to gas handling,
  the op handler will be fork-specific, if so we can inline the gas computation handler at compile-time and be even more efficient. https://github.com/status-im/nimbus/blob/b27996ef905553235a4cb0bdab4bfff3ce2a6502/nimbus/vm_types.nim#L42-L47
• Instead of using a dispatch table for opcode dispatch, use Nim computed gotos

[zah]

What are the future plans here? Is BaseComputation going to gain a static generic parameter describing the VM fork, thus allowing gasCosts to be looked up in a static way?

[mratsim]

To Be Discussed, I can remove the concept of forks and use a consumeGas template that inline the proc called (and no need for the gasCosts table as well).

[zah]

Seems to me that if you want to compute the final gas price at compile-time, you'll also want to know the current fork at compile-time. This will surely produce a lot of code bloat, but perhaps we can compile only two cases: "latest vm" and "older VMs with dynamic lookups"

[mratsim]

Even right now we have a different function per fork that changed the gas price. For example for the EXP opcode that was changed in Spurious Dragon we have:

func basegasExp(value: Uint256): GasInt {.nimcall.} =
   ## Value is the exponent
   result = 10
   if not value.isZero:
     result += 10 * (1 + log256(value))

func spuriousgasExp(value: Uint256): GasInt {.nimcall.} =
   ## Value is the exponent
   result = 10
   if not value.isZero:
     result += 50 * (1 + log256(value))

What will change is that we would need a dispatch loop for each fork instead of checking the fork inside each procs.

I.e. what will be duplicated would be the dispatch loop for each fork, but we save if/then/else branching in each proc or inheritance.

import random, sequtils, strformat

type
  Op = enum
    A
    B
    C
    Halt

proc baseA() {.inline.}=
  echo "gas: ", 10
proc baseB() {.inline.}=
  echo "gas: ", 20
proc baseC() {.inline.}=
  echo "gas: ", 30

# Fork changes just C
proc tangerineC() {.inline.}=
  echo "gas: ", 5000

template callFork(prefix, enumOp: untyped) =
  `prefix enumOp`()

proc baseVM(instructions: seq[Op]) =
  var pc = 0
  while true:
    {.computedGoto.}
    let instr = instructions[pc]

    case instr
    of A:
      callFork(base, A)
    of B:
      callFork(base, B)
    of C:
      callFork(base, C)
    of Halt:
      break
    inc(pc)

proc tangerineVM(instructions: seq[Op]) =
  var pc = 0
  while true:
    {.computedGoto.}
    let instr = instructions[pc]

    case instr
    of A:
      callFork(base, A)      # <-- in the real case the ident would be added by a macro/template
    of B:
      callFork(base, B)
    of C:
      callFork(tangerine, C) # <-- I only change what is needed
    of Halt:
      break
    inc(pc)

proc process(start_block, end_block: int) =

  for blk in start_block..end_block:
    let length = rand(2..10)
    var instructions = newSeqWith(length, rand(0..2).Op) # (we don't have 1 instruction per block in the real case)
    instructions.add Halt
    echo &"Block: {blk}, instructions: {instructions}"

    case blk:
    # This case statement is easily predictable since it doesn't change for months
    of {0..10}:
      echo "using FrontierVM"
      baseVM(instructions)
      echo "block done\n"
    else:
      echo "using TangerineVM"
      tangerineVM(instructions)
      echo "block done\n"

randomize()
process(0, 20)

[arnetheduck]

I might be missing something obvious here, but how is this different from simply using 'method'/virtual calls? it seems to express a similar idea, with more code..?

[mratsim]

This is not a pure while true .. case of interpreter loop, there is computed goto, the generated C code is faster than dispatch through a array of functions.

See my benchmark here: https://gist.github.com/mratsim/99426f641ae25706c1e5418d133bd588

## Results on i5-5257U (Broadwell mobile dual core 2.7 turbo 3.1Ghz) 
# Note that since Haswell, Intel CPU are significantly improved on Switch prediction
# This probably won't carry to ARM devices 

# Warmup: 4.081501s
# result: -14604293096444
# interp_switch took 8.604712000000003s for 1000000000 instructions: 116.2153945419672 Mips (M instructions/s)
# result: -14604293096444
# interp_cgoto took 7.367597000000004s for 1000000000 instructions: 135.7294651159665 Mips (M instructions/s)
# result: -201628509198920 <--- some bug here to fix
# interp_ftable took 8.957571000000002s for 1000000000 instructions: 111.6374070604631 Mips (M instructions/s)
# result: -14604293096444
# interp_handlers took 11.039072s for 1000000000 instructions: 90.58732473164413 Mips (M instructions/s)
# result: -14604293096444
# interp_methods took 23.359635s for 1000000000 instructions: 42.80888806695823 Mips (M instructions/s)

[zah]

consumeGas is a bit inefficient as a proc taking a string. Consider making it a template that uses the supplied reason string in a lazy fashion or a proc taking cstring which will avoid the temp string allocations.

[mratsim]

Ah, interesting. I'll pay attention to string proc in further refactoring. In this PR I didn't touch consumeGas at all.

[coffeepots]

LGTM! Great work.

[coffeepots]

I wonder if, in a future PR, we should make all the procs/funcs in this module templates to ensure inlining, as they're called a lot for such small amounts of code.

[mratsim]

In my profiler they would pop up if they were not inlined. I never had an issue of GCC/Nim not inlining the code.