caller or token owner should be able to approve self as operator for … (

#407) * caller or token owner should be able to approve self as operator for own tokens * fixed line lengths (linter) * fixed 'mixed tabs and spaces' (linter)
chiru-labs · Aug 16, 2022 · 7fd5b84 · 7fd5b84
1 parent 002562b
commit 7fd5b84
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 9 deletions.
diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol
@@ -459,8 +459,6 @@ contract ERC721A is IERC721A {
      * Emits an {ApprovalForAll} event.
      */
     function setApprovalForAll(address operator, bool approved) public virtual override {
-        if (operator == _msgSenderERC721A()) revert ApproveToCaller();
-
         _operatorApprovals[_msgSenderERC721A()][operator] = approved;
         emit ApprovalForAll(_msgSenderERC721A(), operator, approved);
     }

diff --git a/contracts/IERC721A.sol b/contracts/IERC721A.sol
@@ -18,11 +18,6 @@ interface IERC721A {
      */
     error ApprovalQueryForNonexistentToken();
 
-    /**
-     * The caller cannot approve to their own address.
-     */
-    error ApproveToCaller();
-
     /**
      * Cannot query the balance for the zero address.
      */

diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js
@@ -268,6 +268,21 @@ const createTestSuite = ({ contract, constructorArgs }) =>
               this.erc721a.connect(this.addr1).transferFrom(this.addr3.address, this.addr1.address, this.tokenId)
             ).to.be.revertedWith('TransferCallerNotOwnerNorApproved');
           });
+
+          it('token owner can approve self as operator', async function () {
+            expect(await this.erc721a.getApproved(this.tokenId)).to.not.equal(this.addr1.address);
+            await expect(this.erc721a.connect(this.addr1).approve(this.addr1.address, this.tokenId)
+            ).to.not.be.reverted;
+            expect(await this.erc721a.getApproved(this.tokenId)).to.equal(this.addr1.address);
+          });
+
+          it('self-approval is cleared on token transfer', async function () {
+            await this.erc721a.connect(this.addr1).approve(this.addr1.address, this.tokenId); 
+            expect(await this.erc721a.getApproved(this.tokenId)).to.equal(this.addr1.address);
+
+            await this.erc721a.connect(this.addr1).transferFrom(this.addr1.address, this.addr2.address, this.tokenId);
+            expect(await this.erc721a.getApproved(this.tokenId)).to.not.equal(this.addr1.address);
+          });
         });
 
         describe('setApprovalForAll', async function () {
@@ -279,10 +294,16 @@ const createTestSuite = ({ contract, constructorArgs }) =>
             expect(await this.erc721a.isApprovedForAll(this.owner.address, this.addr1.address)).to.be.true;
           });
 
-          it('sets rejects approvals for non msg senders', async function () {
+          it('caller can approve all with self as operator', async function () {
+            expect(
+              await this.erc721a.connect(this.addr1).isApprovedForAll(this.addr1.address, this.addr1.address)
+            ).to.be.false;
             await expect(
               this.erc721a.connect(this.addr1).setApprovalForAll(this.addr1.address, true)
-            ).to.be.revertedWith('ApproveToCaller');
+            ).to.not.be.reverted;
+            expect(
+              await this.erc721a.connect(this.addr1).isApprovedForAll(this.addr1.address, this.addr1.address)
+            ).to.be.true;
           });
         });
 
@@ -459,6 +480,12 @@ const createTestSuite = ({ contract, constructorArgs }) =>
             await this.erc721a.connect(this.addr2)['burn(uint256)'](this.tokenIdToBurn);
             expect(await this.erc721a.exists(this.tokenIdToBurn)).to.be.false;
           });
+
+          it('cannot burn a token owned by another if not approved', async function () {
+            expect(await this.erc721a.exists(this.tokenIdToBurn)).to.be.true;
+            await this.erc721a.connect(this.addr2)['burn(uint256)'](this.tokenIdToBurn);
+            expect(await this.erc721a.exists(this.tokenIdToBurn)).to.be.false;
+          });
         });
 
         describe('_initializeOwnershipAt', async function () {