From 2c9f7bb665a53630f58c0c28e4f3f939f46486c3 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 4 Jul 2023 01:53:26 -0300
Subject: [PATCH 1/8] Refactor ERC721 to use _update pattern

---
 contracts/interfaces/draft-IERC6093.sol       |   6 +-
 .../token/ERC721ConsecutiveEnumerableMock.sol |  22 +-
 .../mocks/token/ERC721ConsecutiveMock.sol     |  25 +--
 contracts/token/ERC721/ERC721.sol             | 212 ++++++------------
 .../ERC721/extensions/ERC721Burnable.sol      |   8 +-
 .../ERC721/extensions/ERC721Consecutive.sol   |  52 ++---
 .../ERC721/extensions/ERC721Enumerable.sol    |  23 +-
 .../ERC721/extensions/ERC721Pausable.sol      |   8 +-
 .../token/ERC721/extensions/ERC721Royalty.sol |  11 +-
 .../ERC721/extensions/ERC721URIStorage.sol    |  12 +-
 .../token/ERC721/extensions/ERC721Votes.sol   |  19 +-
 .../token/ERC721/extensions/ERC721Wrapper.sol |   7 +-
 test/governance/utils/Votes.behavior.js       |   6 +-
 test/token/ERC721/ERC721.behavior.js          |  16 +-
 .../ERC721/extensions/ERC721Burnable.test.js  |   8 +-
 .../ERC721/extensions/ERC721Consecutive.t.sol |   6 +-
 .../extensions/ERC721Consecutive.test.js      |  15 +-
 .../ERC721/extensions/ERC721Pausable.test.js  |   2 +-
 .../ERC721/extensions/ERC721Royalty.test.js   |   2 +-
 .../extensions/ERC721URIStorage.test.js       |   4 +-
 20 files changed, 171 insertions(+), 293 deletions(-)

diff --git a/contracts/interfaces/draft-IERC6093.sol b/contracts/interfaces/draft-IERC6093.sol
index 08e77553c20..2f9374e5790 100644
--- a/contracts/interfaces/draft-IERC6093.sol
+++ b/contracts/interfaces/draft-IERC6093.sol
@@ -69,11 +69,11 @@ interface IERC721Errors {
 
     /**
      * @dev Indicates an error related to the ownership over a particular token. Used in transfers.
-     * @param sender Address whose tokens are being transferred.
+     * @param claimedOwner Address whose tokens are being transferred.
      * @param tokenId Identifier number of a token.
-     * @param owner Address of the current owner of a token.
+     * @param actualOwner Address of the current owner of a token.
      */
-    error ERC721IncorrectOwner(address sender, uint256 tokenId, address owner);
+    error ERC721IncorrectOwner(address claimedOwner, uint256 tokenId, address actualOwner);
 
     /**
      * @dev Indicates a failure with the token `sender`. Used in transfers.
diff --git a/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol b/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
index 081b8d99dee..a3c751a2624 100644
--- a/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
+++ b/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
@@ -28,25 +28,11 @@ 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 _update(address from, address to, uint256 tokenId) internal virtual override(ERC721Consecutive, ERC721Enumerable) {
+        super._update(from, to, tokenId);
     }
 
-    function _beforeTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Enumerable) {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
-    }
-
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Consecutive) {
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+    function _updateBalance(address account, uint128 value) internal virtual override(ERC721, ERC721Enumerable) {
+        super._updateBalance(account, value);
     }
 }
diff --git a/contracts/mocks/token/ERC721ConsecutiveMock.sol b/contracts/mocks/token/ERC721ConsecutiveMock.sol
index 166959e922b..6864ccafdb3 100644
--- a/contracts/mocks/token/ERC721ConsecutiveMock.sol
+++ b/contracts/mocks/token/ERC721ConsecutiveMock.sol
@@ -41,26 +41,19 @@ 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(
+    function _update(
         address from,
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Pausable) {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
+        uint256 tokenId
+    ) internal virtual override(ERC721Consecutive, ERC721Pausable, ERC721Votes) {
+        super._update(from, to, tokenId);
     }
 
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 firstTokenId,
-        uint256 batchSize
-    ) internal virtual override(ERC721, ERC721Votes, ERC721Consecutive) {
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
+    function _updateBalance(
+        address account,
+        uint128 value
+    ) internal virtual override(ERC721, ERC721Votes) {
+        super._updateBalance(account, value);
     }
 }
 
diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index 25ac69fa96a..a58e28cf705 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -151,28 +151,23 @@ 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);
-        }
-
+    function transferFrom(address from, address to, uint256 tokenId) public {
+        _checkApproved(from, _msgSender(), tokenId);
         _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, "");
     }
 
     /**
      * @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);
-        }
+    function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public {
+        _checkApproved(from, _msgSender(), tokenId);
         _safeTransfer(from, to, tokenId, data);
     }
 
@@ -194,15 +189,18 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Emits a {Transfer} event.
      */
-    function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
+    function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal {
         _transfer(from, to, tokenId);
-        if (!_checkOnERC721Received(from, to, tokenId, data)) {
-            revert ERC721InvalidReceiver(to);
-        }
+        _checkOnERC721Received(from, to, tokenId, data);
     }
 
     /**
-     * @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist
+     * @dev Returns the owner of the `tokenId`. Returns `address(0)` if token doesn't exist.
+     *
+     * 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 {_updateBalance} to keep balances
+     * consistent with 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`.
      */
     function _ownerOf(uint256 tokenId) internal view virtual returns (address) {
         return _owners[tokenId];
@@ -221,15 +219,26 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     }
 
     /**
-     * @dev Returns whether `spender` is allowed to manage `tokenId`.
+     * @dev Returns whether `spender` is allowed to manage `owner`'s tokens, or `tokenId` in
+     * particular (ignoring whether it is owned by `owner`).
      *
      * Requirements:
      *
      * - `tokenId` must exist.
      */
-    function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) {
-        address owner = ownerOf(tokenId);
-        return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender);
+    function _isApproved(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) {
+        return owner == spender || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender;
+    }
+
+    function _checkApproved(address owner, address spender, uint256 tokenId) internal view virtual {
+        if (!_isApproved(owner, spender, tokenId)) {
+            address actualOwner = _ownerOf(tokenId);
+            if (owner == actualOwner) {
+                revert ERC721InsufficientApproval(spender, tokenId);
+            } else {
+                revert ERC721IncorrectOwner(owner, tokenId, actualOwner);
+            }
+        }
     }
 
     /**
@@ -242,7 +251,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Emits a {Transfer} event.
      */
-    function _safeMint(address to, uint256 tokenId) internal virtual {
+    function _safeMint(address to, uint256 tokenId) internal {
         _safeMint(to, tokenId, "");
     }
 
@@ -250,11 +259,9 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @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 {
+    function _safeMint(address to, uint256 tokenId, bytes memory data) internal {
         _mint(to, tokenId);
-        if (!_checkOnERC721Received(address(0), to, tokenId, data)) {
-            revert ERC721InvalidReceiver(to);
-        }
+        _checkOnERC721Received(address(0), to, tokenId, data);
     }
 
     /**
@@ -269,34 +276,11 @@ 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)) {
-            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);
-
-        _afterTokenTransfer(address(0), to, tokenId, 1);
+        _update(address(0), to, tokenId);
     }
 
     /**
@@ -310,26 +294,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(address from, uint256 tokenId) internal {
+        if (from == address(0)) {
+            revert ERC721InvalidSender(address(0));
+        }
+        _update(from, address(0), tokenId);
     }
 
     /**
@@ -343,41 +312,51 @@ 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 (from == address(0)) {
+            revert ERC721InvalidSender(address(0));
         }
         if (to == address(0)) {
             revert ERC721InvalidReceiver(address(0));
         }
+        _update(from, to, tokenId);
+    }
 
-        _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);
+    function _update(address from, address to, uint256 tokenId) internal virtual {
+        address owner = _ownerOf(tokenId);
+        if (from != owner) {
+            if (owner == address(0)) {
+                revert ERC721NonexistentToken(tokenId);
+            } else {
+                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;
+        if (from != address(0)) {
+            delete _tokenApprovals[tokenId];
+            unchecked {
+                _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;
+        if (to != address(0)) {
+            unchecked {
+                _balances[to] += 1;
+            }
         }
 
         _owners[tokenId] = to;
 
         emit Transfer(from, to, tokenId);
+    }
 
-        _afterTokenTransfer(from, to, tokenId, 1);
+    /**
+     * @dev Unsafe write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
+     */
+    function _updateBalance(address account, uint128 value) internal virtual {
+        unchecked {
+            _balances[account] += value;
+        }
     }
 
     /**
@@ -420,17 +399,18 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @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) {
+    ) 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);
@@ -441,52 +421,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;
-    }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Burnable.sol b/contracts/token/ERC721/extensions/ERC721Burnable.sol
index 607265328ec..11325b3f1dd 100644
--- a/contracts/token/ERC721/extensions/ERC721Burnable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Burnable.sol
@@ -18,10 +18,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);
+    function burn(address from, uint256 tokenId) public virtual {
+        _checkApproved(from, _msgSender(), tokenId);
+        _burn(from, tokenId);
     }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Consecutive.sol b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
index e25edfcd907..94475b01419 100644
--- a/contracts/token/ERC721/extensions/ERC721Consecutive.sol
+++ b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
@@ -53,11 +53,6 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
      */
     error ERC721ForbiddenMint();
 
-    /**
-     * @dev Batch burn is not supported.
-     */
-    error ERC721ForbiddenBatchBurn();
-
     /**
      * @dev Maximum size of a batch of consecutive tokens. This is designed to limit stress on off-chain indexing
      * services that have to record one entry per token, and have protections against "unreasonably large" batches of
@@ -120,61 +115,40 @@ 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);
+            // Preserve the invariant required by `_ownerOf`, given that the new sequentialOwnership
+            // checkpoint is attributing ownership of `batchSize` new tokens to account `to`.
+            _updateBalance(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.
-     *
-     * WARNING: Using {ERC721Consecutive} prevents using {_mint} during construction in favor of {_mintConsecutive}.
-     * After construction, {_mintConsecutive} is no longer available and {_mint} becomes available.
+     * @dev See {ERC721-_update}. Extends token updates so that non-consecutive minting is disabled
+     * during construction, and burning of tokens that have been sequentially minted are complete.
      */
-    function _mint(address to, uint256 tokenId) internal virtual override {
-        if (address(this).code.length == 0) {
+    function _update(address from, address to, uint256 tokenId) internal virtual override {
+        if (to == 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 {
+        super._update(from, to, tokenId);
+
         if (
             to == address(0) && // if we burn
-            firstTokenId >= _firstConsecutiveId() &&
-            firstTokenId < _nextConsecutiveId() &&
-            !_sequentialBurn.get(firstTokenId)
+            tokenId >= _firstConsecutiveId() &&
+            tokenId < _nextConsecutiveId() &&
+            !_sequentialBurn.get(tokenId)
         ) // and the token was never marked as burnt
         {
-            if (batchSize != 1) {
-                revert ERC721ForbiddenBatchBurn();
-            }
-            _sequentialBurn.set(firstTokenId);
+            _sequentialBurn.set(tokenId);
         }
-        super._afterTokenTransfer(from, to, firstTokenId, batchSize);
     }
 
     /**
diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
index 2e33123b6cf..4f8a59f884d 100644
--- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
@@ -76,21 +76,11 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
     /**
      * @dev See {ERC721-_beforeTokenTransfer}.
      */
-    function _beforeTokenTransfer(
+    function _update(
         address from,
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
+        uint256 tokenId
     ) 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;
-
         if (from == address(0)) {
             _addTokenToAllTokensEnumeration(tokenId);
         } else if (from != to) {
@@ -101,6 +91,8 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         } else if (to != from) {
             _addTokenToOwnerEnumeration(to, tokenId);
         }
+
+        super._update(from, to, tokenId);
     }
 
     /**
@@ -175,4 +167,11 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         delete _allTokensIndex[tokenId];
         _allTokens.pop();
     }
+
+    function _updateBalance(address account, uint128 value) internal virtual override {
+        if (value > 0) {
+            revert ERC721EnumerableForbiddenBatchMint();
+        }
+        super._updateBalance(account, value);
+    }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Pausable.sol b/contracts/token/ERC721/extensions/ERC721Pausable.sol
index 5777ac36ea2..96871de76c9 100644
--- a/contracts/token/ERC721/extensions/ERC721Pausable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Pausable.sol
@@ -27,14 +27,12 @@ abstract contract ERC721Pausable is ERC721, Pausable {
      *
      * - the contract must not be paused.
      */
-    function _beforeTokenTransfer(
+    function _update(
         address from,
         address to,
-        uint256 firstTokenId,
-        uint256 batchSize
+        uint256 tokenId
     ) internal virtual override {
-        super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
-
         _requireNotPaused();
+        super._update(from, to, tokenId);
     }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol
index b4518dc2482..79afc24927f 100644
--- a/contracts/token/ERC721/extensions/ERC721Royalty.sol
+++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol
@@ -29,10 +29,13 @@ abstract contract ERC721Royalty is ERC2981, ERC721 {
     }
 
     /**
-     * @dev See {ERC721-_burn}. This override additionally clears the royalty information for the token.
+     * @dev See {ERC721-_update}. This override additionally clears the royalty information for the token on burns.
      */
-    function _burn(uint256 tokenId) internal virtual override {
-        super._burn(tokenId);
-        _resetTokenRoyalty(tokenId);
+    function _update(address from, address to, uint256 tokenId) internal virtual override {
+        super._update(from, to, tokenId);
+
+        if (to == address(0)) {
+            _resetTokenRoyalty(tokenId);
+        }
     }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721URIStorage.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol
index 737d28e27f2..7f7e754136f 100644
--- a/contracts/token/ERC721/extensions/ERC721URIStorage.sol
+++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol
@@ -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}. This override additionally extends burns to see 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 from, address to, uint256 tokenId) internal virtual override {
+        super._update(from, to, tokenId);
 
-        if (bytes(_tokenURIs[tokenId]).length != 0) {
-            delete _tokenURIs[tokenId];
+        if (to == address(0)) {
+            if (bytes(_tokenURIs[tokenId]).length != 0) {
+                delete _tokenURIs[tokenId];
+            }
         }
     }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Votes.sol b/contracts/token/ERC721/extensions/ERC721Votes.sol
index 1e694e4c728..1ecc8baf68b 100644
--- a/contracts/token/ERC721/extensions/ERC721Votes.sol
+++ b/contracts/token/ERC721/extensions/ERC721Votes.sol
@@ -22,14 +22,17 @@ abstract contract ERC721Votes is ERC721, Votes {
      *
      * 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 from, address to, uint256 tokenId) internal virtual override {
+        super._update(from, to, tokenId);
+        _transferVotingUnits(from, to, 1);
+    }
+
+    /**
+     * @dev See {ERC721-_updateBalance}. We need that to account tokens that were minted in batch.
+     */
+    function _updateBalance(address account, uint128 value) internal virtual override {
+        super._updateBalance(account, value);
+        _transferVotingUnits(address(0), account, value);
     }
 
     /**
diff --git a/contracts/token/ERC721/extensions/ERC721Wrapper.sol b/contracts/token/ERC721/extensions/ERC721Wrapper.sol
index b8c396c3e34..f50a33ac456 100644
--- a/contracts/token/ERC721/extensions/ERC721Wrapper.sol
+++ b/contracts/token/ERC721/extensions/ERC721Wrapper.sol
@@ -52,10 +52,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);
+            address owner = _ownerOf(tokenId);
+            _checkApproved(owner, _msgSender(), tokenId);
+            _burn(owner, tokenId);
             // 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
diff --git a/test/governance/utils/Votes.behavior.js b/test/governance/utils/Votes.behavior.js
index 5836cc35154..77fff6abde6 100644
--- a/test/governance/utils/Votes.behavior.js
+++ b/test/governance/utils/Votes.behavior.js
@@ -269,16 +269,16 @@ function shouldBehaveLikeVotes(accounts, tokens, { mode = 'blocknumber', fungibl
         const t1 = await this.votes.$_mint(accounts[1], tokens[1]);
         await time.advanceBlock();
         // t2 = burn #1
-        const t2 = await this.votes.$_burn(...(fungible ? [accounts[1]] : []), tokens[1]);
+        const t2 = await this.votes.$_burn(accounts[1], tokens[1]);
         await time.advanceBlock();
         // t3 = mint #2
         const t3 = await this.votes.$_mint(accounts[1], tokens[2]);
         await time.advanceBlock();
         // t4 = burn #0
-        const t4 = await this.votes.$_burn(...(fungible ? [accounts[1]] : []), tokens[0]);
+        const t4 = await this.votes.$_burn(accounts[1], tokens[0]);
         await time.advanceBlock();
         // t5 = burn #2
-        const t5 = await this.votes.$_burn(...(fungible ? [accounts[1]] : []), tokens[2]);
+        const t5 = await this.votes.$_burn(accounts[1], tokens[2]);
         await time.advanceBlock();
 
         t0.timepoint = await clockFromReceipt[mode](t0.receipt);
diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js
index 75700f6ab56..6a27bbea23d 100644
--- a/test/token/ERC721/ERC721.behavior.js
+++ b/test/token/ERC721/ERC721.behavior.js
@@ -705,14 +705,14 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
       });
 
       it('reverts when adding a token id that already exists', async function () {
-        await expectRevertCustomError(this.token.$_mint(owner, firstTokenId), 'ERC721InvalidSender', [ZERO_ADDRESS]);
+        await expectRevertCustomError(this.token.$_mint(owner, firstTokenId), 'ERC721IncorrectOwner', [ZERO_ADDRESS, firstTokenId, owner]);
       });
     });
   });
 
   describe('_burn', function () {
     it('reverts when burning a non-existent token id', async function () {
-      await expectRevertCustomError(this.token.$_burn(nonExistentTokenId), 'ERC721NonexistentToken', [
+      await expectRevertCustomError(this.token.$_burn(owner, nonExistentTokenId), 'ERC721NonexistentToken', [
         nonExistentTokenId,
       ]);
     });
@@ -725,7 +725,7 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
 
       context('with burnt token', function () {
         beforeEach(async function () {
-          this.receipt = await this.token.$_burn(firstTokenId);
+          this.receipt = await this.token.$_burn(owner, firstTokenId);
         });
 
         it('emits a Transfer event', function () {
@@ -738,7 +738,7 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
 
         it('reverts when burning a token id that has been deleted', async function () {
-          await expectRevertCustomError(this.token.$_burn(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
+          await expectRevertCustomError(this.token.$_burn(owner, firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
         });
       });
     });
@@ -820,7 +820,7 @@ function shouldBehaveLikeERC721Enumerable(owner, newOwner, approved, anotherAppr
           const newTokenId = new BN(300);
           const anotherNewTokenId = new BN(400);
 
-          await this.token.$_burn(tokenId);
+          await this.token.$_burn(owner, tokenId);
           await this.token.$_mint(newOwner, newTokenId);
           await this.token.$_mint(newOwner, anotherNewTokenId);
 
@@ -860,7 +860,7 @@ function shouldBehaveLikeERC721Enumerable(owner, newOwner, approved, anotherAppr
 
   describe('_burn', function () {
     it('reverts when burning a non-existent token id', async function () {
-      await expectRevertCustomError(this.token.$_burn(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
+      await expectRevertCustomError(this.token.$_burn(owner, firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
     });
 
     context('with minted tokens', function () {
@@ -871,7 +871,7 @@ function shouldBehaveLikeERC721Enumerable(owner, newOwner, approved, anotherAppr
 
       context('with burnt token', function () {
         beforeEach(async function () {
-          this.receipt = await this.token.$_burn(firstTokenId);
+          this.receipt = await this.token.$_burn(owner, firstTokenId);
         });
 
         it('removes that token from the token list of the owner', async function () {
@@ -883,7 +883,7 @@ function shouldBehaveLikeERC721Enumerable(owner, newOwner, approved, anotherAppr
         });
 
         it('burns all tokens', async function () {
-          await this.token.$_burn(secondTokenId, { from: owner });
+          await this.token.$_burn(owner, secondTokenId, { from: owner });
           expect(await this.token.totalSupply()).to.be.bignumber.equal('0');
           await expectRevertCustomError(this.token.tokenByIndex(0), 'ERC721OutOfBoundsIndex', [ZERO_ADDRESS, 0]);
         });
diff --git a/test/token/ERC721/extensions/ERC721Burnable.test.js b/test/token/ERC721/extensions/ERC721Burnable.test.js
index df059e09078..b234832ef79 100644
--- a/test/token/ERC721/extensions/ERC721Burnable.test.js
+++ b/test/token/ERC721/extensions/ERC721Burnable.test.js
@@ -31,7 +31,7 @@ contract('ERC721Burnable', function (accounts) {
 
       describe('when successful', function () {
         beforeEach(async function () {
-          receipt = await this.token.burn(tokenId, { from: owner });
+          receipt = await this.token.burn(owner, tokenId, { from: owner });
         });
 
         it('burns the given token ID and adjusts the balance of the owner', async function () {
@@ -51,7 +51,7 @@ contract('ERC721Burnable', function (accounts) {
       describe('when there is a previous approval burned', function () {
         beforeEach(async function () {
           await this.token.approve(approved, tokenId, { from: owner });
-          receipt = await this.token.burn(tokenId, { from: owner });
+          receipt = await this.token.burn(owner, tokenId, { from: owner });
         });
 
         context('getApproved', function () {
@@ -63,7 +63,7 @@ contract('ERC721Burnable', function (accounts) {
 
       describe('when there is no previous approval burned', function () {
         it('reverts', async function () {
-          await expectRevertCustomError(this.token.burn(tokenId, { from: another }), 'ERC721InsufficientApproval', [
+          await expectRevertCustomError(this.token.burn(owner, tokenId, { from: another }), 'ERC721InsufficientApproval', [
             another,
             tokenId,
           ]);
@@ -72,7 +72,7 @@ contract('ERC721Burnable', function (accounts) {
 
       describe('when the given token ID was not tracked by this contract', function () {
         it('reverts', async function () {
-          await expectRevertCustomError(this.token.burn(unknownTokenId, { from: owner }), 'ERC721NonexistentToken', [
+          await expectRevertCustomError(this.token.burn(owner, unknownTokenId, { from: owner }), 'ERC721NonexistentToken', [
             unknownTokenId,
           ]);
         });
diff --git a/test/token/ERC721/extensions/ERC721Consecutive.t.sol b/test/token/ERC721/extensions/ERC721Consecutive.t.sol
index 617b17a4669..fc03fc685d7 100644
--- a/test/token/ERC721/extensions/ERC721Consecutive.t.sol
+++ b/test/token/ERC721/extensions/ERC721Consecutive.t.sol
@@ -28,8 +28,8 @@ contract ERC721ConsecutiveTarget is StdUtils, ERC721Consecutive {
         }
     }
 
-    function burn(uint256 tokenId) public {
-        _burn(tokenId);
+    function burn(address from, uint256 tokenId) public {
+        _burn(from, tokenId);
     }
 
     function _firstConsecutiveId() internal view virtual override returns (uint96) {
@@ -96,7 +96,7 @@ contract ERC721ConsecutiveTest is Test {
 
         // burn a token in [0; supply[
         uint256 tokenId = bound(unboundedTokenId, startingTokenId, startingTokenId + supply - 1);
-        token.burn(tokenId);
+        token.burn(receiver, tokenId);
 
         // balance should have decreased
         assertEq(token.balanceOf(receiver), supply - 1);
diff --git a/test/token/ERC721/extensions/ERC721Consecutive.test.js b/test/token/ERC721/extensions/ERC721Consecutive.test.js
index 55c26dffe08..f056b99e62a 100644
--- a/test/token/ERC721/extensions/ERC721Consecutive.test.js
+++ b/test/token/ERC721/extensions/ERC721Consecutive.test.js
@@ -131,7 +131,7 @@ contract('ERC721Consecutive', function (accounts) {
         });
 
         it('tokens can be burned and re-minted #1', async function () {
-          expectEvent(await this.token.$_burn(tokenId, { from: user1 }), 'Transfer', {
+          expectEvent(await this.token.$_burn(user1, tokenId, { from: user1 }), 'Transfer', {
             from: user1,
             to: constants.ZERO_ADDRESS,
             tokenId,
@@ -161,7 +161,7 @@ contract('ERC721Consecutive', function (accounts) {
           expect(await this.token.ownerOf(tokenId), user1);
 
           // burn
-          expectEvent(await this.token.$_burn(tokenId, { from: user1 }), 'Transfer', {
+          expectEvent(await this.token.$_burn(user1, tokenId, { from: user1 }), 'Transfer', {
             from: user1,
             to: constants.ZERO_ADDRESS,
             tokenId,
@@ -180,17 +180,6 @@ contract('ERC721Consecutive', function (accounts) {
           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',
-            [],
-          );
-        });
       });
     });
   }
diff --git a/test/token/ERC721/extensions/ERC721Pausable.test.js b/test/token/ERC721/extensions/ERC721Pausable.test.js
index ec99dea96b9..2526e01bbbc 100644
--- a/test/token/ERC721/extensions/ERC721Pausable.test.js
+++ b/test/token/ERC721/extensions/ERC721Pausable.test.js
@@ -57,7 +57,7 @@ contract('ERC721Pausable', function (accounts) {
     });
 
     it('reverts when trying to burn', async function () {
-      await expectRevertCustomError(this.token.$_burn(firstTokenId), 'EnforcedPause', []);
+      await expectRevertCustomError(this.token.$_burn(owner, firstTokenId), 'EnforcedPause', []);
     });
 
     describe('getApproved', function () {
diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js
index 1c0536bf5ff..b0b024b8714 100644
--- a/test/token/ERC721/extensions/ERC721Royalty.test.js
+++ b/test/token/ERC721/extensions/ERC721Royalty.test.js
@@ -29,7 +29,7 @@ contract('ERC721Royalty', function (accounts) {
     });
 
     it('removes royalty information after burn', async function () {
-      await this.token.$_burn(tokenId1);
+      await this.token.$_burn(account1, tokenId1);
       const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice);
 
       expect(tokenInfo[0]).to.be.equal(constants.ZERO_ADDRESS);
diff --git a/test/token/ERC721/extensions/ERC721URIStorage.test.js b/test/token/ERC721/extensions/ERC721URIStorage.test.js
index 34738cae12a..58fb77ad0db 100644
--- a/test/token/ERC721/extensions/ERC721URIStorage.test.js
+++ b/test/token/ERC721/extensions/ERC721URIStorage.test.js
@@ -84,7 +84,7 @@ contract('ERC721URIStorage', function (accounts) {
     });
 
     it('tokens without URI can be burnt ', async function () {
-      await this.token.$_burn(firstTokenId, { from: owner });
+      await this.token.$_burn(owner, firstTokenId, { from: owner });
 
       expect(await this.token.$_exists(firstTokenId)).to.equal(false);
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
@@ -93,7 +93,7 @@ contract('ERC721URIStorage', function (accounts) {
     it('tokens with URI can be burnt ', async function () {
       await this.token.$_setTokenURI(firstTokenId, sampleUri);
 
-      await this.token.$_burn(firstTokenId, { from: owner });
+      await this.token.$_burn(owner, firstTokenId, { from: owner });
 
       expect(await this.token.$_exists(firstTokenId)).to.equal(false);
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);

From 930ee64d89cafabd273a5c591256c57a97c0f6e1 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Thu, 6 Jul 2023 21:42:30 -0300
Subject: [PATCH 2/8] Update contracts/token/ERC721/ERC721.sol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/token/ERC721/ERC721.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index a58e28cf705..6b8cf2c51fd 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -199,7 +199,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * 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 {_updateBalance} to keep balances
-     * consistent with ownership. The invariant being that for any address `a` the value returned by
+     * 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 _ownerOf(uint256 tokenId) internal view virtual returns (address) {

From 14ffe4d0fd0a5c1e863986a9a26cd023ba198fd2 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 12:36:28 -0600
Subject: [PATCH 3/8] Apply suggestions

---
 contracts/token/ERC721/ERC721.sol             | 44 ++++++++++++-------
 .../ERC721/extensions/ERC721URIStorage.sol    |  6 +--
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index 6b8cf2c51fd..d7321abaf67 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -118,8 +118,9 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
             revert ERC721InvalidOperator(owner);
         }
 
-        if (_msgSender() != owner && !isApprovedForAll(owner, _msgSender())) {
-            revert ERC721InvalidApprover(_msgSender());
+        address sender = _msgSender();
+        if (sender != owner && !isApprovedForAll(owner, sender)) {
+            revert ERC721InvalidApprover(sender);
         }
 
         _approve(to, tokenId);
@@ -182,9 +183,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Requirements:
      *
-     * - `from` cannot be the zero address.
-     * - `to` cannot be the zero address.
-     * - `tokenId` token must exist and be owned by `from`.
+     * - `from` and `to` cannot be the zero address
+     * - `tokenId` token must exist `from` should be the current owner of the token or an approved operator.
      * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
      *
      * Emits a {Transfer} event.
@@ -230,6 +230,10 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
         return owner == spender || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender;
     }
 
+    /**
+     * @dev Checks whether `spender` was approved by `owner` to operate on `tokenId` or is the owner of `tokenId`.
+     * Reverts if `spender` is not approved nor owner of `tokenId`.
+     */
     function _checkApproved(address owner, address spender, uint256 tokenId) internal view virtual {
         if (!_isApproved(owner, spender, tokenId)) {
             address actualOwner = _ownerOf(tokenId);
@@ -290,7 +294,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      *
      * Requirements:
      *
-     * - `tokenId` must exist.
+     * - `tokenId` must exist, so `from` cannot be the zero address.
+     * - `from` should be the current owner of the token or an approved operator.
      *
      * Emits a {Transfer} event.
      */
@@ -303,12 +308,12 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
 
     /**
      * @dev Transfers `tokenId` from `from` to `to`.
-     *  As opposed to {transferFrom}, this imposes no restrictions on msg.sender.
+     * As opposed to {transferFrom}, this imposes no restrictions on msg.sender.
      *
      * Requirements:
      *
-     * - `to` cannot be the zero address.
-     * - `tokenId` token must be owned by `from`.
+     * - `from` and `to` cannot be the zero address
+     * - `from` should be the current owner of the token or an approved operator.
      *
      * Emits a {Transfer} event.
      */
@@ -322,6 +327,18 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
         _update(from, to, tokenId);
     }
 
+    /**
+     * @dev Updates the ownership of toke with id `tokenId` from `from` to `to`.
+     * It clears the approval for the token when is transferred.
+     *
+     * Requirements:
+     *
+     * - `from` should be the current owner of the token or an approved operator.
+     *
+     * NOTE: The `from` should be `address(0)` for mints and `to` should be `address(0)` for burns.
+     *
+     * Emits a {Transfer} event.
+     */
     function _update(address from, address to, uint256 tokenId) internal virtual {
         address owner = _ownerOf(tokenId);
         if (from != owner) {
@@ -335,6 +352,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
         if (from != address(0)) {
             delete _tokenApprovals[tokenId];
             unchecked {
+                // Decrease balance with unchecked arithmetic because `owner == from` and
+                // therefore we can guarantee `_balances[from] >= 1`.
                 _balances[from] -= 1;
             }
         }
@@ -400,12 +419,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @param tokenId uint256 ID of the token to be transferred
      * @param data bytes optional data to send along with the call
      */
-    function _checkOnERC721Received(
-        address from,
-        address to,
-        uint256 tokenId,
-        bytes memory data
-    ) private {
+    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) {
                 if (retval != IERC721Receiver.onERC721Received.selector) {
diff --git a/contracts/token/ERC721/extensions/ERC721URIStorage.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol
index 7f7e754136f..6f7ed72246f 100644
--- a/contracts/token/ERC721/extensions/ERC721URIStorage.sol
+++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol
@@ -71,10 +71,8 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
     function _update(address from, address to, uint256 tokenId) internal virtual override {
         super._update(from, to, tokenId);
 
-        if (to == address(0)) {
-            if (bytes(_tokenURIs[tokenId]).length != 0) {
-                delete _tokenURIs[tokenId];
-            }
+        if (to == address(0) && bytes(_tokenURIs[tokenId]).length != 0) {
+            delete _tokenURIs[tokenId];
         }
     }
 }

From 63717fc33028c5e8340328c00e115da5da68db35 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 12:43:55 -0600
Subject: [PATCH 4/8] Rename updateBalance to increaseBalance

---
 .../mocks/token/ERC721ConsecutiveEnumerableMock.sol | 10 +++++++---
 contracts/mocks/token/ERC721ConsecutiveMock.sol     |  7 ++-----
 contracts/token/ERC721/ERC721.sol                   |  7 ++++---
 .../token/ERC721/extensions/ERC721Consecutive.sol   |  2 +-
 .../token/ERC721/extensions/ERC721Enumerable.sol    | 13 ++++++-------
 contracts/token/ERC721/extensions/ERC721Votes.sol   |  6 +++---
 6 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol b/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
index a3c751a2624..725aaaa5dda 100644
--- a/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
+++ b/contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
@@ -28,11 +28,15 @@ contract ERC721ConsecutiveEnumerableMock is ERC721Consecutive, ERC721Enumerable
         return super._ownerOf(tokenId);
     }
 
-    function _update(address from, address to, uint256 tokenId) internal virtual override(ERC721Consecutive, ERC721Enumerable) {
+    function _update(
+        address from,
+        address to,
+        uint256 tokenId
+    ) internal virtual override(ERC721Consecutive, ERC721Enumerable) {
         super._update(from, to, tokenId);
     }
 
-    function _updateBalance(address account, uint128 value) internal virtual override(ERC721, ERC721Enumerable) {
-        super._updateBalance(account, value);
+    function _increaseBalance(address account, uint128 value) internal virtual override(ERC721, ERC721Enumerable) {
+        super._increaseBalance(account, value);
     }
 }
diff --git a/contracts/mocks/token/ERC721ConsecutiveMock.sol b/contracts/mocks/token/ERC721ConsecutiveMock.sol
index 6864ccafdb3..c6f0e67caf4 100644
--- a/contracts/mocks/token/ERC721ConsecutiveMock.sol
+++ b/contracts/mocks/token/ERC721ConsecutiveMock.sol
@@ -49,11 +49,8 @@ contract ERC721ConsecutiveMock is ERC721Consecutive, ERC721Pausable, ERC721Votes
         super._update(from, to, tokenId);
     }
 
-    function _updateBalance(
-        address account,
-        uint128 value
-    ) internal virtual override(ERC721, ERC721Votes) {
-        super._updateBalance(account, value);
+    function _increaseBalance(address account, uint128 value) internal virtual override(ERC721, ERC721Votes) {
+        super._increaseBalance(account, value);
     }
 }
 
diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index d7321abaf67..abfb44dcd20 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -198,7 +198,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev Returns the owner of the `tokenId`. Returns `address(0)` if token doesn't exist.
      *
      * 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 {_updateBalance} to keep balances
+     * 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`.
      */
@@ -370,9 +370,10 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     }
 
     /**
-     * @dev Unsafe write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
+     * @dev Increases the balance of `account` by `value`.
+     * Write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
      */
-    function _updateBalance(address account, uint128 value) internal virtual {
+    function _increaseBalance(address account, uint128 value) internal virtual {
         unchecked {
             _balances[account] += value;
         }
diff --git a/contracts/token/ERC721/extensions/ERC721Consecutive.sol b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
index 94475b01419..ed3b919935c 100644
--- a/contracts/token/ERC721/extensions/ERC721Consecutive.sol
+++ b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
@@ -121,7 +121,7 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
 
             // Preserve the invariant required by `_ownerOf`, given that the new sequentialOwnership
             // checkpoint is attributing ownership of `batchSize` new tokens to account `to`.
-            _updateBalance(to, batchSize);
+            _increaseBalance(to, batchSize);
 
             emit ConsecutiveTransfer(next, last, address(0), to);
         }
diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
index 4f8a59f884d..e9fe2fdd167 100644
--- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
@@ -76,11 +76,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
     /**
      * @dev See {ERC721-_beforeTokenTransfer}.
      */
-    function _update(
-        address from,
-        address to,
-        uint256 tokenId
-    ) internal virtual override {
+    function _update(address from, address to, uint256 tokenId) internal virtual override {
         if (from == address(0)) {
             _addTokenToAllTokensEnumeration(tokenId);
         } else if (from != to) {
@@ -168,10 +164,13 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         _allTokens.pop();
     }
 
-    function _updateBalance(address account, uint128 value) internal virtual override {
+    /**
+     * @dev See {ERC721-_increaseBalance}.
+     */
+    function _increaseBalance(address account, uint128 value) internal virtual override {
         if (value > 0) {
             revert ERC721EnumerableForbiddenBatchMint();
         }
-        super._updateBalance(account, value);
+        super._increaseBalance(account, value);
     }
 }
diff --git a/contracts/token/ERC721/extensions/ERC721Votes.sol b/contracts/token/ERC721/extensions/ERC721Votes.sol
index 1ecc8baf68b..6e8894f5978 100644
--- a/contracts/token/ERC721/extensions/ERC721Votes.sol
+++ b/contracts/token/ERC721/extensions/ERC721Votes.sol
@@ -28,10 +28,10 @@ abstract contract ERC721Votes is ERC721, Votes {
     }
 
     /**
-     * @dev See {ERC721-_updateBalance}. We need that to account tokens that were minted in batch.
+     * @dev See {ERC721-_increaseBalance}. We need that to account tokens that were minted in batch.
      */
-    function _updateBalance(address account, uint128 value) internal virtual override {
-        super._updateBalance(account, value);
+    function _increaseBalance(address account, uint128 value) internal virtual override {
+        super._increaseBalance(account, value);
         _transferVotingUnits(address(0), account, value);
     }
 

From 5f05fc9ba126740fc5caf1a3fc363c56aa3f0a5c Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 14:29:12 -0600
Subject: [PATCH 5/8] Fix tests (omg hopefully)

---
 contracts/token/ERC721/ERC721.sol               |  2 +-
 .../ERC721/extensions/ERC721Consecutive.sol     |  2 +-
 .../ERC721/extensions/ERC721Enumerable.sol      |  8 ++++----
 .../token/ERC721/extensions/ERC721Pausable.sol  |  6 +-----
 test/token/ERC721/ERC721.behavior.js            | 10 ++++++++--
 .../ERC721/extensions/ERC721Burnable.test.js    | 17 ++++++++++-------
 .../ERC721/extensions/ERC721Consecutive.test.js | 14 +++++---------
 7 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index abfb44dcd20..5317ab778d0 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -328,7 +328,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     }
 
     /**
-     * @dev Updates the ownership of toke with id `tokenId` from `from` to `to`.
+     * @dev Updates the ownership of token with id `tokenId` from `from` to `to`.
      * It clears the approval for the token when is transferred.
      *
      * Requirements:
diff --git a/contracts/token/ERC721/extensions/ERC721Consecutive.sol b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
index ed3b919935c..e82a31d037d 100644
--- a/contracts/token/ERC721/extensions/ERC721Consecutive.sol
+++ b/contracts/token/ERC721/extensions/ERC721Consecutive.sol
@@ -134,7 +134,7 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
      * during construction, and burning of tokens that have been sequentially minted are complete.
      */
     function _update(address from, address to, uint256 tokenId) internal virtual override {
-        if (to == address(0) && address(this).code.length == 0) {
+        if (from == address(0) && address(this).code.length == 0) {
             revert ERC721ForbiddenMint();
         }
 
diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
index e9fe2fdd167..ce0a0730e48 100644
--- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol
@@ -77,6 +77,8 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
      * @dev See {ERC721-_beforeTokenTransfer}.
      */
     function _update(address from, address to, uint256 tokenId) internal virtual override {
+        super._update(from, to, tokenId);
+
         if (from == address(0)) {
             _addTokenToAllTokensEnumeration(tokenId);
         } else if (from != to) {
@@ -87,8 +89,6 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
         } else if (to != from) {
             _addTokenToOwnerEnumeration(to, tokenId);
         }
-
-        super._update(from, to, tokenId);
     }
 
     /**
@@ -97,7 +97,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; // Balance is already increased by super._update
         _ownedTokens[to][length] = tokenId;
         _ownedTokensIndex[tokenId] = length;
     }
@@ -123,7 +123,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); // Balance is already decreased by super._update
         uint256 tokenIndex = _ownedTokensIndex[tokenId];
 
         // When the token to delete is the last token, the swap operation is unnecessary
diff --git a/contracts/token/ERC721/extensions/ERC721Pausable.sol b/contracts/token/ERC721/extensions/ERC721Pausable.sol
index 96871de76c9..f549c6486d9 100644
--- a/contracts/token/ERC721/extensions/ERC721Pausable.sol
+++ b/contracts/token/ERC721/extensions/ERC721Pausable.sol
@@ -27,11 +27,7 @@ abstract contract ERC721Pausable is ERC721, Pausable {
      *
      * - the contract must not be paused.
      */
-    function _update(
-        address from,
-        address to,
-        uint256 tokenId
-    ) internal virtual override {
+    function _update(address from, address to, uint256 tokenId) internal virtual override {
         _requireNotPaused();
         super._update(from, to, tokenId);
     }
diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js
index 6a27bbea23d..ec660d28754 100644
--- a/test/token/ERC721/ERC721.behavior.js
+++ b/test/token/ERC721/ERC721.behavior.js
@@ -705,7 +705,11 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
       });
 
       it('reverts when adding a token id that already exists', async function () {
-        await expectRevertCustomError(this.token.$_mint(owner, firstTokenId), 'ERC721IncorrectOwner', [ZERO_ADDRESS, firstTokenId, owner]);
+        await expectRevertCustomError(this.token.$_mint(owner, firstTokenId), 'ERC721IncorrectOwner', [
+          ZERO_ADDRESS,
+          firstTokenId,
+          owner,
+        ]);
       });
     });
   });
@@ -738,7 +742,9 @@ function shouldBehaveLikeERC721(owner, newOwner, approved, anotherApproved, oper
         });
 
         it('reverts when burning a token id that has been deleted', async function () {
-          await expectRevertCustomError(this.token.$_burn(owner, firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
+          await expectRevertCustomError(this.token.$_burn(owner, firstTokenId), 'ERC721NonexistentToken', [
+            firstTokenId,
+          ]);
         });
       });
     });
diff --git a/test/token/ERC721/extensions/ERC721Burnable.test.js b/test/token/ERC721/extensions/ERC721Burnable.test.js
index b234832ef79..1779c8c3396 100644
--- a/test/token/ERC721/extensions/ERC721Burnable.test.js
+++ b/test/token/ERC721/extensions/ERC721Burnable.test.js
@@ -63,18 +63,21 @@ contract('ERC721Burnable', function (accounts) {
 
       describe('when there is no previous approval burned', function () {
         it('reverts', async function () {
-          await expectRevertCustomError(this.token.burn(owner, tokenId, { from: another }), 'ERC721InsufficientApproval', [
-            another,
-            tokenId,
-          ]);
+          await expectRevertCustomError(
+            this.token.burn(owner, tokenId, { from: another }),
+            'ERC721InsufficientApproval',
+            [another, tokenId],
+          );
         });
       });
 
       describe('when the given token ID was not tracked by this contract', function () {
         it('reverts', async function () {
-          await expectRevertCustomError(this.token.burn(owner, unknownTokenId, { from: owner }), 'ERC721NonexistentToken', [
-            unknownTokenId,
-          ]);
+          await expectRevertCustomError(
+            this.token.burn(owner, unknownTokenId, { from: owner }),
+            'ERC721NonexistentToken',
+            [unknownTokenId],
+          );
         });
       });
     });
diff --git a/test/token/ERC721/extensions/ERC721Consecutive.test.js b/test/token/ERC721/extensions/ERC721Consecutive.test.js
index f056b99e62a..1166f211b72 100644
--- a/test/token/ERC721/extensions/ERC721Consecutive.test.js
+++ b/test/token/ERC721/extensions/ERC721Consecutive.test.js
@@ -117,7 +117,11 @@ contract('ERC721Consecutive', function (accounts) {
 
           expect(await this.token.$_exists(tokenId)).to.be.equal(true);
 
-          await expectRevertCustomError(this.token.$_mint(user1, tokenId), 'ERC721InvalidSender', [ZERO_ADDRESS]);
+          await expectRevertCustomError(this.token.$_mint(user1, tokenId), 'ERC721IncorrectOwner', [
+            ZERO_ADDRESS,
+            tokenId,
+            await this.token.ownerOf(tokenId),
+          ]);
         });
       });
 
@@ -201,14 +205,6 @@ contract('ERC721Consecutive', function (accounts) {
       );
     });
 
-    it('cannot use single minting during construction', async function () {
-      await expectRevertCustomError(
-        ERC721ConsecutiveNoConstructorMintMock.new(name, symbol),
-        'ERC721ForbiddenMint',
-        [],
-      );
-    });
-
     it('consecutive mint not compatible with enumerability', async function () {
       await expectRevertCustomError(
         ERC721ConsecutiveEnumerableMock.new(

From 38c0bf00c11e4200fbba8881019ca1c287cb9cca Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 14:31:59 -0600
Subject: [PATCH 6/8] Add changeset

---
 .changeset/bright-tomatoes-sing.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.changeset/bright-tomatoes-sing.md b/.changeset/bright-tomatoes-sing.md
index dcd6d7ff027..9eb230ad17d 100644
--- a/.changeset/bright-tomatoes-sing.md
+++ b/.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), [#4419](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4419), [#3876](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3876))

From ca05bc561be220e7c7207a098e1bcf8b5a9cb084 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 14:38:00 -0600
Subject: [PATCH 7/8] Use _increaseBalance in _update

---
 contracts/token/ERC721/ERC721.sol | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index 5317ab778d0..ab2b7dc7fa5 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -359,9 +359,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
         }
 
         if (to != address(0)) {
-            unchecked {
-                _balances[to] += 1;
-            }
+            _increaseBalance(to, 1);
         }
 
         _owners[tokenId] = to;
@@ -370,8 +368,10 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     }
 
     /**
-     * @dev Increases the balance of `account` by `value`.
-     * Write access to the balances, used by extensions that "mint" tokens using an {ownerOf} override.
+     * @dev Increases the balance of `account` by an arbitrary `value`.
+     *
+     * This function provides write access to the balances, which can be used by extensions that
+     * "mint" tokens using an {ownerOf} override.
      */
     function _increaseBalance(address account, uint128 value) internal virtual {
         unchecked {

From cdacda12173d540137d263ff3779985b0519ae9a Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 7 Jul 2023 15:18:22 -0600
Subject: [PATCH 8/8] Rollback _increaseBalance in _update

---
 contracts/token/ERC721/ERC721.sol | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
index ab2b7dc7fa5..a872e7307a4 100644
--- a/contracts/token/ERC721/ERC721.sol
+++ b/contracts/token/ERC721/ERC721.sol
@@ -359,7 +359,9 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
         }
 
         if (to != address(0)) {
-            _increaseBalance(to, 1);
+            unchecked {
+                _balances[to] += 1;
+            }
         }
 
         _owners[tokenId] = to;