foundry icon indicating copy to clipboard operation
foundry copied to clipboard

Add options for state changes output and json output in cast run command

Open cassc opened this issue 1 year ago • 3 comments

Motivation

The current human-readable output from cast run is great for manual interpretation but requires additional parsing to be used programmatically. Additionally, there is a need to inspect storage changes for each call, but currently, these changes are not recorded.

Solution

This PR introduces a -j option for cast run to output traces in JSON format, making it easier to consume programmatically. It also adds a --with-state-changes option that outputs storage changes.

Example usage:

cast run 0xefc789b63631b255aeb6f97d948c9eac14bae4b7f021122fe24c7c6e4f34667f \
  -r https://eth.llamarpc.com -q --decode-internal --with-state-changes -j | \
  jq -R 'select(try fromjson? | . != null) | fromjson'

cassc avatar Oct 03 '24 08:10 cassc

Hi everyone, please let me know if there's anything else I can do to help move this PR forward.

cassc avatar Oct 16 '24 04:10 cassc

For the CI error:

error: this function has too many arguments (9/7)
   --> crates/cli/src/utils/cmd.rs:357:1
    |
357 | / pub async fn handle_traces(
358 | |     mut result: TraceResult,
359 | |     config: &Config,
360 | |     chain: Option<Chain>,
...   |
366 | |     with_state_changes: bool,
367 | | ) -> Result<()> {
    | |_______________^

I believe we have a few options to address this:

  1. Move the options debug, decode_internal, verbose, json, and with_state_changes into the Config struct.
  2. Suppress the warning by adding #[allow(clippy::too_many_arguments)] to this function.
  3. Extract the above 5 options into a new struct.

Please let me know your thoughts or suggestions.

cassc avatar Oct 18 '24 03:10 cassc

  1. Suppress the warning by adding #[allow(clippy::too_many_arguments)] to this function.

this should be fine IMO

grandizzy avatar Oct 18 '24 06:10 grandizzy

Hi everyone, @zerosnacks @grandizzy any updates on this PR? It's a bit time consuming to keep merging updates on the main branch. Thank you!

cassc avatar Nov 15 '24 03:11 cassc

Any updates on this PR? It's a bit time consuming to keep merging updates on the main branch. Thank you!

Hey @cassc sorry for the delay, some small notes in regards to the global --json and global -v (verbosity) flag introduced recently. I don't think there are blockers beyond that.

zerosnacks avatar Nov 15 '24 08:11 zerosnacks

@cassc could you please check https://github.com/foundry-rs/foundry/issues/2846#issuecomment-1260103355 would like to know if we could tackle all points in the PR (or as a follow-up). Thank you!

grandizzy avatar Nov 21 '24 05:11 grandizzy

also wonder if we need the new --with-state-changes arg or we could just use verbosity for displaying such, @zerosnacks wdyt? thanks

grandizzy avatar Nov 21 '24 05:11 grandizzy

@cassc could you please check #2846 (comment) would like to know if we could tackle all points in the PR (or as a follow-up). Thank you!

Hi @grandizzy I think it's better to tackle in another PR, especially I think at the memoment I might not have enough knowledge about the test-level state changes.

cassc avatar Nov 25 '24 07:11 cassc

@cassc could you please check #2846 (comment) would like to know if we could tackle all points in the PR (or as a follow-up). Thank you!

Hi @grandizzy I think it's better to tackle in another PR, especially I think at the memoment I might not have enough knowledge about the test-level state changes.

make sense! @mds1 could you pls chime in re UX for cast run point in https://github.com/foundry-rs/foundry/issues/2846#issuecomment-1260103355 ? (that is adding the --with-state-changes arg to cast run and the way storage diffs are displayed). Then we can extend it (in a follow up PR) to forge test and add the cheatcode as suggested in 2nd point of your comment

grandizzy avatar Nov 25 '24 08:11 grandizzy

this should be good to go but probably worth one more reviewer since @zerosnacks and I were also adding code to it

grandizzy avatar Nov 27 '24 11:11 grandizzy