Optimize _approve and add test (#436)

chiru-labs · Nov 15, 2022 · 2342b59 · 2342b59
1 parent 0476a00
commit 2342b59
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 8 deletions.
diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol
@@ -343,7 +343,7 @@ contract ERC721A is IERC721A {
     /**
      * Returns the packed ownership data of `tokenId`.
      */
-    function _packedOwnershipOf(uint256 tokenId) private view returns (uint256 packed) {    
+    function _packedOwnershipOf(uint256 tokenId) private view returns (uint256 packed) {
         if (_startTokenId() <= tokenId) {
             packed = _packedOwnerships[tokenId];
             // If not burned.
@@ -888,7 +888,6 @@ contract ERC721A is IERC721A {
     //                       APPROVAL OPERATIONS
     // =============================================================
 
-
     /**
      * @dev Equivalent to `_approve(to, tokenId, false)`.
      */
@@ -909,13 +908,18 @@ contract ERC721A is IERC721A {
      *
      * Emits an {Approval} event.
      */
-    function _approve(address to, uint256 tokenId, bool approvalCheck) internal virtual {
+    function _approve(
+        address to,
+        uint256 tokenId,
+        bool approvalCheck
+    ) internal virtual {
         address owner = ownerOf(tokenId);
 
-        if (approvalCheck && _msgSenderERC721A() != owner)
-            if (!isApprovedForAll(owner, _msgSenderERC721A())) {
-                revert ApprovalCallerNotOwnerNorApproved();
-            }
+        if (approvalCheck)
+            if (_msgSenderERC721A() != owner)
+                if (!isApprovedForAll(owner, _msgSenderERC721A())) {
+                    revert ApprovalCallerNotOwnerNorApproved();
+                }
 
         _tokenApprovals[tokenId].value = to;
         emit Approval(owner, to, tokenId);

diff --git a/contracts/mocks/ERC721AMock.sol b/contracts/mocks/ERC721AMock.sol
@@ -33,6 +33,10 @@ contract ERC721AMock is ERC721A {
         _setAux(owner, aux);
     }
 
+    function directApprove(address to, uint256 tokenId) public {
+        _approve(to, tokenId);
+    }
+
     function baseURI() public view returns (string memory) {
         return _baseURI();
     }

diff --git a/package-lock.json b/package-lock.json
diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js
@@ -283,6 +283,12 @@ const createTestSuite = ({ contract, constructorArgs }) =>
             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);
           });
+
+          it('direct approve works', async function () {
+            expect(await this.erc721a.getApproved(this.tokenId)).to.not.equal(this.addr1.address);
+            await this.erc721a.connect(this.addr2).directApprove(this.addr1.address, this.tokenId); 
+            expect(await this.erc721a.getApproved(this.tokenId)).to.equal(this.addr1.address);
+          });
         });
 
         describe('setApprovalForAll', async function () {