EIP-165 `supportsInterface` for `IERC721Receiver`

[songhobby]

function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) {
        // In addition to the current interfaceId, also support previous version of the interfaceId that did not
        // include the castVoteWithReasonAndParams() function as standard
        return
            interfaceId ==
            (type(IGovernor).interfaceId ^
                this.castVoteWithReasonAndParams.selector ^
                this.castVoteWithReasonAndParamsBySig.selector ^
                this.getVotesWithParams.selector) ||
            interfaceId == type(IGovernor).interfaceId ||
            interfaceId == type(IERC1155Receiver).interfaceId ||
            super.supportsInterface(interfaceId);
    }

=>

function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) {
        // In addition to the current interfaceId, also support previous version of the interfaceId that did not
        // include the castVoteWithReasonAndParams() function as standard
        return
            interfaceId ==
            (type(IGovernor).interfaceId ^
                this.castVoteWithReasonAndParams.selector ^
                this.castVoteWithReasonAndParamsBySig.selector ^
                this.getVotesWithParams.selector) ||
            interfaceId == type(IGovernor).interfaceId ||
            interfaceId == type(IERC721Receiver).interfaceId ||
            interfaceId == type(IERC1155Receiver).interfaceId ||
            super.supportsInterface(interfaceId);
    }

[Amxx]

Hello @songhobby

In ERC1155, ERC165 is explicitly mentioned, and it is stated that:

Smart contracts MUST implement the ERC-165 supportsInterface function and signify support for the ERC1155TokenReceiver interface to accept transfers. See “ERC1155TokenReceiver ERC-165 rules” for further detail.

In ERC721, ERC165 is mentioned for the ERC721 contract itself, but not for the ERC721TokenReceiver. That means type(IERC721Receiver).interfaceId is not standardized, and that is why we don't include it.

You can see that

• token/ERC721/utils/ERC721Holder.sol doesn't include ERC165
• token/ERC1155/utils/ERC1155Holder.sol does include ERC165 (through ERC1155Receiver)

@frangio, so you think we should include type(IERC721Receiver).interfaceId in the governor/timelock ?

[frangio]

That's interesting, this is the first time we hear about EIP-165 for EIP-721 receivers. The EIP doesn't seem to require it.

If we add this in Governor we should add it in ERC721Holder as well.

For some background, Gnosis Safe includes support for the interface:

https://github.com/safe-global/safe-contracts/blob/c36bcab46578a442862d043e12a83fec41143dec/contracts/handler/DefaultCallbackHandler.sol#L58

I'd be curious to see other contracts that do this.

@songhobby Do you know of any contracts or systems that assume the interface will be present in supportsInterface?

[songhobby]

@frangio Not what I'm aware of except for the Gnosis Safe as you pointed out! My rationale to submit issue is that if a contract implements IERC165, its supportsInterface SHOULD work for all the interfaces the contract implements, or at least for those public well-known interfaces like IERC721Receiver.

[Amxx]

We discussed that issue internally and decided to not implement it for the following reason.

While EIP-1155, which requires the receiver to support EIP-165 and to advertise ERC1155Receiver support, EIP-721 has no similar requirements. This means that we don't need to include that (and we don't). Adding this support would be going the extra mile to include a feature that is not required.

We feel like including it ourselves by default is an opinionated choice. We try to avoid these, and let the user chose which "options" they want to add to their contract. Adding EIP-165 and advertising IERC721Receiver support is easy. Removing it is way harder. So we are to add it, there must be a clear advantage.

The downside is clear.

We would be forcing the user to include a feature that is not strictly needed, that they might not want, and that will add to the contract size. Because the function selector for support interface is quite small (0x01ffc9a7), adding EIP-165 support to a contract will actually make most calls to the contract marginally more expensive (functions lookup is usually linear and performed on the sorted selectors).

The upside is not that clear.

Because EIP-721 does not require the receiver to have EIP-165 support, someone that is looking for an ERC721Receiver should not rely on EIP-165. Relying on it will fail for most receiver contracts that strictly apply EIP-721. The whole point of EIP-165 is to effectively identify features in contract. This is achieved when EIP-165 support is required, like for the EIP-721 token contract, but its not working when many/most contracts don't support it (we could think of EIP-20).

In the case of ERC721Receiver, we fell like, in most cases, it the cost would not be justified. Developers are of course free to add that support themselves (its quite easy to do), but we don't want to force them into this path and make the decision for them.