-
Notifications
You must be signed in to change notification settings - Fork 796
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
feat: add support for Geth built-in tracer and config #2121
Conversation
#[serde(default, deserialize_with = "from_int_or_hex")] | ||
pub gas: U256, | ||
#[serde(default, deserialize_with = "from_int_or_hex", rename = "gasUsed")] | ||
pub gas_used: U256, |
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.
The gas
and gas_used
fields are not compatible with the legacy format. Should we change it to Option
, and which might be a break change?
For now, to be compatible with previous versions, I've only added a default handling, which returns U256::zero()
instead of None
.
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.
default
is fine w me
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.
thanks,
left some nits.
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] | ||
pub struct PreStateMode(pub BTreeMap<Address, AccountState>); |
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.
I think this should probably b #[serde(transparent)]
?
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.
Seems transparent is enabled for newtype by default?
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.
TIL
ethers-core/src/types/trace/geth/test_data/call_tracer/default.rs
Outdated
Show resolved
Hide resolved
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
Motivation
Now only
callTracer
is supported, add morebuild-in
tracer and config support.Solution
Refs: #2064
PR Checklist