From 25d4344a1dc9c2dbec963a6ab1999679debeef5e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 28 Jan 2021 16:45:49 +0100 Subject: [PATCH 1/9] Replace storage dependency of ERC165 with virtual inheritance --- contracts/introspection/ERC165.sol | 31 +-------------------- contracts/mocks/ERC165Mock.sol | 27 ++++++++++++++++-- contracts/token/ERC1155/ERC1155.sol | 13 +++++---- contracts/token/ERC1155/ERC1155Receiver.sol | 8 ++++-- contracts/token/ERC721/ERC721.sol | 13 ++++++--- 5 files changed, 48 insertions(+), 44 deletions(-) diff --git a/contracts/introspection/ERC165.sol b/contracts/introspection/ERC165.sol index 7c4d3639f24..dbc978861d4 100644 --- a/contracts/introspection/ERC165.sol +++ b/contracts/introspection/ERC165.sol @@ -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; } } diff --git a/contracts/mocks/ERC165Mock.sol b/contracts/mocks/ERC165Mock.sol index 54677d3d43d..00ad3b8582c 100644 --- a/contracts/mocks/ERC165Mock.sol +++ b/contracts/mocks/ERC165Mock.sol @@ -2,10 +2,31 @@ pragma solidity ^0.8.0; -import "../introspection/ERC165.sol"; +import "../introspection/IERC165.sol"; + +contract ERC165Mock is IERC165 { + /** + * @dev A mapping of interface id to whether or not it's supported. + */ + mapping(bytes4 => bool) private _supportedInterfaces; + + /** + * @dev A contract implementing SupportsInterfaceWithLookup + * implement ERC165 itself. + */ + constructor () { + registerInterface(type(IERC165).interfaceId); + } + + /** + * @dev Implement supportsInterface(bytes4) using a lookup table. + */ + function supportsInterface(bytes4 interfaceId) public view override returns (bool) { + return _supportedInterfaces[interfaceId]; + } -contract ERC165Mock is ERC165 { function registerInterface(bytes4 interfaceId) public { - _registerInterface(interfaceId); + require(interfaceId != 0xffffffff, "ERC165: invalid interface id"); + _supportedInterfaces[interfaceId] = true; } } diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index dea9f28d07f..12cece9a025 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); } /** From 592e8c3dabd6252129d559a39468e43ba5fe8589 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 12 Feb 2021 19:21:11 +0100 Subject: [PATCH 2/9] add a ERC165Storage implementation --- contracts/introspection/ERC165Storage.sol | 41 +++++++++++++++++++++++ contracts/mocks/ERC165Mock.sol | 27 ++------------- 2 files changed, 44 insertions(+), 24 deletions(-) create mode 100644 contracts/introspection/ERC165Storage.sol diff --git a/contracts/introspection/ERC165Storage.sol b/contracts/introspection/ERC165Storage.sol new file mode 100644 index 00000000000..9e12a27e7d6 --- /dev/null +++ b/contracts/introspection/ERC165Storage.sol @@ -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; + } +} diff --git a/contracts/mocks/ERC165Mock.sol b/contracts/mocks/ERC165Mock.sol index 00ad3b8582c..f61a01c7b93 100644 --- a/contracts/mocks/ERC165Mock.sol +++ b/contracts/mocks/ERC165Mock.sol @@ -2,31 +2,10 @@ pragma solidity ^0.8.0; -import "../introspection/IERC165.sol"; - -contract ERC165Mock is IERC165 { - /** - * @dev A mapping of interface id to whether or not it's supported. - */ - mapping(bytes4 => bool) private _supportedInterfaces; - - /** - * @dev A contract implementing SupportsInterfaceWithLookup - * implement ERC165 itself. - */ - constructor () { - registerInterface(type(IERC165).interfaceId); - } - - /** - * @dev Implement supportsInterface(bytes4) using a lookup table. - */ - function supportsInterface(bytes4 interfaceId) public view override returns (bool) { - return _supportedInterfaces[interfaceId]; - } +import "../introspection/ERC165Storage.sol"; +contract ERC165Mock is ERC165Storage { function registerInterface(bytes4 interfaceId) public { - require(interfaceId != 0xffffffff, "ERC165: invalid interface id"); - _supportedInterfaces[interfaceId] = true; + _registerInterface(interfaceId); } } From 0604e828c6987b10055f1d76e8faae12820c8f2a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 12 Feb 2021 23:51:02 +0100 Subject: [PATCH 3/9] improve coverage --- test/introspection/ERC165.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/introspection/ERC165.test.js b/test/introspection/ERC165.test.js index 5e91e58d361..ffb54cd670d 100644 --- a/test/introspection/ERC165.test.js +++ b/test/introspection/ERC165.test.js @@ -9,6 +9,12 @@ contract('ERC165', function (accounts) { 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'); }); From 41eb0ddcc0d475b96c37d79b1524b306aaf7de8d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 17 Feb 2021 13:40:57 +0100 Subject: [PATCH 4/9] rename ERC165Mock --- contracts/mocks/ERC1155ReceiverMock.sol | 4 ++-- contracts/mocks/ERC165StorageMock.sol | 11 +++++++++++ test/introspection/ERC165Storage.test.js | 25 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 contracts/mocks/ERC165StorageMock.sol create mode 100644 test/introspection/ERC165Storage.test.js 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/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/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', + ]); +}); From 0b39f85996b285e4aba46401d13acc1fe193f0d1 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 17 Feb 2021 14:59:47 -0300 Subject: [PATCH 5/9] adjust documentation for new implementations --- contracts/introspection/ERC165.sol | 12 ++++++++++-- contracts/introspection/ERC165Storage.sol | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/contracts/introspection/ERC165.sol b/contracts/introspection/ERC165.sol index dbc978861d4..fc9b52f1f7c 100644 --- a/contracts/introspection/ERC165.sol +++ b/contracts/introspection/ERC165.sol @@ -7,8 +7,16 @@ 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 { /** diff --git a/contracts/introspection/ERC165Storage.sol b/contracts/introspection/ERC165Storage.sol index 9e12a27e7d6..02a97dbb852 100644 --- a/contracts/introspection/ERC165Storage.sol +++ b/contracts/introspection/ERC165Storage.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.0; import "./ERC165.sol"; /** - * @dev Implementation of the {IERC165} interface. + * @dev Storage based implementation of the {IERC165} interface. * * Contracts may inherit from this and call {_registerInterface} to declare * their support of an interface. From 1cac9b8c1e9af8420c876533ca73e58c9ac3961c Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 17 Feb 2021 18:27:13 -0300 Subject: [PATCH 6/9] remove storage test from plain 165 test --- contracts/mocks/ERC165Mock.sol | 7 ++----- test/introspection/ERC165.test.js | 10 ---------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/contracts/mocks/ERC165Mock.sol b/contracts/mocks/ERC165Mock.sol index f61a01c7b93..5f9ff3f69a8 100644 --- a/contracts/mocks/ERC165Mock.sol +++ b/contracts/mocks/ERC165Mock.sol @@ -2,10 +2,7 @@ pragma solidity ^0.8.0; -import "../introspection/ERC165Storage.sol"; +import "../introspection/ERC165.sol"; -contract ERC165Mock is ERC165Storage { - function registerInterface(bytes4 interfaceId) public { - _registerInterface(interfaceId); - } +contract ERC165Mock is ERC165 { } diff --git a/test/introspection/ERC165.test.js b/test/introspection/ERC165.test.js index ffb54cd670d..4cf46082b13 100644 --- a/test/introspection/ERC165.test.js +++ b/test/introspection/ERC165.test.js @@ -9,16 +9,6 @@ contract('ERC165', function (accounts) { 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', ]); From 88c576b270223085cfe8bbf6dac2cd574402ab2f Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 17 Feb 2021 18:29:37 -0300 Subject: [PATCH 7/9] lint --- test/introspection/ERC165.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/introspection/ERC165.test.js b/test/introspection/ERC165.test.js index 4cf46082b13..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'); From 7a4897627857fc47aa0b23866bdf6a30ce364b1a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Feb 2021 15:01:16 +0100 Subject: [PATCH 8/9] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38a94bdc263..48c009a1ddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * `Context`: making `_msgData` return `bytes calldata` instead of `bytes memory` ([#2492](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2492)) * `ERC20`: Removed the `_setDecimals` function and the storage slot associated to decimals. ([#2502](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2502)) * `Strings`: addition of a `toHexString` function. ([#2504](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2504)) + * `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) From c7fcbf8210493bbd674db34a2a2c179145992732 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 18 Feb 2021 15:57:13 -0300 Subject: [PATCH 9/9] add ERC165Storage to docs site --- contracts/introspection/README.adoc | 2 ++ 1 file changed, 2 insertions(+) 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