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 call depth to vm.stopAndReturnStateDiff() results #7234

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

RPate97
Copy link
Contributor

@RPate97 RPate97 commented Feb 24, 2024

Motivation

Closes #6632

Solution

The call depth was already recorded in the cheatcodes::inspector::AccountAccess wrapper. This PR removes that wrapper type in favor of only using cheatcodes::vm::AccountAccess, which now has a depth field.

Credit to ercembu in #6667 for the initial implementation. I decided to pick this up since it's important to us at Sphinx, and that PR appears to be abandoned. I recommend closing that once this is merged.

@RPate97 RPate97 force-pushed the pate/issue-6632 branch 4 times, most recently from 36d4c64 to a6a9e29 Compare February 25, 2024 00:57
Comment on lines -86 to -92
#[derive(Clone, Debug)]
pub struct AccountAccess {
/// The account access.
pub access: crate::Vm::AccountAccess,
/// The call depth the account was accessed.
pub depth: u64,
}
Copy link
Contributor Author

@RPate97 RPate97 Feb 25, 2024

Choose a reason for hiding this comment

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

I've removed this wrapper type. It's unnecessary since depth is now a field on the underlying AccountAccess type. I'm happy to reverse this change if there are plans to use this wrapper for some other purpose, but it seems redundant to me.

Copy link
Member

@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.

make sense,

this requires a forge-std change as well, right?

@DaniPopes
Copy link
Member

FYI I've edited the description to use GitHub keywords when referencing issues.

@RPate97
Copy link
Contributor Author

RPate97 commented Feb 26, 2024

this requires a forge-std change as well, right?

Yeah, I believe so. Is there a contributor guide I can reference for that?

@DaniPopes
Copy link
Member

There is no need, this will get included in the next release automatically.

@DaniPopes DaniPopes merged commit 7b2315c into foundry-rs:master Feb 27, 2024
19 checks passed
zeroXbrock pushed a commit to flashbots/suavex-foundry that referenced this pull request Apr 2, 2024
…y-rs#7234)

* Added depth member to AccountAccess

* Combine AccountAccess structs

---------

Co-authored-by: ercembu <ercembulut@gmail.com>
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.

Include call depth in vm.stopAndReturnStateDiff() results
4 participants