From bc7b177ccd2d26ec78e2c6fce11a6d68a50b9e67 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 16:24:53 +0200 Subject: [PATCH 01/11] min changes for new functionality --- v2/contracts/evm/GatewayEVM.sol | 18 ++++++++++++++---- v2/contracts/evm/interfaces/IGatewayEVM.sol | 12 ++++++++++++ v2/contracts/zevm/GatewayZEVM.sol | 10 +++++----- v2/contracts/zevm/interfaces/IGatewayZEVM.sol | 9 +++++++-- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 18a176696..b007d970f 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.26; import { RevertContext, RevertOptions, Revertable } from "../../contracts/Revert.sol"; import { ZetaConnectorBase } from "./ZetaConnectorBase.sol"; import { IERC20Custody } from "./interfaces/IERC20Custody.sol"; -import { IGatewayEVM } from "./interfaces/IGatewayEVM.sol"; +import { IGatewayEVM, Callable, MessageContext } from "./interfaces/IGatewayEVM.sol"; import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; @@ -75,13 +75,17 @@ contract GatewayEVM is /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. - function _execute(address destination, bytes calldata data) internal returns (bytes memory) { + function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { (bool success, bytes memory result) = destination.call{ value: msg.value }(data); if (!success) revert ExecutionFailed(); return result; } + function _executeAuthenticatedCall(MessageContext calldata messageContext, address destination, bytes calldata data) internal returns (bytes memory) { + return Callable(destination).onCall(messageContext, data); + } + /// @notice Pause contract. function pause() external onlyRole(PAUSER_ROLE) { _pause(); @@ -121,6 +125,7 @@ contract GatewayEVM is /// @param data Calldata to pass to the call. /// @return The result of the call. function execute( + MessageContext calldata messageContext, address destination, bytes calldata data ) @@ -132,7 +137,12 @@ contract GatewayEVM is returns (bytes memory) { if (destination == address(0)) revert ZeroAddress(); - bytes memory result = _execute(destination, data); + bytes memory result; + if (messageContext.isArbitraryCall) { + result = _executeArbitraryCall(destination, data); + } else { + result = _executeAuthenticatedCall(messageContext, destination, data); + } emit Executed(destination, msg.value, data); @@ -163,7 +173,7 @@ contract GatewayEVM is if (!resetApproval(token, to)) revert ApprovalFailed(); if (!IERC20(token).approve(to, amount)) revert ApprovalFailed(); // Execute the call on the target contract - _execute(to, data); + _executeArbitraryCall(to, data); // Reset approval if (!resetApproval(token, to)) revert ApprovalFailed(); diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index ff91df1fa..626f0d927 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -171,3 +171,15 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { /// @param revertOptions Revert options. function call(address receiver, bytes calldata payload, RevertOptions calldata revertOptions) external; } + +struct MessageContext { + address sender; + bool isArbitraryCall; +} + +interface Callable { + function onCall( + MessageContext calldata messageContext, + bytes calldata message + ) external returns (bytes memory); +} \ No newline at end of file diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index a00186bbd..a38b1a795 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import { IGatewayZEVM } from "./interfaces/IGatewayZEVM.sol"; +import { IGatewayZEVM, CallOptions } from "./interfaces/IGatewayZEVM.sol"; import { RevertContext, RevertOptions } from "../../contracts/Revert.sol"; import "./interfaces/IWZETA.sol"; @@ -242,13 +242,13 @@ contract GatewayZEVM is /// @param receiver The receiver address on the external chain. /// @param zrc20 Address of zrc20 to pay fees. /// @param message The calldata to pass to the contract call. - /// @param gasLimit Gas limit. + /// @param callOptions Call options including gas limit and arbirtrary call flag. /// @param revertOptions Revert options. function call( bytes memory receiver, address zrc20, bytes calldata message, - uint256 gasLimit, + CallOptions calldata callOptions, RevertOptions calldata revertOptions ) external @@ -258,12 +258,12 @@ contract GatewayZEVM is if (receiver.length == 0) revert ZeroAddress(); if (message.length == 0) revert EmptyMessage(); - (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); + (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(callOptions.gasLimit); if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) { revert GasFeeTransferFailed(); } - emit Called(msg.sender, zrc20, receiver, message, gasLimit, revertOptions); + emit Called(msg.sender, zrc20, receiver, message, callOptions, revertOptions); } /// @notice Deposit foreign coins into ZRC20. diff --git a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol index 0cbb0195b..06a4ab430 100644 --- a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol +++ b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol @@ -12,14 +12,14 @@ interface IGatewayZEVMEvents { /// @param zrc20 Address of zrc20 to pay fees. /// @param receiver The receiver address on the external chain. /// @param message The calldata passed to the contract call. - /// @param gasLimit Gas limit. + /// @param callOptions Call options including gas limit and arbirtrary call flag. /// @param revertOptions Revert options. event Called( address indexed sender, address indexed zrc20, bytes receiver, bytes message, - uint256 gasLimit, + CallOptions callOptions, RevertOptions revertOptions ); @@ -231,3 +231,8 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { ) external; } + +struct CallOptions { + uint256 gasLimit; + bool isArbitraryCall; +} \ No newline at end of file From 9a81d8acdfeb1200503c924f2d050349d0e5343f Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 16:36:45 +0200 Subject: [PATCH 02/11] simplify interface --- v2/contracts/evm/GatewayEVM.sol | 2 +- v2/contracts/evm/interfaces/IGatewayEVM.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index b007d970f..a4959af33 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -83,7 +83,7 @@ contract GatewayEVM is } function _executeAuthenticatedCall(MessageContext calldata messageContext, address destination, bytes calldata data) internal returns (bytes memory) { - return Callable(destination).onCall(messageContext, data); + return Callable(destination).onCall(messageContext.sender, data); } /// @notice Pause contract. diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 626f0d927..61e39a4ff 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -109,7 +109,7 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { /// @param destination The address of the contract to call. /// @param data The calldata to pass to the contract call. /// @return The result of the contract call. - function execute(address destination, bytes calldata data) external payable returns (bytes memory); + function execute(MessageContext calldata messageContext, address destination, bytes calldata data) external payable returns (bytes memory); /// @notice Executes a revertable call to a contract using ERC20 tokens. /// @param token The address of the ERC20 token. @@ -179,7 +179,7 @@ struct MessageContext { interface Callable { function onCall( - MessageContext calldata messageContext, + address sender, bytes calldata message ) external returns (bytes memory); } \ No newline at end of file From 5ac4847c4ef8f91cb6331deba43b3f611b6e7ac9 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 22:14:10 +0200 Subject: [PATCH 03/11] fixes so tests pass --- v2/contracts/evm/GatewayEVM.sol | 29 ++++++++++++- v2/contracts/evm/interfaces/IGatewayEVM.sol | 16 +++++--- v2/contracts/zevm/GatewayZEVM.sol | 24 ++++++++++- v2/contracts/zevm/interfaces/IGatewayZEVM.sol | 13 +++++- v2/test/GatewayEVMZEVM.t.sol | 12 ++++-- v2/test/GatewayZEVM.t.sol | 9 +++- v2/test/utils/GatewayEVMUpgradeTest.sol | 41 +++++++++++++++++-- 7 files changed, 127 insertions(+), 17 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index a4959af33..0a298d157 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.26; import { RevertContext, RevertOptions, Revertable } from "../../contracts/Revert.sol"; import { ZetaConnectorBase } from "./ZetaConnectorBase.sol"; import { IERC20Custody } from "./interfaces/IERC20Custody.sol"; -import { IGatewayEVM, Callable, MessageContext } from "./interfaces/IGatewayEVM.sol"; +import { Callable, IGatewayEVM, MessageContext } from "./interfaces/IGatewayEVM.sol"; import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; @@ -82,7 +82,14 @@ contract GatewayEVM is return result; } - function _executeAuthenticatedCall(MessageContext calldata messageContext, address destination, bytes calldata data) internal returns (bytes memory) { + function _executeAuthenticatedCall( + MessageContext calldata messageContext, + address destination, + bytes calldata data + ) + internal + returns (bytes memory) + { return Callable(destination).onCall(messageContext.sender, data); } @@ -149,6 +156,24 @@ contract GatewayEVM is return result; } + function execute( + address destination, + bytes calldata data + ) + external + payable + onlyRole(TSS_ROLE) + whenNotPaused + returns (bytes memory) + { + if (destination == address(0)) revert ZeroAddress(); + bytes memory result = _executeArbitraryCall(destination, data); + + emit Executed(destination, msg.value, data); + + return result; + } + /// @notice Executes a call to a destination contract using ERC20 tokens. /// @dev This function can only be called by the custody or connector address. /// It uses the ERC20 allowance system, resetting gateway allowance at the end. diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 61e39a4ff..13440ba3b 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -109,7 +109,16 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { /// @param destination The address of the contract to call. /// @param data The calldata to pass to the contract call. /// @return The result of the contract call. - function execute(MessageContext calldata messageContext, address destination, bytes calldata data) external payable returns (bytes memory); + function execute(address destination, bytes calldata data) external payable returns (bytes memory); + + function execute( + MessageContext calldata messageContext, + address destination, + bytes calldata data + ) + external + payable + returns (bytes memory); /// @notice Executes a revertable call to a contract using ERC20 tokens. /// @param token The address of the ERC20 token. @@ -178,8 +187,5 @@ struct MessageContext { } interface Callable { - function onCall( - address sender, - bytes calldata message - ) external returns (bytes memory); + function onCall(address sender, bytes calldata message) external returns (bytes memory); } \ No newline at end of file diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index a38b1a795..6968c4e90 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import { IGatewayZEVM, CallOptions } from "./interfaces/IGatewayZEVM.sol"; +import { CallOptions, IGatewayZEVM } from "./interfaces/IGatewayZEVM.sol"; import { RevertContext, RevertOptions } from "../../contracts/Revert.sol"; import "./interfaces/IWZETA.sol"; @@ -266,6 +266,28 @@ contract GatewayZEVM is emit Called(msg.sender, zrc20, receiver, message, callOptions, revertOptions); } + function call( + bytes memory receiver, + address zrc20, + bytes calldata message, + uint256 gasLimit, + RevertOptions calldata revertOptions + ) + external + nonReentrant + whenNotPaused + { + if (receiver.length == 0) revert ZeroAddress(); + if (message.length == 0) revert EmptyMessage(); + + (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); + if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) { + revert GasFeeTransferFailed(); + } + + emit Called(msg.sender, zrc20, receiver, message, CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), revertOptions); + } + /// @notice Deposit foreign coins into ZRC20. /// @param zrc20 The address of the ZRC20 token. /// @param amount The amount of tokens to deposit. diff --git a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol index 06a4ab430..9e440c73c 100644 --- a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol +++ b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol @@ -153,8 +153,17 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { /// @param receiver The receiver address on the external chain. /// @param zrc20 Address of zrc20 to pay fees. /// @param message The calldata to pass to the contract call. - /// @param gasLimit Gas limit. + /// @param callOptions Call options including gas limit and arbirtrary call flag. /// @param revertOptions Revert options. + function call( + bytes memory receiver, + address zrc20, + bytes calldata message, + CallOptions calldata callOptions, + RevertOptions calldata revertOptions + ) + external; + function call( bytes memory receiver, address zrc20, @@ -235,4 +244,4 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { struct CallOptions { uint256 gasLimit; bool isArbitraryCall; -} \ No newline at end of file +} diff --git a/v2/test/GatewayEVMZEVM.t.sol b/v2/test/GatewayEVMZEVM.t.sol index a81ab8e3e..64a73b796 100644 --- a/v2/test/GatewayEVMZEVM.t.sol +++ b/v2/test/GatewayEVMZEVM.t.sol @@ -17,8 +17,7 @@ import "./utils/SystemContractMock.sol"; import { GatewayZEVM } from "../contracts/zevm/GatewayZEVM.sol"; import { IGatewayZEVM } from "../contracts/zevm/GatewayZEVM.sol"; -import { IGatewayZEVMErrors } from "../contracts/zevm/interfaces/IGatewayZEVM.sol"; -import { IGatewayZEVMEvents } from "../contracts/zevm/interfaces/IGatewayZEVM.sol"; +import { CallOptions, IGatewayZEVMErrors, IGatewayZEVMEvents } from "../contracts/zevm/interfaces/IGatewayZEVM.sol"; import { IGatewayEVMErrors } from "../contracts/evm/interfaces/IGatewayEVM.sol"; import { IGatewayEVMEvents } from "../contracts/evm/interfaces/IGatewayEVM.sol"; @@ -135,7 +134,14 @@ contract GatewayEVMZEVMTest is bytes memory message = abi.encodeWithSelector(receiverEVM.receivePayable.selector, str, num, flag); vm.prank(ownerZEVM); vm.expectEmit(true, true, true, true, address(gatewayZEVM)); - emit Called(address(ownerZEVM), address(zrc20), abi.encodePacked(receiverEVM), message, 1, revertOptions); + emit Called( + address(ownerZEVM), + address(zrc20), + abi.encodePacked(receiverEVM), + message, + CallOptions({ gasLimit: 1, isArbitraryCall: true }), + revertOptions + ); gatewayZEVM.call(abi.encodePacked(receiverEVM), address(zrc20), message, 1, revertOptions); // Call execute on evm diff --git a/v2/test/GatewayZEVM.t.sol b/v2/test/GatewayZEVM.t.sol index ee91ab4db..3c2bc1bc8 100644 --- a/v2/test/GatewayZEVM.t.sol +++ b/v2/test/GatewayZEVM.t.sol @@ -334,7 +334,14 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectEmit(true, true, true, true, address(gateway)); - emit Called(owner, address(zrc20), abi.encodePacked(addr1), message, 1, revertOptions); + emit Called( + owner, + address(zrc20), + abi.encodePacked(addr1), + message, + CallOptions({ gasLimit: 1, isArbitraryCall: true }), + revertOptions + ); gateway.call(abi.encodePacked(addr1), address(zrc20), message, 1, revertOptions); } } diff --git a/v2/test/utils/GatewayEVMUpgradeTest.sol b/v2/test/utils/GatewayEVMUpgradeTest.sol index 83a9b8f57..84150cc39 100644 --- a/v2/test/utils/GatewayEVMUpgradeTest.sol +++ b/v2/test/utils/GatewayEVMUpgradeTest.sol @@ -79,13 +79,24 @@ contract GatewayEVMUpgradeTest is /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. - function _execute(address destination, bytes calldata data) internal returns (bytes memory) { + function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { (bool success, bytes memory result) = destination.call{ value: msg.value }(data); if (!success) revert ExecutionFailed(); return result; } + function _executeAuthenticatedCall( + MessageContext calldata messageContext, + address destination, + bytes calldata data + ) + internal + returns (bytes memory) + { + return Callable(destination).onCall(messageContext.sender, data); + } + /// @notice Pause contract. function pause() external onlyRole(PAUSER_ROLE) { _pause(); @@ -125,6 +136,7 @@ contract GatewayEVMUpgradeTest is /// @param data Calldata to pass to the call. /// @return The result of the call. function execute( + MessageContext calldata messageContext, address destination, bytes calldata data ) @@ -136,7 +148,30 @@ contract GatewayEVMUpgradeTest is returns (bytes memory) { if (destination == address(0)) revert ZeroAddress(); - bytes memory result = _execute(destination, data); + bytes memory result; + if (messageContext.isArbitraryCall) { + result = _executeArbitraryCall(destination, data); + } else { + result = _executeAuthenticatedCall(messageContext, destination, data); + } + + emit Executed(destination, msg.value, data); + + return result; + } + + function execute( + address destination, + bytes calldata data + ) + external + payable + onlyRole(TSS_ROLE) + whenNotPaused + returns (bytes memory) + { + if (destination == address(0)) revert ZeroAddress(); + bytes memory result = _executeArbitraryCall(destination, data); emit ExecutedV2(destination, msg.value, data); @@ -167,7 +202,7 @@ contract GatewayEVMUpgradeTest is if (!resetApproval(token, to)) revert ApprovalFailed(); if (!IERC20(token).approve(to, amount)) revert ApprovalFailed(); // Execute the call on the target contract - _execute(to, data); + _executeArbitraryCall(to, data); // Reset approval if (!resetApproval(token, to)) revert ApprovalFailed(); From b40d43ab3dae7743f772e5d0ddb6a1a321901ae1 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 22:14:41 +0200 Subject: [PATCH 04/11] prevent calling onCall in execute arb call --- v2/contracts/evm/GatewayEVM.sol | 10 ++++++++++ v2/contracts/evm/interfaces/IGatewayEVM.sol | 2 ++ v2/contracts/zevm/GatewayZEVM.sol | 9 ++++++++- v2/test/GatewayEVM.t.sol | 8 ++++++++ v2/test/utils/IReceiverEVM.sol | 2 ++ v2/test/utils/ReceiverEVM.sol | 4 ++++ 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 0a298d157..417bb50e4 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -76,6 +76,16 @@ contract GatewayEVM is /// @param data Calldata to pass to the call. /// @return The result of the call. function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { + if (data.length >= 4) { + bytes4 functionSelector; + assembly { + functionSelector := calldataload(data.offset) + } + + if (functionSelector == Callable.onCall.selector) { + revert NotAllowedToCallOnCall(); + } + } (bool success, bytes memory result) = destination.call{ value: msg.value }(data); if (!success) revert ExecutionFailed(); diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 13440ba3b..176438d0d 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -80,6 +80,8 @@ interface IGatewayEVMErrors { /// @notice Error when trying to transfer not whitelisted token to custody. error NotWhitelistedInCustody(); + + error NotAllowedToCallOnCall(); } /// @title IGatewayEVM diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 6968c4e90..20e500d93 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -285,7 +285,14 @@ contract GatewayZEVM is revert GasFeeTransferFailed(); } - emit Called(msg.sender, zrc20, receiver, message, CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), revertOptions); + emit Called( + msg.sender, + zrc20, + receiver, + message, + CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), + revertOptions + ); } /// @notice Deposit foreign coins into ZRC20. diff --git a/v2/test/GatewayEVM.t.sol b/v2/test/GatewayEVM.t.sol index 11932ee44..f7a24b4b4 100644 --- a/v2/test/GatewayEVM.t.sol +++ b/v2/test/GatewayEVM.t.sol @@ -182,6 +182,14 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } + function testForwardCallToReceiveOnCallFails() public { + bytes memory data = abi.encodeWithSignature("onCall(address,bytes)", address(123), bytes("")); + + vm.prank(tssAddress); + vm.expectRevert(NotAllowedToCallOnCall.selector); + gateway.execute(address(receiver), data); + } + function testExecuteFailsIfDestinationIsZeroAddress() public { bytes memory data = abi.encodeWithSignature("receiveNoParams()"); diff --git a/v2/test/utils/IReceiverEVM.sol b/v2/test/utils/IReceiverEVM.sol index 02d3f381c..d08c7dfd2 100644 --- a/v2/test/utils/IReceiverEVM.sol +++ b/v2/test/utils/IReceiverEVM.sol @@ -36,4 +36,6 @@ interface IReceiverEVMEvents { /// @param sender The address of the sender. /// @param revertContext Revert Context. event ReceivedRevert(address sender, RevertContext revertContext); + + event ReceivedOnCall(); } diff --git a/v2/test/utils/ReceiverEVM.sol b/v2/test/utils/ReceiverEVM.sol index 3d05ab2f5..61557527d 100644 --- a/v2/test/utils/ReceiverEVM.sol +++ b/v2/test/utils/ReceiverEVM.sol @@ -72,6 +72,10 @@ contract ReceiverEVM is IReceiverEVMEvents, ReentrancyGuard { emit ReceivedRevert(msg.sender, revertContext); } + function onCall(address sender, bytes calldata message) external returns (bytes memory) { + emit ReceivedOnCall(); + } + /// @notice Receives ETH. receive() external payable { } From 27e58e7f837ec5be665781c4356c755fa97ce6e2 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 22:23:49 +0200 Subject: [PATCH 05/11] refactor a bit --- v2/contracts/evm/interfaces/IGatewayEVM.sol | 2 +- v2/contracts/zevm/GatewayZEVM.sol | 33 ++++++++++----------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 176438d0d..4a5454109 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -190,4 +190,4 @@ struct MessageContext { interface Callable { function onCall(address sender, bytes calldata message) external returns (bytes memory); -} \ No newline at end of file +} diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 20e500d93..5a8f09734 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -255,15 +255,7 @@ contract GatewayZEVM is nonReentrant whenNotPaused { - if (receiver.length == 0) revert ZeroAddress(); - if (message.length == 0) revert EmptyMessage(); - - (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(callOptions.gasLimit); - if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) { - revert GasFeeTransferFailed(); - } - - emit Called(msg.sender, zrc20, receiver, message, callOptions, revertOptions); + _call(receiver, zrc20, message, callOptions, revertOptions); } function call( @@ -276,23 +268,28 @@ contract GatewayZEVM is external nonReentrant whenNotPaused + { + _call(receiver, zrc20, message, CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), revertOptions); + } + + function _call( + bytes memory receiver, + address zrc20, + bytes calldata message, + CallOptions memory callOptions, + RevertOptions memory revertOptions + ) + internal { if (receiver.length == 0) revert ZeroAddress(); if (message.length == 0) revert EmptyMessage(); - (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); + (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(callOptions.gasLimit); if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) { revert GasFeeTransferFailed(); } - emit Called( - msg.sender, - zrc20, - receiver, - message, - CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), - revertOptions - ); + emit Called(msg.sender, zrc20, receiver, message, callOptions, revertOptions); } /// @notice Deposit foreign coins into ZRC20. From d05288d0936354d67adc34e73e1ba5b701d6da71 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 12 Sep 2024 23:26:14 +0200 Subject: [PATCH 06/11] add overloads for withdrawAndcall functions --- v2/contracts/zevm/GatewayZEVM.sol | 81 ++++++++++++++++++- v2/contracts/zevm/interfaces/IGatewayZEVM.sol | 4 +- v2/test/GatewayEVMZEVM.t.sol | 11 ++- v2/test/GatewayZEVM.t.sol | 28 ++++++- 4 files changed, 112 insertions(+), 12 deletions(-) diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 5a8f09734..81e06976b 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -151,7 +151,7 @@ contract GatewayZEVM is gasFee, IZRC20(zrc20).PROTOCOL_FLAT_FEE(), "", - IZRC20(zrc20).GAS_LIMIT(), + CallOptions({ gasLimit: IZRC20(zrc20).GAS_LIMIT(), isArbitraryCall: true }), revertOptions ); } @@ -188,7 +188,37 @@ contract GatewayZEVM is gasFee, IZRC20(zrc20).PROTOCOL_FLAT_FEE(), message, - gasLimit, + CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), + revertOptions + ); + } + + function withdrawAndCall( + bytes memory receiver, + uint256 amount, + address zrc20, + bytes calldata message, + CallOptions calldata callOptions, + RevertOptions calldata revertOptions + ) + external + nonReentrant + whenNotPaused + { + if (receiver.length == 0) revert ZeroAddress(); + if (amount == 0) revert InsufficientZRC20Amount(); + + uint256 gasFee = _withdrawZRC20WithGasLimit(amount, zrc20, callOptions.gasLimit); + emit Withdrawn( + msg.sender, + 0, + receiver, + zrc20, + amount, + gasFee, + IZRC20(zrc20).PROTOCOL_FLAT_FEE(), + message, + callOptions, revertOptions ); } @@ -211,7 +241,18 @@ contract GatewayZEVM is if (amount == 0) revert InsufficientZetaAmount(); _transferZETA(amount, FUNGIBLE_MODULE_ADDRESS); - emit Withdrawn(msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, "", 0, revertOptions); + emit Withdrawn( + msg.sender, + chainId, + receiver, + address(zetaToken), + amount, + 0, + 0, + "", + CallOptions({ gasLimit: 0, isArbitraryCall: true }), + revertOptions + ); } /// @notice Withdraw ZETA tokens and call a smart contract on an external chain. @@ -235,7 +276,39 @@ contract GatewayZEVM is if (amount == 0) revert InsufficientZetaAmount(); _transferZETA(amount, FUNGIBLE_MODULE_ADDRESS); - emit Withdrawn(msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, message, 0, revertOptions); + emit Withdrawn( + msg.sender, + chainId, + receiver, + address(zetaToken), + amount, + 0, + 0, + message, + CallOptions({ gasLimit: 0, isArbitraryCall: true }), + revertOptions + ); + } + + function withdrawAndCall( + bytes memory receiver, + uint256 amount, + uint256 chainId, + bytes calldata message, + CallOptions calldata callOptions, + RevertOptions calldata revertOptions + ) + external + nonReentrant + whenNotPaused + { + if (receiver.length == 0) revert ZeroAddress(); + if (amount == 0) revert InsufficientZetaAmount(); + + _transferZETA(amount, FUNGIBLE_MODULE_ADDRESS); + emit Withdrawn( + msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, message, callOptions, revertOptions + ); } /// @notice Call a smart contract on an external chain without asset transfer. diff --git a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol index 9e440c73c..36e009fad 100644 --- a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol +++ b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol @@ -32,7 +32,7 @@ interface IGatewayZEVMEvents { /// @param gasfee The gas fee for the withdrawal. /// @param protocolFlatFee The protocol flat fee for the withdrawal. /// @param message The calldata passed to the contract call. - /// @param gasLimit Gas limit. + /// @param callOptions Call options including gas limit and arbirtrary call flag. /// @param revertOptions Revert options. event Withdrawn( address indexed sender, @@ -43,7 +43,7 @@ interface IGatewayZEVMEvents { uint256 gasfee, uint256 protocolFlatFee, bytes message, - uint256 gasLimit, + CallOptions callOptions, RevertOptions revertOptions ); } diff --git a/v2/test/GatewayEVMZEVM.t.sol b/v2/test/GatewayEVMZEVM.t.sol index 64a73b796..c5380deae 100644 --- a/v2/test/GatewayEVMZEVM.t.sol +++ b/v2/test/GatewayEVMZEVM.t.sol @@ -201,11 +201,18 @@ contract GatewayEVMZEVMTest is expectedGasFee, zrc20.PROTOCOL_FLAT_FEE(), message, - 1, + CallOptions({ gasLimit: 1, isArbitraryCall: true }), revertOptions ); vm.prank(ownerZEVM); - gatewayZEVM.withdrawAndCall(abi.encodePacked(receiverEVM), 500_000, address(zrc20), message, 1, revertOptions); + gatewayZEVM.withdrawAndCall( + abi.encodePacked(receiverEVM), + 500_000, + address(zrc20), + message, + CallOptions({ gasLimit: 1, isArbitraryCall: true }), + revertOptions + ); // Check the balance after withdrawal uint256 balanceOfAfterWithdrawal = zrc20.balanceOf(ownerZEVM); diff --git a/v2/test/GatewayZEVM.t.sol b/v2/test/GatewayZEVM.t.sol index 3c2bc1bc8..1f21a591d 100644 --- a/v2/test/GatewayZEVM.t.sol +++ b/v2/test/GatewayZEVM.t.sol @@ -87,7 +87,7 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors 0, zrc20.PROTOCOL_FLAT_FEE(), "", - 0, + CallOptions({ gasLimit: 0, isArbitraryCall: true }), revertOptions ); gateway.withdraw(abi.encodePacked(addr1), amount, address(zrc20), revertOptions); @@ -189,7 +189,7 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors expectedGasFee, zrc20.PROTOCOL_FLAT_FEE(), message, - gasLimit, + CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), revertOptions ); gateway.withdrawAndCall(abi.encodePacked(addr1), amount, address(zrc20), message, gasLimit, revertOptions); @@ -228,7 +228,18 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors uint256 chainId = 1; vm.expectEmit(true, true, true, true, address(gateway)); - emit Withdrawn(owner, chainId, abi.encodePacked(addr1), address(zetaToken), amount, 0, 0, "", 0, revertOptions); + emit Withdrawn( + owner, + chainId, + abi.encodePacked(addr1), + address(zetaToken), + amount, + 0, + 0, + "", + CallOptions({ gasLimit: 0, isArbitraryCall: true }), + revertOptions + ); gateway.withdraw(abi.encodePacked(addr1), amount, chainId, revertOptions); uint256 ownerBalanceAfter = zetaToken.balanceOf(owner); @@ -285,7 +296,16 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors vm.expectEmit(true, true, true, true, address(gateway)); emit Withdrawn( - owner, chainId, abi.encodePacked(addr1), address(zetaToken), amount, 0, 0, message, 0, revertOptions + owner, + chainId, + abi.encodePacked(addr1), + address(zetaToken), + amount, + 0, + 0, + message, + CallOptions({ gasLimit: 0, isArbitraryCall: true }), + revertOptions ); gateway.withdrawAndCall(abi.encodePacked(addr1), amount, chainId, message, revertOptions); From ec645b2f5742c0e5eaaf1376ab3f025d8e4cbc29 Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 13 Sep 2024 14:20:27 +0200 Subject: [PATCH 07/11] missing natspec --- v2/contracts/evm/GatewayEVM.sol | 6 +++ v2/contracts/evm/interfaces/IGatewayEVM.sol | 11 +++++ v2/contracts/zevm/GatewayZEVM.sol | 14 ++++++ v2/contracts/zevm/interfaces/IGatewayZEVM.sol | 43 +++++++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 417bb50e4..221ab6d7c 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -138,6 +138,7 @@ contract GatewayEVM is /// @notice Executes a call to a destination address without ERC20 tokens. /// @dev This function can only be called by the TSS address and it is payable. + /// @param messageContext Message context containing sender and arbitrary call flag. /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. @@ -166,6 +167,11 @@ contract GatewayEVM is return result; } + /// @notice Executes a call to a destination address without ERC20 tokens. + /// @dev This function can only be called by the TSS address and it is payable. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. function execute( address destination, bytes calldata data diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 4a5454109..241e29516 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -81,6 +81,7 @@ interface IGatewayEVMErrors { /// @notice Error when trying to transfer not whitelisted token to custody. error NotWhitelistedInCustody(); + /// @notice Error when trying to call onCall method using arbitrary call. error NotAllowedToCallOnCall(); } @@ -113,6 +114,12 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { /// @return The result of the contract call. function execute(address destination, bytes calldata data) external payable returns (bytes memory); + /// @notice Executes a call to a destination address without ERC20 tokens. + /// @dev This function can only be called by the TSS address and it is payable. + /// @param messageContext Message context containing sender and arbitrary call flag. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. function execute( MessageContext calldata messageContext, address destination, @@ -183,11 +190,15 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { function call(address receiver, bytes calldata payload, RevertOptions calldata revertOptions) external; } +/// @notice Message context passed to execute function. +/// @param sender Sender from omnichain contract. +/// @param isArbitraryCall Indicates if call should be arbitrary or authenticated. struct MessageContext { address sender; bool isArbitraryCall; } +/// @notice Interface implemented by contracts receiving authenticated calls. interface Callable { function onCall(address sender, bytes calldata message) external returns (bytes memory); } diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 81e06976b..836ed2b83 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -193,6 +193,13 @@ contract GatewayZEVM is ); } + /// @notice Withdraw ZRC20 tokens and call a smart contract on an external chain. + /// @param receiver The receiver address on the external chain. + /// @param amount The amount of tokens to withdraw. + /// @param zrc20 The address of the ZRC20 token. + /// @param message The calldata to pass to the contract call. + /// @param callOptions Call options including gas limit and arbirtrary call flag. + /// @param revertOptions Revert options. function withdrawAndCall( bytes memory receiver, uint256 amount, @@ -290,6 +297,13 @@ contract GatewayZEVM is ); } + /// @notice Withdraw ZETA tokens and call a smart contract on an external chain. + /// @param receiver The receiver address on the external chain. + /// @param amount The amount of tokens to withdraw. + /// @param chainId Chain id of the external chain. + /// @param message The calldata to pass to the contract call. + /// @param callOptions Call options including gas limit and arbirtrary call flag. + /// @param revertOptions Revert options. function withdrawAndCall( bytes memory receiver, uint256 amount, diff --git a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol index 36e009fad..73108d3b4 100644 --- a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol +++ b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol @@ -134,17 +134,51 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { ) external; + /// @notice Withdraw ZRC20 tokens and call a smart contract on an external chain. + /// @param receiver The receiver address on the external chain. + /// @param amount The amount of tokens to withdraw. + /// @param zrc20 The address of the ZRC20 token. + /// @param message The calldata to pass to the contract call. + /// @param callOptions Call options including gas limit and arbirtrary call flag. + /// @param revertOptions Revert options. + function withdrawAndCall( + bytes memory receiver, + uint256 amount, + address zrc20, + bytes calldata message, + CallOptions calldata callOptions, + RevertOptions calldata revertOptions + ) + external; + + /// @notice Withdraw ZETA tokens and call a smart contract on an external chain. + /// @param receiver The receiver address on the external chain. + /// @param amount The amount of tokens to withdraw. + /// @param chainId Chain id of the external chain. + /// @param message The calldata to pass to the contract call. + /// @param revertOptions Revert options. + function withdrawAndCall( + bytes memory receiver, + uint256 amount, + uint256 chainId, + bytes calldata message, + RevertOptions calldata revertOptions + ) + external; + /// @notice Withdraw ZETA tokens and call a smart contract on an external chain. /// @param receiver The receiver address on the external chain. /// @param amount The amount of tokens to withdraw. /// @param chainId Chain id of the external chain. /// @param message The calldata to pass to the contract call. + /// @param callOptions Call options including gas limit and arbirtrary call flag. /// @param revertOptions Revert options. function withdrawAndCall( bytes memory receiver, uint256 amount, uint256 chainId, bytes calldata message, + CallOptions calldata callOptions, RevertOptions calldata revertOptions ) external; @@ -164,6 +198,12 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { ) external; + /// @notice Call a smart contract on an external chain without asset transfer. + /// @param receiver The receiver address on the external chain. + /// @param zrc20 Address of zrc20 to pay fees. + /// @param message The calldata to pass to the contract call. + /// @param gasLimit Gas limit. + /// @param revertOptions Revert options. function call( bytes memory receiver, address zrc20, @@ -241,6 +281,9 @@ interface IGatewayZEVM is IGatewayZEVMErrors, IGatewayZEVMEvents { external; } +/// @notice CallOptions struct passed to call and withdrawAndCall functions. +/// @param gasLimit Gas limit. +/// @param isArbitraryCall Indicates if call should be arbitrary or authenticated. struct CallOptions { uint256 gasLimit; bool isArbitraryCall; From d245ed15b8f487175dfbce95c76c1f4013b1c5d1 Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 13 Sep 2024 14:41:15 +0200 Subject: [PATCH 08/11] add execute with msg context tests --- v2/contracts/evm/GatewayEVM.sol | 7 +++- v2/test/GatewayEVM.t.sol | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 221ab6d7c..dcba1fc35 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -71,7 +71,7 @@ contract GatewayEVM is /// @param newImplementation Address of the new implementation. function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) { } - /// @dev Internal function to execute a call to a destination address. + /// @dev Internal function to execute an arbitrary call to a destination address. /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. @@ -92,6 +92,11 @@ contract GatewayEVM is return result; } + /// @dev Internal function to execute an authenticated call to a destination address. + /// @param messageContext Message context containing sender and arbitrary call flag. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. function _executeAuthenticatedCall( MessageContext calldata messageContext, address destination, diff --git a/v2/test/GatewayEVM.t.sol b/v2/test/GatewayEVM.t.sol index f7a24b4b4..12640382e 100644 --- a/v2/test/GatewayEVM.t.sol +++ b/v2/test/GatewayEVM.t.sol @@ -137,6 +137,35 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } + function testForwardCallToReceiveNonPayableUsingArbCall() public { + string[] memory str = new string[](1); + str[0] = "Hello, Foundry!"; + uint256[] memory num = new uint256[](1); + num[0] = 42; + bool flag = true; + + bytes memory data = abi.encodeWithSignature("receiveNonPayable(string[],uint256[],bool)", str, num, flag); + + vm.expectCall(address(receiver), 0, data); + vm.expectEmit(true, true, true, true, address(receiver)); + emit ReceivedNonPayable(address(gateway), str, num, flag); + vm.expectEmit(true, true, true, true, address(gateway)); + emit Executed(address(receiver), 0, data); + vm.prank(tssAddress); + gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(receiver), data); + } + + function testForwardCallToReceiveOnCallUsingAuthCall() public { + vm.expectEmit(true, true, true, true, address(receiver)); + emit ReceivedOnCall(); + vm.expectEmit(true, true, true, true, address(gateway)); + emit Executed(address(receiver), 0, bytes("1")); + vm.prank(tssAddress); + gateway.execute( + MessageContext({ sender: address(0x123), isArbitraryCall: false }), address(receiver), bytes("1") + ); + } + function testForwardCallToReceiveNonPayableFailsIfSenderIsNotTSS() public { string[] memory str = new string[](1); str[0] = "Hello, Foundry!"; @@ -150,6 +179,19 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } + function testForwardCallToReceiveNonPayableWithMsgContextFailsIfSenderIsNotTSS() public { + string[] memory str = new string[](1); + str[0] = "Hello, Foundry!"; + uint256[] memory num = new uint256[](1); + num[0] = 42; + bool flag = true; + bytes memory data = abi.encodeWithSignature("receiveNonPayable(string[],uint256[],bool)", str, num, flag); + + vm.prank(owner); + vm.expectRevert(abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, owner, TSS_ROLE)); + gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: false }), address(receiver), data); + } + function testForwardCallToReceivePayable() public { string memory str = "Hello, Foundry!"; uint256 num = 42; @@ -182,6 +224,18 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } + function testForwardCallToReceiveNoParamsWithMsgContext() public { + bytes memory data = abi.encodeWithSignature("receiveNoParams()"); + + vm.expectCall(address(receiver), 0, data); + vm.expectEmit(true, true, true, true, address(receiver)); + emit ReceivedNoParams(address(gateway)); + vm.expectEmit(true, true, true, true, address(gateway)); + emit Executed(address(receiver), 0, data); + vm.prank(tssAddress); + gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(receiver), data); + } + function testForwardCallToReceiveOnCallFails() public { bytes memory data = abi.encodeWithSignature("onCall(address,bytes)", address(123), bytes("")); @@ -198,6 +252,14 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(0), data); } + function testExecuteWithMsgContextFailsIfDestinationIsZeroAddress() public { + bytes memory data = abi.encodeWithSignature("receiveNoParams()"); + + vm.prank(tssAddress); + vm.expectRevert(ZeroAddress.selector); + gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(0), data); + } + function testForwardCallToReceiveNoParamsTogglePause() public { vm.prank(tssAddress); vm.expectRevert(abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, tssAddress, PAUSER_ROLE)); From 40385f91936603b8ea460dac56b75b54f39feded Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 13 Sep 2024 15:25:25 +0200 Subject: [PATCH 09/11] tests --- v2/contracts/zevm/GatewayZEVM.sol | 6 ++ v2/test/GatewayZEVM.t.sol | 151 ++++++++++++++++++++++++++++-- 2 files changed, 149 insertions(+), 8 deletions(-) diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 836ed2b83..18d47dd80 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -345,6 +345,12 @@ contract GatewayZEVM is _call(receiver, zrc20, message, callOptions, revertOptions); } + /// @notice Call a smart contract on an external chain without asset transfer. + /// @param receiver The receiver address on the external chain. + /// @param zrc20 Address of zrc20 to pay fees. + /// @param message The calldata to pass to the contract call. + /// @param gasLimit Gas limit. + /// @param revertOptions Revert options. function call( bytes memory receiver, address zrc20, diff --git a/v2/test/GatewayZEVM.t.sol b/v2/test/GatewayZEVM.t.sol index 1f21a591d..1e54b1957 100644 --- a/v2/test/GatewayZEVM.t.sol +++ b/v2/test/GatewayZEVM.t.sol @@ -27,6 +27,7 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors address addr1; address fungibleModule; RevertOptions revertOptions; + CallOptions callOptions; error ZeroAddress(); error LowBalance(); @@ -71,6 +72,8 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors revertMessage: "", onRevertGasLimit: 0 }); + + callOptions = CallOptions({ gasLimit: 1, isArbitraryCall: true }); } function testWithdrawZRC20() public { @@ -198,6 +201,61 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors assertEq(ownerBalanceBefore - amount - expectedGasFee, ownerBalanceAfter); } + function testWithdrawAndCallZRC20WithCallOptsFailsIfReceiverIsZeroAddress() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(ZeroAddress.selector); + gateway.withdrawAndCall(abi.encodePacked(""), 1, address(zrc20), message, callOptions, revertOptions); + } + + function testWithdrawAndCallZRC20WithCallOptsFailsIfAmountIsZero() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(InsufficientZRC20Amount.selector); + gateway.withdrawAndCall(abi.encodePacked(addr1), 0, address(zrc20), message, callOptions, revertOptions); + } + + function testWithdrawZRC20WithMessageWithCallOptsFailsIfNoAllowance() public { + uint256 amount = 1; + uint256 ownerBalanceBefore = zrc20.balanceOf(owner); + + // Remove allowance for gateway + vm.prank(owner); + zrc20.approve(address(gateway), 0); + + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(); + gateway.withdrawAndCall(abi.encodePacked(addr1), amount, address(zrc20), message, callOptions, revertOptions); + + // Check that balance didn't change + uint256 ownerBalanceAfter = zrc20.balanceOf(owner); + assertEq(ownerBalanceBefore, ownerBalanceAfter); + } + + function testWithdrawZRC20WithCallOptsWithMessage() public { + uint256 amount = 1; + uint256 ownerBalanceBefore = zrc20.balanceOf(owner); + + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + uint256 expectedGasFee = 1; + uint256 gasLimit = 1; + vm.expectEmit(true, true, true, true, address(gateway)); + emit Withdrawn( + owner, + 0, + abi.encodePacked(addr1), + address(zrc20), + amount, + expectedGasFee, + zrc20.PROTOCOL_FLAT_FEE(), + message, + CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), + revertOptions + ); + gateway.withdrawAndCall(abi.encodePacked(addr1), amount, address(zrc20), message, gasLimit, revertOptions); + + uint256 ownerBalanceAfter = zrc20.balanceOf(owner); + assertEq(ownerBalanceBefore - amount - expectedGasFee, ownerBalanceAfter); + } + function testWithdrawZETAFailsIfAmountIsZero() public { vm.expectRevert(InsufficientZetaAmount.selector); gateway.withdraw(abi.encodePacked(addr1), 0, 1, revertOptions); @@ -220,6 +278,18 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors gateway.withdrawAndCall(abi.encodePacked(""), 1, 1, message, revertOptions); } + function testWithdrawAndCallZETAWithCallOptsFailsIfAmountIsZero() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(InsufficientZetaAmount.selector); + gateway.withdrawAndCall(abi.encodePacked(addr1), 0, 1, message, callOptions, revertOptions); + } + + function testWithdrawAndCallZETAWithCallOptsFailsIfAmountIsReceiverIsZeroAddress() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(ZeroAddress.selector); + gateway.withdrawAndCall(abi.encodePacked(""), 1, 1, message, callOptions, revertOptions); + } + function testWithdrawZETA() public { uint256 amount = 1; uint256 ownerBalanceBefore = zetaToken.balanceOf(owner); @@ -344,6 +414,64 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors assertEq(fungibleModuleBalanceBefore, fungibleModule.balance); } + function testWithdrawZETAWithCallOptsWithMessage() public { + uint256 amount = 1; + uint256 ownerBalanceBefore = zetaToken.balanceOf(owner); + uint256 gatewayBalanceBefore = zetaToken.balanceOf(address(gateway)); + uint256 fungibleModuleBalanceBefore = fungibleModule.balance; + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + uint256 chainId = 1; + + vm.expectEmit(true, true, true, true, address(gateway)); + emit Withdrawn( + owner, + chainId, + abi.encodePacked(addr1), + address(zetaToken), + amount, + 0, + 0, + message, + callOptions, + revertOptions + ); + gateway.withdrawAndCall(abi.encodePacked(addr1), amount, chainId, message, callOptions, revertOptions); + + uint256 ownerBalanceAfter = zetaToken.balanceOf(owner); + assertEq(ownerBalanceBefore - 1, ownerBalanceAfter); + + uint256 gatewayBalanceAfter = zetaToken.balanceOf(address(gateway)); + assertEq(gatewayBalanceBefore, gatewayBalanceAfter); + + // Verify amount is transfered to fungible module + assertEq(fungibleModuleBalanceBefore + 1, fungibleModule.balance); + } + + function testWithdrawZETAWithCallOptsWithMessageFailsIfNoAllowance() public { + uint256 amount = 1; + uint256 ownerBalanceBefore = zetaToken.balanceOf(owner); + uint256 gatewayBalanceBefore = zetaToken.balanceOf(address(gateway)); + uint256 fungibleModuleBalanceBefore = fungibleModule.balance; + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + uint256 chainId = 1; + + // Remove allowance for gateway + vm.prank(owner); + zetaToken.approve(address(gateway), 0); + + vm.expectRevert(); + gateway.withdrawAndCall(abi.encodePacked(addr1), amount, chainId, message, callOptions, revertOptions); + + // Verify balances not changed + uint256 ownerBalanceAfter = zetaToken.balanceOf(owner); + assertEq(ownerBalanceBefore, ownerBalanceAfter); + + uint256 gatewayBalanceAfter = zetaToken.balanceOf(address(gateway)); + assertEq(gatewayBalanceBefore, gatewayBalanceAfter); + + assertEq(fungibleModuleBalanceBefore, fungibleModule.balance); + } + function testCallFailsIfReceiverIsZeroAddress() public { bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectRevert(ZeroAddress.selector); @@ -354,16 +482,23 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectEmit(true, true, true, true, address(gateway)); - emit Called( - owner, - address(zrc20), - abi.encodePacked(addr1), - message, - CallOptions({ gasLimit: 1, isArbitraryCall: true }), - revertOptions - ); + emit Called(owner, address(zrc20), abi.encodePacked(addr1), message, callOptions, revertOptions); gateway.call(abi.encodePacked(addr1), address(zrc20), message, 1, revertOptions); } + + function testCallWithCallOptsFailsIfReceiverIsZeroAddress() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectRevert(ZeroAddress.selector); + gateway.call(abi.encodePacked(""), address(zrc20), message, callOptions, revertOptions); + } + + function testCallWithCallOpts() public { + bytes memory message = abi.encodeWithSignature("hello(address)", addr1); + vm.expectEmit(true, true, true, true, address(gateway)); + + emit Called(owner, address(zrc20), abi.encodePacked(addr1), message, callOptions, revertOptions); + gateway.call(abi.encodePacked(addr1), address(zrc20), message, callOptions, revertOptions); + } } contract GatewayZEVMOutboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors { From df3576c551b20778510a245532b5daefa32c8f3c Mon Sep 17 00:00:00 2001 From: skosito Date: Mon, 16 Sep 2024 22:50:07 +0200 Subject: [PATCH 10/11] pr comment --- v2/contracts/evm/GatewayEVM.sol | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index dcba1fc35..53559ed28 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -76,16 +76,7 @@ contract GatewayEVM is /// @param data Calldata to pass to the call. /// @return The result of the call. function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { - if (data.length >= 4) { - bytes4 functionSelector; - assembly { - functionSelector := calldataload(data.offset) - } - - if (functionSelector == Callable.onCall.selector) { - revert NotAllowedToCallOnCall(); - } - } + revertIfAuthenticatedCall(data); (bool success, bytes memory result) = destination.call{ value: msg.value }(data); if (!success) revert ExecutionFailed(); @@ -441,4 +432,18 @@ contract GatewayEVM is IERC20(token).safeTransfer(custody, amount); } } + + // @dev prevent calling onCall function reserved for authenticated calls + function revertIfAuthenticatedCall(bytes calldata data) private pure { + if (data.length >= 4) { + bytes4 functionSelector; + assembly { + functionSelector := calldataload(data.offset) + } + + if (functionSelector == Callable.onCall.selector) { + revert NotAllowedToCallOnCall(); + } + } + } } From 7319423190de968d14398191c12765d48ba5722c Mon Sep 17 00:00:00 2001 From: skosito Date: Mon, 16 Sep 2024 23:25:34 +0200 Subject: [PATCH 11/11] refactor execute funcs --- v2/contracts/evm/GatewayEVM.sol | 68 +++++++++--------- v2/contracts/evm/interfaces/IGatewayEVM.sol | 4 +- v2/test/GatewayEVM.t.sol | 40 ++--------- v2/test/GatewayEVMZEVM.t.sol | 2 +- v2/test/utils/GatewayEVMUpgradeTest.sol | 76 +++++++++++++-------- v2/test/utils/ReceiverEVM.sol | 3 +- 6 files changed, 89 insertions(+), 104 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 53559ed28..e1a1a9a3d 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -71,34 +71,6 @@ contract GatewayEVM is /// @param newImplementation Address of the new implementation. function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) { } - /// @dev Internal function to execute an arbitrary call to a destination address. - /// @param destination Address to call. - /// @param data Calldata to pass to the call. - /// @return The result of the call. - function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { - revertIfAuthenticatedCall(data); - (bool success, bytes memory result) = destination.call{ value: msg.value }(data); - if (!success) revert ExecutionFailed(); - - return result; - } - - /// @dev Internal function to execute an authenticated call to a destination address. - /// @param messageContext Message context containing sender and arbitrary call flag. - /// @param destination Address to call. - /// @param data Calldata to pass to the call. - /// @return The result of the call. - function _executeAuthenticatedCall( - MessageContext calldata messageContext, - address destination, - bytes calldata data - ) - internal - returns (bytes memory) - { - return Callable(destination).onCall(messageContext.sender, data); - } - /// @notice Pause contract. function pause() external onlyRole(PAUSER_ROLE) { _pause(); @@ -132,9 +104,9 @@ contract GatewayEVM is emit Reverted(destination, address(0), msg.value, data, revertContext); } - /// @notice Executes a call to a destination address without ERC20 tokens. + /// @notice Executes an authenticated call to a destination address without ERC20 tokens. /// @dev This function can only be called by the TSS address and it is payable. - /// @param messageContext Message context containing sender and arbitrary call flag. + /// @param messageContext Message context containing sender. /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. @@ -152,18 +124,14 @@ contract GatewayEVM is { if (destination == address(0)) revert ZeroAddress(); bytes memory result; - if (messageContext.isArbitraryCall) { - result = _executeArbitraryCall(destination, data); - } else { - result = _executeAuthenticatedCall(messageContext, destination, data); - } + result = _executeAuthenticatedCall(messageContext, destination, data); emit Executed(destination, msg.value, data); return result; } - /// @notice Executes a call to a destination address without ERC20 tokens. + /// @notice Executes an arbitrary call to a destination address without ERC20 tokens. /// @dev This function can only be called by the TSS address and it is payable. /// @param destination Address to call. /// @param data Calldata to pass to the call. @@ -433,6 +401,34 @@ contract GatewayEVM is } } + /// @dev Private function to execute an arbitrary call to a destination address. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. + function _executeArbitraryCall(address destination, bytes calldata data) private returns (bytes memory) { + revertIfAuthenticatedCall(data); + (bool success, bytes memory result) = destination.call{ value: msg.value }(data); + if (!success) revert ExecutionFailed(); + + return result; + } + + /// @dev Private function to execute an authenticated call to a destination address. + /// @param messageContext Message context containing sender and arbitrary call flag. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. + function _executeAuthenticatedCall( + MessageContext calldata messageContext, + address destination, + bytes calldata data + ) + private + returns (bytes memory) + { + return Callable(destination).onCall(messageContext, data); + } + // @dev prevent calling onCall function reserved for authenticated calls function revertIfAuthenticatedCall(bytes calldata data) private pure { if (data.length >= 4) { diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index 241e29516..d1f4dcb99 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -192,13 +192,11 @@ interface IGatewayEVM is IGatewayEVMErrors, IGatewayEVMEvents { /// @notice Message context passed to execute function. /// @param sender Sender from omnichain contract. -/// @param isArbitraryCall Indicates if call should be arbitrary or authenticated. struct MessageContext { address sender; - bool isArbitraryCall; } /// @notice Interface implemented by contracts receiving authenticated calls. interface Callable { - function onCall(address sender, bytes calldata message) external returns (bytes memory); + function onCall(MessageContext calldata context, bytes calldata message) external returns (bytes memory); } diff --git a/v2/test/GatewayEVM.t.sol b/v2/test/GatewayEVM.t.sol index 12640382e..771634359 100644 --- a/v2/test/GatewayEVM.t.sol +++ b/v2/test/GatewayEVM.t.sol @@ -137,33 +137,13 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } - function testForwardCallToReceiveNonPayableUsingArbCall() public { - string[] memory str = new string[](1); - str[0] = "Hello, Foundry!"; - uint256[] memory num = new uint256[](1); - num[0] = 42; - bool flag = true; - - bytes memory data = abi.encodeWithSignature("receiveNonPayable(string[],uint256[],bool)", str, num, flag); - - vm.expectCall(address(receiver), 0, data); - vm.expectEmit(true, true, true, true, address(receiver)); - emit ReceivedNonPayable(address(gateway), str, num, flag); - vm.expectEmit(true, true, true, true, address(gateway)); - emit Executed(address(receiver), 0, data); - vm.prank(tssAddress); - gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(receiver), data); - } - function testForwardCallToReceiveOnCallUsingAuthCall() public { vm.expectEmit(true, true, true, true, address(receiver)); emit ReceivedOnCall(); vm.expectEmit(true, true, true, true, address(gateway)); emit Executed(address(receiver), 0, bytes("1")); vm.prank(tssAddress); - gateway.execute( - MessageContext({ sender: address(0x123), isArbitraryCall: false }), address(receiver), bytes("1") - ); + gateway.execute(MessageContext({ sender: address(0x123) }), address(receiver), bytes("1")); } function testForwardCallToReceiveNonPayableFailsIfSenderIsNotTSS() public { @@ -189,7 +169,7 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver vm.prank(owner); vm.expectRevert(abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, owner, TSS_ROLE)); - gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: false }), address(receiver), data); + gateway.execute(MessageContext({ sender: address(0x123) }), address(receiver), data); } function testForwardCallToReceivePayable() public { @@ -224,20 +204,8 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver gateway.execute(address(receiver), data); } - function testForwardCallToReceiveNoParamsWithMsgContext() public { - bytes memory data = abi.encodeWithSignature("receiveNoParams()"); - - vm.expectCall(address(receiver), 0, data); - vm.expectEmit(true, true, true, true, address(receiver)); - emit ReceivedNoParams(address(gateway)); - vm.expectEmit(true, true, true, true, address(gateway)); - emit Executed(address(receiver), 0, data); - vm.prank(tssAddress); - gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(receiver), data); - } - function testForwardCallToReceiveOnCallFails() public { - bytes memory data = abi.encodeWithSignature("onCall(address,bytes)", address(123), bytes("")); + bytes memory data = abi.encodeWithSignature("onCall((address),bytes)", address(123), bytes("")); vm.prank(tssAddress); vm.expectRevert(NotAllowedToCallOnCall.selector); @@ -257,7 +225,7 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver vm.prank(tssAddress); vm.expectRevert(ZeroAddress.selector); - gateway.execute(MessageContext({ sender: address(0x123), isArbitraryCall: true }), address(0), data); + gateway.execute(MessageContext({ sender: address(0x123) }), address(0), data); } function testForwardCallToReceiveNoParamsTogglePause() public { diff --git a/v2/test/GatewayEVMZEVM.t.sol b/v2/test/GatewayEVMZEVM.t.sol index c5380deae..f4af846f3 100644 --- a/v2/test/GatewayEVMZEVM.t.sol +++ b/v2/test/GatewayEVMZEVM.t.sol @@ -13,7 +13,7 @@ import "./utils/TestERC20.sol"; import "./utils/SenderZEVM.sol"; -import "./utils/SystemContractMock.sol"; +import { SystemContractMock } from "./utils/SystemContractMock.sol"; import { GatewayZEVM } from "../contracts/zevm/GatewayZEVM.sol"; import { IGatewayZEVM } from "../contracts/zevm/GatewayZEVM.sol"; diff --git a/v2/test/utils/GatewayEVMUpgradeTest.sol b/v2/test/utils/GatewayEVMUpgradeTest.sol index 84150cc39..6db211f66 100644 --- a/v2/test/utils/GatewayEVMUpgradeTest.sol +++ b/v2/test/utils/GatewayEVMUpgradeTest.sol @@ -75,28 +75,6 @@ contract GatewayEVMUpgradeTest is /// @param newImplementation Address of the new implementation. function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) { } - /// @dev Internal function to execute a call to a destination address. - /// @param destination Address to call. - /// @param data Calldata to pass to the call. - /// @return The result of the call. - function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { - (bool success, bytes memory result) = destination.call{ value: msg.value }(data); - if (!success) revert ExecutionFailed(); - - return result; - } - - function _executeAuthenticatedCall( - MessageContext calldata messageContext, - address destination, - bytes calldata data - ) - internal - returns (bytes memory) - { - return Callable(destination).onCall(messageContext.sender, data); - } - /// @notice Pause contract. function pause() external onlyRole(PAUSER_ROLE) { _pause(); @@ -132,6 +110,7 @@ contract GatewayEVMUpgradeTest is /// @notice Executes a call to a destination address without ERC20 tokens. /// @dev This function can only be called by the TSS address and it is payable. + /// @param messageContext Message context containing sender and arbitrary call flag. /// @param destination Address to call. /// @param data Calldata to pass to the call. /// @return The result of the call. @@ -149,17 +128,18 @@ contract GatewayEVMUpgradeTest is { if (destination == address(0)) revert ZeroAddress(); bytes memory result; - if (messageContext.isArbitraryCall) { - result = _executeArbitraryCall(destination, data); - } else { - result = _executeAuthenticatedCall(messageContext, destination, data); - } + result = _executeAuthenticatedCall(messageContext, destination, data); emit Executed(destination, msg.value, data); return result; } + /// @notice Executes a call to a destination address without ERC20 tokens. + /// @dev This function can only be called by the TSS address and it is payable. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. function execute( address destination, bytes calldata data @@ -424,4 +404,46 @@ contract GatewayEVMUpgradeTest is IERC20(token).safeTransfer(custody, amount); } } + + /// @dev Internal function to execute an arbitrary call to a destination address. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. + function _executeArbitraryCall(address destination, bytes calldata data) internal returns (bytes memory) { + revertIfAuthenticatedCall(data); + (bool success, bytes memory result) = destination.call{ value: msg.value }(data); + if (!success) revert ExecutionFailed(); + + return result; + } + + /// @dev Internal function to execute an authenticated call to a destination address. + /// @param messageContext Message context containing sender and arbitrary call flag. + /// @param destination Address to call. + /// @param data Calldata to pass to the call. + /// @return The result of the call. + function _executeAuthenticatedCall( + MessageContext calldata messageContext, + address destination, + bytes calldata data + ) + internal + returns (bytes memory) + { + return Callable(destination).onCall(messageContext, data); + } + + // @dev prevent calling onCall function reserved for authenticated calls + function revertIfAuthenticatedCall(bytes calldata data) internal pure { + if (data.length >= 4) { + bytes4 functionSelector; + assembly { + functionSelector := calldataload(data.offset) + } + + if (functionSelector == Callable.onCall.selector) { + revert NotAllowedToCallOnCall(); + } + } + } } diff --git a/v2/test/utils/ReceiverEVM.sol b/v2/test/utils/ReceiverEVM.sol index 61557527d..e8063551b 100644 --- a/v2/test/utils/ReceiverEVM.sol +++ b/v2/test/utils/ReceiverEVM.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.26; import { RevertContext } from "../../contracts/Revert.sol"; +import { MessageContext } from "../../contracts/evm/interfaces/IGatewayEVM.sol"; import "./IReceiverEVM.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; @@ -72,7 +73,7 @@ contract ReceiverEVM is IReceiverEVMEvents, ReentrancyGuard { emit ReceivedRevert(msg.sender, revertContext); } - function onCall(address sender, bytes calldata message) external returns (bytes memory) { + function onCall(MessageContext calldata messageContext, bytes calldata message) external returns (bytes memory) { emit ReceivedOnCall(); }