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: add decoding for individual trace steps #157

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

klkvr
Copy link
Contributor

@klkvr klkvr commented Jul 1, 2024

ref foundry-rs/foundry#8222
ref foundry-rs/foundry#8198

Adds structs and extends TraceWriter to support formatting of decoded trace steps. Currently two decoding formats are supported:

  • Internal calls. Similar to a decoded call trace, decoded internal function invocation which spans over multiple steps. Kept as decoded function name, inputs, outputs and index of the last step.
  • Arbitrary strings. This might be useful for formatting decoded opcodes (e.g. adding ├─ [sload] <slot> to trace. It might make sense to extend it to something more configurable once we start implementing this

@klkvr klkvr requested review from DaniPopes and mattsse as code owners July 1, 2024 16:21
/// Represents a decoded internal function call.
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct DecodedInternalCall {
Copy link
Member

Choose a reason for hiding this comment

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

maybe worth also adding an optional style field?

Copy link
Contributor Author

@klkvr klkvr Jul 1, 2024

Choose a reason for hiding this comment

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

we don't separate contract name and function name for internal calls, so I think func_name can be styled directly?

I think this could be useful, though not sure how to deal with serde for it

Copy link
Member

Choose a reason for hiding this comment

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

Can be a follow-up where we add it to other structs as well

Copy link
Contributor

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,

pending @DaniPopes

@mattsse mattsse merged commit 5679165 into paradigmxyz:main Jul 2, 2024
11 checks passed
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.

3 participants