From 4dd8575bb6e94d84014bbcb098c981d34da5bb80 Mon Sep 17 00:00:00 2001 From: nikeshnazareth Date: Fri, 8 Mar 2019 09:41:22 +1100 Subject: [PATCH] Improve erc165 testing #1203 (#1666) * Rename variable from thing to contractUnderTest * Compute function signatures in ERC165 interfaces The ERC165 tests currently precompute some known interface ids. This commit extracts the interfaces into a separate object and precomputes the individual function signatures. This will be useful to identify contracts that support an interface but do not implement all of the corresponding functions. * Add tests for ERC165 interface implementations The ERC165 tests confirm that contracts claim to support particular interfaces ( using the supportsInterface method ) This commit extends those tests to confirm that the corresponding functions are included in the contract ABI. It also rewords the existing test names in order to group the implementation tests with the corresponding interface tests. * Remove obsolete ERC721Exists interface constant --- .../SupportsInterface.behavior.js | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/test/introspection/SupportsInterface.behavior.js b/test/introspection/SupportsInterface.behavior.js index fde7f5cc6..da8ed98c1 100644 --- a/test/introspection/SupportsInterface.behavior.js +++ b/test/introspection/SupportsInterface.behavior.js @@ -1,10 +1,10 @@ const { makeInterfaceId } = require('openzeppelin-test-helpers'); -const INTERFACE_IDS = { - ERC165: makeInterfaceId([ +const INTERFACES = { + ERC165: [ 'supportsInterface(bytes4)', - ]), - ERC721: makeInterfaceId([ + ], + ERC721: [ 'balanceOf(address)', 'ownerOf(uint256)', 'approve(address,uint256)', @@ -14,38 +14,56 @@ const INTERFACE_IDS = { 'transferFrom(address,address,uint256)', 'safeTransferFrom(address,address,uint256)', 'safeTransferFrom(address,address,uint256,bytes)', - ]), - ERC721Enumerable: makeInterfaceId([ + ], + ERC721Enumerable: [ 'totalSupply()', 'tokenOfOwnerByIndex(address,uint256)', 'tokenByIndex(uint256)', - ]), - ERC721Metadata: makeInterfaceId([ + ], + ERC721Metadata: [ 'name()', 'symbol()', 'tokenURI(uint256)', - ]), - ERC721Exists: makeInterfaceId([ - 'exists(uint256)', - ]), + ], }; +const INTERFACE_IDS = {}; +const FN_SIGNATURES = {}; +for (const k of Object.getOwnPropertyNames(INTERFACES)) { + INTERFACE_IDS[k] = makeInterfaceId(INTERFACES[k]); + for (const fnName of INTERFACES[k]) { + // the interface id of a single function is equivalent to its function signature + FN_SIGNATURES[fnName] = makeInterfaceId([fnName]); + } +} + function shouldSupportInterfaces (interfaces = []) { - describe('ERC165\'s supportsInterface(bytes4)', function () { + describe('Contract interface', function () { beforeEach(function () { - this.thing = this.mock || this.token; + this.contractUnderTest = this.mock || this.token; }); for (const k of interfaces) { const interfaceId = INTERFACE_IDS[k]; describe(k, function () { - it('should use less than 30k gas', async function () { - (await this.thing.supportsInterface.estimateGas(interfaceId)).should.be.lte(30000); + describe('ERC165\'s supportsInterface(bytes4)', function () { + it('should use less than 30k gas', async function () { + (await this.contractUnderTest.supportsInterface.estimateGas(interfaceId)).should.be.lte(30000); + }); + + it('should claim support', async function () { + (await this.contractUnderTest.supportsInterface(interfaceId)).should.equal(true); + }); }); - it('is supported', async function () { - (await this.thing.supportsInterface(interfaceId)).should.equal(true); - }); + for (const fnName of INTERFACES[k]) { + const fnSig = FN_SIGNATURES[fnName]; + describe(fnName, function () { + it('should be implemented', function () { + this.contractUnderTest.abi.filter(fn => fn.signature === fnSig).length.should.equal(1); + }); + }); + } }); } });