-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
core/vm/runtime: invoke tx-end hook #30711
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.
LGTM, we could set more fields in the receipt than just the gasUsed e.g. Type
, PostState
, Status
, ... but I don't know if thats really important. The only thing that might be is PostState imo, but that would require us to call Commit() on the passed statedb. That is something that I don't think we should do
It is not important. It's IMO a bit annoying that we need to even create this pretend-receipt. I'd rather just pass in nil, but it turns out that some tracers crash on nil-receipts. |
In general IMO it's not the best idea to pass too much, too high level data to callbacks directly. It expands our API surface a lot and also locks them in, since all of a sudden all the types are API with guarantees; and it also makes optimizations harder because you cannot avoid computing something that a tracer depends on later. |
Yep, I agree |
When using the `core/vm/runtime` helpers to execute code, callbacks for the tx end were not invoked. This change fixes it by invoking them.
When using the `core/vm/runtime` helpers to execute code, callbacks for the tx end were not invoked. This change fixes it by invoking them.
While updating goevmlab, I noticed that the way I invoked the executions, using the
core/vm/runtime
helpers, I didn't get any callbacks on the tx end. This PR fixes it by invoking them.