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); + } + }); }); }); });