Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth/tracers/logger: do not include elapsed time in evm output #26291

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Dec 2, 2022

When working with evm traces, in tools like goevmlab, it's a bit annoying when traces are not deterministic.

Example:

{"pc":2619,"op":80,"gas":"0x13a8","gasCost":"0x2","memSize":2240,"stack":["0x7dce2faf43218578e3fcf2ad22df9918a89e2fba"],"depth":1,"refund":0,"opName":"POP"}
{"pc":2620,"op":0,"gas":"0x13a6","gasCost":"0x0","memSize":2240,"stack":[],"depth":1,"refund":0,"opName":"STOP"}
{"output":"","gasUsed":"0xb2e6d","time":4638993}
{"stateRoot": "ad1024c87b5548e77c937aa50f72b6cb620d278f4dd79bae7f78f71ff75af458"}

The time is what makes the output change, and does not really bring a whole lot of added value. This PR removes the time element on the output line.

cc @s1na didn't you also gripe about this at some point?

Compatibility note

This introduces a backwards-incompatible change in the native tracing interface. The paramter t time.Duration is dropped from CaptureEnd.

@holiman holiman requested a review from s1na as a code owner December 2, 2022 18:04
@s1na
Copy link
Contributor

s1na commented Dec 4, 2022

Yes on-board with this. I went ahead and wiped it from the logger interface. If a tracer needs the time they can measure it themselves. No need for evm to do this.

Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman
Copy link
Contributor Author

holiman commented Dec 5, 2022

I went ahead and wiped it from the logger interface. If a tracer needs the time they can measure it themselves. No need for evm to do this.

SGTM 👍

@holiman holiman added this to the 1.11.0 milestone Dec 5, 2022
@holiman holiman merged commit 1f35988 into ethereum:master Dec 5, 2022
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
…erface (ethereum#26291)

This removes the 'time' field from logs, as well as from the tracer interface. This change makes the trace output deterministic.  If a tracer needs the time they can measure it themselves. No need for evm to do this.

Co-authored-by: Sina Mahmoodi <[email protected]>
holiman pushed a commit that referenced this pull request Oct 23, 2023
HighGoal1991 added a commit to HighGoal1991/go-ethereum that referenced this pull request Mar 21, 2024
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.

3 participants