From 17f064f44fd93497aee304b6f3e1c61c373e3799 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 15 Nov 2022 19:47:54 +0000 Subject: [PATCH 1/2] Optimize and add test --- contracts/ERC721A.sol | 9 +++++---- contracts/mocks/ERC721AMock.sol | 4 ++++ test/ERC721A.test.js | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 8654ca83d..e131c9d0c 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -912,10 +912,11 @@ contract ERC721A is IERC721A { 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 index 4ea0036ea..ca5e53523 100644 --- 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/test/ERC721A.test.js b/test/ERC721A.test.js index e3e1c8b5b..b0591eafc 100644 --- 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.only('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 () { From e2d6a402e27f393bddea9ebe3c185ba1cae9d043 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 15 Nov 2022 19:55:26 +0000 Subject: [PATCH 2/2] Lint --- contracts/ERC721A.sol | 9 ++++++--- package-lock.json | 2 +- test/ERC721A.test.js | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index e131c9d0c..09cbfd551 100644 --- 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,7 +908,11 @@ 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) diff --git a/package-lock.json b/package-lock.json index 344b32c7f..ac85ca254 100644 --- a/package-lock.json +++ b/package-lock.json @@ -48430,4 +48430,4 @@ } } } -} \ No newline at end of file +} diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js index b0591eafc..62c7f420f 100644 --- a/test/ERC721A.test.js +++ b/test/ERC721A.test.js @@ -284,7 +284,7 @@ const createTestSuite = ({ contract, constructorArgs }) => expect(await this.erc721a.getApproved(this.tokenId)).to.not.equal(this.addr1.address); }); - it.only('direct approve works', async function () { + 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);