-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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: do system contract processing prior to parallel-tracing #30520
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay from a glance (but I'll defer to @s1na to give thumbs up on this). One thing that stands out is that we repeat this exact same logic in multiple places across this file, and it ought to be deduplicated into it's own function.
Please add a meaningful comment as to why the code you just added is needed and for this PR description what was broken and how it was fixed. There's no info about anything, just "here's some code that fixes something somehow, maybe" |
@karalabe Sorry for that. I have maked it more clear. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@s1na PTAL as a codeowner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
debug_traceBlockByNumber
may return incorrect result on some blocks, with JS tracers.This PR tries to fix it.
traceBlockParallel
does not applyProcessBeaconBlockRoot
andProcessParentBlockHash
before executing block's transactions, which may cause some transactions trace to report unexpected reverted errors.Case
request
In response, tx
0x8236b9e4749102df80e43130d7df505cb7e9b3036c2fa8759867b5fbbf837370
would show"error": "execution reverted"
, that's wrong.Expected result