From 3d5eeaf75985d4596034970274d2e79e2606a1dd Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Mon, 12 Aug 2024 11:36:51 -0300 Subject: [PATCH] Fix withdraw method --- src/Gateway.sol | 171 ++++++++++++++++++++-------------- src/utils/BranchlessMath.sol | 13 +++ src/utils/GasUtils.sol | 173 ++++++++++++++++++++--------------- test/GasUtils.t.sol | 31 ++++--- test/Gateway.t.sol | 50 ++++++---- test/TestUtils.sol | 14 ++- 6 files changed, 267 insertions(+), 185 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index b7e38a3..4843374 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -12,7 +12,6 @@ import {IGateway} from "./interfaces/IGateway.sol"; import {IUpgradable} from "./interfaces/IUpgradable.sol"; import {IGmpReceiver} from "./interfaces/IGmpReceiver.sol"; import {IExecutor} from "./interfaces/IExecutor.sol"; - import { TssKey, GmpMessage, @@ -402,7 +401,9 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { returns (GmpStatus status, bytes32 result) { uint256 initialGas = gasleft(); - initialGas = initialGas.saturatingAdd(431); + // Add the solidity selector overhead to the initial gas, this way we guarantee that + // the `initialGas` represents the actual gas that was available to this contract. + initialGas = initialGas.saturatingAdd(453); // Theoretically we could remove the destination network field // and fill it up with the network id of the contract, then the signature will fail. @@ -422,7 +423,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { unchecked { // Compute GMP gas used uint256 gasUsed = 7214; - gasUsed = gasUsed.saturatingAdd(GasUtils.calldataBaseCost()); + gasUsed = gasUsed.saturatingAdd(GasUtils.txBaseCost()); gasUsed = gasUsed.saturatingAdd(GasUtils.proxyOverheadGasCost(uint16(msg.data.length), 64)); gasUsed = gasUsed.saturatingAdd(initialGas - gasleft()); @@ -436,74 +437,6 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { } } - function _setNetworkInfo(bytes32 executor, bytes32 messageHash, UpdateNetworkInfo calldata info) private { - require(info.mortality >= block.number, "message expired"); - require(executor != bytes32(0), "executor cannot be zero"); - - // Verify signature and if the message was already executed - require(_executedMessages[messageHash] == bytes32(0), "message already executed"); - - // Update network info - NetworkInfo memory stored = _networkInfo[info.networkId]; - - // Verify and update domain separator if it's not zero - stored.domainSeparator = - BranchlessMath.ternary(info.domainSeparator != bytes32(0), info.domainSeparator, stored.domainSeparator); - require(stored.domainSeparator != bytes32(0), "domain separator cannot be zero"); - - // Update gas limit if it's not zero - stored.gasLimit = BranchlessMath.ternaryU64(info.gasLimit > 0, info.gasLimit, stored.gasLimit); - - // Update relative gas price and base fee if any of them are greater than zero - { - bool shouldUpdate = UFloat9x56.unwrap(info.relativeGasPrice) > 0 || info.baseFee > 0; - stored.relativeGasPrice = UFloat9x56.wrap( - BranchlessMath.ternaryU64( - shouldUpdate, UFloat9x56.unwrap(info.relativeGasPrice), UFloat9x56.unwrap(stored.relativeGasPrice) - ) - ); - stored.baseFee = BranchlessMath.ternaryU128(shouldUpdate, info.baseFee, stored.baseFee); - } - - // Save the message hash to prevent replay attacks - _executedMessages[messageHash] = executor; - - // Update network info - _networkInfo[info.networkId] = stored; - - emit NetworkUpdated( - messageHash, - info.networkId, - stored.domainSeparator, - stored.relativeGasPrice, - stored.baseFee, - stored.gasLimit - ); - } - - /** - * @dev set network info using admin account - */ - function setNetworkInfo(UpdateNetworkInfo calldata info) external { - require(msg.sender == _getAdmin(), "unauthorized"); - bytes32 messageHash = info.eip712TypedHash(DOMAIN_SEPARATOR); - _setNetworkInfo(bytes32(uint256(uint160(_getAdmin()))), messageHash, info); - } - - /** - * @dev Update network information - * @param signature Schnorr signature - * @param info new network info - */ - function setNetworkInfo(Signature calldata signature, UpdateNetworkInfo calldata info) external { - // Verify signature and check if the message was already executed - bytes32 messageHash = info.eip712TypedHash(DOMAIN_SEPARATOR); - _verifySignature(signature, messageHash); - - // Update network info - _setNetworkInfo(bytes32(signature.xCoord), messageHash, info); - } - /** * @dev Send message from this chain to another chain. * @param destinationAddress the target address on the destination chain @@ -575,6 +508,10 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { } } + /*////////////////////////////////////////////////////////////// + FEE AND PAYMENT LOGIC + //////////////////////////////////////////////////////////////*/ + /** * @notice Estimate the gas cost of execute a GMP message. * @dev This function is called on the destination chain before calling the gateway to execute a source contract. @@ -601,11 +538,103 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { return GasUtils.estimateWeiCost(relativeGasPrice, baseFee, uint16(messageSize), 0, gasLimit); } + function _setNetworkInfo(bytes32 executor, bytes32 messageHash, UpdateNetworkInfo calldata info) private { + require(info.mortality >= block.number, "message expired"); + require(executor != bytes32(0), "executor cannot be zero"); + + // Verify signature and if the message was already executed + require(_executedMessages[messageHash] == bytes32(0), "message already executed"); + + // Update network info + NetworkInfo memory stored = _networkInfo[info.networkId]; + + // Verify and update domain separator if it's not zero + stored.domainSeparator = + BranchlessMath.ternary(info.domainSeparator != bytes32(0), info.domainSeparator, stored.domainSeparator); + require(stored.domainSeparator != bytes32(0), "domain separator cannot be zero"); + + // Update gas limit if it's not zero + stored.gasLimit = BranchlessMath.ternaryU64(info.gasLimit > 0, info.gasLimit, stored.gasLimit); + + // Update relative gas price and base fee if any of them are greater than zero + { + bool shouldUpdate = UFloat9x56.unwrap(info.relativeGasPrice) > 0 || info.baseFee > 0; + stored.relativeGasPrice = UFloat9x56.wrap( + BranchlessMath.ternaryU64( + shouldUpdate, UFloat9x56.unwrap(info.relativeGasPrice), UFloat9x56.unwrap(stored.relativeGasPrice) + ) + ); + stored.baseFee = BranchlessMath.ternaryU128(shouldUpdate, info.baseFee, stored.baseFee); + } + + // Save the message hash to prevent replay attacks + _executedMessages[messageHash] = executor; + + // Update network info + _networkInfo[info.networkId] = stored; + + emit NetworkUpdated( + messageHash, + info.networkId, + stored.domainSeparator, + stored.relativeGasPrice, + stored.baseFee, + stored.gasLimit + ); + } + + /** + * @dev set network info using admin account + */ + function setNetworkInfo(UpdateNetworkInfo calldata info) external { + require(msg.sender == _getAdmin(), "unauthorized"); + bytes32 messageHash = info.eip712TypedHash(DOMAIN_SEPARATOR); + _setNetworkInfo(bytes32(uint256(uint160(_getAdmin()))), messageHash, info); + } + + /** + * @dev Update network information + * @param signature Schnorr signature + * @param info new network info + */ + function setNetworkInfo(Signature calldata signature, UpdateNetworkInfo calldata info) external { + // Verify signature and check if the message was already executed + bytes32 messageHash = info.eip712TypedHash(DOMAIN_SEPARATOR); + _verifySignature(signature, messageHash); + + // Update network info + _setNetworkInfo(bytes32(signature.xCoord), messageHash, info); + } + /** * Deposit funds to the gateway contract */ function deposit() external payable {} + /** + * Withdraw funds from the gateway contract + * @param amount The amount to withdraw + * @param recipient The recipient address + * @param data The data to send to the recipient (in case it is a contract) + */ + function withdraw(uint256 amount, address recipient, bytes calldata data) external returns (bytes memory output) { + require(msg.sender == _getAdmin(), "unauthorized"); + // Check if the recipient is a contract + if (recipient.code.length > 0) { + bool success; + (success, output) = recipient.call{value: amount, gas: gasleft()}(data); + if (!success) { + /// @solidity memory-safe-assembly + assembly { + revert(add(output, 32), mload(output)) + } + } + } else { + payable(recipient).transfer(amount); + output = ""; + } + } + /*////////////////////////////////////////////////////////////// ADMIN LOGIC //////////////////////////////////////////////////////////////*/ diff --git a/src/utils/BranchlessMath.sol b/src/utils/BranchlessMath.sol index 368f629..95742a8 100644 --- a/src/utils/BranchlessMath.sol +++ b/src/utils/BranchlessMath.sol @@ -178,6 +178,19 @@ library BranchlessMath { } } + /** + * @dev Unsigned saturating left shift, bounds to UINT256 MAX instead of overflowing. + */ + function saturatingShl(uint256 x, uint8 shift) internal pure returns (uint256 r) { + assembly { + // Detect overflow by checking if (x >> (256 - shift)) > 0 + r := gt(shr(sub(256, shift), x), 0) + + // Bounds to `type(uint256).max` if an overflow happened + r := or(shl(shift, x), sub(0, r)) + } + } + /** * @dev Cast a boolean (false or true) to a uint256 (0 or 1) with no jump. */ diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 8179f6f..7a993dc 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -13,12 +13,12 @@ library GasUtils { /** * @dev Base cost of the `IExecutor.execute` method. */ - uint256 internal constant EXECUTION_BASE_COST = 37647 + 6800; + uint256 internal constant EXECUTION_BASE_COST = 44469; /** * @dev Base cost of the `IGateway.submitMessage` method. */ - uint256 internal constant SUBMIT_BASE_COST = 9640 + 6800 + 6500; + uint256 internal constant SUBMIT_BASE_COST = 23147; using BranchlessMath for uint256; @@ -33,8 +33,8 @@ library GasUtils { calldataLen = calldataLen.saturatingAdd(31) >> 5; returnLen = returnLen.saturatingAdd(31) >> 5; - // Base cost: OPCODES + COLD READ STORAGE _implementation - uint256 gasCost = 2257 + 2500; + // Base cost: OPCODES + COLD SLOAD + COLD DELEGATECALL + uint256 gasCost = 57 + 2100 + 2600; // CALLDATACOPY gasCost = gasCost.saturatingAdd(calldataLen * 3); @@ -50,19 +50,13 @@ library GasUtils { } /** - * @dev Compute the gas cost of the `IGateway.submitMessage` method. - * @param messageSize The size of the message in bytes. + * @dev Compute the gas cost of the `IGateway.submitMessage` method, without the `GatewayProxy` overhead. + * @param messageSize The size of the message in bytes. This is the `gmp.data.length`. */ - function submitMessageGasCost(uint16 messageSize) internal pure returns (uint256 gasCost) { + function _submitMessageGasCost(uint16 messageSize) private pure returns (uint256 gasCost) { unchecked { gasCost = SUBMIT_BASE_COST; - // Convert message size to calldata size - uint256 calldataSize = ((messageSize + 31) & 0xffe0) + 164; - - // Proxy overhead - gasCost += proxyOverheadGasCost(uint16(calldataSize), 32); - // `countNonZeros` gas cost uint256 words = (messageSize + 31) >> 5; gasCost += (words * 106) + (((words + 254) / 255) * 214); @@ -84,6 +78,41 @@ library GasUtils { } } + /** + * @dev Compute the gas cost of the `IGateway.submitMessage` method including the `GatewayProxy` overhead. + * @param messageSize The size of the message in bytes. This is the `gmp.data.length`. + */ + function submitMessageGasCost(uint16 messageSize) internal pure returns (uint256 gasCost) { + unchecked { + // Compute the gas cost of the `IGateway.submitMessage` method. + gasCost = _submitMessageGasCost(messageSize); + + // Convert `gmp.data.length` to `abi.encodeCall(IGateway.submitMessage, (...)).length`. + uint256 calldataSize = ((messageSize + 31) & 0xffe0) + 164; + + // Compute the `GatewayProxy` gas overhead. + gasCost += proxyOverheadGasCost(uint16(calldataSize), 32); + + return gasCost; + } + } + + /** + * @dev Compute the minimal gas needed to execute the `IGateway.submitMessage` method. + * @param messageSize The size of the message in bytes. This is the `gmp.data.length`. + */ + function submitMessageGasNeeded(uint16 messageSize) internal pure returns (uint256 gasNeeded) { + unchecked { + // Compute the gas cost of the `IGateway.submitMessage` method. + gasNeeded = _submitMessageGasCost(messageSize); + // gasNeeded = gasNeeded.saturatingSub(2114); + // gasNeeded = _inverseOfAllButOne64th(); + uint256 calldataSize = ((messageSize + 31) & 0xffe0) + 164; + gasNeeded = gasNeeded.saturatingAdd(proxyOverheadGasCost(calldataSize, 32)); + gasNeeded = gasNeeded.saturatingSub(36); + } + } + /** * @dev Estimate the price in wei for send an GMP message. * @param gasPrice The gas price in UFloat9x56 format. @@ -132,53 +161,66 @@ library GasUtils { } } - /** - * @dev Compute the gas needed for the transaction, this is different from the gas used. - * `gas needed > gas used` because of EIP-150 - */ - function executionGasNeeded(uint256 messageSize, uint256 gasLimit) internal pure returns (uint256 gasNeeded) { + function _executionGasCost(uint256 messageSize, uint256 gasUsed) private pure returns (uint256) { + // Add the base execution gas cost + uint256 gas = EXECUTION_BASE_COST.saturatingAdd(gasUsed); + + // Safety: The operations below can't overflow because the message size can't be greater than 2^16 unchecked { - gasNeeded = EXECUTION_BASE_COST + 2114; + // Add padding to the message size, making it a multiple of 32 + messageSize = (uint256(messageSize) + 31) & 0xffffe0; - // Convert message size to calldata size - uint256 calldataSize = ((messageSize + 31) & 0xffe0) + 388; + // selector + Signature + GmpMessage + uint256 words = messageSize.saturatingAdd(388 + 31) >> 5; - // Base cost - uint256 words = (calldataSize + 31) >> 5; - gasNeeded += (words * 106) + (((words + 254) / 255) * 214); + // Add `countZeros` gas cost + gas = gas.saturatingAdd((words * 106) + (((words + 254) / 255) * 214)); - // CALLDATACOPY - words = (messageSize + 31) >> 5; - gasNeeded += words * 3; + // calldatacopy (3 gas per word) + words = messageSize >> 5; + gas = gas.saturatingAdd(words * 3); // keccak256 (6 gas per word) - gasNeeded += words * 6; + gas = gas.saturatingAdd(words * 6); // Memory expansion cost words = 0xa4 + (words << 5); // onGmpReceived encoded call size - words = (words + 31) & 0xffe0; + words = (words + 31) & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0; words += 0x0200; // Memory size words = (words + 31) >> 5; // to words - gasNeeded += ((words * words) >> 9) + (words * 3); - - // Add the gas limit - gasNeeded += gasLimit.saturatingMul(64).saturatingDiv(63); - - // Add `all but one 64th`, as the defined by EIP-150 - gasNeeded = gasNeeded.saturatingMul(64).saturatingDiv(63); - gasNeeded += 2600; // DELEGATECALL + COLD CONTRACT ADDRESS - gasNeeded += 2121; // OPCODES + COLD SLOAD + gas = gas.saturatingAdd(((words * words) >> 9) + (words * 3)); + return gas; + } + } - // CALLDATACOPY - words = (calldataSize + 31) >> 5; - gasNeeded += 3; - gasNeeded += words * 3; + /** + * @dev Compute the inverse of `N - floor(N / 64)` defined by EIP-150, used to + * compute the gas needed for a transaction. + */ + function _inverseOfAllButOne64th(uint256 x) private pure returns (uint256 inverse) { + unchecked { + // inverse = (x * 64) / 63 + inverse = x.saturatingShl(6).saturatingDiv(63); - // Memory Expansion - gasNeeded += words * 3; - gasNeeded += (words * words) >> 9; + // Subtract 1 if `inverse` is a multiple of 64 and greater than 0 + inverse -= BranchlessMath.toUint(inverse > 0 && (inverse % 64) == 0); + } + } - return gasNeeded; + /** + * @dev Compute the gas needed for the transaction, this is different from the gas used. + * `gas needed > gas used` because of EIP-150 + */ + function executionGasNeeded(uint256 messageSize, uint256 gasLimit) internal pure returns (uint256 gasNeeded) { + unchecked { + gasNeeded = _inverseOfAllButOne64th(gasLimit); + gasNeeded = gasNeeded.saturatingAdd(_executionGasCost(messageSize, gasLimit)); + gasNeeded = gasNeeded.saturatingAdd(2114); + gasNeeded = _inverseOfAllButOne64th(gasNeeded); + messageSize = (uint256(messageSize).saturatingAdd(31) >> 5) << 5; + messageSize = messageSize.saturatingAdd(388); + gasNeeded = gasNeeded.saturatingAdd(proxyOverheadGasCost(messageSize, 64)); + gasNeeded = gasNeeded.saturatingSub(39); } } @@ -192,40 +234,19 @@ library GasUtils { pure returns (uint256 executionCost) { - // Add the base execution gas cost - executionCost = EXECUTION_BASE_COST.saturatingAdd(gasUsed); + // Add the base `IExecutor.execute` gas cost. + executionCost = _executionGasCost(messageSize, gasUsed); - // Safety: The operations below can't overflow because the message size can't be greater than 2^16 + // Add `GatewayProxy` gas overhead unchecked { - // Add padding to the message size, making it a multiple of 32 - uint256 messagePadded = (uint256(messageSize) + 31) & 0xffffe0; - - // Proxy Overhead - uint256 words = messagePadded + 388; // selector + Signature + GmpMessage - executionCost = executionCost.saturatingAdd(proxyOverheadGasCost(words, 64)); - - // Base Cost calculation - words = (words + 31) >> 5; - executionCost = executionCost.saturatingAdd((words * 106) + (((words + 254) / 255) * 214)); - - // calldatacopy (3 gas per word) - words = messagePadded >> 5; - executionCost = executionCost.saturatingAdd(words * 3); - - // keccak256 (6 gas per word) - executionCost = executionCost.saturatingAdd(words * 6); - - // Memory expansion cost - words = 0xa4 + (words << 5); // onGmpReceived encoded call size - words = (words + 31) & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0; - words += 0x0200; // Memory size - words = (words + 31) >> 5; // to words - executionCost = executionCost.saturatingAdd(((words * words) >> 9) + (words * 3)); + // Safety: The operations below can't overflow because the message size can't be greater than 2^16 + uint256 calldataSize = ((uint256(messageSize) + 31) & 0xffffe0) + 388; // selector + Signature + GmpMessage + executionCost = executionCost.saturatingAdd(proxyOverheadGasCost(calldataSize, 64)); } } /** - * @dev Count the number of non-zero bytes in a byte sequence. + * @dev Count the number of non-zero bytes in a byte sequence from memory. * gas cost = 217 + (words * 112) + ((words - 1) * 193) */ function countNonZeros(bytes memory data) internal pure returns (uint256 nonZeros) { @@ -273,7 +294,7 @@ library GasUtils { /** * @dev Count the number of non-zero bytes from calldata. - * gas cost = 224 + (words * 106) + (((words - 1) / 255) * 214) + * gas cost = 224 + (words * 106) + (((words + 254) / 255) * 214) */ function countNonZerosCalldata(bytes calldata data) internal pure returns (uint256 nonZeros) { /// @solidity memory-safe-assembly @@ -317,7 +338,7 @@ library GasUtils { /** * @dev Compute the transaction base cost. */ - function calldataBaseCost() internal pure returns (uint256) { + function txBaseCost() internal pure returns (uint256) { unchecked { uint256 nonZeros = countNonZerosCalldata(msg.data); uint256 zeros = msg.data.length - nonZeros; diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index 165fe43..5c75d7d 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -36,7 +36,7 @@ contract GasUtilsMock { pure returns (uint256 baseCost, uint256 nonZeros, uint256 zeros) { - baseCost = GasUtils.calldataBaseCost(); + baseCost = GasUtils.txBaseCost(); nonZeros = GasUtils.countNonZerosCalldata(msg.data); zeros = msg.data.length - nonZeros; } @@ -167,9 +167,9 @@ contract GasUtilsBase is Test { } /** - * Test the `GasUtils.calldataBaseCost` method. + * Test the `GasUtils.txBaseCost` method. */ - function test_calldataBaseCost() external view { + function test_txBaseCost() external view { // Build and sign GMP message GmpMessage memory gmp = GmpMessage({ source: address(0x1111111111111111111111111111111111111111).toSender(false), @@ -228,22 +228,23 @@ contract GasUtilsBase is Test { } function test_gasUtils() external pure { - assertEq(GasUtils.estimateGas(0, 0, 0), 76186); - assertEq(GasUtils.estimateGas(0, 33, 0), 76559); - assertEq(GasUtils.estimateGas(33, 0, 0), 77219); - assertEq(GasUtils.estimateGas(20, 13, 0), 76959); + uint256 baseCost = GasUtils.EXECUTION_BASE_COST; + assertEq(GasUtils.estimateGas(0, 0, 0), 31739 + baseCost); + assertEq(GasUtils.estimateGas(0, 33, 0), 32112 + baseCost); + assertEq(GasUtils.estimateGas(33, 0, 0), 32772 + baseCost); + assertEq(GasUtils.estimateGas(20, 13, 0), 32512 + baseCost); UFloat9x56 one = UFloatMath.ONE; - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 76186); - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76559); - assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77219); - assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76959); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 31739 + baseCost); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 32112 + baseCost); + assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 32772 + baseCost); + assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 32512 + baseCost); UFloat9x56 two = UFloat9x56.wrap(0x8080000000000000); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 76186 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76559 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77219 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76959 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), (31739 + baseCost) * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), (32112 + baseCost) * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), (32772 + baseCost) * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), (32512 + baseCost) * 2); } } diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 28e4254..cefc9bf 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -216,7 +216,7 @@ contract GatewayBase is Test { function test_estimateMessageCost() external { vm.txGasPrice(1); uint256 cost = gateway.estimateMessageCost(DEST_NETWORK_ID, 96, 100000); - assertEq(cost, 178479); + assertEq(cost, 178501); } function test_checkPayloadSize() external { @@ -263,7 +263,7 @@ contract GatewayBase is Test { * @dev Test the gas metering for the `execute` function. */ function test_gasMeter(uint16 messageSize) external { - vm.assume(messageSize < 1000); + vm.assume(messageSize <= 0x6000); vm.txGasPrice(1); address sender = TestUtils.createTestAccount(100 ether); @@ -277,7 +277,6 @@ contract GatewayBase is Test { salt: 0, data: new bytes(messageSize) }); - Signature memory sig = sign(gmp); // Calculate memory expansion cost and base cost @@ -316,12 +315,19 @@ contract GatewayBase is Test { assertEq(ctx.executionCost, executionCost, "ctx.executionCost != executionCost"); assertEq(gatewayBalance - address(gateway).balance, executionCost + baseCost, "wrong refund amount"); assertEq(senderBalance, address(sender).balance, "sender balance should not change"); + assertEq( + ctx.gasLimit - baseCost, GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit), "gas needed mismatch" + ); - // Submit the transaction + // Submit GMP message { + // Calculate the minimal gmp value minus one uint256 nonZeros = GasUtils.countNonZeros(gmp.data); uint256 zeros = gmp.data.length - nonZeros; ctx.value = GasUtils.estimateGas(uint16(nonZeros), uint16(zeros), gmp.gasLimit) - 1; + + // Add sufficient gas + ctx.gasLimit += gmp.data.length * 8; } // Must revert if fund are insufficient @@ -336,7 +342,6 @@ contract GatewayBase is Test { // Must work if the funds are sufficient ctx.value += 1; - ctx.gasLimit += gmp.data.length * 8; ctx.submitMessage(gmp); assertEq( @@ -347,7 +352,7 @@ contract GatewayBase is Test { } function test_submitMessageMeter(uint16 messageSize) external { - vm.assume(messageSize < 0x6000); + vm.assume(messageSize <= 0x6000); vm.txGasPrice(1); address sender = TestUtils.createTestAccount(1000 ether); @@ -362,17 +367,22 @@ contract GatewayBase is Test { data: new bytes(messageSize) }); - Signature memory sig = sign(gmp); - // Calculate memory expansion cost and base cost - (uint256 baseCost,) = GatewayUtils.computeGmpGasCost(sig, gmp); + uint256 baseCost; + { + bytes memory encoded = + abi.encodeCall(IGateway.submitMessage, (gmp.dest, gmp.destNetwork, gmp.gasLimit, gmp.data)); + assertEq(encoded.length, ((gmp.data.length + 31) & 0xffe0) + 164, "wrong encoded length"); + emit log_named_bytes(" calldata", encoded); + baseCost = TestUtils.calculateBaseCost(encoded); + } // Transaction Parameters CallOptions memory ctx = CallOptions({ from: sender, to: address(gateway), value: 0, - gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost, + gasLimit: GasUtils.submitMessageGasNeeded(uint16(gmp.data.length)) + baseCost, executionCost: 0, baseCost: 0 }); @@ -381,23 +391,17 @@ contract GatewayBase is Test { { uint256 nonZeros = GasUtils.countNonZeros(gmp.data); uint256 zeros = gmp.data.length - nonZeros; - ctx.value = GasUtils.estimateGas(uint16(nonZeros), uint16(zeros), gmp.gasLimit) - 1; + ctx.value = GasUtils.estimateGas(uint16(nonZeros), uint16(zeros), gmp.gasLimit); } - // Must revert if fund are insufficient - vm.expectRevert("insufficient tx value"); - ctx.submitMessage(gmp); - - // Must work if the funds are sufficient - ctx.value += 1; - ctx.gasLimit += gmp.data.length * 8; + uint256 snapshot = vm.snapshot(); + // Must work if the funds and gas limit are sufficient bytes32 id = gmp.eip712TypedHash(_dstDomainSeparator); vm.expectEmit(true, true, true, true); emit IGateway.GmpCreated( id, GmpSender.unwrap(gmp.source), gmp.dest, gmp.destNetwork, gmp.gasLimit, gmp.salt, gmp.data ); - bytes32 returnedId = ctx.submitMessage(gmp); - assertEq(returnedId, id, "unexpected GMP id"); + assertEq(ctx.submitMessage(gmp), id, "unexpected GMP id"); // Verify the execution cost assertEq( @@ -405,6 +409,12 @@ contract GatewayBase is Test { GasUtils.submitMessageGasCost(uint16(gmp.data.length)), "unexpected submit message gas cost" ); + + // Must revert if fund are insufficient + vm.revertTo(snapshot); + ctx.value -= 1; + vm.expectRevert("insufficient tx value"); + ctx.submitMessage(gmp); } function test_refund() external { diff --git a/test/TestUtils.sol b/test/TestUtils.sol index cd52c04..621a64c 100644 --- a/test/TestUtils.sol +++ b/test/TestUtils.sol @@ -229,14 +229,22 @@ library TestUtils { internal returns (uint256 executionCost, uint256 baseCost, bytes memory out) { + // Guarantee there's enough gas to execute the call + { + uint256 gasRequired = (gasLimit * 64) / 63; + gasRequired += 50_000; + require(gasleft() > gasRequired, "insufficient gas left to execute call"); + } + // Compute the base tx cost (21k + 4 * zeros + 16 * nonZeros) baseCost = calculateBaseCost(data); - // Decrement sender base cost + // Decrement sender base cost and value { - uint256 txFees = baseCost.saturatingAdd(gasLimit).saturatingMul(tx.gasprice); + uint256 txFees = gasLimit.saturatingMul(tx.gasprice); require(sender.balance >= txFees.saturatingAdd(value), "account has no sufficient funds"); vm.deal(sender, sender.balance - txFees); + gasLimit = gasLimit.saturatingSub(baseCost); } // Execute @@ -244,7 +252,7 @@ library TestUtils { { (VmSafe.CallerMode callerMode, address msgSender, address txOrigin) = setCallerMode(VmSafe.CallerMode.RecurrentPrank, sender, sender); - (executionCost, success, out) = _call(dest, gasLimit.saturatingSub(baseCost), value, data); + (executionCost, success, out) = _call(dest, gasLimit, value, data); setCallerMode(callerMode, msgSender, txOrigin); }