From a193bd99de15057be85ea13988122ceea8584737 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 16:47:15 +0100 Subject: [PATCH 01/11] Move ERC721 and ERC1155 receiver checks to dedicate libraries --- contracts/token/ERC1155/ERC1155.sol | 72 +---------------- .../token/ERC1155/utils/ERC1155Utils.sol | 77 +++++++++++++++++++ contracts/token/ERC721/ERC721.sol | 36 +-------- contracts/token/ERC721/utils/ERC721Utils.sol | 39 ++++++++++ 4 files changed, 123 insertions(+), 101 deletions(-) create mode 100644 contracts/token/ERC1155/utils/ERC1155Utils.sol create mode 100644 contracts/token/ERC721/utils/ERC721Utils.sol diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 2ec3ef7be86..5e05e4791dd 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -4,8 +4,8 @@ pragma solidity ^0.8.20; import {IERC1155} from "./IERC1155.sol"; -import {IERC1155Receiver} from "./IERC1155Receiver.sol"; import {IERC1155MetadataURI} from "./extensions/IERC1155MetadataURI.sol"; +import {ERC1155Utils} from "./utils/ERC1155Utils.sol"; import {Context} from "../../utils/Context.sol"; import {IERC165, ERC165} from "../../utils/introspection/ERC165.sol"; import {Arrays} from "../../utils/Arrays.sol"; @@ -203,9 +203,9 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER if (ids.length == 1) { uint256 id = ids.unsafeMemoryAccess(0); uint256 value = values.unsafeMemoryAccess(0); - _doSafeTransferAcceptanceCheck(operator, from, to, id, value, data); + ERC1155Utils.checkOnERC1155Received(operator, from, to, id, value, data); } else { - _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, values, data); + ERC1155Utils.checkOnERC1155BatchReceived(operator, from, to, ids, values, data); } } } @@ -374,72 +374,6 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER emit ApprovalForAll(owner, operator, approved); } - /** - * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address - * if it contains code at the moment of execution. - */ - function _doSafeTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256 id, - uint256 value, - bytes memory data - ) private { - if (to.code.length > 0) { - try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { - if (response != IERC1155Receiver.onERC1155Received.selector) { - // Tokens rejected - revert ERC1155InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - // non-IERC1155Receiver implementer - revert ERC1155InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } - - /** - * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address - * if it contains code at the moment of execution. - */ - function _doSafeBatchTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory values, - bytes memory data - ) private { - if (to.code.length > 0) { - try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, values, data) returns ( - bytes4 response - ) { - if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { - // Tokens rejected - revert ERC1155InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - // non-IERC1155Receiver implementer - revert ERC1155InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } - /** * @dev Creates an array in memory with only one value for each of the elements provided. */ diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol new file mode 100644 index 00000000000..8af75fa1415 --- /dev/null +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC1155Receiver} from "../IERC1155Receiver.sol"; +import {IERC1155Errors} from "../../../interfaces/draft-IERC6093.sol"; + +/** + * @title ERC1155Utils + */ +library ERC1155Utils { + /** + * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address + * if it contains code at the moment of execution. + */ + function checkOnERC1155Received( + address operator, + address from, + address to, + uint256 id, + uint256 value, + bytes memory data + ) internal { + if (to.code.length > 0) { + try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { + if (response != IERC1155Receiver.onERC1155Received.selector) { + // Tokens rejected + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + // non-IERC1155Receiver implementer + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } + + /** + * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address + * if it contains code at the moment of execution. + */ + function checkOnERC1155BatchReceived( + address operator, + address from, + address to, + uint256[] memory ids, + uint256[] memory values, + bytes memory data + ) internal { + if (to.code.length > 0) { + try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, values, data) returns ( + bytes4 response + ) { + if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { + // Tokens rejected + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + // non-IERC1155Receiver implementer + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } +} diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 1b38f06814e..62ff1def969 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -4,8 +4,8 @@ pragma solidity ^0.8.20; import {IERC721} from "./IERC721.sol"; -import {IERC721Receiver} from "./IERC721Receiver.sol"; import {IERC721Metadata} from "./extensions/IERC721Metadata.sol"; +import {ERC721Utils} from "./utils/ERC721Utils.sol"; import {Context} from "../../utils/Context.sol"; import {Strings} from "../../utils/Strings.sol"; import {IERC165, ERC165} from "../../utils/introspection/ERC165.sol"; @@ -158,7 +158,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual { transferFrom(from, to, tokenId); - _checkOnERC721Received(from, to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data); } /** @@ -311,7 +311,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual { _mint(to, tokenId); - _checkOnERC721Received(address(0), to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), address(0), to, tokenId, data); } /** @@ -384,7 +384,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual { _transfer(from, to, tokenId); - _checkOnERC721Received(from, to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data); } /** @@ -452,32 +452,4 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er } return owner; } - - /** - * @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 - */ - function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes memory data) private { - if (to.code.length > 0) { - try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) { - if (retval != IERC721Receiver.onERC721Received.selector) { - revert ERC721InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - revert ERC721InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } } diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol new file mode 100644 index 00000000000..774b20d00f9 --- /dev/null +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC721Receiver} from "../IERC721Receiver.sol"; +import {IERC721Errors} from "../../../interfaces/draft-IERC6093.sol"; + +/** + * @title ERC721Utils + */ +library ERC721Utils { + /** + * @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 + */ + function checkOnERC721Received(address operator, address from, address to, uint256 tokenId, bytes memory data) internal { + if (to.code.length > 0) { + try IERC721Receiver(to).onERC721Received(operator, from, tokenId, data) returns (bytes4 retval) { + if (retval != IERC721Receiver.onERC721Received.selector) { + revert IERC721Errors.ERC721InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + revert IERC721Errors.ERC721InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } +} From 4eecf9f96fa71b0e9e8a86c0b4626583c1443ea4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 16:59:31 +0100 Subject: [PATCH 02/11] doc --- contracts/token/ERC1155/utils/ERC1155Utils.sol | 10 ++++++---- contracts/token/ERC721/utils/ERC721Utils.sol | 10 +++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 8af75fa1415..15d38e8c535 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -10,8 +10,9 @@ import {IERC1155Errors} from "../../../interfaces/draft-IERC6093.sol"; */ library ERC1155Utils { /** - * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address - * if it contains code at the moment of execution. + * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address if it + * contains code at the moment of execution. 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. */ function checkOnERC1155Received( address operator, @@ -42,8 +43,9 @@ library ERC1155Utils { } /** - * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address - * if it contains code at the moment of execution. + * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address if it + * contains code at the moment of execution. 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. */ function checkOnERC1155BatchReceived( address operator, diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index 774b20d00f9..3bfb1b290d2 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -10,13 +10,9 @@ import {IERC721Errors} from "../../../interfaces/draft-IERC6093.sol"; */ library ERC721Utils { /** - * @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 + * @dev Performs an acceptance check by calling {IERC721Receiver-onERC721Received} on the `to` address if it + * contains code at the moment of execution. 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. */ function checkOnERC721Received(address operator, address from, address to, uint256 tokenId, bytes memory data) internal { if (to.code.length > 0) { From f372e76739ffb1ae5778e1dccdb1b19ceccc7bd1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 17:00:33 +0100 Subject: [PATCH 03/11] doc --- contracts/token/ERC1155/utils/ERC1155Utils.sol | 3 ++- contracts/token/ERC721/utils/ERC721Utils.sol | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 15d38e8c535..6bdac5001b7 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -6,7 +6,8 @@ import {IERC1155Receiver} from "../IERC1155Receiver.sol"; import {IERC1155Errors} from "../../../interfaces/draft-IERC6093.sol"; /** - * @title ERC1155Utils + * @dev Implementation of the ERC-1155 acceptance checks used in safe transfers. + * See https://eips.ethereum.org/EIPS/eip-1155 */ library ERC1155Utils { /** diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index 3bfb1b290d2..9d0b776be7b 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -6,7 +6,8 @@ import {IERC721Receiver} from "../IERC721Receiver.sol"; import {IERC721Errors} from "../../../interfaces/draft-IERC6093.sol"; /** - * @title ERC721Utils + * @dev Implementation of the ERC-721 acceptance checks used in safe transfers. + * See https://eips.ethereum.org/EIPS/eip-721 */ library ERC721Utils { /** From 0e853c0d0cc9e8420cfae99eef4a4466bfdad37c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 17:00:47 +0100 Subject: [PATCH 04/11] doc --- .changeset/poor-chefs-cheat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/poor-chefs-cheat.md diff --git a/.changeset/poor-chefs-cheat.md b/.changeset/poor-chefs-cheat.md new file mode 100644 index 00000000000..ace9600ffba --- /dev/null +++ b/.changeset/poor-chefs-cheat.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC721Utils` and `ERC1155Utils`: reusable libraries for performing the acceptance checks (safeTransfer) From 9c5e8275835ed721fa9fc81aaec8071e8c6416b2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 17:04:12 +0100 Subject: [PATCH 05/11] up --- contracts/token/ERC1155/utils/ERC1155Utils.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 6bdac5001b7..947dd349194 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -26,12 +26,10 @@ library ERC1155Utils { if (to.code.length > 0) { try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { if (response != IERC1155Receiver.onERC1155Received.selector) { - // Tokens rejected revert IERC1155Errors.ERC1155InvalidReceiver(to); } } catch (bytes memory reason) { if (reason.length == 0) { - // non-IERC1155Receiver implementer revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { /// @solidity memory-safe-assembly @@ -61,12 +59,10 @@ library ERC1155Utils { bytes4 response ) { if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { - // Tokens rejected revert IERC1155Errors.ERC1155InvalidReceiver(to); } } catch (bytes memory reason) { if (reason.length == 0) { - // non-IERC1155Receiver implementer revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { /// @solidity memory-safe-assembly From dc2f8f41397adf55c3bb039524bb05d0e7d10947 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jan 2024 21:51:55 +0100 Subject: [PATCH 06/11] fix lint --- contracts/token/ERC721/utils/ERC721Utils.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index 9d0b776be7b..6be0aab640d 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -15,7 +15,13 @@ library ERC721Utils { * contains code at the moment of execution. 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. */ - function checkOnERC721Received(address operator, address from, address to, uint256 tokenId, bytes memory data) internal { + function checkOnERC721Received( + address operator, + address from, + address to, + uint256 tokenId, + bytes memory data + ) internal { if (to.code.length > 0) { try IERC721Receiver(to).onERC721Received(operator, from, tokenId, data) returns (bytes4 retval) { if (retval != IERC721Receiver.onERC721Received.selector) { From 29f3b16f438a55233ae45665601acbfe1a66ff93 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 30 Jan 2024 14:33:24 +0100 Subject: [PATCH 07/11] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto GarcĂ­a --- .changeset/poor-chefs-cheat.md | 2 +- contracts/token/ERC1155/utils/ERC1155Utils.sol | 4 ++++ contracts/token/ERC721/utils/ERC721Utils.sol | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.changeset/poor-chefs-cheat.md b/.changeset/poor-chefs-cheat.md index ace9600ffba..8f48d61642e 100644 --- a/.changeset/poor-chefs-cheat.md +++ b/.changeset/poor-chefs-cheat.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC721Utils` and `ERC1155Utils`: reusable libraries for performing the acceptance checks (safeTransfer) +`ERC721Utils` and `ERC1155Utils`: reusable libraries with functions to perform acceptance checks on `IERC721Receiver` and `IERC1155Receiver` implementers. diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 947dd349194..6bdac5001b7 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -26,10 +26,12 @@ library ERC1155Utils { if (to.code.length > 0) { try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { if (response != IERC1155Receiver.onERC1155Received.selector) { + // Tokens rejected revert IERC1155Errors.ERC1155InvalidReceiver(to); } } catch (bytes memory reason) { if (reason.length == 0) { + // non-IERC1155Receiver implementer revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { /// @solidity memory-safe-assembly @@ -59,10 +61,12 @@ library ERC1155Utils { bytes4 response ) { if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { + // Tokens rejected revert IERC1155Errors.ERC1155InvalidReceiver(to); } } catch (bytes memory reason) { if (reason.length == 0) { + // non-IERC1155Receiver implementer revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { /// @solidity memory-safe-assembly diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index 6be0aab640d..bc50b3c5b65 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -25,10 +25,12 @@ library ERC721Utils { if (to.code.length > 0) { try IERC721Receiver(to).onERC721Received(operator, from, tokenId, data) returns (bytes4 retval) { if (retval != IERC721Receiver.onERC721Received.selector) { + // Token rejected revert IERC721Errors.ERC721InvalidReceiver(to); } } catch (bytes memory reason) { if (reason.length == 0) { + // non-IERC721Receiver implementer revert IERC721Errors.ERC721InvalidReceiver(to); } else { /// @solidity memory-safe-assembly From d292276c8a570af9238ae6b25649d4c0ada49236 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 30 Jan 2024 11:08:07 -0600 Subject: [PATCH 08/11] Nits --- .changeset/poor-chefs-cheat.md | 2 +- .../token/ERC1155/utils/ERC1155Utils.sol | 23 ++++++++++++------- contracts/token/ERC721/utils/ERC721Utils.sol | 14 +++++++---- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/.changeset/poor-chefs-cheat.md b/.changeset/poor-chefs-cheat.md index 8f48d61642e..39db3d5139c 100644 --- a/.changeset/poor-chefs-cheat.md +++ b/.changeset/poor-chefs-cheat.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC721Utils` and `ERC1155Utils`: reusable libraries with functions to perform acceptance checks on `IERC721Receiver` and `IERC1155Receiver` implementers. +`ERC721Utils` and `ERC1155Utils`: Add reusable libraries with functions to perform acceptance checks on `IERC721Receiver` and `IERC1155Receiver` implementers. diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 6bdac5001b7..0ff2bf146a4 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -6,14 +6,18 @@ import {IERC1155Receiver} from "../IERC1155Receiver.sol"; import {IERC1155Errors} from "../../../interfaces/draft-IERC6093.sol"; /** - * @dev Implementation of the ERC-1155 acceptance checks used in safe transfers. - * See https://eips.ethereum.org/EIPS/eip-1155 + * @dev Library that provide common ERC-1155 utility functions. + * + * See https://eips.ethereum.org/EIPS/eip-1155[ERC-1155]. */ library ERC1155Utils { /** - * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address if it - * contains code at the moment of execution. 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. + * @dev Performs an acceptance check for the provided `operator` by calling {IERC1155-onERC1155Received} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept + * the transfer. */ function checkOnERC1155Received( address operator, @@ -44,9 +48,12 @@ library ERC1155Utils { } /** - * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address if it - * contains code at the moment of execution. 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. + * @dev Performs a batch acceptance check for the provided `operator` by calling {IERC1155-onERC1155BatchReceived} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept + * the transfer. */ function checkOnERC1155BatchReceived( address operator, diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index bc50b3c5b65..b923e403736 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -6,14 +6,18 @@ import {IERC721Receiver} from "../IERC721Receiver.sol"; import {IERC721Errors} from "../../../interfaces/draft-IERC6093.sol"; /** - * @dev Implementation of the ERC-721 acceptance checks used in safe transfers. - * See https://eips.ethereum.org/EIPS/eip-721 + * @dev Library that provide common ERC-721 utility functions. + * + * See https://eips.ethereum.org/EIPS/eip-721[ERC-721]. */ library ERC721Utils { /** - * @dev Performs an acceptance check by calling {IERC721Receiver-onERC721Received} on the `to` address if it - * contains code at the moment of execution. 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. + * @dev Performs an acceptance check for the provided `operator` by calling {IERC721-onERC721Received} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC721Receiver-onERC721Received} and return the acceptance magic value to accept + * the transfer. */ function checkOnERC721Received( address operator, From aaf7cdae40e0b8a1f4629848fae64fe4b09e73e3 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 30 Jan 2024 12:11:41 -0600 Subject: [PATCH 09/11] Add tests --- test/token/ERC1155/utils/ERC1155Utils.test.js | 299 ++++++++++++++++++ test/token/ERC721/utils/ERC721Utils.test.js | 93 ++++++ 2 files changed, 392 insertions(+) create mode 100644 test/token/ERC1155/utils/ERC1155Utils.test.js create mode 100644 test/token/ERC721/utils/ERC721Utils.test.js diff --git a/test/token/ERC1155/utils/ERC1155Utils.test.js b/test/token/ERC1155/utils/ERC1155Utils.test.js new file mode 100644 index 00000000000..5687568d341 --- /dev/null +++ b/test/token/ERC1155/utils/ERC1155Utils.test.js @@ -0,0 +1,299 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { RevertType } = require('../../../helpers/enums'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const firstTokenId = 1n; +const secondTokenId = 2n; +const firstTokenValue = 1000n; +const secondTokenValue = 1000n; + +const RECEIVER_SINGLE_MAGIC_VALUE = '0xf23a6e61'; +const RECEIVER_BATCH_MAGIC_VALUE = '0xbc197c81'; + +const deployReceiver = ( + revertType, + returnValueSingle = RECEIVER_SINGLE_MAGIC_VALUE, + returnValueBatched = RECEIVER_BATCH_MAGIC_VALUE, +) => ethers.deployContract('$ERC1155ReceiverMock', [returnValueSingle, returnValueBatched, revertType]); + +const fixture = async () => { + const [eoa, operator, owner] = await ethers.getSigners(); + const utils = await ethers.deployContract('$ERC1155Utils'); + + const receivers = { + correct: await deployReceiver(RevertType.None), + invalid: await deployReceiver(RevertType.None, '0xdeadbeef', '0xdeadbeef'), + message: await deployReceiver(RevertType.RevertWithMessage), + empty: await deployReceiver(RevertType.RevertWithoutMessage), + customError: await deployReceiver(RevertType.RevertWithCustomError), + panic: await deployReceiver(RevertType.Panic), + nonReceiver: await ethers.deployContract('CallReceiverMock'), + eoa, + }; + + return { operator, owner, utils, receivers }; +}; + +describe('ERC1155Utils', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('onERC1155Received', function () { + it('succeeds when called by an EOA', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.eoa, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.correct, + firstTokenId, + firstTokenValue, + data, + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.correct, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.invalid, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.message, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.be.revertedWith('ERC1155ReceiverMock: reverting on receive'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.empty, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.customError, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_SINGLE_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.panic, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC1155Received', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.nonReceiver, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); + + describe('onERC1155BatchReceived', function () { + it('succeeds when called by an EOA', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.eoa, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.correct, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + data, + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.correct, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.invalid, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.message, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.be.revertedWith('ERC1155ReceiverMock: reverting on batch receive'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.empty, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.customError, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_SINGLE_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.panic, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC1155BatchReceived', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.nonReceiver, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); +}); diff --git a/test/token/ERC721/utils/ERC721Utils.test.js b/test/token/ERC721/utils/ERC721Utils.test.js new file mode 100644 index 00000000000..2ebfb95e3ee --- /dev/null +++ b/test/token/ERC721/utils/ERC721Utils.test.js @@ -0,0 +1,93 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { RevertType } = require('../../../helpers/enums'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const tokenId = 1n; + +const RECEIVER_MAGIC_VALUE = '0x150b7a02'; + +const deployReceiver = (revertType, returnValue = RECEIVER_MAGIC_VALUE) => + ethers.deployContract('$ERC721ReceiverMock', [returnValue, revertType]); + +const fixture = async () => { + const [operator, owner] = await ethers.getSigners(); + const utils = await ethers.deployContract('$ERC721Utils'); + + const receivers = { + correct: await deployReceiver(RevertType.None), + invalid: await deployReceiver(RevertType.None, '0xdeadbeef'), + message: await deployReceiver(RevertType.RevertWithMessage), + empty: await deployReceiver(RevertType.RevertWithoutMessage), + customError: await deployReceiver(RevertType.RevertWithCustomError), + panic: await deployReceiver(RevertType.Panic), + nonReceiver: await ethers.deployContract('CallReceiverMock'), + }; + + return { operator, owner, utils, receivers }; +}; + +describe('ERC721Utils', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('onERC721Received', function () { + it('succeeds when called by an EOA', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.eoa, tokenId, '0x')).to + .not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.correct, tokenId, data)) + .to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.correct, tokenId, '0x')) + .to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.invalid, tokenId, '0x')) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.message, tokenId, '0x'), + ).to.be.revertedWith('ERC721ReceiverMock: reverting'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.empty, tokenId, '0x')) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.customError, tokenId, '0x'), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.panic, tokenId, '0x'), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC721Received', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.nonReceiver, tokenId, '0x'), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); +}); From d551d5c2c12c9c701cb10f25b169606004af81f8 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 30 Jan 2024 12:16:52 -0600 Subject: [PATCH 10/11] Fix tests --- test/token/ERC721/utils/ERC721Utils.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/token/ERC721/utils/ERC721Utils.test.js b/test/token/ERC721/utils/ERC721Utils.test.js index 2ebfb95e3ee..e90c18e4f26 100644 --- a/test/token/ERC721/utils/ERC721Utils.test.js +++ b/test/token/ERC721/utils/ERC721Utils.test.js @@ -12,7 +12,7 @@ const deployReceiver = (revertType, returnValue = RECEIVER_MAGIC_VALUE) => ethers.deployContract('$ERC721ReceiverMock', [returnValue, revertType]); const fixture = async () => { - const [operator, owner] = await ethers.getSigners(); + const [eoa, operator, owner] = await ethers.getSigners(); const utils = await ethers.deployContract('$ERC721Utils'); const receivers = { @@ -23,12 +23,13 @@ const fixture = async () => { customError: await deployReceiver(RevertType.RevertWithCustomError), panic: await deployReceiver(RevertType.Panic), nonReceiver: await ethers.deployContract('CallReceiverMock'), + eoa, }; return { operator, owner, utils, receivers }; }; -describe('ERC721Utils', function () { +describe.only('ERC721Utils', function () { beforeEach(async function () { Object.assign(this, await loadFixture(fixture)); }); From ed00c2ac8c943fd4c8114a8b52ff05093b6a5682 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 30 Jan 2024 12:22:25 -0600 Subject: [PATCH 11/11] Remove unnecessary .only --- test/token/ERC721/utils/ERC721Utils.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC721/utils/ERC721Utils.test.js b/test/token/ERC721/utils/ERC721Utils.test.js index e90c18e4f26..2327d1ac7da 100644 --- a/test/token/ERC721/utils/ERC721Utils.test.js +++ b/test/token/ERC721/utils/ERC721Utils.test.js @@ -29,7 +29,7 @@ const fixture = async () => { return { operator, owner, utils, receivers }; }; -describe.only('ERC721Utils', function () { +describe('ERC721Utils', function () { beforeEach(async function () { Object.assign(this, await loadFixture(fixture)); });