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

VIP: ERC165 support #2619

Open
Tracked by #2620
fubuloubu opened this issue Jan 23, 2022 · 10 comments
Open
Tracked by #2620

VIP: ERC165 support #2619

fubuloubu opened this issue Jan 23, 2022 · 10 comments
Labels
VIP: Approved VIP Approved

Comments

@fubuloubu
Copy link
Member

Simple Summary

ERC165 allows contracts to quickly determine if they are able to interact with a particular contract. Deeper support into Vyper will help improve contracts that would like to implement ERC165

Motivation

ERC721 introduced the concept of mandatory ERC165 interface checks in order to acheive compliance with a given ERC. This has been problematic for Vyper in the past because of the missing bytes4 type (which is also something we should add and support).

Until support is added for bytes4, it will not be possible to comply with ERC165 (and thus any ERCs which require ERC165 for full compliance). However, it seems like a potential solution to overload the implements: keyword, which currently like Vyper contracts specify what interfaces they comply to, with functionality that adds ERC165 compliance more implicitly to a contract

Specification

When the implements: keyword is used in a Vyper contract with an Interface type, Vyper will add the def supportsInterface(interface_id: bytes4) -> bool function directly to the contract's bytecode, which returns whether or not the contract supports a given interface_id.

ERC165 specifies interface_id as the logical XOR of all the mandatory method_ids located in the defined interface. Computing these manually requires adding lines to the constructor function in order to enable it correctly, or otherwise making large view function for supportsInterface with lots of manual typing.

A discussion point is whether these interface IDs only be limited to defined ERCs in Vyper's interfaces library, which would reduce clutter from using non-standardized interfaces with the implements: keyword

Lasty, any contracts which use this feature will also implicitly use the implements: ERC165 specifier. A discussion could be had that this support would only enabled by adding this specification to a contract (e.g. a contract will not produce the implementsInterface function unless implements: ERC165 is added), which would make this behavior more "opt in"

Backwards Compatibility

Any contracts that use the implements: keyword currently would raise an error if the supportsInterface function was also defined.

Dependencies

Not required for implementation, but the bytes4 type would make implementation of this VIP likely easier.

References

Copyright

Copyright and related rights waived via CC0

@fubuloubu fubuloubu added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Jan 23, 2022
@tserg
Copy link
Collaborator

tserg commented Jan 24, 2022

I think injecting the def supportsInterface(interface_id: bytes4) -> bool function based on the implements: keyword can be confusing given Vyper's goal of simplicity. I have in mind someone starting out in Vyper implementing a simple ERC20, deploying it to a testnet and then trying to verify the Vyper code on Etherscan but it fails because the compiled code includes this function but the local copy of the written contract does not. Furthermore, it also seems like a creeping in of class inheritance (of the ERC165 interface).

IMO, I think it is better to require the def supportsInterface(interface_id: bytes4) -> bool to be expressly included in the contract, but have a built-in attribute for interfaces that provides the interface ID (e.g. Solidity has an interfaceId for interfaces).

@fubuloubu
Copy link
Member Author

I think injecting the def supportsInterface(interface_id: bytes4) -> bool function based on the implements: keyword can be confusing given Vyper's goal of simplicity. I have in mind someone starting out in Vyper implementing a simple ERC20, deploying it to a testnet and then trying to verify the Vyper code on Etherscan but it fails because the compiled code includes this function but the local copy of the written contract does not. Furthermore, it also seems like a creeping in of class inheritance (of the ERC165 interface).

IMO, I think it is better to require the def supportsInterface(interface_id: bytes4) -> bool to be expressly included in the contract, but have a built-in attribute for interfaces that provides the interface ID (e.g. Solidity has an interfaceId for interfaces).

Yes, got a lot of feedback from @charles-cooper about this too. I think adding the MyInterface.interface_id property to an interface object would be a better way to go. Then you can just do something like this:

@view
@external
def supportsInterface(interface_id: bytes4) -> bool:
    return interface_id in [
        ERC20.interface_id,
        ERC721.interface_id,
        ...  # as many as you want to support
    ]

This has the most optimal balance of flexibility and convenience

@fubuloubu
Copy link
Member Author

Wonder if this should also add .method_id to method types e.g.:

@external
def marked(amount: uint256):
    ...

# self.marked.method_id == 0x92e37a93

@charles-cooper
Copy link
Member

Wonder if this should also add .method_id to method types e.g.:

@external
def marked(amount: uint256):
    ...

# self.marked.method_id == 0x92e37a93

#2098

@charles-cooper
Copy link
Member

meeting notes: implements: is too implicit, we will implement interface_id member (or maybe call it erc165_id)

@charles-cooper charles-cooper added VIP: Approved VIP Approved and removed VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting labels Feb 7, 2022
@liam-ot

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@liam-ot

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@liam-ot

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Approved VIP Approved
Projects
None yet
Development

No branches or pull requests

4 participants