Browse Source

Merge branch 'master' into chore/certora-CVL2

ernestognw 2 years ago
parent
commit
de70816716
54 changed files with 527 additions and 564 deletions
  1. 1 1
      .changeset/bright-tomatoes-sing.md
  2. 5 0
      .changeset/eighty-lemons-shake.md
  3. 13 1
      CHANGELOG.md
  4. 5 5
      contracts/access/AccessControl.sol
  5. 1 1
      contracts/access/extensions/AccessControlEnumerable.sol
  6. 1 1
      contracts/finance/VestingWallet.sol
  7. 1 1
      contracts/governance/Governor.sol
  8. 1 1
      contracts/governance/TimelockController.sol
  9. 2 2
      contracts/governance/extensions/GovernorCountingSimple.sol
  10. 1 1
      contracts/governance/extensions/GovernorPreventLateQuorum.sol
  11. 1 1
      contracts/governance/extensions/GovernorStorage.sol
  12. 1 1
      contracts/governance/extensions/GovernorTimelockControl.sol
  13. 4 4
      contracts/governance/utils/Votes.sol
  14. 0 4
      contracts/metatx/ERC2771Forwarder.sol
  15. 1 1
      contracts/mocks/ERC165/ERC165InterfacesSupported.sol
  16. 2 2
      contracts/mocks/StorageSlotMock.sol
  17. 1 1
      contracts/mocks/VotesMock.sol
  18. 4 4
      contracts/mocks/token/ERC20VotesLegacyMock.sol
  19. 7 17
      contracts/mocks/token/ERC721ConsecutiveEnumerableMock.sol
  20. 7 17
      contracts/mocks/token/ERC721ConsecutiveMock.sol
  21. 2 4
      contracts/token/ERC1155/ERC1155.sol
  22. 1 1
      contracts/token/ERC1155/extensions/ERC1155Supply.sol
  23. 1 1
      contracts/token/ERC1155/extensions/ERC1155URIStorage.sol
  24. 2 2
      contracts/token/ERC20/ERC20.sol
  25. 184 211
      contracts/token/ERC721/ERC721.sol
  26. 1 1
      contracts/token/ERC721/IERC721.sol
  27. 3 4
      contracts/token/ERC721/extensions/ERC721Burnable.sol
  28. 16 33
      contracts/token/ERC721/extensions/ERC721Consecutive.sol
  29. 24 30
      contracts/token/ERC721/extensions/ERC721Enumerable.sol
  30. 6 9
      contracts/token/ERC721/extensions/ERC721Pausable.sol
  31. 9 4
      contracts/token/ERC721/extensions/ERC721Royalty.sol
  32. 8 6
      contracts/token/ERC721/extensions/ERC721URIStorage.sol
  33. 15 9
      contracts/token/ERC721/extensions/ERC721Votes.sol
  34. 3 4
      contracts/token/ERC721/extensions/ERC721Wrapper.sol
  35. 1 1
      contracts/token/common/ERC2981.sol
  36. 1 1
      contracts/utils/Nonces.sol
  37. 1 1
      contracts/utils/structs/BitMaps.sol
  38. 1 1
      contracts/utils/structs/DoubleEndedQueue.sol
  39. 1 1
      contracts/utils/structs/EnumerableMap.sol
  40. 1 1
      contracts/utils/structs/EnumerableSet.sol
  41. 15 1
      docs/modules/ROOT/pages/index.adoc
  42. 1 1
      remappings.txt
  43. 1 1
      scripts/generate/templates/EnumerableMap.js
  44. 1 1
      scripts/generate/templates/EnumerableSet.js
  45. 2 2
      test/governance/Governor.t.sol
  46. 2 2
      test/metatx/ERC2771Forwarder.t.sol
  47. 5 5
      test/token/ERC20/extensions/ERC4626.t.sol
  48. 154 131
      test/token/ERC721/ERC721.behavior.js
  49. 2 2
      test/token/ERC721/extensions/ERC721Consecutive.t.sol
  50. 2 17
      test/token/ERC721/extensions/ERC721Consecutive.test.js
  51. 0 6
      test/token/ERC721/extensions/ERC721Pausable.test.js
  52. 0 2
      test/token/ERC721/extensions/ERC721URIStorage.test.js
  53. 1 1
      test/utils/ShortStrings.t.sol
  54. 1 1
      test/utils/math/Math.t.sol

+ 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:

+ 5 - 5
contracts/access/AccessControl.sol

@@ -48,11 +48,11 @@ import {ERC165} from "../utils/introspection/ERC165.sol";
  */
 abstract contract AccessControl is Context, IAccessControl, ERC165 {
     struct RoleData {
-        mapping(address => bool) members;
+        mapping(address account => bool) hasRole;
         bytes32 adminRole;
     }
 
-    mapping(bytes32 => RoleData) private _roles;
+    mapping(bytes32 role => RoleData) private _roles;
 
     bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
 
@@ -76,7 +76,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      * @dev Returns `true` if `account` has been granted `role`.
      */
     function hasRole(bytes32 role, address account) public view virtual returns (bool) {
-        return _roles[role].members[account];
+        return _roles[role].hasRole[account];
     }
 
     /**
@@ -182,7 +182,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      */
     function _grantRole(bytes32 role, address account) internal virtual returns (bool) {
         if (!hasRole(role, account)) {
-            _roles[role].members[account] = true;
+            _roles[role].hasRole[account] = true;
             emit RoleGranted(role, account, _msgSender());
             return true;
         } else {
@@ -199,7 +199,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      */
     function _revokeRole(bytes32 role, address account) internal virtual returns (bool) {
         if (hasRole(role, account)) {
-            _roles[role].members[account] = false;
+            _roles[role].hasRole[account] = false;
             emit RoleRevoked(role, account, _msgSender());
             return true;
         } else {

+ 1 - 1
contracts/access/extensions/AccessControlEnumerable.sol

@@ -13,7 +13,7 @@ import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol";
 abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;
 
-    mapping(bytes32 => EnumerableSet.AddressSet) private _roleMembers;
+    mapping(bytes32 role => EnumerableSet.AddressSet) private _roleMembers;
 
     /**
      * @dev See {IERC165-supportsInterface}.

+ 1 - 1
contracts/finance/VestingWallet.sol

@@ -37,7 +37,7 @@ contract VestingWallet is Context, Ownable {
     error VestingWalletInvalidBeneficiary(address beneficiary);
 
     uint256 private _released;
-    mapping(address => uint256) private _erc20Released;
+    mapping(address token => uint256) private _erc20Released;
     uint64 private immutable _start;
     uint64 private immutable _duration;
 

+ 1 - 1
contracts/governance/Governor.sol

@@ -46,7 +46,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     bytes32 private constant _ALL_PROPOSAL_STATES_BITMAP = bytes32((2 ** (uint8(type(ProposalState).max) + 1)) - 1);
     string private _name;
 
-    mapping(uint256 => ProposalCore) private _proposals;
+    mapping(uint256 proposalId => ProposalCore) private _proposals;
 
     // This queue keeps track of the governor operating on itself. Calls to functions protected by the
     // {onlyGovernance} modifier needs to be whitelisted in this queue. Whitelisting is set in {_beforeExecute},

+ 1 - 1
contracts/governance/TimelockController.sol

@@ -27,7 +27,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     bytes32 public constant CANCELLER_ROLE = keccak256("CANCELLER_ROLE");
     uint256 internal constant _DONE_TIMESTAMP = uint256(1);
 
-    mapping(bytes32 => uint256) private _timestamps;
+    mapping(bytes32 id => uint256) private _timestamps;
     uint256 private _minDelay;
 
     enum OperationState {

+ 2 - 2
contracts/governance/extensions/GovernorCountingSimple.sol

@@ -22,10 +22,10 @@ abstract contract GovernorCountingSimple is Governor {
         uint256 againstVotes;
         uint256 forVotes;
         uint256 abstainVotes;
-        mapping(address => bool) hasVoted;
+        mapping(address voter => bool) hasVoted;
     }
 
-    mapping(uint256 => ProposalVote) private _proposalVotes;
+    mapping(uint256 proposalId => ProposalVote) private _proposalVotes;
 
     /**
      * @dev See {IGovernor-COUNTING_MODE}.

+ 1 - 1
contracts/governance/extensions/GovernorPreventLateQuorum.sol

@@ -18,7 +18,7 @@ import {Math} from "../../utils/math/Math.sol";
 abstract contract GovernorPreventLateQuorum is Governor {
     uint48 private _voteExtension;
 
-    mapping(uint256 => uint48) private _extendedDeadlines;
+    mapping(uint256 proposalId => uint48) private _extendedDeadlines;
 
     /// @dev Emitted when a proposal deadline is pushed back due to reaching quorum late in its voting period.
     event ProposalExtended(uint256 indexed proposalId, uint64 extendedDeadline);

+ 1 - 1
contracts/governance/extensions/GovernorStorage.sol

@@ -21,7 +21,7 @@ abstract contract GovernorStorage is Governor {
     }
 
     uint256[] private _proposalIds;
-    mapping(uint256 => ProposalDetails) private _proposalDetails;
+    mapping(uint256 proposalId => ProposalDetails) private _proposalDetails;
 
     /**
      * @dev Hook into the proposing mechanism

+ 1 - 1
contracts/governance/extensions/GovernorTimelockControl.sol

@@ -24,7 +24,7 @@ import {SafeCast} from "../../utils/math/SafeCast.sol";
  */
 abstract contract GovernorTimelockControl is Governor {
     TimelockController private _timelock;
-    mapping(uint256 => bytes32) private _timelockIds;
+    mapping(uint256 proposalId => bytes32) private _timelockIds;
 
     /**
      * @dev Emitted when the timelock controller used for proposal execution is modified.

+ 4 - 4
contracts/governance/utils/Votes.sol

@@ -34,9 +34,9 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
     bytes32 private constant _DELEGATION_TYPEHASH =
         keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
 
-    mapping(address => address) private _delegation;
+    mapping(address account => address) private _delegatee;
 
-    mapping(address => Checkpoints.Trace224) private _delegateCheckpoints;
+    mapping(address delegatee => Checkpoints.Trace224) private _delegateCheckpoints;
 
     Checkpoints.Trace224 private _totalCheckpoints;
 
@@ -124,7 +124,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
      * @dev Returns the delegate that `account` has chosen.
      */
     function delegates(address account) public view virtual returns (address) {
-        return _delegation[account];
+        return _delegatee[account];
     }
 
     /**
@@ -166,7 +166,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
      */
     function _delegate(address account, address delegatee) internal virtual {
         address oldDelegate = delegates(account);
-        _delegation[account] = delegatee;
+        _delegatee[account] = delegatee;
 
         emit DelegateChanged(account, oldDelegate, delegatee);
         _moveDelegateVotes(oldDelegate, delegatee, _getVotingUnits(account));

+ 0 - 4
contracts/metatx/ERC2771Forwarder.sol

@@ -27,10 +27,6 @@ import {Address} from "../utils/Address.sol";
  * transactions in the mempool. In these cases the recommendation is to distribute the load among
  * multiple accounts.
  *
- * WARNING: Do not approve this contract to spend tokens. Anyone can use this forwarder
- * to execute calls with an arbitrary calldata to any address. Any form of approval may
- * result in a loss of funds for the approving party.
- *
  * NOTE: Batching requests includes an optional refund for unused `msg.value` that is achieved by
  * performing a call with empty calldata. While this is within the bounds of ERC-2771 compliance,
  * if the refund receiver happens to consider the forwarder a trusted forwarder, it MUST properly

+ 1 - 1
contracts/mocks/ERC165/ERC165InterfacesSupported.sol

@@ -23,7 +23,7 @@ contract SupportsInterfaceWithLookupMock is IERC165 {
     /**
      * @dev A mapping of interface id to whether or not it's supported.
      */
-    mapping(bytes4 => bool) private _supportedInterfaces;
+    mapping(bytes4 interfaceId => bool) private _supportedInterfaces;
 
     /**
      * @dev A contract implementing SupportsInterfaceWithLookup

+ 2 - 2
contracts/mocks/StorageSlotMock.sol

@@ -39,7 +39,7 @@ contract StorageSlotMock {
         return slot.getUint256Slot().value;
     }
 
-    mapping(uint256 => string) public stringMap;
+    mapping(uint256 key => string) public stringMap;
 
     function setString(bytes32 slot, string calldata value) public {
         slot.getStringSlot().value = value;
@@ -57,7 +57,7 @@ contract StorageSlotMock {
         return stringMap[key].getStringSlot().value;
     }
 
-    mapping(uint256 => bytes) public bytesMap;
+    mapping(uint256 key => bytes) public bytesMap;
 
     function setBytes(bytes32 slot, bytes calldata value) public {
         slot.getBytesSlot().value = value;

+ 1 - 1
contracts/mocks/VotesMock.sol

@@ -5,7 +5,7 @@ pragma solidity ^0.8.20;
 import {Votes} from "../governance/utils/Votes.sol";
 
 abstract contract VotesMock is Votes {
-    mapping(address => uint256) private _votingUnits;
+    mapping(address voter => uint256) private _votingUnits;
 
     function getTotalSupply() public view returns (uint256) {
         return _getTotalSupply();

+ 4 - 4
contracts/mocks/token/ERC20VotesLegacyMock.sol

@@ -20,8 +20,8 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit {
     bytes32 private constant _DELEGATION_TYPEHASH =
         keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
 
-    mapping(address => address) private _delegates;
-    mapping(address => Checkpoint[]) private _checkpoints;
+    mapping(address account => address) private _delegatee;
+    mapping(address delegatee => Checkpoint[]) private _checkpoints;
     Checkpoint[] private _totalSupplyCheckpoints;
 
     /**
@@ -42,7 +42,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit {
      * @dev Get the address `account` is currently delegating to.
      */
     function delegates(address account) public view virtual returns (address) {
-        return _delegates[account];
+        return _delegatee[account];
     }
 
     /**
@@ -188,7 +188,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit {
     function _delegate(address delegator, address delegatee) internal virtual {
         address currentDelegate = delegates(delegator);
         uint256 delegatorBalance = balanceOf(delegator);
-        _delegates[delegator] = delegatee;
+        _delegatee[delegator] = delegatee;
 
         emit DelegateChanged(delegator, currentDelegate, delegatee);
 

+ 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);
     }
 }
 

+ 2 - 4
contracts/token/ERC1155/ERC1155.sol

@@ -20,11 +20,9 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
     using Arrays for uint256[];
     using Arrays for address[];
 
-    // Mapping from token ID to account balances
-    mapping(uint256 => mapping(address => uint256)) private _balances;
+    mapping(uint256 id => mapping(address account => uint256)) private _balances;
 
-    // Mapping from account to operator approvals
-    mapping(address => mapping(address => bool)) private _operatorApprovals;
+    mapping(address account => mapping(address operator => bool)) private _operatorApprovals;
 
     // Used as the URI for all token types by relying on ID substitution, e.g. https://token-cdn-domain/{id}.json
     string private _uri;

+ 1 - 1
contracts/token/ERC1155/extensions/ERC1155Supply.sol

@@ -19,7 +19,7 @@ import {ERC1155} from "../ERC1155.sol";
  * CAUTION: This extension should not be added in an upgrade to an already deployed contract.
  */
 abstract contract ERC1155Supply is ERC1155 {
-    mapping(uint256 => uint256) private _totalSupply;
+    mapping(uint256 id => uint256) private _totalSupply;
     uint256 private _totalSupplyAll;
 
     /**

+ 1 - 1
contracts/token/ERC1155/extensions/ERC1155URIStorage.sol

@@ -17,7 +17,7 @@ abstract contract ERC1155URIStorage is ERC1155 {
     string private _baseURI = "";
 
     // Optional mapping for token URIs
-    mapping(uint256 => string) private _tokenURIs;
+    mapping(uint256 tokenId => string) private _tokenURIs;
 
     /**
      * @dev See {IERC1155MetadataURI-uri}.

+ 2 - 2
contracts/token/ERC20/ERC20.sol

@@ -36,9 +36,9 @@ import {IERC20Errors} from "../../interfaces/draft-IERC6093.sol";
  * allowances. See {IERC20-approve}.
  */
 abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
-    mapping(address => uint256) private _balances;
+    mapping(address account => uint256) private _balances;
 
-    mapping(address => mapping(address => uint256)) private _allowances;
+    mapping(address account => mapping(address spender => uint256)) private _allowances;
 
     uint256 private _totalSupply;
 

+ 184 - 211
contracts/token/ERC721/ERC721.sol

@@ -25,17 +25,13 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     // Token symbol
     string private _symbol;
 
-    // Mapping from token ID to owner address
-    mapping(uint256 => address) private _owners;
+    mapping(uint256 tokenId => address) private _owners;
 
-    // Mapping owner address to token count
-    mapping(address => uint256) private _balances;
+    mapping(address owner => uint256) private _balances;
 
-    // Mapping from token ID to approved address
-    mapping(uint256 => address) private _tokenApprovals;
+    mapping(uint256 tokenId => address) private _tokenApprovals;
 
-    // Mapping from owner to operator approvals
-    mapping(address => mapping(address => bool)) private _operatorApprovals;
+    mapping(address owner => mapping(address operator => bool)) private _operatorApprovals;
 
     /**
      * @dev Initializes the contract by setting a `name` and a `symbol` to the token collection.
@@ -113,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());
     }
 
     /**
@@ -131,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);
     }
 
     /**
@@ -152,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, "");
     }
 
@@ -170,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;
     }
 
     /**
@@ -269,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);
     }
 
     /**
@@ -310,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);
+        }
     }
 
     /**
@@ -343,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);
@@ -407,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);
@@ -441,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;
     }
 
     /**

+ 24 - 30
contracts/token/ERC721/extensions/ERC721Enumerable.sol

@@ -15,17 +15,11 @@ import {IERC165} from "../../../utils/introspection/ERC165.sol";
  * interfere with enumerability and should not be used together with `ERC721Enumerable`.
  */
 abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
-    // Mapping from owner to list of owned token IDs
-    mapping(address => mapping(uint256 => uint256)) private _ownedTokens;
+    mapping(address owner => mapping(uint256 index => uint256)) private _ownedTokens;
+    mapping(uint256 tokenId => uint256) private _ownedTokensIndex;
 
-    // Mapping from token ID to index of the owner tokens list
-    mapping(uint256 => uint256) private _ownedTokensIndex;
-
-    // Array with all token ids, used for enumeration
     uint256[] private _allTokens;
-
-    // Mapping from token id to position in the allTokens array
-    mapping(uint256 => uint256) private _allTokensIndex;
+    mapping(uint256 tokenId => uint256) private _allTokensIndex;
 
     /**
      * @dev An `owner`'s token query was out of bounds for `index`.
@@ -74,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;
     }
 
     /**
@@ -109,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;
     }
@@ -135,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
@@ -175,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;
     }
 }

+ 8 - 6
contracts/token/ERC721/extensions/ERC721URIStorage.sol

@@ -15,7 +15,7 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
     using Strings for uint256;
 
     // Optional mapping for token URIs
-    mapping(uint256 => string) private _tokenURIs;
+    mapping(uint256 tokenId => string) private _tokenURIs;
 
     /**
      * @dev See {IERC165-supportsInterface}
@@ -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

+ 1 - 1
contracts/token/common/ERC2981.sol

@@ -26,7 +26,7 @@ abstract contract ERC2981 is IERC2981, ERC165 {
     }
 
     RoyaltyInfo private _defaultRoyaltyInfo;
-    mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo;
+    mapping(uint256 tokenId => RoyaltyInfo) private _tokenRoyaltyInfo;
 
     /**
      * @dev The default royalty set is invalid (eg. (numerator / denominator) >= 1).

+ 1 - 1
contracts/utils/Nonces.sol

@@ -10,7 +10,7 @@ abstract contract Nonces {
      */
     error InvalidAccountNonce(address account, uint256 currentNonce);
 
-    mapping(address => uint256) private _nonces;
+    mapping(address account => uint256) private _nonces;
 
     /**
      * @dev Returns an the next unused nonce for an address.

+ 1 - 1
contracts/utils/structs/BitMaps.sol

@@ -17,7 +17,7 @@ pragma solidity ^0.8.20;
  */
 library BitMaps {
     struct BitMap {
-        mapping(uint256 => uint256) _data;
+        mapping(uint256 bucket => uint256) _data;
     }
 
     /**

+ 1 - 1
contracts/utils/structs/DoubleEndedQueue.sol

@@ -45,7 +45,7 @@ library DoubleEndedQueue {
     struct Bytes32Deque {
         uint128 _begin;
         uint128 _end;
-        mapping(uint128 => bytes32) _data;
+        mapping(uint128 index => bytes32) _data;
     }
 
     /**

+ 1 - 1
contracts/utils/structs/EnumerableMap.sol

@@ -65,7 +65,7 @@ library EnumerableMap {
     struct Bytes32ToBytes32Map {
         // Storage of keys
         EnumerableSet.Bytes32Set _keys;
-        mapping(bytes32 => bytes32) _values;
+        mapping(bytes32 key => bytes32) _values;
     }
 
     /**

+ 1 - 1
contracts/utils/structs/EnumerableSet.sol

@@ -53,7 +53,7 @@ library EnumerableSet {
         bytes32[] _values;
         // Position of the value in the `values` array, plus 1 because index 0
         // means a value is not in the set.
-        mapping(bytes32 => uint256) _indexes;
+        mapping(bytes32 value => uint256) _indexes;
     }
 
     /**

+ 15 - 1
docs/modules/ROOT/pages/index.adoc

@@ -11,11 +11,25 @@
 [[install]]
 === Installation
 
+==== Hardhat, Truffle (npm)
+
 ```console
 $ npm install @openzeppelin/contracts
 ```
 
-OpenZeppelin Contracts features a xref:releases-stability.adoc#api-stability[stable API], which means your contracts won't break unexpectedly when upgrading to a newer minor version.
+OpenZeppelin Contracts features a xref:releases-stability.adoc#api-stability[stable API], which means that your contracts won't break unexpectedly when upgrading to a newer minor version.
+
+==== Foundry (git)
+
+WARNING: When installing via git, it is a common error to use the `master` branch. This is a development branch that should be avoided in favor of tagged releases. The release process involves security measures that the `master` branch does not guarantee.
+
+WARNING: Foundry installs the latest version initially, but subsequent `forge update` commands will use the `master` branch.
+
+```console
+$ forge install OpenZeppelin/openzeppelin-contracts
+```
+
+Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` 
 
 [[usage]]
 === Usage

+ 1 - 1
remappings.txt

@@ -1 +1 @@
-openzeppelin/=contracts/
+@openzeppelin/contracts/=contracts/

+ 1 - 1
scripts/generate/templates/EnumerableMap.js

@@ -74,7 +74,7 @@ error EnumerableMapNonexistentKey(bytes32 key);
 struct Bytes32ToBytes32Map {
     // Storage of keys
     EnumerableSet.Bytes32Set _keys;
-    mapping(bytes32 => bytes32) _values;
+    mapping(bytes32 key => bytes32) _values;
 }
 
 /**

+ 1 - 1
scripts/generate/templates/EnumerableSet.js

@@ -63,7 +63,7 @@ struct Set {
     bytes32[] _values;
     // Position of the value in the \`values\` array, plus 1 because index 0
     // means a value is not in the set.
-    mapping(bytes32 => uint256) _indexes;
+    mapping(bytes32 value => uint256) _indexes;
 }
 
 /**

+ 2 - 2
test/governance/Governor.t.sol

@@ -3,8 +3,8 @@
 pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
-import {Strings} from "../../contracts/utils/Strings.sol";
-import {Governor} from "../../contracts/governance/Governor.sol";
+import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
+import {Governor} from "@openzeppelin/contracts/governance/Governor.sol";
 
 contract GovernorInternalTest is Test, Governor {
     constructor() Governor("") {}

+ 2 - 2
test/metatx/ERC2771Forwarder.t.sol

@@ -3,8 +3,8 @@
 pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
-import {ERC2771Forwarder} from "contracts/metatx/ERC2771Forwarder.sol";
-import {CallReceiverMockTrustingForwarder, CallReceiverMock} from "contracts/mocks/CallReceiverMock.sol";
+import {ERC2771Forwarder} from "@openzeppelin/contracts/metatx/ERC2771Forwarder.sol";
+import {CallReceiverMockTrustingForwarder, CallReceiverMock} from "@openzeppelin/contracts/mocks/CallReceiverMock.sol";
 
 struct ForwardRequest {
     address from;

+ 5 - 5
test/token/ERC20/extensions/ERC4626.t.sol

@@ -3,12 +3,12 @@ pragma solidity ^0.8.20;
 
 import {ERC4626Test} from "erc4626-tests/ERC4626.test.sol";
 
-import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol";
-import {ERC4626} from "openzeppelin/token/ERC20/extensions/ERC4626.sol";
+import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
 
-import {ERC20Mock} from "openzeppelin/mocks/token/ERC20Mock.sol";
-import {ERC4626Mock} from "openzeppelin/mocks/token/ERC4626Mock.sol";
-import {ERC4626OffsetMock} from "openzeppelin/mocks/token/ERC4626OffsetMock.sol";
+import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
+import {ERC4626Mock} from "@openzeppelin/contracts/mocks/token/ERC4626Mock.sol";
+import {ERC4626OffsetMock} from "@openzeppelin/contracts/mocks/token/ERC4626OffsetMock.sol";
 
 contract ERC4626VaultOffsetMock is ERC4626OffsetMock {
     constructor(

+ 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 - 2
test/token/ERC721/extensions/ERC721Consecutive.t.sol

@@ -4,8 +4,8 @@ pragma solidity ^0.8.20;
 
 // solhint-disable func-name-mixedcase
 
-import {ERC721} from "../../../../contracts/token/ERC721/ERC721.sol";
-import {ERC721Consecutive} from "../../../../contracts/token/ERC721/extensions/ERC721Consecutive.sol";
+import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
+import {ERC721Consecutive} from "@openzeppelin/contracts/token/ERC721/extensions/ERC721Consecutive.sol";
 import {Test, StdUtils} from "forge-std/Test.sol";
 
 function toSingleton(address account) pure returns (address[] memory) {

+ 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]);
     });
   });

+ 1 - 1
test/utils/ShortStrings.t.sol

@@ -4,7 +4,7 @@ pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
 
-import {ShortStrings, ShortString} from "../../contracts/utils/ShortStrings.sol";
+import {ShortStrings, ShortString} from "@openzeppelin/contracts/utils/ShortStrings.sol";
 
 contract ShortStringsTest is Test {
     string _fallback;

+ 1 - 1
test/utils/math/Math.t.sol

@@ -4,7 +4,7 @@ pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
 
-import {Math} from "../../../contracts/utils/math/Math.sol";
+import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
 
 contract MathTest is Test {
     // CEILDIV