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.

Sign up for GitHub

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

Jump to bottom

eth/tracers: native prestate tracer #24268

Merged
merged 13 commits into from
Jan 31, 2022
Merged

eth/tracers: native prestate tracer #24268

merged 13 commits into from
Jan 31, 2022

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Jan 21, 2022 •
edited
Loading

Some data points from a live node. One native measurement, then JS, then native again to rule out caching etc.

Block 14119300  Native1: 150ms  JS: 2726ms      Native2: 132ms
Block 14119301  Native1: 97ms   JS: 1116ms      Native2: 96ms
Block 14119302  Native1: 986ms  JS: 8903ms      Native2: 729ms
Block 14119303  Native1: 4530ms JS: 8484ms      Native2: 2879ms
Block 14119304  Native1: 360ms  JS: 4509ms      Native2: 538ms
Block 14119305  Native1: 1ms    JS: 2ms Native2: 2ms
Block 14119306  Native1: 2095ms JS: 5529ms      Native2: 316ms
Block 14119307  Native1: 549ms  JS: 8219ms      Native2: 1287ms
Block 14119308  Native1: 27ms   JS: 419ms       Native2: 24ms
Block 14119309  Native1: 846ms  JS: 10408ms     Native2: 784ms
Block 14119310  Native1: 93ms   JS: 996ms       Native2: 83ms
Block 14119311  Native1: 92ms   JS: 2233ms      Native2: 121ms
Block 14119312  Native1: 3422ms JS: 9541ms      Native2: 2091ms
Block 14119313  Native1: 548ms  JS: 7851ms      Native2: 1132ms
Block 14119314  Native1: 598ms  JS: 3712ms      Native2: 306ms

@s1na
Copy link
Contributor Author

s1na commented Jan 24, 2022

I came across a difference between the native and JS tracers when it comes to recipient, as in https://goerli.etherscan.io/tx/0x330ed9ccffff3530ed4148bf8ff017d6ac04891d7df17c677a3d2d95cf2150e7. JS tracer says 0x5eb19848419e963b and native one says 0x5eb19b611657556b. I believe the JS one is incorrect. That's because there we compute the sender's balance at the end (by re-adding value and gas costs). That doesn't account for any transfers to the tx sender during the tx as happens in this tx.

s1na and others added 3 commits January 25, 2022 14:06
@s1na @holiman
use switch instead
44d8b57 
Co-authored-by: Martin Holst Swende <martin@swende.se>
@s1na
minor fix
1efc147
@s1na
lookup create2 account
c9c70e7
@holiman
Copy link
Contributor

holiman commented Jan 25, 2022

I believe the JS one is incorrect. That's because there we compute the sender's balance at the end (by re-adding value and gas costs). That doesn't account for any transfers to the tx sender during the tx as happens in this tx.

Yep, makes sense. Should be a fairly common occurrence, e.g. if someone swaps a token to eth.

@holiman
Copy link
Contributor

holiman commented Jan 28, 2022

Still marked as draft -- is that correct?

@s1na
Copy link
Contributor Author

s1na commented Jan 28, 2022

Still marked as draft -- is that correct?

I was just waiting to be able to test it on some mainnet blocks. By now I've compared the native output against JS for 100+ mainnet blocks and they match. Taking it out of draft now. Here's a comparison of time (tracing done on a live node):

Block 14094980  JS: 2514ms      Native: 632ms
Block 14094981  JS: 1088ms      Native: 103ms
Block 14094982  JS: 1394ms      Native: 181ms
Block 14094983  JS: 187ms       Native: 22ms
Block 14094984  JS: 5555ms      Native: 392ms
Block 14094985  JS: 1160ms      Native: 112ms
Block 14094986  JS: 210ms       Native: 13ms
Block 14094987  JS: 852ms       Native: 56ms
Block 14094988  JS: 3324ms      Native: 596ms
Block 14094989  JS: 3517ms      Native: 531ms
Block 14094990  JS: 1049ms      Native: 86ms
Block 14094991  JS: 5970ms      Native: 3155ms
Block 14094992  JS: 4687ms      Native: 534ms
Block 14094993  JS: 1587ms      Native: 314ms
Block 14094994  JS: 5710ms      Native: 270ms
Block 14094995  JS: 3243ms      Native: 335ms
Block 14094996  JS: 3463ms      Native: 674ms
Block 14094997  JS: 1655ms      Native: 71ms
Block 14094998  JS: 3463ms      Native: 237ms
Block 14094999  JS: 801ms       Native: 62ms

Also this PR has now fixes for 2 issues in the JS prestate tracer:

  • Simple transfer txes failed completely
  • EXTCODEHASH op wasn't being accounted for

@s1na s1na marked this pull request as ready for review January 28, 2022 15:36
holiman
holiman approved these changes Jan 28, 2022
View reviewed changes
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.

Awesome, LGTM!

@holiman holiman added this to the 1.10.16 milestone Jan 28, 2022
@holiman holiman merged commit cac09a3 into ethereum:master Jan 31, 2022
@s1na
Copy link
Contributor Author

s1na commented Feb 1, 2022

I noticed I could improve the benchmark's accuracy by changing up the order of calling the tracers. So what I did this time is first trace native, then JS, then native again just to make sure caching is not at play. It's a live node so there are some jitters. Seems like mostly the gains are there, maybe in a few cases a bit less:

Block 14119300  Native1: 150ms  JS: 2726ms      Native2: 132ms
Block 14119301  Native1: 97ms   JS: 1116ms      Native2: 96ms
Block 14119302  Native1: 986ms  JS: 8903ms      Native2: 729ms
Block 14119303  Native1: 4530ms JS: 8484ms      Native2: 2879ms
Block 14119304  Native1: 360ms  JS: 4509ms      Native2: 538ms
Block 14119305  Native1: 1ms    JS: 2ms Native2: 2ms
Block 14119306  Native1: 2095ms JS: 5529ms      Native2: 316ms
Block 14119307  Native1: 549ms  JS: 8219ms      Native2: 1287ms
Block 14119308  Native1: 27ms   JS: 419ms       Native2: 24ms
Block 14119309  Native1: 846ms  JS: 10408ms     Native2: 784ms
Block 14119310  Native1: 93ms   JS: 996ms       Native2: 83ms
Block 14119311  Native1: 92ms   JS: 2233ms      Native2: 121ms
Block 14119312  Native1: 3422ms JS: 9541ms      Native2: 2091ms
Block 14119313  Native1: 548ms  JS: 7851ms      Native2: 1132ms
Block 14119314  Native1: 598ms  JS: 3712ms      Native2: 306ms

sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Feb 1, 2022
@s1na @holiman
eth/tracers: native prestate tracer (ethereum#24268)
7cf2586 
* eth/tracers: add initial native prestate tracer

* fix balance hex

* handle prestate for tx from and to

* drop created contract from prestate

* fix sender balance

* use switch instead

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

* minor fix

* lookup create2 account

* mv code around a bit

* check stackLen for create2

* fix transfer tx for js prestate tracer

* fix create2 addr

* track extcodehash in js prestate tracer

Co-authored-by: Martin Holst Swende <martin@swende.se>
JacekGlen pushed a commit to JacekGlen/go-ethereum that referenced this pull request May 26, 2022
@s1na @holiman @JacekGlen
eth/tracers: native prestate tracer (ethereum#24268)
820c47c 
* eth/tracers: add initial native prestate tracer

* fix balance hex

* handle prestate for tx from and to

* drop created contract from prestate

* fix sender balance

* use switch instead

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

* minor fix

* lookup create2 account

* mv code around a bit

* check stackLen for create2

* fix transfer tx for js prestate tracer

* fix create2 addr

* track extcodehash in js prestate tracer

Co-authored-by: Martin Holst Swende <martin@swende.se>
@palango palango mentioned this pull request Apr 26, 2023
Merged
@lispc lispc mentioned this pull request Sep 6, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holiman holiman holiman approved these changes

@karalabe karalabe Awaiting requested review from karalabe

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.16
Development

Successfully merging this pull request may close these issues.
2 participants
@s1na @holiman