feat: added additional checks for call contract with token (#249)

* Added additional checks but code size is too large * Added value for contractCallWithToken and optimized contract size * trying to fix tests * Update the create3address of ITS to use a custom bytecodehash. * prettier * fix tests * Added a few tests * fixed one more test * fixed all tests * prettier * made lint happy * working on slither * made slither happy * prettier * Using constant for the hash as well * addressed comments * added some tests * added some coverage tests, found a bug too! * a small style fix * fixed a bug * addressed some comments * prettier * fixed a test * remove modifier that should not exist * rename a function * Update contracts/InterchainTokenService.sol Co-authored-by: Milap Sheth <[email protected]> * reinteroduce the modifiers since they are needed after all * Update contracts/utils/Create3AddressFixed.sol * add a docstring * prettier and fixed tests * address comments * fix factory import --------- Co-authored-by: Milap Sheth <[email protected]> Co-authored-by: Milap Sheth <[email protected]>
axelarnetwork · Apr 12, 2024 · 2ed153a · 2ed153a
1 parent e2cb43c
commit 2ed153a
Show file tree

Hide file tree

Showing 12 changed files with 640 additions and 81 deletions.
diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol
@@ -55,10 +55,10 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
      * @param chainNameHash_ The hash of the chain name.
      * @param deployer The address of the deployer.
      * @param salt A unique identifier to generate the salt.
-     * @return bytes32 The calculated salt for the interchain token.
+     * @return tokenSalt The calculated salt for the interchain token.
      */
-    function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32) {
-        return keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt));
+    function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32 tokenSalt) {
+        tokenSalt = keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt));
     }
 
     /**

diff --git a/contracts/InterchainTokenService.sol b/contracts/InterchainTokenService.sol
@@ -7,7 +7,6 @@ import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contr
 import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol';
 import { ExpressExecutorTracker } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/express/ExpressExecutorTracker.sol';
 import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol';
-import { Create3Address } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3Address.sol';
 import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol';
 import { Multicall } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Multicall.sol';
 import { Pausable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Pausable.sol';
@@ -22,6 +21,7 @@ import { IInterchainTokenExecutable } from './interfaces/IInterchainTokenExecuta
 import { IInterchainTokenExpressExecutable } from './interfaces/IInterchainTokenExpressExecutable.sol';
 import { ITokenManager } from './interfaces/ITokenManager.sol';
 import { IERC20Named } from './interfaces/IERC20Named.sol';
+import { Create3AddressFixed } from './utils/Create3AddressFixed.sol';
 
 import { Operator } from './utils/Operator.sol';
 
@@ -37,7 +37,7 @@ contract InterchainTokenService is
     Operator,
     Pausable,
     Multicall,
-    Create3Address,
+    Create3AddressFixed,
     ExpressExecutorTracker,
     InterchainAddressTracker,
     IInterchainTokenService
@@ -343,13 +343,7 @@ contract InterchainTokenService is
         string calldata sourceAddress,
         bytes calldata payload
     ) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) {
-        (uint256 messageType, bytes32 tokenId, , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, uint256));
-
-        if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
-            revert InvalidExpressMessageType(messageType);
-        }
-
-        return (validTokenAddress(tokenId), amount);
+        return _contractCallValue(payload);
     }
 
     /**
@@ -626,32 +620,59 @@ contract InterchainTokenService is
      * @param sourceAddress The address of the remote ITS where the transaction originates from.
      * @param payload The encoded data payload for the transaction.
      */
-    function execute(bytes32 commandId, string calldata sourceChain, string calldata sourceAddress, bytes calldata payload) public {
+    function execute(
+        bytes32 commandId,
+        string calldata sourceChain,
+        string calldata sourceAddress,
+        bytes calldata payload
+    ) external onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
         bytes32 payloadHash = keccak256(payload);
 
         if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash)) revert NotApprovedByGateway();
 
         _execute(commandId, sourceChain, sourceAddress, payload, payloadHash);
     }
 
+    /**
+     * @notice Returns the amount of token that this call is worth.
+     * @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address.
+     * @param sourceChain The source chain.
+     * @param sourceAddress The source address on the source chain.
+     * @param payload The payload sent with the call.
+     * @param symbol The symbol symbol for the call.
+     * @param amount The amount for the call.
+     * @return address The token address.
+     * @return uint256 The value the call is worth.
+     */
     function contractCallWithTokenValue(
-        string calldata /*sourceChain*/,
-        string calldata /*sourceAddress*/,
-        bytes calldata /*payload*/,
-        string calldata /*symbol*/,
-        uint256 /*amount*/
-    ) public view virtual returns (address, uint256) {
-        revert ExecuteWithTokenNotSupported();
+        string calldata sourceChain,
+        string calldata sourceAddress,
+        bytes calldata payload,
+        string calldata symbol,
+        uint256 amount
+    ) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) {
+        _checkPayloadAgainstGatewayData(payload, symbol, amount);
+        return _contractCallValue(payload);
     }
 
+    /**
+     * @notice Express executes with a gateway token operations based on the payload and selector.
+     * @param commandId The unique message id.
+     * @param sourceChain The chain where the transaction originates from.
+     * @param sourceAddress The address of the remote ITS where the transaction originates from.
+     * @param payload The encoded data payload for the transaction.
+     * @param tokenSymbol The symbol symbol for the call.
+     * @param amount The amount for the call.
+     */
     function expressExecuteWithToken(
         bytes32 commandId,
         string calldata sourceChain,
         string calldata sourceAddress,
         bytes calldata payload,
-        string calldata /*tokenSymbol*/,
-        uint256 /*amount*/
+        string calldata tokenSymbol,
+        uint256 amount
     ) external payable {
+        _checkPayloadAgainstGatewayData(payload, tokenSymbol, amount);
         // It should be ok to ignore the symbol and amount since this info exists on the payload.
         expressExecute(commandId, sourceChain, sourceAddress, payload);
     }
@@ -663,13 +684,22 @@ contract InterchainTokenService is
         bytes calldata payload,
         string calldata tokenSymbol,
         uint256 amount
-    ) external {
-        bytes32 payloadHash = keccak256(payload);
+    ) external onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
+        _executeWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount);
+    }
 
-        if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount))
-            revert NotApprovedByGateway();
+    /**
+     * @notice Check that the tokenId from the payload is a token that is registered in the gateway with the proper tokenSymbol, with the right amount from the payload.
+     * Also check that the amount in the payload matches the one for the call.
+     * @param payload The payload for the call contract with token.
+     * @param tokenSymbol The tokenSymbol for the call contract with token.
+     * @param amount The amount for the call contract with token.
+     */
+    function _checkPayloadAgainstGatewayData(bytes calldata payload, string calldata tokenSymbol, uint256 amount) internal view {
+        (, bytes32 tokenId, , , uint256 amountInPayload) = abi.decode(payload, (uint256, bytes32, uint256, uint256, uint256));
 
-        _execute(commandId, sourceChain, sourceAddress, payload, payloadHash);
+        if (validTokenAddress(tokenId) != gateway.tokenAddresses(tokenSymbol) || amount != amountInPayload)
+            revert InvalidGatewayTokenTransfer(tokenId, payload, tokenSymbol, amount);
     }
 
     /**
@@ -857,15 +887,10 @@ contract InterchainTokenService is
         string calldata sourceAddress,
         bytes calldata payload,
         bytes32 payloadHash
-    ) internal onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
+    ) internal {
         uint256 messageType = abi.decode(payload, (uint256));
         if (messageType == MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
-            address expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash);
-
-            if (expressExecutor != address(0)) {
-                emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor);
-            }
-
+            address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash);
             _processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload);
         } else if (messageType == MESSAGE_TYPE_DEPLOY_TOKEN_MANAGER) {
             _processDeployTokenManagerPayload(payload);
@@ -876,6 +901,32 @@ contract InterchainTokenService is
         }
     }
 
+    function _executeWithToken(
+        bytes32 commandId,
+        string calldata sourceChain,
+        string calldata sourceAddress,
+        bytes calldata payload,
+        string calldata tokenSymbol,
+        uint256 amount
+    ) internal {
+        bytes32 payloadHash = keccak256(payload);
+
+        if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount))
+            revert NotApprovedByGateway();
+
+        uint256 messageType = abi.decode(payload, (uint256));
+        if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
+            revert InvalidMessageType(messageType);
+        }
+
+        _checkPayloadAgainstGatewayData(payload, tokenSymbol, amount);
+
+        // slither-disable-next-line reentrancy-events
+        address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash);
+
+        _processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload);
+    }
+
     /**
      * @notice Deploys a token manager on a destination chain.
      * @param tokenId The ID of the token.
@@ -1114,4 +1165,33 @@ contract InterchainTokenService is
 
         return (amount, tokenAddress);
     }
+
+    /**
+     * @notice Returns the amount of token that this call is worth.
+     * @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address.
+     * @param payload The payload sent with the call.
+     * @return address The token address.
+     * @return uint256 The value the call is worth.
+     */
+    function _contractCallValue(bytes calldata payload) internal view returns (address, uint256) {
+        (uint256 messageType, bytes32 tokenId, , , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, bytes, uint256));
+        if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
+            revert InvalidExpressMessageType(messageType);
+        }
+
+        return (validTokenAddress(tokenId), amount);
+    }
+
+    function _getExpressExecutorAndEmitEvent(
+        bytes32 commandId,
+        string calldata sourceChain,
+        string calldata sourceAddress,
+        bytes32 payloadHash
+    ) internal returns (address expressExecutor) {
+        expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash);
+
+        if (expressExecutor != address(0)) {
+            emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor);
+        }
+    }
 }
diff --git a/contracts/interfaces/IInterchainTokenFactory.sol b/contracts/interfaces/IInterchainTokenFactory.sol
@@ -37,9 +37,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
      * @param chainNameHash_ The hash of the chain name.
      * @param deployer The address of the deployer.
      * @param salt A unique identifier to generate the salt.
-     * @return bytes32 The calculated salt for the interchain token.
+     * @return tokenSalt The calculated salt for the interchain token.
      */
-    function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32);
+    function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32 tokenSalt);
 
     /**
      * @notice Computes the ID for an interchain token based on the deployer and a salt.
@@ -97,9 +97,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
      * @notice Calculates the salt for a canonical interchain token.
      * @param chainNameHash_ The hash of the chain name.
      * @param tokenAddress The address of the token.
-     * @return salt The calculated salt for the interchain token.
+     * @return tokenSalt The calculated salt for the interchain token.
      */
-    function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 salt);
+    function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 tokenSalt);
 
     /**
      * @notice Computes the ID for a canonical interchain token based on its address.

diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol
@@ -48,6 +48,7 @@ interface IInterchainTokenService is
     error EmptyData();
     error PostDeployFailed(bytes data);
     error ZeroAmount();
+    error InvalidGatewayTokenTransfer(bytes32 tokenId, bytes payload, string tokenSymbol, uint256 amount);
 
     event InterchainTransfer(
         bytes32 indexed tokenId,

diff --git a/contracts/test/utils/TestCreate3Fixed.sol b/contracts/test/utils/TestCreate3Fixed.sol
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { Create3Fixed } from '../../utils/Create3Fixed.sol';
+
+contract TestCreate3Fixed is Create3Fixed {
+    event Deployed(address addr);
+
+    function deploy(bytes memory code, bytes32 salt) public payable returns (address addr) {
+        addr = _create3(code, salt);
+
+        emit Deployed(addr);
+    }
+
+    function deployedAddress(bytes32 salt) public view returns (address addr) {
+        addr = _create3Address(salt);
+    }
+}
diff --git a/contracts/utils/Create3AddressFixed.sol b/contracts/utils/Create3AddressFixed.sol
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+/**
+ * @title Create3AddressFixed contract
+ * @notice This contract can be used to predict the deterministic deployment address of a contract deployed with the `CREATE3` technique.
+ * It is equivalent to the Create3Address found in axelar-gmp-sdk-solidity repo but uses a fixed bytecode for CreateDeploy,
+ * which allows changing compilation options (like number of runs) without affecting the future deployment addresses.
+ */
+contract Create3AddressFixed {
+    // slither-disable-next-line too-many-digits
+    bytes internal constant CREATE_DEPLOY_BYTECODE =
+        hex'608060405234801561001057600080fd5b50610162806100206000396000f3fe60806040526004361061001d5760003560e01c806277436014610022575b600080fd5b61003561003036600461007b565b610037565b005b8051602082016000f061004957600080fd5b50565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b60006020828403121561008d57600080fd5b813567ffffffffffffffff808211156100a557600080fd5b818401915084601f8301126100b957600080fd5b8135818111156100cb576100cb61004c565b604051601f8201601f19908116603f011681019083821181831017156100f3576100f361004c565b8160405282815287602084870101111561010c57600080fd5b82602086016020830137600092810160200192909252509594505050505056fea264697066735822122094780ce55d28f1d568f4e0ab1b9dc230b96e952b73d2e06456fbff2289fa27f464736f6c63430008150033';
+    bytes32 internal constant CREATE_DEPLOY_BYTECODE_HASH = keccak256(CREATE_DEPLOY_BYTECODE);
+
+    /**
+     * @notice Compute the deployed address that will result from the `CREATE3` method.
+     * @param deploySalt A salt to influence the contract address
+     * @return deployed The deterministic contract address if it was deployed
+     */
+    function _create3Address(bytes32 deploySalt) internal view returns (address deployed) {
+        address deployer = address(
+            uint160(uint256(keccak256(abi.encodePacked(hex'ff', address(this), deploySalt, CREATE_DEPLOY_BYTECODE_HASH))))
+        );
+
+        deployed = address(uint160(uint256(keccak256(abi.encodePacked(hex'd6_94', deployer, hex'01')))));
+    }
+}
diff --git a/contracts/utils/Create3Fixed.sol b/contracts/utils/Create3Fixed.sol
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { IDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IDeploy.sol';
+import { ContractAddress } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/ContractAddress.sol';
+import { CreateDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/CreateDeploy.sol';
+import { Create3AddressFixed } from './Create3AddressFixed.sol';
+
+/**
+ * @title Create3Fixed contract
+ * @notice This contract can be used to deploy a contract with a deterministic address that depends only on
+ * the deployer address and deployment salt, not the contract bytecode and constructor parameters.
+ * It uses a fixed bytecode to allow changing the compilation settings without affecting the deployment address in the future.
+ */
+contract Create3Fixed is Create3AddressFixed, IDeploy {
+    using ContractAddress for address;
+
+    /**
+     * @notice Deploys a new contract using the `CREATE3` method.
+     * @dev This function first deploys the CreateDeploy contract using
+     * the `CREATE2` opcode and then utilizes the CreateDeploy to deploy the
+     * new contract with the `CREATE` opcode.
+     * @param bytecode The bytecode of the contract to be deployed
+     * @param deploySalt A salt to influence the contract address
+     * @return deployed The address of the deployed contract
+     */
+    function _create3(bytes memory bytecode, bytes32 deploySalt) internal returns (address deployed) {
+        deployed = _create3Address(deploySalt);
+
+        if (bytecode.length == 0) revert EmptyBytecode();
+        if (deployed.isContract()) revert AlreadyDeployed();
+
+        // Deploy using create2
+        CreateDeploy createDeploy;
+        bytes memory createDeployBytecode_ = CREATE_DEPLOY_BYTECODE;
+        uint256 length = createDeployBytecode_.length;
+        assembly {
+            createDeploy := create2(0, add(createDeployBytecode_, 0x20), length, deploySalt)
+        }
+
+        if (address(createDeploy) == address(0)) revert DeployFailed();
+        // Deploy using create
+        createDeploy.deploy(bytecode);
+    }
+}
diff --git a/contracts/utils/InterchainTokenDeployer.sol b/contracts/utils/InterchainTokenDeployer.sol
@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.0;
 
-import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol';
+import { Create3Fixed } from './Create3Fixed.sol';
 
 import { IInterchainTokenDeployer } from '../interfaces/IInterchainTokenDeployer.sol';
 import { IInterchainToken } from '../interfaces/IInterchainToken.sol';
@@ -11,7 +11,7 @@ import { IInterchainToken } from '../interfaces/IInterchainToken.sol';
  * @title InterchainTokenDeployer
  * @notice This contract is used to deploy new instances of the InterchainTokenProxy contract.
  */
-contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3 {
+contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3Fixed {
     address public immutable implementationAddress;
 
     /**