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
43 changes: 11 additions & 32 deletions contracts/introspection/ERC165.sol
Expand Up @@ -7,43 +7,22 @@ import "./IERC165.sol";
/**
* @dev Implementation of the {IERC165} interface.
*
* Contracts may inherit from this and call {_registerInterface} to declare
* their support of an interface.
* Contracts that want to implement ERC165 should inherit from this contract and override {supportsInterface} to check
* for the additional interface id that will be supported. For example:
*
* ```solidity
* function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
* return interfaceId == type(MyInterface).interfaceId || super.supportsInterface(interfaceId);
* }
* ```
*
* Alternatively, {ERC165Storage} provides an easier to use but more expensive implementation.
*/
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
@@ -0,0 +1,41 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "./ERC165.sol";

/**
* @dev Storage based 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/ERC1155ReceiverMock.sol
Expand Up @@ -2,10 +2,10 @@

pragma solidity ^0.8.0;

import "../introspection/ERC165.sol";
import "../token/ERC1155/IERC1155Receiver.sol";
import "./ERC165Mock.sol";

contract ERC1155ReceiverMock is IERC1155Receiver, ERC165Mock {
contract ERC1155ReceiverMock is IERC1155Receiver, ERC165 {
bytes4 private _recRetval;
bool private _recReverts;
bytes4 private _batRetval;
Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/ERC165Mock.sol
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
11 changes: 11 additions & 0 deletions contracts/mocks/ERC165StorageMock.sol
@@ -0,0 +1,11 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

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

contract ERC165StorageMock is ERC165Storage {
function registerInterface(bytes4 interfaceId) public {
_registerInterface(interfaceId);
}
}
13 changes: 8 additions & 5 deletions contracts/token/ERC1155/ERC1155.sol
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
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
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
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
25 changes: 25 additions & 0 deletions test/introspection/ERC165Storage.test.js
@@ -0,0 +1,25 @@
const { expectRevert } = require('@openzeppelin/test-helpers');

const { shouldSupportInterfaces } = require('./SupportsInterface.behavior');

const ERC165Mock = artifacts.require('ERC165StorageMock');

contract('ERC165Storage', function (accounts) {
beforeEach(async function () {
this.mock = await ERC165Mock.new();
});

it('register interface', async function () {
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');
});

shouldSupportInterfaces([
'ERC165',
]);
});