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

fix(trace): properly report fallback() fn #9287

Merged
merged 5 commits into from
Nov 10, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Nov 8, 2024

Motivation

closes #9115

Solution

  • when decoding functions for contracts with fallback make sure the decoded function is in contract sigs, otherwise use fallback for trace

@grandizzy grandizzy marked this pull request as ready for review November 8, 2024 12:53
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! Small note in regards to test case completeness w/ JSON

@sakulstra would you mind double checking the proposed output format?

crates/forge/tests/cli/cmd.rs Outdated Show resolved Hide resolved
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, pending final check from @sakulstra

@sakulstra
Copy link
Contributor

looks fine to me, thanks a lot for looking into this!

@grandizzy grandizzy enabled auto-merge (squash) November 10, 2024 10:35
@grandizzy grandizzy enabled auto-merge (squash) November 10, 2024 11:07
@grandizzy grandizzy changed the title fix(trace): check fn sigs for contract with fallbacks fix(trace): properly report fallback() fn Nov 10, 2024
@grandizzy grandizzy merged commit e028b92 into foundry-rs:master Nov 10, 2024
21 checks passed
@grandizzy grandizzy deleted the issue-9115 branch November 10, 2024 11:19
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.

bug: --gas-report groups all methods of all contracts that use the same type of proxy contract
4 participants