-
Notifications
You must be signed in to change notification settings - Fork 11.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
Proposal: ERC165Checker - getSupportedInterfaces #2429
Comments
Hi @conspyrosy! Thanks for the suggestion, it is really appreciated. The project owner will review your suggestion as soon as they can. As per our conversation in the forum, please wait until we have discussed this idea before writing any code or submitting a Pull Request, so we can go through the design beforehand. We don’t want you to waste your time! |
Hi @conspyrosy, I think this idea makes sense. Have you ran into the need for a function like this yourself? Feel free to open a PR. I'm not sure if returning an empty array is the right choice in case the first check fails. Maybe if you explain the context where this is needed we can reason through it and find out what would be the best return value in that case. |
@frangio Yes, i've ran into the need for this function which is why i'm proposing this. Let me explain my scenario: Options protocols come in different shapes and sizes and support different functionality. I'm creating a generic OptionsAdapter that generifies options protocols into a single base interface so they can be interacted with in a uniform way. Additional functionality can be added on top of the base interface too. The adapter pattern is a fairly common pattern in software development. Here's the interfaces I have so far: IOptionsProtocol - base interface (mandatory) Users of the adapter will be able to pass in an arbitrary list of protocol addresses that conform to at least the base interface. So I want to form some kind of in memory object that describes the protocol features, which then informs me how i can interact with the protocol:
That way I can pull the details of the protocol before looping some functionality and can also perform logic conditionally based on the functionality supported without having to make redundant calls per protocol action. It's expected that ERC165 is always implemented on the addresses checked as the base interface (IOptionsProtocol) will implement ERC165. I think if the ERC165 method was not implemented it wouldnt really make sense to call this method as there's no way to check for interfaces that exist without it being implemented - that would imply the address isn't an IOptionsProtocol (which is actually an abstract contract implementing ERC165). The alternative to returning an empty array is reverting which I guess could be more appropriate since they passed in an invalid address? |
I was actually thinking of returning an array of the same length where all values are false. So in your case this is what I imagine: you would ask if the contract supports In fact the same would happen if the contract implements ERC165 but none of these interfaces, and I think it makes sense to treat those two situations uniformly. |
@frangio I was initially against this as I thought it was possible that an interface was implemented without implementing ERC165 (in which case the result wouldn't strictly be true). But i've just remembered in the design mentioned ERC165 is on the base level interface which everything extends from so this will work perfectly. I'll fix the code over the next week, add some tests then PR in. |
@conspyrosy Are you still considering submitting a PR for this ? |
@Amxx yes, bare with me as I'm very busy but I have the code. Just need to write a couple tests. I will try get it done over the next week. |
🧐 Motivation
ERC165 allows you to check which interfaces are implemented on a contract. OZ has a library
ERC165Checker.sol
with helper methods for checking if some contract implements an interface. First, it checks that ERC165 is implemented and then it checks if the interface is implemented. For batch checking interfaces, there is a separate methodsupportsAllInterfaces
which checks if ERC165 is implemented only once, and then checks each interface to ensure they are implemented. Here is the relevant existing code:However, this batching mechanism reduces the result to a single boolean value "true" or "false". The proposal is to replicate this behaviour but for each interface return a separate true/false value as sometimes you anticipate some interfaces will be implemented but not all and can tailor your logic around the interfaces supported. The benefit of the helper method vs looping over
supportsInterface
is that redundant checks tosupportsERC165
aren't made.📝 Details
Here is the code I've written which is pretty self-explanatory. If ERC165 isnt implemented on the contract then no checks can be made and an empty array is returned, else a boolean array is returned corresponding to the interface ids passed.
If this proposal is good and OZ is happy to go ahead with it, I can write tests and PR in.
The text was updated successfully, but these errors were encountered: