diff --git a/contracts/src/Gateway.sol b/contracts/src/Gateway.sol index 5cfcdda1d..d3c51f3e0 100644 --- a/contracts/src/Gateway.sol +++ b/contracts/src/Gateway.sol @@ -209,9 +209,9 @@ contract Gateway is IGatewayBase, IGatewayV1, IGatewayV2, IInitializable, IUpgra success = false; } } else if (message.command == CommandV1.MintForeignToken) { - try Gateway(this).v1_handleMintForeignToken{gas: maxDispatchGas}( - message.channelID, message.params - ) {} catch { + try Gateway(this) + .v1_handleMintForeignToken{gas: maxDispatchGas}(message.channelID, message.params) {} + catch { success = false; } } else { @@ -431,11 +431,14 @@ contract Gateway is IGatewayBase, IGatewayV1, IGatewayV2, IInitializable, IUpgra revert IGatewayBase.InvalidProof(); } - // Dispatch the message payload. The boolean returned indicates whether all commands succeeded. - bool success = v2_dispatch(message); - - // Emit the event with a success value "true" if all commands successfully executed, otherwise "false" - // if all or some of the commands failed. + // Dispatch all commands in the message + (bool insufficientGasLimit, bool success) = + Gateway(this).v2_dispatch(message.commands, message.origin, message.nonce); + // Revert if the gas limit provided was insufficient for any command + if (insufficientGasLimit) { + revert IGatewayV2.InsufficientGasLimit(); + } + // Emit event for the overall message dispatch result emit IGatewayV2.InboundMessageDispatched( message.nonce, message.topic, success, rewardAddress ); @@ -476,104 +479,53 @@ contract Gateway is IGatewayBase, IGatewayV1, IGatewayV2, IInitializable, IUpgra CallsV2.createAgent(id); } - /** - * APIv2 Message Handlers - */ - - // Perform an upgrade of the gateway - function v2_handleUpgrade(bytes calldata data) external onlySelf { - HandlersV2.upgrade(data); - } - - // Set the operating mode of the gateway - function v2_handleSetOperatingMode(bytes calldata data) external onlySelf { - HandlersV2.setOperatingMode(data); - } - - // Unlock Native token - function v2_handleUnlockNativeToken(bytes calldata data) external onlySelf { - HandlersV2.unlockNativeToken(AGENT_EXECUTOR, data); - } - - // Register a new fungible Polkadot token for an agent - function v2_handleRegisterForeignToken(bytes calldata data) external onlySelf { - HandlersV2.registerForeignToken(data); - } - - // Mint foreign token from polkadot - function v2_handleMintForeignToken(bytes calldata data) external onlySelf { - HandlersV2.mintForeignToken(data); - } - - // Call an arbitrary contract function - function v2_handleCallContract(bytes32 origin, bytes calldata data) external onlySelf { - HandlersV2.callContract(origin, AGENT_EXECUTOR, data); - } - - /** - * APIv2 Internal functions - */ - - // Internal helper to dispatch a single command - function _dispatchCommand(CommandV2 calldata command, bytes32 origin) - internal - returns (bool) + // Dispatch all the commands within the batch of commands in the message payload. Each command is processed + // independently, returns: + // 1. insufficientGasLimit: true if the gas limit provided was insufficient for any command + // 2. success: true if all commands executed successfully, false if any command failed. + function v2_dispatch(CommandV2[] calldata commands, bytes32 origin, uint64 nonce) + external + onlySelf + returns (bool, bool) { - // check that there is enough gas available to forward to the command handler - if (gasleft() * 63 / 64 < command.gas + DISPATCH_OVERHEAD_GAS_V2) { - revert IGatewayV2.InsufficientGasLimit(); + bool success = true; + for (uint256 i = 0; i < commands.length; i++) { + CommandV2 calldata command = commands[i]; + // check that there is enough gas available to forward to the command handler + uint256 requiredGas = command.gas + DISPATCH_OVERHEAD_GAS_V2; + if (gasleft() * 63 / 64 < requiredGas) { + return (true, false); + } + try Gateway(this).v2_dispatchCommand{gas: command.gas}(command, origin) {} + catch { + success = false; + emit IGatewayV2.CommandFailed(nonce, i); + } } + return (false, success); + } + // Dispatch a single command to its handler + function v2_dispatchCommand(CommandV2 calldata command, bytes32 origin) + external + virtual + onlySelf + { if (command.kind == CommandKind.Upgrade) { - try Gateway(this).v2_handleUpgrade{gas: command.gas}(command.payload) {} - catch { - return false; - } + HandlersV2.upgrade(command.payload); } else if (command.kind == CommandKind.SetOperatingMode) { - try Gateway(this).v2_handleSetOperatingMode{gas: command.gas}(command.payload) {} - catch { - return false; - } + HandlersV2.setOperatingMode(command.payload); } else if (command.kind == CommandKind.UnlockNativeToken) { - try Gateway(this).v2_handleUnlockNativeToken{gas: command.gas}(command.payload) {} - catch { - return false; - } + HandlersV2.unlockNativeToken(AGENT_EXECUTOR, command.payload); } else if (command.kind == CommandKind.RegisterForeignToken) { - try Gateway(this).v2_handleRegisterForeignToken{gas: command.gas}(command.payload) {} - catch { - return false; - } + HandlersV2.registerForeignToken(command.payload); } else if (command.kind == CommandKind.MintForeignToken) { - try Gateway(this).v2_handleMintForeignToken{gas: command.gas}(command.payload) {} - catch { - return false; - } + HandlersV2.mintForeignToken(command.payload); } else if (command.kind == CommandKind.CallContract) { - try Gateway(this).v2_handleCallContract{gas: command.gas}(origin, command.payload) {} - catch { - return false; - } + HandlersV2.callContract(origin, AGENT_EXECUTOR, command.payload); } else { - // Unknown command - return false; + revert IGatewayV2.InvalidCommand(); } - return true; - } - - // Dispatch all the commands within the batch of commands in the message payload. Each command is processed - // independently, such that failures emit a `CommandFailed` event without stopping execution of subsequent commands. - function v2_dispatch(InboundMessageV2 calldata message) internal returns (bool) { - bool allCommandsSucceeded = true; - - for (uint256 i = 0; i < message.commands.length; i++) { - if (!_dispatchCommand(message.commands[i], message.origin)) { - emit IGatewayV2.CommandFailed(message.nonce, i); - allCommandsSucceeded = false; - } - } - - return allCommandsSucceeded; } /** diff --git a/contracts/src/v2/IGateway.sol b/contracts/src/v2/IGateway.sol index 5fb684578..829ed3f13 100644 --- a/contracts/src/v2/IGateway.sol +++ b/contracts/src/v2/IGateway.sol @@ -11,6 +11,7 @@ interface IGatewayV2 { error InvalidNetwork(); error InvalidAsset(); error InsufficientGasLimit(); + error InvalidCommand(); error InsufficientValue(); error ExceededMaximumValue(); error TooManyAssets(); diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index d1963cf4b..63f3a73b0 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -496,9 +496,8 @@ contract GatewayV2Test is Test { vm.expectRevert(); hoax(user1); - IGatewayV2(payable(address(gateway))).v2_sendMessage{value: 1 ether}( - "", assets, "", 0.1 ether, 0.4 ether - ); + IGatewayV2(payable(address(gateway))) + .v2_sendMessage{value: 1 ether}("", assets, "", 0.1 ether, 0.4 ether); assertEq(feeToken.balanceOf(assetHubAgent), 0); } @@ -1104,11 +1103,14 @@ contract GatewayV2Test is Test { function test_onlySelf_enforced_on_external_calls() public { MockGateway gw = MockGateway(address(gateway)); - // calling the handler externally should revert with Unauthorized + // calling the dispatch entrypoint externally should revert with Unauthorized SetOperatingModeParams memory p = SetOperatingModeParams({mode: OperatingMode.Normal}); bytes memory payload = abi.encode(p); + CommandV2 memory cmd = CommandV2({ + kind: CommandKind.SetOperatingMode, gas: uint64(200_000), payload: payload + }); vm.expectRevert(IGatewayBase.Unauthorized.selector); - gw.v2_handleSetOperatingMode(payload); + gw.v2_dispatchCommand(cmd, bytes32(0)); } function test_call_handleSetOperatingMode_via_self_changes_mode() public { @@ -1125,7 +1127,7 @@ contract GatewayV2Test is Test { function test_dispatch_unknown_command_returns_false() public { MockGateway gw = MockGateway(address(gateway)); CommandV2 memory cmd = CommandV2({kind: 0xFF, gas: 100_000, payload: ""}); - bool ok = gw.exposed_dispatchCommand(cmd, bytes32(0)); + bool ok = gw.callDispatch(cmd, bytes32(0)); assertFalse(ok, "unknown command must return false"); } diff --git a/contracts/test/mocks/MockGateway.sol b/contracts/test/mocks/MockGateway.sol index dcf4bb9fc..507d43cc1 100644 --- a/contracts/test/mocks/MockGateway.sol +++ b/contracts/test/mocks/MockGateway.sol @@ -13,6 +13,7 @@ import {IInitializable} from "../../src/interfaces/IInitializable.sol"; import {UD60x18} from "prb/math/src/UD60x18.sol"; import {Command as CommandV2} from "../../src/v2/Types.sol"; +import {IGatewayV2} from "../../src/v2/IGateway.sol"; import {Agent} from "../../src/Agent.sol"; import {AgentExecutor} from "../../src/AgentExecutor.sol"; import {Constants} from "../../src/Constants.sol"; @@ -93,7 +94,17 @@ contract MockGateway is Gateway { } function callDispatch(CommandV2 calldata command, bytes32 origin) external returns (bool) { - return super._dispatchCommand(command, origin); + // Mirror v2_dispatch per-command behavior: enforce gas budget and surface failure as false + uint256 requiredGas = command.gas + DISPATCH_OVERHEAD_GAS_V2; + if (gasleft() * 63 / 64 < requiredGas) { + revert IGatewayV2.InsufficientGasLimit(); + } + + try this.v2_dispatchCommand{gas: command.gas}(command, origin) { + return true; + } catch { + return false; + } } function deployAgent() external returns (address) { @@ -114,14 +125,6 @@ contract MockGateway is Gateway { return v1_transactionBaseGas(); } - // Wrapper to call an internal dispatch and return the boolean result - function exposed_dispatchCommand(CommandV2 calldata cmd, bytes32 origin) - external - returns (bool) - { - return _dispatchCommand(cmd, origin); - } - // Helper to call vulnerable-onlySelf handler from within the contract (so msg.sender == this) function setOperatingMode(bytes calldata data) external { HandlersV2.setOperatingMode(data);