From f85a4e97cfc5c2d934141a999193774d3c2728aa Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 05:29:39 +0000 Subject: [PATCH 01/12] Some minor optimizations and refactors --- contracts/ERC721A.sol | 66 ++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 3008f202c..a6ee330a7 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -580,15 +580,34 @@ contract ERC721A is IERC721A { } /** - * @dev Zeroes out _tokenApprovals[tokenId] + * @dev Returns the storage slot and value for the approved address of `tokenId`. */ - function _removeTokenApproval(uint256 tokenId) private { - mapping(uint256 => address) storage tokenApprovalPtr = _tokenApprovals; + function _getApprovedAddress( + uint256 tokenId + ) private view returns (uint256 approvedAddressSlot, address approvedAddress) { + mapping(uint256 => address) storage tokenApprovalsPtr = _tokenApprovals; assembly { + // Compute the slot and load it. This is equivalent to `approvedAddress[tokenId]`. mstore(0x00, tokenId) - mstore(0x20, tokenApprovalPtr.slot) - let hash := keccak256(0, 0x40) - sstore(hash, 0) + mstore(0x20, tokenApprovalsPtr.slot) + approvedAddressSlot := keccak256(0x00, 0x40) + approvedAddress := sload(approvedAddressSlot) + } + } + + /** + * @dev Returns whether the `approvedAddress` is equals to `from` or `msgSender`. + */ + function _isOwnerOrApproved( + address approvedAddress, + address from, + address msgSender + ) private pure returns (bool result) { + assembly { + // Mask `from` to the lower 160 bits, in case the upper bits somehow aren't clean. + from := and(from, BITMASK_ADDRESS) + // `msgSender == from || msgSender == approvedAddress`. + result := or(eq(msgSender, from), eq(msgSender, approvedAddress)) } } @@ -611,20 +630,22 @@ contract ERC721A is IERC721A { if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); - address approvedAddress = _tokenApprovals[tokenId]; + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); - bool isApprovedOrOwner = (_msgSenderERC721A() == from || - isApprovedForAll(from, _msgSenderERC721A()) || - approvedAddress == _msgSenderERC721A()); + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!isApprovedForAll(from, _msgSenderERC721A())) + revert TransferCallerNotOwnerNorApproved(); - if (!isApprovedOrOwner) revert TransferCallerNotOwnerNorApproved(); if (to == address(0)) revert TransferToZeroAddress(); _beforeTokenTransfers(from, to, tokenId, 1); // Clear approvals from the previous owner. - if (approvedAddress != address(0)) { - _removeTokenApproval(tokenId); + assembly { + if approvedAddress { + // This is equivalent to delete `_tokenApprovals[tokenId]`. + sstore(approvedAddressSlot, 0) + } } // Underflow of the sender's balance is impossible because we check for @@ -684,21 +705,22 @@ contract ERC721A is IERC721A { uint256 prevOwnershipPacked = _packedOwnershipOf(tokenId); address from = address(uint160(prevOwnershipPacked)); - address approvedAddress = _tokenApprovals[tokenId]; - if (approvalCheck) { - bool isApprovedOrOwner = (_msgSenderERC721A() == from || - isApprovedForAll(from, _msgSenderERC721A()) || - approvedAddress == _msgSenderERC721A()); + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); - if (!isApprovedOrOwner) revert TransferCallerNotOwnerNorApproved(); - } + if (approvalCheck) + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!isApprovedForAll(from, _msgSenderERC721A())) + revert TransferCallerNotOwnerNorApproved(); _beforeTokenTransfers(from, address(0), tokenId, 1); // Clear approvals from the previous owner. - if (approvedAddress != address(0)) { - _removeTokenApproval(tokenId); + assembly { + if approvedAddress { + // This is equivalent to delete `_tokenApprovals[tokenId]`. + sstore(approvedAddressSlot, 0) + } } // Underflow of the sender's balance is impossible because we check for From 48a870dd6434e917f6c1eb3c4b87643f0879759d Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 05:32:01 +0000 Subject: [PATCH 02/12] Fix comments --- contracts/ERC721A.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index a6ee330a7..c42c9715c 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -643,7 +643,7 @@ contract ERC721A is IERC721A { // Clear approvals from the previous owner. assembly { if approvedAddress { - // This is equivalent to delete `_tokenApprovals[tokenId]`. + // This is equivalent to `delete _tokenApprovals[tokenId]`. sstore(approvedAddressSlot, 0) } } @@ -718,7 +718,7 @@ contract ERC721A is IERC721A { // Clear approvals from the previous owner. assembly { if approvedAddress { - // This is equivalent to delete `_tokenApprovals[tokenId]`. + // This is equivalent to `delete _tokenApprovals[tokenId]`. sstore(approvedAddressSlot, 0) } } From 5c650d5d1003eb18483884db095f80959b3d2f60 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 05:35:32 +0000 Subject: [PATCH 03/12] Fix comments --- contracts/ERC721A.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index c42c9715c..528771a1a 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -586,11 +586,13 @@ contract ERC721A is IERC721A { uint256 tokenId ) private view returns (uint256 approvedAddressSlot, address approvedAddress) { mapping(uint256 => address) storage tokenApprovalsPtr = _tokenApprovals; + // The following is equivalent to `approvedAddress[tokenId]`. assembly { - // Compute the slot and load it. This is equivalent to `approvedAddress[tokenId]`. + // Compute the slot. mstore(0x00, tokenId) mstore(0x20, tokenApprovalsPtr.slot) approvedAddressSlot := keccak256(0x00, 0x40) + // Load the slot's value from storage. approvedAddress := sload(approvedAddressSlot) } } From 86fc6cc68226d6724c98ee04c8f590b78758aee0 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 06:27:15 +0000 Subject: [PATCH 04/12] Add comments --- contracts/ERC721A.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 528771a1a..73e5bf53f 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -586,7 +586,7 @@ contract ERC721A is IERC721A { uint256 tokenId ) private view returns (uint256 approvedAddressSlot, address approvedAddress) { mapping(uint256 => address) storage tokenApprovalsPtr = _tokenApprovals; - // The following is equivalent to `approvedAddress[tokenId]`. + // The following is equivalent to `approvedAddress = _tokenApprovals[tokenId]`. assembly { // Compute the slot. mstore(0x00, tokenId) @@ -608,6 +608,8 @@ contract ERC721A is IERC721A { assembly { // Mask `from` to the lower 160 bits, in case the upper bits somehow aren't clean. from := and(from, BITMASK_ADDRESS) + // Mask `msgSender` to the lower 160 bits, in case the upper bits somehow aren't clean. + msgSender := and(msgSender, BITMASK_ADDRESS) // `msgSender == from || msgSender == approvedAddress`. result := or(eq(msgSender, from), eq(msgSender, approvedAddress)) } From 6cadd7bc985aaa3ca64eac9e7b84e6d1e4bdcf14 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 06:36:28 +0000 Subject: [PATCH 05/12] Add comments --- contracts/ERC721A.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 73e5bf53f..09da493ba 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -487,7 +487,7 @@ contract ERC721A is IERC721A { */ function _mint(address to, uint256 quantity) internal { uint256 startTokenId = _currentIndex; - if (to == address(0)) revert MintToZeroAddress(); + // if (to == address(0)) revert MintToZeroAddress(); if (quantity == 0) revert MintZeroQuantity(); _beforeTokenTransfers(address(0), to, startTokenId, quantity); From f1ec758529e0bcf43d6acd761efd29fa53325140 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 06:36:41 +0000 Subject: [PATCH 06/12] Add comments --- contracts/ERC721A.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 09da493ba..73e5bf53f 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -487,7 +487,7 @@ contract ERC721A is IERC721A { */ function _mint(address to, uint256 quantity) internal { uint256 startTokenId = _currentIndex; - // if (to == address(0)) revert MintToZeroAddress(); + if (to == address(0)) revert MintToZeroAddress(); if (quantity == 0) revert MintZeroQuantity(); _beforeTokenTransfers(address(0), to, startTokenId, quantity); From 8e7849177e0ae1e3b464b09f21b02f37a82e2596 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 07:52:48 +0000 Subject: [PATCH 07/12] Refactor --- contracts/ERC721A.sol | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 73e5bf53f..e0916770c 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -331,15 +331,6 @@ contract ERC721A is IERC721A { return ''; } - /** - * @dev Casts the boolean to uint256 without branching. - */ - function _boolToUint256(bool value) private pure returns (uint256 result) { - assembly { - result := value - } - } - /** * @dev See {IERC721-approve}. */ @@ -493,14 +484,14 @@ contract ERC721A is IERC721A { _beforeTokenTransfers(address(0), to, startTokenId, quantity); // Overflows are incredibly unrealistic. - // balance or numberMinted overflow if current value of either + quantity > 1.8e19 (2**64) - 1 - // updatedIndex overflows if _currentIndex + quantity > 1.2e77 (2**256) - 1 + // `balance` and `numberMinted` have a maximum limit of 2**64. + // `tokenId` has a maximum limit of 2**256. unchecked { // Updates: // - `balance += quantity`. // - `numberMinted += quantity`. // - // We can directly add to the balance and number minted. + // We can directly add to the `balance` and `numberMinted`. _packedAddressData[to] += quantity * ((1 << BITPOS_NUMBER_MINTED) | 1); // Updates: @@ -510,14 +501,14 @@ contract ERC721A is IERC721A { // - `nextInitialized` to `quantity == 1`. _packedOwnerships[startTokenId] = _packOwnershipData( to, - (_boolToUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) + (_toUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) ); - uint256 offset = startTokenId; + uint256 tokenId = startTokenId; uint256 end = quantity + startTokenId; do { - emit Transfer(address(0), to, offset++); - } while (offset < end); + emit Transfer(address(0), to, tokenId++); + } while (tokenId < end); _currentIndex = startTokenId + quantity; } @@ -559,7 +550,7 @@ contract ERC721A is IERC721A { // - `balance += quantity`. // - `numberMinted += quantity`. // - // We can directly add to the balance and number minted. + // We can directly add to the `balance` and `numberMinted`. _packedAddressData[to] += quantity * ((1 << BITPOS_NUMBER_MINTED) | 1); // Updates: @@ -569,7 +560,7 @@ contract ERC721A is IERC721A { // - `nextInitialized` to `quantity == 1`. _packedOwnerships[startTokenId] = _packOwnershipData( to, - (_boolToUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) + (_toUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) ); emit ConsecutiveTransfer(startTokenId, startTokenId + quantity - 1, address(0), to); @@ -582,13 +573,15 @@ contract ERC721A is IERC721A { /** * @dev Returns the storage slot and value for the approved address of `tokenId`. */ - function _getApprovedAddress( - uint256 tokenId - ) private view returns (uint256 approvedAddressSlot, address approvedAddress) { + function _getApprovedAddress(uint256 tokenId) + private + view + returns (uint256 approvedAddressSlot, address approvedAddress) + { mapping(uint256 => address) storage tokenApprovalsPtr = _tokenApprovals; // The following is equivalent to `approvedAddress = _tokenApprovals[tokenId]`. assembly { - // Compute the slot. + // Compute the slot. mstore(0x00, tokenId) mstore(0x20, tokenApprovalsPtr.slot) approvedAddressSlot := keccak256(0x00, 0x40) @@ -602,7 +595,7 @@ contract ERC721A is IERC721A { */ function _isOwnerOrApproved( address approvedAddress, - address from, + address from, address msgSender ) private pure returns (bool result) { assembly { @@ -615,6 +608,15 @@ contract ERC721A is IERC721A { } } + /** + * @dev Casts the boolean to uint256 without branching. + */ + function _toUint256(bool value) private pure returns (uint256 result) { + assembly { + result := value + } + } + /** * @dev Transfers `tokenId` from `from` to `to`. * @@ -637,8 +639,7 @@ contract ERC721A is IERC721A { (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) - if (!isApprovedForAll(from, _msgSenderERC721A())) - revert TransferCallerNotOwnerNorApproved(); + if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); if (to == address(0)) revert TransferToZeroAddress(); @@ -648,7 +649,7 @@ contract ERC721A is IERC721A { assembly { if approvedAddress { // This is equivalent to `delete _tokenApprovals[tokenId]`. - sstore(approvedAddressSlot, 0) + sstore(approvedAddressSlot, 0) } } @@ -712,10 +713,9 @@ contract ERC721A is IERC721A { (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); - if (approvalCheck) - if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) - if (!isApprovedForAll(from, _msgSenderERC721A())) - revert TransferCallerNotOwnerNorApproved(); + if (approvalCheck) + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); _beforeTokenTransfers(from, address(0), tokenId, 1); @@ -723,13 +723,13 @@ contract ERC721A is IERC721A { assembly { if approvedAddress { // This is equivalent to `delete _tokenApprovals[tokenId]`. - sstore(approvedAddressSlot, 0) + sstore(approvedAddressSlot, 0) } } // Underflow of the sender's balance is impossible because we check for // ownership above and the recipient's balance can't realistically overflow. - // Counter overflow is incredibly unrealistic as tokenId would have to be 2**256. + // Counter overflow is incredibly unrealistic as `tokenId` would have to be 2**256. unchecked { // Updates: // - `balance -= 1`. From 28ecf02228beabdde9bb5eb4c2f7f1fa451a3d8a Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 08:12:14 +0000 Subject: [PATCH 08/12] Inline transfer --- contracts/ERC721A.sol | 139 +++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 75 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index e0916770c..85df69dcf 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -373,14 +373,75 @@ contract ERC721A is IERC721A { } /** - * @dev See {IERC721-transferFrom}. + * @dev Transfers `tokenId` from `from` to `to`. + * + * Requirements: + * + * - `to` cannot be the zero address. + * - `tokenId` token must be owned by `from`. + * + * Emits a {Transfer} event. */ function transferFrom( address from, address to, uint256 tokenId ) public virtual override { - _transfer(from, to, tokenId); + uint256 prevOwnershipPacked = _packedOwnershipOf(tokenId); + + if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); + + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); + + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); + + if (to == address(0)) revert TransferToZeroAddress(); + + _beforeTokenTransfers(from, to, tokenId, 1); + + // Clear approvals from the previous owner. + assembly { + if approvedAddress { + // This is equivalent to `delete _tokenApprovals[tokenId]`. + sstore(approvedAddressSlot, 0) + } + } + + // Underflow of the sender's balance is impossible because we check for + // ownership above and the recipient's balance can't realistically overflow. + // Counter overflow is incredibly unrealistic as tokenId would have to be 2**256. + unchecked { + // We can directly increment and decrement the balances. + --_packedAddressData[from]; // Updates: `balance -= 1`. + ++_packedAddressData[to]; // Updates: `balance += 1`. + + // Updates: + // - `address` to the next owner. + // - `startTimestamp` to the timestamp of transfering. + // - `burned` to `false`. + // - `nextInitialized` to `true`. + _packedOwnerships[tokenId] = _packOwnershipData( + to, + BITMASK_NEXT_INITIALIZED | _nextExtraData(from, to, prevOwnershipPacked) + ); + + // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . + if (prevOwnershipPacked & BITMASK_NEXT_INITIALIZED == 0) { + uint256 nextTokenId = tokenId + 1; + // If the next slot's address is zero and not burned (i.e. packed value is zero). + if (_packedOwnerships[nextTokenId] == 0) { + // If the next slot is within bounds. + if (nextTokenId != _currentIndex) { + // Initialize the next slot to maintain correctness for `ownerOf(tokenId + 1)`. + _packedOwnerships[nextTokenId] = prevOwnershipPacked; + } + } + } + } + + emit Transfer(from, to, tokenId); + _afterTokenTransfers(from, to, tokenId, 1); } /** @@ -403,7 +464,7 @@ contract ERC721A is IERC721A { uint256 tokenId, bytes memory _data ) public virtual override { - _transfer(from, to, tokenId); + transferFrom(from, to, tokenId); if (to.code.length != 0) if (!_checkContractOnERC721Received(from, to, tokenId, _data)) { revert TransferToNonERC721ReceiverImplementer(); @@ -617,78 +678,6 @@ contract ERC721A is IERC721A { } } - /** - * @dev Transfers `tokenId` from `from` to `to`. - * - * Requirements: - * - * - `to` cannot be the zero address. - * - `tokenId` token must be owned by `from`. - * - * Emits a {Transfer} event. - */ - function _transfer( - address from, - address to, - uint256 tokenId - ) private { - uint256 prevOwnershipPacked = _packedOwnershipOf(tokenId); - - if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); - - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); - - if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) - if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); - - if (to == address(0)) revert TransferToZeroAddress(); - - _beforeTokenTransfers(from, to, tokenId, 1); - - // Clear approvals from the previous owner. - assembly { - if approvedAddress { - // This is equivalent to `delete _tokenApprovals[tokenId]`. - sstore(approvedAddressSlot, 0) - } - } - - // Underflow of the sender's balance is impossible because we check for - // ownership above and the recipient's balance can't realistically overflow. - // Counter overflow is incredibly unrealistic as tokenId would have to be 2**256. - unchecked { - // We can directly increment and decrement the balances. - --_packedAddressData[from]; // Updates: `balance -= 1`. - ++_packedAddressData[to]; // Updates: `balance += 1`. - - // Updates: - // - `address` to the next owner. - // - `startTimestamp` to the timestamp of transfering. - // - `burned` to `false`. - // - `nextInitialized` to `true`. - _packedOwnerships[tokenId] = _packOwnershipData( - to, - BITMASK_NEXT_INITIALIZED | _nextExtraData(from, to, prevOwnershipPacked) - ); - - // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . - if (prevOwnershipPacked & BITMASK_NEXT_INITIALIZED == 0) { - uint256 nextTokenId = tokenId + 1; - // If the next slot's address is zero and not burned (i.e. packed value is zero). - if (_packedOwnerships[nextTokenId] == 0) { - // If the next slot is within bounds. - if (nextTokenId != _currentIndex) { - // Initialize the next slot to maintain correctness for `ownerOf(tokenId + 1)`. - _packedOwnerships[nextTokenId] = prevOwnershipPacked; - } - } - } - } - - emit Transfer(from, to, tokenId); - _afterTokenTransfers(from, to, tokenId, 1); - } - /** * @dev Equivalent to `_burn(tokenId, false)`. */ From bb27e9bafeb04e0b14a80920b888a57b0e9500f6 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 08:33:28 +0000 Subject: [PATCH 09/12] Fix test typo --- test/ERC721A.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ERC721A.test.js b/test/ERC721A.test.js index 8596a50f9..4174dd999 100644 --- a/test/ERC721A.test.js +++ b/test/ERC721A.test.js @@ -112,7 +112,7 @@ const createTestSuite = ({ contract, constructorArgs }) => describe('tokenURI (ERC721Metadata)', async function () { describe('tokenURI', async function () { - it('sends an emtpy uri by default', async function () { + it('sends an empty uri by default', async function () { expect(await this.erc721a.tokenURI(offsetted(0))).to.eq(''); }); From d8a4b3050b53d7b27d2fd17c36f6ae5a2bb56174 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 15:59:27 +0000 Subject: [PATCH 10/12] Rename back to _boolToUint256 --- contracts/ERC721A.sol | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 85df69dcf..168b7e295 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -282,12 +282,12 @@ contract ERC721A is IERC721A { /** * @dev Packs ownership data into a single uint256. */ - function _packOwnershipData(address owner, uint256 flags) private view returns (uint256 value) { + function _packOwnershipData(address owner, uint256 flags) private view returns (uint256 result) { assembly { // Mask `owner` to the lower 160 bits, in case the upper bits somehow aren't clean. owner := and(owner, BITMASK_ADDRESS) // `owner | (block.timestamp << BITPOS_START_TIMESTAMP) | flags`. - value := or(owner, or(shl(BITPOS_START_TIMESTAMP, timestamp()), flags)) + result := or(owner, or(shl(BITPOS_START_TIMESTAMP, timestamp()), flags)) } } @@ -331,6 +331,15 @@ contract ERC721A is IERC721A { return ''; } + /** + * @dev Casts the boolean to uint256 without branching. + */ + function _boolToUint256(bool value) private pure returns (uint256 result) { + assembly { + result := value + } + } + /** * @dev See {IERC721-approve}. */ @@ -562,7 +571,7 @@ contract ERC721A is IERC721A { // - `nextInitialized` to `quantity == 1`. _packedOwnerships[startTokenId] = _packOwnershipData( to, - (_toUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) + (_boolToUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) ); uint256 tokenId = startTokenId; @@ -621,7 +630,7 @@ contract ERC721A is IERC721A { // - `nextInitialized` to `quantity == 1`. _packedOwnerships[startTokenId] = _packOwnershipData( to, - (_toUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) + (_boolToUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED) | _nextExtraData(address(0), to, 0) ); emit ConsecutiveTransfer(startTokenId, startTokenId + quantity - 1, address(0), to); @@ -669,15 +678,6 @@ contract ERC721A is IERC721A { } } - /** - * @dev Casts the boolean to uint256 without branching. - */ - function _toUint256(bool value) private pure returns (uint256 result) { - assembly { - result := value - } - } - /** * @dev Equivalent to `_burn(tokenId, false)`. */ From 7c365890b602124dc40793c9b5e5d8d8cab30da8 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 16:02:58 +0000 Subject: [PATCH 11/12] Shift transferFrom to minimize diff --- contracts/ERC721A.sol | 144 +++++++++++++++++++++--------------------- 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 168b7e295..0fe6e1150 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -381,78 +381,6 @@ contract ERC721A is IERC721A { return _operatorApprovals[owner][operator]; } - /** - * @dev Transfers `tokenId` from `from` to `to`. - * - * Requirements: - * - * - `to` cannot be the zero address. - * - `tokenId` token must be owned by `from`. - * - * Emits a {Transfer} event. - */ - function transferFrom( - address from, - address to, - uint256 tokenId - ) public virtual override { - uint256 prevOwnershipPacked = _packedOwnershipOf(tokenId); - - if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); - - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); - - if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) - if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); - - if (to == address(0)) revert TransferToZeroAddress(); - - _beforeTokenTransfers(from, to, tokenId, 1); - - // Clear approvals from the previous owner. - assembly { - if approvedAddress { - // This is equivalent to `delete _tokenApprovals[tokenId]`. - sstore(approvedAddressSlot, 0) - } - } - - // Underflow of the sender's balance is impossible because we check for - // ownership above and the recipient's balance can't realistically overflow. - // Counter overflow is incredibly unrealistic as tokenId would have to be 2**256. - unchecked { - // We can directly increment and decrement the balances. - --_packedAddressData[from]; // Updates: `balance -= 1`. - ++_packedAddressData[to]; // Updates: `balance += 1`. - - // Updates: - // - `address` to the next owner. - // - `startTimestamp` to the timestamp of transfering. - // - `burned` to `false`. - // - `nextInitialized` to `true`. - _packedOwnerships[tokenId] = _packOwnershipData( - to, - BITMASK_NEXT_INITIALIZED | _nextExtraData(from, to, prevOwnershipPacked) - ); - - // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . - if (prevOwnershipPacked & BITMASK_NEXT_INITIALIZED == 0) { - uint256 nextTokenId = tokenId + 1; - // If the next slot's address is zero and not burned (i.e. packed value is zero). - if (_packedOwnerships[nextTokenId] == 0) { - // If the next slot is within bounds. - if (nextTokenId != _currentIndex) { - // Initialize the next slot to maintain correctness for `ownerOf(tokenId + 1)`. - _packedOwnerships[nextTokenId] = prevOwnershipPacked; - } - } - } - } - - emit Transfer(from, to, tokenId); - _afterTokenTransfers(from, to, tokenId, 1); - } - /** * @dev See {IERC721-safeTransferFrom}. */ @@ -678,6 +606,78 @@ contract ERC721A is IERC721A { } } + /** + * @dev Transfers `tokenId` from `from` to `to`. + * + * Requirements: + * + * - `to` cannot be the zero address. + * - `tokenId` token must be owned by `from`. + * + * Emits a {Transfer} event. + */ + function transferFrom( + address from, + address to, + uint256 tokenId + ) public virtual override { + uint256 prevOwnershipPacked = _packedOwnershipOf(tokenId); + + if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); + + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); + + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); + + if (to == address(0)) revert TransferToZeroAddress(); + + _beforeTokenTransfers(from, to, tokenId, 1); + + // Clear approvals from the previous owner. + assembly { + if approvedAddress { + // This is equivalent to `delete _tokenApprovals[tokenId]`. + sstore(approvedAddressSlot, 0) + } + } + + // Underflow of the sender's balance is impossible because we check for + // ownership above and the recipient's balance can't realistically overflow. + // Counter overflow is incredibly unrealistic as tokenId would have to be 2**256. + unchecked { + // We can directly increment and decrement the balances. + --_packedAddressData[from]; // Updates: `balance -= 1`. + ++_packedAddressData[to]; // Updates: `balance += 1`. + + // Updates: + // - `address` to the next owner. + // - `startTimestamp` to the timestamp of transfering. + // - `burned` to `false`. + // - `nextInitialized` to `true`. + _packedOwnerships[tokenId] = _packOwnershipData( + to, + BITMASK_NEXT_INITIALIZED | _nextExtraData(from, to, prevOwnershipPacked) + ); + + // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . + if (prevOwnershipPacked & BITMASK_NEXT_INITIALIZED == 0) { + uint256 nextTokenId = tokenId + 1; + // If the next slot's address is zero and not burned (i.e. packed value is zero). + if (_packedOwnerships[nextTokenId] == 0) { + // If the next slot is within bounds. + if (nextTokenId != _currentIndex) { + // Initialize the next slot to maintain correctness for `ownerOf(tokenId + 1)`. + _packedOwnerships[nextTokenId] = prevOwnershipPacked; + } + } + } + } + + emit Transfer(from, to, tokenId); + _afterTokenTransfers(from, to, tokenId, 1); + } + /** * @dev Equivalent to `_burn(tokenId, false)`. */ From e907aac71422c32fc589277d3385f11e52a73485 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 14 Jun 2022 16:06:47 +0000 Subject: [PATCH 12/12] Add comments on nested ifs --- contracts/ERC721A.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 0fe6e1150..90a821217 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -627,6 +627,7 @@ contract ERC721A is IERC721A { (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); + // The nested ifs save around 20+ gas over a compound boolean condition. if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); @@ -703,6 +704,7 @@ contract ERC721A is IERC721A { (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); if (approvalCheck) + // The nested ifs save around 20+ gas over a compound boolean condition. if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved();