diff --git a/CHANGELOG.md b/CHANGELOG.md index 85d0f74a167..34edd4fa885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * `ERC721`: optimize burn by making approval clearing implicit instead of emitting an event. ([#3538](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3538)) * `ERC721`: Fix balance accounting when a custom `_beforeTokenTransfer` hook results in a transfer of the token under consideration. ([#3611](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3611)) * `ERC721`: use unchecked arithmetic for balance updates. ([#3524](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3524)) + * `ERC721Consecutive`: Implementation of EIP-2309 that allows batch minting of ERC721 tokens during construction. ([#3311](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3311)) * `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515)) * `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565)) * `ECDSA`: Remove redundant check on the `v` value. ([#3591](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3591)) @@ -44,10 +45,12 @@ +import "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; ``` -### Compatibility Note +### ERC-721 Compatibility Note ERC-721 integrators that interpret contract state from events should make sure that they implement the clearing of approval that is implicit in every transfer according to the EIP. Previous versions of OpenZeppelin Contracts emitted an explicit `Approval` event even though it was not required by the specification, and this is no longer the case. +With the new `ERC721Consecutive` extension, the internal workings of `ERC721` are slightly changed. Custom extensions to ERC721 should be reviewed to ensure they remain correct. The new internal functions that should be considered are `_ownerOf`, `_beforeConsecutiveTokenTransfer`, and `_afterConsecutiveTokenTransfer`, and the existing internal functions that should be reviewed are `_exists`, `_beforeTokenTransfer`, and `_afterTokenTransfer`. + ## 4.7.3 ### Breaking changes diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index 829d516c577..a93231ca987 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -184,7 +184,7 @@ abstract contract IGovernor is IERC165 { /** * @notice module:voting - * @dev Returns weither `account` has cast a vote on `proposalId`. + * @dev Returns whether `account` has cast a vote on `proposalId`. */ function hasVoted(uint256 proposalId, address account) public view virtual returns (bool); diff --git a/contracts/interfaces/IERC2309.sol b/contracts/interfaces/IERC2309.sol new file mode 100644 index 00000000000..9b2b42b1f77 --- /dev/null +++ b/contracts/interfaces/IERC2309.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +/** + * @dev ERC-2309: ERC-721 Consecutive Transfer Extension. + * + * _Available since v4.8._ + */ +interface IERC2309 { + /** + * @dev Emitted when the tokens from `fromTokenId` to `toTokenId` are transferred from `fromAddress` to `toAddress`. + */ + event ConsecutiveTransfer( + uint256 indexed fromTokenId, + uint256 toTokenId, + address indexed fromAddress, + address indexed toAddress + ); +} diff --git a/contracts/mocks/CheckpointsMock.sol b/contracts/mocks/CheckpointsMock.sol index d630f2e3443..f1dadabaf7d 100644 --- a/contracts/mocks/CheckpointsMock.sol +++ b/contracts/mocks/CheckpointsMock.sol @@ -14,6 +14,22 @@ contract CheckpointsMock { return _totalCheckpoints.latest(); } + function latestCheckpoint() + public + view + returns ( + bool, + uint256, + uint256 + ) + { + return _totalCheckpoints.latestCheckpoint(); + } + + function length() public view returns (uint256) { + return _totalCheckpoints.length(); + } + function push(uint256 value) public returns (uint256, uint256) { return _totalCheckpoints.push(value); } @@ -25,10 +41,6 @@ contract CheckpointsMock { function getAtProbablyRecentBlock(uint256 blockNumber) public view returns (uint256) { return _totalCheckpoints.getAtProbablyRecentBlock(blockNumber); } - - function length() public view returns (uint256) { - return _totalCheckpoints._checkpoints.length; - } } contract Checkpoints224Mock { @@ -40,6 +52,22 @@ contract Checkpoints224Mock { return _totalCheckpoints.latest(); } + function latestCheckpoint() + public + view + returns ( + bool, + uint32, + uint224 + ) + { + return _totalCheckpoints.latestCheckpoint(); + } + + function length() public view returns (uint256) { + return _totalCheckpoints.length(); + } + function push(uint32 key, uint224 value) public returns (uint224, uint224) { return _totalCheckpoints.push(key, value); } @@ -51,10 +79,6 @@ contract Checkpoints224Mock { function upperLookup(uint32 key) public view returns (uint224) { return _totalCheckpoints.upperLookup(key); } - - function length() public view returns (uint256) { - return _totalCheckpoints._checkpoints.length; - } } contract Checkpoints160Mock { @@ -66,6 +90,22 @@ contract Checkpoints160Mock { return _totalCheckpoints.latest(); } + function latestCheckpoint() + public + view + returns ( + bool, + uint96, + uint160 + ) + { + return _totalCheckpoints.latestCheckpoint(); + } + + function length() public view returns (uint256) { + return _totalCheckpoints.length(); + } + function push(uint96 key, uint160 value) public returns (uint160, uint160) { return _totalCheckpoints.push(key, value); } @@ -77,8 +117,4 @@ contract Checkpoints160Mock { function upperLookup(uint96 key) public view returns (uint160) { return _totalCheckpoints.upperLookup(key); } - - function length() public view returns (uint256) { - return _totalCheckpoints._checkpoints.length; - } } diff --git a/contracts/mocks/ERC721ConsecutiveMock.sol b/contracts/mocks/ERC721ConsecutiveMock.sol new file mode 100644 index 00000000000..11c93d188fb --- /dev/null +++ b/contracts/mocks/ERC721ConsecutiveMock.sol @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../token/ERC721/extensions/ERC721Burnable.sol"; +import "../token/ERC721/extensions/ERC721Consecutive.sol"; +import "../token/ERC721/extensions/ERC721Enumerable.sol"; +import "../token/ERC721/extensions/ERC721Pausable.sol"; +import "../token/ERC721/extensions/draft-ERC721Votes.sol"; + +/** + * @title ERC721ConsecutiveMock + */ +contract ERC721ConsecutiveMock is ERC721Burnable, ERC721Consecutive, ERC721Pausable, ERC721Votes { + constructor( + string memory name, + string memory symbol, + address[] memory delegates, + address[] memory receivers, + uint96[] memory amounts + ) ERC721(name, symbol) EIP712(name, "1") { + for (uint256 i = 0; i < delegates.length; ++i) { + _delegate(delegates[i], delegates[i]); + } + + for (uint256 i = 0; i < receivers.length; ++i) { + _mintConsecutive(receivers[i], amounts[i]); + } + } + + function pause() external { + _pause(); + } + + function unpause() external { + _unpause(); + } + + function exists(uint256 tokenId) public view returns (bool) { + return _exists(tokenId); + } + + function mint(address to, uint256 tokenId) public { + _mint(to, tokenId); + } + + function mintConsecutive(address to, uint96 amount) public { + _mintConsecutive(to, amount); + } + + function safeMint(address to, uint256 tokenId) public { + _safeMint(to, tokenId); + } + + function _ownerOf(uint256 tokenId) internal view virtual override(ERC721, ERC721Consecutive) returns (address) { + return super._ownerOf(tokenId); + } + + function _mint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Consecutive) { + super._mint(to, tokenId); + } + + function _beforeTokenTransfer( + address from, + address to, + uint256 tokenId + ) internal virtual override(ERC721, ERC721Pausable) { + super._beforeTokenTransfer(from, to, tokenId); + } + + function _afterTokenTransfer( + address from, + address to, + uint256 tokenId + ) internal virtual override(ERC721, ERC721Votes, ERC721Consecutive) { + super._afterTokenTransfer(from, to, tokenId); + } + + function _beforeConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override(ERC721, ERC721Pausable) { + super._beforeConsecutiveTokenTransfer(from, to, first, size); + } + + function _afterConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override(ERC721, ERC721Votes) { + super._afterConsecutiveTokenTransfer(from, to, first, size); + } +} + +contract ERC721ConsecutiveEnumerableMock is ERC721Consecutive, ERC721Enumerable { + constructor( + string memory name, + string memory symbol, + address[] memory receivers, + uint96[] memory amounts + ) ERC721(name, symbol) { + for (uint256 i = 0; i < receivers.length; ++i) { + _mintConsecutive(receivers[i], amounts[i]); + } + } + + function supportsInterface(bytes4 interfaceId) + public + view + virtual + override(ERC721, ERC721Enumerable) + returns (bool) + { + return super.supportsInterface(interfaceId); + } + + function _ownerOf(uint256 tokenId) internal view virtual override(ERC721, ERC721Consecutive) returns (address) { + return super._ownerOf(tokenId); + } + + function _mint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Consecutive) { + super._mint(to, tokenId); + } + + function _beforeTokenTransfer( + address from, + address to, + uint256 tokenId + ) internal virtual override(ERC721, ERC721Enumerable) { + super._beforeTokenTransfer(from, to, tokenId); + } + + function _afterTokenTransfer( + address from, + address to, + uint256 tokenId + ) internal virtual override(ERC721, ERC721Consecutive) { + super._afterTokenTransfer(from, to, tokenId); + } + + function _beforeConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override(ERC721, ERC721Enumerable) { + super._beforeConsecutiveTokenTransfer(from, to, first, size); + } +} + +contract ERC721ConsecutiveNoConstructorMintMock is ERC721Consecutive { + constructor(string memory name, string memory symbol) ERC721(name, symbol) { + _mint(msg.sender, 0); + } +} diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index ed4975ee48f..a8e5aa4abc9 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -68,7 +68,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * @dev See {IERC721-ownerOf}. */ function ownerOf(uint256 tokenId) public view virtual override returns (address) { - address owner = _owners[tokenId]; + address owner = _ownerOf(tokenId); require(owner != address(0), "ERC721: invalid token ID"); return owner; } @@ -210,6 +210,13 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { require(_checkOnERC721Received(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer"); } + /** + * @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist + */ + function _ownerOf(uint256 tokenId) internal view virtual returns (address) { + return _owners[tokenId]; + } + /** * @dev Returns whether `tokenId` exists. * @@ -219,7 +226,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * and stop existing when they are burned (`_burn`). */ function _exists(uint256 tokenId) internal view virtual returns (bool) { - return _owners[tokenId] != address(0); + return _ownerOf(tokenId) != address(0); } /** @@ -444,8 +451,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { } /** - * @dev Hook that is called before any token transfer. This includes minting - * and burning. + * @dev Hook that is called before any (single) token transfer. This includes minting and burning. + * See {_beforeConsecutiveTokenTransfer}. * * Calling conditions: * @@ -464,8 +471,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { ) internal virtual {} /** - * @dev Hook that is called after any transfer of tokens. This includes - * minting and burning. + * @dev Hook that is called after any (single) transfer of tokens. This includes minting and burning. + * See {_afterConsecutiveTokenTransfer}. * * Calling conditions: * @@ -479,4 +486,36 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { address to, uint256 tokenId ) internal virtual {} + + /** + * @dev Hook that is called before consecutive token transfers. + * Calling conditions are similar to {_beforeTokenTransfer}. + * + * The default implementation include balances updates that extensions such as {ERC721Consecutive} cannot perform + * directly. + */ + function _beforeConsecutiveTokenTransfer( + address from, + address to, + uint256, /*first*/ + uint96 size + ) internal virtual { + if (from != address(0)) { + _balances[from] -= size; + } + if (to != address(0)) { + _balances[to] += size; + } + } + + /** + * @dev Hook that is called after consecutive token transfers. + * Calling conditions are similar to {_afterTokenTransfer}. + */ + function _afterConsecutiveTokenTransfer( + address, /*from*/ + address, /*to*/ + uint256, /*first*/ + uint96 /*size*/ + ) internal virtual {} } diff --git a/contracts/token/ERC721/README.adoc b/contracts/token/ERC721/README.adoc index 92959576bc1..b3377afeff3 100644 --- a/contracts/token/ERC721/README.adoc +++ b/contracts/token/ERC721/README.adoc @@ -22,6 +22,7 @@ OpenZeppelin Contracts provides implementations of all four interfaces: Additionally there are a few of other extensions: +* {ERC721Consecutive}: An implementation of https://eips.ethereum.org/EIPS/eip-2309[ERC2309] for minting batchs of tokens during construction, in accordance with ERC721. * {ERC721URIStorage}: A more flexible but more expensive way of storing metadata. * {ERC721Votes}: Support for voting and vote delegation. * {ERC721Royalty}: A way to signal royalty information following ERC2981. @@ -50,6 +51,8 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC721Burnable}} +{{ERC721Consecutive}} + {{ERC721URIStorage}} {{ERC721Votes}} diff --git a/contracts/token/ERC721/extensions/ERC721Consecutive.sol b/contracts/token/ERC721/extensions/ERC721Consecutive.sol new file mode 100644 index 00000000000..15ecdd98650 --- /dev/null +++ b/contracts/token/ERC721/extensions/ERC721Consecutive.sol @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC721.sol"; +import "../../../interfaces/IERC2309.sol"; +import "../../../utils/Checkpoints.sol"; +import "../../../utils/math/SafeCast.sol"; +import "../../../utils/structs/BitMaps.sol"; + +/** + * @dev Implementation of the ERC2309 "Consecutive Transfer Extension" as defined in + * https://eips.ethereum.org/EIPS/eip-2309[EIP-2309]. + * + * This extension allows the minting of large batches of tokens during the contract construction. These batches are + * limited to 5000 tokens at a time to accommodate off-chain indexers. + * + * Using this extension removes the ability to mint single tokens during the contract construction. This ability is + * regained after construction. During construction, only batch minting is allowed. + * + * IMPORTANT: This extension bypasses the hooks {_beforeTokenTransfer} and {_afterTokenTransfer} for tokens minted in + * batch. When using this extension, you should consider the {_beforeConsecutiveTokenTransfer} and + * {_afterConsecutiveTokenTransfer} hooks in addition to {_beforeTokenTransfer} and {_afterTokenTransfer}. + * + * IMPORTANT: When overriding {_afterTokenTransfer}, be careful about call ordering. {ownerOf} may return invalid + * values during the {_afterTokenTransfer} execution if the super call is not called first. To be safe, execute the + * super call before your custom logic. + * + * _Available since v4.8._ + */ +abstract contract ERC721Consecutive is IERC2309, ERC721 { + using BitMaps for BitMaps.BitMap; + using Checkpoints for Checkpoints.Trace160; + + Checkpoints.Trace160 private _sequentialOwnership; + BitMaps.BitMap private _sequentialBurn; + + /** + * @dev See {ERC721-_ownerOf}. Override that checks the sequential ownership structure for tokens that have + * been minted as part of a batch, and not yet transferred. + */ + function _ownerOf(uint256 tokenId) internal view virtual override returns (address) { + address owner = super._ownerOf(tokenId); + + // If token is owned by the core, or beyond consecutive range, return base value + if (owner != address(0) || tokenId > type(uint96).max) { + return owner; + } + + // Otherwise, check the token was not burned, and fetch ownership from the anchors + // Note: no need for safe cast, we know that tokenId <= type(uint96).max + return _sequentialBurn.get(tokenId) ? address(0) : address(_sequentialOwnership.lowerLookup(uint96(tokenId))); + } + + /** + * @dev Mint a batch of tokens of length `batchSize` for `to`. + * + * WARNING: Consecutive mint is only available during construction. ERC721 requires that any minting done after + * construction emits a `Transfer` event, which is not the case of mints performed using this function. + * + * WARNING: Consecutive mint is limited to batches of 5000 tokens. Further minting is possible from a contract's + * point of view but would cause indexing issues for off-chain services. + * + * Emits a {ConsecutiveTransfer} event. + */ + function _mintConsecutive(address to, uint96 batchSize) internal virtual returns (uint96) { + uint96 first = _totalConsecutiveSupply(); + + // minting a batch of size 0 is a no-op + if (batchSize > 0) { + require(!Address.isContract(address(this)), "ERC721Consecutive: batch minting restricted to constructor"); + require(to != address(0), "ERC721Consecutive: mint to the zero address"); + require(batchSize <= 5000, "ERC721Consecutive: batch too large"); + + // hook before + _beforeConsecutiveTokenTransfer(address(0), to, first, batchSize); + + // push an ownership checkpoint & emit event + uint96 last = first + batchSize - 1; + _sequentialOwnership.push(last, uint160(to)); + emit ConsecutiveTransfer(first, last, address(0), to); + + // hook after + _afterConsecutiveTokenTransfer(address(0), to, first, batchSize); + } + + return first; + } + + /** + * @dev See {ERC721-_mint}. Override version that restricts normal minting to after construction. + * + * Warning: Using {ERC721Consecutive} prevents using {_mint} during construction in favor of {_mintConsecutive}. + * After construction, {_mintConsecutive} is no longer available and {_mint} becomes available. + */ + function _mint(address to, uint256 tokenId) internal virtual override { + require(Address.isContract(address(this)), "ERC721Consecutive: can't mint during construction"); + super._mint(to, tokenId); + } + + /** + * @dev See {ERC721-_afterTokenTransfer}. Burning of token that have been sequentially minted must be explicit. + */ + function _afterTokenTransfer( + address from, + address to, + uint256 tokenId + ) internal virtual override { + if ( + to == address(0) && // if we burn + tokenId <= _totalConsecutiveSupply() && // and the tokenId was minted is a batch + !_sequentialBurn.get(tokenId) // and the token was never marked as burnt + ) { + _sequentialBurn.set(tokenId); + } + super._afterTokenTransfer(from, to, tokenId); + } + + function _totalConsecutiveSupply() private view returns (uint96) { + (bool exists, uint96 latestId, ) = _sequentialOwnership.latestCheckpoint(); + return exists ? latestId + 1 : 0; + } +} diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol index 4c8a65c505e..c766133410d 100644 --- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol +++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol @@ -87,6 +87,30 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { } } + /** + * @dev Hook that is called before any batch token transfer. For now this is limited + * to batch minting by the {ERC721Consecutive} extension. + * + * Calling conditions: + * + * - When `from` and `to` are both non-zero, ``from``'s `tokenId` will be + * transferred to `to`. + * - When `from` is zero, `tokenId` will be minted for `to`. + * - When `to` is zero, ``from``'s `tokenId` will be burned. + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. + * + * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. + */ + function _beforeConsecutiveTokenTransfer( + address, + address, + uint256, + uint96 + ) internal virtual override { + revert("ERC721Enumerable: consecutive transfers not supported"); + } + /** * @dev Private function to add a token to this extension's ownership-tracking data structures. * @param to address representing the new owner of the given token ID diff --git a/contracts/token/ERC721/extensions/ERC721Pausable.sol b/contracts/token/ERC721/extensions/ERC721Pausable.sol index fbf8b638236..22163b0f7c9 100644 --- a/contracts/token/ERC721/extensions/ERC721Pausable.sol +++ b/contracts/token/ERC721/extensions/ERC721Pausable.sol @@ -30,4 +30,15 @@ abstract contract ERC721Pausable is ERC721, Pausable { require(!paused(), "ERC721Pausable: token transfer while paused"); } + + function _beforeConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override { + super._beforeConsecutiveTokenTransfer(from, to, first, size); + + require(!paused(), "ERC721Pausable: token transfer while paused"); + } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Votes.sol b/contracts/token/ERC721/extensions/draft-ERC721Votes.sol index b4ec91eab5e..1d23c78549d 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Votes.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Votes.sol @@ -31,6 +31,21 @@ abstract contract ERC721Votes is ERC721, Votes { super._afterTokenTransfer(from, to, tokenId); } + /** + * @dev Adjusts votes when a batch of tokens is transferred. + * + * Emits a {Votes-DelegateVotesChanged} event. + */ + function _afterConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override { + _transferVotingUnits(from, to, size); + super._afterConsecutiveTokenTransfer(from, to, first, size); + } + /** * @dev Returns the balance of `account`. */ diff --git a/contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol b/contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol index 11b9787800d..65ffef44b9b 100644 --- a/contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol +++ b/contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol @@ -124,6 +124,15 @@ contract ERC721PresetMinterPauserAutoId is super._beforeTokenTransfer(from, to, tokenId); } + function _beforeConsecutiveTokenTransfer( + address from, + address to, + uint256 first, + uint96 size + ) internal virtual override(ERC721, ERC721Enumerable, ERC721Pausable) { + super._beforeConsecutiveTokenTransfer(from, to, first, size); + } + /** * @dev See {IERC165-supportsInterface}. */ diff --git a/contracts/utils/Checkpoints.sol b/contracts/utils/Checkpoints.sol index 6199d8d7773..c8a062ae7d1 100644 --- a/contracts/utils/Checkpoints.sol +++ b/contracts/utils/Checkpoints.sol @@ -26,14 +26,6 @@ library Checkpoints { uint224 _value; } - /** - * @dev Returns the value in the latest checkpoint, or zero if there are no checkpoints. - */ - function latest(History storage self) internal view returns (uint256) { - uint256 pos = self._checkpoints.length; - return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; - } - /** * @dev Returns the value at a given block number. If a checkpoint is not available at that block, the closest one * before it is returned, or zero otherwise. @@ -99,6 +91,43 @@ library Checkpoints { return push(self, op(latest(self), delta)); } + /** + * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. + */ + function latest(History storage self) internal view returns (uint224) { + uint256 pos = self._checkpoints.length; + return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; + } + + /** + * @dev Returns whether there is a checkpoint in the structure (i.e. it is not empty), and if so the key and value + * in the most recent checkpoint. + */ + function latestCheckpoint(History storage self) + internal + view + returns ( + bool exists, + uint32 _blockNumber, + uint224 _value + ) + { + uint256 pos = self._checkpoints.length; + if (pos == 0) { + return (false, 0, 0); + } else { + Checkpoint memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + return (true, ckpt._blockNumber, ckpt._value); + } + } + + /** + * @dev Returns the number of checkpoint. + */ + function length(History storage self) internal view returns (uint256) { + return self._checkpoints.length; + } + /** * @dev Pushes a (`key`, `value`) pair into an ordered list of checkpoints, either by inserting a new checkpoint, * or by updating the last one. @@ -192,14 +221,6 @@ library Checkpoints { uint224 _value; } - /** - * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. - */ - function latest(Trace224 storage self) internal view returns (uint224) { - uint256 pos = self._checkpoints.length; - return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; - } - /** * @dev Pushes a (`key`, `value`) pair into a Trace224 so that it is stored as the checkpoint. * @@ -231,6 +252,43 @@ library Checkpoints { return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; } + /** + * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. + */ + function latest(Trace224 storage self) internal view returns (uint224) { + uint256 pos = self._checkpoints.length; + return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; + } + + /** + * @dev Returns whether there is a checkpoint in the structure (i.e. it is not empty), and if so the key and value + * in the most recent checkpoint. + */ + function latestCheckpoint(Trace224 storage self) + internal + view + returns ( + bool exists, + uint32 _key, + uint224 _value + ) + { + uint256 pos = self._checkpoints.length; + if (pos == 0) { + return (false, 0, 0); + } else { + Checkpoint224 memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + return (true, ckpt._key, ckpt._value); + } + } + + /** + * @dev Returns the number of checkpoint. + */ + function length(Trace224 storage self) internal view returns (uint256) { + return self._checkpoints.length; + } + /** * @dev Pushes a (`key`, `value`) pair into an ordered list of checkpoints, either by inserting a new checkpoint, * or by updating the last one. @@ -328,14 +386,6 @@ library Checkpoints { uint160 _value; } - /** - * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. - */ - function latest(Trace160 storage self) internal view returns (uint160) { - uint256 pos = self._checkpoints.length; - return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; - } - /** * @dev Pushes a (`key`, `value`) pair into a Trace160 so that it is stored as the checkpoint. * @@ -367,6 +417,43 @@ library Checkpoints { return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; } + /** + * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. + */ + function latest(Trace160 storage self) internal view returns (uint160) { + uint256 pos = self._checkpoints.length; + return pos == 0 ? 0 : _unsafeAccess(self._checkpoints, pos - 1)._value; + } + + /** + * @dev Returns whether there is a checkpoint in the structure (i.e. it is not empty), and if so the key and value + * in the most recent checkpoint. + */ + function latestCheckpoint(Trace160 storage self) + internal + view + returns ( + bool exists, + uint96 _key, + uint160 _value + ) + { + uint256 pos = self._checkpoints.length; + if (pos == 0) { + return (false, 0, 0); + } else { + Checkpoint160 memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + return (true, ckpt._key, ckpt._value); + } + } + + /** + * @dev Returns the number of checkpoint. + */ + function length(Trace160 storage self) internal view returns (uint256) { + return self._checkpoints.length; + } + /** * @dev Pushes a (`key`, `value`) pair into an ordered list of checkpoints, either by inserting a new checkpoint, * or by updating the last one. diff --git a/scripts/generate/templates/Checkpoints.js b/scripts/generate/templates/Checkpoints.js index e0bdd2055ee..61c398d8fd9 100644 --- a/scripts/generate/templates/Checkpoints.js +++ b/scripts/generate/templates/Checkpoints.js @@ -32,14 +32,6 @@ struct ${opts.checkpointTypeName} { /* eslint-disable max-len */ const operations = opts => `\ -/** - * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. - */ -function latest(${opts.historyTypeName} storage self) internal view returns (${opts.valueTypeName}) { - uint256 pos = self.${opts.checkpointFieldName}.length; - return pos == 0 ? 0 : _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1).${opts.valueFieldName}; -} - /** * @dev Pushes a (\`key\`, \`value\`) pair into a ${opts.historyTypeName} so that it is stored as the checkpoint. * @@ -73,14 +65,6 @@ function upperLookup(${opts.historyTypeName} storage self, ${opts.keyTypeName} k `; const legacyOperations = opts => `\ -/** - * @dev Returns the value in the latest checkpoint, or zero if there are no checkpoints. - */ -function latest(${opts.historyTypeName} storage self) internal view returns (uint256) { - uint256 pos = self.${opts.checkpointFieldName}.length; - return pos == 0 ? 0 : _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1).${opts.valueFieldName}; -} - /** * @dev Returns the value at a given block number. If a checkpoint is not available at that block, the closest one * before it is returned, or zero otherwise. @@ -147,7 +131,44 @@ function push( } `; -const helpers = opts => `\ +const common = opts => `\ +/** + * @dev Returns the value in the most recent checkpoint, or zero if there are no checkpoints. + */ +function latest(${opts.historyTypeName} storage self) internal view returns (${opts.valueTypeName}) { + uint256 pos = self.${opts.checkpointFieldName}.length; + return pos == 0 ? 0 : _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1).${opts.valueFieldName}; +} + +/** + * @dev Returns whether there is a checkpoint in the structure (i.e. it is not empty), and if so the key and value + * in the most recent checkpoint. + */ +function latestCheckpoint(${opts.historyTypeName} storage self) + internal + view + returns ( + bool exists, + ${opts.keyTypeName} ${opts.keyFieldName}, + ${opts.valueTypeName} ${opts.valueFieldName} + ) +{ + uint256 pos = self.${opts.checkpointFieldName}.length; + if (pos == 0) { + return (false, 0, 0); + } else { + ${opts.checkpointTypeName} memory ckpt = _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1); + return (true, ckpt.${opts.keyFieldName}, ckpt.${opts.valueFieldName}); + } +} + +/** + * @dev Returns the number of checkpoint. + */ +function length(${opts.historyTypeName} storage self) internal view returns (uint256) { + return self.${opts.checkpointFieldName}.length; +} + /** * @dev Pushes a (\`key\`, \`value\`) pair into an ordered list of checkpoints, either by inserting a new checkpoint, * or by updating the last one. @@ -266,12 +287,12 @@ module.exports = format( // Legacy types & functions types(LEGACY_OPTS), legacyOperations(LEGACY_OPTS), - helpers(LEGACY_OPTS), + common(LEGACY_OPTS), // New flavors ...OPTS.flatMap(opts => [ types(opts), operations(opts), - helpers(opts), + common(opts), ]), ], '}', diff --git a/scripts/generate/templates/CheckpointsMock.js b/scripts/generate/templates/CheckpointsMock.js index 2feb112409e..145f0840802 100755 --- a/scripts/generate/templates/CheckpointsMock.js +++ b/scripts/generate/templates/CheckpointsMock.js @@ -18,6 +18,14 @@ contract CheckpointsMock { return _totalCheckpoints.latest(); } + function latestCheckpoint() public view returns (bool, uint256, uint256) { + return _totalCheckpoints.latestCheckpoint(); + } + + function length() public view returns (uint256) { + return _totalCheckpoints.length(); + } + function push(uint256 value) public returns (uint256, uint256) { return _totalCheckpoints.push(value); } @@ -29,10 +37,6 @@ contract CheckpointsMock { function getAtProbablyRecentBlock(uint256 blockNumber) public view returns (uint256) { return _totalCheckpoints.getAtProbablyRecentBlock(blockNumber); } - - function length() public view returns (uint256) { - return _totalCheckpoints._checkpoints.length; - } } `; @@ -46,6 +50,14 @@ contract Checkpoints${length}Mock { return _totalCheckpoints.latest(); } + function latestCheckpoint() public view returns (bool, uint${256 - length}, uint${length}) { + return _totalCheckpoints.latestCheckpoint(); + } + + function length() public view returns (uint256) { + return _totalCheckpoints.length(); + } + function push(uint${256 - length} key, uint${length} value) public returns (uint${length}, uint${length}) { return _totalCheckpoints.push(key, value); } @@ -57,10 +69,6 @@ contract Checkpoints${length}Mock { function upperLookup(uint${256 - length} key) public view returns (uint${length}) { return _totalCheckpoints.upperLookup(key); } - - function length() public view returns (uint256) { - return _totalCheckpoints._checkpoints.length; - } } `; diff --git a/test/token/ERC721/extensions/ERC721Consecutive.test.js b/test/token/ERC721/extensions/ERC721Consecutive.test.js new file mode 100644 index 00000000000..de3af0a077a --- /dev/null +++ b/test/token/ERC721/extensions/ERC721Consecutive.test.js @@ -0,0 +1,191 @@ +const { constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); + +const ERC721ConsecutiveMock = artifacts.require('ERC721ConsecutiveMock'); +const ERC721ConsecutiveEnumerableMock = artifacts.require('ERC721ConsecutiveEnumerableMock'); +const ERC721ConsecutiveNoConstructorMintMock = artifacts.require('ERC721ConsecutiveNoConstructorMintMock'); + +contract('ERC721Consecutive', function (accounts) { + const [ user1, user2, user3, receiver ] = accounts; + + const name = 'Non Fungible Token'; + const symbol = 'NFT'; + const batches = [ + { receiver: user1, amount: 0 }, + { receiver: user1, amount: 3 }, + { receiver: user2, amount: 5 }, + { receiver: user3, amount: 0 }, + { receiver: user1, amount: 7 }, + ]; + const delegates = [ user1, user3 ]; + + describe('with valid batches', function () { + beforeEach(async function () { + this.token = await ERC721ConsecutiveMock.new( + name, + symbol, + delegates, + batches.map(({ receiver }) => receiver), + batches.map(({ amount }) => amount), + ); + }); + + describe('minting during construction', function () { + it('events are emitted at construction', async function () { + let first = 0; + + for (const batch of batches) { + if (batch.amount > 0) { + await expectEvent.inConstruction(this.token, 'ConsecutiveTransfer', { + fromTokenId: web3.utils.toBN(first), + toTokenId: web3.utils.toBN(first + batch.amount - 1), + fromAddress: constants.ZERO_ADDRESS, + toAddress: batch.receiver, + }); + } else { + // expectEvent.notEmitted.inConstruction only looks at event name, and doesn't check the parameters + } + first += batch.amount; + } + }); + + it('ownership is set', async function () { + const owners = batches.flatMap(({ receiver, amount }) => Array(amount).fill(receiver)); + + for (const tokenId in owners) { + expect(await this.token.ownerOf(tokenId)) + .to.be.equal(owners[tokenId]); + } + }); + + it('balance & voting power are set', async function () { + for (const account of accounts) { + const balance = batches + .filter(({ receiver }) => receiver === account) + .map(({ amount }) => amount) + .reduce((a, b) => a + b, 0); + + expect(await this.token.balanceOf(account)) + .to.be.bignumber.equal(web3.utils.toBN(balance)); + + // If not delegated at construction, check before + do delegation + if (!delegates.includes(account)) { + expect(await this.token.getVotes(account)) + .to.be.bignumber.equal(web3.utils.toBN(0)); + + await this.token.delegate(account, { from: account }); + } + + // At this point all accounts should have delegated + expect(await this.token.getVotes(account)) + .to.be.bignumber.equal(web3.utils.toBN(balance)); + } + }); + }); + + describe('minting after construction', function () { + it('consecutive minting is not possible after construction', async function () { + await expectRevert( + this.token.mintConsecutive(user1, 10), + 'ERC721Consecutive: batch minting restricted to constructor', + ); + }); + + it('simple minting is possible after construction', async function () { + const tokenId = batches.reduce((acc, { amount }) => acc + amount, 0); + + expect(await this.token.exists(tokenId)).to.be.equal(false); + + expectEvent( + await this.token.mint(user1, tokenId), + 'Transfer', + { from: constants.ZERO_ADDRESS, to: user1, tokenId: tokenId.toString() }, + ); + }); + + it('cannot mint a token that has been batched minted', async function () { + const tokenId = batches.reduce((acc, { amount }) => acc + amount, 0) - 1; + + expect(await this.token.exists(tokenId)).to.be.equal(true); + + await expectRevert( + this.token.mint(user1, tokenId), + 'ERC721: token already minted', + ); + }); + }); + + describe('ERC721 behavior', function () { + it('core takes over ownership on transfer', async function () { + await this.token.transferFrom(user1, receiver, 1, { from: user1 }); + + expect(await this.token.ownerOf(1)).to.be.equal(receiver); + }); + + it('tokens can be burned and re-minted', async function () { + expectEvent( + await this.token.burn(1, { from: user1 }), + 'Transfer', + { from: user1, to: constants.ZERO_ADDRESS, tokenId: '1' }, + ); + + await expectRevert(this.token.ownerOf(1), 'ERC721: invalid token ID'); + + expectEvent( + await this.token.mint(user2, 1), + 'Transfer', + { from: constants.ZERO_ADDRESS, to: user2, tokenId: '1' }, + ); + + expect(await this.token.ownerOf(1)).to.be.equal(user2); + }); + }); + }); + + describe('invalid use', function () { + it('cannot mint a batch larger than 5000', async function () { + await expectRevert( + ERC721ConsecutiveMock.new( + name, + symbol, + [], + [user1], + ['5001'], + ), + 'ERC721Consecutive: batch too large', + ); + }); + + it('cannot use single minting during construction', async function () { + await expectRevert( + ERC721ConsecutiveNoConstructorMintMock.new( + name, + symbol, + ), + 'ERC721Consecutive: can\'t mint during construction', + ); + }); + + it('cannot use single minting during construction', async function () { + await expectRevert( + ERC721ConsecutiveNoConstructorMintMock.new( + name, + symbol, + ), + 'ERC721Consecutive: can\'t mint during construction', + ); + }); + + it('consecutive mint not compatible with enumerability', async function () { + await expectRevert( + ERC721ConsecutiveEnumerableMock.new( + name, + symbol, + batches.map(({ receiver }) => receiver), + batches.map(({ amount }) => amount), + ), + 'ERC721Enumerable: consecutive transfers not supported', + ); + }); + }); +}); diff --git a/test/utils/Checkpoints.test.js b/test/utils/Checkpoints.test.js index 28525729f0d..9bdb4aef2ce 100644 --- a/test/utils/Checkpoints.test.js +++ b/test/utils/Checkpoints.test.js @@ -18,6 +18,11 @@ contract('Checkpoints', function (accounts) { describe('without checkpoints', function () { it('returns zero as latest value', async function () { expect(await this.checkpoint.latest()).to.be.bignumber.equal('0'); + + const ckpt = await this.checkpoint.latestCheckpoint(); + expect(ckpt[0]).to.be.equal(false); + expect(ckpt[1]).to.be.bignumber.equal('0'); + expect(ckpt[2]).to.be.bignumber.equal('0'); }); it('returns zero as past value', async function () { @@ -41,6 +46,11 @@ contract('Checkpoints', function (accounts) { it('returns latest value', async function () { expect(await this.checkpoint.latest()).to.be.bignumber.equal('3'); + + const ckpt = await this.checkpoint.latestCheckpoint(); + expect(ckpt[0]).to.be.equal(true); + expect(ckpt[1]).to.be.bignumber.equal(web3.utils.toBN(this.tx3.receipt.blockNumber)); + expect(ckpt[2]).to.be.bignumber.equal(web3.utils.toBN('3')); }); for (const fn of [ 'getAtBlock(uint256)', 'getAtProbablyRecentBlock(uint256)' ]) { @@ -104,6 +114,11 @@ contract('Checkpoints', function (accounts) { describe('without checkpoints', function () { it('returns zero as latest value', async function () { expect(await this.contract.latest()).to.be.bignumber.equal('0'); + + const ckpt = await this.contract.latestCheckpoint(); + expect(ckpt[0]).to.be.equal(false); + expect(ckpt[1]).to.be.bignumber.equal('0'); + expect(ckpt[2]).to.be.bignumber.equal('0'); }); it('lookup returns 0', async function () { @@ -115,11 +130,11 @@ contract('Checkpoints', function (accounts) { describe('with checkpoints', function () { beforeEach('pushing checkpoints', async function () { this.checkpoints = [ - { key: 2, value: '17' }, - { key: 3, value: '42' }, - { key: 5, value: '101' }, - { key: 7, value: '23' }, - { key: 11, value: '99' }, + { key: '2', value: '17' }, + { key: '3', value: '42' }, + { key: '5', value: '101' }, + { key: '7', value: '23' }, + { key: '11', value: '99' }, ]; for (const { key, value } of this.checkpoints) { await this.contract.push(key, value); @@ -127,8 +142,12 @@ contract('Checkpoints', function (accounts) { }); it('returns latest value', async function () { - expect(await this.contract.latest()) - .to.be.bignumber.equal(last(this.checkpoints).value); + expect(await this.contract.latest()).to.be.bignumber.equal(last(this.checkpoints).value); + + const ckpt = await this.contract.latestCheckpoint(); + expect(ckpt[0]).to.be.equal(true); + expect(ckpt[1]).to.be.bignumber.equal(last(this.checkpoints).key); + expect(ckpt[2]).to.be.bignumber.equal(last(this.checkpoints).value); }); it('cannot push values in the past', async function () {