Skip to content

Add vmTrace option to the tracing RPC functions#768

Draft
jenikd wants to merge 3 commits intomainfrom
jenikd/add_vmTrace
Draft

Add vmTrace option to the tracing RPC functions#768
jenikd wants to merge 3 commits intomainfrom
jenikd/add_vmTrace

Conversation

@jenikd
Copy link
Copy Markdown
Contributor

@jenikd jenikd commented Mar 26, 2026

This PR adds full VM opcode tracing to the RPC API calls trace_call and trace_callMany, compatible with Erigon/OpenEthereum formats.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 91.25683% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
txtrace/vm_trace.go 88.39% 9 Missing and 4 partials ⚠️
ethapi/tx_trace.go 95.77% 2 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
txtrace/state_diff.go 100.00% <ø> (ø)
ethapi/tx_trace.go 45.82% <95.77%> (+5.47%) ⬆️
txtrace/vm_trace.go 88.39% <88.39%> (ø)

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new vmTrace tracing mode to the trace_call / trace_callMany RPC flow, intended to expose per-opcode VM execution traces compatible with Erigon/OpenEthereum formats.

Changes:

  • Introduces a new txtrace.VmTraceLogger that builds per-opcode traces (code, ops, stack “push”, memory snapshot, storage writes, subtraces).
  • Extends trace option parsing and EVM setup to enable vmTrace alongside existing trace and stateDiff.
  • Adds substantial unit test coverage for vmTrace logger behavior and for hook-merging logic in ethapi.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
txtrace/vm_trace.go New VM opcode tracing structs/logger and stack “push” computation.
txtrace/vm_trace_test.go New tests for computePushed and VmTraceLogger behavior.
txtrace/state_diff.go Changes TraceCallResult.VmTrace from any to *VmTrace.
ethapi/tx_trace.go Adds VmTrace support via TraceOptions, state wrapping, and merged VM hooks.
ethapi/tx_trace_test.go Updates trace option usage and adds tests for mergeVMHooks and vmTrace-only/all-types flows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vm.ADD, vm.EXP, vm.CALLER, vm.KECCAK256, vm.SUB, vm.ADDRESS, vm.GAS, vm.MUL, vm.RETURNDATASIZE, vm.NOT, vm.SHR, vm.SHL,
vm.EXTCODESIZE, vm.SLT, vm.OR, vm.NUMBER, vm.PC, vm.TIMESTAMP, vm.BALANCE, vm.SELFBALANCE, vm.MULMOD, vm.ADDMOD, vm.BASEFEE,
vm.BLOCKHASH, vm.BYTE, vm.XOR, vm.ORIGIN, vm.CODESIZE, vm.MOD, vm.SIGNEXTEND, vm.GASLIMIT, vm.DIFFICULTY, vm.SGT, vm.GASPRICE,
vm.MSIZE, vm.EXTCODEHASH, vm.SMOD, vm.CHAINID, vm.COINBASE, vm.TLOAD:
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computePushed treats CALL/CREATE-family opcodes as pushing 0 items, but these opcodes always push a result onto the stack (e.g., CALL*/STATICCALL/DELEGATECALL/CALLCODE push a success word; CREATE/CREATE2 push the created address or 0). To make vmTrace actually reflect stack effects (and match the stated Erigon/OpenEthereum compatibility), include these opcodes in the single-return set so push contains the top-of-stack result.

Suggested change
vm.MSIZE, vm.EXTCODEHASH, vm.SMOD, vm.CHAINID, vm.COINBASE, vm.TLOAD:
vm.MSIZE, vm.EXTCODEHASH, vm.SMOD, vm.CHAINID, vm.COINBASE, vm.TLOAD,
vm.CALL, vm.CALLCODE, vm.DELEGATECALL, vm.STATICCALL, vm.CREATE, vm.CREATE2:

Copilot uses AI. Check for mistakes.
Comment on lines +151 to +157
ops := []vm.OpCode{
vm.STOP, vm.MSTORE, vm.MSTORE8, vm.SSTORE,
vm.JUMP, vm.JUMPI, vm.JUMPDEST,
vm.POP, vm.RETURN, vm.REVERT,
vm.CALL, vm.DELEGATECALL, vm.STATICCALL,
vm.CREATE, vm.CREATE2, vm.SELFDESTRUCT,
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test currently asserts that CALL/DELEGATECALL/STATICCALL and CREATE/CREATE2 return no stack items (empty push), but these opcodes push a result word/address onto the stack. Once computePushed is corrected, update this test to remove those opcodes from the zero-return list (and add assertions that they produce a single pushed value).

Copilot uses AI. Check for mistakes.
Comment on lines 327 to 334
// Execute the transaction using core.ApplyMessage to capture raw return data.
result, applyErr := core.ApplyMessage(tracedEVM.vmenv, msg, new(core.GasPool).AddGas(msg.GasLimit))
result, err := core.ApplyMessage(tracedEVM.vmenv, msg, new(core.GasPool).AddGas(msg.GasLimit))
if err != nil {
return nil, err
}

// Finalize the transaction state regardless of execution outcome.
tracedEVM.activeState.EndTransaction()
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

traceCallExec now returns immediately on core.ApplyMessage error, which skips activeState.EndTransaction() and tracer.OnTxEnd(...). That changes trace_call behavior (previously it could return an error trace/result) and can leave per-tx state/trace hooks unfinalized. Consider ensuring EndTransaction/OnTxEnd always run (even on ApplyMessage error) and preserve the prior behavior of returning a structured error trace when requested instead of a hard RPC error.

Copilot uses AI. Check for mistakes.
if memData == nil {
memData = []byte{}
}
op.Ex.Mem = &MemoryDiff{Off: uint64(len(memData)), Data: hexutil.Bytes(memData)}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MemoryDiff.Off is being set to len(memData), which makes the snapshot’s offset point to the end of the provided Data. For a full memory snapshot (as the struct/doc comment indicates), Off should be the start offset (typically 0) so consumers interpret Data correctly and remain compatible with Erigon/OpenEthereum vmTrace formats.

Suggested change
op.Ex.Mem = &MemoryDiff{Off: uint64(len(memData)), Data: hexutil.Bytes(memData)}
// For a full memory snapshot, the offset should be 0 so consumers interpret Data correctly.
op.Ex.Mem = &MemoryDiff{Off: 0, Data: hexutil.Bytes(memData)}

Copilot uses AI. Check for mistakes.
if memData == nil {
memData = []byte{}
}
prevOp.Ex.Mem = &MemoryDiff{Off: uint64(len(memData)), Data: hexutil.Bytes(memData)}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same MemoryDiff.Off issue as above: Off is set to len(memData) when finalizing the previous opcode. This will produce inconsistent/invalid mem entries across ops; consider using Off: 0 (or the correct base offset per spec) for full snapshots.

Suggested change
prevOp.Ex.Mem = &MemoryDiff{Off: uint64(len(memData)), Data: hexutil.Bytes(memData)}
// Store a full memory snapshot starting at offset 0.
prevOp.Ex.Mem = &MemoryDiff{Off: 0, Data: hexutil.Bytes(memData)}

Copilot uses AI. Check for mistakes.
@jenikd jenikd marked this pull request as draft March 27, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants