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: add golang 4byte tracer #23882

Merged
merged 5 commits into from Nov 11, 2021
Merged

eth/tracers: add golang 4byte tracer #23882

merged 5 commits into from Nov 11, 2021

Conversation

wardbradt
Copy link
Contributor

No description provided.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

eth/tracers/native/4byte.go Outdated Show resolved Hide resolved
eth/tracers/native/4byte.go Outdated Show resolved Hide resolved
eth/tracers/native/4byte.go Outdated Show resolved Hide resolved
wardbradt and others added 3 commits November 10, 2021 13:51
Co-authored-by: Martin Holst Swende <martin@swende.se>
Co-authored-by: Martin Holst Swende <martin@swende.se>
@holiman
Copy link
Contributor

holiman commented Nov 11, 2021

Some timing stats

native: 8m22.495952253s:

INFO [11-11|08:30:28.658] Chain tracing finished                   start=13,401,259 end=13,402,259 transactions=188,695 elapsed=8m22.495952253s

js: 12m24.845998177s

INFO [11-11|08:53:48.225] Chain tracing finished                   start=13,401,259 end=13,402,259 transactions=188,695 elapsed=12m24.845998177s

js-legacy: 30m36.802869401s

INFO [11-11|09:48:25.003] Chain tracing finished                   start=13,401,259 end=13,402,259 transactions=188,695 elapsed=30m36.802869401s

I'm going to go through the results and see if there are any discrepancies

@holiman
Copy link
Contributor

holiman commented Nov 11, 2021

I've found zero discrepancies between js and go, however, there are differences between (js/go) vs legacy. They are of this type:

{
    ...skipped 2 object properties...,
    "traces": [
        ...skipped 50 array elements...,
        {
            "result": {
                ...skipped 1 object property...,
                "0x60806040-1708": 1,
                ...skipped 1 object property...
            }
        },
        ...skipped 106 array elements...,
        {
            "result": {
                "0x3d602d80-51": 1,
                ...skipped 8 object properties...
            }
        },

Which looks like code, to me. I suspect the legacy one correctly does not catch initcode, but the newer traces doesn't. So the new tracers needs to fixed. I'll push a fix

@holiman
Copy link
Contributor

holiman commented Nov 11, 2021

After a fix-up, the discrepancies are gone.
@wardbradt you seem to have disabled maintainer-commit on your fork of geth, so I can't push to this branch.
Can you cherry-pick this commit holiman@d57724b -- of my fixup branch?

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.

Very cool, thanks!

)

func init() {
register("4byte", newFourByteTracer)
Copy link
Contributor

Choose a reason for hiding this comment

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

So this would introduce a new tracer. If we want to make it the default it has to be called 4byteTracer

@holiman
Copy link
Contributor

holiman commented Nov 11, 2021

Second attempt,

js : 12m15.214591921s
native (warmed up by the previous run): 4m30.635180831s

Yes, I think we should make the go-tracer be the default, so register it as 4byteTracer

holiman
holiman previously approved these changes Nov 11, 2021
@holiman holiman dismissed their stale review November 11, 2021 14:02

Need additional fix-commit

Copy link
Contributor

@holiman holiman 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 holiman merged commit e9294a7 into ethereum:master Nov 11, 2021
@holiman holiman added this to the 1.10.13 milestone Nov 11, 2021
@holiman
Copy link
Contributor

holiman commented Nov 11, 2021

TODO for follow-up PR: make the golang tracer default

sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Nov 12, 2021
* native 4byte tracer

* Update eth/tracers/native/4byte.go

Co-authored-by: Martin Holst Swende <martin@swende.se>

* Update eth/tracers/native/4byte.go

Co-authored-by: Martin Holst Swende <martin@swende.se>

* goimports

* eth/tracers: make 4byte tracer not care about create

Co-authored-by: Martin Holst Swende <martin@swende.se>
yongjun925 pushed a commit to DODOEX/go-ethereum that referenced this pull request Dec 3, 2022
* native 4byte tracer

* Update eth/tracers/native/4byte.go

Co-authored-by: Martin Holst Swende <martin@swende.se>

* Update eth/tracers/native/4byte.go

Co-authored-by: Martin Holst Swende <martin@swende.se>

* goimports

* eth/tracers: make 4byte tracer not care about create

Co-authored-by: Martin Holst Swende <martin@swende.se>
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.

None yet

3 participants