diff --git a/analysis/control-flow/ERC1363.png b/analysis/control-flow/ERC1363.png index b02ad5c..30ea82c 100644 Binary files a/analysis/control-flow/ERC1363.png and b/analysis/control-flow/ERC1363.png differ diff --git a/contracts/mocks/ERC1363ReceiverMock.sol b/contracts/mocks/ERC1363ReceiverMock.sol index e8c6695..2568507 100644 --- a/contracts/mocks/ERC1363ReceiverMock.sol +++ b/contracts/mocks/ERC1363ReceiverMock.sol @@ -15,19 +15,19 @@ contract ERC1363ReceiverMock is IERC1363Receiver { } bytes4 private _retval; - RevertType private _error; + RevertType private _err; event Received(address operator, address from, uint256 value, bytes data); error CustomError(bytes4); constructor() { _retval = IERC1363Receiver.onTransferReceived.selector; - _error = RevertType.None; + _err = RevertType.None; } - function setUp(bytes4 retval, RevertType error) public { + function setUp(bytes4 retval, RevertType err) public { _retval = retval; - _error = error; + _err = err; } function onTransferReceived( @@ -36,13 +36,13 @@ contract ERC1363ReceiverMock is IERC1363Receiver { uint256 value, bytes calldata data ) external override returns (bytes4) { - if (_error == RevertType.RevertWithoutMessage) { + if (_err == RevertType.RevertWithoutMessage) { revert(); - } else if (_error == RevertType.RevertWithMessage) { + } else if (_err == RevertType.RevertWithMessage) { revert("ERC1363ReceiverMock: reverting"); - } else if (_error == RevertType.RevertWithCustomError) { + } else if (_err == RevertType.RevertWithCustomError) { revert CustomError(_retval); - } else if (_error == RevertType.Panic) { + } else if (_err == RevertType.Panic) { uint256 a = uint256(0) / uint256(0); a; } diff --git a/contracts/mocks/ERC1363ReturnFalseOnERC20Mock.sol b/contracts/mocks/ERC1363ReturnFalseOnERC20Mock.sol new file mode 100644 index 0000000..4298fe5 --- /dev/null +++ b/contracts/mocks/ERC1363ReturnFalseOnERC20Mock.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC20, ERC20, ERC1363} from "../token/ERC1363/ERC1363.sol"; + +// mock class testing an ERC-20 token that returns false +abstract contract ERC1363ReturnFalseOnERC20Mock is ERC1363 { + function transfer(address, uint256) public pure override(IERC20, ERC20) returns (bool) { + return false; + } + + function transferFrom(address, address, uint256) public pure override(IERC20, ERC20) returns (bool) { + return false; + } + + function approve(address, uint256) public pure override(IERC20, ERC20) returns (bool) { + return false; + } +} diff --git a/contracts/mocks/ERC1363SpenderMock.sol b/contracts/mocks/ERC1363SpenderMock.sol index f3bf437..2243091 100644 --- a/contracts/mocks/ERC1363SpenderMock.sol +++ b/contracts/mocks/ERC1363SpenderMock.sol @@ -15,29 +15,29 @@ contract ERC1363SpenderMock is IERC1363Spender { } bytes4 private _retval; - RevertType private _error; + RevertType private _err; event Approved(address owner, uint256 value, bytes data); error CustomError(bytes4); constructor() { _retval = IERC1363Spender.onApprovalReceived.selector; - _error = RevertType.None; + _err = RevertType.None; } - function setUp(bytes4 retval, RevertType error) public { + function setUp(bytes4 retval, RevertType err) public { _retval = retval; - _error = error; + _err = err; } function onApprovalReceived(address owner, uint256 value, bytes calldata data) external override returns (bytes4) { - if (_error == RevertType.RevertWithoutMessage) { + if (_err == RevertType.RevertWithoutMessage) { revert(); - } else if (_error == RevertType.RevertWithMessage) { + } else if (_err == RevertType.RevertWithMessage) { revert("ERC1363SpenderMock: reverting"); - } else if (_error == RevertType.RevertWithCustomError) { + } else if (_err == RevertType.RevertWithCustomError) { revert CustomError(_retval); - } else if (_error == RevertType.Panic) { + } else if (_err == RevertType.Panic) { uint256 a = uint256(0) / uint256(0); a; } diff --git a/contracts/token/ERC1363/ERC1363.sol b/contracts/token/ERC1363/ERC1363.sol index 2ebd546..59b49f8 100644 --- a/contracts/token/ERC1363/ERC1363.sol +++ b/contracts/token/ERC1363/ERC1363.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.20; -import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {IERC20, ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {IERC165, ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import {IERC1363} from "./IERC1363.sol"; @@ -35,7 +35,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { * @inheritdoc IERC1363 */ function transferAndCall(address to, uint256 value, bytes memory data) public virtual returns (bool) { - transfer(to, value); + if (!transfer(to, value)) { + revert ERC1363TransferFailed(to, value); + } _checkOnTransferReceived(_msgSender(), to, value, data); return true; } @@ -56,7 +58,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { uint256 value, bytes memory data ) public virtual returns (bool) { - transferFrom(from, to, value); + if (!transferFrom(from, to, value)) { + revert ERC1363TransferFromFailed(from, to, value); + } _checkOnTransferReceived(from, to, value, data); return true; } @@ -72,7 +76,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { * @inheritdoc IERC1363 */ function approveAndCall(address spender, uint256 value, bytes memory data) public virtual returns (bool) { - approve(spender, value); + if (!approve(spender, value)) { + revert ERC1363ApproveFailed(spender, value); + } _checkOnApprovalReceived(spender, value, data); return true; } diff --git a/contracts/token/ERC1363/IERC1363Errors.sol b/contracts/token/ERC1363/IERC1363Errors.sol index 9ca1171..09a7e69 100644 --- a/contracts/token/ERC1363/IERC1363Errors.sol +++ b/contracts/token/ERC1363/IERC1363Errors.sol @@ -30,4 +30,26 @@ interface IERC1363Errors { * @param spender The address which will spend the funds. */ error ERC1363InvalidSpender(address spender); + + /** + * @dev Indicates a failure with the ERC-20 `transfer` during a `transferAndCall` operation. Used in transfers. + * @param receiver The address to which tokens are being transferred. + * @param value The amount of tokens to be transferred. + */ + error ERC1363TransferFailed(address receiver, uint256 value); + + /** + * @dev Indicates a failure with the ERC-20 `transferFrom` during a `transferFromAndCall` operation. Used in transfers. + * @param sender The address from which to send tokens. + * @param receiver The address to which tokens are being transferred. + * @param value The amount of tokens to be transferred. + */ + error ERC1363TransferFromFailed(address sender, address receiver, uint256 value); + + /** + * @dev Indicates a failure with the ERC-20 `approve` during a `approveAndCall` operation. Used in approvals. + * @param spender The address which will spend the funds. + * @param value The amount of tokens to be spent. + */ + error ERC1363ApproveFailed(address spender, uint256 value); } diff --git a/dist/ERC1363.dist.sol b/dist/ERC1363.dist.sol index 782987d..9977e21 100644 --- a/dist/ERC1363.dist.sol +++ b/dist/ERC1363.dist.sol @@ -802,6 +802,28 @@ interface IERC1363Errors { * @param spender The address which will spend the funds. */ error ERC1363InvalidSpender(address spender); + + /** + * @dev Indicates a failure with the ERC-20 `transfer` during a `transferAndCall` operation. Used in transfers. + * @param receiver The address to which tokens are being transferred. + * @param value The amount of tokens to be transferred. + */ + error ERC1363TransferFailed(address receiver, uint256 value); + + /** + * @dev Indicates a failure with the ERC-20 `transferFrom` during a `transferFromAndCall` operation. Used in transfers. + * @param sender The address from which to send tokens. + * @param receiver The address to which tokens are being transferred. + * @param value The amount of tokens to be transferred. + */ + error ERC1363TransferFromFailed(address sender, address receiver, uint256 value); + + /** + * @dev Indicates a failure with the ERC-20 `approve` during a `approveAndCall` operation. Used in approvals. + * @param spender The address which will spend the funds. + * @param value The amount of tokens to be spent. + */ + error ERC1363ApproveFailed(address spender, uint256 value); } @@ -900,7 +922,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { * @inheritdoc IERC1363 */ function transferAndCall(address to, uint256 value, bytes memory data) public virtual returns (bool) { - transfer(to, value); + if (!transfer(to, value)) { + revert ERC1363TransferFailed(to, value); + } _checkOnTransferReceived(_msgSender(), to, value, data); return true; } @@ -921,7 +945,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { uint256 value, bytes memory data ) public virtual returns (bool) { - transferFrom(from, to, value); + if (!transferFrom(from, to, value)) { + revert ERC1363TransferFromFailed(from, to, value); + } _checkOnTransferReceived(from, to, value, data); return true; } @@ -937,7 +963,9 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363, IERC1363Errors { * @inheritdoc IERC1363 */ function approveAndCall(address spender, uint256 value, bytes memory data) public virtual returns (bool) { - approve(spender, value); + if (!approve(spender, value)) { + revert ERC1363ApproveFailed(spender, value); + } _checkOnApprovalReceived(spender, value, data); return true; } diff --git a/docs/index.md b/docs/index.md index f69ab01..923f88e 100644 --- a/docs/index.md +++ b/docs/index.md @@ -347,6 +347,52 @@ _Indicates a failure with the token `spender`. Used in approvals._ | ---- | ---- | ----------- | | spender | address | The address which will spend the funds. | +### ERC1363TransferFailed + +```solidity +error ERC1363TransferFailed(address receiver, uint256 value) +``` + +_Indicates a failure with the ERC-20 `transfer` during a `transferAndCall` operation. Used in transfers._ + +#### Parameters + +| Name | Type | Description | +| ---- | ---- | ----------- | +| receiver | address | The address to which tokens are being transferred. | +| value | uint256 | The amount of tokens to be transferred. | + +### ERC1363TransferFromFailed + +```solidity +error ERC1363TransferFromFailed(address sender, address receiver, uint256 value) +``` + +_Indicates a failure with the ERC-20 `transferFrom` during a `transferFromAndCall` operation. Used in transfers._ + +#### Parameters + +| Name | Type | Description | +| ---- | ---- | ----------- | +| sender | address | The address from which to send tokens. | +| receiver | address | The address to which tokens are being transferred. | +| value | uint256 | The amount of tokens to be transferred. | + +### ERC1363ApproveFailed + +```solidity +error ERC1363ApproveFailed(address spender, uint256 value) +``` + +_Indicates a failure with the ERC-20 `approve` during a `approveAndCall` operation. Used in approvals._ + +#### Parameters + +| Name | Type | Description | +| ---- | ---- | ----------- | +| spender | address | The address which will spend the funds. | +| value | uint256 | The amount of tokens to be spent. | + ## IERC1363Receiver _Interface for any contract that wants to support `transferAndCall` or `transferFromAndCall` from ERC-1363 token contracts._ diff --git a/package-lock.json b/package-lock.json index cfb1d3c..13a2695 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "erc-payable-token", - "version": "5.1.6", + "version": "5.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "erc-payable-token", - "version": "5.1.6", + "version": "5.2.0", "license": "MIT", "dependencies": { "@openzeppelin/contracts": "5.0.1" diff --git a/package.json b/package.json index b144725..72a7ca1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "erc-payable-token", - "version": "5.1.7", + "version": "5.2.0", "description": "ERC-1363 Payable Token Implementation", "files": [ "contracts", diff --git a/test/helpers/enums.js b/test/helpers/enums.js index 9ec1136..a654f1d 100644 --- a/test/helpers/enums.js +++ b/test/helpers/enums.js @@ -3,6 +3,5 @@ function Enum(...options) { } module.exports = { - Enum, RevertType: Enum('None', 'RevertWithoutMessage', 'RevertWithMessage', 'RevertWithCustomError', 'Panic'), }; diff --git a/test/mocks/ERC1363ReturnFalseOnERC20.js b/test/mocks/ERC1363ReturnFalseOnERC20.js new file mode 100644 index 0000000..9fb66a4 --- /dev/null +++ b/test/mocks/ERC1363ReturnFalseOnERC20.js @@ -0,0 +1,120 @@ +const { BN } = require('@openzeppelin/test-helpers'); + +const { expectRevertCustomError } = require('../helpers/customError'); + +const ERC1363ReturnFalseOnERC20 = artifacts.require('$ERC1363ReturnFalseOnERC20Mock'); + +contract('ERC1363ReturnFalseOnERC20', function (accounts) { + const [owner, spender, recipient] = accounts; + + const name = 'My Token'; + const symbol = 'MTKN'; + const initialSupply = new BN(100); + + const data = '0x42'; + + beforeEach(async function () { + this.token = await ERC1363ReturnFalseOnERC20.new(name, symbol); + await this.token.$_mint(owner, initialSupply); + }); + + context('when ERC20 methods return false', function () { + describe('transfers', function () { + describe('via transferAndCall', function () { + const transferAndCallWithData = function (to, value, opts) { + return this.token.methods['transferAndCall(address,uint256,bytes)'](to, value, data, opts); + }; + + const transferAndCallWithoutData = function (to, value, opts) { + return this.token.methods['transferAndCall(address,uint256)'](to, value, opts); + }; + + describe('with data', function () { + it('reverts', async function () { + await expectRevertCustomError( + transferAndCallWithData.call(this, recipient, initialSupply, { from: owner }), + 'ERC1363TransferFailed', + [recipient, initialSupply], + ); + }); + }); + + describe('without data', function () { + it('reverts', async function () { + await expectRevertCustomError( + transferAndCallWithoutData.call(this, recipient, initialSupply, { from: owner }), + 'ERC1363TransferFailed', + [recipient, initialSupply], + ); + }); + }); + }); + + describe('via transferFromAndCall', function () { + beforeEach(async function () { + await this.token.approve(spender, initialSupply, { from: owner }); + }); + + const transferFromAndCallWithData = function (from, to, value, opts) { + return this.token.methods['transferFromAndCall(address,address,uint256,bytes)'](from, to, value, data, opts); + }; + + const transferFromAndCallWithoutData = function (from, to, value, opts) { + return this.token.methods['transferFromAndCall(address,address,uint256)'](from, to, value, opts); + }; + + describe('with data', function () { + it('reverts', async function () { + await expectRevertCustomError( + transferFromAndCallWithData.call(this, owner, recipient, initialSupply, { from: spender }), + 'ERC1363TransferFromFailed', + [owner, recipient, initialSupply], + ); + }); + }); + + describe('without data', function () { + it('reverts', async function () { + await expectRevertCustomError( + transferFromAndCallWithoutData.call(this, owner, recipient, initialSupply, { from: spender }), + 'ERC1363TransferFromFailed', + [owner, recipient, initialSupply], + ); + }); + }); + }); + }); + + describe('approvals', function () { + describe('via approveAndCall', function () { + const approveAndCallWithData = function (spender, value, opts) { + return this.token.methods['approveAndCall(address,uint256,bytes)'](spender, value, data, opts); + }; + + const approveAndCallWithoutData = function (spender, value, opts) { + return this.token.methods['approveAndCall(address,uint256)'](spender, value, opts); + }; + + describe('with data', function () { + it('reverts', async function () { + await expectRevertCustomError( + approveAndCallWithData.call(this, recipient, initialSupply, { from: owner }), + 'ERC1363ApproveFailed', + [recipient, initialSupply], + ); + }); + }); + + describe('without data', function () { + it('reverts', async function () { + await expectRevertCustomError( + approveAndCallWithoutData.call(this, recipient, initialSupply, { from: owner }), + 'ERC1363ApproveFailed', + [recipient, initialSupply], + ); + }); + }); + }); + }); + }); +});