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

Cheaper ERC-165 implementation #291

Open
fulldecent opened this issue Dec 13, 2021 · 2 comments
Open

Cheaper ERC-165 implementation #291

fulldecent opened this issue Dec 13, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@fulldecent
Copy link
Collaborator

The approach in this implementation is:

supportedInterfaces[0x80ac58cd] = true; // ERC721

But this approach has gotten much more expensive with recent gas cost changes.

We can improve this to a pure implementation to save gas.

@xpepermint xpepermint added the enhancement New feature or request label Dec 14, 2021
@MoMannn
Copy link
Collaborator

MoMannn commented Dec 20, 2021

@fulldecent
A downside with this is combining implementations.

So if someone does this:

contract NFTokenMetadataEnumerableMock is
  NFTokenEnumerable,
  NFTokenMetadata, 

Then he is also responsible to override supportInterface to assure both are supported. With the current implementation that is done automatically.

This is a drawback and we have to decide which takes priority. A bit lower gas fee when deploying the contract or simpler implementation / usage.

@MoMannn
Copy link
Collaborator

MoMannn commented Dec 20, 2021

The compiler does throw an error for combining implementations since it does not know which method to use. This does not solve the correct implementation tho. But either way I created an implementation so we can see it and decide afterwards if we want to use it or not.

#292

@xpepermint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants