Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove tokenOfOwnerByIndex #107

Merged
merged 4 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 2 additions & 63 deletions contracts/ERC721A.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ error URIQueryForNonexistentToken();
*
* Assumes that the maximum token id cannot exceed 2**128 - 1 (max value of uint128).
*/
contract ERC721A is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable {
contract ERC721A is Context, ERC165, IERC721, IERC721Metadata {
using Address for address;
using Strings for uint256;

Expand Down Expand Up @@ -100,82 +100,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);
}

Expand Down
5 changes: 0 additions & 5 deletions test/ERC721A.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
38 changes: 0 additions & 38 deletions test/extensions/ERC721ABurnable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down