diff --git a/CHANGELOG.md b/CHANGELOG.md index b62692b4dfc..92e20f3ad06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * `Strings`: addition of a `toHexString` function. ([#2504](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2504)) * `EnumerableMap`: change implementation to optimize for `key → value` lookups instead of enumeration. ([#2518](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2518)) * `GSN`: Deprecate GSNv1 support in favor of upcomming support for GSNv2. ([#2521](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2521)) + * `ERC165`: Remove uses of storage in the base ERC165 implementation. ERC165 based contracts now use storage-less virtual functions. Old behaviour remains available in the `ERC165Storage` extension. ([#2505](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2505)) ## 3.4.0 (2021-02-02) diff --git a/contracts/introspection/ERC165.sol b/contracts/introspection/ERC165.sol index 7c4d3639f24..fc9b52f1f7c 100644 --- a/contracts/introspection/ERC165.sol +++ b/contracts/introspection/ERC165.sol @@ -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; } } diff --git a/contracts/introspection/ERC165Storage.sol b/contracts/introspection/ERC165Storage.sol new file mode 100644 index 00000000000..02a97dbb852 --- /dev/null +++ b/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; + } +} diff --git a/contracts/introspection/README.adoc b/contracts/introspection/README.adoc index 38bd7828181..0668de417e2 100644 --- a/contracts/introspection/README.adoc +++ b/contracts/introspection/README.adoc @@ -20,6 +20,8 @@ Note that, in all cases, accounts simply _declare_ their interfaces, but they ar {{ERC165}} +{{ERC165Storage}} + {{ERC165Checker}} == Global diff --git a/contracts/mocks/ERC1155ReceiverMock.sol b/contracts/mocks/ERC1155ReceiverMock.sol index 6faaaa33658..c3a60d6b94b 100644 --- a/contracts/mocks/ERC1155ReceiverMock.sol +++ b/contracts/mocks/ERC1155ReceiverMock.sol @@ -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; diff --git a/contracts/mocks/ERC165Mock.sol b/contracts/mocks/ERC165Mock.sol index 54677d3d43d..5f9ff3f69a8 100644 --- a/contracts/mocks/ERC165Mock.sol +++ b/contracts/mocks/ERC165Mock.sol @@ -5,7 +5,4 @@ pragma solidity ^0.8.0; import "../introspection/ERC165.sol"; contract ERC165Mock is ERC165 { - function registerInterface(bytes4 interfaceId) public { - _registerInterface(interfaceId); - } } diff --git a/contracts/mocks/ERC165StorageMock.sol b/contracts/mocks/ERC165StorageMock.sol new file mode 100644 index 00000000000..36262d37d3c --- /dev/null +++ b/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); + } +} diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index d1b4de4effb..a618f0da024 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -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); } /** diff --git a/contracts/token/ERC1155/ERC1155Receiver.sol b/contracts/token/ERC1155/ERC1155Receiver.sol index 03a6eb58ec7..0e2d6eecdae 100644 --- a/contracts/token/ERC1155/ERC1155Receiver.sol +++ b/contracts/token/ERC1155/ERC1155Receiver.sol @@ -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); } } diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 0ca91ee4ce7..f78cced381a 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -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); } /** diff --git a/test/introspection/ERC165.test.js b/test/introspection/ERC165.test.js index 5e91e58d361..c891500e369 100644 --- a/test/introspection/ERC165.test.js +++ b/test/introspection/ERC165.test.js @@ -1,5 +1,3 @@ -const { expectRevert } = require('@openzeppelin/test-helpers'); - const { shouldSupportInterfaces } = require('./SupportsInterface.behavior'); const ERC165Mock = artifacts.require('ERC165Mock'); @@ -9,10 +7,6 @@ contract('ERC165', function (accounts) { this.mock = await ERC165Mock.new(); }); - it('does not allow 0xffffffff', async function () { - await expectRevert(this.mock.registerInterface('0xffffffff'), 'ERC165: invalid interface id'); - }); - shouldSupportInterfaces([ 'ERC165', ]); diff --git a/test/introspection/ERC165Storage.test.js b/test/introspection/ERC165Storage.test.js new file mode 100644 index 00000000000..568d64576fe --- /dev/null +++ b/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', + ]); +});