fix(js): correct gas cost reporting by invoking step callback in step_end#396
Open
fix(js): correct gas cost reporting by invoking step callback in step_end#396
Conversation
1f43956 to
f8fcf34
Compare
…_end This fixes the long-standing issue where `log.getCost()` in JS tracers returned incorrect gas costs. The problem was that the JS step callback was invoked in `step()` (before opcode execution), but the gas cost is only known after the opcode executes. Changes: - Add `PendingStep` struct to capture pre-execution state (stack, memory, pc, op, etc.) in `step()` - Invoke the JS step callback in `step_end()` with the captured state and the correctly computed gas cost - Add `MemorySnapshot` to properly snapshot memory contents (SharedMemory clone is shallow due to Rc) - Update test to expect `[3, 3, 0]` (correct PUSH1, PUSH1, STOP costs) instead of `[0, 3, 3]` (lagged costs) The key insight is that we need the pre-execution stack/memory state but the post-execution gas cost. This requires a two-phase approach: 1. `step()`: Capture pre-execution snapshot 2. `step_end()`: Compute gas delta and invoke JS callback Fixes: paradigmxyz/reth#10959 Closes: #200 Co-authored-by: Delweng <delweng@gmail.com>
f8fcf34 to
c8b6f72
Compare
yongkangc
approved these changes
Jan 21, 2026
yongkangc
left a comment
There was a problem hiding this comment.
This is an interesting approach, makes sense
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes the long-standing issue where
log.getCost()in JS tracers returned incorrect gas costs (paradigmxyz/reth#10959, #200).Problem
The JS step callback was invoked in
step()(before opcode execution), but the gas cost is only known after the opcode executes. This caused the first opcode to report cost=0 and each subsequent opcode to report the previous opcode's cost.Before (incorrect):
[0, 3, 3]for PUSH1, PUSH1, STOPAfter (correct):
[3, 3, 0]for PUSH1, PUSH1, STOPSolution
The key insight is that we need the pre-execution stack/memory state but the post-execution gas cost. This requires a two-phase approach:
step(): Capture pre-execution snapshot (stack clone, memory snapshot, pc, op, etc.)step_end(): Compute gas delta and invoke JS callback with the snapshotChanges
PendingStepstruct to capture pre-execution state instep()MemorySnapshotto properly snapshot memory contents (SharedMemory clone is shallow due to Rc)step_end()with captured state and correct gas costTesting
Verified against live node that the fix produces correct gas costs:
All existing tests pass.
Breaking Change Note
JS tracer errors that occur on the final opcode (like STOP) can no longer revert the transaction since the opcode has already completed. The transaction succeeds but the trace may be incomplete. This matches the expected behavior since the tracer is observational.
Fixes: paradigmxyz/reth#10959
Closes: #200