Fix EVM tracer producing wrong order of CALL family

nimbus/evm/computation.nim

@@ -395,6 +395,9 @@ proc traceOpCodeStarted*(c: Computation, op: Op): int {.gcsafe, raises: [].} =
     c.gasMeter.gasRemaining,
     c.msg.depth + 1)
 
+proc traceCallFamilyGas*(c: Computation, op: Op, gas: GasInt) {.gcsafe, raises: [].} =
+  c.vmState.callFamilyGas(op, gas, c.msg.depth + 1)
+
 proc traceOpCodeEnded*(c: Computation, op: Op, opIndex: int) {.gcsafe, raises: [].} =
   c.vmState.captureOpEnd(
     c.code.pc - 1,

nimbus/evm/interpreter/op_handlers/oph_call.nim

@@ -203,8 +203,11 @@ const
       raise newException(
         StaticContextError,
         "Cannot modify state while inside of a STATICCALL context")
-
-    let p = cpt.callParams
+
+    let
+      gasAtStart = cpt.gasMeter.gasRemaining
+      p = cpt.callParams
+
     cpt.asyncChainTo(ifNecessaryGetAccounts(cpt.vmState, @[p.sender])):
       cpt.asyncChainTo(ifNecessaryGetCodeForAccounts(cpt.vmState, @[p.contractAddress, p.codeAddress])):
         var (gasCost, childGasLimit) = cpt.gasCosts[Call].c_handler(
@@ -238,6 +241,9 @@ const
           raise newException(
             OutOfGas, "Gas not enough to perform calculation (call)")
 
+        gasCost = gasAtStart - cpt.gasMeter.gasRemaining
+        cpt.traceCallFamilyGas(Call, gasCost)
+
         cpt.memory.extend(p.memInPos, p.memInLen)
         cpt.memory.extend(p.memOutPos, p.memOutLen)
 
@@ -287,6 +293,7 @@ const
     ## 0xf2, Message-call into this account with an alternative account's code.
     let
       cpt = k.cpt
+      gasAtStart = cpt.gasMeter.gasRemaining
       p = cpt.callCodeParams
 
     cpt.asyncChainTo(ifNecessaryGetAccounts(cpt.vmState, @[p.sender])):
@@ -325,6 +332,9 @@ const
           raise newException(
             OutOfGas, "Gas not enough to perform calculation (callCode)")
 
+        gasCost = gasAtStart - cpt.gasMeter.gasRemaining
+        cpt.traceCallFamilyGas(CallCode, gasCost)
+
         cpt.memory.extend(p.memInPos, p.memInLen)
         cpt.memory.extend(p.memOutPos, p.memOutLen)
 
@@ -375,6 +385,7 @@ const
     ##       code, but persisting the current values for sender and value.
     let
       cpt = k.cpt
+      gasAtStart = cpt.gasMeter.gasRemaining
       p = cpt.delegateCallParams
 
     cpt.asyncChainTo(ifNecessaryGetAccounts(cpt.vmState, @[p.sender])):
@@ -409,6 +420,9 @@ const
           raise newException(
             OutOfGas, "Gas not enough to perform calculation (delegateCall)")
 
+        gasCost = gasAtStart - cpt.gasMeter.gasRemaining
+        cpt.traceCallFamilyGas(DelegateCall, gasCost)
+
         cpt.memory.extend(p.memInPos, p.memInLen)
         cpt.memory.extend(p.memOutPos, p.memOutLen)
 
@@ -451,6 +465,7 @@ const
 
     let
       cpt = k.cpt
+      gasAtStart = cpt.gasMeter.gasRemaining
       p = cpt.staticCallParams
 
     cpt.asyncChainTo(ifNecessaryGetAccounts(cpt.vmState, @[p.sender])):
@@ -490,6 +505,9 @@ const
           raise newException(
             OutOfGas, "Gas not enough to perform calculation (staticCall)")
 
+        gasCost = gasAtStart - cpt.gasMeter.gasRemaining
+        cpt.traceCallFamilyGas(StaticCall, gasCost)
+
         cpt.memory.extend(p.memInPos, p.memInLen)
         cpt.memory.extend(p.memOutPos, p.memOutLen)
 

nimbus/evm/interpreter/op_handlers/oph_create.nim

@@ -99,11 +99,13 @@ const
       raise newException(InitcodeError,
         &"CREATE: have {memLen}, max {EIP3860_MAX_INITCODE_SIZE}")
 
-    let gasParams = GasParams(
-      kind:              Create,
-      cr_currentMemSize: k.cpt.memory.len,
-      cr_memOffset:      memPos,
-      cr_memLength:      memLen)
+    let
+      gasAtStart = k.cpt.gasMeter.gasRemaining
+      gasParams = GasParams(
+        kind:              Create,
+        cr_currentMemSize: k.cpt.memory.len,
+        cr_memOffset:      memPos,
+        cr_memLength:      memLen)
 
     var gasCost = k.cpt.gasCosts[Create].c_handler(1.u256, gasParams).gasCost
     k.cpt.gasMeter.consumeGas(
@@ -132,6 +134,9 @@ const
       createMsgGas -= createMsgGas div 64
     k.cpt.gasMeter.consumeGas(createMsgGas, reason = "CREATE")
 
+    gasCost = gasAtStart - k.cpt.gasMeter.gasRemaining
+    k.cpt.traceCallFamilyGas(Create, gasCost)
+
     when evmc_enabled:
       let
         msg = new(nimbus_message)
@@ -177,11 +182,13 @@ const
       raise newException(InitcodeError,
         &"CREATE2: have {memLen}, max {EIP3860_MAX_INITCODE_SIZE}")
 
-    let gasParams = GasParams(
-      kind:              Create,
-      cr_currentMemSize: k.cpt.memory.len,
-      cr_memOffset:      memPos,
-      cr_memLength:      memLen)
+    let
+      gasAtStart = k.cpt.gasMeter.gasRemaining
+      gasParams = GasParams(
+        kind:              Create,
+        cr_currentMemSize: k.cpt.memory.len,
+        cr_memOffset:      memPos,
+        cr_memLength:      memLen)
 
     var gasCost = k.cpt.gasCosts[Create].c_handler(1.u256, gasParams).gasCost
     gasCost = gasCost + k.cpt.gasCosts[Create2].m_handler(0, 0, memLen)
@@ -212,6 +219,9 @@ const
       createMsgGas -= createMsgGas div 64
     k.cpt.gasMeter.consumeGas(createMsgGas, reason = "CREATE2")
 
+    gasCost = gasAtStart - k.cpt.gasMeter.gasRemaining
+    k.cpt.traceCallFamilyGas(Create2, gasCost)
+
     when evmc_enabled:
       let
         msg = new(nimbus_message)

nimbus/evm/interpreter_dispatch.nim

@@ -257,8 +257,8 @@ proc executeOpcodes*(c: Computation, shouldPrepareTracer: bool = true)
     except CatchableError as e:
       let
         msg = e.msg
-        depth = $c.msg.depth
-      c.setError("Opcode Dispatch Error msg=" & msg & ", depth=" & depth, true)
+        depth = $(c.msg.depth + 1) # plus one to match tracer depth, and avoid confusion
+      c.setError("Opcode Dispatch Error: " & msg & ", depth=" & depth, true)
 
   if c.isError() and c.continuation.isNil:
     if c.tracingEnabled: c.traceError()

nimbus/evm/stack.nim

@@ -58,7 +58,7 @@ proc ensurePop(elements: Stack, a: int) =
   let expected = a
   if num < expected:
     raise newException(InsufficientStack,
-      &"Stack underflow: expected {expected} elements, got {num} instead.")
+      &"Stack underflow, expect {expected}, got {num}")
 
 proc popAux[T](stack: var Stack, value: var T) =
   ensurePop(stack, 1)
@@ -103,7 +103,7 @@ proc swap*(stack: var Stack, position: int) =
     (stack.values[^1], stack.values[^idx]) = (stack.values[^idx], stack.values[^1])
   else:
     raise newException(InsufficientStack,
-                      &"Insufficient stack items for SWAP{position}")
+                      "Stack underflow for SWAP" & $position)
 
 template getInt(x: int): int = x
 
@@ -114,7 +114,7 @@ proc dup*(stack: var Stack, position: int | UInt256) =
     stack.push(stack.values[^position])
   else:
     raise newException(InsufficientStack,
-                      &"Insufficient stack items for DUP{position}")
+                      "Stack underflow for DUP" & $position)
 
 proc peek*(stack: Stack): UInt256 =
   # This should be used only for testing purposes!

nimbus/evm/state.nim

@@ -393,6 +393,12 @@ proc captureOpStart*(vmState: BaseVMState, pc: int,
   if vmState.tracingEnabled:
     result = vmState.tracer.captureOpStart(pc, op, gas, depth)
 
+proc callFamilyGas*(vmState: BaseVMState,
+                    op: Op, gas: GasInt,
+                    depth: int) =
+  if vmState.tracingEnabled:          
+    vmState.tracer.callFamilyGas(op, gas, depth)
+
 proc captureOpEnd*(vmState: BaseVMState, pc: int,
                    op: Op, gas: GasInt, refund: GasInt,
                    rData: openArray[byte],

nimbus/evm/tracer/json_tracer.nim

@@ -29,6 +29,17 @@ type
     stack: JsonNode
     storageKeys: seq[HashSet[UInt256]]
     index: int
+    callFamilyNode: JsonNode
+
+const
+  callFamily = [
+    Create,
+    Create2,
+    Call,
+    CallCode,
+    DelegateCall,
+    StaticCall,
+  ]
 
 template stripLeadingZeros(value: string): string =
   var cidx = 0
@@ -41,7 +52,10 @@ proc encodeHexInt(x: SomeInteger): JsonNode =
   %("0x" & x.toHex.stripLeadingZeros.toLowerAscii)
 
 proc `%`(x: openArray[byte]): JsonNode =
-  %("0x" & x.toHex)
+  if x.len == 0:
+    %("")
+  else:
+    %("0x" & x.toHex)
 
 proc writeJson(ctx: JsonTracer, res: JsonNode) =
   try:
@@ -113,7 +127,10 @@ proc captureOpImpl(ctx: JsonTracer, pc: int,
   if error.isSome:
     res["error"] = %(error.get)
 
-  ctx.writeJson(res)
+  if op in callFamily:
+    ctx.callFamilyNode = res
+  else:
+    ctx.writeJson(res)
 
 proc newJsonTracer*(stream: Stream, flags: set[TracerFlags], pretty: bool): JsonTracer =
   JsonTracer(
@@ -171,13 +188,34 @@ method captureOpStart*(ctx: JsonTracer, pc: int,
     except ValueError as ex:
       error "JsonTracer captureOpStart", msg=ex.msg
 
+  if op in callFamily:
+    try:
+      ctx.captureOpImpl(pc, op, 0, 0, [], depth, none(string))
+    except RlpError as ex:
+      error "JsonTracer captureOpEnd", msg=ex.msg
+
+  # make sure captureOpEnd get the right opIndex
   result = ctx.index
   inc ctx.index
 
+method callFamilyGas*(ctx: JsonTracer,
+                      op: Op, gas: GasInt,
+                      depth: int) {.gcsafe.} =
+  doAssert(op in callFamily)
+  doAssert(ctx.callFamilyNode.isNil.not)
+  let res = ctx.callFamilyNode
+  res["gasCost"] = encodeHexInt(gas)
+  ctx.writeJson(res)
+
 method captureOpEnd*(ctx: JsonTracer, pc: int,
                      op: Op, gas: GasInt, refund: GasInt,
                      rData: openArray[byte],
                      depth: int, opIndex: int) {.gcsafe.} =
+
+  if op in callFamily:
+    # call family opcode is processed in captureOpStart
+    return
+
   try:
     ctx.captureOpImpl(pc, op, gas, refund, rData, depth, none(string))
   except RlpError as ex:

nimbus/evm/types.nim

@@ -173,6 +173,11 @@ method captureOpStart*(ctx: TracerRef, pc: int,
                        depth: int): int {.base, gcsafe.} =
   discard
 
+method callFamilyGas*(ctx: TracerRef,
+                      op: Op, gas: GasInt,
+                      depth: int) {.base, gcsafe.} =
+  discard
+
 method captureOpEnd*(ctx: TracerRef, pc: int,
                      op: Op, gas: GasInt, refund: GasInt,
                      rData: openArray[byte],
@@ -188,4 +193,3 @@ method captureFault*(ctx: TracerRef, pc: int,
 
 method capturePrepare*(ctx: TracerRef, depth: int) {.base, gcsafe.} =
   discard
-

nimbus/vm_state.nim

@@ -43,6 +43,7 @@ export
   vms.captureEnter,
   vms.captureExit,
   vms.captureOpStart,
+  vms.callFamilyGas,
   vms.captureOpEnd,
   vms.captureFault,
   vms.capturePrepare

tests/fixtures/TracerTests/block46402.json

@@ -283,7 +283,7 @@
             "0000000000000000000000000000000000000000000000000000000000000000",
             "0000000000000000000000000000000000000000000000000000000000000060"
           ],
-          "error": "Opcode Dispatch Error msg=Out of gas: Needed 20000 - Remaining 412 - Reason: SSTORE, depth=0",
+          "error": "Opcode Dispatch Error: Out of gas: Needed 20000 - Remaining 412 - Reason: SSTORE, depth=1",
           "gasCost": 0
         }
       ],
@@ -705,7 +705,7 @@
           "0000000000000000000000000000000000000000000000000000000000000000",
           "0000000000000000000000000000000000000000000000000000000000000060"
         ],
-        "error": "Opcode Dispatch Error msg=Out of gas: Needed 20000 - Remaining 412 - Reason: SSTORE, depth=0",
+        "error": "Opcode Dispatch Error: Out of gas: Needed 20000 - Remaining 412 - Reason: SSTORE, depth=1",
         "gasCost": 0
       }
     ]

tools/t8n/helpers.nim

@@ -212,9 +212,6 @@ proc parseTx(n: JsonNode, chainId: ChainID): Transaction =
   required(tx, GasInt, gas)
   required(tx, UInt256, value)
   required(tx, Blob, input)
-  required(tx, int64, v)
-  required(tx, UInt256, r)
-  required(tx, UInt256, s)
 
   if n.hasKey("to"):
     tx.to = some(EthAddress.fromJson(n, "to"))
@@ -248,6 +245,9 @@ proc parseTx(n: JsonNode, chainId: ChainID): Transaction =
     let secretKey = PrivateKey.fromRaw(data).tryGet
     signTransaction(tx, secretKey, chainId, eip155)
   else:
+    required(tx, int64, v)
+    required(tx, UInt256, r)
+    required(tx, UInt256, s)
     tx
 
 proc parseTxLegacy(item: var Rlp): Result[Transaction, string] =

tools/t8n/t8n_test.nim

@@ -498,14 +498,23 @@ const
       expOut: "exp.json",
     ),
     TestSpec(
-      name  : "EVM tracer crash bug",
+      name  : "EVM tracer nil stack crash bug",
       base  : "testdata/00-519",
       input : t8nInput(
         "alloc.json", "txs.json", "env.json", "Shanghai", "0",
       ),
       output: T8nOutput(trace: true),
       expOut: "exp.txt",
     ),
+    TestSpec(
+      name  : "EVM tracer wrong order for CALL family opcodes",
+      base  : "testdata/00-520",
+      input : t8nInput(
+        "alloc.json", "txs.json", "env.json", "Merge", "0",
+      ),
+      output: T8nOutput(trace: true, result: true),
+      expOut: "exp.txt",
+    ),
   ]
 
 proc main() =

tools/t8n/testdata/00-519/exp.txt

@@ -1,2 +1,2 @@
 {"pc":0,"op":0,"gas":"0x0","gasCost":"0xfffffffffffecb68","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"STOP","error":"Blake2b F function invalid input"}
-{"output":"0x","gasUsed":"0x13498","error":"Blake2b F function invalid input"}
+{"output":"","gasUsed":"0x13498","error":"Blake2b F function invalid input"}