From 81b0331d01a2ac9fce4a243b712a389f630d5dea Mon Sep 17 00:00:00 2001 From: cygaar Date: Mon, 23 May 2022 21:23:07 -0500 Subject: [PATCH 1/2] Remove setOwnersExplicit --- .../extensions/ERC721AOwnersExplicit.sol | 66 --------- .../ERC721ABurnableOwnersExplicitMock.sol | 24 --- contracts/mocks/ERC721AMock.sol | 6 +- contracts/mocks/ERC721AOwnersExplicitMock.sol | 27 ---- .../ERC721AOwnersExplicitStartTokenIdMock.sol | 20 --- .../ERC721AQueryableOwnersExplicitMock.sol | 20 --- test/ERC721A.test.js | 15 ++ .../ERC721ABurnableOwnersExplicit.test.js | 44 ------ test/extensions/ERC721AOwnersExplicit.test.js | 137 ------------------ test/extensions/ERC721AQueryable.test.js | 9 -- 10 files changed, 20 insertions(+), 348 deletions(-) delete mode 100644 contracts/extensions/ERC721AOwnersExplicit.sol delete mode 100644 contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol delete mode 100644 contracts/mocks/ERC721AOwnersExplicitMock.sol delete mode 100644 contracts/mocks/ERC721AOwnersExplicitStartTokenIdMock.sol delete mode 100644 contracts/mocks/ERC721AQueryableOwnersExplicitMock.sol delete mode 100644 test/extensions/ERC721ABurnableOwnersExplicit.test.js delete mode 100644 test/extensions/ERC721AOwnersExplicit.test.js diff --git a/contracts/extensions/ERC721AOwnersExplicit.sol b/contracts/extensions/ERC721AOwnersExplicit.sol deleted file mode 100644 index 9b7d3d4b2..000000000 --- a/contracts/extensions/ERC721AOwnersExplicit.sol +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v3.3.0 -// Creator: Chiru Labs - -pragma solidity ^0.8.4; - -import '../ERC721A.sol'; - -abstract contract ERC721AOwnersExplicit is ERC721A { - /** - * No more ownership slots to explicity initialize. - */ - error AllOwnershipsInitialized(); - - /** - * The `quantity` must be more than zero. - */ - error InitializeZeroQuantity(); - - /** - * At least one token needs to be minted. - */ - error NoTokensMintedYet(); - - // The next token ID to explicity initialize ownership data. - uint256 private _currentIndexOwnersExplicit; - - /** - * @dev Returns the next token ID to be explicity initialized. - */ - function _nextTokenIdOwnersExplicit() internal view returns (uint256) { - uint256 tokenId = _currentIndexOwnersExplicit; - if (tokenId == 0) { - tokenId = _startTokenId(); - } - return tokenId; - } - - /** - * @dev Explicitly initialize ownerships to eliminate loops in future calls of `ownerOf()`. - */ - function _initializeOwnersExplicit(uint256 quantity) internal { - if (quantity == 0) revert InitializeZeroQuantity(); - uint256 stopLimit = _nextTokenId(); - if (stopLimit == _startTokenId()) revert NoTokensMintedYet(); - uint256 start = _nextTokenIdOwnersExplicit(); - if (start >= stopLimit) revert AllOwnershipsInitialized(); - - // Index underflow is impossible. - // Counter or index overflow is incredibly unrealistic. - unchecked { - uint256 stop = start + quantity; - - // Set the end index to be the last token index - if (stop > stopLimit) { - stop = stopLimit; - } - - for (uint256 i = start; i < stop; i++) { - _initializeOwnershipAt(i); - } - - _currentIndexOwnersExplicit = stop; - } - } -} diff --git a/contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol b/contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol deleted file mode 100644 index bfbdb95a2..000000000 --- a/contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v3.3.0 -// Creators: Chiru Labs - -pragma solidity ^0.8.4; - -import '../extensions/ERC721ABurnable.sol'; -import '../extensions/ERC721AOwnersExplicit.sol'; - -contract ERC721ABurnableOwnersExplicitMock is ERC721A, ERC721ABurnable, ERC721AOwnersExplicit { - constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} - - function safeMint(address to, uint256 quantity) public { - _safeMint(to, quantity); - } - - function initializeOwnersExplicit(uint256 quantity) public { - _initializeOwnersExplicit(quantity); - } - - function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) { - return _ownershipAt(index); - } -} diff --git a/contracts/mocks/ERC721AMock.sol b/contracts/mocks/ERC721AMock.sol index 4c33704b5..85c069c43 100644 --- a/contracts/mocks/ERC721AMock.sol +++ b/contracts/mocks/ERC721AMock.sol @@ -73,7 +73,11 @@ contract ERC721AMock is ERC721A { return _ownershipAt(index); } - function getOwnershipOf(uint index) public view returns (TokenOwnership memory) { + function getOwnershipOf(uint256 index) public view returns (TokenOwnership memory) { return _ownershipOf(index); } + + function initializeOwnershipAt(uint256 tokenId) public { + _initializeOwnershipAt(tokenId); + } } diff --git a/contracts/mocks/ERC721AOwnersExplicitMock.sol b/contracts/mocks/ERC721AOwnersExplicitMock.sol deleted file mode 100644 index e1c3bbe0a..000000000 --- a/contracts/mocks/ERC721AOwnersExplicitMock.sol +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v3.3.0 -// Creators: Chiru Labs - -pragma solidity ^0.8.4; - -import '../extensions/ERC721AOwnersExplicit.sol'; - -contract ERC721AOwnersExplicitMock is ERC721AOwnersExplicit { - constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} - - function safeMint(address to, uint256 quantity) public { - _safeMint(to, quantity); - } - - function initializeOwnersExplicit(uint256 quantity) public { - _initializeOwnersExplicit(quantity); - } - - function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) { - return _ownershipAt(index); - } - - function nextTokenIdOwnersExplicit() public view returns (uint256) { - return _nextTokenIdOwnersExplicit(); - } -} diff --git a/contracts/mocks/ERC721AOwnersExplicitStartTokenIdMock.sol b/contracts/mocks/ERC721AOwnersExplicitStartTokenIdMock.sol deleted file mode 100644 index eb93a2651..000000000 --- a/contracts/mocks/ERC721AOwnersExplicitStartTokenIdMock.sol +++ /dev/null @@ -1,20 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v3.3.0 -// Creators: Chiru Labs - -pragma solidity ^0.8.4; - -import './ERC721AOwnersExplicitMock.sol'; -import './StartTokenIdHelper.sol'; - -contract ERC721AOwnersExplicitStartTokenIdMock is StartTokenIdHelper, ERC721AOwnersExplicitMock { - constructor( - string memory name_, - string memory symbol_, - uint256 startTokenId_ - ) StartTokenIdHelper(startTokenId_) ERC721AOwnersExplicitMock(name_, symbol_) {} - - function _startTokenId() internal view override returns (uint256) { - return startTokenId; - } -} diff --git a/contracts/mocks/ERC721AQueryableOwnersExplicitMock.sol b/contracts/mocks/ERC721AQueryableOwnersExplicitMock.sol deleted file mode 100644 index 3ff3211fb..000000000 --- a/contracts/mocks/ERC721AQueryableOwnersExplicitMock.sol +++ /dev/null @@ -1,20 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v3.3.0 -// Creators: Chiru Labs - -pragma solidity ^0.8.4; - -import './ERC721AQueryableMock.sol'; -import '../extensions/ERC721AOwnersExplicit.sol'; - -contract ERC721AQueryableOwnersExplicitMock is ERC721AQueryableMock, ERC721AOwnersExplicit { - constructor(string memory name_, string memory symbol_) ERC721AQueryableMock(name_, symbol_) {} - - function initializeOwnersExplicit(uint256 quantity) public { - _initializeOwnersExplicit(quantity); - } - - function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) { - return _ownershipAt(index); - } -} diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js index 1c25de0d2..cdd72313d 100644 --- a/test/ERC721A.test.js +++ b/test/ERC721A.test.js @@ -454,6 +454,21 @@ const createTestSuite = ({ contract, constructorArgs }) => expect(await this.erc721a.exists(this.tokenIdToBurn)).to.be.false; }); }); + + describe('_initializeOwnershipAt', async function () { + it('successfuly sets ownership of empty slot', async function () { + const lastTokenId = this.addr3.expected.tokens[2]; + expect(await this.erc721a.ownerOf(lastTokenId)).to.be.equal(this.addr3.address); + const tx1 = await this.erc721a.initializeOwnershipAt(lastTokenId); + expect(await this.erc721a.ownerOf(lastTokenId)).to.be.equal(this.addr3.address); + const tx2 = await this.erc721a.initializeOwnershipAt(lastTokenId); + + // We assume the initialization worked due to less gas used by the 2nd txn + const receipt1 = await tx1.wait(); + const receipt2 = await tx2.wait(); + expect(receipt1.gasUsed.toNumber()).to.be.greaterThan(receipt2.gasUsed.toNumber()); + }); + }); }); context('test mint functionality', function () { diff --git a/test/extensions/ERC721ABurnableOwnersExplicit.test.js b/test/extensions/ERC721ABurnableOwnersExplicit.test.js deleted file mode 100644 index bc6f9f56b..000000000 --- a/test/extensions/ERC721ABurnableOwnersExplicit.test.js +++ /dev/null @@ -1,44 +0,0 @@ -const { deployContract } = require('../helpers.js'); -const { expect } = require('chai'); -const { constants } = require('@openzeppelin/test-helpers'); -const { ZERO_ADDRESS } = constants; - -describe('ERC721ABurnableOwnersExplicit', function () { - beforeEach(async function () { - this.token = await deployContract('ERC721ABurnableOwnersExplicitMock', ['Azuki', 'AZUKI']); - }); - - beforeEach(async function () { - const [owner, addr1, addr2, addr3] = await ethers.getSigners(); - this.owner = owner; - this.addr1 = addr1; - this.addr2 = addr2; - this.addr3 = addr3; - await this.token['safeMint(address,uint256)'](addr1.address, 1); - await this.token['safeMint(address,uint256)'](addr2.address, 2); - await this.token['safeMint(address,uint256)'](addr3.address, 3); - await this.token.connect(this.addr1).burn(0); - await this.token.connect(this.addr3).burn(4); - await this.token.initializeOwnersExplicit(6); - }); - - it('ownerships correctly initialized', async function () { - for (let tokenId = 0; tokenId < 6; tokenId++) { - let owner = await this.token.getOwnershipAt(tokenId); - expect(owner[0]).to.not.equal(ZERO_ADDRESS); - if (tokenId == 0 || tokenId == 4) { - expect(owner[2]).to.equal(true); - await expect(this.token.ownerOf(tokenId)).to.be.revertedWith( - 'OwnerQueryForNonexistentToken' - ) - } else { - expect(owner[2]).to.equal(false); - if (tokenId < 1+2) { - expect(await this.token.ownerOf(tokenId)).to.be.equal(this.addr2.address); - } else { - expect(await this.token.ownerOf(tokenId)).to.be.equal(this.addr3.address); - } - } - } - }); -}); diff --git a/test/extensions/ERC721AOwnersExplicit.test.js b/test/extensions/ERC721AOwnersExplicit.test.js deleted file mode 100644 index 54b618b5f..000000000 --- a/test/extensions/ERC721AOwnersExplicit.test.js +++ /dev/null @@ -1,137 +0,0 @@ -const { deployContract, getBlockTimestamp, mineBlockTimestamp } = require('../helpers.js'); -const { expect } = require('chai'); -const { constants } = require('@openzeppelin/test-helpers'); -const { ZERO_ADDRESS } = constants; - -const createTestSuite = ({ contract, constructorArgs }) => - function () { - context(`${contract}`, function () { - beforeEach(async function () { - this.erc721aOwnersExplicit = await deployContract(contract, constructorArgs); - - this.startTokenId = this.erc721aOwnersExplicit.startTokenId - ? (await this.erc721aOwnersExplicit.startTokenId()).toNumber() - : 0; - }); - - context('with no minted tokens', async function () { - it('does not have enough tokens minted', async function () { - await expect(this.erc721aOwnersExplicit.initializeOwnersExplicit(1)).to.be.revertedWith( - 'NoTokensMintedYet' - ); - }); - }); - - context('with minted tokens', async function () { - beforeEach(async function () { - const [owner, addr1, addr2, addr3] = await ethers.getSigners(); - this.owner = owner; - this.addr1 = addr1; - this.addr2 = addr2; - this.addr3 = addr3; - - this.timestampBefore = await getBlockTimestamp(); - this.timestampToMine = (this.timestampBefore) + 100; - await mineBlockTimestamp(this.timestampToMine); - this.timestampMined = await getBlockTimestamp(); - - // After the following mints, our ownership array will look like this: - // | 1 | 2 | Empty | 3 | Empty | Empty | - await this.erc721aOwnersExplicit['safeMint(address,uint256)'](addr1.address, 1); - await this.erc721aOwnersExplicit['safeMint(address,uint256)'](addr2.address, 2); - await this.erc721aOwnersExplicit['safeMint(address,uint256)'](addr3.address, 3); - }); - - describe('initializeOwnersExplicit', async function () { - it('rejects 0 quantity', async function () { - await expect(this.erc721aOwnersExplicit.initializeOwnersExplicit(0)).to.be.revertedWith( - 'InitializeZeroQuantity' - ); - }); - - it('handles single increment properly', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(1); - expect(await this.erc721aOwnersExplicit.nextTokenIdOwnersExplicit()).to.equal( - (1 + this.startTokenId).toString() - ); - }); - - it('properly initializes the ownership of index 2', async function () { - let ownerAtTwo = await this.erc721aOwnersExplicit.getOwnershipAt(2 + this.startTokenId); - expect(ownerAtTwo[0]).to.equal(ZERO_ADDRESS); - await this.erc721aOwnersExplicit.initializeOwnersExplicit(3); - ownerAtTwo = await this.erc721aOwnersExplicit.getOwnershipAt(2); - expect(ownerAtTwo[0]).to.equal(this.addr2.address); - expect(await this.erc721aOwnersExplicit.nextTokenIdOwnersExplicit()).to.equal( - (3 + this.startTokenId).toString() - ); - }); - - it('initializes all ownerships in one go', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(6); - for (let tokenId = this.startTokenId; tokenId < 6 + this.startTokenId; tokenId++) { - let owner = await this.erc721aOwnersExplicit.getOwnershipAt(tokenId); - expect(owner[0]).to.not.equal(ZERO_ADDRESS); - } - }); - - it('initializes all ownerships with overflowing quantity', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(15); - for (let tokenId = this.startTokenId; tokenId < 6 + this.startTokenId; tokenId++) { - let owner = await this.erc721aOwnersExplicit.getOwnershipAt(tokenId); - expect(owner[0]).to.not.equal(ZERO_ADDRESS); - } - }); - - it('initializes all ownerships in multiple calls', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(2); - expect(await this.erc721aOwnersExplicit.nextTokenIdOwnersExplicit()).to.equal( - (2 + this.startTokenId).toString() - ); - await this.erc721aOwnersExplicit.initializeOwnersExplicit(1); - expect(await this.erc721aOwnersExplicit.nextTokenIdOwnersExplicit()).to.equal( - (3 + this.startTokenId).toString() - ); - await this.erc721aOwnersExplicit.initializeOwnersExplicit(3); - for (let tokenId = this.startTokenId; tokenId < 6 + this.startTokenId; tokenId++) { - let owner = await this.erc721aOwnersExplicit.getOwnershipAt(tokenId); - expect(owner[0]).to.not.equal(ZERO_ADDRESS); - } - }); - - it('rejects after all ownerships have been initialized', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(6); - await expect(this.erc721aOwnersExplicit.initializeOwnersExplicit(1)).to.be.revertedWith( - 'AllOwnershipsInitialized' - ); - }); - - it('initializes startTimestamps correctly', async function () { - await this.erc721aOwnersExplicit.initializeOwnersExplicit(6); - expect(this.timestampToMine).to.be.eq(this.timestampMined); - for (let tokenId = this.startTokenId; tokenId < 6 + this.startTokenId; tokenId++) { - let owner = await this.erc721aOwnersExplicit.getOwnershipAt(tokenId); - expect(owner.startTimestamp).to.be.gt(this.timestampBefore); - expect(owner.startTimestamp).to.be.gte(this.timestampToMine); - } - }); - }); - }); - }); - }; - -describe( - 'ERC721AOwnersExplicit', - createTestSuite({ - contract: 'ERC721AOwnersExplicitMock', - constructorArgs: ['Azuki', 'AZUKI'], - }) -); - -describe( - 'ERC721AOwnersExplicit override _startTokenId()', - createTestSuite({ - contract: 'ERC721AOwnersExplicitStartTokenIdMock', - constructorArgs: ['Azuki', 'AZUKI', 1], - }) -); diff --git a/test/extensions/ERC721AQueryable.test.js b/test/extensions/ERC721AQueryable.test.js index c5733bc6d..9665af134 100644 --- a/test/extensions/ERC721AQueryable.test.js +++ b/test/extensions/ERC721AQueryable.test.js @@ -311,12 +311,3 @@ describe( constructorArgs: ['Azuki', 'AZUKI', 1], }) ); - -describe( - 'ERC721AQueryableOwnersExplicit', - createTestSuite({ - contract: 'ERC721AQueryableOwnersExplicitMock', - constructorArgs: ['Azuki', 'AZUKI'], - initializeOwnersExplicit: true, - }) -); From 855abfeaf9eafdea73a8874cfe8451f5a77994a1 Mon Sep 17 00:00:00 2001 From: cygaar Date: Mon, 23 May 2022 22:10:47 -0500 Subject: [PATCH 2/2] Update test --- contracts/mocks/ERC721AMock.sol | 4 ++-- test/ERC721A.test.js | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/contracts/mocks/ERC721AMock.sol b/contracts/mocks/ERC721AMock.sol index 85c069c43..66fd299ed 100644 --- a/contracts/mocks/ERC721AMock.sol +++ b/contracts/mocks/ERC721AMock.sol @@ -77,7 +77,7 @@ contract ERC721AMock is ERC721A { return _ownershipOf(index); } - function initializeOwnershipAt(uint256 tokenId) public { - _initializeOwnershipAt(tokenId); + function initializeOwnershipAt(uint256 index) public { + _initializeOwnershipAt(index); } } diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js index cdd72313d..6faf1c1aa 100644 --- a/test/ERC721A.test.js +++ b/test/ERC721A.test.js @@ -455,18 +455,14 @@ const createTestSuite = ({ contract, constructorArgs }) => }); }); - describe('_initializeOwnershipAt', async function () { + describe.only('_initializeOwnershipAt', async function () { it('successfuly sets ownership of empty slot', async function () { const lastTokenId = this.addr3.expected.tokens[2]; - expect(await this.erc721a.ownerOf(lastTokenId)).to.be.equal(this.addr3.address); - const tx1 = await this.erc721a.initializeOwnershipAt(lastTokenId); - expect(await this.erc721a.ownerOf(lastTokenId)).to.be.equal(this.addr3.address); - const tx2 = await this.erc721a.initializeOwnershipAt(lastTokenId); - - // We assume the initialization worked due to less gas used by the 2nd txn - const receipt1 = await tx1.wait(); - const receipt2 = await tx2.wait(); - expect(receipt1.gasUsed.toNumber()).to.be.greaterThan(receipt2.gasUsed.toNumber()); + const ownership1 = await this.erc721a.getOwnershipAt(lastTokenId); + expect(ownership1[0]).to.equal(ZERO_ADDRESS); + await this.erc721a.initializeOwnershipAt(lastTokenId); + const ownership2 = await this.erc721a.getOwnershipAt(lastTokenId); + expect(ownership2[0]).to.equal(this.addr3.address); }); }); });