eth/tracers: various fixes #30540
eth/tracers: various fixes #30540
Conversation
|
I think some other fields in VMContext which are block-related can also be moved to the constructor. But keeping that for future work. |
|
Apart of the nits, SGTM |
|
I have addressed the comments. PTAL. |
|
Please add a description in the description-field. Mostly so that
Doesn't have to be a novel. It can be sufficient with "This PR addresses minor internal rough edges and should have no observable difference". (Note, haven't checked the code yet, have no idea if that really is the case here, just an example) |
|
We should have the chain config passed for the live tracer constructors and drop the blockchain init hook altogether - unless there's a good reason to have it. It might also be interesting to think about how we could access the state / chain on init. |
That's a breaking change for live tracers. I would rather not do it in this PR. Ah come to think of it removing ChainConfig from the VMContext is also a breaking change for live tracers. |
|
Then all current APIs should be moved into a v1 package and a v2 created that's designed better. It is unacceptable like it is now. |
Breaking changes:
- The ChainConfig was exposed to tracers via VMContext passed in
`OnTxStart`. This is unnecessary specially looking through the lens of
live tracers as chain config remains the same throughout the lifetime of
the program. It was there so that native API-invoked tracers could
access it. So instead we moved it to the constructor of API tracers.
Non-breaking:
- Change the default config of the tracers to be `{}` instead of nil.
This way an extra nil check can be avoided.
Refactoring:
- Rename `supply` struct to `supplyTracer`.
- Un-export some hook definitions.
Breaking changes:
- The ChainConfig was exposed to tracers via VMContext passed in
`OnTxStart`. This is unnecessary specially looking through the lens of
live tracers as chain config remains the same throughout the lifetime of
the program. It was there so that native API-invoked tracers could
access it. So instead we moved it to the constructor of API tracers.
Non-breaking:
- Change the default config of the tracers to be `{}` instead of nil.
This way an extra nil check can be avoided.
Refactoring:
- Rename `supply` struct to `supplyTracer`.
- Un-export some hook definitions.
Changes in this PR
Breaking
OnTxStart. This is unnecessary specially looking through the lens of live tracers as chain config remains the same throughout the lifetime of the program. It was there so that native API-invoked tracers could access it. So instead we moved it to the constructor of API tracers.Non-breaking
{}instead of nil. This way an extra nil check can be avoided.Refactoring
supplystruct tosupplyTracer.