Feat: parseJson #2293

odyslam · 2022-07-13T09:08:14Z

Motivation

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

fixes feat: JSON parsing cheatcodes #2153
related to feat: add parseJson helper functions forge-std#138

Checklist

mattsse

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

evm/src/executor/inspector/cheatcodes/ext.rs

mattsse · 2022-07-13T11:20:52Z

evm/src/executor/inspector/cheatcodes/ext.rs

+}
+fn parse_json(state: &mut Cheatcodes, json: &str, key: &str) -> Result<Bytes, Bytes> {
+    // let v: Value = serde_json::from_str(json).map_err(util::encode_error)?;
+    let values: Value = jsonpath_rust::JsonPathFinder::from_str(json, key)?.find();


what's the benefit if JsonPathFinder here?

For the crate you mean? So that I don't have to implement the jsonPath syntax

odyslam · 2022-07-13T15:13:00Z

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?

gakonst · 2022-07-13T19:55:52Z

testdata/cheats/ParseJson.t.sol

+    function test_uintArray() public {
+        bytes memory data = cheats.parseJson(json, ".uintArray");
+        uint[] memory decodedData = abi.decode(data, (uint[]));
+        assertEq(42, decodedData[0]);
+        assertEq(43, decodedData[1]);
+    }


This is nice, what if I want to query a JSON object multiple times? e.g. say I'd like to do bytes memory data = cheats.parseJson(json) and then data.getUintArray(".uintArray") followed by data.getString(".str")? I guess we'd want a parseJson variant which does not take a second argument and reads the entire thing, and then a JSON library (which we discussed before) which gives nice utils that handle the abi decoding (in forge-std probably)

parseJson variant which does not take a second argument and reads the entire thing

Reads it into what? We have the JSON in stringified via the readFile. So what we could do is basically a library that reads the JSON via readFile, stores the string locally, and then does parseJson(key) on the stored inner value.

Agreed on the library for forge-std.

mds1 · 2022-07-13T20:13:16Z

evm/src/executor/inspector/cheatcodes/ext.rs

 use crate::{
    abi::HEVMCalls,
-    executor::inspector::{cheatcodes::util, Cheatcodes},
+    executor::inspector::{


(arbitrary comment location to start a thread)

Have not take a close look at this yet, but just wanted to comment on this:

Address inference from a string is not great. Could it misinterpret a hex-encoded bytes string of the same length? Is this an assumption we are willing to make?

I think ethers.js has a good convention for distinguishing hex strings that are numbers from hex strings that are bytes. From their docs:

For example, the string "0x1234" is 6 characters long (and in this case 6 bytes long). This is not equivalent to the array [ 0x12, 0x34 ], which is 2 bytes long.

So we can use the same convention for json parsing / script output writing. I quickly sanity checked based on that portion of their docs, but would love someone to double check that / confirm it's a good approach here

mattsse · 2022-07-13T20:27:10Z

@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 · 2022-07-14T08:28:14Z

@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 · 2022-07-14T12:12:47Z

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 · 2022-07-26T08:24:37Z

@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

gakonst

there's a commented out test + needs a test for writeJSON, not sure i exactly get how it's supposed to be used!

the parseJSON flow looks good

evm/src/executor/inspector/cheatcodes/ext.rs

odyslam · 2022-07-31T09:46:23Z

@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.

mattsse

couple nits, otherwise ,lg

evm/src/executor/inspector/cheatcodes/ext.rs

OliverNChalk · 2022-08-01T07:53:45Z

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: proximacapital@e6867e1

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 · 2022-08-01T17:24:02Z

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

OliverNChalk · 2022-08-01T23:41:49Z

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

mattsse

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

needs rebase though

mds1 · 2022-08-10T20:29:42Z

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 · 2022-08-10T21:14:29Z

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 · 2022-08-11T00:09:47Z

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

mattsse · 2022-08-11T13:00:06Z

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 · 2022-08-11T13:06:44Z

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

odyslam · 2022-08-11T14:03:34Z

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

Thanks for the JSON file test fix.

0xPhaze · 2022-08-21T16:15:23Z

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

OliverNChalk · 2022-08-21T16:32:51Z

@0xPhaze I am able to parse this:

Into this:

0xPhaze · 2022-08-21T16:39:30Z

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 · 2022-08-21T17:05:30Z

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 · 2022-08-21T17:29:19Z

@0xPhaze that should do it

#2866

* refactor get_env * feat: test all possible types * chore: add jsonpath * feat: parse JSON paths to abi-encoded bytes * feat: flat nested json into a vec of <Value, abi_type> * fix: support nested json objects as tuples * chore: add test for nested object * feat: function overload to load entire json * fix: minor improvements * chore: add comments * chore: forge fmt * feat: writeJson(), without tests * fix: remove commented-out test * fix: improve error handling * fix: address Matt's comments * fix: bool * chore: remove writeJson code * fix: cherry pick shenanigan * chore: format, lint, remove old tests * fix: cargo clippy * fix: json file test Co-authored-by: Matthias Seitz <[email protected]>

mattsse requested changes Jul 13, 2022

View reviewed changes

gakonst reviewed Jul 13, 2022

View reviewed changes

mds1 reviewed Jul 13, 2022

View reviewed changes

onbjerg added the T-feature Type: feature label Jul 16, 2022

odyslam mentioned this pull request Jul 26, 2022

Change forge script artifact keys to support Json parsing #2473

Closed

odyslam mentioned this pull request Jul 27, 2022

chore: Make consistent forge script artifacts #2490

Merged

gakonst reviewed Jul 30, 2022

View reviewed changes

evm/src/executor/inspector/cheatcodes/ext.rs Outdated Show resolved Hide resolved

mattsse requested changes Jul 31, 2022

View reviewed changes

evm/src/executor/inspector/cheatcodes/ext.rs Outdated Show resolved Hide resolved

evm/src/executor/inspector/cheatcodes/ext.rs Outdated Show resolved Hide resolved

evm/src/executor/inspector/cheatcodes/ext.rs Outdated Show resolved Hide resolved

mattsse approved these changes Aug 10, 2022

View reviewed changes

odyslam marked this pull request as ready for review August 11, 2022 05:28

odyslam added 8 commits August 11, 2022 08:07

refactor get_env

536ff70

feat: test all possible types

3c38d0c

chore: add jsonpath

5bd287c

feat: parse JSON paths to abi-encoded bytes

1001eaf

feat: flat nested json into a vec of <Value, abi_type>

67a9687

fix: support nested json objects as tuples

bb94a01

chore: add test for nested object

58428b9

feat: function overload to load entire json

e18f40e

odyslam added 10 commits August 11, 2022 08:16

fix: minor improvements

6e3fcb8

chore: add comments

dfb551e

chore: forge fmt

f625745

feat: writeJson(), without tests

8035fe8

fix: remove commented-out test

b4e882d

fix: improve error handling

fa2a2b8

fix: address Matt's comments

aba4beb

fix: bool

3792d56

chore: remove writeJson code

fa15e22

fix: cherry pick shenanigan

2573fc7

odyslam force-pushed the feat/parseJson branch from b99283e to 2573fc7 Compare August 11, 2022 06:21

odyslam added 2 commits August 11, 2022 08:50

chore: format, lint, remove old tests

05e72a4

fix: cargo clippy

4389672

fix: json file test

7403c0b

mattsse merged commit 655a69f into foundry-rs:master Aug 11, 2022

onbjerg mentioned this pull request Aug 15, 2022

Document vm.parseJson foundry-rs/book#531

Closed

zhiqiangxu mentioned this pull request Sep 2, 2022

Is there a more generic way to load struct parameters from local file? foundry-rs/forge-std#169

Closed

Feat: parseJson #2293

Feat: parseJson #2293

Uh oh!

Conversation

odyslam commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Checklist

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattsse Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

odyslam Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

odyslam commented Jul 13, 2022

Uh oh!

gakonst Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

odyslam Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

mds1 Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattsse commented Jul 13, 2022

Uh oh!

odyslam commented Jul 14, 2022

Uh oh!

mattsse commented Jul 14, 2022

Uh oh!

odyslam commented Jul 26, 2022

Uh oh!

gakonst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

odyslam commented Jul 31, 2022

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OliverNChalk commented Aug 1, 2022

Uh oh!

gakonst commented Aug 1, 2022

Uh oh!

OliverNChalk commented Aug 1, 2022

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

mds1 commented Aug 10, 2022

Uh oh!

odyslam commented Aug 10, 2022

Uh oh!

gakonst commented Aug 11, 2022

Uh oh!

mattsse commented Aug 11, 2022

Uh oh!

odyslam commented Aug 11, 2022 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odyslam commented Aug 11, 2022

Uh oh!

0xPhaze commented Aug 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OliverNChalk commented Aug 21, 2022

Uh oh!

0xPhaze commented Aug 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

odyslam commented Jul 13, 2022 •

edited

Loading

mds1 Jul 13, 2022 •

edited

Loading

odyslam commented Aug 11, 2022 via email •

edited

Loading

0xPhaze commented Aug 21, 2022 •

edited

Loading

0xPhaze commented Aug 21, 2022 •

edited

Loading

odyslam commented Aug 21, 2022 •

edited

Loading