From 0616203ebeb2c09f0df0ba55f0dcb8ec2be14f36 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Sun, 12 Dec 2021 21:25:19 +0100 Subject: [PATCH 01/15] erc1363 --- contracts/interfaces/IERC1363.sol | 13 +- contracts/mocks/ERC1363Mock.sol | 54 ++++++++ contracts/token/ERC20/extensions/ERC1363.sol | 63 ++++++++++ test/token/ERC20/extensions/ERC1363.test.js | 116 ++++++++++++++++++ .../SupportsInterface.behavior.js | 8 ++ 5 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 contracts/mocks/ERC1363Mock.sol create mode 100644 contracts/token/ERC20/extensions/ERC1363.sol create mode 100644 test/token/ERC20/extensions/ERC1363.test.js diff --git a/contracts/interfaces/IERC1363.sol b/contracts/interfaces/IERC1363.sol index f18c7a4fac1..51295e7534f 100644 --- a/contracts/interfaces/IERC1363.sol +++ b/contracts/interfaces/IERC1363.sol @@ -6,19 +6,14 @@ pragma solidity ^0.8.0; import "./IERC20.sol"; import "./IERC165.sol"; -interface IERC1363 is IERC165, IERC20 { +interface IERC1363 is IERC20, IERC165 { /* - * Note: the ERC-165 identifier for this interface is 0x4bbee2df. - * 0x4bbee2df === + * Note: the ERC-165 identifier for this interface is 0xb0202a11. + * 0xb0202a11 === * bytes4(keccak256('transferAndCall(address,uint256)')) ^ * bytes4(keccak256('transferAndCall(address,uint256,bytes)')) ^ * bytes4(keccak256('transferFromAndCall(address,address,uint256)')) ^ - * bytes4(keccak256('transferFromAndCall(address,address,uint256,bytes)')) - */ - - /* - * Note: the ERC-165 identifier for this interface is 0xfb9ec8ce. - * 0xfb9ec8ce === + * bytes4(keccak256('transferFromAndCall(address,address,uint256,bytes)')) ^ * bytes4(keccak256('approveAndCall(address,uint256)')) ^ * bytes4(keccak256('approveAndCall(address,uint256,bytes)')) */ diff --git a/contracts/mocks/ERC1363Mock.sol b/contracts/mocks/ERC1363Mock.sol new file mode 100644 index 00000000000..497155345b6 --- /dev/null +++ b/contracts/mocks/ERC1363Mock.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../token/ERC20/extensions/ERC1363.sol"; + +contract ERC1363Mock is ERC1363 { + constructor( + string memory name, + string memory symbol, + address initialAccount, + uint256 initialBalance + ) payable ERC20(name, symbol) { + _mint(initialAccount, initialBalance); + } + + function mint(address account, uint256 amount) public { + _mint(account, amount); + } + + function burn(address account, uint256 amount) public { + _burn(account, amount); + } +} + +contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { + event TransferReceived(address operator, address from, uint256 value, bytes data); + event ApprovalReceived(address owner, uint256 value, bytes data); + + function onTransferReceived( + address operator, + address from, + uint256 value, + bytes memory data + ) external override returns (bytes4) { + if (data.length == 1) { + require(data[0] == 0x00, "onTransferReceived revert"); + } + emit TransferReceived(operator, from, value, data); + return this.onTransferReceived.selector; + } + + function onApprovalReceived( + address owner, + uint256 value, + bytes memory data + ) external override returns (bytes4) { + if (data.length == 1) { + require(data[0] == 0x00, "onApprovalReceived revert"); + } + emit ApprovalReceived(owner, value, data); + return this.onApprovalReceived.selector; + } +} \ No newline at end of file diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol new file mode 100644 index 00000000000..29d475605c0 --- /dev/null +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "../ERC20.sol"; +import "../../../interfaces/IERC1363.sol"; +import "../../../interfaces/IERC1363Receiver.sol"; +import "../../../interfaces/IERC1363Spender.sol"; +import "../../../utils/introspection/ERC165.sol"; + + +abstract contract ERC1363 is IERC1363, ERC20, ERC165 { + function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { + return interfaceId == type(IERC1363).interfaceId || super.supportsInterface(interfaceId); + } + + function transferAndCall(address to, uint256 value) public override returns (bool) { + return transferAndCall(to, value, bytes("")); + } + + function transferAndCall(address to, uint256 value, bytes memory data) public override returns (bool) { + require(transfer(to, value)); + try IERC1363Receiver(to).onTransferReceived(_msgSender(), _msgSender(), value, data) returns (bytes4 selector) { + require(selector == IERC1363Receiver(to).onTransferReceived.selector, "ERC1363: onTransferReceived invalid result"); + } catch Error(string memory reason) { + revert(reason); + } catch { + revert("ERC1363: onTransferReceived reverted without reason"); + } + return true; + } + + function transferFromAndCall(address from, address to, uint256 value) public override returns (bool) { + return transferFromAndCall(from, to, value, bytes("")); + } + + function transferFromAndCall(address from, address to, uint256 value, bytes memory data) public override returns (bool) { + require(transferFrom(from, to, value)); + try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 selector) { + require(selector == IERC1363Receiver(to).onTransferReceived.selector, "ERC1363: onTransferReceived invalid result"); + } catch Error(string memory reason) { + revert(reason); + } catch { + revert("ERC1363: onTransferReceived reverted without reason"); + } + return true; + } + + function approveAndCall(address spender, uint256 value) public override returns (bool) { + return approveAndCall(spender, value, bytes("")); + } + + function approveAndCall(address spender, uint256 value, bytes memory data) public override returns (bool) { + require(approve(spender, value)); + try IERC1363Spender(spender).onApprovalReceived(_msgSender(), value, data) returns (bytes4 selector) { + require(selector == IERC1363Spender(spender).onApprovalReceived.selector, "ERC1363: onApprovalReceived invalid result"); + } catch Error(string memory reason) { + revert(reason); + } catch { + revert("ERC1363: onApprovalReceived reverted without reason"); + } + return true; + } +} diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js new file mode 100644 index 00000000000..34616de14a9 --- /dev/null +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -0,0 +1,116 @@ +const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); + +const ERC1363Mock = artifacts.require('ERC1363Mock'); +const ERC1363ReceiverMock = artifacts.require('ERC1363ReceiverMock'); + +contract('ERC1363', function (accounts) { + const [ holder, operator ] = accounts; + + const initialSupply = new BN(100); + const value = new BN(10); + + const name = 'My Token'; + const symbol = 'MTKN'; + + beforeEach(async function () { + this.token = await ERC1363Mock.new(name, symbol, holder, initialSupply); + this.receiver = await ERC1363ReceiverMock.new(); + }); + + shouldSupportInterfaces([ + 'ERC165', + 'ERC1363', + ]); + + describe('transferAndCall', function () { + it('without data', async function () { + const data = null; + + const { tx } = await this.token.methods['transferAndCall(address,uint256)'](this.receiver.address, value, { from: holder }); + + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator: holder, from: holder, value, data }); + }); + + it('with data', async function () { + const data = '0x123456'; + + const { tx } = await this.token.methods['transferAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }); + + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator: holder, from: holder, value, data }); + }); + + it('with reverting hook', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['transferAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }), + 'onTransferReceived revert', + ); + }); + }); + + describe('transferFromAndCall', function () { + beforeEach(async function () { + await this.token.approve(operator, initialSupply, { from: holder }); + }); + + it('without data', async function () { + const data = null; + + const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256)'](holder, this.receiver.address, value, { from: operator }); + + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator, from: holder, value, data }); + }); + + it('with data', async function () { + const data = '0x123456'; + + const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256,bytes)'](holder, this.receiver.address, value, data, { from: operator }); + + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator, from: holder, value, data }); + }); + + it('with reverting hook', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['transferFromAndCall(address,address,uint256,bytes)'](holder, this.receiver.address, value, data, { from: operator }), + 'onTransferReceived revert', + ); + }); + }); + + describe('approveAndCall', function () { + it('without data', async function () { + const data = null; + + const { tx } = await this.token.methods['approveAndCall(address,uint256)'](this.receiver.address, value, { from: holder }); + + await expectEvent.inTransaction(tx, this.token, 'Approval', { owner: holder, spender: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { owner: holder, value, data }); + }); + + it('with data', async function () { + const data = '0x123456'; + + const { tx } = await this.token.methods['approveAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }); + + await expectEvent.inTransaction(tx, this.token, 'Approval', { owner: holder, spender: this.receiver.address, value }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { owner: holder, value, data }); + }); + + it('with reverting hook', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['approveAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }), + 'onApprovalReceived revert', + ); + }); + }); +}); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index 2027b4057d0..a426c714f0d 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -39,6 +39,14 @@ const INTERFACES = { 'onERC1155Received(address,address,uint256,uint256,bytes)', 'onERC1155BatchReceived(address,address,uint256[],uint256[],bytes)', ], + ERC1363: [ + 'transferAndCall(address,uint256)', + 'transferAndCall(address,uint256,bytes)', + 'transferFromAndCall(address,address,uint256)', + 'transferFromAndCall(address,address,uint256,bytes)', + 'approveAndCall(address,uint256)', + 'approveAndCall(address,uint256,bytes)', + ], AccessControl: [ 'hasRole(bytes32,address)', 'getRoleAdmin(bytes32)', From 786fa7ce2d99f2ccdbc8acfb8d645f6aac6f59c9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Sun, 12 Dec 2021 23:19:10 +0100 Subject: [PATCH 02/15] fix lint --- contracts/mocks/ERC1363Mock.sol | 2 +- contracts/token/ERC20/extensions/ERC1363.sol | 41 ++++-- test/token/ERC20/extensions/ERC1363.test.js | 141 ++++++++++++++++--- 3 files changed, 153 insertions(+), 31 deletions(-) diff --git a/contracts/mocks/ERC1363Mock.sol b/contracts/mocks/ERC1363Mock.sol index 497155345b6..289227d86e3 100644 --- a/contracts/mocks/ERC1363Mock.sol +++ b/contracts/mocks/ERC1363Mock.sol @@ -51,4 +51,4 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { emit ApprovalReceived(owner, value, data); return this.onApprovalReceived.selector; } -} \ No newline at end of file +} diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol index 29d475605c0..3678a995505 100644 --- a/contracts/token/ERC20/extensions/ERC1363.sol +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -7,7 +7,6 @@ import "../../../interfaces/IERC1363Receiver.sol"; import "../../../interfaces/IERC1363Spender.sol"; import "../../../utils/introspection/ERC165.sol"; - abstract contract ERC1363 is IERC1363, ERC20, ERC165 { function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { return interfaceId == type(IERC1363).interfaceId || super.supportsInterface(interfaceId); @@ -17,10 +16,17 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { return transferAndCall(to, value, bytes("")); } - function transferAndCall(address to, uint256 value, bytes memory data) public override returns (bool) { + function transferAndCall( + address to, + uint256 value, + bytes memory data + ) public override returns (bool) { require(transfer(to, value)); try IERC1363Receiver(to).onTransferReceived(_msgSender(), _msgSender(), value, data) returns (bytes4 selector) { - require(selector == IERC1363Receiver(to).onTransferReceived.selector, "ERC1363: onTransferReceived invalid result"); + require( + selector == IERC1363Receiver(to).onTransferReceived.selector, + "ERC1363: onTransferReceived invalid result" + ); } catch Error(string memory reason) { revert(reason); } catch { @@ -29,14 +35,26 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { return true; } - function transferFromAndCall(address from, address to, uint256 value) public override returns (bool) { + function transferFromAndCall( + address from, + address to, + uint256 value + ) public override returns (bool) { return transferFromAndCall(from, to, value, bytes("")); } - function transferFromAndCall(address from, address to, uint256 value, bytes memory data) public override returns (bool) { + function transferFromAndCall( + address from, + address to, + uint256 value, + bytes memory data + ) public override returns (bool) { require(transferFrom(from, to, value)); try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 selector) { - require(selector == IERC1363Receiver(to).onTransferReceived.selector, "ERC1363: onTransferReceived invalid result"); + require( + selector == IERC1363Receiver(to).onTransferReceived.selector, + "ERC1363: onTransferReceived invalid result" + ); } catch Error(string memory reason) { revert(reason); } catch { @@ -49,10 +67,17 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { return approveAndCall(spender, value, bytes("")); } - function approveAndCall(address spender, uint256 value, bytes memory data) public override returns (bool) { + function approveAndCall( + address spender, + uint256 value, + bytes memory data + ) public override returns (bool) { require(approve(spender, value)); try IERC1363Spender(spender).onApprovalReceived(_msgSender(), value, data) returns (bytes4 selector) { - require(selector == IERC1363Spender(spender).onApprovalReceived.selector, "ERC1363: onApprovalReceived invalid result"); + require( + selector == IERC1363Spender(spender).onApprovalReceived.selector, + "ERC1363: onApprovalReceived invalid result" + ); } catch Error(string memory reason) { revert(reason); } catch { diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 34616de14a9..64f29f83fff 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -14,7 +14,7 @@ contract('ERC1363', function (accounts) { const symbol = 'MTKN'; beforeEach(async function () { - this.token = await ERC1363Mock.new(name, symbol, holder, initialSupply); + this.token = await ERC1363Mock.new(name, symbol, holder, initialSupply); this.receiver = await ERC1363ReceiverMock.new(); }); @@ -27,26 +27,58 @@ contract('ERC1363', function (accounts) { it('without data', async function () { const data = null; - const { tx } = await this.token.methods['transferAndCall(address,uint256)'](this.receiver.address, value, { from: holder }); + const { tx } = await this.token.methods['transferAndCall(address,uint256)']( + this.receiver.address, + value, + { from: holder }, + ); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator: holder, from: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: holder, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: holder, + from: holder, + value, + data, + }); }); it('with data', async function () { const data = '0x123456'; - const { tx } = await this.token.methods['transferAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }); + const { tx } = await this.token.methods['transferAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator: holder, from: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: holder, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: holder, + from: holder, + value, + data, + }); }); it('with reverting hook', async function () { const data = '0x01'; await expectRevert( - this.token.methods['transferAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }), + this.token.methods['transferAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), 'onTransferReceived revert', ); }); @@ -60,26 +92,61 @@ contract('ERC1363', function (accounts) { it('without data', async function () { const data = null; - const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256)'](holder, this.receiver.address, value, { from: operator }); + const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256)']( + holder, + this.receiver.address, + value, + { from: operator }, + ); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator, from: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: holder, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator, + from: holder, + value, + data, + }); }); it('with data', async function () { const data = '0x123456'; - const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256,bytes)'](holder, this.receiver.address, value, data, { from: operator }); + const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( + holder, + this.receiver.address, + value, + data, + { from: operator }, + ); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: holder, to: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { operator, from: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: holder, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator, + from: holder, + value, + data, + }); }); it('with reverting hook', async function () { const data = '0x01'; await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256,bytes)'](holder, this.receiver.address, value, data, { from: operator }), + this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( + holder, + this.receiver.address, + value, + data, + { from: operator }, + ), 'onTransferReceived revert', ); }); @@ -89,26 +156,56 @@ contract('ERC1363', function (accounts) { it('without data', async function () { const data = null; - const { tx } = await this.token.methods['approveAndCall(address,uint256)'](this.receiver.address, value, { from: holder }); + const { tx } = await this.token.methods['approveAndCall(address,uint256)']( + this.receiver.address, + value, + { from: holder }, + ); - await expectEvent.inTransaction(tx, this.token, 'Approval', { owner: holder, spender: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { owner: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Approval', { + owner: holder, + spender: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { + owner: holder, + value, + data, + }); }); it('with data', async function () { const data = '0x123456'; - const { tx } = await this.token.methods['approveAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }); + const { tx } = await this.token.methods['approveAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ); - await expectEvent.inTransaction(tx, this.token, 'Approval', { owner: holder, spender: this.receiver.address, value }); - await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { owner: holder, value, data }); + await expectEvent.inTransaction(tx, this.token, 'Approval', { + owner: holder, + spender: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { + owner: holder, + value, + data, + }); }); it('with reverting hook', async function () { const data = '0x01'; await expectRevert( - this.token.methods['approveAndCall(address,uint256,bytes)'](this.receiver.address, value, data, { from: holder }), + this.token.methods['approveAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), 'onApprovalReceived revert', ); }); From 553002bf86a5aeaf8913b2d3171ec285a1dbe609 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 13 Dec 2021 17:51:36 +0100 Subject: [PATCH 03/15] improve coverage --- contracts/mocks/ERC1363Mock.sol | 8 +- test/token/ERC20/extensions/ERC1363.test.js | 166 +++++++++++++++----- 2 files changed, 135 insertions(+), 39 deletions(-) diff --git a/contracts/mocks/ERC1363Mock.sol b/contracts/mocks/ERC1363Mock.sol index 289227d86e3..4a6e1e75520 100644 --- a/contracts/mocks/ERC1363Mock.sol +++ b/contracts/mocks/ERC1363Mock.sol @@ -34,7 +34,9 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { bytes memory data ) external override returns (bytes4) { if (data.length == 1) { - require(data[0] == 0x00, "onTransferReceived revert"); + if (data[0] == 0x00) return bytes4(0); + if (data[0] == 0x01) revert("onTransferReceived revert"); + if (data[0] == 0x02) assert(false); } emit TransferReceived(operator, from, value, data); return this.onTransferReceived.selector; @@ -46,7 +48,9 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { bytes memory data ) external override returns (bytes4) { if (data.length == 1) { - require(data[0] == 0x00, "onApprovalReceived revert"); + if (data[0] == 0x00) return bytes4(0); + if (data[0] == 0x01) revert("onApprovalReceived revert"); + if (data[0] == 0x02) assert(false); } emit ApprovalReceived(owner, value, data); return this.onApprovalReceived.selector; diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 64f29f83fff..065884bfa53 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -69,18 +69,48 @@ contract('ERC1363', function (accounts) { }); }); - it('with reverting hook', async function () { - const data = '0x01'; - - await expectRevert( - this.token.methods['transferAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'onTransferReceived revert', - ); + describe('revert', function () { + it('invalid return value', async function () { + const data = '0x00'; + + await expectRevert( + this.token.methods['transferAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'ERC1363: onTransferReceived invalid result', + ); + }); + + it('hook reverts with message', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['transferAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'onTransferReceived revert', + ); + }); + + it('hook reverts with error', async function () { + const data = '0x02'; + + await expectRevert( + this.token.methods['transferAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'ERC1363: onTransferReceived reverted without reason', + ); + }); }); }); @@ -136,19 +166,51 @@ contract('ERC1363', function (accounts) { }); }); - it('with reverting hook', async function () { - const data = '0x01'; - - await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( - holder, - this.receiver.address, - value, - data, - { from: operator }, - ), - 'onTransferReceived revert', - ); + describe('revert', function () { + it('invalid return value', async function () { + const data = '0x00'; + + await expectRevert( + this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( + holder, + this.receiver.address, + value, + data, + { from: operator }, + ), + 'ERC1363: onTransferReceived invalid result', + ); + }); + + it('hook reverts with message', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( + holder, + this.receiver.address, + value, + data, + { from: operator }, + ), + 'onTransferReceived revert', + ); + }); + + it('hook reverts with error', async function () { + const data = '0x02'; + + await expectRevert( + this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( + holder, + this.receiver.address, + value, + data, + { from: operator }, + ), + 'ERC1363: onTransferReceived reverted without reason', + ); + }); }); }); @@ -196,18 +258,48 @@ contract('ERC1363', function (accounts) { }); }); - it('with reverting hook', async function () { - const data = '0x01'; - - await expectRevert( - this.token.methods['approveAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'onApprovalReceived revert', - ); + describe('revert', function () { + it('invalid return value', async function () { + const data = '0x00'; + + await expectRevert( + this.token.methods['approveAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'ERC1363: onApprovalReceived invalid result', + ); + }); + + it('hook reverts with message', async function () { + const data = '0x01'; + + await expectRevert( + this.token.methods['approveAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'onApprovalReceived revert', + ); + }); + + it('hook reverts with error', async function () { + const data = '0x02'; + + await expectRevert( + this.token.methods['approveAndCall(address,uint256,bytes)']( + this.receiver.address, + value, + data, + { from: holder }, + ), + 'ERC1363: onApprovalReceived reverted without reason', + ); + }); }); }); }); From 65ac716e3bf0e81fc5aab1eacab355b82b68ab9a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 13 Dec 2021 18:32:17 +0100 Subject: [PATCH 04/15] skip mocks and preset --- scripts/inheritanceOrdering.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/inheritanceOrdering.js b/scripts/inheritanceOrdering.js index 8a052f36fe1..937b0f5a90a 100644 --- a/scripts/inheritanceOrdering.js +++ b/scripts/inheritanceOrdering.js @@ -12,6 +12,11 @@ for (const artifact of artifacts) { for (const source in solcOutput.contracts) { for (const contractDef of findAll('ContractDefinition', solcOutput.sources[source].ast)) { + // Skip mocks and presets + if ([ 'Mock', 'Preset' ].some(e => contractDef.name.includes(e))) { + continue; + } + names[contractDef.id] = contractDef.name; linearized.push(contractDef.linearizedBaseContracts); From b88a69b6a6a6c87467d4b7da1514ac23a70aa43f Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 14 Dec 2021 11:34:56 +0100 Subject: [PATCH 05/15] refactor test for readability --- test/token/ERC20/extensions/ERC1363.test.js | 368 +++++++------------- 1 file changed, 131 insertions(+), 237 deletions(-) diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 065884bfa53..e84c654e6de 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -25,92 +25,56 @@ contract('ERC1363', function (accounts) { describe('transferAndCall', function () { it('without data', async function () { - const data = null; - - const { tx } = await this.token.methods['transferAndCall(address,uint256)']( - this.receiver.address, - value, - { from: holder }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: holder, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator: holder, - from: holder, - value, - data, - }); + this.function = 'transferAndCall(address,uint256)'; + this.operator = holder; }); it('with data', async function () { - const data = '0x123456'; - - const { tx } = await this.token.methods['transferAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: holder, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator: holder, - from: holder, - value, - data, - }); + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x123456'; + this.operator = holder; }); - describe('revert', function () { - it('invalid return value', async function () { - const data = '0x00'; - - await expectRevert( - this.token.methods['transferAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'ERC1363: onTransferReceived invalid result', - ); - }); - - it('hook reverts with message', async function () { - const data = '0x01'; + it('invalid return value', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x00'; + this.operator = holder; + this.revert = 'ERC1363: onTransferReceived invalid result'; + }); - await expectRevert( - this.token.methods['transferAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'onTransferReceived revert', - ); - }); + it('hook reverts with message', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x01'; + this.operator = holder; + this.revert = 'onTransferReceived revert'; + }); - it('hook reverts with error', async function () { - const data = '0x02'; + it('hook reverts with error', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x02'; + this.operator = holder; + this.revert = 'ERC1363: onTransferReceived reverted without reason'; + }); - await expectRevert( - this.token.methods['transferAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'ERC1363: onTransferReceived reverted without reason', - ); - }); + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ this.receiver.address, value, this.data ].filter(Boolean), { from: this.operator }); + + if (this.revert == undefined) { + const { tx } = await txPromise; + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: this.from || this.operator, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: this.operator, + from: this.from || this.operator, + value, + data: this.data ?? null, + }); + } else { + await expectRevert(txPromise, this.revert); + } }); }); @@ -120,186 +84,116 @@ contract('ERC1363', function (accounts) { }); it('without data', async function () { - const data = null; - - const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256)']( - holder, - this.receiver.address, - value, - { from: operator }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: holder, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator, - from: holder, - value, - data, - }); + this.function = 'transferFromAndCall(address,address,uint256)'; + this.from = holder; + this.operator = operator; }); it('with data', async function () { - const data = '0x123456'; - - const { tx } = await this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( - holder, - this.receiver.address, - value, - data, - { from: operator }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: holder, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator, - from: holder, - value, - data, - }); + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x123456'; + this.from = holder; + this.operator = operator; }); - describe('revert', function () { - it('invalid return value', async function () { - const data = '0x00'; - - await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( - holder, - this.receiver.address, - value, - data, - { from: operator }, - ), - 'ERC1363: onTransferReceived invalid result', - ); - }); - - it('hook reverts with message', async function () { - const data = '0x01'; + it('invalid return value', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x00'; + this.from = holder; + this.operator = operator; + this.revert = 'ERC1363: onTransferReceived invalid result'; + }); - await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( - holder, - this.receiver.address, - value, - data, - { from: operator }, - ), - 'onTransferReceived revert', - ); - }); + it('hook reverts with message', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x01'; + this.from = holder; + this.operator = operator; + this.revert = 'onTransferReceived revert'; + }); - it('hook reverts with error', async function () { - const data = '0x02'; + it('hook reverts with error', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x02'; + this.from = holder; + this.operator = operator; + this.revert = 'ERC1363: onTransferReceived reverted without reason'; + }); - await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256,bytes)']( - holder, - this.receiver.address, - value, - data, - { from: operator }, - ), - 'ERC1363: onTransferReceived reverted without reason', - ); - }); + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ this.from, this.receiver.address, value, this.data ].filter(Boolean), { from: this.operator }); + + if (this.revert == undefined) { + const { tx } = await txPromise; + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: this.from || this.operator, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: this.operator, + from: this.from || this.operator, + value, + data: this.data ?? null, + }); + } else { + await expectRevert(txPromise, this.revert); + } }); }); describe('approveAndCall', function () { it('without data', async function () { - const data = null; - - const { tx } = await this.token.methods['approveAndCall(address,uint256)']( - this.receiver.address, - value, - { from: holder }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Approval', { - owner: holder, - spender: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { - owner: holder, - value, - data, - }); + this.function = 'approveAndCall(address,uint256)'; + this.owner = holder; }); it('with data', async function () { - const data = '0x123456'; - - const { tx } = await this.token.methods['approveAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ); - - await expectEvent.inTransaction(tx, this.token, 'Approval', { - owner: holder, - spender: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { - owner: holder, - value, - data, - }); + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x123456'; + this.owner = holder; }); - describe('revert', function () { - it('invalid return value', async function () { - const data = '0x00'; - - await expectRevert( - this.token.methods['approveAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'ERC1363: onApprovalReceived invalid result', - ); - }); - - it('hook reverts with message', async function () { - const data = '0x01'; + it('invalid return value', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x00'; + this.owner = holder; + this.revert = 'ERC1363: onApprovalReceived invalid result'; + }); - await expectRevert( - this.token.methods['approveAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'onApprovalReceived revert', - ); - }); + it('hook reverts with message', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x01'; + this.owner = holder; + this.revert = 'onApprovalReceived revert'; + }); - it('hook reverts with error', async function () { - const data = '0x02'; + it('hook reverts with error', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x02'; + this.owner = holder; + this.revert = 'ERC1363: onApprovalReceived reverted without reason'; + }); - await expectRevert( - this.token.methods['approveAndCall(address,uint256,bytes)']( - this.receiver.address, - value, - data, - { from: holder }, - ), - 'ERC1363: onApprovalReceived reverted without reason', - ); - }); + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ this.receiver.address, value, this.data ].filter(Boolean), { from: this.owner }) + + if (this.revert == undefined) { + const { tx } = await txPromise; + + await expectEvent.inTransaction(tx, this.token, 'Approval', { + owner: this.owner, + spender: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { + owner: this.owner, + value, + data: this.data ?? null, + }); + } else { + await expectRevert(txPromise, this.revert); + } }); }); }); From 0208e2728a4e0c8fd9430ff588f8e695cd61fbdb Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 14 Dec 2021 11:58:26 +0100 Subject: [PATCH 06/15] fix lint --- test/token/ERC20/extensions/ERC1363.test.js | 102 +++++++++++--------- 1 file changed, 59 insertions(+), 43 deletions(-) diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index e84c654e6de..503fb9ca90f 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -31,35 +31,40 @@ contract('ERC1363', function (accounts) { it('with data', async function () { this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x123456'; + this.data = '0x123456'; this.operator = holder; }); it('invalid return value', async function () { this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x00'; + this.data = '0x00'; this.operator = holder; - this.revert = 'ERC1363: onTransferReceived invalid result'; + this.revert = 'ERC1363: onTransferReceived invalid result'; }); it('hook reverts with message', async function () { this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x01'; + this.data = '0x01'; this.operator = holder; - this.revert = 'onTransferReceived revert'; + this.revert = 'onTransferReceived revert'; }); it('hook reverts with error', async function () { this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x02'; + this.data = '0x02'; this.operator = holder; - this.revert = 'ERC1363: onTransferReceived reverted without reason'; + this.revert = 'ERC1363: onTransferReceived reverted without reason'; }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ this.receiver.address, value, this.data ].filter(Boolean), { from: this.operator }); - - if (this.revert == undefined) { + const txPromise = this.token.methods[this.function](...[ + this.receiver.address, + value, + this.data, + { from: this.operator }, + ].filter(Boolean)); + + if (this.revert === undefined) { const { tx } = await txPromise; await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.from || this.operator, @@ -70,7 +75,7 @@ contract('ERC1363', function (accounts) { operator: this.operator, from: this.from || this.operator, value, - data: this.data ?? null, + data: this.data || null, }); } else { await expectRevert(txPromise, this.revert); @@ -85,45 +90,51 @@ contract('ERC1363', function (accounts) { it('without data', async function () { this.function = 'transferFromAndCall(address,address,uint256)'; - this.from = holder; + this.from = holder; this.operator = operator; }); it('with data', async function () { this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x123456'; - this.from = holder; + this.data = '0x123456'; + this.from = holder; this.operator = operator; }); it('invalid return value', async function () { this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x00'; - this.from = holder; + this.data = '0x00'; + this.from = holder; this.operator = operator; - this.revert = 'ERC1363: onTransferReceived invalid result'; + this.revert = 'ERC1363: onTransferReceived invalid result'; }); it('hook reverts with message', async function () { this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x01'; - this.from = holder; + this.data = '0x01'; + this.from = holder; this.operator = operator; - this.revert = 'onTransferReceived revert'; + this.revert = 'onTransferReceived revert'; }); it('hook reverts with error', async function () { this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x02'; - this.from = holder; + this.data = '0x02'; + this.from = holder; this.operator = operator; - this.revert = 'ERC1363: onTransferReceived reverted without reason'; + this.revert = 'ERC1363: onTransferReceived reverted without reason'; }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ this.from, this.receiver.address, value, this.data ].filter(Boolean), { from: this.operator }); - - if (this.revert == undefined) { + const txPromise = this.token.methods[this.function](...[ + this.from, + this.receiver.address, + value, + this.data, + { from: this.operator }, + ].filter(Boolean)); + + if (this.revert === undefined) { const { tx } = await txPromise; await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.from || this.operator, @@ -134,7 +145,7 @@ contract('ERC1363', function (accounts) { operator: this.operator, from: this.from || this.operator, value, - data: this.data ?? null, + data: this.data || null, }); } else { await expectRevert(txPromise, this.revert); @@ -145,40 +156,45 @@ contract('ERC1363', function (accounts) { describe('approveAndCall', function () { it('without data', async function () { this.function = 'approveAndCall(address,uint256)'; - this.owner = holder; + this.owner = holder; }); it('with data', async function () { this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x123456'; - this.owner = holder; + this.data = '0x123456'; + this.owner = holder; }); it('invalid return value', async function () { this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x00'; - this.owner = holder; - this.revert = 'ERC1363: onApprovalReceived invalid result'; + this.data = '0x00'; + this.owner = holder; + this.revert = 'ERC1363: onApprovalReceived invalid result'; }); it('hook reverts with message', async function () { this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x01'; - this.owner = holder; - this.revert = 'onApprovalReceived revert'; + this.data = '0x01'; + this.owner = holder; + this.revert = 'onApprovalReceived revert'; }); it('hook reverts with error', async function () { this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x02'; - this.owner = holder; - this.revert = 'ERC1363: onApprovalReceived reverted without reason'; + this.data = '0x02'; + this.owner = holder; + this.revert = 'ERC1363: onApprovalReceived reverted without reason'; }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ this.receiver.address, value, this.data ].filter(Boolean), { from: this.owner }) - - if (this.revert == undefined) { + const txPromise = this.token.methods[this.function](...[ + this.receiver.address, + value, + this.data, + { from: this.owner }, + ].filter(Boolean)); + + if (this.revert === undefined) { const { tx } = await txPromise; await expectEvent.inTransaction(tx, this.token, 'Approval', { @@ -189,7 +205,7 @@ contract('ERC1363', function (accounts) { await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { owner: this.owner, value, - data: this.data ?? null, + data: this.data || null, }); } else { await expectRevert(txPromise, this.revert); From 1ec46cfec4c69fc81a162af5c2ec04281616441a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 13 Jun 2022 18:53:52 +0200 Subject: [PATCH 07/15] refactor preset skip --- scripts/checks/inheritance-ordering.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/scripts/checks/inheritance-ordering.js b/scripts/checks/inheritance-ordering.js index 0afcebb113c..84366c40249 100755 --- a/scripts/checks/inheritance-ordering.js +++ b/scripts/checks/inheritance-ordering.js @@ -13,16 +13,11 @@ for (const artifact of artifacts) { const linearized = []; for (const source in solcOutput.contracts) { - if (source.includes('/mocks/')) { + if ([ '/mocks/', '/presets/' ].some(skip => source.includes(skip))) { continue; } for (const contractDef of findAll('ContractDefinition', solcOutput.sources[source].ast)) { - // Skip mocks and presets - if ([ 'Mock', 'Preset' ].some(e => contractDef.name.includes(e))) { - continue; - } - names[contractDef.id] = contractDef.name; linearized.push(contractDef.linearizedBaseContracts); From c24145f528b854d3c505c3c1890e101d5666c300 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 16 Jun 2022 15:10:20 +0200 Subject: [PATCH 08/15] allow transfer and call to EOA --- contracts/mocks/ERC1363Mock.sol | 6 +- contracts/token/ERC20/extensions/ERC1363.sol | 122 +++++-- test/token/ERC20/extensions/ERC1363.test.js | 364 +++++++++++-------- 3 files changed, 308 insertions(+), 184 deletions(-) diff --git a/contracts/mocks/ERC1363Mock.sol b/contracts/mocks/ERC1363Mock.sol index 4a6e1e75520..193c6e24041 100644 --- a/contracts/mocks/ERC1363Mock.sol +++ b/contracts/mocks/ERC1363Mock.sol @@ -36,7 +36,8 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { if (data.length == 1) { if (data[0] == 0x00) return bytes4(0); if (data[0] == 0x01) revert("onTransferReceived revert"); - if (data[0] == 0x02) assert(false); + if (data[0] == 0x02) revert(); + if (data[0] == 0x03) assert(false); } emit TransferReceived(operator, from, value, data); return this.onTransferReceived.selector; @@ -50,7 +51,8 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { if (data.length == 1) { if (data[0] == 0x00) return bytes4(0); if (data[0] == 0x01) revert("onApprovalReceived revert"); - if (data[0] == 0x02) assert(false); + if (data[0] == 0x02) revert(); + if (data[0] == 0x03) assert(false); } emit ApprovalReceived(owner, value, data); return this.onApprovalReceived.selector; diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol index 3678a995505..7514254f171 100644 --- a/contracts/token/ERC20/extensions/ERC1363.sol +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -6,35 +6,44 @@ import "../../../interfaces/IERC1363.sol"; import "../../../interfaces/IERC1363Receiver.sol"; import "../../../interfaces/IERC1363Spender.sol"; import "../../../utils/introspection/ERC165.sol"; +import "../../../utils/Address.sol"; abstract contract ERC1363 is IERC1363, ERC20, ERC165 { + using Address for address; + + /** + * @dev See {IERC165-supportsInterface}. + */ function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { return interfaceId == type(IERC1363).interfaceId || super.supportsInterface(interfaceId); } + /** + * @dev See {IERC1363-transferAndCall}. + */ function transferAndCall(address to, uint256 value) public override returns (bool) { return transferAndCall(to, value, bytes("")); } + /** + * @dev See {IERC1363-transferAndCall}. + */ function transferAndCall( address to, uint256 value, bytes memory data ) public override returns (bool) { require(transfer(to, value)); - try IERC1363Receiver(to).onTransferReceived(_msgSender(), _msgSender(), value, data) returns (bytes4 selector) { - require( - selector == IERC1363Receiver(to).onTransferReceived.selector, - "ERC1363: onTransferReceived invalid result" - ); - } catch Error(string memory reason) { - revert(reason); - } catch { - revert("ERC1363: onTransferReceived reverted without reason"); - } + require( + _checkOnTransferReceived(_msgSender(), to, value, data), + "ERC1363: transfer to non ERC1363Receiver implementer" + ); return true; } + /** + * @dev See {IERC1363-transferFromAndCall}. + */ function transferFromAndCall( address from, address to, @@ -43,6 +52,9 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { return transferFromAndCall(from, to, value, bytes("")); } + /** + * @dev See {IERC1363-transferFromAndCall}. + */ function transferFromAndCall( address from, address to, @@ -50,39 +62,89 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { bytes memory data ) public override returns (bool) { require(transferFrom(from, to, value)); - try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 selector) { - require( - selector == IERC1363Receiver(to).onTransferReceived.selector, - "ERC1363: onTransferReceived invalid result" - ); - } catch Error(string memory reason) { - revert(reason); - } catch { - revert("ERC1363: onTransferReceived reverted without reason"); - } + require( + _checkOnTransferReceived(from, to, value, data), + "ERC1363: transfer to non ERC1363Receiver implementer" + ); return true; } + /** + * @dev See {IERC1363-approveAndCall}. + */ function approveAndCall(address spender, uint256 value) public override returns (bool) { return approveAndCall(spender, value, bytes("")); } + /** + * @dev See {IERC1363-approveAndCall}. + */ function approveAndCall( address spender, uint256 value, bytes memory data ) public override returns (bool) { require(approve(spender, value)); - try IERC1363Spender(spender).onApprovalReceived(_msgSender(), value, data) returns (bytes4 selector) { - require( - selector == IERC1363Spender(spender).onApprovalReceived.selector, - "ERC1363: onApprovalReceived invalid result" - ); - } catch Error(string memory reason) { - revert(reason); - } catch { - revert("ERC1363: onApprovalReceived reverted without reason"); - } + require( + _checkOnApprovalReceived(_msgSender(), spender, value, data), + "ERC1363: transfer to non ERC1363Spender implementer" + ); 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. + */ + function _checkOnTransferReceived( + address from, + address to, + uint256 value, + bytes memory data + ) private returns (bool) { + if (to.isContract()) { + try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 retval) { + return retval == IERC1363Receiver.onTransferReceived.selector; + } catch (bytes memory reason) { + if (reason.length == 0) { + revert("ERC1363: transfer to non ERC1363Receiver implementer"); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } else { + return true; + } + } + + /** + * @dev Internal function to invoke {IERC1363Spender-onApprovalReceived} on a target address. + * The call is not executed if the target address is not a contract. + */ + function _checkOnApprovalReceived( + address owner, + address spender, + uint256 value, + bytes memory data + ) private returns (bool) { + if (spender.isContract()) { + try IERC1363Spender(spender).onApprovalReceived(owner, value, data) returns (bytes4 retval) { + return retval == IERC1363Spender.onApprovalReceived.selector; + } catch (bytes memory reason) { + if (reason.length == 0) { + revert("ERC1363: transfer to non ERC1363Spender implementer"); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } else { + return true; + } + } } diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 503fb9ca90f..d7333437914 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -5,7 +5,7 @@ const ERC1363Mock = artifacts.require('ERC1363Mock'); const ERC1363ReceiverMock = artifacts.require('ERC1363ReceiverMock'); contract('ERC1363', function (accounts) { - const [ holder, operator ] = accounts; + const [ holder, operator, other ] = accounts; const initialSupply = new BN(100); const value = new BN(10); @@ -24,62 +24,80 @@ contract('ERC1363', function (accounts) { ]); describe('transferAndCall', function () { - it('without data', async function () { - this.function = 'transferAndCall(address,uint256)'; - this.operator = holder; + it('to EOA', async function () { + const { receipt } = await this.token.methods['transferAndCall(address,uint256)'](other, value, { from: holder }); + expectEvent(receipt, 'Transfer', { + from: holder, + to: other, + value, + }); }); - it('with data', async function () { - this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x123456'; - this.operator = holder; - }); + describe('to receiver', function () { + it('without data', async function () { + this.function = 'transferAndCall(address,uint256)'; + this.operator = holder; + }); - it('invalid return value', async function () { - this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x00'; - this.operator = holder; - this.revert = 'ERC1363: onTransferReceived invalid result'; - }); + it('with data', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x123456'; + this.operator = holder; + }); - it('hook reverts with message', async function () { - this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x01'; - this.operator = holder; - this.revert = 'onTransferReceived revert'; - }); + it('invalid return value', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x00'; + this.operator = holder; + this.revert = 'ERC1363: transfer to non ERC1363Receiver implementer'; + }); - it('hook reverts with error', async function () { - this.function = 'transferAndCall(address,uint256,bytes)'; - this.data = '0x02'; - this.operator = holder; - this.revert = 'ERC1363: onTransferReceived reverted without reason'; - }); + it('hook reverts with message', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x01'; + this.operator = holder; + this.revert = 'onTransferReceived revert'; + }); - afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.receiver.address, - value, - this.data, - { from: this.operator }, - ].filter(Boolean)); - - if (this.revert === undefined) { - const { tx } = await txPromise; - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: this.from || this.operator, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator: this.operator, - from: this.from || this.operator, + it('hook reverts without message', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x02'; + this.operator = holder; + this.revert = 'ERC1363: transfer to non ERC1363Receiver implementer'; + }); + + it('hook reverts with assert(false)', async function () { + this.function = 'transferAndCall(address,uint256,bytes)'; + this.data = '0x03'; + this.operator = holder; + this.revert = 'reverted with panic code 0x1 (Assertion error)'; + }); + + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ + this.receiver.address, value, - data: this.data || null, - }); - } else { - await expectRevert(txPromise, this.revert); - } + this.data, + { from: this.operator }, + ].filter(Boolean)); + + if (this.revert === undefined) { + const { tx } = await txPromise; + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: this.from || this.operator, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: this.operator, + from: this.from || this.operator, + value, + data: this.data || null, + }); + } else { + await expectRevert(txPromise, this.revert); + } + }); }); }); @@ -88,128 +106,170 @@ contract('ERC1363', function (accounts) { await this.token.approve(operator, initialSupply, { from: holder }); }); - it('without data', async function () { - this.function = 'transferFromAndCall(address,address,uint256)'; - this.from = holder; - this.operator = operator; + it('to EOA', async function () { + const { receipt } = await this.token.methods['transferFromAndCall(address,address,uint256)']( + holder, + other, + value, + { from: operator }, + ); + expectEvent(receipt, 'Transfer', { + from: holder, + to: other, + value, + }); }); - it('with data', async function () { - this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x123456'; - this.from = holder; - this.operator = operator; - }); + describe('to receiver', function () { + it('without data', async function () { + this.function = 'transferFromAndCall(address,address,uint256)'; + this.from = holder; + this.operator = operator; + }); - it('invalid return value', async function () { - this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x00'; - this.from = holder; - this.operator = operator; - this.revert = 'ERC1363: onTransferReceived invalid result'; - }); + it('with data', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x123456'; + this.from = holder; + this.operator = operator; + }); - it('hook reverts with message', async function () { - this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x01'; - this.from = holder; - this.operator = operator; - this.revert = 'onTransferReceived revert'; - }); + it('invalid return value', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x00'; + this.from = holder; + this.operator = operator; + this.revert = 'ERC1363: transfer to non ERC1363Receiver implementer'; + }); - it('hook reverts with error', async function () { - this.function = 'transferFromAndCall(address,address,uint256,bytes)'; - this.data = '0x02'; - this.from = holder; - this.operator = operator; - this.revert = 'ERC1363: onTransferReceived reverted without reason'; - }); + it('hook reverts with message', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x01'; + this.from = holder; + this.operator = operator; + this.revert = 'onTransferReceived revert'; + }); - afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.from, - this.receiver.address, - value, - this.data, - { from: this.operator }, - ].filter(Boolean)); - - if (this.revert === undefined) { - const { tx } = await txPromise; - await expectEvent.inTransaction(tx, this.token, 'Transfer', { - from: this.from || this.operator, - to: this.receiver.address, - value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { - operator: this.operator, - from: this.from || this.operator, + it('hook reverts without message', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x02'; + this.from = holder; + this.operator = operator; + this.revert = 'ERC1363: transfer to non ERC1363Receiver implementer'; + }); + + it('hook reverts with assert(false)', async function () { + this.function = 'transferFromAndCall(address,address,uint256,bytes)'; + this.data = '0x03'; + this.operator = holder; + this.operator = operator; + this.revert = 'reverted with panic code 0x1 (Assertion error)'; + }); + + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ + this.from, + this.receiver.address, value, - data: this.data || null, - }); - } else { - await expectRevert(txPromise, this.revert); - } + this.data, + { from: this.operator }, + ].filter(Boolean)); + + if (this.revert === undefined) { + const { tx } = await txPromise; + await expectEvent.inTransaction(tx, this.token, 'Transfer', { + from: this.from || this.operator, + to: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'TransferReceived', { + operator: this.operator, + from: this.from || this.operator, + value, + data: this.data || null, + }); + } else { + await expectRevert(txPromise, this.revert); + } + }); }); }); describe('approveAndCall', function () { - it('without data', async function () { - this.function = 'approveAndCall(address,uint256)'; - this.owner = holder; + it('to EOA', async function () { + const { receipt } = await this.token.methods['approveAndCall(address,uint256)'](other, value, { from: holder }); + expectEvent(receipt, 'Approval', { + owner: holder, + spender: other, + value, + }); }); - it('with data', async function () { - this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x123456'; - this.owner = holder; - }); + describe('to receiver', function () { + it('without data', async function () { + this.function = 'approveAndCall(address,uint256)'; + this.owner = holder; + }); - it('invalid return value', async function () { - this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x00'; - this.owner = holder; - this.revert = 'ERC1363: onApprovalReceived invalid result'; - }); + it('with data', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x123456'; + this.owner = holder; + }); - it('hook reverts with message', async function () { - this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x01'; - this.owner = holder; - this.revert = 'onApprovalReceived revert'; - }); + it('invalid return value', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x00'; + this.owner = holder; + this.revert = 'ERC1363: transfer to non ERC1363Spender implementer'; + }); - it('hook reverts with error', async function () { - this.function = 'approveAndCall(address,uint256,bytes)'; - this.data = '0x02'; - this.owner = holder; - this.revert = 'ERC1363: onApprovalReceived reverted without reason'; - }); + it('hook reverts with message', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x01'; + this.owner = holder; + this.revert = 'onApprovalReceived revert'; + }); - afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.receiver.address, - value, - this.data, - { from: this.owner }, - ].filter(Boolean)); + it('hook reverts without message', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x02'; + this.owner = holder; + this.revert = 'ERC1363: transfer to non ERC1363Spender implementer'; + }); - if (this.revert === undefined) { - const { tx } = await txPromise; + it('hook reverts with assert(false)', async function () { + this.function = 'approveAndCall(address,uint256,bytes)'; + this.data = '0x03'; + this.operator = holder; + this.revert = 'reverted with panic code 0x1 (Assertion error)'; + }); - await expectEvent.inTransaction(tx, this.token, 'Approval', { - owner: this.owner, - spender: this.receiver.address, + afterEach(async function () { + const txPromise = this.token.methods[this.function](...[ + this.receiver.address, value, - }); - await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { - owner: this.owner, - value, - data: this.data || null, - }); - } else { - await expectRevert(txPromise, this.revert); - } + this.data, + { from: this.owner }, + ].filter(Boolean)); + + if (this.revert === undefined) { + const { tx } = await txPromise; + + await expectEvent.inTransaction(tx, this.token, 'Approval', { + owner: this.owner, + spender: this.receiver.address, + value, + }); + await expectEvent.inTransaction(tx, this.receiver, 'ApprovalReceived', { + owner: this.owner, + value, + data: this.data || null, + }); + } else { + await expectRevert(txPromise, this.revert); + } + }); }); }); }); From a3ef6b445ab5bb8c5d3bf2e6073708ce503bea0a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 17 Jun 2022 14:10:15 +0200 Subject: [PATCH 09/15] revert allow transferAndCall to EOA --- contracts/token/ERC20/extensions/ERC1363.sol | 44 ++++++++------------ test/token/ERC20/extensions/ERC1363.test.js | 42 +++++++++---------- 2 files changed, 38 insertions(+), 48 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol index 7514254f171..59fc2779e92 100644 --- a/contracts/token/ERC20/extensions/ERC1363.sol +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -102,21 +102,17 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { uint256 value, bytes memory data ) private returns (bool) { - if (to.isContract()) { - try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 retval) { - return retval == IERC1363Receiver.onTransferReceived.selector; - } catch (bytes memory reason) { - if (reason.length == 0) { - revert("ERC1363: transfer to non ERC1363Receiver implementer"); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } + try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 retval) { + return retval == IERC1363Receiver.onTransferReceived.selector; + } catch (bytes memory reason) { + if (reason.length == 0) { + revert("ERC1363: transfer to non ERC1363Receiver implementer"); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) } } - } else { - return true; } } @@ -130,21 +126,17 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { uint256 value, bytes memory data ) private returns (bool) { - if (spender.isContract()) { - try IERC1363Spender(spender).onApprovalReceived(owner, value, data) returns (bytes4 retval) { - return retval == IERC1363Spender.onApprovalReceived.selector; - } catch (bytes memory reason) { - if (reason.length == 0) { - revert("ERC1363: transfer to non ERC1363Spender implementer"); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } + try IERC1363Spender(spender).onApprovalReceived(owner, value, data) returns (bytes4 retval) { + return retval == IERC1363Spender.onApprovalReceived.selector; + } catch (bytes memory reason) { + if (reason.length == 0) { + revert("ERC1363: transfer to non ERC1363Spender implementer"); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) } } - } else { - return true; } } } diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index d7333437914..453bef0e51a 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -25,12 +25,14 @@ contract('ERC1363', function (accounts) { describe('transferAndCall', function () { it('to EOA', async function () { - const { receipt } = await this.token.methods['transferAndCall(address,uint256)'](other, value, { from: holder }); - expectEvent(receipt, 'Transfer', { - from: holder, - to: other, - value, - }); + await expectRevert( + this.token.methods['transferAndCall(address,uint256)']( + other, + value, + { from: holder }, + ), + 'function call to a non-contract account', + ); }); describe('to receiver', function () { @@ -107,17 +109,15 @@ contract('ERC1363', function (accounts) { }); it('to EOA', async function () { - const { receipt } = await this.token.methods['transferFromAndCall(address,address,uint256)']( - holder, - other, - value, - { from: operator }, + await expectRevert( + this.token.methods['transferFromAndCall(address,address,uint256)']( + holder, + other, + value, + { from: operator }, + ), + 'function call to a non-contract account', ); - expectEvent(receipt, 'Transfer', { - from: holder, - to: other, - value, - }); }); describe('to receiver', function () { @@ -197,12 +197,10 @@ contract('ERC1363', function (accounts) { describe('approveAndCall', function () { it('to EOA', async function () { - const { receipt } = await this.token.methods['approveAndCall(address,uint256)'](other, value, { from: holder }); - expectEvent(receipt, 'Approval', { - owner: holder, - spender: other, - value, - }); + await expectRevert( + this.token.methods['approveAndCall(address,uint256)'](other, value, { from: holder }), + 'function call to a non-contract account', + ); }); describe('to receiver', function () { From eadcad5e6945462b3e1d056ec770353207ed57c5 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Jul 2022 10:46:48 +0200 Subject: [PATCH 10/15] fix test for compilers >= 0.8.10 --- test/token/ERC20/extensions/ERC1363.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 453bef0e51a..5d4e516d22c 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -31,7 +31,7 @@ contract('ERC1363', function (accounts) { value, { from: holder }, ), - 'function call to a non-contract account', + 'function returned an unexpected amount of data', ); }); @@ -116,7 +116,7 @@ contract('ERC1363', function (accounts) { value, { from: operator }, ), - 'function call to a non-contract account', + 'function returned an unexpected amount of data', ); }); @@ -199,7 +199,7 @@ contract('ERC1363', function (accounts) { it('to EOA', async function () { await expectRevert( this.token.methods['approveAndCall(address,uint256)'](other, value, { from: holder }), - 'function call to a non-contract account', + 'function returned an unexpected amount of data', ); }); From 08dd234f57f0372260422ac85281a21d70adac50 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Jul 2022 13:11:20 +0200 Subject: [PATCH 11/15] code consistency --- contracts/token/ERC20/extensions/ERC1363.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol index 59fc2779e92..966e89ddc0e 100644 --- a/contracts/token/ERC20/extensions/ERC1363.sol +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -35,7 +35,7 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { ) public override returns (bool) { require(transfer(to, value)); require( - _checkOnTransferReceived(_msgSender(), to, value, data), + _checkOnTransferReceived(_msgSender(), _msgSender(), to, value, data), "ERC1363: transfer to non ERC1363Receiver implementer" ); return true; @@ -63,7 +63,7 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { ) public override returns (bool) { require(transferFrom(from, to, value)); require( - _checkOnTransferReceived(from, to, value, data), + _checkOnTransferReceived(_msgSender(), from, to, value, data), "ERC1363: transfer to non ERC1363Receiver implementer" ); return true; @@ -97,12 +97,13 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { * The call is not executed if the target address is not a contract. */ function _checkOnTransferReceived( + address operator, address from, address to, uint256 value, bytes memory data ) private returns (bool) { - try IERC1363Receiver(to).onTransferReceived(_msgSender(), from, value, data) returns (bytes4 retval) { + try IERC1363Receiver(to).onTransferReceived(operator, from, value, data) returns (bytes4 retval) { return retval == IERC1363Receiver.onTransferReceived.selector; } catch (bytes memory reason) { if (reason.length == 0) { From 7e2bbfdb53234d46f645e044eefc22a0d437a780 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 23 Feb 2023 15:31:30 +0100 Subject: [PATCH 12/15] add changeset and update to hardat-expose --- .changeset/tasty-apples-serve.md | 5 ++++ .../ERC1363ReceiverMock.sol} | 24 +++---------------- test/token/ERC20/extensions/ERC1363.test.js | 15 ++++++------ 3 files changed, 16 insertions(+), 28 deletions(-) create mode 100644 .changeset/tasty-apples-serve.md rename contracts/mocks/{ERC1363Mock.sol => token/ERC1363ReceiverMock.sol} (72%) diff --git a/.changeset/tasty-apples-serve.md b/.changeset/tasty-apples-serve.md new file mode 100644 index 00000000000..83d242c40fa --- /dev/null +++ b/.changeset/tasty-apples-serve.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC1363`: Add an extension to `ERC20` for performing transferAndCall & approveAndCall operations following ERC-1363. diff --git a/contracts/mocks/ERC1363Mock.sol b/contracts/mocks/token/ERC1363ReceiverMock.sol similarity index 72% rename from contracts/mocks/ERC1363Mock.sol rename to contracts/mocks/token/ERC1363ReceiverMock.sol index 193c6e24041..9f3b3cc7e6d 100644 --- a/contracts/mocks/ERC1363Mock.sol +++ b/contracts/mocks/token/ERC1363ReceiverMock.sol @@ -2,26 +2,8 @@ pragma solidity ^0.8.0; -import "../token/ERC20/extensions/ERC1363.sol"; - -contract ERC1363Mock is ERC1363 { - constructor( - string memory name, - string memory symbol, - address initialAccount, - uint256 initialBalance - ) payable ERC20(name, symbol) { - _mint(initialAccount, initialBalance); - } - - function mint(address account, uint256 amount) public { - _mint(account, amount); - } - - function burn(address account, uint256 amount) public { - _burn(account, amount); - } -} +import "../../interfaces/IERC1363Receiver.sol"; +import "../../interfaces/IERC1363Spender.sol"; contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { event TransferReceived(address operator, address from, uint256 value, bytes data); @@ -57,4 +39,4 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { emit ApprovalReceived(owner, value, data); return this.onApprovalReceived.selector; } -} +} \ No newline at end of file diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 5d4e516d22c..285d889285e 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -1,21 +1,22 @@ -const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); -const ERC1363Mock = artifacts.require('ERC1363Mock'); +const ERC1363Mock = artifacts.require('$ERC1363'); const ERC1363ReceiverMock = artifacts.require('ERC1363ReceiverMock'); contract('ERC1363', function (accounts) { const [ holder, operator, other ] = accounts; - const initialSupply = new BN(100); - const value = new BN(10); - const name = 'My Token'; const symbol = 'MTKN'; + const supply = web3.utils.toBN(100); + const value = web3.utils.toBN(10); beforeEach(async function () { - this.token = await ERC1363Mock.new(name, symbol, holder, initialSupply); + this.token = await ERC1363Mock.new(name, symbol); this.receiver = await ERC1363ReceiverMock.new(); + + await this.token.$_mint(holder, supply); }); shouldSupportInterfaces([ @@ -105,7 +106,7 @@ contract('ERC1363', function (accounts) { describe('transferFromAndCall', function () { beforeEach(async function () { - await this.token.approve(operator, initialSupply, { from: holder }); + await this.token.approve(operator, supply, { from: holder }); }); it('to EOA', async function () { From b48278682727d9927190faf38328eb36d53361a4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 23 Feb 2023 15:49:56 +0100 Subject: [PATCH 13/15] fix lint --- contracts/mocks/token/ERC1363ReceiverMock.sol | 8 +--- contracts/token/ERC20/extensions/ERC1363.sol | 18 ++----- scripts/checks/inheritance-ordering.js | 2 +- test/token/ERC20/extensions/ERC1363.test.js | 48 +++++-------------- 4 files changed, 19 insertions(+), 57 deletions(-) diff --git a/contracts/mocks/token/ERC1363ReceiverMock.sol b/contracts/mocks/token/ERC1363ReceiverMock.sol index 9f3b3cc7e6d..f20b44b375e 100644 --- a/contracts/mocks/token/ERC1363ReceiverMock.sol +++ b/contracts/mocks/token/ERC1363ReceiverMock.sol @@ -25,11 +25,7 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { return this.onTransferReceived.selector; } - function onApprovalReceived( - address owner, - uint256 value, - bytes memory data - ) external override returns (bytes4) { + function onApprovalReceived(address owner, uint256 value, bytes memory data) external override returns (bytes4) { if (data.length == 1) { if (data[0] == 0x00) return bytes4(0); if (data[0] == 0x01) revert("onApprovalReceived revert"); @@ -39,4 +35,4 @@ contract ERC1363ReceiverMock is IERC1363Receiver, IERC1363Spender { emit ApprovalReceived(owner, value, data); return this.onApprovalReceived.selector; } -} \ No newline at end of file +} diff --git a/contracts/token/ERC20/extensions/ERC1363.sol b/contracts/token/ERC20/extensions/ERC1363.sol index 966e89ddc0e..1a50208b5c3 100644 --- a/contracts/token/ERC20/extensions/ERC1363.sol +++ b/contracts/token/ERC20/extensions/ERC1363.sol @@ -28,11 +28,7 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { /** * @dev See {IERC1363-transferAndCall}. */ - function transferAndCall( - address to, - uint256 value, - bytes memory data - ) public override returns (bool) { + function transferAndCall(address to, uint256 value, bytes memory data) public override returns (bool) { require(transfer(to, value)); require( _checkOnTransferReceived(_msgSender(), _msgSender(), to, value, data), @@ -44,11 +40,7 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { /** * @dev See {IERC1363-transferFromAndCall}. */ - function transferFromAndCall( - address from, - address to, - uint256 value - ) public override returns (bool) { + function transferFromAndCall(address from, address to, uint256 value) public override returns (bool) { return transferFromAndCall(from, to, value, bytes("")); } @@ -79,11 +71,7 @@ abstract contract ERC1363 is IERC1363, ERC20, ERC165 { /** * @dev See {IERC1363-approveAndCall}. */ - function approveAndCall( - address spender, - uint256 value, - bytes memory data - ) public override returns (bool) { + function approveAndCall(address spender, uint256 value, bytes memory data) public override returns (bool) { require(approve(spender, value)); require( _checkOnApprovalReceived(_msgSender(), spender, value, data), diff --git a/scripts/checks/inheritance-ordering.js b/scripts/checks/inheritance-ordering.js index c4bfbb6a7f4..620c322e3df 100755 --- a/scripts/checks/inheritance-ordering.js +++ b/scripts/checks/inheritance-ordering.js @@ -13,7 +13,7 @@ for (const artifact of artifacts) { const linearized = []; for (const source in solcOutput.contracts) { - if ([ '/mocks/', '/presets/' ].some(skip => source.includes(skip))) { + if (['/mocks/', '/presets/'].some(skip => source.includes(skip))) { continue; } diff --git a/test/token/ERC20/extensions/ERC1363.test.js b/test/token/ERC20/extensions/ERC1363.test.js index 285d889285e..6d8c1d8df43 100644 --- a/test/token/ERC20/extensions/ERC1363.test.js +++ b/test/token/ERC20/extensions/ERC1363.test.js @@ -5,7 +5,7 @@ const ERC1363Mock = artifacts.require('$ERC1363'); const ERC1363ReceiverMock = artifacts.require('ERC1363ReceiverMock'); contract('ERC1363', function (accounts) { - const [ holder, operator, other ] = accounts; + const [holder, operator, other] = accounts; const name = 'My Token'; const symbol = 'MTKN'; @@ -19,19 +19,12 @@ contract('ERC1363', function (accounts) { await this.token.$_mint(holder, supply); }); - shouldSupportInterfaces([ - 'ERC165', - 'ERC1363', - ]); + shouldSupportInterfaces(['ERC165', 'ERC1363']); describe('transferAndCall', function () { it('to EOA', async function () { await expectRevert( - this.token.methods['transferAndCall(address,uint256)']( - other, - value, - { from: holder }, - ), + this.token.methods['transferAndCall(address,uint256)'](other, value, { from: holder }), 'function returned an unexpected amount of data', ); }); @@ -77,12 +70,9 @@ contract('ERC1363', function (accounts) { }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.receiver.address, - value, - this.data, - { from: this.operator }, - ].filter(Boolean)); + const txPromise = this.token.methods[this.function]( + ...[this.receiver.address, value, this.data, { from: this.operator }].filter(Boolean), + ); if (this.revert === undefined) { const { tx } = await txPromise; @@ -111,12 +101,7 @@ contract('ERC1363', function (accounts) { it('to EOA', async function () { await expectRevert( - this.token.methods['transferFromAndCall(address,address,uint256)']( - holder, - other, - value, - { from: operator }, - ), + this.token.methods['transferFromAndCall(address,address,uint256)'](holder, other, value, { from: operator }), 'function returned an unexpected amount of data', ); }); @@ -168,13 +153,9 @@ contract('ERC1363', function (accounts) { }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.from, - this.receiver.address, - value, - this.data, - { from: this.operator }, - ].filter(Boolean)); + const txPromise = this.token.methods[this.function]( + ...[this.from, this.receiver.address, value, this.data, { from: this.operator }].filter(Boolean), + ); if (this.revert === undefined) { const { tx } = await txPromise; @@ -245,12 +226,9 @@ contract('ERC1363', function (accounts) { }); afterEach(async function () { - const txPromise = this.token.methods[this.function](...[ - this.receiver.address, - value, - this.data, - { from: this.owner }, - ].filter(Boolean)); + const txPromise = this.token.methods[this.function]( + ...[this.receiver.address, value, this.data, { from: this.owner }].filter(Boolean), + ); if (this.revert === undefined) { const { tx } = await txPromise; From 3c0df204f87dda32c3e3a1e833d6e7fbe00207a4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 23 Feb 2023 15:55:20 +0100 Subject: [PATCH 14/15] cleanup diff --- scripts/checks/inheritance-ordering.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checks/inheritance-ordering.js b/scripts/checks/inheritance-ordering.js index 620c322e3df..45c707e6fcd 100755 --- a/scripts/checks/inheritance-ordering.js +++ b/scripts/checks/inheritance-ordering.js @@ -13,7 +13,7 @@ for (const artifact of artifacts) { const linearized = []; for (const source in solcOutput.contracts) { - if (['/mocks/', '/presets/'].some(skip => source.includes(skip))) { + if (source.includes('/mocks/')) { continue; } From 561310c062c0a1a71e9df63c6c0e492bab87f5a5 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 23 Feb 2023 16:06:55 +0100 Subject: [PATCH 15/15] we actaully need this until 4.9 :/ --- scripts/checks/inheritance-ordering.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checks/inheritance-ordering.js b/scripts/checks/inheritance-ordering.js index 45c707e6fcd..620c322e3df 100755 --- a/scripts/checks/inheritance-ordering.js +++ b/scripts/checks/inheritance-ordering.js @@ -13,7 +13,7 @@ for (const artifact of artifacts) { const linearized = []; for (const source in solcOutput.contracts) { - if (source.includes('/mocks/')) { + if (['/mocks/', '/presets/'].some(skip => source.includes(skip))) { continue; }