From 708d5cf2c87c0e5aa40eb5a2d0fe621f09a2e3d5 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Thu, 8 Aug 2024 17:24:03 -0300 Subject: [PATCH 01/17] Make submitMessage gas cost predictable --- src/Gateway.sol | 51 ++++++++++++------ src/utils/GasUtils.sol | 63 ++++++++++++++++++++--- test/GasUtils.t.sol | 114 ++++++++++++++++++++++++++++++++++------- test/Gateway.t.sol | 4 +- test/TestUtils.sol | 13 +++-- 5 files changed, 196 insertions(+), 49 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index a10acde..76e8e87 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -340,7 +340,8 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // https://eips.ethereum.org/EIPS/eip-150 uint256 gasNeeded = gasLimit.saturatingMul(64).saturatingDiv(63); // to guarantee it was provided enough gas to execute the GMP message - gasNeeded = gasNeeded.saturatingAdd(6412); + // gasNeeded = gasNeeded.saturatingAdd(6412); + gasNeeded = gasNeeded.saturatingAdd(10000); require(gasleft() >= gasNeeded, "insufficient gas to execute GMP message"); } @@ -348,7 +349,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { bool success; address dest = message.dest; - /// @solidity memory-safe-assembly + // /// @solidity memory-safe-assembly assembly { // Using low-level assembly because the GMP is considered executed // regardless if the call reverts or not. @@ -413,11 +414,9 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { unchecked { // Compute GMP gas used uint256 gasUsed = 7214; - { - gasUsed = gasUsed.saturatingAdd(GasUtils.calldataGasCost()); - gasUsed = gasUsed.saturatingAdd(GasUtils.proxyOverheadGasCost(uint16(msg.data.length), 64)); - gasUsed = gasUsed.saturatingAdd(initialGas - gasleft()); - } + gasUsed = gasUsed.saturatingAdd(GasUtils.calldataGasCost()); + gasUsed = gasUsed.saturatingAdd(GasUtils.proxyOverheadGasCost(uint16(msg.data.length), 64)); + gasUsed = gasUsed.saturatingAdd(initialGas - gasleft()); // Compute refund amount uint256 refund = BranchlessMath.min(gasUsed.saturatingMul(tx.gasprice), address(this).balance); @@ -494,7 +493,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { address destinationAddress, uint16 destinationNetwork, uint256 executionGasLimit, - bytes memory data + bytes calldata data ) external payable returns (bytes32) { // Check if the message data is too large require(data.length < MAX_PAYLOAD_SIZE, "msg data too large"); @@ -506,7 +505,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // Check if the sender has deposited enougth funds to execute the GMP message { - uint256 nonZeros = GasUtils.countNonZeros(data); + uint256 nonZeros = GasUtils.countNonZerosCalldata(data); uint256 zeros = data.length - nonZeros; uint256 msgPrice = GasUtils.estimateWeiCost( info.relativeGasPrice, info.baseFee, uint16(nonZeros), uint16(zeros), executionGasLimit @@ -524,15 +523,33 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { uint256 salt = BranchlessMath.select(prevHash == FIRST_MESSAGE_PLACEHOLDER, 0, uint256(prevHash)); // Create GMP message and update prevMessageHash - GmpMessage memory message = - GmpMessage(source, NETWORK_ID, destinationAddress, destinationNetwork, executionGasLimit, salt, data); - prevHash = message.eip712TypedHash(domainSeparator); - prevMessageHash = prevHash; + bytes memory payload; + { + GmpMessage memory message = + GmpMessage(source, NETWORK_ID, destinationAddress, destinationNetwork, executionGasLimit, salt, data); + prevHash = message.eip712TypedHash(domainSeparator); + prevMessageHash = prevHash; + payload = message.data; + } + // Emit event without copy the data + bytes32 eventSelector = GmpCreated.selector; + assembly { + let ptr := sub(payload, 0x80) + mstore(ptr, destinationNetwork) // dest network + mstore(add(ptr, 0x20), executionGasLimit) // gas limit + mstore(add(ptr, 0x40), salt) // salt + mstore(add(ptr, 0x60), 0x80) // data offset + let size := and(add(mload(payload), 31), 0xffffffe0) + size := add(size, 160) + log4(ptr, size, eventSelector, prevHash, source, destinationAddress) + mstore(0, prevHash) + return(0, 32) + } - emit GmpCreated( - prevHash, GmpSender.unwrap(source), destinationAddress, destinationNetwork, executionGasLimit, salt, data - ); - return prevHash; + // emit GmpCreated( + // prevHash, GmpSender.unwrap(source), destinationAddress, destinationNetwork, executionGasLimit, salt, data + // ); + // return prevHash; } /** diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 41edbaf..4362fee 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -7,7 +7,7 @@ import {UFloat9x56, UFloatMath} from "./Float9x56.sol"; import {BranchlessMath} from "./BranchlessMath.sol"; /** - * @dev Utilities for branchless operations, useful when a constant gas cost is required. + * @dev Utilities for compute the GMP gas price, gas cost and gas needed. */ library GasUtils { uint256 internal constant EXECUTION_BASE_COST = 39361 + 6700; @@ -155,25 +155,25 @@ library GasUtils { // Proxy Overhead uint256 words = messagePadded + 388; // selector + Signature + GmpMessage words = BranchlessMath.min(words, type(uint16).max); - executionCost += proxyOverheadGasCost(uint16(words), 64); + executionCost = executionCost.saturatingAdd(proxyOverheadGasCost(uint16(words), 64)); // Base Cost calculation words = (words + 31) >> 5; - executionCost += ((words - 1) / 15) * 1845; + executionCost = executionCost.saturatingAdd(((words - 1) / 15) * 1845); // calldatacopy (3 gas per word) words = messagePadded >> 5; - executionCost += words * 3; + executionCost = executionCost.saturatingAdd(words * 3); // keccak256 (6 gas per word) - executionCost += words * 6; + 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 += ((words * words) >> 9) + (words * 3); + executionCost = executionCost.saturatingAdd(((words * words) >> 9) + (words * 3)); } } @@ -181,7 +181,7 @@ library GasUtils { * @dev Compute the transaction base cost. * OBS: This function must be used ONLY inside Gateway.execute method, because it also consider itself gas cost. */ - function executionGasCost(uint256 messageSize) internal pure returns (uint256 baseCost, uint256 executionCost) { + function internalGasCost(uint256 messageSize) internal pure returns (uint256 baseCost, uint256 executionCost) { // Calculate Gateway.execute dynamic cost executionCost = EXECUTION_BASE_COST; unchecked { @@ -215,6 +215,7 @@ library GasUtils { /** * @dev Count the number of non-zero bytes in a byte sequence. + * gas cost = 217 + (words * 112) + ((words - 1) * 193) */ function countNonZeros(bytes memory data) internal pure returns (uint256 nonZeros) { /// @solidity memory-safe-assembly @@ -259,6 +260,54 @@ library GasUtils { } } + /** + * @dev Count the number of non-zero bytes in a byte sequence. + * gas cost = 224 + (words * 106) + (((words - 1) / 255) * 214) + */ + function countNonZerosCalldata(bytes calldata data) internal pure returns (uint256 nonZeros) { + /// @solidity memory-safe-assembly + assembly { + let offset := data.offset + nonZeros := 0 + for { + let ptr := data.offset + let end := add(ptr, data.length) + + // range := min(ptr + data.length, ptr + 8160) + let range := add(ptr, 8160) + range := xor(end, mul(xor(range, end), lt(range, end))) + } true { + range := add(ptr, 8160) + range := xor(end, mul(xor(range, end), lt(range, end))) + } { + // Normalize and count non-zero bytes in parallel + let v := 0 + for {} lt(ptr, range) { ptr := add(ptr, 32) } { + let r := calldataload(ptr) + r := or(r, shr(4, r)) + r := or(r, shr(2, r)) + r := or(r, shr(1, r)) + r := and(r, 0x0101010101010101010101010101010101010101010101010101010101010101) + v := add(v, r) + } + + // Sum bytes in parallel + { + let l := and(v, 0x00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff) + v := shr(8, xor(v, l)) + v := add(v, l) + } + v := add(v, shr(128, v)) + v := add(v, shr(64, v)) + v := add(v, shr(32, v)) + v := add(v, shr(16, v)) + v := and(v, 0xffff) + nonZeros := add(nonZeros, v) + if iszero(lt(ptr, end)) { break } + } + } + } + /** * @dev Count non-zeros of a single 32 bytes word. */ diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index 4232c39..863bb76 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -75,6 +75,14 @@ contract GasUtilsBase is Test { _srcDomainSeparator = GatewayUtils.computeDomainSeparator(SRC_NETWORK_ID, address(gateway)); _dstDomainSeparator = GatewayUtils.computeDomainSeparator(DEST_NETWORK_ID, address(gateway)); + // Obs: This is a special contract that wastes an exact amount of gas you send to it, helpful for testing GMP refunds and gas limits. + // See the file `HelperContract.opcode` for more details. + { + bytes memory bytecode = + hex"603b80600c6000396000f3fe5a600201803d523d60209160643560240135146018575bfd5b60345a116018575a604803565b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5bf3"; + receiver = IGmpReceiver(TestUtils.deployContract(bytecode)); + } + vm.stopPrank(); } @@ -91,42 +99,112 @@ contract GasUtilsBase is Test { } /** - * @dev Compare the estimated gas cost VS the actual gas cost of the `execute` method. + * @dev Create a GMP message with the provided parameters. */ - function test_baseExecutionCost(uint16 messageSize) external { - vm.assume(messageSize <= 0x6000); - vm.txGasPrice(1); - address sender = TestUtils.createTestAccount(100 ether); + function _buildGmpMessage(address sender, uint256 gasLimit, uint256 gasUsed, uint256 messageSize) + private + view + returns (GmpMessage memory message, Signature memory signature, CallOptions memory context) + { + require(gasUsed == 0 || messageSize >= 32, "If gasUsed > 0, then messageSize must be >= 32"); + require(messageSize <= 0x6000, "message is too big"); + + // Setup data and receiver addresses. + bytes memory data = new bytes(messageSize); + address gmpReceiver; + if (gasUsed > 0) { + gmpReceiver = address(receiver); + assembly { + mstore(add(data, 32), gasUsed) + } + } else { + // Create a new unique receiver address for each message, otherwise the gas refund will not work. + gmpReceiver = address(bytes20(keccak256(abi.encode(sender, gasLimit, messageSize)))); + } - // Build and sign GMP message - GmpMessage memory gmp = GmpMessage({ + // Build the GMP message + message = GmpMessage({ source: sender.toSender(false), srcNetwork: SRC_NETWORK_ID, - dest: address(bytes20(keccak256("dummy_address"))), + dest: gmpReceiver, destNetwork: DEST_NETWORK_ID, - gasLimit: 0, + gasLimit: gasLimit, salt: 0, - data: new bytes(messageSize) + data: data }); - Signature memory sig = sign(gmp); + // Sign the message + signature = sign(message); // Calculate memory expansion cost and base cost - (uint256 baseCost,) = GatewayUtils.computeGmpGasCost(sig, gmp); + (uint256 baseCost, uint256 executionCost) = GatewayUtils.computeGmpGasCost(signature, message); - // Transaction Parameters - CallOptions memory ctx = CallOptions({ + // Set Transaction Parameters + context = CallOptions({ from: sender, to: address(gateway), value: 0, - // gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost, - gasLimit: baseCost + 1_000_000, - executionCost: 0, - baseCost: 0 + gasLimit: GasUtils.executionGasNeeded(message.data.length, message.gasLimit).saturatingAdd(baseCost), + executionCost: executionCost, + baseCost: baseCost }); + } + // bdfbbea6 + // 079264c4b4bfcd7fe3a7b7b92b6c439f3a5b3abcd29189bf7b54d781ff03d722 + // 51f46b1ff68263bee778aeef6b875d06636012244855e8cb1d445d8472f2fea1 + // 21039e3d8d9db737ad1d19b9b8e5fbc04e6c8e6e4530df76cf5e5a988e324b96 + // 0000000000000000000000000000000000000000000000000000000000000080 + // 000000000000000000000000dba77ed08c6610c2cbabc48216d85aa0f83b3e35 + // 00000000000000000000000000000000000000000000000000000000000004d2 + // 0000000000000000000000007ef5c78b60535b879a77ce245070c34e120a5cac + // 0000000000000000000000000000000000000000000000000000000000000539 + // 000000000000000000000000000000000000000000000000000000000000097c + // 0000000000000000000000000000000000000000000000000000000000000000 + // 00000000000000000000000000000000000000000000000000000000000000e0 + // 000000000000000000000000000000000000000000000000000000000000388a + // 000000000000000000000000000000000000000000000000000000000000097c + // 0000000000000000000000000000000000000000000000000000000000000000 + // 0000000000000000000000000000000000000000000000000000000000000000 + // 0000000000000000000000000000000000000000000000000000000000000000 + // 0000000000000000000000000000000000000000000000000000000000000000 + // ... 14336 zeros + /** + * @dev Compare the estimated gas cost VS the actual gas cost of the `execute` method. + */ + + function test_baseExecutionCost(uint16 messageSize, uint16 gasLimit) external { + vm.assume(gasLimit >= 5000); + // vm.assume(messageSize <= (0x6000 - 32)); + vm.assume(messageSize <= 8160); + messageSize += 32; + vm.txGasPrice(1); + address sender = TestUtils.createTestAccount(100 ether); + + // Build the GMP message + GmpMessage memory gmp; + Signature memory sig; + CallOptions memory ctx; + (gmp, sig, ctx) = _buildGmpMessage(sender, gasLimit, gasLimit, messageSize); + + // Increase the gas limit to avoid out-of-gas errors + ctx.gasLimit = ctx.gasLimit.saturatingAdd(10_000_000); // Execute the GMP message ctx.execute(sig, gmp); + // { + // (GmpStatus status, bytes32 result) = ctx.execute(sig, gmp); + // if (status != GmpStatus.SUCCESS) { + // bytes memory bla = abi.encodeCall(IExecutor.execute, (sig, gmp)); + // emit log_named_bytes("encoded call", bla); + // emit log_named_uint("gmp status", uint256(status)); + // emit log_named_uint("gmp result", uint256(result)); + // emit log_named_uint("requested size", uint256(messageSize)); + // emit log_named_uint("actual size", uint256(gmp.data.length)); + // emit log_named_uint("gas limit", uint256(gasLimit)); + // } + // assertEq(uint256(status), uint256(GmpStatus.SUCCESS), "GMP execution failed"); + // assertEq(result, bytes32(uint256(gasLimit)), "unexpected result"); + // } // Calculate the expected base cost uint256 dynamicCost = diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index d1bddc0..3b760a8 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -100,7 +100,7 @@ library GatewayUtils { pure returns (uint256 baseCost, uint256 executionCost) { - (, executionCost) = GasUtils.executionGasCost(message.data.length); + (, executionCost) = GasUtils.internalGasCost(message.data.length); bytes memory encodedCall = abi.encodeCall(IExecutor.execute, (signature, message)); baseCost = TestUtils.calculateBaseCost(encodedCall); } @@ -137,7 +137,7 @@ contract GatewayBase is Test { bytes32 private _srcDomainSeparator; bytes32 private _dstDomainSeparator; - uint256 private constant SUBMIT_GAS_COST = 6095 + 9206; + uint256 private constant SUBMIT_GAS_COST = 14925; uint16 private constant SRC_NETWORK_ID = 1234; uint16 internal constant DEST_NETWORK_ID = 1337; uint8 private constant GMP_STATUS_SUCCESS = 1; diff --git a/test/TestUtils.sol b/test/TestUtils.sol index 93d3520..cd52c04 100644 --- a/test/TestUtils.sol +++ b/test/TestUtils.sol @@ -6,6 +6,7 @@ pragma solidity >=0.8.0; import {VmSafe, Vm} from "forge-std/Vm.sol"; import {Schnorr} from "@frost-evm/Schnorr.sol"; import {SECP256K1} from "@frost-evm/SECP256K1.sol"; +import {BranchlessMath} from "../src/utils/BranchlessMath.sol"; struct VerifyingKey { uint256 px; @@ -21,6 +22,8 @@ struct SigningKey { * @dev Utilities for testing purposes */ library TestUtils { + using BranchlessMath for uint256; + // Cheat code address, 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D. address internal constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); @@ -196,7 +199,7 @@ library TestUtils { private returns (uint256 gasUsed, bool success, bytes memory out) { - require(gasleft() > (gasLimit + 5000), "insufficient gas"); + require(gasleft() > gasLimit.saturatingAdd(5000), "insufficient gas"); require(addr.code.length > 0, "Not a contract address"); /// @solidity memory-safe-assembly assembly { @@ -231,8 +234,8 @@ library TestUtils { // Decrement sender base cost { - uint256 txFees = (baseCost + gasLimit) * tx.gasprice; - require(sender.balance >= (txFees + value), "account has no sufficient funds"); + uint256 txFees = baseCost.saturatingAdd(gasLimit).saturatingMul(tx.gasprice); + require(sender.balance >= txFees.saturatingAdd(value), "account has no sufficient funds"); vm.deal(sender, sender.balance - txFees); } @@ -241,12 +244,12 @@ library TestUtils { { (VmSafe.CallerMode callerMode, address msgSender, address txOrigin) = setCallerMode(VmSafe.CallerMode.RecurrentPrank, sender, sender); - (executionCost, success, out) = _call(dest, gasLimit - baseCost, value, data); + (executionCost, success, out) = _call(dest, gasLimit.saturatingSub(baseCost), value, data); setCallerMode(callerMode, msgSender, txOrigin); } // Refund unused gas - uint256 refund = (gasLimit - executionCost) * tx.gasprice; + uint256 refund = gasLimit.saturatingSub(executionCost).saturatingMul(tx.gasprice); if (refund > 0) { vm.deal(sender, sender.balance + refund); } From 9f70338b15c8ba7c3fa88972fce5ad86ca245e39 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Thu, 8 Aug 2024 17:29:45 -0300 Subject: [PATCH 02/17] Improve docs --- src/Gateway.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 76e8e87..ac64aae 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -531,7 +531,8 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { prevMessageHash = prevHash; payload = message.data; } - // Emit event without copy the data + + // Emit `GmpCreated` event without copy the data bytes32 eventSelector = GmpCreated.selector; assembly { let ptr := sub(payload, 0x80) From 6f34e672ab4b52782f9fff3c5876ca2090ded21c Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 11:11:07 -0300 Subject: [PATCH 03/17] Fix gas meter --- src/Gateway.sol | 16 ++- src/utils/GasUtils.sol | 257 +++++++++++++---------------------------- test/GasUtils.t.sol | 118 +++++++++++-------- test/Gateway.t.sol | 166 +++++++++++++++++++++----- 4 files changed, 298 insertions(+), 259 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index ac64aae..6683165 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -206,13 +206,14 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { return bytes32(tssKey.xCoord); } - // Converts a `TssKey` into an `KeyInfo` unique identifier + // Initialize networks function _updateNetworks(Network[] calldata networks) private { for (uint256 i = 0; i < networks.length; i++) { Network calldata network = networks[i]; - bytes32 domainSeparator = computeDomainSeparator(network.id, network.gateway); NetworkInfo storage info = _networkInfo[network.id]; - info.domainSeparator = domainSeparator; + require(info.domainSeparator == bytes32(0), "network already initialized"); + require(network.id != NETWORK_ID || network.gateway == address(this), "wrong gateway address"); + info.domainSeparator = computeDomainSeparator(network.id, network.gateway); info.gasLimit = 15_000_000; // Default to 15M gas info.relativeGasPrice = UFloatMath.ONE; info.baseFee = 0; @@ -435,15 +436,18 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // Verify signature and if the message was already executed require(_executedMessages[messageHash] == bytes32(0), "message already executed"); - // Update network info and store the message hash to prevent replay attacks + // Update network info NetworkInfo storage networkInfo = _networkInfo[data.networkId]; // Verify if the domain separator is not zero require((networkInfo.domainSeparator | data.domainSeparator) != bytes32(0), "domain separator cannot be zero"); + // Store the message hash to prevent replay attacks + _executedMessages[messageHash] = executor; + // Update domain separator if it's not zero if (data.domainSeparator != bytes32(0)) { - networkInfo.domainSeparator = messageHash; + networkInfo.domainSeparator = data.domainSeparator; } // Update gas limit if it's not zero @@ -496,7 +500,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { bytes calldata data ) external payable returns (bytes32) { // Check if the message data is too large - require(data.length < MAX_PAYLOAD_SIZE, "msg data too large"); + require(data.length <= MAX_PAYLOAD_SIZE, "msg data too large"); // Check if the destination network is supported NetworkInfo storage info = _networkInfo[destinationNetwork]; diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 4362fee..b14429b 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -10,7 +10,15 @@ import {BranchlessMath} from "./BranchlessMath.sol"; * @dev Utilities for compute the GMP gas price, gas cost and gas needed. */ library GasUtils { - uint256 internal constant EXECUTION_BASE_COST = 39361 + 6700; + /** + * @dev Base cost of the `IExecutor.execute` method. + */ + uint256 internal constant EXECUTION_BASE_COST = 37483 + 6800; + + /** + * @dev Base cost of the `IGateway.submitMessage` method. + */ + uint256 internal constant SUBMIT_BASE_COST = 9550 + 13300; using BranchlessMath for uint256; @@ -38,6 +46,37 @@ library GasUtils { } } + function submitMessageGasCost(uint16 messageSize) internal 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); + + // CALLDATACOPY + gasCost += words * 3; + + // keccak256 (6 gas per word) + gasCost += words * 6; + + // emit GmpCreated() gas cost (8 gas per byte) + gasCost += words << 8; + + // Memory expansion cost + words += 13; + gasCost += ((words * words) >> 9) + (words * 3); + + return gasCost; + } + } + /** * @dev Estimate the price in wei for send an GMP message. * @param gasPrice The gas price in UFloat9x56 format. @@ -99,7 +138,7 @@ library GasUtils { // Base cost uint256 words = (calldataSize + 31) >> 5; - gasNeeded += ((words - 1) / 15) * 1845; + gasNeeded += (words * 106) + (((words + 254) / 255) * 214); // CALLDATACOPY words = (messageSize + 31) >> 5; @@ -138,14 +177,16 @@ library GasUtils { /** * @dev Compute the gas that should be refunded to the executor for the execution. + * @param messageSize The size of the message. + * @param gasUsed The gas used by the gmp message. */ - function computeExecutionRefund(uint16 messageSize, uint256 gasLimit) + function computeExecutionRefund(uint16 messageSize, uint256 gasUsed) internal pure returns (uint256 executionCost) { // Add the base execution gas cost - executionCost = EXECUTION_BASE_COST.saturatingAdd(gasLimit); + executionCost = EXECUTION_BASE_COST.saturatingAdd(gasUsed); // Safety: The operations below can't overflow because the message size can't be greater than 2^16 unchecked { @@ -159,7 +200,7 @@ library GasUtils { // Base Cost calculation words = (words + 31) >> 5; - executionCost = executionCost.saturatingAdd(((words - 1) / 15) * 1845); + executionCost = executionCost.saturatingAdd((words * 106) + (((words + 254) / 255) * 214)); // calldatacopy (3 gas per word) words = messagePadded >> 5; @@ -191,7 +232,7 @@ library GasUtils { // Base Cost calculation words = (words + 31) >> 5; - executionCost += ((words - 1) / 15) * 1845; + executionCost += (words * 106) + (((words + 254) / 255) * 214); // calldatacopy (3 gas per word) words = (messageSize + 31) >> 5; @@ -210,7 +251,7 @@ library GasUtils { // Efficient algorithm for counting non-zero calldata bytes in chunks of 480 bytes at time // computation gas cost = 1845 * ceil(msg.data.length / 480) + 61 - baseCost = calldataGasCost(); + baseCost = countNonZerosCalldata(msg.data); } /** @@ -218,7 +259,7 @@ library GasUtils { * gas cost = 217 + (words * 112) + ((words - 1) * 193) */ function countNonZeros(bytes memory data) internal pure returns (uint256 nonZeros) { - /// @solidity memory-safe-assembly + // /// @solidity memory-safe-assembly assembly { // Efficient algorithm for counting non-zero bytes in parallel let size := mload(data) @@ -261,25 +302,21 @@ library GasUtils { } /** - * @dev Count the number of non-zero bytes in a byte sequence. + * @dev Count the number of non-zero bytes from calldata. * gas cost = 224 + (words * 106) + (((words - 1) / 255) * 214) */ function countNonZerosCalldata(bytes calldata data) internal pure returns (uint256 nonZeros) { /// @solidity memory-safe-assembly assembly { - let offset := data.offset nonZeros := 0 for { let ptr := data.offset let end := add(ptr, data.length) - - // range := min(ptr + data.length, ptr + 8160) + } lt(ptr, end) {} { + // calculate min(ptr + data.length, ptr + 8160) let range := add(ptr, 8160) range := xor(end, mul(xor(range, end), lt(range, end))) - } true { - range := add(ptr, 8160) - range := xor(end, mul(xor(range, end), lt(range, end))) - } { + // Normalize and count non-zero bytes in parallel let v := 0 for {} lt(ptr, range) { ptr := add(ptr, 32) } { @@ -303,176 +340,40 @@ library GasUtils { v := add(v, shr(16, v)) v := and(v, 0xffff) nonZeros := add(nonZeros, v) - if iszero(lt(ptr, end)) { break } } } } - /** - * @dev Count non-zeros of a single 32 bytes word. - */ - function countNonZeros(bytes32 value) internal pure returns (uint256 nonZeros) { - /// @solidity memory-safe-assembly - assembly { - // Normalize and count non-zero bytes in parallel - value := or(value, shr(4, value)) - value := or(value, shr(2, value)) - value := or(value, shr(1, value)) - value := and(value, 0x0101010101010101010101010101010101010101010101010101010101010101) - - // Sum bytes in parallel - value := add(value, shr(128, value)) - value := add(value, shr(64, value)) - value := add(value, shr(32, value)) - value := add(value, shr(16, value)) - value := add(value, shr(8, value)) - nonZeros := and(value, 0xff) - } - } + // /** + // * @dev Count non-zeros of a single 32 bytes word. + // */ + // function countNonZeros(bytes32 value) internal pure returns (uint256 nonZeros) { + // /// @solidity memory-safe-assembly + // assembly { + // // Normalize and count non-zero bytes in parallel + // value := or(value, shr(4, value)) + // value := or(value, shr(2, value)) + // value := or(value, shr(1, value)) + // value := and(value, 0x0101010101010101010101010101010101010101010101010101010101010101) + + // // Sum bytes in parallel + // value := add(value, shr(128, value)) + // value := add(value, shr(64, value)) + // value := add(value, shr(32, value)) + // value := add(value, shr(16, value)) + // value := add(value, shr(8, value)) + // nonZeros := and(value, 0xff) + // } + // } /** * @dev Compute the transaction base cost. */ - function calldataGasCost() internal pure returns (uint256 baseCost) { - // Efficient algorithm for counting non-zero calldata bytes in chunks of 480 bytes at time - // computation gas cost = 1845 * ceil(msg.data.length / 480) + 61 - assembly { - baseCost := 0 - for { - let ptr := 0 - let mask := 0x0101010101010101010101010101010101010101010101010101010101010101 - } lt(ptr, calldatasize()) { ptr := add(ptr, 32) } { - // 1 - let v := calldataload(ptr) - v := or(v, shr(4, v)) - v := or(v, shr(2, v)) - v := or(v, shr(1, v)) - v := and(v, mask) - { - // 2 - ptr := add(ptr, 32) - let r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 3 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 4 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 5 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 6 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 7 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 8 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 9 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 10 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 11 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 12 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 13 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 14 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - // 15 - ptr := add(ptr, 32) - r := calldataload(ptr) - r := or(r, shr(4, r)) - r := or(r, shr(2, r)) - r := or(r, shr(1, r)) - r := and(r, mask) - v := add(v, r) - } - - // Count bytes in parallel - v := add(v, shr(128, v)) - v := add(v, shr(64, v)) - v := add(v, shr(32, v)) - v := add(v, shr(16, v)) - v := and(v, 0xffff) - v := add(and(v, 0xff), shr(8, v)) - baseCost := add(baseCost, v) - } - baseCost := add(21000, add(mul(sub(calldatasize(), baseCost), 4), mul(baseCost, 16))) + function calldataGasCost() internal pure returns (uint256) { + unchecked { + uint256 nonZeros = countNonZerosCalldata(msg.data); + uint256 zeros = msg.data.length - nonZeros; + return 21000 + (nonZeros * 16) + (zeros * 4); } } } diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index 863bb76..fae39a4 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -30,6 +30,18 @@ import { uint256 constant secret = 0x42; uint256 constant nonce = 0x69; +contract GasUtilsMock { + function execute(Signature calldata, GmpMessage calldata) + external + pure + returns (uint256 baseCost, uint256 nonZeros, uint256 zeros) + { + baseCost = GasUtils.calldataGasCost(); + nonZeros = GasUtils.countNonZerosCalldata(msg.data); + zeros = msg.data.length - nonZeros; + } +} + contract GasUtilsBase is Test { using PrimitiveUtils for UpdateKeysMessage; using PrimitiveUtils for GmpMessage; @@ -38,6 +50,7 @@ contract GasUtilsBase is Test { using GatewayUtils for CallOptions; using BranchlessMath for uint256; + GasUtilsMock internal mock; Gateway internal gateway; Signer internal signer; @@ -56,6 +69,9 @@ contract GasUtilsBase is Test { address deployer = TestUtils.createTestAccount(100 ether); vm.startPrank(deployer, deployer); + // Deploy the GasUtilsMock contract + mock = new GasUtilsMock(); + // 1 - Deploy the implementation contract address proxyAddr = vm.computeCreateAddress(deployer, vm.getNonce(deployer) + 1); Gateway implementation = new Gateway(DEST_NETWORK_ID, proxyAddr); @@ -149,29 +165,33 @@ contract GasUtilsBase is Test { baseCost: baseCost }); } - // bdfbbea6 - // 079264c4b4bfcd7fe3a7b7b92b6c439f3a5b3abcd29189bf7b54d781ff03d722 - // 51f46b1ff68263bee778aeef6b875d06636012244855e8cb1d445d8472f2fea1 - // 21039e3d8d9db737ad1d19b9b8e5fbc04e6c8e6e4530df76cf5e5a988e324b96 - // 0000000000000000000000000000000000000000000000000000000000000080 - // 000000000000000000000000dba77ed08c6610c2cbabc48216d85aa0f83b3e35 - // 00000000000000000000000000000000000000000000000000000000000004d2 - // 0000000000000000000000007ef5c78b60535b879a77ce245070c34e120a5cac - // 0000000000000000000000000000000000000000000000000000000000000539 - // 000000000000000000000000000000000000000000000000000000000000097c - // 0000000000000000000000000000000000000000000000000000000000000000 - // 00000000000000000000000000000000000000000000000000000000000000e0 - // 000000000000000000000000000000000000000000000000000000000000388a - // 000000000000000000000000000000000000000000000000000000000000097c - // 0000000000000000000000000000000000000000000000000000000000000000 - // 0000000000000000000000000000000000000000000000000000000000000000 - // 0000000000000000000000000000000000000000000000000000000000000000 - // 0000000000000000000000000000000000000000000000000000000000000000 - // ... 14336 zeros + /** - * @dev Compare the estimated gas cost VS the actual gas cost of the `execute` method. + * Test the `GasUtils.calldataGasCost` method. */ + function test_calldataGasCost() external view { + // Build and sign GMP message + GmpMessage memory gmp = GmpMessage({ + source: address(0x1111111111111111111111111111111111111111).toSender(false), + srcNetwork: 1234, + dest: address(0x2222222222222222222222222222222222222222), + destNetwork: 1337, + gasLimit: 0, + salt: 0, + data: hex"00" + }); + Signature memory sig = sign(gmp); + // Check if `IExecutor.execute` match the expected base cost + (uint256 baseCost, uint256 nonZeros, uint256 zeros) = mock.execute(sig, gmp); + assertEq(baseCost, 24444, "Wrong calldata gas cost"); + assertEq(nonZeros, 147, "wrong number of non-zeros"); + assertEq(zeros, 273, "wrong number of zeros"); + } + + /** + * @dev Compare the estimated gas cost VS the actual gas cost of the `execute` method. + */ function test_baseExecutionCost(uint16 messageSize, uint16 gasLimit) external { vm.assume(gasLimit >= 5000); // vm.assume(messageSize <= (0x6000 - 32)); @@ -190,21 +210,25 @@ contract GasUtilsBase is Test { ctx.gasLimit = ctx.gasLimit.saturatingAdd(10_000_000); // Execute the GMP message - ctx.execute(sig, gmp); - // { - // (GmpStatus status, bytes32 result) = ctx.execute(sig, gmp); - // if (status != GmpStatus.SUCCESS) { - // bytes memory bla = abi.encodeCall(IExecutor.execute, (sig, gmp)); - // emit log_named_bytes("encoded call", bla); - // emit log_named_uint("gmp status", uint256(status)); - // emit log_named_uint("gmp result", uint256(result)); - // emit log_named_uint("requested size", uint256(messageSize)); - // emit log_named_uint("actual size", uint256(gmp.data.length)); - // emit log_named_uint("gas limit", uint256(gasLimit)); - // } - // assertEq(uint256(status), uint256(GmpStatus.SUCCESS), "GMP execution failed"); - // assertEq(result, bytes32(uint256(gasLimit)), "unexpected result"); - // } + // ctx.execute(sig, gmp); + + { + uint256 balanceBefore = ctx.from.balance; + (GmpStatus status, bytes32 result) = ctx.execute(sig, gmp); + uint256 balanceAfter = ctx.from.balance; + if (status == GmpStatus.SUCCESS) { + // bytes memory bla = abi.encodeCall(IExecutor.execute, (sig, gmp)); + // emit log_named_bytes("encoded call", bla); + emit log_named_uint("gmp status", uint256(status)); + emit log_named_uint("gmp result", uint256(result)); + emit log_named_uint("requested size", uint256(messageSize)); + emit log_named_uint("actual size", uint256(gmp.data.length)); + emit log_named_uint("gas limit", uint256(gasLimit)); + assertEq(balanceBefore, balanceAfter, "Balance should not change"); + } + assertEq(uint256(status), uint256(GmpStatus.SUCCESS), "GMP execution failed"); + assertEq(result, bytes32(uint256(gasLimit)), "unexpected result"); + } // Calculate the expected base cost uint256 dynamicCost = @@ -214,22 +238,22 @@ contract GasUtilsBase is Test { } function test_gasUtils() external pure { - assertEq(GasUtils.estimateGas(0, 0, 0), 76208); - assertEq(GasUtils.estimateGas(0, 33, 0), 76369); - assertEq(GasUtils.estimateGas(33, 0, 0), 77029); - assertEq(GasUtils.estimateGas(20, 13, 0), 76769); + assertEq(GasUtils.estimateGas(0, 0, 0), 75976); + assertEq(GasUtils.estimateGas(0, 33, 0), 76349); + assertEq(GasUtils.estimateGas(33, 0, 0), 77009); + assertEq(GasUtils.estimateGas(20, 13, 0), 76749); UFloat9x56 one = UFloatMath.ONE; - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 76208); - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76369); - assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77029); - assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76769); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 75976); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76349); + assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77009); + assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76749); UFloat9x56 two = UFloat9x56.wrap(0x8080000000000000); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 76208 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76369 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77029 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76769 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 75976 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76349 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77009 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76749 * 2); } } diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 3b760a8..6dd00ad 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -100,7 +100,7 @@ library GatewayUtils { pure returns (uint256 baseCost, uint256 executionCost) { - (, executionCost) = GasUtils.internalGasCost(message.data.length); + executionCost = GasUtils.computeExecutionRefund(uint16(message.data.length), 0); bytes memory encodedCall = abi.encodeCall(IExecutor.execute, (signature, message)); baseCost = TestUtils.calculateBaseCost(encodedCall); } @@ -137,7 +137,7 @@ contract GatewayBase is Test { bytes32 private _srcDomainSeparator; bytes32 private _dstDomainSeparator; - uint256 private constant SUBMIT_GAS_COST = 14925; + uint256 private constant SUBMIT_GAS_COST = 15034; uint16 private constant SRC_NETWORK_ID = 1234; uint16 internal constant DEST_NETWORK_ID = 1337; uint8 private constant GMP_STATUS_SUCCESS = 1; @@ -216,7 +216,47 @@ contract GatewayBase is Test { function test_estimateMessageCost() external { vm.txGasPrice(1); uint256 cost = gateway.estimateMessageCost(DEST_NETWORK_ID, 96, 100000); - assertEq(cost, 180028); + assertEq(cost, 178269); + } + + function test_checkPayloadSize() external { + vm.txGasPrice(1); + address sender = TestUtils.createTestAccount(100 ether); + + // Build and sign GMP message + GmpMessage memory gmp = GmpMessage({ + source: sender.toSender(false), + srcNetwork: SRC_NETWORK_ID, + dest: address(bytes20(keccak256("dummy_address"))), + destNetwork: DEST_NETWORK_ID, + gasLimit: 0, + salt: 0, + data: new bytes(24576 + 1) + }); + + Signature memory sig = sign(gmp); + + // Calculate memory expansion cost and base cost + (uint256 baseCost, uint256 executionCost) = GatewayUtils.computeGmpGasCost(sig, gmp); + + // Transaction Parameters + CallOptions memory ctx = CallOptions({ + from: sender, + to: address(gateway), + value: 0, + gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost + 1_000_000, + executionCost: 0, + baseCost: 0 + }); + + GmpStatus status; + bytes32 returned; + + // Expect a revert + vm.expectRevert("msg data too large"); + (status, returned) = ctx.execute(sig, gmp); + assertLt(ctx.executionCost, executionCost, "revert should use less gas!!"); + assertEq(ctx.baseCost, baseCost, "unexpected base cost"); } function test_gasMeter() external { @@ -231,9 +271,11 @@ contract GatewayBase is Test { destNetwork: DEST_NETWORK_ID, gasLimit: 0, salt: 0, - data: hex"" + data: new bytes(55 * 32) }); // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff + // 1111111111111111111111111111111111111111111111111111111111111111 + // 2222222222222222222222222222222222222222222222222222222222222222 Signature memory sig = sign(gmp); @@ -274,16 +316,94 @@ contract GatewayBase is Test { assertEq(gatewayBalance - address(gateway).balance, executionCost + baseCost, "wrong refund amount"); assertEq(senderBalance, address(sender).balance, "sender balance should not change"); + // Submit the transaction { uint256 nonZeros = GasUtils.countNonZeros(gmp.data); uint256 zeros = gmp.data.length - nonZeros; ctx.value = GasUtils.estimateGas(uint16(nonZeros), uint16(zeros), gmp.gasLimit) - 1; } + + // Must revert if fund are insufficient vm.expectRevert("insufficient tx value"); ctx.submitMessage(gmp); + { + bytes memory submitEncoded = + abi.encodeCall(IGateway.submitMessage, (gmp.dest, gmp.destNetwork, gmp.gasLimit, gmp.data)); + assertEq(submitEncoded.length, ((gmp.data.length + 31) & 0xffe0) + 164, "wrong encoded length"); + } + + // Must work if the funds are sufficient ctx.value += 1; + ctx.gasLimit += gmp.data.length * 8; + ctx.submitMessage(gmp); + + assertEq( + ctx.executionCost, + GasUtils.submitMessageGasCost(uint16(gmp.data.length)) + 1, + "unexpected submit message gas cost" + ); + } + + function test_submitMessageMeter(uint16 messageSize) external { + vm.assume(messageSize < 0x6000); + vm.txGasPrice(1); + address sender = TestUtils.createTestAccount(1000 ether); + + // Build and sign GMP message + GmpMessage memory gmp = GmpMessage({ + source: sender.toSender(false), + srcNetwork: DEST_NETWORK_ID, + dest: address(bytes20(keccak256("dummy_address"))), + destNetwork: DEST_NETWORK_ID, + gasLimit: 0, + salt: 0, + data: new bytes(messageSize) + }); + + Signature memory sig = sign(gmp); + + // Calculate memory expansion cost and base cost + (uint256 baseCost,) = GatewayUtils.computeGmpGasCost(sig, gmp); + + // Transaction Parameters + CallOptions memory ctx = CallOptions({ + from: sender, + to: address(gateway), + value: 0, + gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost, + executionCost: 0, + baseCost: 0 + }); + + // Submit the transaction + { + uint256 nonZeros = GasUtils.countNonZeros(gmp.data); + uint256 zeros = gmp.data.length - nonZeros; + ctx.value = GasUtils.estimateGas(uint16(nonZeros), uint16(zeros), gmp.gasLimit) - 1; + } + + // 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; + 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"); + + // Verify the execution cost + assertEq( + ctx.executionCost, + GasUtils.submitMessageGasCost(uint16(gmp.data.length)), + "unexpected submit message gas cost" + ); } function test_refund() external { @@ -291,7 +411,7 @@ contract GatewayBase is Test { GmpSender sender = TestUtils.createTestAccount(100 ether).toSender(false); // GMP message gas used - uint256 gmpGasUsed = 1_000; + uint256 gmpGasUsed = 2_000; // Build and sign GMP message GmpMessage memory gmp = GmpMessage({ @@ -307,36 +427,25 @@ contract GatewayBase is Test { // Deposit funds (uint256 baseCost, uint256 executionCost) = GatewayUtils.computeGmpGasCost(sig, gmp); - uint256 expectGasUsed = baseCost + executionCost + gmp.gasLimit; - // Calculate memory expansion cost and base cost - // uint256 baseCost; // { - // bytes memory encodedExecuteCall = abi.encodeCall(IExecutor.execute, (sig, gmp)); - // baseCost = TestUtils.calculateBaseCost(encodedExecuteCall); - // expectGasUsed += TestUtils.memExpansionCost(encodedExecuteCall.length); + // uint256 temp = GasUtils.computeExecutionRefund(uint16(gmp.data.length), gmp.gasLimit); + // assertEq(temp, executionCost, "unexpected execution cost"); // } - // Deposit funds - // { - // GmpSender gmpSender = sender.toSender(false); - // assertEq(gateway.depositOf(gmpSender, DEST_NETWORK_ID), 0); - // vm.prank(sender, sender); - // gateway.deposit{value: expectGasUsed + baseCost}(gmpSender, DEST_NETWORK_ID); - // assertEq(gateway.depositOf(gmpSender, DEST_NETWORK_ID), expectGasUsed + baseCost); - // } + uint256 expectGasUsed = baseCost + executionCost + gmp.gasLimit; // Execute GMP message uint256 beforeBalance = sender.toAddress().balance; - CallOptions memory ctx = CallOptions({ - from: sender.toAddress(), - to: address(gateway), - value: 0, - gasLimit: expectGasUsed + 2160 + 785 + 10, - executionCost: 0, - baseCost: 0 - }); { + CallOptions memory ctx = CallOptions({ + from: sender.toAddress(), + to: address(gateway), + value: 0, + gasLimit: expectGasUsed + 2160 + 785 + 10, + executionCost: 0, + baseCost: 0 + }); (GmpStatus status, bytes32 returned) = ctx.execute(sig, gmp); // Verify the GMP message status @@ -349,6 +458,7 @@ contract GatewayBase is Test { // Verify the gas cost assertEq(ctx.executionCost + ctx.baseCost, expectGasUsed, "unexpected gas used"); + assertEq(ctx.executionCost, executionCost, "unexpected execution cost"); } // Verify the gas refund @@ -494,7 +604,7 @@ contract GatewayBase is Test { ctx.submitMessage(gmp); // Verify the gas cost - uint256 expectedCost = SUBMIT_GAS_COST + 2800 + 2000 + 2000; + uint256 expectedCost = GasUtils.submitMessageGasCost(uint16(gmp.data.length)); assertEq(ctx.executionCost, expectedCost, "unexpected execution gas cost"); // Now the second GMP message should have the salt equals to previous gmp hash From 6d682c9622f3bc765877073e04b6e10e10c4f267 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 13:57:43 -0300 Subject: [PATCH 04/17] Fix unit tests --- src/utils/GasUtils.sol | 2 +- test/GasUtils.t.sol | 35 +++++++++++++++++++++-------------- test/Gateway.t.sol | 35 ++++++++++++----------------------- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index b14429b..9bedfa3 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -18,7 +18,7 @@ library GasUtils { /** * @dev Base cost of the `IGateway.submitMessage` method. */ - uint256 internal constant SUBMIT_BASE_COST = 9550 + 13300; + uint256 internal constant SUBMIT_BASE_COST = 9550 + 6800 + 6500; using BranchlessMath for uint256; diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index fae39a4..2c3c1f9 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -95,7 +95,7 @@ contract GasUtilsBase is Test { // See the file `HelperContract.opcode` for more details. { bytes memory bytecode = - hex"603b80600c6000396000f3fe5a600201803d523d60209160643560240135146018575bfd5b60345a116018575a604803565b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5bf3"; + hex"603c80600a5f395ff3fe5a600201803d523d60209160643560240135146018575bfd5b60365a116018575a604903565b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5bf3"; receiver = IGmpReceiver(TestUtils.deployContract(bytecode)); } @@ -216,9 +216,16 @@ contract GasUtilsBase is Test { uint256 balanceBefore = ctx.from.balance; (GmpStatus status, bytes32 result) = ctx.execute(sig, gmp); uint256 balanceAfter = ctx.from.balance; - if (status == GmpStatus.SUCCESS) { + if (status != GmpStatus.SUCCESS) { // bytes memory bla = abi.encodeCall(IExecutor.execute, (sig, gmp)); // emit log_named_bytes("encoded call", bla); + bytes32 gmp_id = gmp.eip712TypedHash(_dstDomainSeparator); + bytes memory jose = abi.encodeCall( + IGmpReceiver.onGmpReceived, (gmp_id, gmp.srcNetwork, GmpSender.unwrap(gmp.source), gmp.data) + ); + emit log_named_bytes("onGmpReceived", jose); + emit log_named_address("gmp receiver", gmp.dest); + emit log_named_address(" receiver", address(receiver)); emit log_named_uint("gmp status", uint256(status)); emit log_named_uint("gmp result", uint256(result)); emit log_named_uint("requested size", uint256(messageSize)); @@ -238,22 +245,22 @@ contract GasUtilsBase is Test { } function test_gasUtils() external pure { - assertEq(GasUtils.estimateGas(0, 0, 0), 75976); - assertEq(GasUtils.estimateGas(0, 33, 0), 76349); - assertEq(GasUtils.estimateGas(33, 0, 0), 77009); - assertEq(GasUtils.estimateGas(20, 13, 0), 76749); + assertEq(GasUtils.estimateGas(0, 0, 0), 76022); + assertEq(GasUtils.estimateGas(0, 33, 0), 76395); + assertEq(GasUtils.estimateGas(33, 0, 0), 77055); + assertEq(GasUtils.estimateGas(20, 13, 0), 76795); UFloat9x56 one = UFloatMath.ONE; - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 75976); - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76349); - assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77009); - assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76749); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 76022); + assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76395); + assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77055); + assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76795); UFloat9x56 two = UFloat9x56.wrap(0x8080000000000000); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 75976 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76349 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77009 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76749 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 76022 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76395 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77055 * 2); + assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76795 * 2); } } diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 6dd00ad..1c09b38 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -178,7 +178,7 @@ contract GatewayBase is Test { // See the file `HelperContract.opcode` for more details. { bytes memory bytecode = - hex"603b80600c6000396000f3fe5a600201803d523d60209160643560240135146018575bfd5b60345a116018575a604803565b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5bf3"; + hex"603c80600a5f395ff3fe5a600201803d523d60209160643560240135146018575bfd5b60365a116018575a604903565b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5bf3"; receiver = IGmpReceiver(TestUtils.deployContract(bytecode)); } } @@ -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, 178269); + assertEq(cost, 178315); } function test_checkPayloadSize() external { @@ -340,7 +340,7 @@ contract GatewayBase is Test { assertEq( ctx.executionCost, - GasUtils.submitMessageGasCost(uint16(gmp.data.length)) + 1, + GasUtils.submitMessageGasCost(uint16(gmp.data.length)) - 4500, "unexpected submit message gas cost" ); } @@ -411,7 +411,7 @@ contract GatewayBase is Test { GmpSender sender = TestUtils.createTestAccount(100 ether).toSender(false); // GMP message gas used - uint256 gmpGasUsed = 2_000; + uint256 gmpGasUsed = 1000; //2_000; // Build and sign GMP message GmpMessage memory gmp = GmpMessage({ @@ -425,14 +425,8 @@ contract GatewayBase is Test { }); Signature memory sig = sign(gmp); - // Deposit funds + // Estimate execution cost (uint256 baseCost, uint256 executionCost) = GatewayUtils.computeGmpGasCost(sig, gmp); - - // { - // uint256 temp = GasUtils.computeExecutionRefund(uint16(gmp.data.length), gmp.gasLimit); - // assertEq(temp, executionCost, "unexpected execution cost"); - // } - uint256 expectGasUsed = baseCost + executionCost + gmp.gasLimit; // Execute GMP message @@ -442,7 +436,7 @@ contract GatewayBase is Test { from: sender.toAddress(), to: address(gateway), value: 0, - gasLimit: expectGasUsed + 2160 + 785 + 10, + gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost - 15, //31, executionCost: 0, baseCost: 0 }); @@ -458,7 +452,7 @@ contract GatewayBase is Test { // Verify the gas cost assertEq(ctx.executionCost + ctx.baseCost, expectGasUsed, "unexpected gas used"); - assertEq(ctx.executionCost, executionCost, "unexpected execution cost"); + assertEq(ctx.executionCost, executionCost + gmp.gasLimit, "unexpected execution cost"); } // Verify the gas refund @@ -599,12 +593,12 @@ contract GatewayBase is Test { id, GmpSender.unwrap(gmp.source), gmp.dest, gmp.destNetwork, gmp.gasLimit, gmp.salt, gmp.data ); - // Submit message + // Submit message with sufficient funds ctx.value += 1; - ctx.submitMessage(gmp); + assertEq(ctx.submitMessage(gmp), id, "unexpected GMP id"); // Verify the gas cost - uint256 expectedCost = GasUtils.submitMessageGasCost(uint16(gmp.data.length)); + uint256 expectedCost = GasUtils.submitMessageGasCost(uint16(gmp.data.length)) - 6500; assertEq(ctx.executionCost, expectedCost, "unexpected execution gas cost"); // Now the second GMP message should have the salt equals to previous gmp hash @@ -616,13 +610,8 @@ contract GatewayBase is Test { emit IGateway.GmpCreated( id, GmpSender.unwrap(gmp.source), gmp.dest, gmp.destNetwork, gmp.gasLimit, gmp.salt, gmp.data ); - ctx.submitMessage(gmp); - - if (ctx.baseCost > 0) { - return; - } - expectedCost = SUBMIT_GAS_COST; - assertEq(ctx.executionCost, expectedCost, "unexpected execution gas cost"); + assertEq(ctx.submitMessage(gmp), id, "unexpected GMP id"); + assertEq(ctx.executionCost, expectedCost - 6800, "unexpected execution gas cost"); } } From 81c6b3cec8c50fb8800347d80519f127268ad1e9 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:11:58 -0300 Subject: [PATCH 05/17] Remove commented code --- src/Gateway.sol | 13 ++++++------- test/Gateway.t.sol | 19 ++++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 6683165..0beb016 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -341,7 +341,6 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // https://eips.ethereum.org/EIPS/eip-150 uint256 gasNeeded = gasLimit.saturatingMul(64).saturatingDiv(63); // to guarantee it was provided enough gas to execute the GMP message - // gasNeeded = gasNeeded.saturatingAdd(6412); gasNeeded = gasNeeded.saturatingAdd(10000); require(gasleft() >= gasNeeded, "insufficient gas to execute GMP message"); } @@ -536,7 +535,12 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { payload = message.data; } - // Emit `GmpCreated` event without copy the data + // Emit `GmpCreated` event without copy the data, to simplify the gas estimation. + // the assembly code below is equivalent to: + // ```solidity + // emit GmpCreated(prevHash, source, destinationAddress, destinationNetwork, executionGasLimit, salt, data); + // return prevHash; + // ``` bytes32 eventSelector = GmpCreated.selector; assembly { let ptr := sub(payload, 0x80) @@ -550,11 +554,6 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { mstore(0, prevHash) return(0, 32) } - - // emit GmpCreated( - // prevHash, GmpSender.unwrap(source), destinationAddress, destinationNetwork, executionGasLimit, salt, data - // ); - // return prevHash; } /** diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 1c09b38..c3c0d11 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -259,7 +259,11 @@ contract GatewayBase is Test { assertEq(ctx.baseCost, baseCost, "unexpected base cost"); } - function test_gasMeter() external { + /** + * @dev Test the gas metering for the `execute` function. + */ + function test_gasMeter(uint16 messageSize) external { + vm.assume(messageSize < 1000); vm.txGasPrice(1); address sender = TestUtils.createTestAccount(100 ether); @@ -271,11 +275,8 @@ contract GatewayBase is Test { destNetwork: DEST_NETWORK_ID, gasLimit: 0, salt: 0, - data: new bytes(55 * 32) + data: new bytes(messageSize) }); - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff - // 1111111111111111111111111111111111111111111111111111111111111111 - // 2222222222222222222222222222222222222222222222222222222222222222 Signature memory sig = sign(gmp); @@ -288,7 +289,6 @@ contract GatewayBase is Test { to: address(gateway), value: 0, gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost - 1, - // gasLimit: 100_000, executionCost: 0, baseCost: 0 }); @@ -297,7 +297,6 @@ contract GatewayBase is Test { bytes32 returned; // Expect a revert - // vm.expectRevert("insufficient gas to execute GMP message"); vm.expectRevert(); (status, returned) = ctx.execute(sig, gmp); @@ -311,6 +310,8 @@ contract GatewayBase is Test { ctx.gasLimit += 1; (status, returned) = ctx.execute(sig, gmp); + assertEq(uint256(status), uint256(GmpStatus.SUCCESS), "gmp execution failed"); + assertEq(uint256(returned), gmp.gasLimit, "wrong gmp return value"); assertEq(ctx.baseCost, baseCost, "ctx.baseCost != baseCost"); assertEq(ctx.executionCost, executionCost, "ctx.executionCost != executionCost"); assertEq(gatewayBalance - address(gateway).balance, executionCost + baseCost, "wrong refund amount"); @@ -411,7 +412,7 @@ contract GatewayBase is Test { GmpSender sender = TestUtils.createTestAccount(100 ether).toSender(false); // GMP message gas used - uint256 gmpGasUsed = 1000; //2_000; + uint256 gmpGasUsed = 2_000; // Build and sign GMP message GmpMessage memory gmp = GmpMessage({ @@ -436,7 +437,7 @@ contract GatewayBase is Test { from: sender.toAddress(), to: address(gateway), value: 0, - gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost - 15, //31, + gasLimit: GasUtils.executionGasNeeded(gmp.data.length, gmp.gasLimit) + baseCost, executionCost: 0, baseCost: 0 }); From 4c054dbe53e9683e3d213c2b60dd03cdf780d7e3 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:20:26 -0300 Subject: [PATCH 06/17] Remove debug from test_baseExecutionCost --- test/GasUtils.t.sol | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index 2c3c1f9..44b9182 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -194,8 +194,7 @@ contract GasUtilsBase is Test { */ function test_baseExecutionCost(uint16 messageSize, uint16 gasLimit) external { vm.assume(gasLimit >= 5000); - // vm.assume(messageSize <= (0x6000 - 32)); - vm.assume(messageSize <= 8160); + vm.assume(messageSize <= (0x6000 - 32)); messageSize += 32; vm.txGasPrice(1); address sender = TestUtils.createTestAccount(100 ether); @@ -210,31 +209,15 @@ contract GasUtilsBase is Test { ctx.gasLimit = ctx.gasLimit.saturatingAdd(10_000_000); // Execute the GMP message - // ctx.execute(sig, gmp); - { + bytes32 gmpId = gmp.eip712TypedHash(_dstDomainSeparator); + vm.expectEmit(true, true, true, true); + emit IExecutor.GmpExecuted(gmpId, gmp.source, gmp.dest, GmpStatus.SUCCESS, bytes32(uint256(gasLimit))); uint256 balanceBefore = ctx.from.balance; (GmpStatus status, bytes32 result) = ctx.execute(sig, gmp); - uint256 balanceAfter = ctx.from.balance; - if (status != GmpStatus.SUCCESS) { - // bytes memory bla = abi.encodeCall(IExecutor.execute, (sig, gmp)); - // emit log_named_bytes("encoded call", bla); - bytes32 gmp_id = gmp.eip712TypedHash(_dstDomainSeparator); - bytes memory jose = abi.encodeCall( - IGmpReceiver.onGmpReceived, (gmp_id, gmp.srcNetwork, GmpSender.unwrap(gmp.source), gmp.data) - ); - emit log_named_bytes("onGmpReceived", jose); - emit log_named_address("gmp receiver", gmp.dest); - emit log_named_address(" receiver", address(receiver)); - emit log_named_uint("gmp status", uint256(status)); - emit log_named_uint("gmp result", uint256(result)); - emit log_named_uint("requested size", uint256(messageSize)); - emit log_named_uint("actual size", uint256(gmp.data.length)); - emit log_named_uint("gas limit", uint256(gasLimit)); - assertEq(balanceBefore, balanceAfter, "Balance should not change"); - } assertEq(uint256(status), uint256(GmpStatus.SUCCESS), "GMP execution failed"); assertEq(result, bytes32(uint256(gasLimit)), "unexpected result"); + assertEq(balanceBefore, ctx.from.balance, "Balance should not change"); } // Calculate the expected base cost From fc6df94cdd71e1f213eb22c15e753bc4df71d8c2 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:23:27 -0300 Subject: [PATCH 07/17] Fix typo --- src/Gateway.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 0beb016..8d3121e 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -349,7 +349,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { bool success; address dest = message.dest; - // /// @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { // Using low-level assembly because the GMP is considered executed // regardless if the call reverts or not. From fe60dcc33ab0cbe2fe1004fc9a970a5c8f8fb636 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:44:48 -0300 Subject: [PATCH 08/17] Make GasUtils.proxyOverheadGasCost safe --- src/utils/GasUtils.sol | 61 +++++++++++------------------------------- test/GasUtils.t.sol | 24 ++++++++--------- test/Gateway.t.sol | 2 +- 3 files changed, 29 insertions(+), 58 deletions(-) diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 9bedfa3..be7a860 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -13,39 +13,46 @@ library GasUtils { /** * @dev Base cost of the `IExecutor.execute` method. */ - uint256 internal constant EXECUTION_BASE_COST = 37483 + 6800; + uint256 internal constant EXECUTION_BASE_COST = 37647 + 6800; /** * @dev Base cost of the `IGateway.submitMessage` method. */ - uint256 internal constant SUBMIT_BASE_COST = 9550 + 6800 + 6500; + uint256 internal constant SUBMIT_BASE_COST = 9752 + 6800 + 6500; using BranchlessMath for uint256; /** * @dev Compute the amount of gas used by the `GatewayProxy`. - * @param calldataLen The length of the calldata - * @param returnLen The length of the return data + * @param calldataLen The length of the calldata in bytes + * @param returnLen The length of the return data in bytes */ - function proxyOverheadGasCost(uint16 calldataLen, uint16 returnLen) internal pure returns (uint256) { + function proxyOverheadGasCost(uint256 calldataLen, uint256 returnLen) internal pure returns (uint256) { unchecked { + // Convert the calldata and return data length to words + calldataLen = calldataLen.saturatingAdd(31) >> 5; + returnLen = returnLen.saturatingAdd(31) >> 5; + // Base cost: OPCODES + COLD READ STORAGE _implementation uint256 gasCost = 2257 + 2500; // CALLDATACOPY - gasCost += ((uint256(calldataLen) + 31) >> 5) * 3; + gasCost = gasCost.saturatingAdd(calldataLen * 3); // RETURNDATACOPY - gasCost += ((uint256(returnLen) + 31) >> 5) * 3; + gasCost = gasCost.saturatingAdd(returnLen * 3); // MEMORY EXPANSION uint256 words = BranchlessMath.max(calldataLen, returnLen); - words = (words + 31) >> 5; - gasCost += ((words * words) >> 9) + (words * 3); + gasCost = gasCost.saturatingAdd((words.saturatingMul(words) >> 9).saturatingAdd(words * 3)); return gasCost; } } + /** + * @dev Compute the gas cost of the `IGateway.submitMessage` method. + * @param messageSize The size of the message in bytes. + */ function submitMessageGasCost(uint16 messageSize) internal pure returns (uint256 gasCost) { unchecked { gasCost = SUBMIT_BASE_COST; @@ -218,42 +225,6 @@ library GasUtils { } } - /** - * @dev Compute the transaction base cost. - * OBS: This function must be used ONLY inside Gateway.execute method, because it also consider itself gas cost. - */ - function internalGasCost(uint256 messageSize) internal pure returns (uint256 baseCost, uint256 executionCost) { - // Calculate Gateway.execute dynamic cost - executionCost = EXECUTION_BASE_COST; - unchecked { - uint256 words = (messageSize + 31) & 0xffe0; - words += 388; - executionCost += proxyOverheadGasCost(uint16(words), 64); - - // Base Cost calculation - words = (words + 31) >> 5; - executionCost += (words * 106) + (((words + 254) / 255) * 214); - - // calldatacopy (3 gas per word) - words = (messageSize + 31) >> 5; - executionCost += words * 3; - - // keccak256 (6 gas per word) - executionCost += 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 += ((words * words) >> 9) + (words * 3); - } - - // Efficient algorithm for counting non-zero calldata bytes in chunks of 480 bytes at time - // computation gas cost = 1845 * ceil(msg.data.length / 480) + 61 - baseCost = countNonZerosCalldata(msg.data); - } - /** * @dev Count the number of non-zero bytes in a byte sequence. * gas cost = 217 + (words * 112) + ((words - 1) * 193) diff --git a/test/GasUtils.t.sol b/test/GasUtils.t.sol index 44b9182..f3b8d68 100644 --- a/test/GasUtils.t.sol +++ b/test/GasUtils.t.sol @@ -228,22 +228,22 @@ contract GasUtilsBase is Test { } function test_gasUtils() external pure { - assertEq(GasUtils.estimateGas(0, 0, 0), 76022); - assertEq(GasUtils.estimateGas(0, 33, 0), 76395); - assertEq(GasUtils.estimateGas(33, 0, 0), 77055); - assertEq(GasUtils.estimateGas(20, 13, 0), 76795); + 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); UFloat9x56 one = UFloatMath.ONE; - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 0, 0), 76022); - assertEq(GasUtils.estimateWeiCost(one, 0, 0, 33, 0), 76395); - assertEq(GasUtils.estimateWeiCost(one, 0, 33, 0, 0), 77055); - assertEq(GasUtils.estimateWeiCost(one, 0, 20, 13, 0), 76795); + 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); UFloat9x56 two = UFloat9x56.wrap(0x8080000000000000); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 0, 0), 76022 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 0, 33, 0), 76395 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 33, 0, 0), 77055 * 2); - assertEq(GasUtils.estimateWeiCost(two, 0, 20, 13, 0), 76795 * 2); + 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); } } diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index c3c0d11..28e4254 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, 178315); + assertEq(cost, 178479); } function test_checkPayloadSize() external { From 4618c7f565176045678a49554ae16751eebc43a1 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:47:44 -0300 Subject: [PATCH 09/17] Update submitMessage constant --- src/utils/GasUtils.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index be7a860..2f9a573 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -18,7 +18,7 @@ library GasUtils { /** * @dev Base cost of the `IGateway.submitMessage` method. */ - uint256 internal constant SUBMIT_BASE_COST = 9752 + 6800 + 6500; + uint256 internal constant SUBMIT_BASE_COST = 9640 + 6800 + 6500; using BranchlessMath for uint256; @@ -202,8 +202,7 @@ library GasUtils { // Proxy Overhead uint256 words = messagePadded + 388; // selector + Signature + GmpMessage - words = BranchlessMath.min(words, type(uint16).max); - executionCost = executionCost.saturatingAdd(proxyOverheadGasCost(uint16(words), 64)); + executionCost = executionCost.saturatingAdd(proxyOverheadGasCost(words, 64)); // Base Cost calculation words = (words + 31) >> 5; From 1f160f3fad7963ef07c7f9f0cccd61f139374d30 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 14:53:15 -0300 Subject: [PATCH 10/17] Remove unused methods --- src/Gateway.sol | 2 +- src/utils/GasUtils.sol | 24 +----------------------- test/GasUtils.t.sol | 6 +++--- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 8d3121e..ed803ed 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -414,7 +414,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { unchecked { // Compute GMP gas used uint256 gasUsed = 7214; - gasUsed = gasUsed.saturatingAdd(GasUtils.calldataGasCost()); + gasUsed = gasUsed.saturatingAdd(GasUtils.calldataBaseCost()); gasUsed = gasUsed.saturatingAdd(GasUtils.proxyOverheadGasCost(uint16(msg.data.length), 64)); gasUsed = gasUsed.saturatingAdd(initialGas - gasleft()); diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 2f9a573..5d7d6bb 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -314,32 +314,10 @@ library GasUtils { } } - // /** - // * @dev Count non-zeros of a single 32 bytes word. - // */ - // function countNonZeros(bytes32 value) internal pure returns (uint256 nonZeros) { - // /// @solidity memory-safe-assembly - // assembly { - // // Normalize and count non-zero bytes in parallel - // value := or(value, shr(4, value)) - // value := or(value, shr(2, value)) - // value := or(value, shr(1, value)) - // value := and(value, 0x0101010101010101010101010101010101010101010101010101010101010101) - - // // Sum bytes in parallel - // value := add(value, shr(128, value)) - // value := add(value, shr(64, value)) - // value := add(value, shr(32, value)) - // value := add(value, shr(16, value)) - // value := add(value, shr(8, value)) - // nonZeros := and(value, 0xff) - // } - // } - /** * @dev Compute the transaction base cost. */ - function calldataGasCost() internal pure returns (uint256) { + function calldataBaseCost() 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 f3b8d68..165fe43 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.calldataGasCost(); + baseCost = GasUtils.calldataBaseCost(); nonZeros = GasUtils.countNonZerosCalldata(msg.data); zeros = msg.data.length - nonZeros; } @@ -167,9 +167,9 @@ contract GasUtilsBase is Test { } /** - * Test the `GasUtils.calldataGasCost` method. + * Test the `GasUtils.calldataBaseCost` method. */ - function test_calldataGasCost() external view { + function test_calldataBaseCost() external view { // Build and sign GMP message GmpMessage memory gmp = GmpMessage({ source: address(0x1111111111111111111111111111111111111111).toSender(false), From e50d798aad2cfbb5d11565047014fcb9f40b90fd Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 15:44:06 -0300 Subject: [PATCH 11/17] Fix setNetworkInfo method --- src/Gateway.sol | 62 +++++++++++++++++++++++------------- src/utils/BranchlessMath.sol | 54 ++++++++++++++++++++++++++++--- test/GmpTestTools.sol | 2 +- 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index ed803ed..4314b20 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -77,6 +77,10 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // GMP message status mapping(bytes32 => GmpInfo) _messages; + // GAP necessary for migration purposes + mapping(GmpSender => mapping(uint16 => uint256)) _deprecated_Deposits; + mapping(uint16 => bytes32) _deprecated_Networks; + // Hash of the previous GMP message submitted. bytes32 public prevMessageHash; @@ -190,7 +194,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // Load y parity bit, it must be 27 (even), or 28 (odd) // ref: https://ethereum.github.io/yellowpaper/paper.pdf - uint8 yParity = uint8(BranchlessMath.select((status & SHARD_Y_PARITY) > 0, 28, 27)); + uint8 yParity = BranchlessMath.ternaryU8((status & SHARD_Y_PARITY) > 0, uint8(28), uint8(27)); // Verify Signature require( @@ -249,10 +253,10 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { ); // if is a new shard shard, set its initial nonce to 1 - shard.nonce = uint32(BranchlessMath.select(nonce == 0, 1, nonce)); + shard.nonce = uint32(BranchlessMath.ternary(nonce == 0, 1, nonce)); // enable/disable the y-parity flag - status = uint8(BranchlessMath.select(yParity > 0, status | SHARD_Y_PARITY, status & ~SHARD_Y_PARITY)); + status = uint8(BranchlessMath.ternary(yParity > 0, status | SHARD_Y_PARITY, status & ~SHARD_Y_PARITY)); // enable SHARD_ACTIVE bitflag status |= SHARD_ACTIVE; @@ -375,7 +379,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { } // Update GMP status - status = GmpStatus(BranchlessMath.select(success, uint256(GmpStatus.SUCCESS), uint256(GmpStatus.REVERT))); + status = GmpStatus(BranchlessMath.ternary(success, uint256(GmpStatus.SUCCESS), uint256(GmpStatus.REVERT))); // Persist gmp execution status on storage gmp.status = status; @@ -428,37 +432,51 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { } } - function _setNetworkInfo(bytes32 executor, bytes32 messageHash, UpdateNetworkInfo calldata data) private { - require(data.mortality >= block.number, "message expired"); + 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 storage networkInfo = _networkInfo[data.networkId]; + NetworkInfo memory stored = _networkInfo[info.networkId]; - // Verify if the domain separator is not zero - require((networkInfo.domainSeparator | data.domainSeparator) != bytes32(0), "domain separator cannot be zero"); + // Verify and update domain separator if it's not zero + stored.domainSeparator = + BranchlessMath.ternaryB32(info.domainSeparator != bytes32(0), info.domainSeparator, stored.domainSeparator); + require(stored.domainSeparator != bytes32(0), "domain separator cannot be zero"); // Store the message hash to prevent replay attacks _executedMessages[messageHash] = executor; - // Update domain separator if it's not zero - if (data.domainSeparator != bytes32(0)) { - networkInfo.domainSeparator = data.domainSeparator; - } - // Update gas limit if it's not zero - networkInfo.gasLimit = uint64(BranchlessMath.select(data.gasLimit > 0, data.gasLimit, networkInfo.gasLimit)); - if (UFloat9x56.unwrap(data.relativeGasPrice) > 0 || data.baseFee > 0) { - networkInfo.relativeGasPrice = networkInfo.relativeGasPrice; - networkInfo.baseFee = networkInfo.baseFee; + 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, data.networkId, data.domainSeparator, data.relativeGasPrice, data.baseFee, data.gasLimit + messageHash, + info.networkId, + stored.domainSeparator, + stored.relativeGasPrice, + stored.baseFee, + stored.gasLimit ); } @@ -523,7 +541,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { bytes32 prevHash = prevMessageHash; // if the messageHash is the first message, we use a zero salt - uint256 salt = BranchlessMath.select(prevHash == FIRST_MESSAGE_PLACEHOLDER, 0, uint256(prevHash)); + uint256 salt = BranchlessMath.ternary(prevHash == FIRST_MESSAGE_PLACEHOLDER, 0, uint256(prevHash)); // Create GMP message and update prevMessageHash bytes memory payload; @@ -576,7 +594,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { require(baseFee > 0 || UFloat9x56.unwrap(relativeGasPrice) > 0, "unsupported network"); // if the message data is too large, we use the maximum base fee. - baseFee = BranchlessMath.select(messageSize > MAX_PAYLOAD_SIZE, 2 ** 256 - 1, baseFee); + baseFee = BranchlessMath.ternary(messageSize > MAX_PAYLOAD_SIZE, 2 ** 256 - 1, baseFee); // Estimate the cost return GasUtils.estimateWeiCost(relativeGasPrice, baseFee, uint16(messageSize), 0, gasLimit); @@ -594,7 +612,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { function _getAdmin() private view returns (address admin) { admin = ERC1967.getAdmin(); // If the admin slot is empty, then the 0xd4833be6144AF48d4B09E5Ce41f826eEcb7706D6 is the admin - admin = BranchlessMath.select(admin == address(0x0), 0xd4833be6144AF48d4B09E5Ce41f826eEcb7706D6, admin); + admin = BranchlessMath.ternary(admin == address(0x0), 0xd4833be6144AF48d4B09E5Ce41f826eEcb7706D6, admin); } function setAdmin(address newAdmin) external payable { diff --git a/src/utils/BranchlessMath.sol b/src/utils/BranchlessMath.sol index de9c365..940a3b0 100644 --- a/src/utils/BranchlessMath.sol +++ b/src/utils/BranchlessMath.sol @@ -11,20 +11,20 @@ library BranchlessMath { * @dev Returns the smallest of two numbers. */ function min(uint256 x, uint256 y) internal pure returns (uint256) { - return select(x < y, x, y); + return ternary(x < y, x, y); } /** * @dev Returns the largest of two numbers. */ function max(uint256 x, uint256 y) internal pure returns (uint256) { - return select(x > y, x, y); + return ternary(x > y, x, y); } /** * @dev If `condition` is true returns `a`, otherwise returns `b`. */ - function select(bool condition, uint256 a, uint256 b) internal pure returns (uint256) { + function ternary(bool condition, uint256 a, uint256 b) internal pure returns (uint256) { unchecked { // branchless select, works because: // b ^ (a ^ b) == a @@ -40,13 +40,57 @@ library BranchlessMath { /** * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` */ - function select(bool condition, address a, address b) internal pure returns (address) { - return address(uint160(select(condition, uint256(uint160(a)), uint256(uint160(b))))); + function ternary(bool condition, address a, address b) internal pure returns (address r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } + } + + /** + * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` + */ + function ternaryB32(bool condition, bytes32 a, bytes32 b) internal pure returns (bytes32 r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } + } + + /** + * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` + */ + function ternaryU128(bool condition, uint128 a, uint128 b) internal pure returns (uint128 r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } + } + + /** + * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` + */ + function ternaryU64(bool condition, uint64 a, uint64 b) internal pure returns (uint64 r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } + } + + /** + * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` + */ + function ternaryU8(bool condition, uint8 a, uint8 b) internal pure returns (uint8 r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } } /** * @dev If `condition` is true return `value`, otherwise return zero. + * see `BranchlessMath.ternary` */ function selectIf(bool condition, uint256 value) internal pure returns (uint256) { unchecked { diff --git a/test/GmpTestTools.sol b/test/GmpTestTools.sol index ef72c5c..f13c214 100644 --- a/test/GmpTestTools.sol +++ b/test/GmpTestTools.sol @@ -236,7 +236,7 @@ library GmpTestTools { bytes32 slot = _deriveMapping(bytes32(0), shard.pubkey.px); uint256 shardInfo = uint256(vm.load(gateway, slot)); uint256 nonce = shardInfo >> 224; - nonce = BranchlessMath.select(nonce > 0, nonce, 1); + nonce = BranchlessMath.ternary(nonce > 0, nonce, 1); shardInfo = (nonce << 224) | (1 << 216) | ((shard.pubkey.py % 2) << 217); vm.store(gateway, slot, bytes32(shardInfo)); } From d5fdc8ace827fb737e05c44e48a2111e84169438 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 15:49:15 -0300 Subject: [PATCH 12/17] Fix ternary --- src/Gateway.sol | 6 +++--- src/utils/BranchlessMath.sol | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 4314b20..b5d752e 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -194,7 +194,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // Load y parity bit, it must be 27 (even), or 28 (odd) // ref: https://ethereum.github.io/yellowpaper/paper.pdf - uint8 yParity = BranchlessMath.ternaryU8((status & SHARD_Y_PARITY) > 0, uint8(28), uint8(27)); + uint8 yParity = BranchlessMath.ternaryU8((status & SHARD_Y_PARITY) > 0, 28, 27); // Verify Signature require( @@ -253,10 +253,10 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { ); // if is a new shard shard, set its initial nonce to 1 - shard.nonce = uint32(BranchlessMath.ternary(nonce == 0, 1, nonce)); + shard.nonce = uint32(BranchlessMath.ternaryU32(nonce == 0, 1, nonce)); // enable/disable the y-parity flag - status = uint8(BranchlessMath.ternary(yParity > 0, status | SHARD_Y_PARITY, status & ~SHARD_Y_PARITY)); + status = BranchlessMath.ternaryU8(yParity > 0, status | SHARD_Y_PARITY, status & ~SHARD_Y_PARITY); // enable SHARD_ACTIVE bitflag status |= SHARD_ACTIVE; diff --git a/src/utils/BranchlessMath.sol b/src/utils/BranchlessMath.sol index 940a3b0..da41a08 100644 --- a/src/utils/BranchlessMath.sol +++ b/src/utils/BranchlessMath.sol @@ -78,6 +78,16 @@ library BranchlessMath { } } + /** + * @dev If `condition` is true returns `a`, otherwise returns `b`. + * see `BranchlessMath.ternary` + */ + function ternaryU32(bool condition, uint32 a, uint32 b) internal pure returns (uint32 r) { + assembly { + r := xor(b, mul(xor(a, b), condition)) + } + } + /** * @dev If `condition` is true returns `a`, otherwise returns `b`. * see `BranchlessMath.ternary` From f0fb54c08b498a5791ace0e29ab40ad86afc5680 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 15:51:54 -0300 Subject: [PATCH 13/17] Remove duplicated storage access --- src/Gateway.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index b5d752e..cf8b160 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -447,9 +447,6 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { BranchlessMath.ternaryB32(info.domainSeparator != bytes32(0), info.domainSeparator, stored.domainSeparator); require(stored.domainSeparator != bytes32(0), "domain separator cannot be zero"); - // Store the message hash to prevent replay attacks - _executedMessages[messageHash] = executor; - // Update gas limit if it's not zero stored.gasLimit = BranchlessMath.ternaryU64(info.gasLimit > 0, info.gasLimit, stored.gasLimit); From 11444411d3264ae11c8337b4d1d13bcdaac07e1c Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 15:56:53 -0300 Subject: [PATCH 14/17] Add memory-safe-assembly tag to countNonZeros method --- src/utils/GasUtils.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 5d7d6bb..8179f6f 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -229,7 +229,7 @@ library GasUtils { * gas cost = 217 + (words * 112) + ((words - 1) * 193) */ function countNonZeros(bytes memory data) internal pure returns (uint256 nonZeros) { - // /// @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { // Efficient algorithm for counting non-zero bytes in parallel let size := mload(data) From 8aaa40bb26dcfe5a1cd492a3f2b7786fbdbf70d2 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 15:59:45 -0300 Subject: [PATCH 15/17] Fix bytes32 method overloading --- src/Gateway.sol | 2 +- src/utils/BranchlessMath.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index cf8b160..ce3d771 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -444,7 +444,7 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { // Verify and update domain separator if it's not zero stored.domainSeparator = - BranchlessMath.ternaryB32(info.domainSeparator != bytes32(0), info.domainSeparator, 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 diff --git a/src/utils/BranchlessMath.sol b/src/utils/BranchlessMath.sol index da41a08..368f629 100644 --- a/src/utils/BranchlessMath.sol +++ b/src/utils/BranchlessMath.sol @@ -52,7 +52,7 @@ library BranchlessMath { * @dev If `condition` is true returns `a`, otherwise returns `b`. * see `BranchlessMath.ternary` */ - function ternaryB32(bool condition, bytes32 a, bytes32 b) internal pure returns (bytes32 r) { + function ternary(bool condition, bytes32 a, bytes32 b) internal pure returns (bytes32 r) { assembly { r := xor(b, mul(xor(a, b), condition)) } From 1a28daf3c36983b62a5c4a1acd0b5b88b4c8a052 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 16:06:38 -0300 Subject: [PATCH 16/17] Explicitly make deprecated method private --- src/Gateway.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index ce3d771..ae76c49 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -72,24 +72,24 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { bytes32 internal constant FIRST_MESSAGE_PLACEHOLDER = bytes32(uint256(2 ** 256 - 1)); // Shard data, maps the pubkey coordX (which is already collision resistant) to shard info. - mapping(bytes32 => KeyInfo) _shards; + mapping(bytes32 => KeyInfo) private _shards; // GMP message status - mapping(bytes32 => GmpInfo) _messages; + mapping(bytes32 => GmpInfo) private _messages; // GAP necessary for migration purposes - mapping(GmpSender => mapping(uint16 => uint256)) _deprecated_Deposits; - mapping(uint16 => bytes32) _deprecated_Networks; + mapping(GmpSender => mapping(uint16 => uint256)) private _deprecated_Deposits; + mapping(uint16 => bytes32) private _deprecated_Networks; // Hash of the previous GMP message submitted. bytes32 public prevMessageHash; // Replay protection mechanism, stores the hash of the executed messages // messageHash => shardId - mapping(bytes32 => bytes32) _executedMessages; + mapping(bytes32 => bytes32) private _executedMessages; // Network ID => Source network - mapping(uint16 => NetworkInfo) _networkInfo; + mapping(uint16 => NetworkInfo) private _networkInfo; /** * @dev Shard info stored in the Gateway Contract From b1e8439c852e7c05f1ab456f1fb873ebd15197c0 Mon Sep 17 00:00:00 2001 From: Lohann Paterno Coutinho Ferreira Date: Fri, 9 Aug 2024 17:29:52 -0300 Subject: [PATCH 17/17] Add a method for query the network info --- src/Gateway.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Gateway.sol b/src/Gateway.sol index ae76c49..b7e38a3 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -181,6 +181,10 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { return NETWORK_ID; } + function networkInfo(uint16 id) external view returns (NetworkInfo memory) { + return _networkInfo[id]; + } + /** * @dev Verify if shard exists, if the TSS signature is valid then increment shard's nonce. */