Added burn functionality (#61)

* Added burn functionality. * Changed _initOneIndexed * Moved burn function into ERC721ABurnable * Moved burn function into ERC721ABurnable * Remove redundant burn check in ownershipOf * Optimized ownershipOf * Removed aux from AddressData for future PR * Packed currentIndex and totalBurned for gas savings * Added gas optimizations * Added gas optimizations * Added requested changes * Edited comments. * Renamed totalBurned to burnedCounter * Renamed to burnCounter * Updated comments. * Mark transferFrom/safeTransferFrom virtual * Mark transferFrom/safeTransferFrom virtual * Updated comments. * Tidy up tests * Inlined _exists for _burn and _transfer. * Merged custom errors * Merged custom errors * Fixed missing change from #59 * Gas optimization * update specs for _beforeTokenTransfers and _afterTokenTransfers hooks * Added #84 * Added #87 * Added #85 * Added #89 * Added comments on packing _currentIndex and _burnCounter * Removed overflow check for updatedIndex * Added requested test changes * Removed unused variable in burn test * Removed unused variable in burn test Co-authored-by: Amirhossein Banavi <ahbanavi@gmail.com>
chiru-labs · Feb 12, 2022 · fb22621 · fb22621
1 parent d886d4d
commit fb22621
Show file tree

Hide file tree

Showing 9 changed files with 444 additions and 36 deletions.
diff --git a/README.md b/README.md
@@ -54,7 +54,6 @@ contract Azuki is ERC721A {
 
 ## Roadmap
 
-- [] Add burn function
 - [] Add flexibility for the first token id to not start at 0
 - [] Support ERC721 Upgradeable
 - [] Add more documentation on benefits of using ERC721A

diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol
diff --git a/contracts/extensions/ERC721ABurnable.sol b/contracts/extensions/ERC721ABurnable.sol
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: MIT
+// Creator: Chiru Labs
+
+pragma solidity ^0.8.4;
+
+import '../ERC721A.sol';
+import '@openzeppelin/contracts/utils/Context.sol';
+
+/**
+ * @title ERC721A Burnable Token
+ * @dev ERC721A Token that can be irreversibly burned (destroyed).
+ */
+abstract contract ERC721ABurnable is Context, ERC721A {
+
+    /**
+     * @dev Burns `tokenId`. See {ERC721A-_burn}.
+     *
+     * Requirements:
+     *
+     * - The caller must own `tokenId` or be an approved operator.
+     */
+    function burn(uint256 tokenId) public virtual {
+        TokenOwnership memory prevOwnership = ownershipOf(tokenId);
+
+        bool isApprovedOrOwner = (_msgSender() == prevOwnership.addr ||
+            isApprovedForAll(prevOwnership.addr, _msgSender()) ||
+            getApproved(tokenId) == _msgSender());
+
+        if (!isApprovedOrOwner) revert TransferCallerNotOwnerNorApproved();
+
+        _burn(tokenId);
+    }
+}
diff --git a/contracts/extensions/ERC721AOwnersExplicit.sol b/contracts/extensions/ERC721AOwnersExplicit.sol
@@ -32,7 +32,7 @@ abstract contract ERC721AOwnersExplicit is ERC721A {
             }
 
             for (uint256 i = _nextOwnerToExplicitlySet; i <= endIndex; i++) {
-                if (_ownerships[i].addr == address(0)) {
+                if (_ownerships[i].addr == address(0) && !_ownerships[i].burned) {
                     TokenOwnership memory ownership = ownershipOf(i);
                     _ownerships[i].addr = ownership.addr;
                     _ownerships[i].startTimestamp = ownership.startTimestamp;

diff --git a/contracts/mocks/ERC721ABurnableMock.sol b/contracts/mocks/ERC721ABurnableMock.sol
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: MIT
+// Creators: Chiru Labs
+
+pragma solidity ^0.8.4;
+
+import '../extensions/ERC721ABurnable.sol';
+
+contract ERC721ABurnableMock is ERC721A, ERC721ABurnable {
+    constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {}
+
+    function exists(uint256 tokenId) public view returns (bool) {
+        return _exists(tokenId);
+    }
+
+    function safeMint(address to, uint256 quantity) public {
+        _safeMint(to, quantity);
+    }
+
+    function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
+        return _ownerships[index];
+    }
+}
diff --git a/contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol b/contracts/mocks/ERC721ABurnableOwnersExplicitMock.sol
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+// 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 exists(uint256 tokenId) public view returns (bool) {
+        return _exists(tokenId);
+    }
+
+    function safeMint(address to, uint256 quantity) public {
+        _safeMint(to, quantity);
+    }
+
+    function setOwnersExplicit(uint256 quantity) public {
+        _setOwnersExplicit(quantity);
+    }
+
+    function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) {
+        return _ownerships[index];
+    }
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/test/extensions/ERC721ABurnable.test.js b/test/extensions/ERC721ABurnable.test.js
@@ -0,0 +1,157 @@
+const { expect } = require('chai');
+
+describe('ERC721ABurnable', function () {
+
+  beforeEach(async function () {
+    this.ERC721ABurnable = await ethers.getContractFactory('ERC721ABurnableMock');
+    this.token = await this.ERC721ABurnable.deploy('Azuki', 'AZUKI');
+    await this.token.deployed();
+  });
+
+  beforeEach(async function () {
+    const [owner, addr1, addr2] = await ethers.getSigners();
+    this.owner = owner;
+    this.addr1 = addr1;
+    this.addr2 = addr2;
+    this.numTestTokens = 10;
+    this.burnedTokenId = 5;
+    await this.token['safeMint(address,uint256)'](this.addr1.address, this.numTestTokens);
+    await this.token.connect(this.addr1).burn(this.burnedTokenId);
+  });
+
+  it('changes exists', async function () {
+    expect(await this.token.exists(this.burnedTokenId)).to.be.false;
+  });
+
+  it('cannot burn a non-existing token', async function () {
+    const query = this.token.connect(this.addr1).burn(this.numTestTokens);
+    await expect(query).to.be.revertedWith(
+      'OwnerQueryForNonexistentToken'
+    );
+  });
+
+  it('cannot burn a burned token', async function () {
+    const query = this.token.connect(this.addr1).burn(this.burnedTokenId);
+    await expect(query).to.be.revertedWith(
+      'OwnerQueryForNonexistentToken'
+    );
+  })
+
+  it('cannot transfer a burned token', async function () {
+    const query = this.token.connect(this.addr1)
+      .transferFrom(this.addr1.address, this.addr2.address, this.burnedTokenId);
+    await expect(query).to.be.revertedWith(
+      'OwnerQueryForNonexistentToken'
+    );
+  })
+
+  it('reduces totalSupply', async function () {
+    const supplyBefore = await this.token.totalSupply();
+    for (let i = 0; i < 2; ++i) {
+      await this.token.connect(this.addr1).burn(i);
+      expect(supplyBefore - (await this.token.totalSupply())).to.equal(i + 1);  
+    }
+  })
+
+  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;
+      await this.token.connect(this.addr1)
+        .transferFrom(this.addr1.address, this.addr2.address, tokenIdToBurn);
+      expect(await this.token.ownerOf(tokenIdToBurn)).to.be.equal(this.addr2.address);  
+      await this.token.connect(this.addr2).burn(tokenIdToBurn);
+      for (let i = 0; i < this.numTestTokens; ++i) {
+        if (i == tokenIdToBurn || i == this.burnedTokenId) {
+          await expect(this.token.ownerOf(i)).to.be.revertedWith(
+            'OwnerQueryForNonexistentToken'
+          );
+        } else {
+          expect(await this.token.ownerOf(i)).to.be.equal(this.addr1.address);  
+        }
+      }
+    });
+
+    it('with token after previously burnt token transfered and burned', async function () {
+      const tokenIdToBurn = this.burnedTokenId + 1;
+      await this.token.connect(this.addr1)
+        .transferFrom(this.addr1.address, this.addr2.address, tokenIdToBurn);
+      expect(await this.token.ownerOf(tokenIdToBurn)).to.be.equal(this.addr2.address);
+      await this.token.connect(this.addr2).burn(tokenIdToBurn);
+      for (let i = 0; i < this.numTestTokens; ++i) {
+        if (i == tokenIdToBurn || i == this.burnedTokenId) {
+          await expect(this.token.ownerOf(i)).to.be.revertedWith(
+            'OwnerQueryForNonexistentToken'
+          )
+        } else {
+          expect(await this.token.ownerOf(i)).to.be.equal(this.addr1.address);  
+        }
+      }
+    });
+
+    it('with first token burned', async function () {
+      await this.token.connect(this.addr1).burn(0);
+      for (let i = 0; i < this.numTestTokens; ++i) {
+        if (i == 0 || i == this.burnedTokenId) {
+          await expect(this.token.ownerOf(i)).to.be.revertedWith(
+            'OwnerQueryForNonexistentToken'
+          )
+        } else {
+          expect(await this.token.ownerOf(i)).to.be.equal(this.addr1.address);  
+        }
+      }
+    });
+
+    it('with last token burned', async function () {
+      await expect(this.token.ownerOf(this.numTestTokens)).to.be.revertedWith(
+        'OwnerQueryForNonexistentToken'
+      )
+      await this.token.connect(this.addr1).burn(this.numTestTokens - 1);
+      await expect(this.token.ownerOf(this.numTestTokens - 1)).to.be.revertedWith(
+        'OwnerQueryForNonexistentToken'
+      )
+    });
+  });
+});
diff --git a/test/extensions/ERC721ABurnableOwnersExplicit.test.js b/test/extensions/ERC721ABurnableOwnersExplicit.test.js
@@ -0,0 +1,45 @@
+const { expect } = require('chai');
+const { constants } = require('@openzeppelin/test-helpers');
+const { ZERO_ADDRESS } = constants;
+
+describe('ERC721ABurnableOwnersExplicit', function () {
+  beforeEach(async function () {
+    this.ERC721ABurnableOwnersExplicit = await ethers.getContractFactory('ERC721ABurnableOwnersExplicitMock');
+    this.token = await this.ERC721ABurnableOwnersExplicit.deploy('Azuki', 'AZUKI');
+    await this.token.deployed();
+  });
+
+  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.setOwnersExplicit(6);
+  });
+
+  it('ownerships correctly set', 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);  
+        }
+      }
+    }
+  });
+});