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

Consider removing ERC165Storage #3641

Closed
frangio opened this issue Aug 19, 2022 · 4 comments
Closed

Consider removing ERC165Storage #3641

frangio opened this issue Aug 19, 2022 · 4 comments
Labels
breaking change Changes that break backwards compatibility of the public API.

Comments

@frangio
Copy link
Contributor

frangio commented Aug 19, 2022

Does it ever make sense to use this contract over ERC165 with overrides and if statements?

The API is a little simpler because it's just a call to _registerInterface. The use of storage makes it more expensive to execute supportsInterface, and also to deploy the contract. Deployment costs are not necessarily worse though, because when storage is not used it requires more bytecode, but probably not enough to match it.

@frangio frangio added the breaking change Changes that break backwards compatibility of the public API. label Aug 19, 2022
@Amxx Amxx added this to the 5.0 milestone Aug 22, 2022
@frangio
Copy link
Contributor Author

frangio commented Nov 1, 2022

I'm curious how many occurences we can find of ERC165Storage in the Smart Contract Sanctuary. Should give us an idea if people are using this.

@Amxx
Copy link
Collaborator

Amxx commented Nov 2, 2022

Its difficult to count them, as ERC165Storage is mentionned in ERC165 ...

looking "ERC165Storage," returns a lot of result (+500)

@JulissaDantes
Copy link
Contributor

Looking at the total amount of contracts using the ERC165 contract against the amount using the ERC165Storage inside the sanctuary with the following search criteria and results:

Search criteria Contract Results Count
repo:tintinweb/smart-contract-sanctuary file:/mainnet/ content:"contract ERC165" count:all ERC165 121.4k
repo:tintinweb/smart-contract-sanctuary file:/mainnet/ content:"contract ERC165Storage" count:all ERC165Storage 944

We concluded that it can be removed since only ~0.8% of contracts using ERC165 used storage to register the interfaces.

@JulissaDantes
Copy link
Contributor

Implemented by #3880 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes that break backwards compatibility of the public API.
Projects
None yet
Development

No branches or pull requests

3 participants