feat: add parseJson helper functions

[odyslam]

Helper functions that leverage the new parseJson cheatcode and enable the user to easily parse forge script artifacts.

The Structs will probably change a bit, due to the artifacts requiring some changes to avoid Solidity keyword keys and make the artifacts be consistent (produce keys with null values instead of omitting them)

[mds1]

Now that vm.parseJson is merged (thanks a lot @odyslam!) let's revisit this PR and get the required Vm interface updates, types, and any helpers merged

[odyslam]

1. Blocked on https://github.com/foundry-rs/foundry/pull/2914
2. Currently, for some reason, a particular Receipt on the broadcast fixture is not decoded properly for unknown reasons.
3. readReceipt needs some refactoring, as it will result to stack too deep error unless we use --via-ir flag

[odyslam]

All issues fixed and --via-ir is not required any more

[odyslam]

The ci fail is propably because they fix PR that was just merged has not be yet propagated

[mds1]

If I rebase locally against main all tests pass, seems this branch is missing the projectRoot cheatcode definition from https://github.com/foundry-rs/forge-std/pull/158, so let's try that and see if it fixes CI? Though the CI failure does seem different than missing projectRoot

[odyslam]

I addressed all your comments, except for the library. The parser needs the state to have a nicer UX. It also needs console2, vm, so the imports would get a bit funky.

What I can't reason about is the projectRoot thing and why locally it seems to be OK but the file in GH is not.

[mds1]

I'd say three things left before merging this, all easy things at this point

• [x] Refactor into a library using this approach: https://github.com/foundry-rs/forge-std/pull/138#discussion_r956749279
• [x] Make sure we track potential JSON output renames in the foundry repo (cc @mattsse @joshieDo for any thoughts). Now tracking in https://github.com/foundry-rs/foundry/issues/2987
• [x] Potential outdated comment? https://github.com/foundry-rs/forge-std/pull/138#discussion_r956340962
• [x] ~~Suggestion: remove the // json value name = tx comment and rename the field from txDetail to transaction~~: https://github.com/foundry-rs/forge-std/pull/138#discussion_r956751476
• [x] Rename functionName to functionSig: https://github.com/foundry-rs/forge-std/pull/138#discussion_r956751766

[odyslam]

@mds1 I like txDetail, cause we call the outer struct Transaction. It feels very intuitive.

[odyslam]

@mds1 what's up with projectRoot? Locally, I have only one definition in the interface.

[mds1]

Yea I don't know why the CI is failing, I'm gonna take a closer look later today so don't worry about it

[odyslam]

Thank you guys for helping me push this through the finish line. Your suggestions were really good.

[odyslam]

wrt the tests not passing. Shouldn't solc 8.0 support struct decoding?

[ZeroEkkusu]

Those are tests for backward compatibility with solc 0.8.0, 0.7.0, 0.6.0.

One option is to decode the structs manually. But we should discuss this before implementing any workarounds.

[mds1]

I'd be ok with only supporting 0.8.x for now (and updating the StdJson.sol pragmas + CI as needed) and opening a separate issue to track supporting the stdJson lib for older versions, to avoid blocking this PR. Since this is a new feature no one is currently relying on it so only supporting 0.8.x won't break any backwards compatibility, and they'll still be able to use parseJson, just not the helper lib.

[ZeroEkkusu]

We can use "predeploys". Here's proof-of-concept:

We write a contract with the problematic functionality in the latest Solidity. The functions should be external pure. We note down its runtime bytecode.

Then, in Forge Std, we "predeploy" this contract with vm.etch. Making a static call to a desired function will decode the data into a struct.

E.g. usage Decoder.decodeReceiptArray(bytes):

function readReceipts(string memory path)
         internal
         returns (Receipt[] memory)
     {
         string memory deployData = vm.readFile(path);
         bytes memory parsedDeployData = vm.parseJson(deployData, ".receipts");
         RawReceipt[] memory rawReceipts = decoder.decodeReceiptArray(parsedDeployData);
         return rawToConvertedReceipts(rawReceipts);
     }

Proof:

Now we have struct ABI-decoder in Solidity 0.6.0.

[mds1]

Ah nice, that is a great idea! Some thoughts:

1. I think it still will require some changes to CI anyway? (since we'll have a contract in this repo that only compiles with 0.8)
2. I'm worried this might get a bit messy, so we probably want a simple script to automate the process of compiling + injecting the bytecode into the contracts + ensuring the 0.8-specific code is isolated.
3. I'm imagining the lib now calls out to the etched predeploy, so UX-wise it's still the same as a native library?
4. One downside is that because these are external calls they'll now show up in traces, but I don't think there's anything we can do about that if we go this route.

Also, regardless of which approach we use, will the existence of a contract with a 0.8 pragma break builds for users with an 0.6/0.7 pragma, or will that file just get skipped during compilation? I'm not sure offhand.

I do like this idea, but FWIW I'd still be ok only supporting 0.8 to start, then making this change afterwards if requested, since it does add some tech debt/maintenance burden/overhead that we'll need to maintain.

@gakonst lmk if you have any thoughts here, not sure if you have a better grasp of how many 0.6/0.7 users there are + if they'd need this feature

[ZeroEkkusu]

If only solc >= 0.8.0 is going to be supported, we must remove the import in Script and let users import / inherit the std-cheats pack manually. There would be no compilation problems on their end, and CI here would continue to work.

[gakonst]

Could the tests not be 0.8.0 and the contract itself be >=0.6.0 or something?

[mds1]

@gakonst I don't think that helps since the decoding is done in the main Test.sol contract, not in the tests, e.g. check the failed CI here: https://github.com/foundry-rs/forge-std/runs/8059709854?check_suite_focus=true#step:6:38

[odyslam]

@mds1 -- I think that @gakonst meant that we can have Test.sol et al. at 8.0 (and the tests that inherit it) without caring that the user contract (the one the user tests) is in 6.0

[mds1]

Got it, so IIUC correctly the suggestion here is that:

• Users might be writing contracts with solidity 0.6 or 0.7
• But, they can still write their tests in 0.8 and therefore forge-std only needs to support 0.8?

One limitation there is I don't think the toml file currently is flexible enough to support various solc configs? So you're stuck with the defaults for the test version, which maybe is ok. (The config file definitely should support solc profiles per-contract/file though, not sure if that's being tracked somewhere)

[odyslam]

I think that forge will automatically use the appropriate solc version? W

[ZeroEkkusu]

To summarize the discussion:

• The new std-cheats use struct decoding, which is available in Solidity >=0.8.0
• Forge Std supports Solidity versions as low as 0.6.0
• A common practice is to import a contract you want to test into a .t.sol file with a test contract
• If the contract is using a Solidity version below 0.8.0, the project will not compile

pragma solidity >=0.6.0 <0.8.0;

contract Contract {}

// PICK ANY PRAGMA

import "src/Contract.sol";

import "forge-std/Test.sol";

contract ContractTest is Test {
    Contract c;

    function setUp() public {
        c = new Contract();
    }
}

Picking a pragma 0.8.0 or above will not work because the user contract is limited to <0.8.0. Picking a pragma below 0.8.0 will not work because the new std-cheats use struct decoding.

[mds1]

I searched sourcegraph and found 112 repos that specify a solc version in foundry.toml. Of those 110 are 0.8.x and 2 are 0.7.x (nomad and beanstalk).

I know Maker uses <0.8 in a few repos but they are migrating away from that, and I've gotten confirmation they'd be ok with new forge-std features being >= 0.8 only.

@odyslam Do you know if nomad would mind if this parseJson feature is only >= 0.8?

If not, I think the simplest path forward is to make the library >= 0.8 but not import it by default to avoid breaking existing builds. Then, we can import by default whenever (1) we drop support for < 0.8, or (2) we modify the lib to decode manually / use the predeploy trick to add support for < 0.8

[odyslam]

@mds1 would it help if we defined pragma experimental ABIEncoderV2;?

EDIT: with the experimental flag, the whole thing compiles for 0.7.0 but not for 0.7.6 (0.7.6 seems to have issues with decoding arrays of strings/bytes)

That being said, since there is no other way around it, I think it's fair to a) move the helper functions from the Test contract to the library b) Make the library compatible with >=0.8

Personally, the pre-deploy seems too complicated and not worth the effort for backward compatibility.

If we are aligned, I will do (a) and let's merge this gigantic PR

[ZeroEkkusu]

Sgtm; let's make it an abstract contract so we can merge it with StdCheats later without breaking changes:

import "forge-std/StdCheatsJson.sol";

contract ContractTest is Test, StdCheatsJson {}

If we did a library, it would require this syntax (can't merge seamlessly in the future):

lib.stdCheat();

We should address https://github.com/foundry-rs/forge-std/pull/138#discussion_r956757666 as well.

[odyslam]

@ZeroEkkusu -- so changing it back to an abstract contract

• For some reason, it compiles with 0.6.0 and 0.7.0, but not with 0.7.6 🙃
• How can we use string.concat with 0.6 and 0.7 ?

[odyslam]

@ZeroEkkusu @mds1 can I get an ack on alignment for the design before going into implementation

[mds1]

Sorry, catching up now. What's the proposed design here? Is it a >=0.8 lib, or an abstract contract?

Using pragma experimental ABIEncoderV2; was a good idea and I support that if it brings backwards compatibility

[odyslam]

@mds1 abstract contract so it can be compatible (with what I assume) a future refactoring of how cheats works in the std lib

(Quite funnily, it still won't compile with 0.7.6 due to returning arrays of bytes/strings). Go figure.