diff --git a/CHANGELOG.md b/CHANGELOG.md index 288cda55b..a31fb22c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ * Removed presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/). * `TransparentUpgradeableProxy`: Removed `admin` and `implementation` getters, which were only callable by the proxy owner and thus not very useful. ([#3820](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3820)) * `ProxyAdmin`: Removed `getProxyAdmin` and `getProxyImplementation` getters. ([#3820](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3820)) - * `ERC20`: 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)) + * `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)) * `ERC165Storage`: Removed this contract in favor of inheritance based approach. ([#3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880)) ### How to upgrade from 4.x diff --git a/contracts/mocks/ERC1155PausableMock.sol b/contracts/mocks/ERC1155PausableMock.sol index cd068234f..501ed3d1f 100644 --- a/contracts/mocks/ERC1155PausableMock.sol +++ b/contracts/mocks/ERC1155PausableMock.sol @@ -16,14 +16,13 @@ contract ERC1155PausableMock is ERC1155Mock, ERC1155Pausable { _unpause(); } - function _beforeTokenTransfer( - address operator, + function _update( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) internal override(ERC1155, ERC1155Pausable) { - super._beforeTokenTransfer(operator, from, to, ids, amounts, data); + super._update(from, to, ids, amounts, data); } } diff --git a/contracts/mocks/ERC1155SupplyMock.sol b/contracts/mocks/ERC1155SupplyMock.sol index 9c0cd7b63..a7421f045 100644 --- a/contracts/mocks/ERC1155SupplyMock.sol +++ b/contracts/mocks/ERC1155SupplyMock.sol @@ -8,14 +8,13 @@ import "../token/ERC1155/extensions/ERC1155Supply.sol"; contract ERC1155SupplyMock is ERC1155Mock, ERC1155Supply { constructor(string memory uri) ERC1155Mock(uri) {} - function _beforeTokenTransfer( - address operator, + function _update( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) internal override(ERC1155, ERC1155Supply) { - super._beforeTokenTransfer(operator, from, to, ids, amounts, data); + super._update(from, to, ids, amounts, data); } } diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index b20b711d5..e491a2310 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -143,44 +143,76 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { } /** - * @dev Transfers `amount` tokens of token type `id` from `from` to `to`. + * @dev Transfers `amount` tokens of token type `id` from `from` to `to`. Will mint (or burn) if `from` (or `to`) is the zero address. * - * Emits a {TransferSingle} event. + * Emits a {TransferSingle} event if the arrays contain one element, and {TransferBatch} otherwise. * * Requirements: * - * - `to` cannot be the zero address. - * - `from` must have a balance of tokens of type `id` of at least `amount`. - * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the - * acceptance magic value. + * - If `to` refers to a smart contract, it must implement either {IERC1155Receiver-onERC1155Received} + * or {IERC1155Receiver-onERC1155BatchReceived} and return the acceptance magic value. */ - function _safeTransferFrom( + function _update( address from, address to, - uint256 id, - uint256 amount, + uint256[] memory ids, + uint256[] memory amounts, bytes memory data ) internal virtual { - require(to != address(0), "ERC1155: transfer to the zero address"); + require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); address operator = _msgSender(); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); - _beforeTokenTransfer(operator, from, to, ids, amounts, data); + for (uint256 i = 0; i < ids.length; ++i) { + uint256 id = ids[i]; + uint256 amount = amounts[i]; - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - _balances[id][to] += amount; + if (from != address(0)) { + uint256 fromBalance = _balances[id][from]; + require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); + unchecked { + _balances[id][from] = fromBalance - amount; + } + } - emit TransferSingle(operator, from, to, id, amount); + if (to != address(0)) { + _balances[id][to] += amount; + } + } - _afterTokenTransfer(operator, from, to, ids, amounts, data); + if (ids.length == 1) { + uint256 id = ids[0]; + uint256 amount = amounts[0]; + emit TransferSingle(operator, from, to, id, amount); + if (to != address(0)) { + _doSafeTransferAcceptanceCheck(operator, from, to, id, amount, data); + } + } else { + emit TransferBatch(operator, from, to, ids, amounts); + if (to != address(0)) { + _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, amounts, data); + } + } + } - _doSafeTransferAcceptanceCheck(operator, from, to, id, amount, data); + /** + * @dev Transfers `amount` tokens of token type `id` from `from` to `to`. + * + * Emits a {TransferSingle} event. + * + * Requirements: + * + * - `to` cannot be the zero address. + * - `from` must have a balance of tokens of type `id` of at least `amount`. + * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the + * acceptance magic value. + */ + function _safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes memory data) internal { + require(to != address(0), "ERC1155: transfer to the zero address"); + require(from != address(0), "ERC1155: transfer from the zero address"); + uint256[] memory ids = _asSingletonArray(id); + uint256[] memory amounts = _asSingletonArray(amount); + _update(from, to, ids, amounts, data); } /** @@ -199,31 +231,10 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256[] memory ids, uint256[] memory amounts, bytes memory data - ) internal virtual { - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); + ) internal { require(to != address(0), "ERC1155: transfer to the zero address"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, from, to, ids, amounts, data); - - for (uint256 i = 0; i < ids.length; ++i) { - uint256 id = ids[i]; - uint256 amount = amounts[i]; - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - _balances[id][to] += amount; - } - - emit TransferBatch(operator, from, to, ids, amounts); - - _afterTokenTransfer(operator, from, to, ids, amounts, data); - - _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, amounts, data); + require(from != address(0), "ERC1155: transfer from the zero address"); + _update(from, to, ids, amounts, data); } /** @@ -260,21 +271,11 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the * acceptance magic value. */ - function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal virtual { + function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: mint to the zero address"); - - address operator = _msgSender(); uint256[] memory ids = _asSingletonArray(id); uint256[] memory amounts = _asSingletonArray(amount); - - _beforeTokenTransfer(operator, address(0), to, ids, amounts, data); - - _balances[id][to] += amount; - emit TransferSingle(operator, address(0), to, id, amount); - - _afterTokenTransfer(operator, address(0), to, ids, amounts, data); - - _doSafeTransferAcceptanceCheck(operator, address(0), to, id, amount, data); + _update(address(0), to, ids, amounts, data); } /** @@ -288,28 +289,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155BatchReceived} and return the * acceptance magic value. */ - function _mintBatch( - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual { + function _mintBatch(address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data) internal { require(to != address(0), "ERC1155: mint to the zero address"); - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, address(0), to, ids, amounts, data); - - for (uint256 i = 0; i < ids.length; i++) { - _balances[ids[i]][to] += amounts[i]; - } - - emit TransferBatch(operator, address(0), to, ids, amounts); - - _afterTokenTransfer(operator, address(0), to, ids, amounts, data); - - _doSafeBatchTransferAcceptanceCheck(operator, address(0), to, ids, amounts, data); + _update(address(0), to, ids, amounts, data); } /** @@ -322,24 +304,11 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * - `from` cannot be the zero address. * - `from` must have at least `amount` tokens of token type `id`. */ - function _burn(address from, uint256 id, uint256 amount) internal virtual { + function _burn(address from, uint256 id, uint256 amount) internal { require(from != address(0), "ERC1155: burn from the zero address"); - - address operator = _msgSender(); uint256[] memory ids = _asSingletonArray(id); uint256[] memory amounts = _asSingletonArray(amount); - - _beforeTokenTransfer(operator, from, address(0), ids, amounts, ""); - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: burn amount exceeds balance"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - - emit TransferSingle(operator, from, address(0), id, amount); - - _afterTokenTransfer(operator, from, address(0), ids, amounts, ""); + _update(from, address(0), ids, amounts, ""); } /** @@ -351,28 +320,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * * - `ids` and `amounts` must have the same length. */ - function _burnBatch(address from, uint256[] memory ids, uint256[] memory amounts) internal virtual { + function _burnBatch(address from, uint256[] memory ids, uint256[] memory amounts) internal { require(from != address(0), "ERC1155: burn from the zero address"); - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, from, address(0), ids, amounts, ""); - - for (uint256 i = 0; i < ids.length; i++) { - uint256 id = ids[i]; - uint256 amount = amounts[i]; - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: burn amount exceeds balance"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - } - - emit TransferBatch(operator, from, address(0), ids, amounts); - - _afterTokenTransfer(operator, from, address(0), ids, amounts, ""); + _update(from, address(0), ids, amounts, ""); } /** @@ -386,64 +336,6 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { emit ApprovalForAll(owner, operator, approved); } - /** - * @dev Hook that is called before any token transfer. This includes minting - * and burning, as well as batched variants. - * - * The same hook is called on both single and batched variants. For single - * transfers, the length of the `ids` and `amounts` arrays will be 1. - * - * Calling conditions (for each `id` and `amount` pair): - * - * - When `from` and `to` are both non-zero, `amount` of ``from``'s tokens - * of token type `id` will be transferred to `to`. - * - When `from` is zero, `amount` tokens of token type `id` will be minted - * for `to`. - * - when `to` is zero, `amount` of ``from``'s tokens of token type `id` - * will be burned. - * - `from` and `to` are never both zero. - * - `ids` and `amounts` have the same, non-zero length. - * - * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. - */ - function _beforeTokenTransfer( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual {} - - /** - * @dev Hook that is called after any token transfer. This includes minting - * and burning, as well as batched variants. - * - * The same hook is called on both single and batched variants. For single - * transfers, the length of the `id` and `amount` arrays will be 1. - * - * Calling conditions (for each `id` and `amount` pair): - * - * - When `from` and `to` are both non-zero, `amount` of ``from``'s tokens - * of token type `id` will be transferred to `to`. - * - When `from` is zero, `amount` tokens of token type `id` will be minted - * for `to`. - * - when `to` is zero, `amount` of ``from``'s tokens of token type `id` - * will be burned. - * - `from` and `to` are never both zero. - * - `ids` and `amounts` have the same, non-zero length. - * - * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. - */ - function _afterTokenTransfer( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual {} - function _doSafeTransferAcceptanceCheck( address operator, address from, diff --git a/contracts/token/ERC1155/extensions/ERC1155Pausable.sol b/contracts/token/ERC1155/extensions/ERC1155Pausable.sol index 64790e2aa..f37d90e25 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Pausable.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Pausable.sol @@ -17,22 +17,20 @@ import "../../../security/Pausable.sol"; */ abstract contract ERC1155Pausable is ERC1155, Pausable { /** - * @dev See {ERC1155-_beforeTokenTransfer}. + * @dev See {ERC1155-_update}. * * Requirements: * * - the contract must not be paused. */ - function _beforeTokenTransfer( - address operator, + function _update( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) internal virtual override { - super._beforeTokenTransfer(operator, from, to, ids, amounts, data); - require(!paused(), "ERC1155Pausable: token transfer while paused"); + super._update(from, to, ids, amounts, data); } } diff --git a/contracts/token/ERC1155/extensions/ERC1155Supply.sol b/contracts/token/ERC1155/extensions/ERC1155Supply.sol index ec24389a1..77690b59d 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Supply.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Supply.sol @@ -31,18 +31,15 @@ abstract contract ERC1155Supply is ERC1155 { } /** - * @dev See {ERC1155-_beforeTokenTransfer}. + * @dev See {ERC1155-_update}. */ - function _beforeTokenTransfer( - address operator, + function _update( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) internal virtual override { - super._beforeTokenTransfer(operator, from, to, ids, amounts, data); - if (from == address(0)) { for (uint256 i = 0; i < ids.length; ++i) { _totalSupply[ids[i]] += amounts[i]; @@ -60,5 +57,6 @@ abstract contract ERC1155Supply is ERC1155 { } } } + super._update(from, to, ids, amounts, data); } } diff --git a/test/token/ERC1155/ERC1155.test.js b/test/token/ERC1155/ERC1155.test.js index a0a8cf3ff..2f145de5d 100644 --- a/test/token/ERC1155/ERC1155.test.js +++ b/test/token/ERC1155/ERC1155.test.js @@ -120,7 +120,7 @@ contract('ERC1155', function (accounts) { it('reverts when burning a non-existent token id', async function () { await expectRevert( this.token.burn(tokenHolder, tokenId, mintAmount), - 'ERC1155: burn amount exceeds balance', + 'ERC1155: insufficient balance for transfer', ); }); @@ -135,7 +135,7 @@ contract('ERC1155', function (accounts) { await expectRevert( this.token.burn(tokenHolder, tokenId, mintAmount.addn(1)), - 'ERC1155: burn amount exceeds balance', + 'ERC1155: insufficient balance for transfer', ); }); @@ -192,7 +192,7 @@ contract('ERC1155', function (accounts) { it('reverts when burning a non-existent token id', async function () { await expectRevert( this.token.burnBatch(tokenBatchHolder, tokenBatchIds, burnAmounts), - 'ERC1155: burn amount exceeds balance', + 'ERC1155: insufficient balance for transfer', ); });