Skip to content

Conversation

@odyslam
Copy link
Contributor

@odyslam odyslam commented Jul 26, 2022

Helper functions that leverage the new parseJson cheat code 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)

@odyslam odyslam mentioned this pull request Jul 26, 2022
7 tasks
@mds1
Copy link
Collaborator

mds1 commented Aug 12, 2022

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
Copy link
Contributor Author

odyslam commented Aug 25, 2022

  1. Blocked on fix: address false positive foundry#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 odyslam marked this pull request as ready for review August 25, 2022 14:26
@odyslam
Copy link
Contributor Author

odyslam commented Aug 25, 2022

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

@odyslam
Copy link
Contributor Author

odyslam commented Aug 26, 2022

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

@mds1
Copy link
Collaborator

mds1 commented Aug 26, 2022

If I rebase locally against main all tests pass, seems this branch is missing the projectRoot cheatcode definition from #158, so let's try that and see if it fixes CI? Though the CI failure does seem different than missing projectRoot

Copy link
Collaborator

@mds1 mds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking awesome, thanks a lot for all the work!

@odyslam
Copy link
Contributor Author

odyslam commented Aug 28, 2022

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
Copy link
Collaborator

mds1 commented Aug 30, 2022

Let me clarify, StdJson.sol should contain the stdJson library as before

Makes sense.

and StdCheatsJson.sol should contain the StdCheatsJson abstract contract so we can merge it with StdCheats in the future without breaking changes.

But what's this? Duplicate functionality as an abstract contract instead of a library?

@ZeroEkkusu
Copy link
Collaborator

ZeroEkkusu commented Aug 30, 2022

But what's this? Duplicate functionality as an abstract contract instead of a library?

No, we are going to move the problematic std-cheats there and remove the current content.

@ZeroEkkusu
Copy link
Collaborator

ZeroEkkusu commented Aug 30, 2022

Essentially, we can't get Test to compile w/ older solc because the new std-cheats use struct decoding. We are going to move them to a separate component, StdCheatsJson and let users import it manually if they want to use it. Does that make sense? @mds1 @odyslam

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

@ZeroEkkusu to be very clear. We can compile with the experimental pragma. The issue we seem to be hitting on some solc is decoding string[] and bytes[]

@mds1
Copy link
Collaborator

mds1 commented Aug 30, 2022

Ok, I think I follow:

// Library
function parseRaw(string memory json, string memory key)
function readUint(string memory json, string memory key)
function readUintArray(string memory json, string memory key)
function readInt(string memory json, string memory key)
function readIntArray(string memory json, string memory key)
function readBytes32(string memory json, string memory key)
function readBytes32Array(string memory json, string memory key)
function readString(string memory json, string memory key)
function readStringArray(string memory json, string memory key)
function readAddress(string memory json, string memory key)
function readAddressArray(string memory json, string memory key)
function readBool(string memory json, string memory key)
function readBoolArray(string memory json, string memory key)
function readBytes(string memory json, string memory key)
function readBytesArray(string memory json, string memory key)

// StdCheatsJson

// --- snip: all the custom structs ---
function readEIP1559ScriptArtifact(string memory path)
function rawToConvertedEIPTx1559s(RawTx1559[] memory rawTxs)
function rawToConvertedEIPTx1559(RawTx1559 memory rawTx)
function rawToConvertedEIP1559Detail(RawTx1559Detail memory rawDetail)
function readTx1559s(string memory path)
function readTx1559(string memory path, uint256 index)
function readReceipts(string memory path)
function readReceipt(string memory path, uint index)
function rawToConvertedReceipts(RawReceipt[] memory rawReceipts)
function rawToConvertedReceipt(RawReceipt memory rawReceipt)
function rawToConvertedReceiptLogs(RawReceiptLog[] memory rawLogs)
function bytesToUint(bytes memory b) internal pure returns (uint256){

and let users import it manually if they want to use it. Does that make sense

This is the part I was missing, I didn't realize we still had this requirement

@mds1
Copy link
Collaborator

mds1 commented Aug 30, 2022

I think with pragma experimental ABIEncoderV2; we can have this functionality and import it by default?

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

@mds1 even with the flag, we seem not being able to compile for 0.7.6 due to the types I mentioned above. Didn't go into why.

@ZeroEkkusu
Copy link
Collaborator

Lemme check.

@mds1
Copy link
Collaborator

mds1 commented Aug 30, 2022

Ahh I thought it was a test/runtime error, not a compilation error

@ZeroEkkusu
Copy link
Collaborator

So, it works with ABIEncoderV2. I just tried decoding string[] inside a struct with 0.7.6 and it works as well. @odyslam

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

My solc failed to compile. What the actual F

@ZeroEkkusu
Copy link
Collaborator

ZeroEkkusu commented Aug 30, 2022

I was experimenting manually. Going to check out the PR now and compile it myself.

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

Try to build from 7daae38

@ZeroEkkusu
Copy link
Collaborator

Ok, I see with 0.7.6. Looking into it.

@ZeroEkkusu
Copy link
Collaborator

@odyslam It's because it missing pragma experimental ABIEncoderV2;.

@ZeroEkkusu
Copy link
Collaborator

So, let's:

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

@ZeroEkkusu I can't merge

@odyslam odyslam requested a review from ZeroEkkusu August 30, 2022 20:40
@mds1
Copy link
Collaborator

mds1 commented Aug 30, 2022

Love to see it
image

Thanks so much @odyslam for getting this done, thanks @ZeroEkkusu for your help with review. @ZeroEkkusu this LGTM, feel free to squash merge if you wanted to do a final check

@odyslam
Copy link
Contributor Author

odyslam commented Aug 30, 2022

Next steps:

  • Address Artifact parsing
  • Address LegacyTxs (transaction artifacts produced by non 1559 networks)

Co-authored-by: Zero Ekkusu <[email protected]>
@ZeroEkkusu
Copy link
Collaborator

The reason CI didn't fail was because it is configured to use 0.7.0. Now, everything's compiling.

Great work, team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants