Parcourir la source

Use the _update mechanism in ERC721 (#4377)

Co-authored-by: Francisco Giordano <fg@frang.io>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
Hadrien Croubois il y a 2 ans
Parent
commit
9e3f4d60c5

+ 1 - 1
.changeset/bright-tomatoes-sing.md

@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`ERC20`, `ERC1155`: Deleted `_beforeTokenTransfer` and `_afterTokenTransfer` hooks, added a new internal `_update` function for customizations, and refactored all extensions using those hooks to use `_update` instead. ([#3838](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3838), [#3876](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3876))
+`ERC20`, `ERC721`, `ERC1155`: Deleted `_beforeTokenTransfer` and `_afterTokenTransfer` hooks, added a new internal `_update` function for customizations, and refactored all extensions using those hooks to use `_update` instead. ([#3838](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3838), [#3876](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3876), [#4377](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4377))

+ 5 - 0
.changeset/eighty-lemons-shake.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`ERC721`: `_approve` no longer allows approving the owner of the tokenId. `_setApprovalForAll` no longer allows setting address(0) as an operator.

+ 13 - 1
CHANGELOG.md

@@ -36,7 +36,7 @@ These removals were implemented in the following PRs: [#3637](https://github.com
 
 These breaking changes will require modifications to ERC20, ERC721, and ERC1155 contracts, since the `_afterTokenTransfer` and `_beforeTokenTransfer` functions were removed. Any customization made through those hooks should now be done overriding the new `_update` function instead.
 
-Minting and burning are implemented by `_update` and customizations should be done by overriding this function as well. `_mint` and `_burn` are no longer virtual (meaning they are not overridable) to guard against possible inconsistencies.
+Minting and burning are implemented by `_update` and customizations should be done by overriding this function as well. `_transfer`, `_mint` and `_burn` are no longer virtual (meaning they are not overridable) to guard against possible inconsistencies.
 
 For example, a contract using `ERC20`'s `_beforeTokenTransfer` hook would have to be changed in the following way.
 
@@ -53,6 +53,18 @@ For example, a contract using `ERC20`'s `_beforeTokenTransfer` hook would have t
   }
 ```
 
+### More about ERC721
+
+In the case of `ERC721`, the `_update` function does not include a `from` parameter, as the sender is implicitly the previous owner of the `tokenId`. The address of
+this previous owner is returned by the `_update` function, so it can be used for a posteriori checks. In addition to `to` and `tokenId`, a third parameter (`auth`) is
+present in this function. This parameter enabled an optional check that the caller/spender is approved to do the transfer. This check cannot be performed after the transfer (because the transfer resets the approval), and doing it before `_update` would require a duplicate call to `_ownerOf`.
+
+In this logic of removing hidden SLOADs, the `_isApprovedOrOwner` function was removed in favor of a new `_isAuthorized` function. Overrides that used to target the
+`_isApprovedOrOwner` should now be performed on the `_isAuthorized` function. Calls to `_isApprovedOrOwner` that preceded a call to `_transfer`, `_burn` or `_approve`
+should be removed in favor of using the `auth` argument in `_update` and `_approve`. This is showcased in `ERC721Burnable.burn` and in `ERC721Wrapper.withdrawTo`.
+
+The `_exists` function was removed. Calls to this function can be replaced by `_ownerOf(tokenId) != address(0)`.
+
 #### ERC165Storage
 
 Users that were registering EIP-165 interfaces with `_registerInterface` from `ERC165Storage` should instead do so so by overriding the `supportsInterface` function as seen below:

+ 7 - 17
contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol

@@ -28,25 +28,15 @@ contract ERC721ConsecutiveEnumerableMock is ERC721Consecutive, ERC721Enumerable
         return super._ownerOf(tokenId);
     }
 
-    function _mint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Consecutive) {
-        super._mint(to, tokenId);
-    }
-
-    function _beforeTokenTransfer(
-        address from,
+    function _update(
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Enumerable) {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
+        uint256 tokenId,
+        address auth
+    ) internal virtual override(ERC721Consecutive, ERC721Enumerable) returns (address) {
+        return super._update(to, tokenId, auth);
     }
 
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Consecutive) {
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+    function _increaseBalance(address account, uint128 amount) internal virtual override(ERC721, ERC721Enumerable) {
+        super._increaseBalance(account, amount);
     }
 }

+ 7 - 17
contracts/mocks/token/ERC721ConsecutiveMock.sol

@@ -41,26 +41,16 @@ contract ERC721ConsecutiveMock is ERC721Consecutive, ERC721Pausable, ERC721Votes
         return super._ownerOf(tokenId);
     }
 
-    function _mint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Consecutive) {
-        super._mint(to, tokenId);
-    }
-
-    function _beforeTokenTransfer(
-        address from,
+    function _update(
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Pausable) {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
+        uint256 tokenId,
+        address auth
+    ) internal virtual override(ERC721Consecutive, ERC721Pausable, ERC721Votes) returns (address) {
+        return super._update(to, tokenId, auth);
     }
 
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Votes, ERC721Consecutive) {
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+    function _increaseBalance(address account, uint128 amount) internal virtual override(ERC721, ERC721Votes) {
+        super._increaseBalance(account, amount);
     }
 }
 

+ 180 - 203
contracts/token/ERC721/ERC721.sol

@@ -109,16 +109,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721-approve}.
      */
     function approve(address to, uint256 tokenId) public virtual {
-        address owner = ownerOf(tokenId);
-        if (to == owner) {
-            revert ERC721InvalidOperator(owner);
-        }
-
-        if (_msgSender() != owner && !isApprovedForAll(owner, _msgSender())) {
-            revert ERC721InvalidApprover(_msgSender());
-        }
-
-        _approve(to, tokenId);
+        _approve(to, tokenId, _msgSender());
     }
 
     /**
@@ -127,7 +118,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     function getApproved(uint256 tokenId) public view virtual returns (address) {
         _requireMinted(tokenId);
 
-        return _tokenApprovals[tokenId];
+        return _getApproved(tokenId);
     }
 
     /**
@@ -148,17 +139,21 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721-transferFrom}.
      */
     function transferFrom(address from, address to, uint256 tokenId) public virtual {
-        if (!_isApprovedOrOwner(_msgSender(), tokenId)) {
-            revert ERC721InsufficientApproval(_msgSender(), tokenId);
+        if (to == address(0)) {
+            revert ERC721InvalidReceiver(address(0));
+        }
+        // Setting an "auth" arguments enables the `_isAuthorized` check which verifies that the token exists
+        // (from != 0). Therefore, it is not needed to verify that the return value is not 0 here.
+        address previousOwner = _update(to, tokenId, _msgSender());
+        if (previousOwner != from) {
+            revert ERC721IncorrectOwner(from, tokenId, previousOwner);
         }
-
-        _transfer(from, to, tokenId);
     }
 
     /**
      * @dev See {IERC721-safeTransferFrom}.
      */
-    function safeTransferFrom(address from, address to, uint256 tokenId) public virtual {
+    function safeTransferFrom(address from, address to, uint256 tokenId) public {
         safeTransferFrom(from, to, tokenId, "");
     }
 
@@ -166,91 +161,113 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721-safeTransferFrom}.
      */
     function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual {
-        if (!_isApprovedOrOwner(_msgSender(), tokenId)) {
-            revert ERC721InsufficientApproval(_msgSender(), tokenId);
-        }
-        _safeTransfer(from, to, tokenId, data);
+        transferFrom(from, to, tokenId);
+        _checkOnERC721Received(from, to, tokenId, data);
     }
 
     /**
-     * @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients
-     * are aware of the ERC721 protocol to prevent tokens from being forever locked.
-     *
-     * `data` is additional data, it has no specified format and it is sent in call to `to`.
-     *
-     * This internal function is equivalent to {safeTransferFrom}, and can be used to e.g.
-     * implement alternative mechanisms to perform token transfer, such as signature-based.
-     *
-     * Requirements:
-     *
-     * - `from` cannot be the zero address.
-     * - `to` cannot be the zero address.
-     * - `tokenId` token must exist and be owned by `from`.
-     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     * @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist
      *
-     * Emits a {Transfer} event.
+     * IMPORTANT: Any overrides to this function that add ownership of tokens not tracked by the
+     * core ERC721 logic MUST be matched with the use of {_increaseBalance} to keep balances
+     * consistent with ownership. The invariant to preserve is that for any address `a` the value returned by
+     * `balanceOf(a)` must be equal to the number of tokens such that `_ownerOf(tokenId)` is `a`.
      */
-    function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
-        _transfer(from, to, tokenId);
-        if (!_checkOnERC721Received(from, to, tokenId, data)) {
-            revert ERC721InvalidReceiver(to);
-        }
+    function _ownerOf(uint256 tokenId) internal view virtual returns (address) {
+        return _owners[tokenId];
     }
 
     /**
-     * @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist
+     * @dev Returns the approved address for `tokenId`. Returns 0 if `tokenId` is not minted.
      */
-    function _ownerOf(uint256 tokenId) internal view virtual returns (address) {
-        return _owners[tokenId];
+    function _getApproved(uint256 tokenId) internal view virtual returns (address) {
+        return _tokenApprovals[tokenId];
     }
 
     /**
-     * @dev Returns whether `tokenId` exists.
-     *
-     * Tokens can be managed by their owner or approved accounts via {approve} or {setApprovalForAll}.
+     * @dev Returns whether `spender` is allowed to manage `owner`'s tokens, or `tokenId` in
+     * particular (ignoring whether it is owned by `owner`).
      *
-     * Tokens start existing when they are minted (`_mint`),
-     * and stop existing when they are burned (`_burn`).
+     * WARNING: This function assumes that `owner` is the actual owner of `tokenId` and does not
+     * verify this assumption.
      */
-    function _exists(uint256 tokenId) internal view virtual returns (bool) {
-        return _ownerOf(tokenId) != address(0);
+    function _isAuthorized(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) {
+        return
+            spender != address(0) &&
+            (owner == spender || isApprovedForAll(owner, spender) || _getApproved(tokenId) == spender);
     }
 
     /**
-     * @dev Returns whether `spender` is allowed to manage `tokenId`.
+     * @dev Checks if `spender` can operate on `tokenId`, assuming the provided `owner` is the actual owner.
+     * Reverts if `spender` has not approval for all assets of the provided `owner` nor the actual owner approved the `spender` for the specific `tokenId`.
      *
-     * Requirements:
-     *
-     * - `tokenId` must exist.
+     * WARNING: This function relies on {_isAuthorized}, so it doesn't check whether `owner` is the
+     * actual owner of `tokenId`.
      */
-    function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) {
-        address owner = ownerOf(tokenId);
-        return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender);
+    function _checkAuthorized(address owner, address spender, uint256 tokenId) internal view virtual {
+        if (!_isAuthorized(owner, spender, tokenId)) {
+            if (owner == address(0)) {
+                revert ERC721NonexistentToken(tokenId);
+            } else {
+                revert ERC721InsufficientApproval(spender, tokenId);
+            }
+        }
     }
 
     /**
-     * @dev Safely mints `tokenId` and transfers it to `to`.
-     *
-     * Requirements:
+     * @dev Unsafe write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
      *
-     * - `tokenId` must not exist.
-     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     * NOTE: the value is limited to type(uint128).max. This protect against _balance overflow. It is unrealistic that
+     * a uint256 would ever overflow from increments when these increments are bounded to uint128 values.
      *
-     * Emits a {Transfer} event.
+     * WARNING: Increasing an account's balance using this function tends to be paired with an override of the
+     * {_ownerOf} function to resolve the ownership of the corresponding tokens so that balances and ownership
+     * remain consistent with one another.
      */
-    function _safeMint(address to, uint256 tokenId) internal virtual {
-        _safeMint(to, tokenId, "");
+    function _increaseBalance(address account, uint128 value) internal virtual {
+        unchecked {
+            _balances[account] += value;
+        }
     }
 
     /**
-     * @dev Same as {xref-ERC721-_safeMint-address-uint256-}[`_safeMint`], with an additional `data` parameter which is
-     * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
+     * @dev Transfers `tokenId` from its current owner to `to`, or alternatively mints (or burns) if the current owner
+     * (or `to`) is the zero address. Returns the owner of the `tokenId` before the update.
+     *
+     * The `auth` argument is optional. If the value passed is non 0, then this function will check that
+     * `auth` is either the owner of the token, or approved to operate on the token (by the owner).
+     *
+     * Emits a {Transfer} event.
+     *
+     * NOTE: If overriding this function in a way that tracks balances, see also {_increaseBalance}.
      */
-    function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual {
-        _mint(to, tokenId);
-        if (!_checkOnERC721Received(address(0), to, tokenId, data)) {
-            revert ERC721InvalidReceiver(to);
+    function _update(address to, uint256 tokenId, address auth) internal virtual returns (address) {
+        address from = _ownerOf(tokenId);
+
+        // Perform (optional) operator check
+        if (auth != address(0)) {
+            _checkAuthorized(from, auth, tokenId);
+        }
+
+        // Execute the update
+        if (from != address(0)) {
+            delete _tokenApprovals[tokenId];
+            unchecked {
+                _balances[from] -= 1;
+            }
+        }
+
+        if (to != address(0)) {
+            unchecked {
+                _balances[to] += 1;
+            }
         }
+
+        _owners[tokenId] = to;
+
+        emit Transfer(from, to, tokenId);
+
+        return from;
     }
 
     /**
@@ -265,34 +282,37 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Emits a {Transfer} event.
      */
-    function _mint(address to, uint256 tokenId) internal virtual {
+    function _mint(address to, uint256 tokenId) internal {
         if (to == address(0)) {
             revert ERC721InvalidReceiver(address(0));
         }
-        if (_exists(tokenId)) {
+        address previousOwner = _update(to, tokenId, address(0));
+        if (previousOwner != address(0)) {
             revert ERC721InvalidSender(address(0));
         }
+    }
 
-        _beforeTokenTransfer(address(0), to, tokenId, 1);
-
-        // Check that tokenId was not minted by `_beforeTokenTransfer` hook
-        if (_exists(tokenId)) {
-            revert ERC721InvalidSender(address(0));
-        }
-
-        unchecked {
-            // Will not overflow unless all 2**256 token ids are minted to the same owner.
-            // Given that tokens are minted one by one, it is impossible in practice that
-            // this ever happens. Might change if we allow batch minting.
-            // The ERC fails to describe this case.
-            _balances[to] += 1;
-        }
-
-        _owners[tokenId] = to;
-
-        emit Transfer(address(0), to, tokenId);
+    /**
+     * @dev Mints `tokenId`, transfers it to `to` and checks for `to` acceptance.
+     *
+     * Requirements:
+     *
+     * - `tokenId` must not exist.
+     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     *
+     * Emits a {Transfer} event.
+     */
+    function _safeMint(address to, uint256 tokenId) internal {
+        _safeMint(to, tokenId, "");
+    }
 
-        _afterTokenTransfer(address(0), to, tokenId, 1);
+    /**
+     * @dev Same as {xref-ERC721-_safeMint-address-uint256-}[`_safeMint`], with an additional `data` parameter which is
+     * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
+     */
+    function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual {
+        _mint(to, tokenId);
+        _checkOnERC721Received(address(0), to, tokenId, data);
     }
 
     /**
@@ -306,26 +326,11 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Emits a {Transfer} event.
      */
-    function _burn(uint256 tokenId) internal virtual {
-        address owner = ownerOf(tokenId);
-
-        _beforeTokenTransfer(owner, address(0), tokenId, 1);
-
-        // Update ownership in case tokenId was transferred by `_beforeTokenTransfer` hook
-        owner = ownerOf(tokenId);
-
-        // Clear approvals
-        delete _tokenApprovals[tokenId];
-
-        // Decrease balance with checked arithmetic, because an `ownerOf` override may
-        // invalidate the assumption that `_balances[from] >= 1`.
-        _balances[owner] -= 1;
-
-        delete _owners[tokenId];
-
-        emit Transfer(owner, address(0), tokenId);
-
-        _afterTokenTransfer(owner, address(0), tokenId, 1);
+    function _burn(uint256 tokenId) internal {
+        address previousOwner = _update(address(0), tokenId, address(0));
+        if (previousOwner == address(0)) {
+            revert ERC721NonexistentToken(tokenId);
+        }
     }
 
     /**
@@ -339,61 +344,83 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Emits a {Transfer} event.
      */
-    function _transfer(address from, address to, uint256 tokenId) internal virtual {
-        address owner = ownerOf(tokenId);
-        if (owner != from) {
-            revert ERC721IncorrectOwner(from, tokenId, owner);
-        }
+    function _transfer(address from, address to, uint256 tokenId) internal {
         if (to == address(0)) {
             revert ERC721InvalidReceiver(address(0));
         }
-
-        _beforeTokenTransfer(from, to, tokenId, 1);
-
-        // Check that tokenId was not transferred by `_beforeTokenTransfer` hook
-        owner = ownerOf(tokenId);
-        if (owner != from) {
-            revert ERC721IncorrectOwner(from, tokenId, owner);
-        }
-
-        // Clear approvals from the previous owner
-        delete _tokenApprovals[tokenId];
-
-        // Decrease balance with checked arithmetic, because an `ownerOf` override may
-        // invalidate the assumption that `_balances[from] >= 1`.
-        _balances[from] -= 1;
-
-        unchecked {
-            // `_balances[to]` could overflow in the conditions described in `_mint`. That would require
-            // all 2**256 token ids to be minted, which in practice is impossible.
-            _balances[to] += 1;
+        address previousOwner = _update(to, tokenId, address(0));
+        if (previousOwner == address(0)) {
+            revert ERC721NonexistentToken(tokenId);
+        } else if (previousOwner != from) {
+            revert ERC721IncorrectOwner(from, tokenId, previousOwner);
         }
+    }
 
-        _owners[tokenId] = to;
-
-        emit Transfer(from, to, tokenId);
+    /**
+     * @dev Safely transfers `tokenId` token from `from` to `to`, checking that contract recipients
+     * are aware of the ERC721 standard to prevent tokens from being forever locked.
+     *
+     * `data` is additional data, it has no specified format and it is sent in call to `to`.
+     *
+     * This internal function is like {safeTransferFrom} in the sense that it invokes
+     * {IERC721Receiver-onERC721Received} on the receiver, and can be used to e.g.
+     * implement alternative mechanisms to perform token transfer, such as signature-based.
+     *
+     * Requirements:
+     *
+     * - `tokenId` token must exist and be owned by `from`.
+     * - `to` cannot be the zero address.
+     * - `from` cannot be the zero address.
+     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     *
+     * Emits a {Transfer} event.
+     */
+    function _safeTransfer(address from, address to, uint256 tokenId) internal {
+        _safeTransfer(from, to, tokenId, "");
+    }
 
-        _afterTokenTransfer(from, to, tokenId, 1);
+    /**
+     * @dev Same as {xref-ERC721-_safeTransfer-address-address-uint256-}[`_safeTransfer`], with an additional `data` parameter which is
+     * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
+     */
+    function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
+        _transfer(from, to, tokenId);
+        _checkOnERC721Received(from, to, tokenId, data);
     }
 
     /**
      * @dev Approve `to` to operate on `tokenId`
      *
+     * The `auth` argument is optional. If the value passed is non 0, then this function will check that `auth` is
+     * either the owner of the token, or approved to operate on all tokens held by this owner.
+     *
      * Emits an {Approval} event.
      */
-    function _approve(address to, uint256 tokenId) internal virtual {
+    function _approve(address to, uint256 tokenId, address auth) internal virtual returns (address) {
+        address owner = ownerOf(tokenId);
+
+        // We do not use _isAuthorized because single-token approvals should not be able to call approve
+        if (auth != address(0) && owner != auth && !isApprovedForAll(owner, auth)) {
+            revert ERC721InvalidApprover(auth);
+        }
+
         _tokenApprovals[tokenId] = to;
-        emit Approval(ownerOf(tokenId), to, tokenId);
+        emit Approval(owner, to, tokenId);
+
+        return owner;
     }
 
     /**
      * @dev Approve `operator` to operate on all of `owner` tokens
      *
+     * Requirements:
+     * - operator can't be the address zero.
+     *
      * Emits an {ApprovalForAll} event.
      */
     function _setApprovalForAll(address owner, address operator, bool approved) internal virtual {
-        if (owner == operator) {
-            revert ERC721InvalidOperator(owner);
+        if (operator == address(0)) {
+            revert ERC721InvalidOperator(operator);
         }
         _operatorApprovals[owner][operator] = approved;
         emit ApprovalForAll(owner, operator, approved);
@@ -403,30 +430,26 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev Reverts if the `tokenId` has not been minted yet.
      */
     function _requireMinted(uint256 tokenId) internal view virtual {
-        if (!_exists(tokenId)) {
+        if (_ownerOf(tokenId) == address(0)) {
             revert ERC721NonexistentToken(tokenId);
         }
     }
 
     /**
-     * @dev Private function to invoke {IERC721Receiver-onERC721Received} on a target address.
-     * The call is not executed if the target address is not a contract.
+     * @dev Private function to invoke {IERC721Receiver-onERC721Received} on a target address. This will revert if the
+     * recipient doesn't accept the token transfer. The call is not executed if the target address is not a contract.
      *
      * @param from address representing the previous owner of the given token ID
      * @param to target address that will receive the tokens
      * @param tokenId uint256 ID of the token to be transferred
      * @param data bytes optional data to send along with the call
-     * @return bool whether the call correctly returned the expected magic value
-     */
-    function _checkOnERC721Received(
-        address from,
-        address to,
-        uint256 tokenId,
-        bytes memory data
-    ) private returns (bool) {
+     */
+    function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes memory data) private {
         if (to.code.length > 0) {
             try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) {
-                return retval == IERC721Receiver.onERC721Received.selector;
+                if (retval != IERC721Receiver.onERC721Received.selector) {
+                    revert ERC721InvalidReceiver(to);
+                }
             } catch (bytes memory reason) {
                 if (reason.length == 0) {
                     revert ERC721InvalidReceiver(to);
@@ -437,52 +460,6 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
                     }
                 }
             }
-        } else {
-            return true;
         }
     }
-
-    /**
-     * @dev Hook that is called before any token transfer. This includes minting and burning. If {ERC721Consecutive} is
-     * used, the hook may be called as part of a consecutive (batch) mint, as indicated by `batchSize` greater than 1.
-     *
-     * Calling conditions:
-     *
-     * - When `from` and `to` are both non-zero, ``from``'s tokens will be transferred to `to`.
-     * - When `from` is zero, the tokens will be minted for `to`.
-     * - When `to` is zero, ``from``'s tokens will be burned.
-     * - `from` and `to` are never both zero.
-     * - `batchSize` is non-zero.
-     *
-     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
-     */
-    function _beforeTokenTransfer(address from, address to, uint256 firstTokenId, uint256 batchSize) internal virtual {}
-
-    /**
-     * @dev Hook that is called after any token transfer. This includes minting and burning. If {ERC721Consecutive} is
-     * used, the hook may be called as part of a consecutive (batch) mint, as indicated by `batchSize` greater than 1.
-     *
-     * Calling conditions:
-     *
-     * - When `from` and `to` are both non-zero, ``from``'s tokens were transferred to `to`.
-     * - When `from` is zero, the tokens were minted for `to`.
-     * - When `to` is zero, ``from``'s tokens were burned.
-     * - `from` and `to` are never both zero.
-     * - `batchSize` is non-zero.
-     *
-     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
-     */
-    function _afterTokenTransfer(address from, address to, uint256 firstTokenId, uint256 batchSize) internal virtual {}
-
-    /**
-     * @dev Unsafe write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
-     *
-     * WARNING: Anyone calling this MUST ensure that the balances remain consistent with the ownership. The invariant
-     * being that for any address `a` the value returned by `balanceOf(a)` must be equal to the number of tokens such
-     * that `ownerOf(tokenId)` is `a`.
-     */
-    // solhint-disable-next-line func-name-mixedcase
-    function __unsafe_increaseBalance(address account, uint256 value) internal {
-        _balances[account] += value;
-    }
 }

+ 1 - 1
contracts/token/ERC721/IERC721.sol

@@ -108,7 +108,7 @@ interface IERC721 is IERC165 {
      *
      * Requirements:
      *
-     * - The `operator` cannot be the caller.
+     * - The `operator` cannot be the address zero.
      *
      * Emits an {ApprovalForAll} event.
      */

+ 3 - 4
contracts/token/ERC721/extensions/ERC721Burnable.sol

@@ -19,9 +19,8 @@ abstract contract ERC721Burnable is Context, ERC721 {
      * - The caller must own `tokenId` or be an approved operator.
      */
     function burn(uint256 tokenId) public virtual {
-        if (!_isApprovedOrOwner(_msgSender(), tokenId)) {
-            revert ERC721InsufficientApproval(_msgSender(), tokenId);
-        }
-        _burn(tokenId);
+        // Setting an "auth" arguments enables the `_isAuthorized` check which verifies that the token exists
+        // (from != 0). Therefore, it is not needed to verify that the return value is not 0 here.
+        _update(address(0), tokenId, _msgSender());
     }
 }

+ 16 - 33
contracts/token/ERC721/extensions/ERC721Consecutive.sol

@@ -118,61 +118,44 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
                 revert ERC721ExceededMaxBatchMint(batchSize, maxBatchSize);
             }
 
-            // hook before
-            _beforeTokenTransfer(address(0), to, next, batchSize);
-
             // push an ownership checkpoint & emit event
             uint96 last = next + batchSize - 1;
             _sequentialOwnership.push(last, uint160(to));
 
             // The invariant required by this function is preserved because the new sequentialOwnership checkpoint
             // is attributing ownership of `batchSize` new tokens to account `to`.
-            __unsafe_increaseBalance(to, batchSize);
+            _increaseBalance(to, batchSize);
 
             emit ConsecutiveTransfer(next, last, address(0), to);
-
-            // hook after
-            _afterTokenTransfer(address(0), to, next, batchSize);
         }
 
         return next;
     }
 
     /**
-     * @dev See {ERC721-_mint}. Override version that restricts normal minting to after construction.
+     * @dev See {ERC721-_update}. 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.
+     * WARNING: Using {ERC721Consecutive} prevents minting during construction in favor of {_mintConsecutive}.
+     * After construction, {_mintConsecutive} is no longer available and minting through {_update} becomes available.
      */
-    function _mint(address to, uint256 tokenId) internal virtual override {
-        if (address(this).code.length == 0) {
+    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
+        address previousOwner = super._update(to, tokenId, auth);
+
+        // only mint after construction
+        if (previousOwner == address(0) && address(this).code.length == 0) {
             revert ERC721ForbiddenMint();
         }
-        super._mint(to, tokenId);
-    }
 
-    /**
-     * @dev See {ERC721-_afterTokenTransfer}. Burning of tokens that have been sequentially minted must be explicit.
-     */
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override {
+        // record burn
         if (
             to == address(0) && // if we burn
-            firstTokenId >= _firstConsecutiveId() &&
-            firstTokenId < _nextConsecutiveId() &&
-            !_sequentialBurn.get(firstTokenId)
-        ) // and the token was never marked as burnt
-        {
-            if (batchSize != 1) {
-                revert ERC721ForbiddenBatchBurn();
-            }
-            _sequentialBurn.set(firstTokenId);
+            tokenId < _nextConsecutiveId() && // and the tokenId was minted in a batch
+            !_sequentialBurn.get(tokenId) // and the token was never marked as burnt
+        ) {
+            _sequentialBurn.set(tokenId);
         }
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+
+        return previousOwner;
     }
 
     /**

+ 21 - 21
contracts/token/ERC721/extensions/ERC721Enumerable.sol

@@ -68,33 +68,23 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
     }
 
     /**
-     * @dev See {ERC721-_beforeTokenTransfer}.
+     * @dev See {ERC721-_update}.
      */
-    function _beforeTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
-
-        if (batchSize > 1) {
-            // Will only trigger during construction. Batch transferring (minting) is not available afterwards.
-            revert ERC721EnumerableForbiddenBatchMint();
-        }
-
-        uint256 tokenId = firstTokenId;
+    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
+        address previousOwner = super._update(to, tokenId, auth);
 
-        if (from == address(0)) {
+        if (previousOwner == address(0)) {
             _addTokenToAllTokensEnumeration(tokenId);
-        } else if (from != to) {
-            _removeTokenFromOwnerEnumeration(from, tokenId);
+        } else if (previousOwner != to) {
+            _removeTokenFromOwnerEnumeration(previousOwner, tokenId);
         }
         if (to == address(0)) {
             _removeTokenFromAllTokensEnumeration(tokenId);
-        } else if (to != from) {
+        } else if (previousOwner != to) {
             _addTokenToOwnerEnumeration(to, tokenId);
         }
+
+        return previousOwner;
     }
 
     /**
@@ -103,7 +93,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
      * @param tokenId uint256 ID of the token to be added to the tokens list of the given address
      */
     function _addTokenToOwnerEnumeration(address to, uint256 tokenId) private {
-        uint256 length = balanceOf(to);
+        uint256 length = balanceOf(to) - 1;
         _ownedTokens[to][length] = tokenId;
         _ownedTokensIndex[tokenId] = length;
     }
@@ -129,7 +119,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         // To prevent a gap in from's tokens array, we store the last token in the index of the token to delete, and
         // then delete the last slot (swap and pop).
 
-        uint256 lastTokenIndex = balanceOf(from) - 1;
+        uint256 lastTokenIndex = balanceOf(from);
         uint256 tokenIndex = _ownedTokensIndex[tokenId];
 
         // When the token to delete is the last token, the swap operation is unnecessary
@@ -169,4 +159,14 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         delete _allTokensIndex[tokenId];
         _allTokens.pop();
     }
+
+    /**
+     * See {ERC721-_increaseBalance}. We need that to account tokens that were minted in batch
+     */
+    function _increaseBalance(address account, uint128 amount) internal virtual override {
+        if (amount > 0) {
+            revert ERC721EnumerableForbiddenBatchMint();
+        }
+        super._increaseBalance(account, amount);
+    }
 }

+ 6 - 9
contracts/token/ERC721/extensions/ERC721Pausable.sol

@@ -21,20 +21,17 @@ import {Pausable} from "../../../security/Pausable.sol";
  */
 abstract contract ERC721Pausable is ERC721, Pausable {
     /**
-     * @dev See {ERC721-_beforeTokenTransfer}.
+     * @dev See {ERC721-_update}.
      *
      * Requirements:
      *
      * - the contract must not be paused.
      */
-    function _beforeTokenTransfer(
-        address from,
+    function _update(
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
-
-        _requireNotPaused();
+        uint256 tokenId,
+        address auth
+    ) internal virtual override whenNotPaused returns (address) {
+        return super._update(to, tokenId, auth);
     }
 }

+ 9 - 4
contracts/token/ERC721/extensions/ERC721Royalty.sol

@@ -26,10 +26,15 @@ abstract contract ERC721Royalty is ERC2981, ERC721 {
     }
 
     /**
-     * @dev See {ERC721-_burn}. This override additionally clears the royalty information for the token.
+     * @dev See {ERC721-_update}. When burning, this override will additionally clear the royalty information for the token.
      */
-    function _burn(uint256 tokenId) internal virtual override {
-        super._burn(tokenId);
-        _resetTokenRoyalty(tokenId);
+    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
+        address previousOwner = super._update(to, tokenId, auth);
+
+        if (to == address(0)) {
+            _resetTokenRoyalty(tokenId);
+        }
+
+        return previousOwner;
     }
 }

+ 7 - 5
contracts/token/ERC721/extensions/ERC721URIStorage.sol

@@ -55,7 +55,7 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
      * - `tokenId` must exist.
      */
     function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
-        if (!_exists(tokenId)) {
+        if (_ownerOf(tokenId) == address(0)) {
             revert ERC721NonexistentToken(tokenId);
         }
         _tokenURIs[tokenId] = _tokenURI;
@@ -64,15 +64,17 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
     }
 
     /**
-     * @dev See {ERC721-_burn}. This override additionally checks to see if a
+     * @dev See {ERC721-_update}. When burning, this override will additionally check if a
      * token-specific URI was set for the token, and if so, it deletes the token URI from
      * the storage mapping.
      */
-    function _burn(uint256 tokenId) internal virtual override {
-        super._burn(tokenId);
+    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
+        address previousOwner = super._update(to, tokenId, auth);
 
-        if (bytes(_tokenURIs[tokenId]).length != 0) {
+        if (to == address(0) && bytes(_tokenURIs[tokenId]).length != 0) {
             delete _tokenURIs[tokenId];
         }
+
+        return previousOwner;
     }
 }

+ 15 - 9
contracts/token/ERC721/extensions/ERC721Votes.sol

@@ -16,18 +16,16 @@ import {Votes} from "../../../governance/utils/Votes.sol";
  */
 abstract contract ERC721Votes is ERC721, Votes {
     /**
-     * @dev See {ERC721-_afterTokenTransfer}. Adjusts votes when tokens are transferred.
+     * @dev See {ERC721-_update}. Adjusts votes when tokens are transferred.
      *
      * Emits a {IVotes-DelegateVotesChanged} event.
      */
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override {
-        _transferVotingUnits(from, to, batchSize);
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
+        address previousOwner = super._update(to, tokenId, auth);
+
+        _transferVotingUnits(previousOwner, to, 1);
+
+        return previousOwner;
     }
 
     /**
@@ -38,4 +36,12 @@ abstract contract ERC721Votes is ERC721, Votes {
     function _getVotingUnits(address account) internal view virtual override returns (uint256) {
         return balanceOf(account);
     }
+
+    /**
+     * @dev See {ERC721-_increaseBalance}. We need that to account tokens that were minted in batch.
+     */
+    function _increaseBalance(address account, uint128 amount) internal virtual override {
+        super._increaseBalance(account, amount);
+        _transferVotingUnits(address(0), account, amount);
+    }
 }

+ 3 - 4
contracts/token/ERC721/extensions/ERC721Wrapper.sol

@@ -50,10 +50,9 @@ abstract contract ERC721Wrapper is ERC721, IERC721Receiver {
         uint256 length = tokenIds.length;
         for (uint256 i = 0; i < length; ++i) {
             uint256 tokenId = tokenIds[i];
-            if (!_isApprovedOrOwner(_msgSender(), tokenId)) {
-                revert ERC721InsufficientApproval(_msgSender(), tokenId);
-            }
-            _burn(tokenId);
+            // Setting an "auth" arguments enables the `_isAuthorized` check which verifies that the token exists
+            // (from != 0). Therefore, it is not needed to verify that the return value is not 0 here.
+            _update(address(0), tokenId, _msgSender());
             // Checks were already performed at this point, and there's no way to retake ownership or approval from
             // the wrapped tokenId after this point, so it's safe to remove the reentrancy check for the next line.
             // slither-disable-next-line reentrancy-no-eth

+ 154 - 131
test/token/ERC721/ERC721.behavior.js

@@ -104,7 +104,7 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
       };
 
-      const shouldTransferTokensByUsers = function (transferFunction) {
+      const shouldTransferTokensByUsers = function (transferFunction, opts = {}) {
         context('when called by the owner', function () {
           beforeEach(async function () {
             receipt = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: owner });
@@ -180,13 +180,19 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
 
         context('when the sender is not authorized for the token id', function () {
-          it('reverts', async function () {
-            await expectRevertCustomError(
-              transferFunction.call(this, owner, other, tokenId, { from: other }),
-              'ERC721InsufficientApproval',
-              [other, tokenId],
-            );
-          });
+          if (opts.unrestricted) {
+            it('does not revert', async function () {
+              await transferFunction.call(this, owner, other, tokenId, { from: other });
+            });
+          } else {
+            it('reverts', async function () {
+              await expectRevertCustomError(
+                transferFunction.call(this, owner, other, tokenId, { from: other }),
+                'ERC721InsufficientApproval',
+                [other, tokenId],
+              );
+            });
+          }
         });
 
         context('when the given token ID does not exist', function () {
@@ -210,145 +216,170 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
       };
 
-      describe('via transferFrom', function () {
-        shouldTransferTokensByUsers(function (from, to, tokenId, opts) {
-          return this.token.transferFrom(from, to, tokenId, opts);
+      const shouldTransferSafely = function (transferFun, data, opts = {}) {
+        describe('to a user account', function () {
+          shouldTransferTokensByUsers(transferFun, opts);
         });
-      });
-
-      describe('via safeTransferFrom', function () {
-        const safeTransferFromWithData = function (from, to, tokenId, opts) {
-          return this.token.methods['safeTransferFrom(address,address,uint256,bytes)'](from, to, tokenId, data, opts);
-        };
-
-        const safeTransferFromWithoutData = function (from, to, tokenId, opts) {
-          return this.token.methods['safeTransferFrom(address,address,uint256)'](from, to, tokenId, opts);
-        };
 
-        const shouldTransferSafely = function (transferFun, data) {
-          describe('to a user account', function () {
-            shouldTransferTokensByUsers(transferFun);
+        describe('to a valid receiver contract', function () {
+          beforeEach(async function () {
+            this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, RevertType.None);
+            this.toWhom = this.receiver.address;
           });
 
-          describe('to a valid receiver contract', function () {
-            beforeEach(async function () {
-              this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, RevertType.None);
-              this.toWhom = this.receiver.address;
-            });
-
-            shouldTransferTokensByUsers(transferFun);
+          shouldTransferTokensByUsers(transferFun, opts);
 
-            it('calls onERC721Received', async function () {
-              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
+          it('calls onERC721Received', async function () {
+            const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
 
-              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-                operator: owner,
-                from: owner,
-                tokenId: tokenId,
-                data: data,
-              });
+            await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+              operator: owner,
+              from: owner,
+              tokenId: tokenId,
+              data: data,
             });
+          });
 
-            it('calls onERC721Received from approved', async function () {
-              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved });
+          it('calls onERC721Received from approved', async function () {
+            const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved });
 
-              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-                operator: approved,
-                from: owner,
-                tokenId: tokenId,
-                data: data,
-              });
+            await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+              operator: approved,
+              from: owner,
+              tokenId: tokenId,
+              data: data,
             });
+          });
 
-            describe('with an invalid token id', function () {
-              it('reverts', async function () {
-                await expectRevertCustomError(
-                  transferFun.call(this, owner, this.receiver.address, nonExistentTokenId, { from: owner }),
-                  'ERC721NonexistentToken',
-                  [nonExistentTokenId],
-                );
-              });
+          describe('with an invalid token id', function () {
+            it('reverts', async function () {
+              await expectRevertCustomError(
+                transferFun.call(this, owner, this.receiver.address, nonExistentTokenId, { from: owner }),
+                'ERC721NonexistentToken',
+                [nonExistentTokenId],
+              );
             });
           });
-        };
-
-        describe('with data', function () {
-          shouldTransferSafely(safeTransferFromWithData, data);
-        });
-
-        describe('without data', function () {
-          shouldTransferSafely(safeTransferFromWithoutData, null);
         });
+      };
 
-        describe('to a receiver contract returning unexpected value', function () {
-          it('reverts', async function () {
-            const invalidReceiver = await ERC721ReceiverMock.new('0x42', RevertType.None);
-            await expectRevertCustomError(
-              this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }),
-              'ERC721InvalidReceiver',
-              [invalidReceiver.address],
-            );
+      for (const { fnName, opts } of [
+        { fnName: 'transferFrom', opts: {} },
+        { fnName: '$_transfer', opts: { unrestricted: true } },
+      ]) {
+        describe(`via ${fnName}`, function () {
+          shouldTransferTokensByUsers(function (from, to, tokenId, opts) {
+            return this.token[fnName](from, to, tokenId, opts);
+          }, opts);
+        });
+      }
+
+      for (const { fnName, opts } of [
+        { fnName: 'safeTransferFrom', opts: {} },
+        { fnName: '$_safeTransfer', opts: { unrestricted: true } },
+      ]) {
+        describe(`via ${fnName}`, function () {
+          const safeTransferFromWithData = function (from, to, tokenId, opts) {
+            return this.token.methods[fnName + '(address,address,uint256,bytes)'](from, to, tokenId, data, opts);
+          };
+
+          const safeTransferFromWithoutData = function (from, to, tokenId, opts) {
+            return this.token.methods[fnName + '(address,address,uint256)'](from, to, tokenId, opts);
+          };
+
+          describe('with data', function () {
+            shouldTransferSafely(safeTransferFromWithData, data, opts);
+          });
+
+          describe('without data', function () {
+            shouldTransferSafely(safeTransferFromWithoutData, null, opts);
+          });
+
+          describe('to a receiver contract returning unexpected value', function () {
+            it('reverts', async function () {
+              const invalidReceiver = await ERC721ReceiverMock.new('0x42', RevertType.None);
+              await expectRevertCustomError(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, invalidReceiver.address, tokenId, {
+                  from: owner,
+                }),
+                'ERC721InvalidReceiver',
+                [invalidReceiver.address],
+              );
+            });
           });
-        });
 
-        describe('to a receiver contract that reverts with message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, RevertType.RevertWithMessage);
-            await expectRevert(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-              'ERC721ReceiverMock: reverting',
-            );
+          describe('to a receiver contract that reverts with message', function () {
+            it('reverts', async function () {
+              const revertingReceiver = await ERC721ReceiverMock.new(
+                RECEIVER_MAGIC_VALUE,
+                RevertType.RevertWithMessage,
+              );
+              await expectRevert(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, revertingReceiver.address, tokenId, {
+                  from: owner,
+                }),
+                'ERC721ReceiverMock: reverting',
+              );
+            });
           });
-        });
 
-        describe('to a receiver contract that reverts without message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(
-              RECEIVER_MAGIC_VALUE,
-              RevertType.RevertWithoutMessage,
-            );
-            await expectRevertCustomError(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-              'ERC721InvalidReceiver',
-              [revertingReceiver.address],
-            );
+          describe('to a receiver contract that reverts without message', function () {
+            it('reverts', async function () {
+              const revertingReceiver = await ERC721ReceiverMock.new(
+                RECEIVER_MAGIC_VALUE,
+                RevertType.RevertWithoutMessage,
+              );
+              await expectRevertCustomError(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, revertingReceiver.address, tokenId, {
+                  from: owner,
+                }),
+                'ERC721InvalidReceiver',
+                [revertingReceiver.address],
+              );
+            });
           });
-        });
 
-        describe('to a receiver contract that reverts with custom error', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(
-              RECEIVER_MAGIC_VALUE,
-              RevertType.RevertWithCustomError,
-            );
-            await expectRevertCustomError(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-              'CustomError',
-              [RECEIVER_MAGIC_VALUE],
-            );
+          describe('to a receiver contract that reverts with custom error', function () {
+            it('reverts', async function () {
+              const revertingReceiver = await ERC721ReceiverMock.new(
+                RECEIVER_MAGIC_VALUE,
+                RevertType.RevertWithCustomError,
+              );
+              await expectRevertCustomError(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, revertingReceiver.address, tokenId, {
+                  from: owner,
+                }),
+                'CustomError',
+                [RECEIVER_MAGIC_VALUE],
+              );
+            });
           });
-        });
 
-        describe('to a receiver contract that panics', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, RevertType.Panic);
-            await expectRevert.unspecified(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-            );
+          describe('to a receiver contract that panics', function () {
+            it('reverts', async function () {
+              const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, RevertType.Panic);
+              await expectRevert.unspecified(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, revertingReceiver.address, tokenId, {
+                  from: owner,
+                }),
+              );
+            });
           });
-        });
 
-        describe('to a contract that does not implement the required function', function () {
-          it('reverts', async function () {
-            const nonReceiver = await NonERC721ReceiverMock.new();
-            await expectRevertCustomError(
-              this.token.safeTransferFrom(owner, nonReceiver.address, tokenId, { from: owner }),
-              'ERC721InvalidReceiver',
-              [nonReceiver.address],
-            );
+          describe('to a contract that does not implement the required function', function () {
+            it('reverts', async function () {
+              const nonReceiver = await NonERC721ReceiverMock.new();
+              await expectRevertCustomError(
+                this.token.methods[fnName + '(address,address,uint256)'](owner, nonReceiver.address, tokenId, {
+                  from: owner,
+                }),
+                'ERC721InvalidReceiver',
+                [nonReceiver.address],
+              );
+            });
           });
         });
-      });
+      }
     });
 
     describe('safe mint', function () {
@@ -524,14 +555,6 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
       });
 
-      context('when the address that receives the approval is the owner', function () {
-        it('reverts', async function () {
-          await expectRevertCustomError(this.token.approve(owner, tokenId, { from: owner }), 'ERC721InvalidOperator', [
-            owner,
-          ]);
-        });
-      });
-
       context('when the sender does not own the given token ID', function () {
         it('reverts', async function () {
           await expectRevertCustomError(
@@ -645,12 +668,12 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
       });
 
-      context('when the operator is the owner', function () {
+      context('when the operator is address zero', function () {
         it('reverts', async function () {
           await expectRevertCustomError(
-            this.token.setApprovalForAll(owner, true, { from: owner }),
+            this.token.setApprovalForAll(constants.ZERO_ADDRESS, true, { from: owner }),
             'ERC721InvalidOperator',
-            [owner],
+            [constants.ZERO_ADDRESS],
           );
         });
       });

+ 2 - 17
test/token/ERC721/extensions/ERC721Consecutive.test.js

@@ -103,7 +103,7 @@ contract('ERC721Consecutive', function (accounts) {
         it('simple minting is possible after construction', async function () {
           const tokenId = sum(...batches.map(b => b.amount)) + offset;
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(false);
+          await expectRevertCustomError(this.token.ownerOf(tokenId), 'ERC721NonexistentToken', [tokenId]);
 
           expectEvent(await this.token.$_mint(user1, tokenId), 'Transfer', {
             from: constants.ZERO_ADDRESS,
@@ -115,7 +115,7 @@ contract('ERC721Consecutive', function (accounts) {
         it('cannot mint a token that has been batched minted', async function () {
           const tokenId = sum(...batches.map(b => b.amount)) + offset - 1;
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(true);
+          expect(await this.token.ownerOf(tokenId)).to.be.not.equal(constants.ZERO_ADDRESS);
 
           await expectRevertCustomError(this.token.$_mint(user1, tokenId), 'ERC721InvalidSender', [ZERO_ADDRESS]);
         });
@@ -151,13 +151,11 @@ contract('ERC721Consecutive', function (accounts) {
         it('tokens can be burned and re-minted #2', async function () {
           const tokenId = web3.utils.toBN(sum(...batches.map(({ amount }) => amount)) + offset);
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(false);
           await expectRevertCustomError(this.token.ownerOf(tokenId), 'ERC721NonexistentToken', [tokenId]);
 
           // mint
           await this.token.$_mint(user1, tokenId);
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(true);
           expect(await this.token.ownerOf(tokenId), user1);
 
           // burn
@@ -167,7 +165,6 @@ contract('ERC721Consecutive', function (accounts) {
             tokenId,
           });
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(false);
           await expectRevertCustomError(this.token.ownerOf(tokenId), 'ERC721NonexistentToken', [tokenId]);
 
           // re-mint
@@ -177,20 +174,8 @@ contract('ERC721Consecutive', function (accounts) {
             tokenId,
           });
 
-          expect(await this.token.$_exists(tokenId)).to.be.equal(true);
           expect(await this.token.ownerOf(tokenId), user2);
         });
-
-        it('reverts burning batches of size != 1', async function () {
-          const tokenId = batches[0].amount + offset;
-          const receiver = batches[0].receiver;
-
-          await expectRevertCustomError(
-            this.token.$_afterTokenTransfer(receiver, ZERO_ADDRESS, tokenId, 2),
-            'ERC721ForbiddenBatchBurn',
-            [],
-          );
-        });
       });
     });
   }

+ 0 - 6
test/token/ERC721/extensions/ERC721Pausable.test.js

@@ -81,12 +81,6 @@ contract('ERC721Pausable', function (accounts) {
       });
     });
 
-    describe('exists', function () {
-      it('returns token existence', async function () {
-        expect(await this.token.$_exists(firstTokenId)).to.equal(true);
-      });
-    });
-
     describe('isApprovedForAll', function () {
       it('returns the approval of the operator', async function () {
         expect(await this.token.isApprovedForAll(owner, operator)).to.equal(false);

+ 0 - 2
test/token/ERC721/extensions/ERC721URIStorage.test.js

@@ -86,7 +86,6 @@ contract('ERC721URIStorage', function (accounts) {
     it('tokens without URI can be burnt ', async function () {
       await this.token.$_burn(firstTokenId, { from: owner });
 
-      expect(await this.token.$_exists(firstTokenId)).to.equal(false);
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
     });
 
@@ -95,7 +94,6 @@ contract('ERC721URIStorage', function (accounts) {
 
       await this.token.$_burn(firstTokenId, { from: owner });
 
-      expect(await this.token.$_exists(firstTokenId)).to.equal(false);
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
     });
   });