From 8c9501a3fcd7c66232bc3a2ab5c90e2c080f0201 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 12 Jul 2022 13:50:16 +0000 Subject: [PATCH 01/25] Prelimary edits --- contracts/ERC721A.sol | 18 +++++-- contracts/extensions/ERC4097A.sol | 84 ++++++++++++++++++++++++++++++ contracts/extensions/IERC4907A.sol | 39 ++++++++++++++ 3 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 contracts/extensions/ERC4097A.sol create mode 100644 contracts/extensions/IERC4907A.sol diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 36e219f98..965ac3ffc 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -598,7 +598,7 @@ contract ERC721A is IERC721A { /** * @dev Returns whether the `approvedAddress` is equals to `from` or `msgSender`. */ - function _isOwnerOrApproved( + function _isSinglyApprovedOrOwner( address approvedAddress, address from, address msgSender @@ -613,6 +613,18 @@ contract ERC721A is IERC721A { } } + /** + * @dev Returns whether `spender` is allowed to manage `tokenId`. + * + * Requirements: + * + * - `tokenId` must exist. + */ + function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { + address owner = ownerOf(tokenId); + return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender); + } + /** * @dev Transfers `tokenId` from `from` to `to`. * @@ -635,7 +647,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 (!_isSinglyApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); if (to == address(0)) revert TransferToZeroAddress(); @@ -712,7 +724,7 @@ contract ERC721A is IERC721A { if (approvalCheck) { // The nested ifs save around 20+ gas over a compound boolean condition. - if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) + if (!_isSinglyApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); } diff --git a/contracts/extensions/ERC4097A.sol b/contracts/extensions/ERC4097A.sol new file mode 100644 index 000000000..96974f642 --- /dev/null +++ b/contracts/extensions/ERC4097A.sol @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creator: Chiru Labs + +pragma solidity ^0.8.4; + +import './IERC4097A.sol'; +import '../IERC721A.sol'; + +/** + * @dev Interface of an ERC4907A compliant contract. + */ +abstract contract ERC4097A is ERC721A, IERC4097A { + struct UserInfo { + address user; // Address of user role. + uint64 expires; // unix timestamp, user expires + } + + // Mapping from token ID to ownership details + // An empty struct value does not necessarily mean the token is unowned. + // See `_packedOwnershipOf` implementation for details. + // + // Bits Layout: + // - [0..159] `addr` + // - [160..223] `startTimestamp` + // - [224] `burned` + // - [225] `nextInitialized` + // - [232..255] `extraData` + mapping(uint256 => uint256) private _packedUserInfo; + + /** + * @dev Emitted when the `user` of an NFT or the `expires` of the `user` is changed. + * The zero address for user indicates that there is no user address. + */ + event UpdateUser(uint256 indexed tokenId, address indexed user, uint64 expires); + + /** + * @dev Sets the `user` and `expires` for `tokenId`. + * The zero address indicates there is no user. + * + * Requirements: + * + * - The caller must own `tokenId` or be an approved operator. + */ + function setUser(uint256 tokenId, address user, uint64 expires) external; + + /** + * @dev Returns the user address for `tokenId`. + * The zero address indicates that there is no user or if the user is expired. + */ + function userOf(uint256 tokenId) external view returns (address); + + /** + * @dev Returns the user's expires of `tokenId`. + */ + function userExpires(uint256 tokenId) external view returns (uint256); +} + + +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creator: Chiru Labs + +pragma solidity ^0.8.4; + +import './IERC721ABurnable.sol'; +import '../ERC721A.sol'; + +/** + * @title ERC721A Burnable Token + * @dev ERC721A Token that can be irreversibly burned (destroyed). + */ +abstract contract ERC721ABurnable is ERC721A, IERC721ABurnable { + /** + * @dev Burns `tokenId`. See {ERC721A-_burn}. + * + * Requirements: + * + * - The caller must own `tokenId` or be an approved operator. + */ + function burn(uint256 tokenId) public virtual override { + _burn(tokenId, true); + } +} diff --git a/contracts/extensions/IERC4907A.sol b/contracts/extensions/IERC4907A.sol new file mode 100644 index 000000000..4aa6e032d --- /dev/null +++ b/contracts/extensions/IERC4907A.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creator: Chiru Labs + +pragma solidity ^0.8.4; + +import '../IERC721A.sol'; + +/** + * @dev Interface of an ERC4907A compliant contract. + */ +interface IERC4907A is IERC721A { + /** + * @dev Emitted when the `user` of an NFT or the `expires` of the `user` is changed. + * The zero address for user indicates that there is no user address. + */ + event UpdateUser(uint256 indexed tokenId, address indexed user, uint64 expires); + + /** + * @dev Sets the `user` and `expires` for `tokenId`. + * The zero address indicates there is no user. + * + * Requirements: + * + * - The caller must own `tokenId` or be an approved operator. + */ + function setUser(uint256 tokenId, address user, uint64 expires) external; + + /** + * @dev Returns the user address for `tokenId`. + * The zero address indicates that there is no user or if the user is expired. + */ + function userOf(uint256 tokenId) external view returns (address); + + /** + * @dev Returns the user's expires of `tokenId`. + */ + function userExpires(uint256 tokenId) external view returns (uint256); +} From b8d6890ee0c0de217a4a98a4b3ee804b659e3693 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 12 Jul 2022 14:52:55 +0000 Subject: [PATCH 02/25] Add ERC4907A code --- contracts/extensions/ERC4097A.sol | 84 ------------------------------ contracts/extensions/ERC4907A.sol | 82 +++++++++++++++++++++++++++++ contracts/extensions/IERC4907A.sol | 6 ++- 3 files changed, 87 insertions(+), 85 deletions(-) delete mode 100644 contracts/extensions/ERC4097A.sol create mode 100644 contracts/extensions/ERC4907A.sol diff --git a/contracts/extensions/ERC4097A.sol b/contracts/extensions/ERC4097A.sol deleted file mode 100644 index 96974f642..000000000 --- a/contracts/extensions/ERC4097A.sol +++ /dev/null @@ -1,84 +0,0 @@ -// SPDX-License-Identifier: MIT -// ERC721A Contracts v4.1.0 -// Creator: Chiru Labs - -pragma solidity ^0.8.4; - -import './IERC4097A.sol'; -import '../IERC721A.sol'; - -/** - * @dev Interface of an ERC4907A compliant contract. - */ -abstract contract ERC4097A is ERC721A, IERC4097A { - struct UserInfo { - address user; // Address of user role. - uint64 expires; // unix timestamp, user expires - } - - // Mapping from token ID to ownership details - // An empty struct value does not necessarily mean the token is unowned. - // See `_packedOwnershipOf` implementation for details. - // - // Bits Layout: - // - [0..159] `addr` - // - [160..223] `startTimestamp` - // - [224] `burned` - // - [225] `nextInitialized` - // - [232..255] `extraData` - mapping(uint256 => uint256) private _packedUserInfo; - - /** - * @dev Emitted when the `user` of an NFT or the `expires` of the `user` is changed. - * The zero address for user indicates that there is no user address. - */ - event UpdateUser(uint256 indexed tokenId, address indexed user, uint64 expires); - - /** - * @dev Sets the `user` and `expires` for `tokenId`. - * The zero address indicates there is no user. - * - * Requirements: - * - * - The caller must own `tokenId` or be an approved operator. - */ - function setUser(uint256 tokenId, address user, uint64 expires) external; - - /** - * @dev Returns the user address for `tokenId`. - * The zero address indicates that there is no user or if the user is expired. - */ - function userOf(uint256 tokenId) external view returns (address); - - /** - * @dev Returns the user's expires of `tokenId`. - */ - function userExpires(uint256 tokenId) external view returns (uint256); -} - - -// SPDX-License-Identifier: MIT -// ERC721A Contracts v4.1.0 -// Creator: Chiru Labs - -pragma solidity ^0.8.4; - -import './IERC721ABurnable.sol'; -import '../ERC721A.sol'; - -/** - * @title ERC721A Burnable Token - * @dev ERC721A Token that can be irreversibly burned (destroyed). - */ -abstract contract ERC721ABurnable is ERC721A, IERC721ABurnable { - /** - * @dev Burns `tokenId`. See {ERC721A-_burn}. - * - * Requirements: - * - * - The caller must own `tokenId` or be an approved operator. - */ - function burn(uint256 tokenId) public virtual override { - _burn(tokenId, true); - } -} diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol new file mode 100644 index 000000000..51ed0e88c --- /dev/null +++ b/contracts/extensions/ERC4907A.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creator: Chiru Labs + +pragma solidity ^0.8.4; + +import './IERC4907A.sol'; +import '../ERC721A.sol'; + +/** + * @dev Interface of an ERC4907A compliant contract. + */ +abstract contract ERC4907A is ERC721A, IERC4907A { + // The bit position of `expires` in packed user info. + uint256 private constant _BITPOS_EXPIRES = 160; + + // Mapping from token ID to user information. + // + // Bits Layout: + // - [0..159] `user` + // - [160..223] `expires` + mapping(uint256 => uint256) private _packedUserInfo; + + /** + * @dev Sets the `user` and `expires` for `tokenId`. + * The zero address indicates there is no user. + * + * Requirements: + * + * - The caller must own `tokenId` or be an approved operator. + */ + function setUser( + uint256 tokenId, + address user, + uint64 expires + ) public { + if (!_isApprovedOrOwner(msg.sender, tokenId)) revert TransferCallerNotOwnerNorApproved(); + + uint256 packed = (uint256(expires) << _BITPOS_EXPIRES) | uint256(uint160(user)); + _packedUserInfo[tokenId] = packed; + + emit UpdateUser(tokenId, user, expires); + } + + /** + * @dev Returns the user address for `tokenId`. + * The zero address indicates that there is no user or if the user is expired. + */ + function userOf(uint256 tokenId) public view returns (address) { + uint256 packed = _packedUserInfo[tokenId]; + assembly { + // Set `packed` to zero if the user has expired. + packed := mul( + packed, + // `expires >= block.timestamp`. + iszero(lt(shr(_BITPOS_EXPIRES, packed), timestamp())) + ) + } + return address(uint160(packed)); + } + + /** + * @dev Returns the user's expires of `tokenId`. + */ + function userExpires(uint256 tokenId) public view returns (uint256) { + return _packedUserInfo[tokenId] >> _BITPOS_EXPIRES; + } + + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, IERC721A) returns (bool) { + return interfaceId == type(IERC4907A).interfaceId || ERC721A.supportsInterface(interfaceId); + } + + /** + * @dev Returns the user address for `tokenId`, ignoring the expiry status. + */ + function _explicitUserOf(uint256 tokenId) internal view returns (address) { + return address(uint160(_packedUserInfo[tokenId])); + } +} diff --git a/contracts/extensions/IERC4907A.sol b/contracts/extensions/IERC4907A.sol index 4aa6e032d..d2d3d311b 100644 --- a/contracts/extensions/IERC4907A.sol +++ b/contracts/extensions/IERC4907A.sol @@ -24,7 +24,11 @@ interface IERC4907A is IERC721A { * * - The caller must own `tokenId` or be an approved operator. */ - function setUser(uint256 tokenId, address user, uint64 expires) external; + function setUser( + uint256 tokenId, + address user, + uint64 expires + ) external; /** * @dev Returns the user address for `tokenId`. From 159236f3f5d59beba0ad7858ff069d49b27ca698 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 12 Jul 2022 15:18:38 +0000 Subject: [PATCH 03/25] Fix comments --- contracts/extensions/ERC4907A.sol | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 51ed0e88c..4a74b0db9 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -8,13 +8,17 @@ import './IERC4907A.sol'; import '../ERC721A.sol'; /** - * @dev Interface of an ERC4907A compliant contract. + * @dev ERC4907 compliant extension of ERC721A. + * + * The ERC4907 standard https://eips.ethereum.org/EIPS/eip-4907[ERC4907] allows + * owners and authorized addresses to add a time-limited role + * with restricted permissions to ERC721 tokens. */ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. uint256 private constant _BITPOS_EXPIRES = 160; - // Mapping from token ID to user information. + // Mapping from token ID to user info. // // Bits Layout: // - [0..159] `user` @@ -70,7 +74,9 @@ abstract contract ERC4907A is ERC721A, IERC4907A { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, IERC721A) returns (bool) { - return interfaceId == type(IERC4907A).interfaceId || ERC721A.supportsInterface(interfaceId); + // The interface ID for ERC4907 is `0xad092b5c`. + // See: https://eips.ethereum.org/EIPS/eip-4907 + return ERC721A.supportsInterface(interfaceId) || interfaceId == 0xad092b5c; } /** From 3cd2864c0a544c73bbe2d7dbc83e416675a67d88 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 12 Jul 2022 22:33:37 +0000 Subject: [PATCH 04/25] Add tests --- contracts/extensions/ERC4907A.sol | 44 +++++- contracts/extensions/IERC4907A.sol | 5 + contracts/interfaces/IERC4907A.sol | 7 + contracts/mocks/ERC4907AMock.sol | 32 +++++ test/extensions/ERC4907A.test.js | 219 +++++++++++++++++++++++++++++ 5 files changed, 303 insertions(+), 4 deletions(-) create mode 100644 contracts/interfaces/IERC4907A.sol create mode 100644 contracts/mocks/ERC4907AMock.sol create mode 100644 test/extensions/ERC4907A.test.js diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 4a74b0db9..529f0a7cc 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -18,6 +18,9 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. uint256 private constant _BITPOS_EXPIRES = 160; + // The mask of the lower 160 bits for addresses. + uint256 private constant _BITMASK_ADDRESS = (1 << 160) - 1; + // Mapping from token ID to user info. // // Bits Layout: @@ -38,10 +41,9 @@ abstract contract ERC4907A is ERC721A, IERC4907A { address user, uint64 expires ) public { - if (!_isApprovedOrOwner(msg.sender, tokenId)) revert TransferCallerNotOwnerNorApproved(); + if (!_isApprovedOrOwner(msg.sender, tokenId)) revert SetUserCallerNotOwnerNorApproved(); - uint256 packed = (uint256(expires) << _BITPOS_EXPIRES) | uint256(uint160(user)); - _packedUserInfo[tokenId] = packed; + _packedUserInfo[tokenId] = (uint256(expires) << _BITPOS_EXPIRES) | uint256(uint160(user)); emit UpdateUser(tokenId, user, expires); } @@ -76,7 +78,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, IERC721A) returns (bool) { // The interface ID for ERC4907 is `0xad092b5c`. // See: https://eips.ethereum.org/EIPS/eip-4907 - return ERC721A.supportsInterface(interfaceId) || interfaceId == 0xad092b5c; + return super.supportsInterface(interfaceId) || interfaceId == 0xad092b5c; } /** @@ -85,4 +87,38 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function _explicitUserOf(uint256 tokenId) internal view returns (address) { return address(uint160(_packedUserInfo[tokenId])); } + + /** + * @dev Overrides the `_beforeTokenTransfers` hook to clear the user info upon transfer. + */ + function _beforeTokenTransfers( + address from, + address to, + uint256 startTokenId, + uint256 quantity + ) internal virtual override { + super._beforeTokenTransfers(from, to, startTokenId, quantity); + + bool mayNeedClearing; + assembly { + // Mask `to` and `from` to the lower 160 bits, + // incase the upper bits aren't clean. + let fromMasked := and(from, _BITMASK_ADDRESS) + let toMasked := and(to, _BITMASK_ADDRESS) + // Equivalent to `quantity == 1 && from != address(0) && to != from`. + mayNeedClearing := and( + eq(quantity, 1), + and( + gt(fromMasked, 0), + iszero(eq(fromMasked, toMasked)) + ) + ) + } + + if (mayNeedClearing) + if (_packedUserInfo[startTokenId] != 0) { + delete _packedUserInfo[startTokenId]; + emit UpdateUser(startTokenId, address(0), 0); + } + } } diff --git a/contracts/extensions/IERC4907A.sol b/contracts/extensions/IERC4907A.sol index d2d3d311b..98d3e42cc 100644 --- a/contracts/extensions/IERC4907A.sol +++ b/contracts/extensions/IERC4907A.sol @@ -10,6 +10,11 @@ import '../IERC721A.sol'; * @dev Interface of an ERC4907A compliant contract. */ interface IERC4907A is IERC721A { + /** + * The caller must own the token or be an approved operator. + */ + error SetUserCallerNotOwnerNorApproved(); + /** * @dev Emitted when the `user` of an NFT or the `expires` of the `user` is changed. * The zero address for user indicates that there is no user address. diff --git a/contracts/interfaces/IERC4907A.sol b/contracts/interfaces/IERC4907A.sol new file mode 100644 index 000000000..6353d4510 --- /dev/null +++ b/contracts/interfaces/IERC4907A.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creator: Chiru Labs + +pragma solidity ^0.8.4; + +import '../extensions/IERC4907A.sol'; diff --git a/contracts/mocks/ERC4907AMock.sol b/contracts/mocks/ERC4907AMock.sol new file mode 100644 index 000000000..f1e8bbcaa --- /dev/null +++ b/contracts/mocks/ERC4907AMock.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT +// ERC721A Contracts v4.1.0 +// Creators: Chiru Labs + +pragma solidity ^0.8.4; + +import '../extensions/ERC4907A.sol'; + +contract ERC4907AMock is ERC721A, ERC4907A { + constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} + + function safeMint(address to, uint256 quantity) public { + _safeMint(to, quantity); + } + + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, ERC4907A) returns (bool) { + return super.supportsInterface(interfaceId); + } + + function _beforeTokenTransfers( + address from, + address to, + uint256 startTokenId, + uint256 quantity + ) internal virtual override(ERC721A, ERC4907A) { + super._beforeTokenTransfers(from, to, startTokenId, quantity); + } + + function explicitUserOf(uint256 tokenId) public view returns (address) { + return _explicitUserOf(tokenId); + } +} diff --git a/test/extensions/ERC4907A.test.js b/test/extensions/ERC4907A.test.js new file mode 100644 index 000000000..10dab94ca --- /dev/null +++ b/test/extensions/ERC4907A.test.js @@ -0,0 +1,219 @@ +const { deployContract, getBlockTimestamp, mineBlockTimestamp } = require('../helpers.js'); +const { expect } = require('chai'); +const { constants } = require('@openzeppelin/test-helpers'); +const { ZERO_ADDRESS } = constants; + +const createTestSuite = ({ contract, constructorArgs }) => + function () { + context(`${contract}`, function () { + beforeEach(async function () { + this.erc4097a = await deployContract(contract, constructorArgs); + }); + + describe('EIP-165 support', async function () { + it('supports ERC165', async function () { + expect(await this.erc4097a.supportsInterface('0x01ffc9a7')).to.eq(true); + }); + + it('supports IERC721', async function () { + expect(await this.erc4097a.supportsInterface('0x80ac58cd')).to.eq(true); + }); + + it('supports ERC721Metadata', async function () { + expect(await this.erc4097a.supportsInterface('0x5b5e139f')).to.eq(true); + }); + + it('supports ERC4907', async function () { + expect(await this.erc4097a.supportsInterface('0xad092b5c')).to.eq(true); + }); + + it('does not support ERC721Enumerable', async function () { + expect(await this.erc4097a.supportsInterface('0x780e9d63')).to.eq(false); + }); + + it('does not support random interface', async function () { + expect(await this.erc4097a.supportsInterface('0x00000042')).to.eq(false); + }); + }); + + context('with minted tokens', async function () { + beforeEach(async function () { + const [owner, addr1] = await ethers.getSigners(); + this.owner = owner; + this.addr1 = addr1; + + await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 1); + await this.erc4097a['safeMint(address,uint256)'](this.addr1.address, 2); + + this.expires = (await getBlockTimestamp()) + 123; + this.tokenId = 2; + this.user = this.owner; + }); + + it('explicitUserOf returns zero address after minting', async function () { + expect(await this.erc4097a.explicitUserOf(0)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.explicitUserOf(1)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.explicitUserOf(2)).to.equal(ZERO_ADDRESS); + }); + + it('userOf returns zero address after minting', async function () { + expect(await this.erc4097a.userOf(0)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.userOf(1)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.userOf(2)).to.equal(ZERO_ADDRESS); + }); + + it('userExpires returns zero timestamp after minting', async function () { + expect(await this.erc4097a.userExpires(0)).to.equal(0); + expect(await this.erc4097a.userExpires(1)).to.equal(0); + expect(await this.erc4097a.userExpires(2)).to.equal(0); + }); + + describe('setUser', async function () { + beforeEach(async function () { + this.setUser = async () => await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + this.setupAuthTest = async () => { + this.tokenId = 0; + await expect(this.setUser()) + .to.be.revertedWith('SetUserCallerNotOwnerNorApproved'); + }; + }); + + it('correctly changes the return value of explicitUserOf', async function () { + await this.setUser(); + expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); + }); + + it('correctly changes the return value of userOf', async function () { + await this.setUser(); + expect(await this.erc4097a.userOf(this.tokenId)).to.equal(this.user.address); + }); + + it('correctly changes the return value of expires', async function () { + await this.setUser(); + expect(await this.erc4097a.userExpires(this.tokenId)).to.equal(this.expires); + }); + + it('emits the UpdateUser event properly', async function () { + await expect(await this.setUser()) + .to.emit(this.erc4097a, 'UpdateUser') + .withArgs(this.tokenId, this.user.address, this.expires); + }); + + it('reverts for an invalid token', async function () { + this.tokenId = 123; + await expect( + this.setUser() + ).to.be.revertedWith('OwnerQueryForNonexistentToken'); + }); + + it('requires token ownership', async function () { + await this.setupAuthTest(); + await this.erc4097a.transferFrom(this.owner.address, this.addr1.address, this.tokenId); + await this.setUser(); + }); + + it('requires token approval', async function () { + await this.setupAuthTest(); + await this.erc4097a.approve(this.addr1.address, this.tokenId); + await this.setUser(); + }); + + it('requires operator approval', async function () { + await this.setupAuthTest(); + await this.erc4097a.setApprovalForAll(this.addr1.address, 1); + await this.setUser(); + }); + }); + + describe('after expiry', async function () { + it('userOf returns zero address after expires', async function () { + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + expect(await this.erc4097a.userOf(this.tokenId)).to.equal(this.user.address); + await mineBlockTimestamp(this.expires); + expect(await this.erc4097a.userOf(this.tokenId)).to.equal(this.user.address); + await mineBlockTimestamp(this.expires + 1); + expect(await this.erc4097a.userOf(this.tokenId)).to.equal(ZERO_ADDRESS); + }); + + it('explicitUserOf returns correct address after expiry', async function () { + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); + await mineBlockTimestamp(this.expires); + expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); + await mineBlockTimestamp(this.expires + 1); + expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); + }); + }); + + describe('after transfer', async function () { + it('resets if set', async function () { + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + this.transferTx = await this.erc4097a.connect(this.addr1) + .transferFrom(this.addr1.address, this.owner.address, this.tokenId); + + expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.userOf(this.tokenId)).to.equal(ZERO_ADDRESS); + expect(await this.erc4097a.userExpires(this.tokenId)).to.equal(0); + }); + + it('emits UpdateUser event if set', async function () { + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + this.transferTx = await this.erc4097a.connect(this.addr1) + .transferFrom(this.addr1.address, this.owner.address, this.tokenId); + + await expect(await this.transferTx) + .to.emit(this.erc4097a, 'UpdateUser') + .withArgs(this.tokenId, ZERO_ADDRESS, 0); + }); + + it('does not emits UpdateUser event if not set', async function () { + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, this.user.address, this.expires); + + await this.erc4097a.connect(this.addr1) + .setUser(this.tokenId, ZERO_ADDRESS, 0); + + this.transferTx = await this.erc4097a.connect(this.addr1) + .transferFrom(this.addr1.address, this.owner.address, this.tokenId); + + await expect(await this.transferTx) + .to.not.emit(this.erc4097a, 'UpdateUser'); + }); + + it('does not emits UpdateUser event if owner is unchanged', async function () { + this.transferTx = await this.erc4097a.connect(this.addr1) + .transferFrom(this.addr1.address, this.addr1.address, this.tokenId); + + await expect(await this.transferTx) + .to.not.emit(this.erc4097a, 'UpdateUser'); + }); + + it('minting does not emits UpdateUser event', async function () { + this.mintTx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 1); + await expect(await this.mintTx).to.not.emit(this.erc4097a, 'UpdateUser'); + + this.mintTx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 2); + await expect(await this.mintTx).to.not.emit(this.erc4097a, 'UpdateUser'); + }); + }); + }); + }); + }; + +describe( + 'ERC4907A', + createTestSuite({ + contract: 'ERC4907AMock', + constructorArgs: ['Azuki', 'AZUKI'], + }) +); From 112337c7e26bbfb85a55b27874dfbe9716ff0ad5 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 12 Jul 2022 23:12:39 +0000 Subject: [PATCH 05/25] Minor optimizations --- contracts/extensions/ERC4907A.sol | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 529f0a7cc..be178b378 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -18,9 +18,6 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. uint256 private constant _BITPOS_EXPIRES = 160; - // The mask of the lower 160 bits for addresses. - uint256 private constant _BITMASK_ADDRESS = (1 << 160) - 1; - // Mapping from token ID to user info. // // Bits Layout: @@ -100,18 +97,17 @@ abstract contract ERC4907A is ERC721A, IERC4907A { super._beforeTokenTransfers(from, to, startTokenId, quantity); bool mayNeedClearing; + // For branchless boolean. Saves 60+ gas. assembly { - // Mask `to` and `from` to the lower 160 bits, - // incase the upper bits aren't clean. - let fromMasked := and(from, _BITMASK_ADDRESS) - let toMasked := and(to, _BITMASK_ADDRESS) - // Equivalent to `quantity == 1 && from != address(0) && to != from`. + // The amount of bits to left shift to clear the upper bits of addresses. + let addrShift := sub(256, _BITPOS_EXPIRES) + // Equivalent to `quantity == 1 && !(from == address(0) || to == from)`. mayNeedClearing := and( - eq(quantity, 1), - and( - gt(fromMasked, 0), - iszero(eq(fromMasked, toMasked)) - ) + eq(quantity, 1), + iszero(or( + iszero(shl(addrShift, from)), + eq(shl(addrShift, from), shl(addrShift, to)) + )) ) } From 78fc9b53027ed7d9d93708a15efd755a513bb30c Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 02:59:07 +0000 Subject: [PATCH 06/25] Tidy assembly --- contracts/extensions/ERC4907A.sol | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index be178b378..7d753af2e 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -52,12 +52,11 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { + let expiry := shr(_BITPOS_EXPIRES, packed) + // `expires >= block.timestamp ? 1 : 0`. + let notExpired := iszero(lt(expiry, timestamp())) // Set `packed` to zero if the user has expired. - packed := mul( - packed, - // `expires >= block.timestamp`. - iszero(lt(shr(_BITPOS_EXPIRES, packed), timestamp())) - ) + packed := mul(packed, notExpired) } return address(uint160(packed)); } @@ -95,20 +94,16 @@ abstract contract ERC4907A is ERC721A, IERC4907A { uint256 quantity ) internal virtual override { super._beforeTokenTransfers(from, to, startTokenId, quantity); - + bool mayNeedClearing; // For branchless boolean. Saves 60+ gas. assembly { // The amount of bits to left shift to clear the upper bits of addresses. let addrShift := sub(256, _BITPOS_EXPIRES) - // Equivalent to `quantity == 1 && !(from == address(0) || to == from)`. - mayNeedClearing := and( - eq(quantity, 1), - iszero(or( - iszero(shl(addrShift, from)), - eq(shl(addrShift, from), shl(addrShift, to)) - )) - ) + // Equivalent to `quantity == 1 && !(from == address(0) || from == to)`. + let isMint := iszero(shl(addrShift, from)) + let fromEqTo := eq(shl(addrShift, from), shl(addrShift, to)) + mayNeedClearing := and(eq(quantity, 1), iszero(or(isMint, fromEqTo))) } if (mayNeedClearing) From 196b95f198fe76d46befd67a44284e8500277fbe Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 08:16:57 +0000 Subject: [PATCH 07/25] Refactor assembly --- contracts/extensions/ERC4907A.sol | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 7d753af2e..199e71f93 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -18,6 +18,9 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. uint256 private constant _BITPOS_EXPIRES = 160; + // The mask of the lower 160 bits for addresses. + uint256 private constant _BITMASK_ADDRESS = (1 << 160) - 1; + // Mapping from token ID to user info. // // Bits Layout: @@ -52,11 +55,12 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { - let expiry := shr(_BITPOS_EXPIRES, packed) - // `expires >= block.timestamp ? 1 : 0`. - let notExpired := iszero(lt(expiry, timestamp())) // Set `packed` to zero if the user has expired. - packed := mul(packed, notExpired) + // Equivalent to `packed *= !(block.timestamp > expires) ? 1 : 0` + packed := mul( + packed, + iszero(gt(timestamp(), shr(_BITPOS_EXPIRES, packed))) + ) } return address(uint160(packed)); } @@ -98,12 +102,18 @@ abstract contract ERC4907A is ERC721A, IERC4907A { bool mayNeedClearing; // For branchless boolean. Saves 60+ gas. assembly { - // The amount of bits to left shift to clear the upper bits of addresses. - let addrShift := sub(256, _BITPOS_EXPIRES) // Equivalent to `quantity == 1 && !(from == address(0) || from == to)`. - let isMint := iszero(shl(addrShift, from)) - let fromEqTo := eq(shl(addrShift, from), shl(addrShift, to)) - mayNeedClearing := and(eq(quantity, 1), iszero(or(isMint, fromEqTo))) + // We need to mask all the address with `_BITMASK_ADDRESS` to + // clear any non-zero excess upper bits. + mayNeedClearing := and( + eq(quantity, 1), + iszero(or( + // Whether it is a mint. + iszero(and(from, _BITMASK_ADDRESS)), + // Whether `from == to`. + eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) + )) + ) } if (mayNeedClearing) From bd082f979ef31313d1345060ac7f6f073fc8a8e8 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 08:19:09 +0000 Subject: [PATCH 08/25] Fix comments --- contracts/extensions/ERC4907A.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 199e71f93..6ee478676 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -56,7 +56,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { uint256 packed = _packedUserInfo[tokenId]; assembly { // Set `packed` to zero if the user has expired. - // Equivalent to `packed *= !(block.timestamp > expires) ? 1 : 0` + // Equivalent to `packed *= !(block.timestamp > expires) ? 1 : 0`. packed := mul( packed, iszero(gt(timestamp(), shr(_BITPOS_EXPIRES, packed))) @@ -103,14 +103,14 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // For branchless boolean. Saves 60+ gas. assembly { // Equivalent to `quantity == 1 && !(from == address(0) || from == to)`. - // We need to mask all the address with `_BITMASK_ADDRESS` to + // We need to mask the addresses with `_BITMASK_ADDRESS` to // clear any non-zero excess upper bits. mayNeedClearing := and( eq(quantity, 1), iszero(or( - // Whether it is a mint. + // Whether it is a mint (i.e. `from == address(0)`). iszero(and(from, _BITMASK_ADDRESS)), - // Whether `from == to`. + // Whether the owner is unchanged (i.e. `from == to`). eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) )) ) From 97f22bc1e89189b97b4362e34eb4c6b54743f193 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 08:25:16 +0000 Subject: [PATCH 09/25] Add more comments --- contracts/extensions/ERC4907A.sol | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 6ee478676..373e40d93 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -55,11 +55,17 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { - // Set `packed` to zero if the user has expired. + // Set `packed` to zero if the user has expired without branching. // Equivalent to `packed *= !(block.timestamp > expires) ? 1 : 0`. packed := mul( packed, - iszero(gt(timestamp(), shr(_BITPOS_EXPIRES, packed))) + // `!(block.timestamp > expires) ? 1 : 0`. + iszero( + gt( + timestamp(), + shr(_BITPOS_EXPIRES, packed) // `expires`. + ) + ) ) } return address(uint160(packed)); @@ -107,19 +113,24 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // clear any non-zero excess upper bits. mayNeedClearing := and( eq(quantity, 1), - iszero(or( - // Whether it is a mint (i.e. `from == address(0)`). - iszero(and(from, _BITMASK_ADDRESS)), - // Whether the owner is unchanged (i.e. `from == to`). - eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) - )) + // `!(from == address(0) || from == to)`. + iszero( + or( + // Whether it is a mint (i.e. `from == address(0)`). + iszero(and(from, _BITMASK_ADDRESS)), + // Whether the owner is unchanged (i.e. `from == to`). + eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) + ) + ) ) } - if (mayNeedClearing) + if (mayNeedClearing) { + // If either `user` or `expires` are non-zero. if (_packedUserInfo[startTokenId] != 0) { delete _packedUserInfo[startTokenId]; emit UpdateUser(startTokenId, address(0), 0); } + } } } From 13921ab49a30f27f7728c4923ac7567883d496df Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 08:39:49 +0000 Subject: [PATCH 10/25] Add more comments --- contracts/extensions/ERC4907A.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 373e40d93..928a5b015 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -55,8 +55,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { - // Set `packed` to zero if the user has expired without branching. - // Equivalent to `packed *= !(block.timestamp > expires) ? 1 : 0`. + // Branchless `packed *= !(block.timestamp > expires) ? 1 : 0`. packed := mul( packed, // `!(block.timestamp > expires) ? 1 : 0`. @@ -106,10 +105,10 @@ abstract contract ERC4907A is ERC721A, IERC4907A { super._beforeTokenTransfers(from, to, startTokenId, quantity); bool mayNeedClearing; - // For branchless boolean. Saves 60+ gas. assembly { - // Equivalent to `quantity == 1 && !(from == address(0) || from == to)`. - // We need to mask the addresses with `_BITMASK_ADDRESS` to + // Branchless `quantity == 1 && !(from == address(0) || from == to)`. + // Saves 60+ gas. + // The addresses are masked with `_BITMASK_ADDRESS` to // clear any non-zero excess upper bits. mayNeedClearing := and( eq(quantity, 1), From 62c44bddef161b9cf431cc604586daaa2a6ec834 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 09:33:07 +0000 Subject: [PATCH 11/25] Simplify tests --- test/extensions/ERC4907A.test.js | 70 +++++++++++--------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/test/extensions/ERC4907A.test.js b/test/extensions/ERC4907A.test.js index 10dab94ca..0bdbe1156 100644 --- a/test/extensions/ERC4907A.test.js +++ b/test/extensions/ERC4907A.test.js @@ -27,10 +27,6 @@ const createTestSuite = ({ contract, constructorArgs }) => expect(await this.erc4097a.supportsInterface('0xad092b5c')).to.eq(true); }); - it('does not support ERC721Enumerable', async function () { - expect(await this.erc4097a.supportsInterface('0x780e9d63')).to.eq(false); - }); - it('does not support random interface', async function () { expect(await this.erc4097a.supportsInterface('0x00000042')).to.eq(false); }); @@ -75,8 +71,7 @@ const createTestSuite = ({ contract, constructorArgs }) => this.setupAuthTest = async () => { this.tokenId = 0; - await expect(this.setUser()) - .to.be.revertedWith('SetUserCallerNotOwnerNorApproved'); + await expect(this.setUser()).to.be.revertedWith('SetUserCallerNotOwnerNorApproved'); }; }); @@ -103,9 +98,7 @@ const createTestSuite = ({ contract, constructorArgs }) => it('reverts for an invalid token', async function () { this.tokenId = 123; - await expect( - this.setUser() - ).to.be.revertedWith('OwnerQueryForNonexistentToken'); + await expect(this.setUser()).to.be.revertedWith('OwnerQueryForNonexistentToken'); }); it('requires token ownership', async function () { @@ -128,10 +121,12 @@ const createTestSuite = ({ contract, constructorArgs }) => }); describe('after expiry', async function () { - it('userOf returns zero address after expires', async function () { + beforeEach(async function () { await this.erc4097a.connect(this.addr1) .setUser(this.tokenId, this.user.address, this.expires); - + }); + + it('userOf returns zero address after expires', async function () { expect(await this.erc4097a.userOf(this.tokenId)).to.equal(this.user.address); await mineBlockTimestamp(this.expires); expect(await this.erc4097a.userOf(this.tokenId)).to.equal(this.user.address); @@ -140,9 +135,6 @@ const createTestSuite = ({ contract, constructorArgs }) => }); it('explicitUserOf returns correct address after expiry', async function () { - await this.erc4097a.connect(this.addr1) - .setUser(this.tokenId, this.user.address, this.expires); - expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); await mineBlockTimestamp(this.expires); expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); @@ -152,58 +144,44 @@ const createTestSuite = ({ contract, constructorArgs }) => }); describe('after transfer', async function () { - it('resets if set', async function () { + beforeEach(async function () { await this.erc4097a.connect(this.addr1) .setUser(this.tokenId, this.user.address, this.expires); - - this.transferTx = await this.erc4097a.connect(this.addr1) - .transferFrom(this.addr1.address, this.owner.address, this.tokenId); + this.destination = this.owner + + this.transfer = () => this.erc4097a.connect(this.addr1) + .transferFrom(this.addr1.address, this.destination.address, this.tokenId); + }); + + it('resets if set', async function () { + await this.transfer(); expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(ZERO_ADDRESS); expect(await this.erc4097a.userOf(this.tokenId)).to.equal(ZERO_ADDRESS); expect(await this.erc4097a.userExpires(this.tokenId)).to.equal(0); }); it('emits UpdateUser event if set', async function () { - await this.erc4097a.connect(this.addr1) - .setUser(this.tokenId, this.user.address, this.expires); - - this.transferTx = await this.erc4097a.connect(this.addr1) - .transferFrom(this.addr1.address, this.owner.address, this.tokenId); - - await expect(await this.transferTx) + await expect(await this.transfer()) .to.emit(this.erc4097a, 'UpdateUser') .withArgs(this.tokenId, ZERO_ADDRESS, 0); }); it('does not emits UpdateUser event if not set', async function () { - await this.erc4097a.connect(this.addr1) - .setUser(this.tokenId, this.user.address, this.expires); - - await this.erc4097a.connect(this.addr1) - .setUser(this.tokenId, ZERO_ADDRESS, 0); - - this.transferTx = await this.erc4097a.connect(this.addr1) - .transferFrom(this.addr1.address, this.owner.address, this.tokenId); - - await expect(await this.transferTx) - .to.not.emit(this.erc4097a, 'UpdateUser'); + await this.erc4097a.connect(this.addr1).setUser(this.tokenId, ZERO_ADDRESS, 0); + await expect(await this.transfer()).to.not.emit(this.erc4097a, 'UpdateUser'); }); it('does not emits UpdateUser event if owner is unchanged', async function () { - this.transferTx = await this.erc4097a.connect(this.addr1) - .transferFrom(this.addr1.address, this.addr1.address, this.tokenId); - - await expect(await this.transferTx) - .to.not.emit(this.erc4097a, 'UpdateUser'); + this.destination = this.addr1; + await expect(await this.transfer()).to.not.emit(this.erc4097a, 'UpdateUser'); }); it('minting does not emits UpdateUser event', async function () { - this.mintTx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 1); - await expect(await this.mintTx).to.not.emit(this.erc4097a, 'UpdateUser'); - - this.mintTx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 2); - await expect(await this.mintTx).to.not.emit(this.erc4097a, 'UpdateUser'); + for (let quantity = 1; quantity < 3; ++quantity) { + let tx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, quantity); + await expect(tx).to.not.emit(this.erc4097a, 'UpdateUser'); + } }); }); }); From fb922e9535cb6eb1dc787e619c9b2b676c6a05cb Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 10:01:52 +0000 Subject: [PATCH 12/25] Remove negation --- contracts/extensions/ERC4907A.sol | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 928a5b015..cdd42242f 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -58,12 +58,10 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // Branchless `packed *= !(block.timestamp > expires) ? 1 : 0`. packed := mul( packed, - // `!(block.timestamp > expires) ? 1 : 0`. - iszero( - gt( - timestamp(), - shr(_BITPOS_EXPIRES, packed) // `expires`. - ) + // `block.timestamp <= expires ? 1 : 0`. + lt( + shl(_BITPOS_EXPIRES, timestamp()), + packed ) ) } From 1f626a7fe2f8d97da877566ee99ff020cc027491 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Wed, 13 Jul 2022 18:18:24 +0800 Subject: [PATCH 13/25] Update comment --- contracts/extensions/ERC4907A.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index cdd42242f..960c1ca3e 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -55,7 +55,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { - // Branchless `packed *= !(block.timestamp > expires) ? 1 : 0`. + // Branchless `packed *= block.timestamp <= expires ? 1 : 0`. packed := mul( packed, // `block.timestamp <= expires ? 1 : 0`. From 57c73ff94e2d0959e2be2b2f81a7ed505a705b04 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Fri, 15 Jul 2022 02:57:17 +0000 Subject: [PATCH 14/25] Remove quantity equals 1 check in hook --- contracts/extensions/ERC4907A.sol | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 960c1ca3e..1ed10ef28 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -59,10 +59,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { packed := mul( packed, // `block.timestamp <= expires ? 1 : 0`. - lt( - shl(_BITPOS_EXPIRES, timestamp()), - packed - ) + lt(shl(_BITPOS_EXPIRES, timestamp()), packed) ) } return address(uint160(packed)); @@ -104,20 +101,16 @@ abstract contract ERC4907A is ERC721A, IERC4907A { bool mayNeedClearing; assembly { - // Branchless `quantity == 1 && !(from == address(0) || from == to)`. + // Branchless `!(from == address(0) || from == to)`. // Saves 60+ gas. // The addresses are masked with `_BITMASK_ADDRESS` to // clear any non-zero excess upper bits. - mayNeedClearing := and( - eq(quantity, 1), - // `!(from == address(0) || from == to)`. - iszero( - or( - // Whether it is a mint (i.e. `from == address(0)`). - iszero(and(from, _BITMASK_ADDRESS)), - // Whether the owner is unchanged (i.e. `from == to`). - eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) - ) + mayNeedClearing := iszero( + or( + // Whether it is a mint (i.e. `from == address(0)`). + iszero(and(from, _BITMASK_ADDRESS)), + // Whether the owner is unchanged (i.e. `from == to`). + eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) ) ) } From a5839ad70bb79ab1dc9ed3704d7cee6f03810f20 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Fri, 15 Jul 2022 18:58:18 +0000 Subject: [PATCH 15/25] Remove auto user reset upon transfer feature --- contracts/extensions/ERC4907A.sol | 36 -------------------------- contracts/mocks/ERC4907AMock.sol | 9 ------- test/extensions/ERC4907A.test.js | 42 ------------------------------- 3 files changed, 87 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 1ed10ef28..d33625913 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -87,40 +87,4 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function _explicitUserOf(uint256 tokenId) internal view returns (address) { return address(uint160(_packedUserInfo[tokenId])); } - - /** - * @dev Overrides the `_beforeTokenTransfers` hook to clear the user info upon transfer. - */ - function _beforeTokenTransfers( - address from, - address to, - uint256 startTokenId, - uint256 quantity - ) internal virtual override { - super._beforeTokenTransfers(from, to, startTokenId, quantity); - - bool mayNeedClearing; - assembly { - // Branchless `!(from == address(0) || from == to)`. - // Saves 60+ gas. - // The addresses are masked with `_BITMASK_ADDRESS` to - // clear any non-zero excess upper bits. - mayNeedClearing := iszero( - or( - // Whether it is a mint (i.e. `from == address(0)`). - iszero(and(from, _BITMASK_ADDRESS)), - // Whether the owner is unchanged (i.e. `from == to`). - eq(and(from, _BITMASK_ADDRESS), and(to, _BITMASK_ADDRESS)) - ) - ) - } - - if (mayNeedClearing) { - // If either `user` or `expires` are non-zero. - if (_packedUserInfo[startTokenId] != 0) { - delete _packedUserInfo[startTokenId]; - emit UpdateUser(startTokenId, address(0), 0); - } - } - } } diff --git a/contracts/mocks/ERC4907AMock.sol b/contracts/mocks/ERC4907AMock.sol index f1e8bbcaa..816599997 100644 --- a/contracts/mocks/ERC4907AMock.sol +++ b/contracts/mocks/ERC4907AMock.sol @@ -17,15 +17,6 @@ contract ERC4907AMock is ERC721A, ERC4907A { return super.supportsInterface(interfaceId); } - function _beforeTokenTransfers( - address from, - address to, - uint256 startTokenId, - uint256 quantity - ) internal virtual override(ERC721A, ERC4907A) { - super._beforeTokenTransfers(from, to, startTokenId, quantity); - } - function explicitUserOf(uint256 tokenId) public view returns (address) { return _explicitUserOf(tokenId); } diff --git a/test/extensions/ERC4907A.test.js b/test/extensions/ERC4907A.test.js index 0bdbe1156..d0839888a 100644 --- a/test/extensions/ERC4907A.test.js +++ b/test/extensions/ERC4907A.test.js @@ -142,48 +142,6 @@ const createTestSuite = ({ contract, constructorArgs }) => expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(this.user.address); }); }); - - describe('after transfer', async function () { - beforeEach(async function () { - await this.erc4097a.connect(this.addr1) - .setUser(this.tokenId, this.user.address, this.expires); - - this.destination = this.owner - - this.transfer = () => this.erc4097a.connect(this.addr1) - .transferFrom(this.addr1.address, this.destination.address, this.tokenId); - }); - - it('resets if set', async function () { - await this.transfer(); - expect(await this.erc4097a.explicitUserOf(this.tokenId)).to.equal(ZERO_ADDRESS); - expect(await this.erc4097a.userOf(this.tokenId)).to.equal(ZERO_ADDRESS); - expect(await this.erc4097a.userExpires(this.tokenId)).to.equal(0); - }); - - it('emits UpdateUser event if set', async function () { - await expect(await this.transfer()) - .to.emit(this.erc4097a, 'UpdateUser') - .withArgs(this.tokenId, ZERO_ADDRESS, 0); - }); - - it('does not emits UpdateUser event if not set', async function () { - await this.erc4097a.connect(this.addr1).setUser(this.tokenId, ZERO_ADDRESS, 0); - await expect(await this.transfer()).to.not.emit(this.erc4097a, 'UpdateUser'); - }); - - it('does not emits UpdateUser event if owner is unchanged', async function () { - this.destination = this.addr1; - await expect(await this.transfer()).to.not.emit(this.erc4097a, 'UpdateUser'); - }); - - it('minting does not emits UpdateUser event', async function () { - for (let quantity = 1; quantity < 3; ++quantity) { - let tx = await this.erc4097a['safeMint(address,uint256)'](this.owner.address, quantity); - await expect(tx).to.not.emit(this.erc4097a, 'UpdateUser'); - } - }); - }); }); }); }; From 2b99f3a1e496da024dddf9dbab4787a410d16c65 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Fri, 15 Jul 2022 19:04:57 +0000 Subject: [PATCH 16/25] Remove unused address mask --- contracts/extensions/ERC4907A.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index d33625913..441a7e2d2 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -18,9 +18,6 @@ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. uint256 private constant _BITPOS_EXPIRES = 160; - // The mask of the lower 160 bits for addresses. - uint256 private constant _BITMASK_ADDRESS = (1 << 160) - 1; - // Mapping from token ID to user info. // // Bits Layout: From e6d451267684b1478cbd3d360a2f7f4b46882ce6 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sat, 16 Jul 2022 16:27:44 +0000 Subject: [PATCH 17/25] Minor De Morgan optimization for _isApprovedOrOwner --- contracts/ERC721A.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 965ac3ffc..2f1927c57 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -622,7 +622,11 @@ contract ERC721A is IERC721A { */ function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { address owner = ownerOf(tokenId); - return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender); + bool result = true; + if (spender != owner) + if (!isApprovedForAll(owner, spender)) + if (getApproved(tokenId) != spender) result = false; + return result; } /** From 7bd79c4f54207f28ad4c9df6ea3de73dc63d61a5 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sat, 16 Jul 2022 16:29:30 +0000 Subject: [PATCH 18/25] Make functions virtual --- contracts/extensions/ERC4907A.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 441a7e2d2..b1dd6d118 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -37,7 +37,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { uint256 tokenId, address user, uint64 expires - ) public { + ) public virtual { if (!_isApprovedOrOwner(msg.sender, tokenId)) revert SetUserCallerNotOwnerNorApproved(); _packedUserInfo[tokenId] = (uint256(expires) << _BITPOS_EXPIRES) | uint256(uint160(user)); @@ -49,7 +49,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { * @dev Returns the user address for `tokenId`. * The zero address indicates that there is no user or if the user is expired. */ - function userOf(uint256 tokenId) public view returns (address) { + function userOf(uint256 tokenId) public view virtual returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { // Branchless `packed *= block.timestamp <= expires ? 1 : 0`. @@ -65,7 +65,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { /** * @dev Returns the user's expires of `tokenId`. */ - function userExpires(uint256 tokenId) public view returns (uint256) { + function userExpires(uint256 tokenId) public view virtual returns (uint256) { return _packedUserInfo[tokenId] >> _BITPOS_EXPIRES; } @@ -81,7 +81,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { /** * @dev Returns the user address for `tokenId`, ignoring the expiry status. */ - function _explicitUserOf(uint256 tokenId) internal view returns (address) { + function _explicitUserOf(uint256 tokenId) internal view virtual returns (address) { return address(uint160(_packedUserInfo[tokenId])); } } From 71b3379a9342e5c3609cbc13030b1a3b986c917e Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 19:20:01 +0000 Subject: [PATCH 19/25] Edit comments --- contracts/extensions/ERC4907A.sol | 14 +++++++------- contracts/extensions/IERC4907A.sol | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index b1dd6d118..9937c93b5 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -8,11 +8,11 @@ import './IERC4907A.sol'; import '../ERC721A.sol'; /** - * @dev ERC4907 compliant extension of ERC721A. + * @title ERC4907A * - * The ERC4907 standard https://eips.ethereum.org/EIPS/eip-4907[ERC4907] allows - * owners and authorized addresses to add a time-limited role - * with restricted permissions to ERC721 tokens. + * @dev [ERC4907](https://eips.ethereum.org/EIPS/eip-4907) compliant + * extension of ERC721A, which allows owners and authorized addresses + * to add a time-limited role with restricted permissions to ERC721 tokens. */ abstract contract ERC4907A is ERC721A, IERC4907A { // The bit position of `expires` in packed user info. @@ -70,11 +70,11 @@ abstract contract ERC4907A is ERC721A, IERC4907A { } /** - * @dev See {IERC165-supportsInterface}. + * @dev Override of {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, IERC721A) returns (bool) { - // The interface ID for ERC4907 is `0xad092b5c`. - // See: https://eips.ethereum.org/EIPS/eip-4907 + // The interface ID for ERC4907 is `0xad092b5c`, + // as defined in [ERC4907](https://eips.ethereum.org/EIPS/eip-4907). return super.supportsInterface(interfaceId) || interfaceId == 0xad092b5c; } diff --git a/contracts/extensions/IERC4907A.sol b/contracts/extensions/IERC4907A.sol index 98d3e42cc..40fe7b298 100644 --- a/contracts/extensions/IERC4907A.sol +++ b/contracts/extensions/IERC4907A.sol @@ -7,7 +7,7 @@ pragma solidity ^0.8.4; import '../IERC721A.sol'; /** - * @dev Interface of an ERC4907A compliant contract. + * @dev Interface of ERC4907A. */ interface IERC4907A is IERC721A { /** From bdcc569785100c1391e18c5d48b2079bb8341730 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 19:44:47 +0000 Subject: [PATCH 20/25] Rename to _getApprovedSlotAndAddress --- contracts/ERC721A.sol | 8 ++++---- contracts/extensions/ERC4907A.sol | 2 +- contracts/mocks/ERC4907AMock.sol | 4 ++-- test/extensions/ERC4907A.test.js | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 2f1927c57..496462481 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -582,7 +582,7 @@ contract ERC721A is IERC721A { /** * @dev Returns the storage slot and value for the approved address of `tokenId`. */ - function _getApprovedAddress(uint256 tokenId) + function _getApprovedSlotAndAddress(uint256 tokenId) private view returns (uint256 approvedAddressSlot, address approvedAddress) @@ -596,7 +596,7 @@ contract ERC721A is IERC721A { } /** - * @dev Returns whether the `approvedAddress` is equals to `from` or `msgSender`. + * @dev Returns whether the `approvedAddress` is equal to `from` or `msgSender`. */ function _isSinglyApprovedOrOwner( address approvedAddress, @@ -648,7 +648,7 @@ contract ERC721A is IERC721A { if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedSlotAndAddress(tokenId); // The nested ifs save around 20+ gas over a compound boolean condition. if (!_isSinglyApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) @@ -724,7 +724,7 @@ contract ERC721A is IERC721A { address from = address(uint160(prevOwnershipPacked)); - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedSlotAndAddress(tokenId); if (approvalCheck) { // The nested ifs save around 20+ gas over a compound boolean condition. diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 9937c93b5..66e430dae 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -52,7 +52,7 @@ abstract contract ERC4907A is ERC721A, IERC4907A { function userOf(uint256 tokenId) public view virtual returns (address) { uint256 packed = _packedUserInfo[tokenId]; assembly { - // Branchless `packed *= block.timestamp <= expires ? 1 : 0`. + // Branchless `packed *= (block.timestamp <= expires ? 1 : 0)`. packed := mul( packed, // `block.timestamp <= expires ? 1 : 0`. diff --git a/contracts/mocks/ERC4907AMock.sol b/contracts/mocks/ERC4907AMock.sol index 816599997..e0f938bd7 100644 --- a/contracts/mocks/ERC4907AMock.sol +++ b/contracts/mocks/ERC4907AMock.sol @@ -9,8 +9,8 @@ import '../extensions/ERC4907A.sol'; contract ERC4907AMock is ERC721A, ERC4907A { constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} - function safeMint(address to, uint256 quantity) public { - _safeMint(to, quantity); + function mint(address to, uint256 quantity) public { + _mint(to, quantity); } function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721A, ERC4907A) returns (bool) { diff --git a/test/extensions/ERC4907A.test.js b/test/extensions/ERC4907A.test.js index d0839888a..f2004271a 100644 --- a/test/extensions/ERC4907A.test.js +++ b/test/extensions/ERC4907A.test.js @@ -38,8 +38,8 @@ const createTestSuite = ({ contract, constructorArgs }) => this.owner = owner; this.addr1 = addr1; - await this.erc4097a['safeMint(address,uint256)'](this.owner.address, 1); - await this.erc4097a['safeMint(address,uint256)'](this.addr1.address, 2); + await this.erc4097a['mint(address,uint256)'](this.owner.address, 1); + await this.erc4097a['mint(address,uint256)'](this.addr1.address, 2); this.expires = (await getBlockTimestamp()) + 123; this.tokenId = 2; From cd48fd0cdbdcd1c55f8223fbc50639cd32b25f58 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 21:31:00 +0000 Subject: [PATCH 21/25] Remove _isApprovedOrOwner --- contracts/ERC721A.sol | 34 ++++++++----------------------- contracts/extensions/ERC4907A.sol | 5 ++++- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 496462481..16182ac5c 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -596,39 +596,23 @@ contract ERC721A is IERC721A { } /** - * @dev Returns whether the `approvedAddress` is equal to `from` or `msgSender`. + * @dev Returns whether the `msgSender` is equal to `owner` or `approvedAddress`. */ - function _isSinglyApprovedOrOwner( + function _isSenderApprovedOrOwner( address approvedAddress, - address from, + address owner, 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) + // Mask `owner` to the lower 160 bits, in case the upper bits somehow aren't clean. + owner := and(owner, 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)) + // `msgSender == owner || msgSender == approvedAddress`. + result := or(eq(msgSender, owner), eq(msgSender, approvedAddress)) } } - /** - * @dev Returns whether `spender` is allowed to manage `tokenId`. - * - * Requirements: - * - * - `tokenId` must exist. - */ - function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { - address owner = ownerOf(tokenId); - bool result = true; - if (spender != owner) - if (!isApprovedForAll(owner, spender)) - if (getApproved(tokenId) != spender) result = false; - return result; - } - /** * @dev Transfers `tokenId` from `from` to `to`. * @@ -651,7 +635,7 @@ contract ERC721A is IERC721A { (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedSlotAndAddress(tokenId); // The nested ifs save around 20+ gas over a compound boolean condition. - if (!_isSinglyApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) + if (!_isSenderApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); if (to == address(0)) revert TransferToZeroAddress(); @@ -728,7 +712,7 @@ contract ERC721A is IERC721A { if (approvalCheck) { // The nested ifs save around 20+ gas over a compound boolean condition. - if (!_isSinglyApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) + if (!_isSenderApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); } diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index 66e430dae..b87a1228c 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -38,7 +38,10 @@ abstract contract ERC4907A is ERC721A, IERC4907A { address user, uint64 expires ) public virtual { - if (!_isApprovedOrOwner(msg.sender, tokenId)) revert SetUserCallerNotOwnerNorApproved(); + address owner = ownerOf(tokenId); + if (_msgSenderERC721A() != owner) + if (!isApprovedForAll(owner, _msgSenderERC721A())) + if (getApproved(tokenId) != _msgSenderERC721A()) revert SetUserCallerNotOwnerNorApproved(); _packedUserInfo[tokenId] = (uint256(expires) << _BITPOS_EXPIRES) | uint256(uint160(user)); From 858ebdbda47f3bdbda00d1727c5766a2e03dc208 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 21:39:42 +0000 Subject: [PATCH 22/25] Fix merge --- contracts/ERC721A.sol | 6 +++--- contracts/mocks/ERC721ReceiverMock.sol | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 8f849bff7..45e0fe3f4 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -622,7 +622,7 @@ contract ERC721A is IERC721A { } /** - * @dev Returns whether the `msgSender` is equal to `owner` or `approvedAddress`. + * @dev Returns whether `msgSender` is equal to `approvedAddress` or `owner`. */ function _isSenderApprovedOrOwner( address approvedAddress, @@ -631,9 +631,9 @@ contract ERC721A is IERC721A { ) private pure returns (bool result) { assembly { // Mask `owner` to the lower 160 bits, in case the upper bits somehow aren't clean. - owner := and(owner, BITMASK_ADDRESS) + owner := and(owner, _BITMASK_ADDRESS) // Mask `msgSender` to the lower 160 bits, in case the upper bits somehow aren't clean. - msgSender := and(msgSender, BITMASK_ADDRESS) + msgSender := and(msgSender, _BITMASK_ADDRESS) // `msgSender == owner || msgSender == approvedAddress`. result := or(eq(msgSender, owner), eq(msgSender, approvedAddress)) } diff --git a/contracts/mocks/ERC721ReceiverMock.sol b/contracts/mocks/ERC721ReceiverMock.sol index 8ab1fed9f..1205a755b 100644 --- a/contracts/mocks/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721ReceiverMock.sol @@ -35,7 +35,7 @@ contract ERC721ReceiverMock is ERC721A__IERC721Receiver { bytes memory data ) public override returns (bytes4) { uint256 dataValue = data.length == 0 ? 0 : uint256(uint8(data[0])); - + // For testing reverts with a message from the receiver contract. if (dataValue == 0x01) { revert('reverted in the receiver contract!'); From 4498a4c0e73c6a433b08c4925db4333d17ebae01 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 22:34:55 +0000 Subject: [PATCH 23/25] Revert changes to ERC721A.sol --- contracts/ERC721A.sol | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/ERC721A.sol b/contracts/ERC721A.sol index 45e0fe3f4..9515d4231 100644 --- a/contracts/ERC721A.sol +++ b/contracts/ERC721A.sol @@ -608,7 +608,7 @@ contract ERC721A is IERC721A { /** * @dev Returns the storage slot and value for the approved address of `tokenId`. */ - function _getApprovedSlotAndAddress(uint256 tokenId) + function _getApprovedAddress(uint256 tokenId) private view returns (uint256 approvedAddressSlot, address approvedAddress) @@ -622,20 +622,20 @@ contract ERC721A is IERC721A { } /** - * @dev Returns whether `msgSender` is equal to `approvedAddress` or `owner`. + * @dev Returns whether the `approvedAddress` is equals to `from` or `msgSender`. */ - function _isSenderApprovedOrOwner( + function _isOwnerOrApproved( address approvedAddress, - address owner, + address from, address msgSender ) private pure returns (bool result) { assembly { - // Mask `owner` to the lower 160 bits, in case the upper bits somehow aren't clean. - owner := and(owner, _BITMASK_ADDRESS) + // 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 == owner || msgSender == approvedAddress`. - result := or(eq(msgSender, owner), eq(msgSender, approvedAddress)) + // `msgSender == from || msgSender == approvedAddress`. + result := or(eq(msgSender, from), eq(msgSender, approvedAddress)) } } @@ -658,10 +658,10 @@ contract ERC721A is IERC721A { if (address(uint160(prevOwnershipPacked)) != from) revert TransferFromIncorrectOwner(); - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedSlotAndAddress(tokenId); + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); // The nested ifs save around 20+ gas over a compound boolean condition. - if (!_isSenderApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); if (to == address(0)) revert TransferToZeroAddress(); @@ -734,11 +734,11 @@ contract ERC721A is IERC721A { address from = address(uint160(prevOwnershipPacked)); - (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedSlotAndAddress(tokenId); + (uint256 approvedAddressSlot, address approvedAddress) = _getApprovedAddress(tokenId); if (approvalCheck) { // The nested ifs save around 20+ gas over a compound boolean condition. - if (!_isSenderApprovedOrOwner(approvedAddress, from, _msgSenderERC721A())) + if (!_isOwnerOrApproved(approvedAddress, from, _msgSenderERC721A())) if (!isApprovedForAll(from, _msgSenderERC721A())) revert TransferCallerNotOwnerNorApproved(); } From fef2e2577bf2f49c1628793142588cc5f56b94f8 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 23:11:15 +0000 Subject: [PATCH 24/25] Add comment --- contracts/extensions/ERC4907A.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index b87a1228c..ed2489ab0 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -38,6 +38,8 @@ abstract contract ERC4907A is ERC721A, IERC4907A { address user, uint64 expires ) public virtual { + // Require the message sender to be either + // the token owner, an approved operator, or an approved address. address owner = ownerOf(tokenId); if (_msgSenderERC721A() != owner) if (!isApprovedForAll(owner, _msgSenderERC721A())) From 1e035d01b0a36d07987fbb281d19558dce53aefb Mon Sep 17 00:00:00 2001 From: Vectorized Date: Sun, 17 Jul 2022 23:17:40 +0000 Subject: [PATCH 25/25] Add comment on the lt --- contracts/extensions/ERC4907A.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/extensions/ERC4907A.sol b/contracts/extensions/ERC4907A.sol index ed2489ab0..7130e61c6 100644 --- a/contracts/extensions/ERC4907A.sol +++ b/contracts/extensions/ERC4907A.sol @@ -58,6 +58,8 @@ abstract contract ERC4907A is ERC721A, IERC4907A { uint256 packed = _packedUserInfo[tokenId]; assembly { // Branchless `packed *= (block.timestamp <= expires ? 1 : 0)`. + // If the `block.timestamp == expires`, the `lt` clause will be true + // if there is a non-zero user address in the lower 160 bits of `packed`. packed := mul( packed, // `block.timestamp <= expires ? 1 : 0`.