Remove tokenOfOwnerByIndex (#107)

* Remove tokenOfOwnerByIndex * Drop tokenByIndex * Remove tokenByIndex test * ERC-165 stop advertising ERC721Enumerable
chiru-labs · Feb 16, 2022 · cbcd99b · cbcd99b
1 parent f3205a7
commit cbcd99b
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 106 deletions.
diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol
@@ -40,7 +40,7 @@ error URIQueryForNonexistentToken();
  *
  * Assumes that the maximum token id cannot exceed 2**256 - 1 (max value of uint256).
  */
-contract ERC721A is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable {
+contract ERC721A is Context, ERC165, IERC721, IERC721Metadata {
     using Address for address;
     using Strings for uint256;
 
@@ -97,82 +97,21 @@ contract ERC721A is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
     /**
      * @dev See {IERC721Enumerable-totalSupply}.
      */
-    function totalSupply() public view override returns (uint256) {
+    function totalSupply() public view returns (uint256) {
         // Counter underflow is impossible as _burnCounter cannot be incremented
         // more than _currentIndex times
         unchecked {
             return _currentIndex - _burnCounter;    
         }
     }
 
-    /**
-     * @dev See {IERC721Enumerable-tokenByIndex}.
-     * This read function is O(totalSupply). If calling from a separate contract, be sure to test gas first.
-     * It may also degrade with extremely large collection sizes (e.g >> 10000), test for your use case.
-     */
-    function tokenByIndex(uint256 index) public view override returns (uint256) {
-        uint256 numMintedSoFar = _currentIndex;
-        uint256 tokenIdsIdx;
-
-        // Counter overflow is impossible as the loop breaks when
-        // uint256 i is equal to another uint256 numMintedSoFar.
-        unchecked {
-            for (uint256 i; i < numMintedSoFar; i++) {
-                TokenOwnership memory ownership = _ownerships[i];
-                if (!ownership.burned) {
-                    if (tokenIdsIdx == index) {
-                        return i;
-                    }
-                    tokenIdsIdx++;
-                }
-            }
-        }
-        revert TokenIndexOutOfBounds();
-    }
-
-    /**
-     * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
-     * This read function is O(totalSupply). If calling from a separate contract, be sure to test gas first.
-     * It may also degrade with extremely large collection sizes (e.g >> 10000), test for your use case.
-     */
-    function tokenOfOwnerByIndex(address owner, uint256 index) public view override returns (uint256) {
-        if (index >= balanceOf(owner)) revert OwnerIndexOutOfBounds();
-        uint256 numMintedSoFar = _currentIndex;
-        uint256 tokenIdsIdx;
-        address currOwnershipAddr;
-
-        // Counter overflow is impossible as the loop breaks when
-        // uint256 i is equal to another uint256 numMintedSoFar.
-        unchecked {
-            for (uint256 i; i < numMintedSoFar; i++) {
-                TokenOwnership memory ownership = _ownerships[i];
-                if (ownership.burned) {
-                    continue;
-                }
-                if (ownership.addr != address(0)) {
-                    currOwnershipAddr = ownership.addr;
-                }
-                if (currOwnershipAddr == owner) {
-                    if (tokenIdsIdx == index) {
-                        return i;
-                    }
-                    tokenIdsIdx++;
-                }
-            }
-        }
-
-        // Execution should never reach this point.
-        revert();
-    }
-
     /**
      * @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/ERC721A.test.js b/test/ERC721A.test.js
@@ -157,11 +157,6 @@ describe('ERC721A', function () {
         it('adjusts owners balances', async function () {
           expect(await this.erc721a.balanceOf(from)).to.be.equal(1);
         });
-
-        it('adjusts owners tokens by index', async function () {
-          expect(await this.erc721a.tokenOfOwnerByIndex(to, 0)).to.be.equal(tokenId);
-          expect(await this.erc721a.tokenOfOwnerByIndex(from, 0)).to.be.not.equal(tokenId);
-        });
       };
 
       const testUnsuccessfulTransfer = function (transferFn) {

diff --git a/test/extensions/ERC721ABurnable.test.js b/test/extensions/ERC721ABurnable.test.js
@@ -53,49 +53,11 @@ describe('ERC721ABurnable', function () {
     }
   })
 
-  it('adjusts owners tokens by index', async function () {
-    const n = await this.token.totalSupply();
-    for (let i = 0; i < this.burnedTokenId; ++i) {
-      expect(await this.token.tokenByIndex(i)).to.be.equal(i);
-    }
-    for (let i = this.burnedTokenId; i < n; ++i) {
-      expect(await this.token.tokenByIndex(i)).to.be.equal(i + 1);
-    }
-    // tokenIds of addr1: [0,1,2,3,4,6,7,8,9]
-    expect(await this.token.tokenOfOwnerByIndex(this.addr1.address, 2))
-      .to.be.equal(2);
-    await this.token.connect(this.addr1).burn(2);
-    // tokenIds of addr1: [0,1,3,4,6,7,8,9]
-    expect(await this.token.tokenOfOwnerByIndex(this.addr1.address, 2))
-      .to.be.equal(3);
-    await this.token.connect(this.addr1).burn(0);
-    // tokenIds of addr1: [1,3,4,6,7,8,9]
-    expect(await this.token.tokenOfOwnerByIndex(this.addr1.address, 2))
-      .to.be.equal(4);
-    await this.token.connect(this.addr1).burn(3);
-    // tokenIds of addr1: [1,4,6,7,8,9]
-    expect(await this.token.tokenOfOwnerByIndex(this.addr1.address, 2))
-      .to.be.equal(6);
-  })
-
   it('adjusts owners balances', async function () {
     expect(await this.token.balanceOf(this.addr1.address))
       .to.be.equal(this.numTestTokens - 1);
   });
 
-  it('adjusts token by index', async function () {
-    const n = await this.token.totalSupply();
-    for (let i = 0; i < this.burnedTokenId; ++i) {
-      expect(await this.token.tokenByIndex(i)).to.be.equal(i);
-    }
-    for (let i = this.burnedTokenId; i < n; ++i) {
-      expect(await this.token.tokenByIndex(i)).to.be.equal(i + 1);
-    }
-    await expect(this.token.tokenByIndex(n)).to.be.revertedWith(
-      'TokenIndexOutOfBounds'
-    );
-  });
-
   describe('ownerships correctly set', async function () {
     it('with token before previously burnt token transfered and burned', async function () {
       const tokenIdToBurn = this.burnedTokenId - 1;