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

chore: remove duplicate interface #560

Merged
merged 1 commit into from
May 16, 2024

Conversation

stevennevins
Copy link
Contributor

@stevennevins stevennevins commented May 16, 2024

Removes a duplicate interface.

I was running into:
Screenshot 2024-05-16 at 3 23 39 PM

with a newly created foundry repo that only had Counters.sol in the src directory. It seems to have been caused by the duplicate IERC721TokenReceiver interfaces

@stevennevins
Copy link
Contributor Author

@mds1 tagging for visibility

@mds1
Copy link
Collaborator

mds1 commented May 16, 2024

How do I reproduce the error? The below sequence only has Counter.sol and runs successfully, without the provided compiler error.

mkdir prj
cd prj
forge init
forge build

@stevennevins
Copy link
Contributor Author

stevennevins commented May 16, 2024

https://github.com/stevennevins/ci-workflows

npm install

forge build

npx wagmi generate

It's more that this causes an issue with the minimal foundry repo when creating a SDK with wagmi. This can be handled on the wagmi config side by excluding the MockERC721 in the options, but removing the duplicate interface seemed like a good option.

Copy link
Collaborator

@mds1 mds1 left a comment

Choose a reason for hiding this comment

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

Makes sense, thank you. I agree we don't need to duplicate the definition of the IERC721TokenReceiver interface in forge-std

@mds1 mds1 merged commit 52715a2 into foundry-rs:master May 16, 2024
3 checks passed
@stevennevins stevennevins deleted the patch-1-stevennevins branch May 29, 2024 01:28
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.

2 participants