-
Notifications
You must be signed in to change notification settings - Fork 2.1k
improve cdd json parsing #9928
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
improve cdd json parsing #9928
Conversation
@mattsse @zerosnacks would you mind checking this PR? |
Hi @tclemos thanks for your PR! Whilst I understand the need to parse this as suggested I don't think the proposed solution is the one we are looking for as it effectively reverts the formatting applied in https://github.com/foundry-rs/foundry/blob/5c6b9ae8143bce995e52e927297429257d354ffd/crates/common/fmt/src/dynamic.rs. If we wanted to make a change to the formatting that would be place to make the change. Personally I think the current formatting for tuples using |
Hey @jenpaff I saw you moved this PR to |
|
||
for i in data { | ||
let i = | ||
re.replace_all(&i, |caps: ®ex::Captures<'_>| format!(r#""{}""#, &caps[0])); |
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.
This is not the right fix, instead we should have a separate function in crates/common/fmt/src/dynamic.rs
that prints as JSON recursively instead of just the top layer, or perhaps even converting DynSolValue
to serde_json::Value
and print that instead.
I understand this is a more complex change, if you don't have capacity / will to do this we should open an issue for it and close this PR.
Closing in favor of #11212 Thanks for your PR |
Motivation
Below is a call to
cast cdd
to decode this particular transaction:https://etherscan.io/tx/0x59b8625421982e22ec159ad4f5c32c2352b9185e3e4d0f8771e25c77719e2391
Ideally, the first argument here, which is a tuple, would be JSON encoded as an array rather than a string:

My workaround for now has been use something like
jq -r '.[0]' | tr "()" "[]" | sed -e 's/\(0x[0-9a-f]*\)/"\1"/gi' | jq '.'
to parse the first argument.Solution
Changed ./crates/cast/main.rs
print_token(tokens)
to parse fields into JSON objects instead of stringsPR Checklist