-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Support ERC-165 identifiers #7856
Comments
I'm a bit reluctant to add explicit ERC numbers to the Solidity language, but I think this is a good feature to have. We should consider adding it to |
A lot of discussion took place in #1447. |
This could be non-breaking. |
Discussion: it should be a member of Brainstorming about names:
|
So, also as a sum-up for myself:
After some discussion IRL, I'd like to propose the following:
The last case would be to construct the checksum of a type (contract) to be able to match it for conformance against, for example, ERC721. |
@christianparpart see my proposal in #1447 (comment), it just needs the ability to pass around interface types. |
About inheritance: We could have two members, one that only concerns the functions in the interface itself and another one that also includes inherited members. They can be combined by xor, so people can use them how they like. |
Brainstorming:
Leaving open the possibility that ERC-165 will be superseded. |
|
@fulldecent @jbaylina @nventuro @frangio We kindly need your input here. We would like to add eip 165 support as a feature to Solidity. Unfortunately, I was not able to find out how EIP 165 is defined with regards to inheritance. If a contract C inherits from A and B, what is its interface identifier? Is it the xor of the function selectors defined in A, B and C or just the ones added by C? |
A contract can support more than a one interface identifier, so it's not possible to say which should be the identifier for a contract. If C inherits from A and B, it may be that both A's and B's identifiers are relevant for the specific protocol, or C's, or any other combination. The proposal at the beginning of this issue suggested to only support asking for the identifier of an interface. This is interesting because interfaces don't support inheritance, so it wouldn't be a problem in that case. |
@frangio "unfortunately", interfaces now do support inheritance. My intuitive approach would be to include all functions, inherited or not. If you want to compare against multiple interfaces, you would of course have to use something like
|
I think I share that intuition. However, in the spec for ERC721, the optional interfaces were documented as: interface ERC721Metadata /* is ERC721 */ { The inheritance was commented out because at the moment it wasn't allowed, but it would nowadays be uncommented. What would you say in this case? What's needed is the id for the uninherited functions of |
@frangio you are right, this is a place where we would need the interface id of the "intrinsic" interface. Other solutions:
|
The ERC-165 identifier uses only the functions directly in the interface and it excludes the functions from the inherited interfaces. Test case 1: interface B {
function f();
}
interface I is B { // The ERC-165 signature uses only the function g
function g();
} Test case 2: interface B {
function f();
}
interface I is B { // The ERC-165 signature uses functions f and g
function f();
function g();
} This is not explicitly stated in ERC-165 (sorry!). But it is implied with its reference to Ethereum Name Service. And ENS prescribes the above process with its dependent interfaces. Other than the ERC-165 standard, the above specification is already implemented in ERC-721 and ERC-1155. For example, ERC-1155 states I believe Solidity should NOT support an ERC-165 interface calculation for
|
@fulldecent thanks for the clarifications! Would you think that |
Yes, I think this is an appropriate name. It is consistent with the function definition at https://eips.ethereum.org/EIPS/eip-165 |
I don't have a strong opinion but I feel that disallowing on contracts is kind of arbitrary. A contract defines an interface too. |
@chriseth Has there been any decision regarding whether the interfaceId will include inherited functions? We need to decide whether to use inheritance in our interfaces for the upcoming OpenZeppelin Contracts 3.0. (OpenZeppelin/openzeppelin-contracts#2113) |
@frangio no decision yet. We are leaning towards disallowing interfaceID for contracts at least for now. What is your take on whether it should include inherited functions or not? How is it used in the community? |
I don't think there's sufficient precedent in the community yet. Most people probably still don't know that interfaces can inherit. My intuition is that inherited functions should not be included for the id. Edit: I realized that I'm contradicting my initial intuition that I shared a few comments back. I guess I'm still undecided on this. Sorry! |
Yeah, I think most of us are :( |
I also believe an interface's id should be computed exclusively based on the functions declared on that interface and not on inherited ones. And I take @fulldecent's comment about how interfaces are interpreted by ERC165 as confirmation that this is how we should proceed. |
I talked to @jbaylina at EthCC and he mentioned he would intuitively think that How would an implementation of ERC165's |
The issue with including inherited interfaces is that would only be useful when referring to that very specific contract, with its entire ABI. Often you're only concerned with a subset of it (e.g. does it support ERC165, does it support ERC20, etc), not the entire thing. I'm not sure I can think of scenarios where I'd use something else other than the 'intrinsic' interface id. |
@nventuro I think the prime example here is an extension interface of ERC20. What should its interface ID be? |
I think I see your point. I was mostly thinking about contracts, not interfaces. I do believe though that 'intrinsic' would definitely be the way to go for contracts: if we're considering adding IDs for them, I'd rather they work the same way as interface IDs do. |
It seems that most of the people who have a specific opinion think that inherited functions should not be included. Because of that, I would propose to just implement it like that now. |
At present, given
Why does
I don't see why this syntax should work for interfaces but not contracts. If you want the ERC165 |
Is the ERC superseded? What to use now? |
Implement
type(I).interfaceId
as the XOR of the function selectors of the external interface of an interface or contract excluding inherited functions.The tests should include a full ERC-165 implementation that includes inheritance.
Original text:
We can add ERC-165 support to Solidity like this:
Test case
Negative test cases
Useful example
Discussion
I just want to note that the convention (established by ENS and followed since then) is:
Test case
References
https://eips.ethereum.org/EIPS/eip-165
The text was updated successfully, but these errors were encountered: