From 9a7c5ace54a1342758e132e7783a963b2a9e884b Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 10 Jun 2020 17:19:16 -0300 Subject: [PATCH 1/7] forwarder: move to separate dir and make fns external --- contracts/common/IForwarder.sol | 18 ------------------ contracts/forwarding/IForwarder.sol | 14 ++++++++++++++ .../{common => forwarding}/IForwarderFee.sol | 0 3 files changed, 14 insertions(+), 18 deletions(-) delete mode 100644 contracts/common/IForwarder.sol create mode 100644 contracts/forwarding/IForwarder.sol rename contracts/{common => forwarding}/IForwarderFee.sol (100%) diff --git a/contracts/common/IForwarder.sol b/contracts/common/IForwarder.sol deleted file mode 100644 index a54b8cfdc..000000000 --- a/contracts/common/IForwarder.sol +++ /dev/null @@ -1,18 +0,0 @@ -/* - * SPDX-License-Identifier: MIT - */ - -pragma solidity ^0.4.24; - - -interface IForwarder { - function isForwarder() external pure returns (bool); - - // TODO: this should be external - // See https://github.com/ethereum/solidity/issues/4832 - function canForward(address sender, bytes evmCallScript) public view returns (bool); - - // TODO: this should be external - // See https://github.com/ethereum/solidity/issues/4832 - function forward(bytes evmCallScript) public; -} diff --git a/contracts/forwarding/IForwarder.sol b/contracts/forwarding/IForwarder.sol new file mode 100644 index 000000000..6c48a7cc5 --- /dev/null +++ b/contracts/forwarding/IForwarder.sol @@ -0,0 +1,14 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + + +interface IForwarder { + function isForwarder() external pure returns (bool); + + function canForward(address sender, bytes evmCallScript) external view returns (bool); + + function forward(bytes evmCallScript) external; +} diff --git a/contracts/common/IForwarderFee.sol b/contracts/forwarding/IForwarderFee.sol similarity index 100% rename from contracts/common/IForwarderFee.sol rename to contracts/forwarding/IForwarderFee.sol From f5ac5f886aead66514cd5b6e302e53c7b0e8e791 Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 10 Jun 2020 17:19:39 -0300 Subject: [PATCH 2/7] forwarder: provide payable interface --- contracts/forwarding/IForwarderPayable.sol | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 contracts/forwarding/IForwarderPayable.sol diff --git a/contracts/forwarding/IForwarderPayable.sol b/contracts/forwarding/IForwarderPayable.sol new file mode 100644 index 000000000..d2ee38101 --- /dev/null +++ b/contracts/forwarding/IForwarderPayable.sol @@ -0,0 +1,16 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; + + +interface IForwarderPayable { + function isForwarder() external pure returns (bool); + + function canForward(address sender, bytes evmCallScript) external view returns (bool); + + function forward(bytes evmCallScript) external payable; +} From 886f149d14fd67af4683df3b4296a9b86679337d Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 10 Jun 2020 17:20:08 -0300 Subject: [PATCH 3/7] forwarder: provide forwarder interface with extra data --- contracts/forwarding/IForwarderV2.sol | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 contracts/forwarding/IForwarderV2.sol diff --git a/contracts/forwarding/IForwarderV2.sol b/contracts/forwarding/IForwarderV2.sol new file mode 100644 index 000000000..2f3a4ddb3 --- /dev/null +++ b/contracts/forwarding/IForwarderV2.sol @@ -0,0 +1,13 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarderPayable.sol"; + + +// TODO: Cannot inherit interfaces in Solidity 0.4.24 +contract IForwarderV2 is IForwarderPayable { + function forward(bytes evmCallScript, bytes context) external payable; +} From e38b46d2bd46f345db575f6f65543f3d89d7c38c Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Fri, 12 Jun 2020 11:24:44 -0300 Subject: [PATCH 4/7] forwarder: remove unused import --- contracts/forwarding/IForwarderPayable.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/forwarding/IForwarderPayable.sol b/contracts/forwarding/IForwarderPayable.sol index d2ee38101..ceb352a16 100644 --- a/contracts/forwarding/IForwarderPayable.sol +++ b/contracts/forwarding/IForwarderPayable.sol @@ -4,8 +4,6 @@ pragma solidity ^0.4.24; -import "./IForwarder.sol"; - interface IForwarderPayable { function isForwarder() external pure returns (bool); From 46f9559970482bfaae623ffa1fd071c41363344a Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 1 Jul 2020 14:44:40 -0300 Subject: [PATCH 5/7] forwarder: improve forwarders hierarchy --- contracts/forwarding/IForwarder.sol | 20 +++++++++--- contracts/forwarding/IForwarderPayable.sol | 14 --------- contracts/forwarding/IForwarderV2.sol | 13 -------- .../forwarding/IForwarderWithContext.sol | 31 +++++++++++++++++++ .../IForwarderWithContextPayable.sol | 31 +++++++++++++++++++ .../forwarding/IForwarderWithoutContext.sol | 30 ++++++++++++++++++ .../IForwarderWithoutContextPayable.sol | 30 ++++++++++++++++++ 7 files changed, 138 insertions(+), 31 deletions(-) delete mode 100644 contracts/forwarding/IForwarderPayable.sol delete mode 100644 contracts/forwarding/IForwarderV2.sol create mode 100644 contracts/forwarding/IForwarderWithContext.sol create mode 100644 contracts/forwarding/IForwarderWithContextPayable.sol create mode 100644 contracts/forwarding/IForwarderWithoutContext.sol create mode 100644 contracts/forwarding/IForwarderWithoutContextPayable.sol diff --git a/contracts/forwarding/IForwarder.sol b/contracts/forwarding/IForwarder.sol index 6c48a7cc5..6a6fffe41 100644 --- a/contracts/forwarding/IForwarder.sol +++ b/contracts/forwarding/IForwarder.sol @@ -5,10 +5,22 @@ pragma solidity ^0.4.24; +/** +* @title Forwarder interface requiring context information +* @dev This interface allows building a simple forwarding protocol. The main purpose is to be able to build +* forwarding chains that allow smart contracts execute EVM scripts in the context of other smart contracts. +* Similar to how delegate calls work but having the ability to chain multiple steps which may not necessarily be synchronous. +*/ interface IForwarder { - function isForwarder() external pure returns (bool); + /** + * @dev Forwarder interface requiring context information + * @return True if the forwarder can forward the given evm script for the requested sender, false otherwise + */ + function canForward(address sender, bytes evmScript) external view returns (bool); - function canForward(address sender, bytes evmCallScript) external view returns (bool); - - function forward(bytes evmCallScript) external; + /** + * @dev Tell the type identification number of the current forwarder + * @return Type identification number of the current forwarder + */ + function forwarderType() external pure returns (uint256 forwarderTypeId); } diff --git a/contracts/forwarding/IForwarderPayable.sol b/contracts/forwarding/IForwarderPayable.sol deleted file mode 100644 index ceb352a16..000000000 --- a/contracts/forwarding/IForwarderPayable.sol +++ /dev/null @@ -1,14 +0,0 @@ -/* - * SPDX-License-Identifier: MIT - */ - -pragma solidity ^0.4.24; - - -interface IForwarderPayable { - function isForwarder() external pure returns (bool); - - function canForward(address sender, bytes evmCallScript) external view returns (bool); - - function forward(bytes evmCallScript) external payable; -} diff --git a/contracts/forwarding/IForwarderV2.sol b/contracts/forwarding/IForwarderV2.sol deleted file mode 100644 index 2f3a4ddb3..000000000 --- a/contracts/forwarding/IForwarderV2.sol +++ /dev/null @@ -1,13 +0,0 @@ -/* - * SPDX-License-Identifier: MIT - */ - -pragma solidity ^0.4.24; - -import "./IForwarderPayable.sol"; - - -// TODO: Cannot inherit interfaces in Solidity 0.4.24 -contract IForwarderV2 is IForwarderPayable { - function forward(bytes evmCallScript, bytes context) external payable; -} diff --git a/contracts/forwarding/IForwarderWithContext.sol b/contracts/forwarding/IForwarderWithContext.sol new file mode 100644 index 000000000..875728cfd --- /dev/null +++ b/contracts/forwarding/IForwarderWithContext.sol @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; + + +/** +* @title Forwarder interface requiring context information +* @dev This forwarder interface enforces an additional piece of information attached to the action being forwarded +* with the purpose of allowing the sender to provide a detailed context for the forwarded action. +* Unlike `IForwarderWithContextPayable`, this forwarder interface allows receiving ETH on the forward entry point. +*/ +contract IForwarderWithContext is IForwarder { + uint256 internal constant FORWARDER_TYPE = 3; + + /** + * @dev Forward an EVM script with an attached context information + */ + function forward(bytes evmScript, bytes context) external; + + /** + * @dev Tell the type identification number of the current forwarder + * @return Always 3 - Forwarder type ID for the non-payable forwarder with context + */ + function forwarderType() external pure returns (uint256) { + return FORWARDER_TYPE; + } +} diff --git a/contracts/forwarding/IForwarderWithContextPayable.sol b/contracts/forwarding/IForwarderWithContextPayable.sol new file mode 100644 index 000000000..c8ca1cd62 --- /dev/null +++ b/contracts/forwarding/IForwarderWithContextPayable.sol @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; + + +/** +* @title Payable Forwarder interface requiring context information +* @dev This forwarder interface enforces an additional piece of information attached to the action being forwarded +* with the purpose of allowing the sender to provide a detailed context for the forwarded action. +* Unlike `IForwarderWithContext`, this forwarder interface allows receiving ETH on the forward entry point. +*/ +contract IForwarderWithContextPayable is IForwarder { + uint256 internal constant FORWARDER_TYPE = 4; + + /** + * @dev Forward an EVM script with an attached context information + */ + function forward(bytes evmScript, bytes context) external payable; + + /** + * @dev Tell the type identification number of the current forwarder + * @return Always 4 - Forwarder type ID for the payable forwarder with context + */ + function forwarderType() external pure returns (uint256) { + return FORWARDER_TYPE; + } +} diff --git a/contracts/forwarding/IForwarderWithoutContext.sol b/contracts/forwarding/IForwarderWithoutContext.sol new file mode 100644 index 000000000..1631b4155 --- /dev/null +++ b/contracts/forwarding/IForwarderWithoutContext.sol @@ -0,0 +1,30 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; + + +/** +* @title Forwarder interface +* @dev This forwarder interface does not support attaching a context information to the forwarded actions. +* Unlike `IForwarderWithoutContextPayable`, this forwarder interface does not allow receiving ETH on the forward entry point. +*/ +contract IForwarderWithoutContext is IForwarder { + uint256 internal constant FORWARDER_TYPE = 1; + + /** + * @dev Forward an EVM script + */ + function forward(bytes evmScript) external payable; + + /** + * @dev Tell the type identification number of the current forwarder + * @return Always 1 - Forwarder type ID for the non-payable forwarder without context + */ + function forwarderType() external pure returns (uint256) { + return FORWARDER_TYPE; + } +} diff --git a/contracts/forwarding/IForwarderWithoutContextPayable.sol b/contracts/forwarding/IForwarderWithoutContextPayable.sol new file mode 100644 index 000000000..182de659e --- /dev/null +++ b/contracts/forwarding/IForwarderWithoutContextPayable.sol @@ -0,0 +1,30 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; + + +/** +* @title Payable Forwarder interface +* @dev This forwarder interface does not support attaching a context information to the forwarded actions. +* Unlike `IForwarderWithoutContext`, this forwarder interface allows receiving ETH on the forward entry point. +*/ +contract IForwarderWithoutContextPayable is IForwarder { + uint256 internal constant FORWARDER_TYPE = 2; + + /** + * @dev Forward an EVM script + */ + function forward(bytes evmScript) external payable; + + /** + * @dev Tell the type identification number of the current forwarder + * @return Always 2 - Forwarder type ID for the payable forwarder without context + */ + function forwarderType() external pure returns (uint256) { + return FORWARDER_TYPE; + } +} From 2774a9fe67023307f33443e82cbbe855147a913d Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Thu, 2 Jul 2020 22:49:24 +0200 Subject: [PATCH 6/7] Forwarding: update forwarding interface names, add tests (#590) --- contracts/forwarding/IAbstractForwarder.sol | 43 +++++++ contracts/forwarding/IForwarder.sol | 24 ++-- contracts/forwarding/IForwarderFee.sol | 10 +- contracts/forwarding/IForwarderPayable.sol | 30 +++++ .../forwarding/IForwarderWithContext.sol | 20 ++- .../IForwarderWithContextPayable.sol | 25 ++-- .../forwarding/IForwarderWithoutContext.sol | 30 ----- .../IForwarderWithoutContextPayable.sol | 30 ----- .../test/mocks/forwarding/ForwardingMock.sol | 42 +++++++ .../test/tests/TestConversionHelpers.sol | 4 +- test/contracts/forwarding/forwarding.js | 116 ++++++++++++++++++ 11 files changed, 275 insertions(+), 99 deletions(-) create mode 100644 contracts/forwarding/IAbstractForwarder.sol create mode 100644 contracts/forwarding/IForwarderPayable.sol delete mode 100644 contracts/forwarding/IForwarderWithoutContext.sol delete mode 100644 contracts/forwarding/IForwarderWithoutContextPayable.sol create mode 100644 contracts/test/mocks/forwarding/ForwardingMock.sol create mode 100644 test/contracts/forwarding/forwarding.js diff --git a/contracts/forwarding/IAbstractForwarder.sol b/contracts/forwarding/IAbstractForwarder.sol new file mode 100644 index 000000000..4a1a8c17b --- /dev/null +++ b/contracts/forwarding/IAbstractForwarder.sol @@ -0,0 +1,43 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + + +/** +* @title Abstract forwarder interface +* @dev This is the base interface for all forwarders. +* Forwarding allows separately installed applications (smart contracts implementing the forwarding interface) to execute multi-step actions via EVM scripts. +* You should only support the forwarding interface if your "action step" is asynchronous (e.g. requiring a delay period or a voting period). +* Note: you should **NOT** directly inherit from this interface; see one of the other, non-abstract interfaces available. +*/ +contract IAbstractForwarder { + enum ForwarderType { + NOT_IMPLEMENTED, + NO_CONTEXT, + WITH_CONTEXT + } + + /** + * @dev Tell whether the proposed forwarding path (an EVM script) from the given sender is allowed. + * The implemented `forward()` method **MUST NOT** revert if `canForward()` returns true for the same parameters. + * @return True if the sender's proposed path is allowed + */ + function canForward(address sender, bytes evmScript) external view returns (bool); + + /** + * @dev Tell the forwarder type + * @return Forwarder type + */ + function forwarderType() external pure returns (ForwarderType); + + /** + * @dev Report whether the implementing app is a forwarder + * Required for backwards compatibility with aragonOS 4 + * @return Always true + */ + function isForwarder() external pure returns (bool) { + return true; + } +} diff --git a/contracts/forwarding/IForwarder.sol b/contracts/forwarding/IForwarder.sol index 6a6fffe41..a20579b01 100644 --- a/contracts/forwarding/IForwarder.sol +++ b/contracts/forwarding/IForwarder.sol @@ -4,23 +4,25 @@ pragma solidity ^0.4.24; +import "./IAbstractForwarder.sol"; + /** -* @title Forwarder interface requiring context information -* @dev This interface allows building a simple forwarding protocol. The main purpose is to be able to build -* forwarding chains that allow smart contracts execute EVM scripts in the context of other smart contracts. -* Similar to how delegate calls work but having the ability to chain multiple steps which may not necessarily be synchronous. +* @title Forwarder interface +* @dev This is the basic forwarder interface, that only supports forwarding an EVM script. +* It does not support forwarding additional context or receiving ETH; other interfaces are available to support those. */ -interface IForwarder { +contract IForwarder is IAbstractForwarder { /** - * @dev Forwarder interface requiring context information - * @return True if the forwarder can forward the given evm script for the requested sender, false otherwise + * @dev Forward an EVM script */ - function canForward(address sender, bytes evmScript) external view returns (bool); + function forward(bytes evmScript) external; /** - * @dev Tell the type identification number of the current forwarder - * @return Type identification number of the current forwarder + * @dev Tell the forwarder type + * @return Always 1 (ForwarderType.NO_CONTEXT) */ - function forwarderType() external pure returns (uint256 forwarderTypeId); + function forwarderType() external pure returns (ForwarderType) { + return ForwarderType.NO_CONTEXT; + } } diff --git a/contracts/forwarding/IForwarderFee.sol b/contracts/forwarding/IForwarderFee.sol index 0fecb3418..621816420 100644 --- a/contracts/forwarding/IForwarderFee.sol +++ b/contracts/forwarding/IForwarderFee.sol @@ -5,6 +5,14 @@ pragma solidity ^0.4.24; +/** +* @title Forwarder fee interface +* @dev Interface for declaring any fee requirements for the `forward()` function. +*/ interface IForwarderFee { - function forwardFee() external view returns (address, uint256); + /** + * @dev Provide details about the required fee token and amount + * @return Fee token and fee amount. If ETH, expects EtherTokenConstant. + */ + function forwardFee() external view returns (address feeToken, uint256 feeAmount); } diff --git a/contracts/forwarding/IForwarderPayable.sol b/contracts/forwarding/IForwarderPayable.sol new file mode 100644 index 000000000..e94fbfd8a --- /dev/null +++ b/contracts/forwarding/IForwarderPayable.sol @@ -0,0 +1,30 @@ +/* + * SPDX-License-Identifier: MIT + */ + +pragma solidity ^0.4.24; + +import "./IForwarder.sol"; +import "./IForwarderFee.sol"; + + +/** +* @title Payable forwarder interface +* @dev This is the basic forwarder interface, that only supports forwarding an EVM script. +* Unlike `IForwarder`, this interface allows `forward()` to receive ETH and thereby includes the IForwarderFee interface. +* It is **RECOMMENDED** that only apps requiring a payable `forward()` use this interface. +*/ +contract IForwarderPayable is IAbstractForwarder, IForwarderFee { + /** + * @dev Forward an EVM script + */ + function forward(bytes evmScript) external payable; + + /** + * @dev Tell the forwarder type + * @return Always 1 (ForwarderType.NO_CONTEXT) + */ + function forwarderType() external pure returns (ForwarderType) { + return ForwarderType.NO_CONTEXT; + } +} diff --git a/contracts/forwarding/IForwarderWithContext.sol b/contracts/forwarding/IForwarderWithContext.sol index 875728cfd..33e8c8074 100644 --- a/contracts/forwarding/IForwarderWithContext.sol +++ b/contracts/forwarding/IForwarderWithContext.sol @@ -4,28 +4,24 @@ pragma solidity ^0.4.24; -import "./IForwarder.sol"; +import "./IAbstractForwarder.sol"; /** * @title Forwarder interface requiring context information -* @dev This forwarder interface enforces an additional piece of information attached to the action being forwarded -* with the purpose of allowing the sender to provide a detailed context for the forwarded action. -* Unlike `IForwarderWithContextPayable`, this forwarder interface allows receiving ETH on the forward entry point. +* @dev This forwarder interface allows for additional context to be attached to the action by the sender. */ -contract IForwarderWithContext is IForwarder { - uint256 internal constant FORWARDER_TYPE = 3; - +contract IForwarderWithContext is IAbstractForwarder { /** - * @dev Forward an EVM script with an attached context information + * @dev Forward an EVM script with an attached context */ function forward(bytes evmScript, bytes context) external; /** - * @dev Tell the type identification number of the current forwarder - * @return Always 3 - Forwarder type ID for the non-payable forwarder with context + * @dev Tell the forwarder type + * @return Always 2 (ForwarderType.WITH_CONTEXT) */ - function forwarderType() external pure returns (uint256) { - return FORWARDER_TYPE; + function forwarderType() external pure returns (ForwarderType) { + return ForwarderType.WITH_CONTEXT; } } diff --git a/contracts/forwarding/IForwarderWithContextPayable.sol b/contracts/forwarding/IForwarderWithContextPayable.sol index c8ca1cd62..629c56f7a 100644 --- a/contracts/forwarding/IForwarderWithContextPayable.sol +++ b/contracts/forwarding/IForwarderWithContextPayable.sol @@ -4,28 +4,27 @@ pragma solidity ^0.4.24; -import "./IForwarder.sol"; +import "./IAbstractForwarder.sol"; +import "./IForwarderFee.sol"; /** -* @title Payable Forwarder interface requiring context information -* @dev This forwarder interface enforces an additional piece of information attached to the action being forwarded -* with the purpose of allowing the sender to provide a detailed context for the forwarded action. -* Unlike `IForwarderWithContext`, this forwarder interface allows receiving ETH on the forward entry point. +* @title Payable forwarder interface requiring context information +* @dev This forwarder interface allows for additional context to be attached to the action by the sender. +* Unlike `IForwarderWithContext`, this interface allows `forward()` to receive ETH and thereby includes the IForwarderFee interface. +* It is **RECOMMENDED** that only apps requiring a payable `forward()` use this interface. */ -contract IForwarderWithContextPayable is IForwarder { - uint256 internal constant FORWARDER_TYPE = 4; - +contract IForwarderWithContextPayable is IAbstractForwarder, IForwarderFee { /** - * @dev Forward an EVM script with an attached context information + * @dev Forward an EVM script with an attached context */ function forward(bytes evmScript, bytes context) external payable; /** - * @dev Tell the type identification number of the current forwarder - * @return Always 4 - Forwarder type ID for the payable forwarder with context + * @dev Tell the forwarder type + * @return Always 2 (ForwarderType.WITH_CONTEXT) */ - function forwarderType() external pure returns (uint256) { - return FORWARDER_TYPE; + function forwarderType() external pure returns (ForwarderType) { + return ForwarderType.WITH_CONTEXT; } } diff --git a/contracts/forwarding/IForwarderWithoutContext.sol b/contracts/forwarding/IForwarderWithoutContext.sol deleted file mode 100644 index 1631b4155..000000000 --- a/contracts/forwarding/IForwarderWithoutContext.sol +++ /dev/null @@ -1,30 +0,0 @@ -/* - * SPDX-License-Identifier: MIT - */ - -pragma solidity ^0.4.24; - -import "./IForwarder.sol"; - - -/** -* @title Forwarder interface -* @dev This forwarder interface does not support attaching a context information to the forwarded actions. -* Unlike `IForwarderWithoutContextPayable`, this forwarder interface does not allow receiving ETH on the forward entry point. -*/ -contract IForwarderWithoutContext is IForwarder { - uint256 internal constant FORWARDER_TYPE = 1; - - /** - * @dev Forward an EVM script - */ - function forward(bytes evmScript) external payable; - - /** - * @dev Tell the type identification number of the current forwarder - * @return Always 1 - Forwarder type ID for the non-payable forwarder without context - */ - function forwarderType() external pure returns (uint256) { - return FORWARDER_TYPE; - } -} diff --git a/contracts/forwarding/IForwarderWithoutContextPayable.sol b/contracts/forwarding/IForwarderWithoutContextPayable.sol deleted file mode 100644 index 182de659e..000000000 --- a/contracts/forwarding/IForwarderWithoutContextPayable.sol +++ /dev/null @@ -1,30 +0,0 @@ -/* - * SPDX-License-Identifier: MIT - */ - -pragma solidity ^0.4.24; - -import "./IForwarder.sol"; - - -/** -* @title Payable Forwarder interface -* @dev This forwarder interface does not support attaching a context information to the forwarded actions. -* Unlike `IForwarderWithoutContext`, this forwarder interface allows receiving ETH on the forward entry point. -*/ -contract IForwarderWithoutContextPayable is IForwarder { - uint256 internal constant FORWARDER_TYPE = 2; - - /** - * @dev Forward an EVM script - */ - function forward(bytes evmScript) external payable; - - /** - * @dev Tell the type identification number of the current forwarder - * @return Always 2 - Forwarder type ID for the payable forwarder without context - */ - function forwarderType() external pure returns (uint256) { - return FORWARDER_TYPE; - } -} diff --git a/contracts/test/mocks/forwarding/ForwardingMock.sol b/contracts/test/mocks/forwarding/ForwardingMock.sol new file mode 100644 index 000000000..6bd25f6fd --- /dev/null +++ b/contracts/test/mocks/forwarding/ForwardingMock.sol @@ -0,0 +1,42 @@ +pragma solidity 0.4.24; + +import "../../../forwarding/IAbstractForwarder.sol"; +import "../../../forwarding/IForwarderFee.sol"; +import "../../../forwarding/IForwarder.sol"; +import "../../../forwarding/IForwarderPayable.sol"; +import "../../../forwarding/IForwarderWithContext.sol"; +import "../../../forwarding/IForwarderWithContextPayable.sol"; + + +contract BaseForwarderMock is IAbstractForwarder { + function canForward(address, bytes) external view returns (bool) { + return true; + } +} + + +contract BaseForwarderPayableMock is BaseForwarderMock, IForwarderFee { + function forwardFee() external view returns (address, uint256) { + return (address(0), 0); + } +} + + +contract ForwarderMock is BaseForwarderMock, IForwarder { + function forward(bytes) external { } +} + + +contract ForwarderPayableMock is BaseForwarderPayableMock, IForwarderPayable { + function forward(bytes) external payable { } +} + + +contract ForwarderWithContextMock is BaseForwarderMock, IForwarderWithContext { + function forward(bytes, bytes) external { } +} + + +contract ForwarderWithContextPayableMock is BaseForwarderPayableMock, IForwarderWithContextPayable { + function forward(bytes, bytes) external payable { } +} diff --git a/contracts/test/tests/TestConversionHelpers.sol b/contracts/test/tests/TestConversionHelpers.sol index ea7da4c6f..ed3612502 100644 --- a/contracts/test/tests/TestConversionHelpers.sol +++ b/contracts/test/tests/TestConversionHelpers.sol @@ -7,11 +7,11 @@ import "../../common/ConversionHelpers.sol"; contract InvalidBytesLengthConversionThrows { - function tryConvertLength(uint256 _badLength) public { + function tryConvertLength(uint256 _badLength) public pure { bytes memory arr = new bytes(_badLength); // Do failing conversion - uint256[] memory arrUint = ConversionHelpers.dangerouslyCastBytesToUintArray(arr); + ConversionHelpers.dangerouslyCastBytesToUintArray(arr); } } diff --git a/test/contracts/forwarding/forwarding.js b/test/contracts/forwarding/forwarding.js new file mode 100644 index 000000000..2f89722ba --- /dev/null +++ b/test/contracts/forwarding/forwarding.js @@ -0,0 +1,116 @@ +const { assertRevert } = require('../../helpers/assertThrow') + +// Mocks +const ForwarderMock = artifacts.require('ForwarderMock') +const ForwarderPayableMock = artifacts.require('ForwarderPayableMock') +const ForwarderWithContextMock = artifacts.require('ForwarderWithContextMock') +const ForwarderWithContextPayableMock = artifacts.require('ForwarderWithContextPayableMock') + +const EMPTY_BYTES = '0x' +const ForwarderType = { + NOT_IMPLEMENTED: 0, + NO_CONTEXT: 1, + WITH_CONTEXT: 2, +} + +contract('Forwarders', () => { + context('IForwarder', () => { + let forwarder + + beforeEach(async () => { + forwarder = await ForwarderMock.new() + }) + + it('is a forwarder', async () => { + assert.isTrue(await forwarder.isForwarder(), 'should be a forwarder') + }) + + it('reports correct forwarder type', async () => { + assert.equal(await forwarder.forwarderType(), ForwarderType.NO_CONTEXT, 'should report correct forwarding type') + }) + + it('can forward', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES)) + }) + + it('cannot forward with ETH payment', async () => { + // Override the contract ABI to let us attempt sending value into this non-payable forwarder + const payableForwarder = ForwarderPayableMock.at(forwarder.address) + await assertRevert(payableForwarder.forward(EMPTY_BYTES, { value: 1 })) + }) + }) + + context('IForwarderPayable', () => { + let forwarder + + beforeEach(async () => { + forwarder = await ForwarderPayableMock.new() + }) + + it('is a forwarder', async () => { + assert.isTrue(await forwarder.isForwarder(), 'should be a forwarder') + }) + + it('reports correct forwarder type', async () => { + assert.equal(await forwarder.forwarderType(), ForwarderType.NO_CONTEXT, 'should report correct forwarding type') + }) + + it('can forward', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES)) + }) + + it('can forward with ETH payment', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES, { value: 1 })) + }) + }) + + context('IForwarderWithContext', () => { + let forwarder + + beforeEach(async () => { + forwarder = await ForwarderWithContextMock.new() + }) + + it('is a forwarder', async () => { + assert.isTrue(await forwarder.isForwarder(), 'should be a forwarder') + }) + + it('reports correct forwarder type', async () => { + assert.equal(await forwarder.forwarderType(), ForwarderType.WITH_CONTEXT, 'should report correct forwarding type') + }) + + it('can forward', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES, EMPTY_BYTES)) + }) + + it('cannot forward with ETH payment', async () => { + // Override the contract ABI to let us attempt sending value into this non-payable forwarder + const payableForwarder = ForwarderWithContextPayableMock.at(forwarder.address) + await assertRevert(payableForwarder.forward(EMPTY_BYTES, EMPTY_BYTES, { value: 1 })) + }) + }) + + context('IForwarderWithContextPayable', () => { + let forwarder + + beforeEach(async () => { + forwarder = await ForwarderWithContextPayableMock.new() + }) + + it('is a forwarder', async () => { + assert.isTrue(await forwarder.isForwarder(), 'should be a forwarder') + }) + + it('reports correct forwarder type', async () => { + assert.equal(await forwarder.forwarderType(), ForwarderType.WITH_CONTEXT, 'should report correct forwarding type') + }) + + it('can forward', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES, EMPTY_BYTES)) + }) + + it('can forward with ETH payment', async () => { + assert.doesNotThrow(async () => await forwarder.forward(EMPTY_BYTES, EMPTY_BYTES, { value: 1 })) + }) + }) +}) From e9059475a0931ae6683f24a0dd2ac5f86bd8f9e2 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Thu, 2 Jul 2020 22:59:33 +0200 Subject: [PATCH 7/7] tests: remove wrong (and unused) test import --- contracts/test/mocks/apps/disputable/DisputableAppMock.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/test/mocks/apps/disputable/DisputableAppMock.sol b/contracts/test/mocks/apps/disputable/DisputableAppMock.sol index 4e8d10ba1..9a96655d9 100644 --- a/contracts/test/mocks/apps/disputable/DisputableAppMock.sol +++ b/contracts/test/mocks/apps/disputable/DisputableAppMock.sol @@ -1,6 +1,5 @@ pragma solidity 0.4.24; -import "../../../../common/IForwarder.sol"; import "../../../../apps/disputable/DisputableAragonApp.sol";