Browse Source

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
nikeshnazareth 6 years ago
parent
commit
4dd8575bb6
1 changed files with 37 additions and 19 deletions
  1. 37 19
      test/introspection/SupportsInterface.behavior.js

+ 37 - 19
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('is supported', async function () {
-          (await this.thing.supportsInterface(interfaceId)).should.equal(true);
+          it('should claim support', async function () {
+            (await this.contractUnderTest.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);
+            });
+          });
+        }
       });
     }
   });