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

Refactor ERC165 to use function overriding instead of storage. #2505

Merged
merged 13 commits into from
Feb 18, 2021
31 changes: 1 addition & 30 deletions contracts/introspection/ERC165.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,10 @@ import "./IERC165.sol";
* their support of an interface.
*/
abstract contract ERC165 is IERC165 {
/**
* @dev Mapping of interface ids to whether or not it's supported.
*/
mapping(bytes4 => bool) private _supportedInterfaces;

constructor () {
// Derived contracts need only register support for their own interfaces,
// we register support for ERC165 itself here
_registerInterface(type(IERC165).interfaceId);
}

/**
* @dev See {IERC165-supportsInterface}.
*
* Time complexity O(1), guaranteed to always use less than 30 000 gas.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
return _supportedInterfaces[interfaceId];
}

/**
* @dev Registers the contract as an implementer of the interface defined by
* `interfaceId`. Support of the actual ERC165 interface is automatic and
* registering its interface id is not required.
*
* See {IERC165-supportsInterface}.
*
* Requirements:
*
* - `interfaceId` cannot be the ERC165 invalid interface (`0xffffffff`).
*/
function _registerInterface(bytes4 interfaceId) internal virtual {
require(interfaceId != 0xffffffff, "ERC165: invalid interface id");
_supportedInterfaces[interfaceId] = true;
return interfaceId == type(IERC165).interfaceId;
}
}
41 changes: 41 additions & 0 deletions contracts/introspection/ERC165Storage.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "./ERC165.sol";

/**
* @dev Implementation of the {IERC165} interface.
*
* Contracts may inherit from this and call {_registerInterface} to declare
* their support of an interface.
*/
abstract contract ERC165Storage is ERC165 {
/**
* @dev Mapping of interface ids to whether or not it's supported.
*/
mapping(bytes4 => bool) private _supportedInterfaces;

/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
return super.supportsInterface(interfaceId) || _supportedInterfaces[interfaceId];
}

/**
* @dev Registers the contract as an implementer of the interface defined by
* `interfaceId`. Support of the actual ERC165 interface is automatic and
* registering its interface id is not required.
*
* See {IERC165-supportsInterface}.
*
* Requirements:
*
* - `interfaceId` cannot be the ERC165 invalid interface (`0xffffffff`).
*/
function _registerInterface(bytes4 interfaceId) internal virtual {
require(interfaceId != 0xffffffff, "ERC165: invalid interface id");
_supportedInterfaces[interfaceId] = true;
}
}
4 changes: 2 additions & 2 deletions contracts/mocks/ERC165Mock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this entire file can be deleted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then ERC165Storage should not be abstract, right ?

pragma solidity ^0.8.0;

import "../introspection/ERC165.sol";
import "../introspection/ERC165Storage.sol";

contract ERC165Mock is ERC165 {
contract ERC165Mock is ERC165Storage {
Amxx marked this conversation as resolved.
Show resolved Hide resolved
function registerInterface(bytes4 interfaceId) public {
_registerInterface(interfaceId);
}
Expand Down
13 changes: 8 additions & 5 deletions contracts/token/ERC1155/ERC1155.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
*/
constructor (string memory uri_) {
_setURI(uri_);
}

// register the supported interfaces to conform to ERC1155 via ERC165
_registerInterface(type(IERC1155).interfaceId);

// register the supported interfaces to conform to ERC1155MetadataURI via ERC165
_registerInterface(type(IERC1155MetadataURI).interfaceId);
/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
return interfaceId == type(IERC1155).interfaceId
|| interfaceId == type(IERC1155MetadataURI).interfaceId
|| super.supportsInterface(interfaceId);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions contracts/token/ERC1155/ERC1155Receiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import "../../introspection/ERC165.sol";
* @dev _Available since v3.1._
*/
abstract contract ERC1155Receiver is ERC165, IERC1155Receiver {
constructor() {
_registerInterface(type(IERC1155Receiver).interfaceId);
/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
return interfaceId == type(IERC1155Receiver).interfaceId
|| super.supportsInterface(interfaceId);
}
}
13 changes: 9 additions & 4 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,16 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
constructor (string memory name_, string memory symbol_) {
_name = name_;
_symbol = symbol_;
}

// register the supported interfaces to conform to ERC721 via ERC165
_registerInterface(type(IERC721).interfaceId);
_registerInterface(type(IERC721Metadata).interfaceId);
_registerInterface(type(IERC721Enumerable).interfaceId);
/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
return interfaceId == type(IERC721).interfaceId
|| interfaceId == type(IERC721Metadata).interfaceId
|| interfaceId == type(IERC721Enumerable).interfaceId
|| super.supportsInterface(interfaceId);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions test/introspection/ERC165.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ contract('ERC165', function (accounts) {
this.mock = await ERC165Mock.new();
});

it('register interface', async function () {
Copy link
Contributor

@frangio frangio Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test should be removed since it pertains to ERC165Storage and not plain ERC165.

expect(await this.mock.supportsInterface('0x00000001')).to.be.equal(false);
await this.mock.registerInterface('0x00000001');
expect(await this.mock.supportsInterface('0x00000001')).to.be.equal(true);
});

it('does not allow 0xffffffff', async function () {
await expectRevert(this.mock.registerInterface('0xffffffff'), 'ERC165: invalid interface id');
});
Expand Down