add 165 to 721 (#972)

* make _tokenId indexed in Transfer and Approval events

via: https://github.com/ethereum/EIPs/pull/1124/files

* fix: make name() and symbol() external instead of public

* feat: implement ERC721's ERC165

* feat: erc165 tests

* fix: don't use chai-as-promised in direct await

* fix: reorganize to /introspection

* feat: abstract all erc165 tests to a behavior

* feat: disallow registering 0xffffffff

contracts/introspection/ERC165.sol

+pragma solidity ^0.4.23;
+
+
+/**
+ * @title ERC165
+ * @dev https://github.com/ethereum/EIPs/blob/master/EIPS/eip-165.md
+ */
+interface ERC165 {
+
+  /**
+   * @notice Query if a contract implements an interface
+   * @param _interfaceId The interface identifier, as specified in ERC-165
+   * @dev Interface identification is specified in ERC-165. This function
+   * @dev  uses less than 30,000 gas.
+   */
+  function supportsInterface(bytes4 _interfaceId)
+    external
+    view
+    returns (bool);
+}

contracts/introspection/SupportsInterfaceWithLookup.sol

+pragma solidity ^0.4.23;
+
+import "./ERC165.sol";
+
+
+/**
+ * @title SupportsInterfaceWithLookup
+ * @author Matt Condon (@shrugs)
+ * @dev Implements ERC165 using a lookup table.
+ */
+contract SupportsInterfaceWithLookup is ERC165 {
+  bytes4 public constant InterfaceId_ERC165 = 0x01ffc9a7;
+  /**
+   * 0x01ffc9a7 ===
+   *   bytes4(keccak256('supportsInterface(bytes4)'))
+   */
+
+  /**
+   * @dev a mapping of interface id to whether or not it's supported
+   */
+  mapping(bytes4 => bool) internal supportedInterfaces;
+
+  /**
+   * @dev A contract implementing SupportsInterfaceWithLookup
+   * @dev  implement ERC165 itself
+   */
+  constructor()
+    public
+  {
+    _registerInterface(InterfaceId_ERC165);
+  }
+
+  /**
+   * @dev implement supportsInterface(bytes4) using a lookup table
+   */
+  function supportsInterface(bytes4 _interfaceId)
+    external
+    view
+    returns (bool)
+  {
+    return supportedInterfaces[_interfaceId];
+  }
+
+  /**
+   * @dev private method for registering an interface
+   */
+  function _registerInterface(bytes4 _interfaceId)
+    internal
+  {
+    require(_interfaceId != 0xffffffff);
+    supportedInterfaces[_interfaceId] = true;
+  }
+}

contracts/mocks/SupportsInterfaceWithLookupMock.sol

+pragma solidity ^0.4.23;
+
+import "../introspection/SupportsInterfaceWithLookup.sol";
+
+
+contract SupportsInterfaceWithLookupMock is SupportsInterfaceWithLookup {
+  function registerInterface(bytes4 _interfaceId)
+    public
+  {
+    _registerInterface(_interfaceId);
+  }
+}

contracts/token/ERC721/ERC721.sol

@@ -8,6 +8,21 @@ import "./ERC721Basic.sol";
  * @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
  */
 contract ERC721Enumerable is ERC721Basic {
+
+  bytes4 private constant InterfaceId_ERC721Enumerable = 0x780e9d63;
+  /**
+   * 0x780e9d63 ===
+   *   bytes4(keccak256('totalSupply()')) ^
+   *   bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) ^
+   *   bytes4(keccak256('tokenByIndex(uint256)'))
+   */
+
+  constructor()
+    public
+  {
+    _registerInterface(InterfaceId_ERC721Enumerable);
+  }
+
   function totalSupply() public view returns (uint256);
   function tokenOfOwnerByIndex(
     address _owner,
@@ -26,8 +41,23 @@ contract ERC721Enumerable is ERC721Basic {
  * @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
  */
 contract ERC721Metadata is ERC721Basic {
-  function name() public view returns (string _name);
-  function symbol() public view returns (string _symbol);
+
+  bytes4 private constant InterfaceId_ERC721Metadata = 0x5b5e139f;
+  /**
+   * 0x5b5e139f ===
+   *   bytes4(keccak256('name()')) ^
+   *   bytes4(keccak256('symbol()')) ^
+   *   bytes4(keccak256('tokenURI(uint256)'))
+   */
+
+  constructor()
+    public
+  {
+    _registerInterface(InterfaceId_ERC721Metadata);
+  }
+
+  function name() external view returns (string _name);
+  function symbol() external view returns (string _symbol);
   function tokenURI(uint256 _tokenId) public view returns (string);
 }
 

contracts/token/ERC721/ERC721Basic.sol

 pragma solidity ^0.4.23;
 
+import "../../introspection/SupportsInterfaceWithLookup.sol";
+
 
 /**
  * @title ERC721 Non-Fungible Token Standard basic interface
  * @dev see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
  */
-contract ERC721Basic {
+contract ERC721Basic is SupportsInterfaceWithLookup {
   event Transfer(
     address indexed _from,
     address indexed _to,
-    uint256 _tokenId
+    uint256 indexed _tokenId
   );
   event Approval(
     address indexed _owner,
     address indexed _approved,
-    uint256 _tokenId
+    uint256 indexed _tokenId
   );
   event ApprovalForAll(
     address indexed _owner,
@@ -22,6 +24,34 @@ contract ERC721Basic {
     bool _approved
   );
 
+  bytes4 private constant InterfaceId_ERC721 = 0x80ac58cd;
+  /*
+   * 0x80ac58cd ===
+   *   bytes4(keccak256('balanceOf(address)')) ^
+   *   bytes4(keccak256('ownerOf(uint256)')) ^
+   *   bytes4(keccak256('approve(address,uint256)')) ^
+   *   bytes4(keccak256('getApproved(uint256)')) ^
+   *   bytes4(keccak256('setApprovalForAll(address,bool)')) ^
+   *   bytes4(keccak256('isApprovedForAll(address,address)')) ^
+   *   bytes4(keccak256('transferFrom(address,address,uint256)')) ^
+   *   bytes4(keccak256('safeTransferFrom(address,address,uint256)')) ^
+   *   bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)'))
+   */
+
+  bytes4 private constant InterfaceId_ERC721Exists = 0x4f558e79;
+  /*
+   * 0x4f558e79 ===
+   *   bytes4(keccak256('exists(uint256)'))
+   */
+
+  constructor ()
+    public
+  {
+    // register the supported interfaces to conform to ERC721 via ERC165
+    _registerInterface(InterfaceId_ERC721);
+    _registerInterface(InterfaceId_ERC721Exists);
+  }
+
   function balanceOf(address _owner) public view returns (uint256 _balance);
   function ownerOf(uint256 _tokenId) public view returns (address _owner);
   function exists(uint256 _tokenId) public view returns (bool _exists);

contracts/token/ERC721/ERC721BasicToken.sol

@@ -16,7 +16,7 @@ contract ERC721BasicToken is ERC721Basic {
 
   // Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`
   // which can be also obtained as `ERC721Receiver(0).onERC721Received.selector`
-  bytes4 constant ERC721_RECEIVED = 0xf0b9e5ba;
+  bytes4 private constant ERC721_RECEIVED = 0xf0b9e5ba;
 
   // Mapping from token ID to owner
   mapping (uint256 => address) internal tokenOwner;

contracts/token/ERC721/ERC721Receiver.sol

@@ -12,7 +12,7 @@ contract ERC721Receiver {
    *  Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`,
    *  which can be also obtained as `ERC721Receiver(0).onERC721Received.selector`
    */
-  bytes4 constant ERC721_RECEIVED = 0xf0b9e5ba;
+  bytes4 internal constant ERC721_RECEIVED = 0xf0b9e5ba;
 
   /**
    * @notice Handle the receipt of an NFT

contracts/token/ERC721/ERC721Token.sol

@@ -44,7 +44,7 @@ contract ERC721Token is ERC721, ERC721BasicToken {
    * @dev Gets the token name
    * @return string representing the token name
    */
-  function name() public view returns (string) {
+  function name() external view returns (string) {
     return name_;
   }
 
@@ -52,7 +52,7 @@ contract ERC721Token is ERC721, ERC721BasicToken {
    * @dev Gets the token symbol
    * @return string representing the token symbol
    */
-  function symbol() public view returns (string) {
+  function symbol() external view returns (string) {
     return symbol_;
   }
 

package.json

@@ -59,6 +59,7 @@
     "solidity-coverage": "^0.5.0",
     "solium": "^1.1.7",
     "truffle": "^4.1.8",
-    "truffle-hdwallet-provider": "0.0.3"
+    "truffle-hdwallet-provider": "0.0.3",
+    "web3-utils": "^1.0.0-beta.34"
   }
 }

test/helpers/makeInterfaceId.js

+import { soliditySha3 } from 'web3-utils';
+
+const INTERFACE_ID_LENGTH = 4;
+
+export default (interfaces = []) => {
+  const interfaceIdBuffer = interfaces
+    .map(methodSignature => soliditySha3(methodSignature)) // keccak256
+    .map(h =>
+      Buffer
+        .from(h.substring(2), 'hex')
+        .slice(0, 4) // bytes4()
+    )
+    .reduce((memo, bytes) => {
+      for (let i = 0; i < INTERFACE_ID_LENGTH; i++) {
+        memo[i] = memo[i] ^ bytes[i]; // xor
+      }
+      return memo;
+    }, Buffer.alloc(INTERFACE_ID_LENGTH));
+
+  return `0x${interfaceIdBuffer.toString('hex')}`;
+};

test/introspection/SupportsInterface.behavior.js

+import makeInterfaceId from '../helpers/makeInterfaceId';
+
+const INTERFACE_IDS = {
+  ERC165: makeInterfaceId([
+    'supportsInterface(bytes4)',
+  ]),
+  ERC721: makeInterfaceId([
+    'balanceOf(address)',
+    'ownerOf(uint256)',
+    'approve(address,uint256)',
+    'getApproved(uint256)',
+    'setApprovalForAll(address,bool)',
+    'isApprovedForAll(address,address)',
+    'transferFrom(address,address,uint256)',
+    'safeTransferFrom(address,address,uint256)',
+    'safeTransferFrom(address,address,uint256,bytes)',
+  ]),
+  ERC721Enumerable: makeInterfaceId([
+    'totalSupply()',
+    'tokenOfOwnerByIndex(address,uint256)',
+    'tokenByIndex(uint256)',
+  ]),
+  ERC721Metadata: makeInterfaceId([
+    'name()',
+    'symbol()',
+    'tokenURI(uint256)',
+  ]),
+  ERC721Exists: makeInterfaceId([
+    'exists(uint256)',
+  ]),
+};
+
+export default function (interfaces = []) {
+  describe('ERC165\'s supportsInterface(bytes4)', function () {
+    beforeEach(function () {
+      this.thing = this.mock || this.token;
+    });
+
+    for (let k of interfaces) {
+      const interfaceId = INTERFACE_IDS[k];
+      describe(k, function () {
+        it('should use less than 30k gas', async function () {
+          const gasEstimate = await this.thing.supportsInterface.estimateGas(interfaceId);
+          gasEstimate.should.be.lte(30000);
+        });
+
+        it('is supported', async function () {
+          const isSupported = await this.thing.supportsInterface(interfaceId);
+          isSupported.should.eq(true);
+        });
+      });
+    }
+  });
+}

test/introspection/SupportsInterfaceWithLookup.test.js

+import shouldSupportInterfaces from './SupportsInterface.behavior';
+import assertRevert from '../helpers/assertRevert';
+
+const SupportsInterfaceWithLookup = artifacts.require('SupportsInterfaceWithLookupMock');
+
+require('chai')
+  .use(require('chai-as-promised'))
+  .should();
+
+contract('SupportsInterfaceWithLookup', function (accounts) {
+  before(async function () {
+    this.mock = await SupportsInterfaceWithLookup.new();
+  });
+
+  it('does not allow 0xffffffff', async function () {
+    await assertRevert(
+      this.mock.registerInterface(0xffffffff)
+    );
+  });
+
+  shouldSupportInterfaces([
+    'ERC165',
+  ]);
+});

test/token/ERC721/ERC721Token.test.js

 import assertRevert from '../../helpers/assertRevert';
 import shouldBehaveLikeERC721BasicToken from './ERC721BasicToken.behaviour';
 import shouldMintAndBurnERC721Token from './ERC721MintBurn.behaviour';
+import shouldSupportInterfaces from '../../introspection/SupportsInterface.behavior';
 import _ from 'lodash';
 
 const BigNumber = web3.BigNumber;
@@ -75,7 +76,7 @@ contract('ERC721Token', function (accounts) {
         await assertRevert(this.token.tokenByIndex(0));
       });
     });
-    
+
     describe('metadata', function () {
       const sampleUri = 'mock://mytoken';
 
@@ -122,7 +123,7 @@ contract('ERC721Token', function (accounts) {
     describe('tokenOfOwnerByIndex', function () {
       const owner = creator;
       const another = accounts[1];
-        
+
       describe('when the given index is lower than the amount of tokens owned by the given address', function () {
         it('returns the token ID placed at the given index', async function () {
           const tokenId = await this.token.tokenOfOwnerByIndex(owner, 0);
@@ -178,14 +179,14 @@ contract('ERC721Token', function (accounts) {
           const owner = accounts[0];
           const newTokenId = 300;
           const anotherNewTokenId = 400;
-          
+
           await this.token.burn(tokenId, { from: owner });
           await this.token.mint(owner, newTokenId, { from: owner });
           await this.token.mint(owner, anotherNewTokenId, { from: owner });
-  
+
           const count = await this.token.totalSupply();
           count.toNumber().should.be.equal(3);
-          
+
           const tokensListed = await Promise.all(_.range(3).map(i => this.token.tokenByIndex(i)));
           const expectedTokens = _.filter(
             [firstTokenId, secondTokenId, newTokenId, anotherNewTokenId],
@@ -196,4 +197,12 @@ contract('ERC721Token', function (accounts) {
       });
     });
   });
+
+  shouldSupportInterfaces([
+    'ERC165',
+    'ERC721',
+    'ERC721Exists',
+    'ERC721Enumerable',
+    'ERC721Metadata',
+  ]);
 });