From cbcd99b354472ec4484f753e0583d4fc86c89843 Mon Sep 17 00:00:00 2001 From: William Entriken Date: Wed, 16 Feb 2022 12:03:18 -0500 Subject: [PATCH] Remove tokenOfOwnerByIndex (#107) * Remove tokenOfOwnerByIndex * Drop tokenByIndex * Remove tokenByIndex test * ERC-165 stop advertising ERC721Enumerable --- contracts/ERC721A.sol | 65 +------------------------ test/ERC721A.test.js | 5 -- test/extensions/ERC721ABurnable.test.js | 38 --------------- 3 files changed, 2 insertions(+), 106 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index dc8bc2bb3..219d25998 100644 --- 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,7 +97,7 @@ 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 { @@ -105,66 +105,6 @@ contract ERC721A is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } } - /** - * @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}. */ @@ -172,7 +112,6 @@ contract ERC721A is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable 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 index b55bb59a6..d059175f4 100644 --- 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 index 6bd82bcf5..76343a1fd 100644 --- 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;