From bb709e96e80b0f7458ea7ab8881253c01dd95066 Mon Sep 17 00:00:00 2001 From: Vittorio Minacori Date: Thu, 21 Sep 2023 16:10:22 +0200 Subject: [PATCH] feat: check transfer or approval reverts instead of returning boolean --- analysis/description-table/ERC1363.md | 6 +-- analysis/uml/ERC1363.svg | 56 +++++++++++++-------------- contracts/token/ERC1363/ERC1363.sol | 33 ++++++---------- flat/ERC1363.flat.sol | 33 ++++++---------- package-lock.json | 4 +- package.json | 2 +- 6 files changed, 56 insertions(+), 78 deletions(-) diff --git a/analysis/description-table/ERC1363.md b/analysis/description-table/ERC1363.md index 645d6d8..825dbdb 100644 --- a/analysis/description-table/ERC1363.md +++ b/analysis/description-table/ERC1363.md @@ -5,7 +5,7 @@ | File Name | SHA-1 Hash | |-------------|--------------| -| flat/ERC1363.flat.sol | 9cbaa5967d433176696921607014870e19a253d0 | +| flat/ERC1363.flat.sol | ca8de52513b8ac954c3a7d0cd4a4aafb6bf4331e | ### Contracts Description Table @@ -81,8 +81,8 @@ | └ | transferFromAndCall | Public ❗️ | 🛑 |NO❗️ | | └ | approveAndCall | Public ❗️ | 🛑 |NO❗️ | | └ | approveAndCall | Public ❗️ | 🛑 |NO❗️ | -| └ | _checkOnTransferReceived | Internal 🔒 | 🛑 | | -| └ | _checkOnApprovalReceived | Internal 🔒 | 🛑 | | +| └ | _checkOnTransferReceived | Private 🔐 | 🛑 | | +| └ | _checkOnApprovalReceived | Private 🔐 | 🛑 | | ### Legend diff --git a/analysis/uml/ERC1363.svg b/analysis/uml/ERC1363.svg index c35ad5e..d5af0c8 100644 --- a/analysis/uml/ERC1363.svg +++ b/analysis/uml/ERC1363.svg @@ -4,11 +4,11 @@ - + UmlClassDiagram - + 0 @@ -165,41 +165,41 @@ 7 - -<<Interface>> -IERC1363Receiver - -External: -     onTransferReceived(spender: address, sender: address, amount: uint256, data: bytes): bytes4 + +<<Interface>> +IERC1363Receiver + +External: +     onTransferReceived(spender: address, sender: address, amount: uint256, data: bytes): bytes4 8 - -<<Interface>> -IERC1363Spender - -External: -     onApprovalReceived(sender: address, amount: uint256, data: bytes): bytes4 + +<<Interface>> +IERC1363Spender + +External: +     onApprovalReceived(sender: address, amount: uint256, data: bytes): bytes4 9 - + <<Abstract>> ERC1363 - -Internal: -    _checkOnTransferReceived(sender: address, recipient: address, amount: uint256, data: bytes): bool -    _checkOnApprovalReceived(spender: address, amount: uint256, data: bytes): bool -Public: -    supportsInterface(interfaceId: bytes4): bool -    transferAndCall(to: address, amount: uint256): bool -    transferAndCall(to: address, amount: uint256, data: bytes): bool -    transferFromAndCall(from: address, to: address, amount: uint256): bool -    transferFromAndCall(from: address, to: address, amount: uint256, data: bytes): bool -    approveAndCall(spender: address, amount: uint256): bool -    approveAndCall(spender: address, amount: uint256, data: bytes): bool + +Private: +    _checkOnTransferReceived(sender: address, recipient: address, amount: uint256, data: bytes) +    _checkOnApprovalReceived(spender: address, amount: uint256, data: bytes) +Public: +    supportsInterface(interfaceId: bytes4): bool +    transferAndCall(to: address, amount: uint256): bool +    transferAndCall(to: address, amount: uint256, data: bytes): bool +    transferFromAndCall(from: address, to: address, amount: uint256): bool +    transferFromAndCall(from: address, to: address, amount: uint256, data: bytes): bool +    approveAndCall(spender: address, amount: uint256): bool +    approveAndCall(spender: address, amount: uint256, data: bytes): bool diff --git a/contracts/token/ERC1363/ERC1363.sol b/contracts/token/ERC1363/ERC1363.sol index e0f78c6..43bf643 100644 --- a/contracts/token/ERC1363/ERC1363.sol +++ b/contracts/token/ERC1363/ERC1363.sol @@ -41,7 +41,7 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { */ function transferAndCall(address to, uint256 amount, bytes memory data) public virtual override returns (bool) { transfer(to, amount); - require(_checkOnTransferReceived(_msgSender(), to, amount, data), "ERC1363: receiver returned wrong data"); + _checkOnTransferReceived(_msgSender(), to, amount, data); return true; } @@ -71,7 +71,7 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { bytes memory data ) public virtual override returns (bool) { transferFrom(from, to, amount); - require(_checkOnTransferReceived(from, to, amount, data), "ERC1363: receiver returned wrong data"); + _checkOnTransferReceived(from, to, amount, data); return true; } @@ -94,31 +94,25 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { */ function approveAndCall(address spender, uint256 amount, bytes memory data) public virtual override returns (bool) { approve(spender, amount); - require(_checkOnApprovalReceived(spender, amount, data), "ERC1363: spender returned wrong data"); + _checkOnApprovalReceived(spender, amount, data); return true; } /** - * @dev Internal function to invoke {IERC1363Receiver-onTransferReceived} on a target address. - * The call is not executed if the target address is not a contract. + * @dev Private function to invoke {IERC1363Receiver-onTransferReceived} on a target address. + * This will revert if the recipient doesn't accept the token transfer or if the target address is not a contract. * @param sender address Representing the previous owner of the given token amount * @param recipient address Target address that will receive the tokens * @param amount uint256 The amount mount of tokens to be transferred * @param data bytes Optional data to send along with the call - * @return whether the call correctly returned the expected magic value */ - function _checkOnTransferReceived( - address sender, - address recipient, - uint256 amount, - bytes memory data - ) internal virtual returns (bool) { + function _checkOnTransferReceived(address sender, address recipient, uint256 amount, bytes memory data) private { if (recipient.code.length == 0) { revert("ERC1363: transfer to non contract address"); } try IERC1363Receiver(recipient).onTransferReceived(_msgSender(), sender, amount, data) returns (bytes4 retval) { - return retval == IERC1363Receiver.onTransferReceived.selector; + require(retval == IERC1363Receiver.onTransferReceived.selector, "ERC1363: receiver returned wrong data"); } catch (bytes memory reason) { if (reason.length == 0) { revert("ERC1363: transfer to non ERC1363Receiver implementer"); @@ -132,24 +126,19 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { } /** - * @dev Internal function to invoke {IERC1363Receiver-onApprovalReceived} on a target address. - * The call is not executed if the target address is not a contract. + * @dev Private function to invoke {IERC1363Receiver-onApprovalReceived} on a target address. + * This will revert if the recipient doesn't accept the token approval or if the target address is not a contract. * @param spender address The address which will spend the funds * @param amount uint256 The amount of tokens to be spent * @param data bytes Optional data to send along with the call - * @return whether the call correctly returned the expected magic value */ - function _checkOnApprovalReceived( - address spender, - uint256 amount, - bytes memory data - ) internal virtual returns (bool) { + function _checkOnApprovalReceived(address spender, uint256 amount, bytes memory data) private { if (spender.code.length == 0) { revert("ERC1363: approve a non contract address"); } try IERC1363Spender(spender).onApprovalReceived(_msgSender(), amount, data) returns (bytes4 retval) { - return retval == IERC1363Spender.onApprovalReceived.selector; + require(retval == IERC1363Spender.onApprovalReceived.selector, "ERC1363: spender returned wrong data"); } catch (bytes memory reason) { if (reason.length == 0) { revert("ERC1363: approve a non ERC1363Spender implementer"); diff --git a/flat/ERC1363.flat.sol b/flat/ERC1363.flat.sol index 3a6c968..c2fb3e4 100644 --- a/flat/ERC1363.flat.sol +++ b/flat/ERC1363.flat.sol @@ -755,7 +755,7 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { */ function transferAndCall(address to, uint256 amount, bytes memory data) public virtual override returns (bool) { transfer(to, amount); - require(_checkOnTransferReceived(_msgSender(), to, amount, data), "ERC1363: receiver returned wrong data"); + _checkOnTransferReceived(_msgSender(), to, amount, data); return true; } @@ -785,7 +785,7 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { bytes memory data ) public virtual override returns (bool) { transferFrom(from, to, amount); - require(_checkOnTransferReceived(from, to, amount, data), "ERC1363: receiver returned wrong data"); + _checkOnTransferReceived(from, to, amount, data); return true; } @@ -808,31 +808,25 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { */ function approveAndCall(address spender, uint256 amount, bytes memory data) public virtual override returns (bool) { approve(spender, amount); - require(_checkOnApprovalReceived(spender, amount, data), "ERC1363: spender returned wrong data"); + _checkOnApprovalReceived(spender, amount, data); return true; } /** - * @dev Internal function to invoke {IERC1363Receiver-onTransferReceived} on a target address. - * The call is not executed if the target address is not a contract. + * @dev Private function to invoke {IERC1363Receiver-onTransferReceived} on a target address. + * This will revert if the recipient doesn't accept the token transfer or if the target address is not a contract. * @param sender address Representing the previous owner of the given token amount * @param recipient address Target address that will receive the tokens * @param amount uint256 The amount mount of tokens to be transferred * @param data bytes Optional data to send along with the call - * @return whether the call correctly returned the expected magic value */ - function _checkOnTransferReceived( - address sender, - address recipient, - uint256 amount, - bytes memory data - ) internal virtual returns (bool) { + function _checkOnTransferReceived(address sender, address recipient, uint256 amount, bytes memory data) private { if (recipient.code.length == 0) { revert("ERC1363: transfer to non contract address"); } try IERC1363Receiver(recipient).onTransferReceived(_msgSender(), sender, amount, data) returns (bytes4 retval) { - return retval == IERC1363Receiver.onTransferReceived.selector; + require(retval == IERC1363Receiver.onTransferReceived.selector, "ERC1363: receiver returned wrong data"); } catch (bytes memory reason) { if (reason.length == 0) { revert("ERC1363: transfer to non ERC1363Receiver implementer"); @@ -846,24 +840,19 @@ abstract contract ERC1363 is ERC20, IERC1363, ERC165 { } /** - * @dev Internal function to invoke {IERC1363Receiver-onApprovalReceived} on a target address. - * The call is not executed if the target address is not a contract. + * @dev Private function to invoke {IERC1363Receiver-onApprovalReceived} on a target address. + * This will revert if the recipient doesn't accept the token approval or if the target address is not a contract. * @param spender address The address which will spend the funds * @param amount uint256 The amount of tokens to be spent * @param data bytes Optional data to send along with the call - * @return whether the call correctly returned the expected magic value */ - function _checkOnApprovalReceived( - address spender, - uint256 amount, - bytes memory data - ) internal virtual returns (bool) { + function _checkOnApprovalReceived(address spender, uint256 amount, bytes memory data) private { if (spender.code.length == 0) { revert("ERC1363: approve a non contract address"); } try IERC1363Spender(spender).onApprovalReceived(_msgSender(), amount, data) returns (bytes4 retval) { - return retval == IERC1363Spender.onApprovalReceived.selector; + require(retval == IERC1363Spender.onApprovalReceived.selector, "ERC1363: spender returned wrong data"); } catch (bytes memory reason) { if (reason.length == 0) { revert("ERC1363: approve a non ERC1363Spender implementer"); diff --git a/package-lock.json b/package-lock.json index a212068..24182b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "erc-payable-token", - "version": "4.9.6", + "version": "4.9.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "erc-payable-token", - "version": "4.9.6", + "version": "4.9.7", "license": "MIT", "dependencies": { "@openzeppelin/contracts": "4.9.3" diff --git a/package.json b/package.json index 46f3412..e2c669d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "erc-payable-token", - "version": "4.9.6", + "version": "4.9.7", "description": "ERC-1363 Payable Token Implementation", "files": [ "contracts",