From 7a1a8ae3289947d0e319a6863a8e611584d47811 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 23 Aug 2021 10:43:27 +0200 Subject: [PATCH 1/4] Add an internal _setApprovalForAll function (721 & 1155) --- contracts/token/ERC1155/ERC1155.sol | 15 ++++++++++++--- contracts/token/ERC721/ERC721.sol | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 634f389345d..1b41106784a 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -101,9 +101,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function setApprovalForAll(address operator, bool approved) public virtual override { require(_msgSender() != operator, "ERC1155: setting approval status for self"); - - _operatorApprovals[_msgSender()][operator] = approved; - emit ApprovalForAll(_msgSender(), operator, approved); + _setApprovalForAll(_msgSender(), operator, approved); } /** @@ -369,6 +367,17 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { emit TransferBatch(operator, account, address(0), ids, amounts); } + /** + * @dev Approve `operator` to operate on all of `owner` tokens + * + * Emits a {ApprovalForAll} event. + */ + function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { + // TODO: add sanity checks? + _operatorApprovals[owner][operator] = approved; + emit ApprovalForAll(owner, operator, approved); + } + /** * @dev Hook that is called before any token transfer. This includes minting * and burning, as well as batched variants. diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 0b37218ec25..50d16baed69 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -134,9 +134,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { */ function setApprovalForAll(address operator, bool approved) public virtual override { require(operator != _msgSender(), "ERC721: approve to caller"); - - _operatorApprovals[_msgSender()][operator] = approved; - emit ApprovalForAll(_msgSender(), operator, approved); + _setApprovalForAll(_msgSender(), operator, approved); } /** @@ -356,6 +354,17 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { emit Approval(ERC721.ownerOf(tokenId), to, tokenId); } + /** + * @dev Approve `operator` to operate on all of `owner` tokens + * + * Emits a {ApprovalForAll} event. + */ + function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { + // TODO: add sanity checks? + _operatorApprovals[owner][operator] = approved; + emit ApprovalForAll(owner, operator, approved); + } + /** * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address. * The call is not executed if the target address is not a contract. From a79e3dc586dcade9b36d02fa9d209f9c1323b555 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 24 Aug 2021 12:04:21 +0200 Subject: [PATCH 2/4] fix lint --- contracts/token/ERC1155/ERC1155.sol | 6 +++++- contracts/token/ERC721/ERC721.sol | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 1b41106784a..8cb557dc744 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -372,7 +372,11 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * * Emits a {ApprovalForAll} event. */ - function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { + function _setApprovalForAll( + address owner, + address operator, + bool approved + ) internal virtual { // TODO: add sanity checks? _operatorApprovals[owner][operator] = approved; emit ApprovalForAll(owner, operator, approved); diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 50d16baed69..6a389b6e655 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -359,7 +359,11 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * * Emits a {ApprovalForAll} event. */ - function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { + function _setApprovalForAll( + address owner, + address operator, + bool approved + ) internal virtual { // TODO: add sanity checks? _operatorApprovals[owner][operator] = approved; emit ApprovalForAll(owner, operator, approved); From 36b65e76036bb1068992782400e50b48a45e9c2b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 09:05:58 +0200 Subject: [PATCH 3/4] move checks to internal functions --- contracts/token/ERC1155/ERC1155.sol | 3 +-- contracts/token/ERC721/ERC721.sol | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 8cb557dc744..31caeea4380 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -100,7 +100,6 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * @dev See {IERC1155-setApprovalForAll}. */ function setApprovalForAll(address operator, bool approved) public virtual override { - require(_msgSender() != operator, "ERC1155: setting approval status for self"); _setApprovalForAll(_msgSender(), operator, approved); } @@ -377,7 +376,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { address operator, bool approved ) internal virtual { - // TODO: add sanity checks? + require(owner != operator, "ERC1155: setting approval status for self"); _operatorApprovals[owner][operator] = approved; emit ApprovalForAll(owner, operator, approved); } diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 6a389b6e655..5e3bbd9a463 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -133,7 +133,6 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * @dev See {IERC721-setApprovalForAll}. */ function setApprovalForAll(address operator, bool approved) public virtual override { - require(operator != _msgSender(), "ERC721: approve to caller"); _setApprovalForAll(_msgSender(), operator, approved); } @@ -364,7 +363,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { address operator, bool approved ) internal virtual { - // TODO: add sanity checks? + require(owner != operator, "ERC721: approve to caller"); _operatorApprovals[owner][operator] = approved; emit ApprovalForAll(owner, operator, approved); } From 50ee0703b79dabeb36a40796938cd168f10c31e0 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 09:06:06 +0200 Subject: [PATCH 4/4] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53b3d458026..bef768da31f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + + * Add internal `_setApprovalForAll` to `ERC721` and `ERC1155`. ([#2834](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2834)) + ## 4.3.0 (2021-08-17) * `ERC2771Context`: use private variable from storage to store the forwarder address. Fixes issues where `_msgSender()` was not callable from constructors. ([#2754](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2754))