foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.34k stars 1.77k forks source link

Add pure JSON serialization cheatcodes #5764

Open vdrg opened 1 year ago

vdrg commented 1 year ago

Component

Forge

Describe the feature you would like

Hi everyone!

The current approach to handling JSON feels somewhat suboptimal. Working with the id -> json map feels like an extra step, which, in my opinion, isn't entirely necessary. If the JSON serialization methods were made "pure", they could operate directly on a JSON string, which has the advantages of not modifying state and being more intuitive.

Consider the current method:

string memory id = "myObj";
vm.serializeString(id, ".foo", "bar");
vm.serializeString(id, ".hello", "world");
string memory json = vm.serializeUint(id, ".someUint", 123);

With a direct JSON string approach, it could be simplified to:

string memory json = '{ "foo": "bar" }';
json = vm.serializeString(json, ".hello", "world");
json = vm.serializeUint(json, ".someUint", 123);

This method offers more clarity in the way we work with JSON. Also, by the convention used by forge-std, in this case the functions could be declared as pure. Of course this would break a lot of things... an alternative would be to add these as new methods (not ideal as the vm interface is already pretty big).

Thoughts?

Additional context

No response

mds1 commented 1 year ago

This does make sense to me and feels simpler.

@odyslam any insight into why we went with the current approach? I feel like I recall expecting it to work in the way suggested here, and can't remember the rationale for the current approach.

Given scope of the change, if we do implement this, my suggestion would be to namespace the cheats behind a _v2 or something and if everyone agrees this is better then at some point there can be a breaking change to remove the old version

zerosnacks commented 4 months ago

Reopening, my apologies - misread the proposed suggestion

Adding to larger meta-tracking ticket for JSON / TOML parsing: https://github.com/foundry-rs/foundry/issues/3801