Use uint packing instead of structs (#272)

contracts/IERC721A.sol

@@ -73,7 +73,6 @@ interface IERC721A {
      */
     error URIQueryForNonexistentToken();
 
-    // Compiler will pack this into a single 256bit word.
     struct TokenOwnership {
         // The address of the owner.
         address addr;
@@ -83,23 +82,9 @@ interface IERC721A {
         bool burned;
     }
 
-    // Compiler will pack this into a single 256bit word.
-    struct AddressData {
-        // Realistically, 2**64-1 is more than enough.
-        uint64 balance;
-        // Keeps track of mint count with minimal overhead for tokenomics.
-        uint64 numberMinted;
-        // Keeps track of burn count with minimal overhead for tokenomics.
-        uint64 numberBurned;
-        // For miscellaneous variable(s) pertaining to the address
-        // (e.g. number of whitelist mint slots used).
-        // If there are multiple variables, please pack them into a uint64.
-        uint64 aux;
-    }
-
     /**
      * @dev Returns the total amount of tokens stored by the contract.
-     * 
+     *
      * Burned tokens are calculated here, use `_totalMinted()` if you want to count just minted tokens.
      */
     function totalSupply() external view returns (uint256);

contracts/extensions/ERC721AOwnersExplicit.sol

@@ -11,12 +11,12 @@ abstract contract ERC721AOwnersExplicit is ERC721A {
      * No more ownership slots to explicity initialize.
      */
     error AllOwnershipsHaveBeenSet();
-    
+
     /**
      * The `quantity` must be more than zero.
      */
     error QuantityMustBeNonZero();
-    
+
     /**
      * At least one token needs to be minted.
      */
@@ -47,11 +47,7 @@ abstract contract ERC721AOwnersExplicit is ERC721A {
             }
 
             for (uint256 i = _nextOwnerToExplicitlySet; i <= endIndex; i++) {
-                if (_ownerships[i].addr == address(0) && !_ownerships[i].burned) {
-                    TokenOwnership memory ownership = _ownershipOf(i);
-                    _ownerships[i].addr = ownership.addr;
-                    _ownerships[i].startTimestamp = ownership.startTimestamp;
-                }
+                _initializeOwnershipAt(i);
             }
 
             nextOwnerToExplicitlySet = endIndex + 1;

contracts/extensions/ERC721AQueryable.sol

@@ -35,7 +35,7 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
         if (tokenId < _startTokenId() || tokenId >= _currentIndex) {
             return ownership;
         }
-        ownership = _ownerships[tokenId];
+        ownership = _ownershipAt(tokenId);
         if (ownership.burned) {
             return ownership;
         }
@@ -111,7 +111,7 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
                 currOwnershipAddr = ownership.addr;
             }
             for (uint256 i = start; i != stop && tokenIdsIdx != tokenIdsMaxLength; ++i) {
-                ownership = _ownerships[i];
+                ownership = _ownershipAt(i);
                 if (ownership.burned) {
                     continue;
                 }
@@ -148,7 +148,7 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
             uint256[] memory tokenIds = new uint256[](tokenIdsLength);
             TokenOwnership memory ownership;
             for (uint256 i = _startTokenId(); tokenIdsIdx != tokenIdsLength; ++i) {
-                ownership = _ownerships[i];
+                ownership = _ownershipAt(i);
                 if (ownership.burned) {
                     continue;
                 }

contracts/mocks/ERC721ABurnableMock.sol

@@ -18,10 +18,14 @@ contract ERC721ABurnableMock is ERC721A, ERC721ABurnable {
     }
 
     function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
-        return _ownerships[index];
+        return _ownershipAt(index);
     }
 
     function totalMinted() public view returns (uint256) {
         return _totalMinted();
     }
+
+    function numberBurned(address owner) public view returns (uint256) {
+        return _numberBurned(owner);
+    }
 }

contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol

@@ -23,6 +23,6 @@ contract ERC721ABurnableOwnersExplicitMock is ERC721A, ERC721ABurnable, ERC721AO
     }
 
     function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
-        return _ownerships[index];
+        return _ownershipAt(index);
     }
 }

contracts/mocks/ERC721AMock.sol

@@ -56,4 +56,8 @@ contract ERC721AMock is ERC721A {
     function toString(uint256 x) public pure returns (string memory) {
         return _toString(x);
     }
+
+    function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
+        return _ownershipAt(index);
+    }
 }

contracts/mocks/ERC721AOwnersExplicitMock.sol

@@ -17,7 +17,7 @@ contract ERC721AOwnersExplicitMock is ERC721AOwnersExplicit {
         _setOwnersExplicit(quantity);
     }
 
-    function getOwnershipAt(uint256 tokenId) public view returns (TokenOwnership memory) {
-        return _ownerships[tokenId];
+    function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
+        return _ownershipAt(index);
     }
 }

contracts/mocks/ERC721AQueryableOwnersExplicitMock.sol

@@ -15,6 +15,6 @@ contract ERC721AQueryableOwnersExplicitMock is ERC721AQueryableMock, ERC721AOwne
     }
 
     function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
-        return _ownerships[index];
+        return _ownershipAt(index);
     }
 }

test/ERC721A.test.js

@@ -1,4 +1,4 @@
-const { deployContract } = require('./helpers.js');
+const { deployContract, getBlockTimestamp, mineBlockTimestamp } = require('./helpers.js');
 const { expect } = require('chai');
 const { BigNumber } = require('ethers');
 const { constants } = require('@openzeppelin/test-helpers');
@@ -205,7 +205,17 @@ const createTestSuite = ({ contract, constructorArgs }) =>
               this.from = sender.address;
               this.to = this.receiver.address;
               await this.erc721a.connect(sender).setApprovalForAll(this.to, true);
+
+              const ownershipBefore = await this.erc721a.getOwnershipAt(this.tokenId);
+              this.timestampBefore = parseInt(ownershipBefore.startTimestamp);
+              this.timestampToMine = (await getBlockTimestamp()) + 100;
+              await mineBlockTimestamp(this.timestampToMine);
+              this.timestampMined = await getBlockTimestamp();
+
               this.transferTx = await this.erc721a.connect(sender)[transferFn](this.from, this.to, this.tokenId);
+
+              const ownershipAfter = await this.erc721a.getOwnershipAt(this.tokenId);
+              this.timestampAfter = parseInt(ownershipAfter.startTimestamp);
             });
 
             it('transfers the ownership of the given token ID to the given address', async function () {
@@ -225,6 +235,12 @@ const createTestSuite = ({ contract, constructorArgs }) =>
             it('adjusts owners balances', async function () {
               expect(await this.erc721a.balanceOf(this.from)).to.be.equal(1);
             });
+
+            it('startTimestamp updated correctly', async function () {
+              expect(this.timestampBefore).to.be.lt(this.timestampToMine);
+              expect(this.timestampAfter).to.be.gte(this.timestampToMine);
+              expect(this.timestampToMine).to.be.eq(this.timestampMined);
+            });
           };
 
           const testUnsuccessfulTransfer = function (transferFn) {

test/GasUsage.test.js

@@ -39,4 +39,15 @@ describe('ERC721A Gas Usage', function () {
       }
     });
   });
+
+  context('transferFrom', function () {
+    it('transfer to and from two addresses', async function () {
+      await this.erc721a.mintTen(this.addr1.address);
+      await this.erc721a.mintTen(this.owner.address);
+      for (let i = 0; i < 10; ++i) {
+        await this.erc721a.connect(this.addr1).transferFrom(this.addr1.address, this.owner.address, 1);
+        await this.erc721a.connect(this.owner).transferFrom(this.owner.address, this.addr1.address, 1);
+      }  
+    });
+  });
 });

test/extensions/ERC721ABurnable.test.js

@@ -1,4 +1,4 @@
-const { deployContract } = require('../helpers.js');
+const { deployContract, getBlockTimestamp, mineBlockTimestamp } = require('../helpers.js');
 const { expect } = require('chai');
 const { constants } = require('@openzeppelin/test-helpers');
 const { ZERO_ADDRESS } = constants;
@@ -44,6 +44,12 @@ const createTestSuite = ({ contract, constructorArgs }) =>
         });
       });
 
+      it('changes numberBurned', async function () {
+        expect(await this.erc721aBurnable.numberBurned(this.addr1.address)).to.equal(1);
+        await this.erc721aBurnable.connect(this.addr1).burn(this.startTokenId);
+        expect(await this.erc721aBurnable.numberBurned(this.addr1.address)).to.equal(2);
+      });
+
       it('changes exists', async function () {
         expect(await this.erc721aBurnable.exists(this.burnedTokenId)).to.be.false;
         expect(await this.erc721aBurnable.exists(this.notBurnedTokenId)).to.be.true;
@@ -106,6 +112,21 @@ const createTestSuite = ({ contract, constructorArgs }) =>
         expect(await this.erc721aBurnable.balanceOf(this.addr1.address)).to.be.equal(this.numTestTokens - 1);
       });
 
+      it('startTimestamp updated correctly', async function () {
+        const tokenIdToBurn = this.burnedTokenId + 1;
+        const ownershipBefore = await this.erc721aBurnable.getOwnershipAt(tokenIdToBurn);
+        const timestampBefore = parseInt(ownershipBefore.startTimestamp);
+        const timestampToMine = (await getBlockTimestamp()) + 100;
+        await mineBlockTimestamp(timestampToMine);
+        const timestampMined = await getBlockTimestamp();
+        await this.erc721aBurnable.connect(this.addr1).burn(tokenIdToBurn);
+        const ownershipAfter = await this.erc721aBurnable.getOwnershipAt(tokenIdToBurn);
+        const timestampAfter = parseInt(ownershipAfter.startTimestamp);
+        expect(timestampBefore).to.be.lt(timestampToMine);
+        expect(timestampAfter).to.be.gte(timestampToMine);
+        expect(timestampToMine).to.be.eq(timestampMined);
+      });
+
       describe('ownerships correctly set', async function () {
         it('with token before previously burnt token transferred and burned', async function () {
           const tokenIdToBurn = this.burnedTokenId - 1;

test/extensions/ERC721AOwnersExplicit.test.js

@@ -1,4 +1,4 @@
-const { deployContract } = require('../helpers.js');
+const { deployContract, getBlockTimestamp, mineBlockTimestamp } = require('../helpers.js');
 const { expect } = require('chai');
 const { constants } = require('@openzeppelin/test-helpers');
 const { ZERO_ADDRESS } = constants;
@@ -27,6 +27,12 @@ const createTestSuite = ({ contract, constructorArgs }) =>
           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);
@@ -95,6 +101,16 @@ const createTestSuite = ({ contract, constructorArgs }) =>
               'AllOwnershipsHaveBeenSet'
             );
           });
+
+          it('sets startTimestamps correctly', async function () {
+            await this.erc721aOwnersExplicit.setOwnersExplicit(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);
+            }
+          });
         });
       });
     });

test/extensions/ERC721AQueryable.test.js

@@ -252,8 +252,8 @@ const createTestSuite = ({ contract, constructorArgs, setOwnersExplicit = false
             const tokenIds = [].concat(this.owner.expected.tokens, this.addr3.expected.tokens);
             const explicitOwnerships = await this.erc721aQueryable.explicitOwnershipsOf(tokenIds);
             for (let i = 0; i < tokenIds.length; ++i) {
-              const tokenId = await this.erc721aQueryable.ownerOf(tokenIds[i]);
-              expectExplicitOwnershipExists(explicitOwnerships[i], tokenId);
+              const owner = await this.erc721aQueryable.ownerOf(tokenIds[i]);
+              expectExplicitOwnershipExists(explicitOwnerships[i], owner);
             }
           });
 
@@ -263,8 +263,8 @@ const createTestSuite = ({ contract, constructorArgs, setOwnersExplicit = false
             const explicitOwnerships = await this.erc721aQueryable.explicitOwnershipsOf(tokenIds);
             expectExplicitOwnershipBurned(explicitOwnerships[0], this.owner.address);
             for (let i = 1; i < tokenIds.length; ++i) {
-              const tokenId = await this.erc721aQueryable.ownerOf(tokenIds[i]);
-              expectExplicitOwnershipExists(explicitOwnerships[i], tokenId);
+              const owner = await this.erc721aQueryable.ownerOf(tokenIds[i]);
+              expectExplicitOwnershipExists(explicitOwnerships[i], owner);
             }
           });
 
@@ -274,8 +274,8 @@ const createTestSuite = ({ contract, constructorArgs, setOwnersExplicit = false
             const explicitOwnerships = await this.erc721aQueryable.explicitOwnershipsOf(tokenIds);
             expectExplicitOwnershipExists(explicitOwnerships[0], this.addr4.address);
             for (let i = 1; i < tokenIds.length; ++i) {
-              const tokenId = await this.erc721aQueryable.ownerOf(tokenIds[i]);
-              expectExplicitOwnershipExists(explicitOwnerships[i], tokenId);
+              const owner = await this.erc721aQueryable.ownerOf(tokenIds[i]);
+              expectExplicitOwnershipExists(explicitOwnerships[i], owner);
             }
           });
 
@@ -284,8 +284,8 @@ const createTestSuite = ({ contract, constructorArgs, setOwnersExplicit = false
             const explicitOwnerships = await this.erc721aQueryable.explicitOwnershipsOf(tokenIds);
             expectExplicitOwnershipNotExists(explicitOwnerships[0]);
             for (let i = 1; i < tokenIds.length; ++i) {
-              const tokenId = await this.erc721aQueryable.ownerOf(tokenIds[i]);
-              expectExplicitOwnershipExists(explicitOwnerships[i], tokenId);
+              const owner = await this.erc721aQueryable.ownerOf(tokenIds[i]);
+              expectExplicitOwnershipExists(explicitOwnerships[i], owner);
             }
           });
         });

test/helpers.js

@@ -12,4 +12,13 @@ const deployContract = async function (contractName, constructorArgs) {
   return contract;
 };
 
-module.exports = { deployContract };
+const getBlockTimestamp = async function () {
+  return parseInt((await ethers.provider.getBlock('latest'))['timestamp']);
+};
+
+const mineBlockTimestamp = async function (timestamp) {
+  await ethers.provider.send('evm_setNextBlockTimestamp', [timestamp]);
+  await ethers.provider.send('evm_mine');
+};
+
+module.exports = { deployContract, getBlockTimestamp, mineBlockTimestamp };