Feat: parseJson

[odyslam]

Motivation

I am opening a draft PR to gather feedback and improve code quality.

• fixes #2153
• related to https://github.com/foundry-rs/forge-std/pull/138

Checklist

• [x] Poc
• [x] Support arbitrarily nested JSON objects
• [x] Add Solidity tests
• [ ] Improve error handling & code quality
• [ ] Rebase master
• [ ] Merge forge-std library
• [ ] Merge docs

[odyslam]

I think that should work

If I understand correctly the ABI spec, structs are encoded as tuples.

exactly

for objects we should be able to convert them into Vec<ParamType> which will encode/decode as tuples, which would mean value_to_type is recursive over Value::Object

@mattsse What do you think of the fact that json objects are not ordered, thus it's possible that the same JSON will be serialized differently with every read of the file?

[mattsse]

@mattsse What do you think of the fact that json objects are not ordered, thus it's possible that the same JSON will be serialized differently with every read of the file?

iirc serde_json::Value::Object is a sorted BTreeMap.

I think we want sorted keys, and hence sorted encoded tuples so order is consistent?

[odyslam]

@mattsse What do you think of the fact that json objects are not ordered, thus it's possible that the same JSON will be serialized differently with every read of the file?

iirc serde_json::Value::Object is a sorted BTreeMap.

I think we want sorted keys, and hence sorted encoded tuples so order is consistent?

Yup, that sounds reasonable. Just underlining it cause that affects the order the user would have to define the attributes in the struct. While they would intuitively do it via some taxonomical or tight-packing order, they would need to know they they have to list the attributes in alphabetical order in order for the tuple to be translated into the struct correctly

[mattsse]

they would need to know they they have to list the attributes in alphabetical

I think this is reasonable, otherwise it will cause problems if you want to parse e.g. a "transaction" json and different nodes may return a different order in the RPC response

[odyslam]

@mattsse finally made it work with arbitrarily nested objects. Switched to handling them via Tokens and it became much simpler. I would like your suggestion on:

• Better error handling, especially inside the closure to reduce unwrap()
• parseAddress: I tried your suggestion, but didn't make it work. I think serde follows the same logic under the hood, trying different types in-order unless one decodes successfully.
• Solidity tests pass. To run them easily, you can use this one-liner: ./target/debug/forge test --root testdata --contracts cheats -vvv --match-contract ParseJson

[odyslam]

@gakonst I added a new comment on the issue on a proposed API for writeJson. I haven't found something that I love, so I want to get some alignment before I get full steam ahead implementing.

The one I have implemented right now, is only good for writing simple values (not arrays, not struct) at the first "level" of a json (so not being able to write in a nested object). I proposed a new API, but it's meh. Any idea is more than welcome.

[OliverNChalk]

Hey, just testing this out locally and haven't had any issues parsing JSON files yet. I merged master in and it was fine, just one compile issue fixed here: https://github.com/proximacapital/foundry/commit/e6867e11c89e21a1dc614d2cc68dd292ec2e3537

Was wondering if there was an eta? I'll continue building out my json usage locally but struggling to see a practical (i.e. not compiling from source) way to get access to this feature in CI before it merges?

[gakonst]

foundryup -P 2293 will get this working for you I think?

[OliverNChalk]

Build currently failing but ill def use that flag in the future, for now running off of a fork

[mds1]

a general usage is limited due to some assumptions and limitations regarding encoding, but it is definitely useful and we should add that

What limitations are these?

Also @odyslam should this be taken out of draft now?

[odyslam]

Tomorrow will do the following:

• remote writeJson code (will be replaced eitherway)
• rebase
• put out of draft and ping for review. Goal to merge tomorrow

Then, I will create a new branch to start working on @mattsse suggestion for writeJson.

Sorry for delaying the feature for so long.

[gakonst]

SGTM. Let's get this over the line and iterate on the writeJSON separately.

[mattsse]

a general usage is limited due to some assumptions and limitations regarding encoding, but it is definitely useful and we should add that

What limitations are these?

for structs, we currently have no way of knowing how the target struct looks like so current assumption is that the sol type matches the json object 1:1, right @odyslam?

[odyslam]

Not only that, but that also the attributes of the struct are defined in alphabetical order

[odyslam]

@mattsse will proceed with forge-std and docs update shortly.

Thanks for the JSON file test fix.

[0xPhaze]

Does this work with bytes32 and bytes? I am getting some weird results there. Edit: I guess these are always encoded as string.

[OliverNChalk]

@0xPhaze I am able to parse this:

Into this:

[0xPhaze]

Hm.. are you able to parse something like this?

I have to decode this into string and then parse the string to get the bytes32.

[odyslam]

Hey,

so currently the above will be returned as a type string. I have added a fix for that and will pr now, but they will be returned as type bytes.

[odyslam]

@0xPhaze that should do it

https://github.com/foundry-rs/foundry/pull/2866