-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: cast decode can decode raw eip2718 txns #5779
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
Conversation
|
@Evalir would really appreciate a review here. I think I'm close to getting this over the line and only struggling with the final step of the solution described above i.e. pretty-printing the decoded output. This is what the current output looks using
There's probably a third way - construct a custom template string which would replicate the desired format exactly, but I doubt if this is the best way to go since it tightly couples Any upstream modification of fields, new types of txns, and even potential migration of foundry away from ethers-rs itself would add an maintenance overhead. Additionally with three variants of the Curious to hear any thoughts and suggestions. P.S - I'm aware the PR is missing proper comments, will add those before finalising it. Apologies if i'm missing something simple... only started with rust a few weeks back. |
mattsse
left a comment
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.
a few nits
mattsse
left a comment
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, last nit
crates/cast/bin/main.rs
Outdated
| // Serialise tx and sig as json strings and reformat to print as a single json object | ||
| let mut tx = serde_json::to_string_pretty(&tx)?; | ||
| tx.truncate(tx.len() - 2); | ||
| let sig = serde_json::to_string_pretty(&sig)?[2..].to_string(); | ||
| println!("{},\n{}", tx, sig); |
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.
nit: both can be serialized into to_value and then merged like this:
fn merge(a: &mut Value, b: Value) {
match (a, b) {
(a @ &mut Value::Object(_), Value::Object(b)) => {
let a = a.as_object_mut().unwrap();
for (k, v) in b {
merge(a.entry(k).or_insert(Value::Null), v);
}
}
(a, b) => *a = b,
}
}https://stackoverflow.com/questions/47070876/how-can-i-merge-two-json-objects-with-rust
since we know the signature has no nested objects we don't need the recursive step
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.
super neat – thanks for pointing this out. will push a fix in just a bit
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.
just pushed and refactord it to be more concise:
// Serialize tx, sig and constructed a merged json string
let mut tx = serde_json::to_value(&tx)?;
let tx_map = tx.as_object_mut().unwrap();
serde_json::to_value(sig)?.as_object().unwrap().iter().for_each(|(k, v)| {
tx_map.entry(k).or_insert(v.clone());
});
mattsse
left a comment
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
|
gas diffs unrelated |
* feat: cast decode can decode raw eip2718 txns * fix: refactor impl, reformat result, qol changes * fix: failing doctests * refactor: merged json output object
Motivation
Resolve #4752 and implement
cast decodeto decode raw EIP2718 transaction payloadsSolution
DecodeSimpleCast::decode_txthat usesethers_core::utils::rlpto decode raw eip2718 signed txns toethers_core::types::transaction::eip2718::TypedTransactionandSignature