-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
ERC-165 Standard Interface Detection #881
Conversation
Hello authors, Christian Reitwießner @chriseth, Nick Johnson @Arachnid, RJ Catalano @VoR0220, Fabian Vogelsteller @frozeman, Hudson Jameson @Souptacular, Jordi Baylina @jbaylina, Griff Green @GriffGreen, William Entriken [email protected] Would you please update your email address here (via PR to this PR or just tell me). Current EIP-X requests email address. So I will delete your username and add any email addresses. Also, I have not verified who wrote the prior version of 165 that this is based on. If you do not feel you should be listed as an author, please remove your name (via PR to this PR or just tell me). |
An argument could be made that the rationale section should explain why we chose to XOR the function selectors rather than have a response for every single function selector. |
Hello @jbaylina, just wanted to check. Are you still on board with this standard? And would you like to maintain the cache, or are you accepting updated to it? |
I wish we just had typeclasses somehow for this. Would make things much easier. |
Found from git log messages around the world.
Meta:
There's the "View" button on the "Files changed" tab, linking to the rendered version in your repo. (At least when viewing on a desktop browser.) Changing the |
Dear esteemed EIP editors, @cdetrio @Souptacular @wanderer @Arachnid @pirapira @vbuterin @nicksavers: By the EIP work flow process, I do hereby respectfully request by this EIP be granted DRAFT status under the number 165 and that you inform me of the next step to take in this voyage. Executive summary
Thank you, |
@veox Thank you, I'm using that now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK to merge as Draft
Please note that Solidity contracts will check that the calldata is long enough in the future. This means that you should pad the data to pull 32 bytes instead of just supplying the first 4 bytes of a |
I'm not sure if the implementation of To avoid more false positives, you should use a separate area for the output and also check that the size is correct using |
@chriseth OK, now I see. Sorry, the assembly needs more comments. Here are the cases I considered:
An all these cases, the discrimination between these two cases is the determinant:
Originally we pulled the contact code length and checked the result of the CALL operation. But none of the matters because of these two cases. Can you think of a case where the current implementation does not work? |
I just think this is bad practice. You are treating a failed call the same as a call that returns true. We only have four bytes for the selector, there are just too many collisions. I would even go so far and treat a function that returns dirty data (i.e. anything that has a bit set that is not the lowermost one) as not implementing the interface. So basically if the contract either fails or returns a non-bool on call to |
Oh and Ah and a final remark: Please only adopt this ERC with |
Ah and a post-final remark: If you provide 30000 gas, the called contract can access and modify state, so the function call is not even guaranteed to return the same value on the same call (which might not even be the case with staticcall because of |
Just implemented |
EIPS/eip-165.md
Outdated
/// use less than 30000 gas. | ||
/// @return `true` if the contract implements `interfaceID` and | ||
/// `interfaceID` is not 0xffffffff, `false` otherwise | ||
function supportsInterface(bytes4 interfaceID) external view returns (bool); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to clarify this example. Does any ERC165-compliant interface need to specify supportsInterface
as part of its interface? Also there should be other functions to have a real example. It might not be clear to the reader that there can be other functions and they should be part of the interface identifier.
It would probably also good to include supportsInteface
in the Solidity101
contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, ok, I think I also just misunderstood this. supportsInterface
can return true on multiple inputs and each input is the xor of a set of functions, right? Each ERC165-compliant contract must return true for the ERC165-interface id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Correct. For example, in ERC-721, you will return true for ERC-165, ERC-721, ERC-721-Metadata, ERC-721-Enumeration. These are all separate interfaces.
EIPS/eip-165.md
Outdated
|
||
### How Interfaces are Identified | ||
|
||
For this standard, an *interface* is a set of [function selectors as calculated in Solidity](http://solidity.readthedocs.io/en/develop/abi-spec.html#function-selector). This a subset of [Solidity's concept of interfaces](http://solidity.readthedocs.io/en/develop/abi-spec.html) and the `interface` keyword definition which also define return types, mutability and events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that function selectors are not specific to Solidity, they are part of the ABI specification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated: "function selectors as defined by the Ethereum ABI"
FYI: The current example on how to read an ERC165 contract is broken, #1640 |
FYI: The example has another issue: it does not check whether enough gas is passed in the call to Indeed due to the behavior of EIP-150 and the fact that STATIC_CALL (and other CALL... opcodes) use the gas value provided only as a maximum (instead of a strict value) it is possible for the following to happen:
There are different ways to fix it. They are described in #1930 which itself propose a EVM based solution.
this require to compute E (the gas between
This works because if the call throw because of not enough gas, the amount of gas left will be lower than 30000/63. But this also require for
For EIP-165 there is yet another option:
The best option is in my opinion 3), the solution proposed by #1930 itself since it would be independent of current gas pricing. It would be great to get this in in the next hardfork, or at least get the conversation going since this is an issue affecting other use-case like meta-transaction (see safe-global/safe-smart-account#100) Note that the bug is present elsewhere (and I expect it to be present in most if not all ERC165 checker, since the specific behavior of EIP-150 is rarely considered) : see openzeppelin here or EIP-1820 here I have some test code here showing the issue There is also a specific example here involving #1820 which has the same issue |
I fail to see how this is an issue. This EIP requires that a call to In the examples, you consider cases where the call was not given 30k gas; how If a particular implementation of The whole point gets even more moot once you consider that the called contract might have Out of the ways to "fix it" that you propose, (1) and (2) seem most appropriate to me, and can be implemented at language-level (no need for low-level support in EVM). |
I am not saying the problem is the EIP spec, doesContractImplementInterface should throw if it was not given enough gas to provide 30k gas to the
Yes this was to illustrate the behavior of the buggy implementation : it allows giving less than 30k gas while it should revert the call at that point. A proper ERC-165 checker implementation would not let that happen.
The problem with 1) is that The problem with 2) is that it only works if the call is not reverting based on a low of gas, as in this case, the validity of the check does not stand anymore. But, such issues are not only affecting ERC-165. meta-transaction are also affected as illustarted in the gnosis safe issue linked above. And these requires more complicated gas prediction. An EVM solution is the proper way to handle this. |
Please post a test case (preferably pastable into Remix IDE) which shows a compliant implementation of the ERC that produces incorrect results in the Since this is a long-ago finalized and oft-referenced ERC I request please that the threshold for reopening a discussion will be a working test case and not just discussion points. |
@fulldecent Also since the example stated in ERC-165 is declaring The bug is still present but because it use the 3 calls the amount of gas where is fails is arround 20000 |
Another thing I realised is that ERC165 spec does not allow This is because at that point of the inner This issue is about the actual spec and should probably be mentioned |
No, it's not buggy. Those test cases are invalid. From ERC165 (emphasis mine):
The standard is not broken. OpenZeppelin's implementation is not wrong. Technically, yes, a contract making the call to the ERC165 contract should static call with 30,000 gas. That has nothing to do with this implementation. All it means is instead of this: foo.supportsInterface(0x1234abcd) you can do this: foo.supportsInterface.gas(30000)(0x1234abcd) |
At the moment it is not immediately obvious to me that the standard is broken. And "immediately obvious" means a reference to a certain sentence in the standard and a minimal test case that anybody can paste into Remix showing this sentence is invalid/unenforceable. In the meantime I am happy to continue the discussion at https://github.com/wighawag/ethereum_gas. This is good analysis from a hardworking individual and I am happy to study further there. Also, @wighawag Ronan has some other good projects I am checking out. I hope we can move this thread to that repo for now. If a minimal test case can be perfected then let's bring it back to this audience at that time. |
I made into a remixable test case. At least as far the ERC165 example implementation (used as a contract to call externally) is concerned I could not make it work as a remix gist url but it should not be hard to recreate it from the gist here : https://gist.github.com/wighawag/9404f81a2a6d0cfa2134549f186d4d46 The inline comments explain the step to reproduce the issue
No the standard is not broken, I am just saying the example implementation is.
It is, see for further discussion : OpenZeppelin/openzeppelin-contracts#1750
If you call it that way you are actually fine, since a throw would be propagated and the caller will also throw. But this cannot be used in all case since you might not want to throw there. You might want to have a different execution path. And that is what the example implementation is allowing you. It gives you a boolean. But as I showed, the example implementation return the wrong boolean in some cases. Please note that part of the problem is that specifying the gas as part of call like In your example that last part is not possible since the throw is propagated, but in the example implementation which use low level calls, the call to |
I suspect the cleanest way to do that would be combining the
I disagree with your interpretation of the standard. To me, it seems that limit was placed to "pin the blame on someone" if the obvious implementation causes an OOG error. I don't think the authors intended to make it so that 30,000 gas is required to be available during the execution of the staticcall, just that any given call doesn't use more than 30,000 gas. There shouldn't be a need to modify the implementation in a way where it has to do some gas calculation on |
That what I meant all along. The bug is in the ERC165Query implementation example. Please have a look at the test cases.
I am not sure I understand all what you are saying but it seems you say that the following statement from ERC-165 does not mean what it says:
Hard to interpret the meaning differently but I agree that the standard is misleading since it also mention :
which could be interpreted as passing the 30,000 gas to the call opcode (which does not ensure that 30,000 gas is actually given). I guess the authors did not realise the implication of EIP-150 and the fact that the gas value passed to the call opcode is a max value only. The problem is that if it was the case that " This function must return a bool and use at most 30,000 gas." did not mean what it means but simply meant "do not give more than 30,000 when calling the function" the standard would be underspecified. A limit is indeed required for In that case too, the current ERC165Query would be invalid as the test cases show : A valid implementation of `supportsInterface use 22000 gas and return true |
Assembly, or a new feature in solidity (that would be dependent on current opcode pricing to not increase), or as I suggest new opcodes (to later replace the current ones) : #1930 But for ERC-165 there is an alternative way that does not require assembly, as I mentioned earlier, check the gas after the call.
Just to be on the safe side, this would probably require to stipulate in the standard that supportsInterface must not revert based on the gas amount ( |
Hey guys, I'm a little new here, so I may not know something about the rules of the discussions here. |
@re-minder Thank you! Yes, you are correct--the ERC is now outdated and the conclusion is incorrect based on changes to gas prices. 165 is a rare ERC in that it actually does really care about the gas prices, so your note is very welcome here. The specification stays the same, although implementation notes are outdated. I am guessing that it is probably appropriate to correct/update this EIP rather than publishing a new one (again, because no specification is changing). To calculate gas prices, you cane use Remix, compile (with optimization), post a transaction, and use the debugging window (at bottom), and click details (the down arrow next to debug) to get exact answers. Another alternative is to read the Yellow Paper and manually add any parts up. Would you like to take a stab at the PR here? |
@fulldecent thank you very much for noting ways of calculating prices, I will use Remix and make a PR very soon. |
This standard works well for input parameters. But how do I check if the method returns the data types I expect, the method signature in solidity does not include the parameters that are returned. And from the point of view of solidity getCarId(string carName) string
getCarId(string carName) uint256 These are the same interfaces. From the point of view of an application that communicates with solidity, these are completely different interfaces. |
Hi @ilya-korotya I'm happy to help you with that. Please open an issue on Stack Overflow and ping me there. Or add it to the agenda on the weekly Community Service Hour call and we can discuss live. We try not to do tech support in these GutHub issues when the standard is finalized. |
Hi @fulldecent . Thank you for your reply. I've created a new question in Stack Overflow. Could you please check it out? |
See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-165.md
Reference
Work plan
mapping(bytes4=>bool)
approach (this is O(1) gas)interfaceId == a || interfaceId == b || ...
approach (this is O(n) gas)BELOW IS A COMPLETE COPY OF THE LATEST FULL TEXT PROPOSAL (VERSION 03c67d2) FOR YOUR CONVENIENCE.
Preamble
Simple Summary
Creates a standard method to publish and detect what interfaces a smart contract implements.
Abstract
Herein, we standardize the following:
Motivation
For some "standard interfaces" like the ERC-20 token interface, it is sometimes useful to query whether a contract supports the interface and if yes, which version of the interface, in order to adapt the way in which the contract is to be interfaced with. Specifically for ERC-20, a version identifier has already been proposed. This proposal stadardizes the concept of interfaces and standardizes the identification (naming) of interfaces.
Specification
How Interfaces are Identified
For this standard, an interface is a set of function selectors as defined by the Ethereum ABI. This a subset of Solidity's concept of interfaces and the
interface
keyword definition which also defines return types, mutability and events.We define the interface identifier as the XOR of all function selectors in the interface. This code example shows how to calculate an interface identifier:
Note: interfaces do not permit optional functions, therefore, the interface identity will not include them.
How a Contract will Publish the Interfaces it Implements
A contract that is compliant with ERC-165 shall implement the following interface (referred as
ERC165.sol
):The interface identifier for this interface is
0x01ffc9a7
. You can calculate this by runningbytes4(keccak256('supportsInterface(bytes4)'));
or using theSelector
contract above.Therefore the implementing contract will have a
supportsInterface
function that returns:true
wheninterfaceID
is0x01ffc9a7
(EIP165 interface)false
wheninterfaceID
is0xffffffff
true
for any otherinterfaceID
this contract implementsfalse
for any otherinterfaceID
This function must return a bool and use at most 30,000 gas.
Implementation note, there are several logical ways to implement this function. Please see the example implementations and the discussion on gas usage.
How to Detect if a Contract Implements ERC-165
STATICCALL
to the destination address with input data:0x01ffc9a701ffc9a700000000000000000000000000000000000000000000000000000000
and gas 30,000. This corresponds tocontract.supportsInterface(0x01ffc9a7)
.0x01ffc9a7ffffffff00000000000000000000000000000000000000000000000000000000
.How to Detect if a Contract Implements any Given Interface
supportsInterface(interfaceID)
to determine if it implements an interface you can use.Rationale
We tried to keep this specification as simple as possible. This implementation is also compatible with the current Solidity version.
Backwards Compatibility
The mechanism described above (with
0xffffffff
) should work with most of the contracts previous to this standard to determine that they do not implement ERC-165.Also the ENS already implements this EIP.
Test Cases
Following is a contract that detects which interfaces other contracts implement. From @fulldecent and @jbaylina.
Implementation
This approach uses a
view
function implementation ofsupportsInterface
. The execution cost is 586 gas for any input. But contract initialization requires storing each interface (SSTORE
is 20,000 gas). TheERC165MappingImplementation
contract is generic and reusable.Following is a
pure
function implementation ofsupportsInterface
. The worst-case execution cost is 236 gas, but increases linearly with a higher number of supported interfaces.With three or more supported interfaces (including ERC165 itself as a required supported interface), the mapping approach (in every case) costs less gas than the pure approach (at worst case).
Copyright
Copyright and related rights waived via CC0.