Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cheatcodes): add vm.getStateDiff to get state diffs as string #9435

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Nov 29, 2024

Motivation

Closes #2846 by adding

    function getStateDiff() external view returns (string memory diff);
    function getStateDiffJson() external view returns (string memory diff);
  • returns diffs recorded by startStateDiffRecording() cheatcode as string (can be printed with console.log or written in file), at the moment of cheatcode call (can be called multiple times between start / stop state diff)
  • diffs contains only storage changes, initial value when start diff called to current (intermediary storage changes are not displayed)

E.g.

    vm.startStateDiffRecording();
    // do work, change state of Counter contract
    console.log(":::: diff 1 ::::");
    console.log(vm.getStateDiff());
    // more work, change state of Counter and AnotherCounter contracts
    console.log(":::: diff 2 ::::");
    console.log(vm.getStateDiff());
    // more work, change state of AnotherCounter contract
    console.log(":::: diff 3 ::::");
    console.log(vm.getStateDiff());

image

Json format vm.getStateDiffJson()

{
   "0x2e234dae75c793f67a35089c9d99245e1c58470b":{
      "label":"NestedStorer",
      "changes":{
         "31391530734884398925509096751136955997235046655136458338700630915422204365175":{
            "previousValue":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "newValue":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "86546418208203448386783321347074308435724792809315873744194221534962779865098":{
            "previousValue":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "newValue":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "89735575844917174604881245405098157398514761457822262993733937076486162048205":{
            "previousValue":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "newValue":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "99655811014363889343382125167956395016210879868288374279890486979400290732814":{
            "previousValue":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "newValue":"0x0000000000000000000000000000000000000000000000000000000000000001"
         }
      }
   }
}

Solution

@zerosnacks zerosnacks self-requested a review November 29, 2024 17:39
zerosnacks
zerosnacks previously approved these changes Nov 29, 2024
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

lgtm

@sakulstra
Copy link
Contributor

I'm honestly a bit conflicted with this api / feature.

With primitive type storage in incremental slots what is proposed here is definitely useful, although I think for us would be more useful in an easier-to-consume & post-process format (e.g. json).

Currently I lack the imagination though to see how i could understand that a change in slot 0x9fe9e472df24c0973d55f45ce8b645fa4fb021df9b3c6db994a661dfb284cb67is a _balances change of some user.
What I originally hoped for when opening #2846 was something closer to evm.storage or tenderly state diff - a state diff including some decoded storage slot description.

@grandizzy
Copy link
Collaborator Author

I'm honestly a bit conflicted with this api / feature.

With primitive type storage in incremental slots what is proposed here is definitely useful, although I think for us would be more useful in an easier-to-consume & post-process format (e.g. json).

can add a flag to cheatcode to return plain text or json formatted

Currently I lack the imagination though to see how i could understand that a change in slot 0x9fe9e472df24c0973d55f45ce8b645fa4fb021df9b3c6db994a661dfb284cb67is a _balances change of some user. What I originally hoped for when opening #2846 was something closer to evm.storage or tenderly state diff - a state diff including some decoded storage slot description.

I think this would imply leveraging of storage layout but probably more complex, will have a look at but most probably feasible to add post v1.

@grandizzy grandizzy changed the title feat(cheatcodes): add vm.getStateDiff() to get state diffs as string feat(cheatcodes): add vm.getStateDiff(isJson) to get state diffs as string Dec 2, 2024
@sakulstra
Copy link
Contributor

sakulstra commented Dec 2, 2024

@grandizzy just had another look at the structure & was wondering it could make sense to change it so sth like:
{<address>: {label: <label>, changes: {<slot>: {from, to}}}} or sth similar.

My thinking is that it's a bit weird that if a label is supplied, the address is lost on the diff.
By providing changes as an object instead of an array, I feel like this might open up the possibility of adding things in the future (like the decoded storage location, or some other meta), without introducing a breaking change on the structure.

@grandizzy
Copy link
Collaborator Author

@grandizzy just had another look at the structure & was wondering it could make sense to change it so sth like: {<address>: {label: <label>, changes: {<slot>: {from, to}}}} or sth similar.

My thinking is that it's a bit weird that if a label is supplied, the address is lost on the diff. By providing changes as an object instead of an array, I feel like this might open up the possibility of adding things in the future (like the decoded storage location, or some other meta), without introducing a breaking change on the structure.

thank you, makes sense, will structure the output this way

@grandizzy
Copy link
Collaborator Author

grandizzy commented Dec 2, 2024

@grandizzy just had another look at the structure & was wondering it could make sense to change it so sth like: {<address>: {label: <label>, changes: {<slot>: {from, to}}}} or sth similar.
My thinking is that it's a bit weird that if a label is supplied, the address is lost on the diff. By providing changes as an object instead of an array, I feel like this might open up the possibility of adding things in the future (like the decoded storage location, or some other meta), without introducing a breaking change on the structure.

thank you, makes sense, will structure the output this way

@sakulstra I formatted it as below in d46080c

{
   "0x2e234dae75c793f67a35089c9d99245e1c58470b":{
      "label":"NestedStorer",
      "changes":{
         "31391530734884398925509096751136955997235046655136458338700630915422204365175":{
            "original":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "dirty":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "86546418208203448386783321347074308435724792809315873744194221534962779865098":{
            "original":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "dirty":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "89735575844917174604881245405098157398514761457822262993733937076486162048205":{
            "original":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "dirty":"0x0000000000000000000000000000000000000000000000000000000000000001"
         },
         "99655811014363889343382125167956395016210879868288374279890486979400290732814":{
            "original":"0x0000000000000000000000000000000000000000000000000000000000000000",
            "dirty":"0x0000000000000000000000000000000000000000000000000000000000000001"
         }
      }
   }
}

@sakulstra
Copy link
Contributor

sakulstra commented Dec 2, 2024

@grandizzy looks good to me.

I personally find the new naming not so optimal, but i think is just taste.
Checked the naming on other tooling and e.g. tenderly is using original & dirty instead of previousValue and newValue - in the end doesn't matter though.

@grandizzy
Copy link
Collaborator Author

@grandizzy looks good to me.

I personally find the new working not so optimal, but i think is just taste. Checked the naming on other tooling and e.g. tenderly is using original & dirty instead of previousValue and newValue - in the end doesn't matter though.

changed in 1536119 and updated comment above

@grandizzy grandizzy dismissed zerosnacks’s stale review December 2, 2024 19:10

rerequesting review as there were several changes (json output)

testdata/cheats/Vm.sol Outdated Show resolved Hide resolved
@grandizzy grandizzy changed the title feat(cheatcodes): add vm.getStateDiff(isJson) to get state diffs as string feat(cheatcodes): add vm.getStateDiff to get state diffs as string Dec 3, 2024
@zerosnacks zerosnacks self-requested a review December 3, 2024 11:25
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Nice! Lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat(cheatcodes): ability to capture and store state diffs
3 participants