feat: addressed ackee's report

contracts/InterchainTokenFactory.sol

@@ -149,6 +149,8 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
             if (minter == address(interchainTokenService)) revert InvalidMinter(minter);
 
             minterBytes = minter.toBytes();
+        } else {
+            revert EmptyInterchainToken();
         }
 
         tokenId = _deployInterchainToken(deploySalt, currentChain, name, symbol, decimals, minterBytes, gasValue);

contracts/InterchainTokenService.sol

@@ -395,6 +395,26 @@ contract InterchainTokenService is
         return _contractCallValue(payload);
     }
 
+    /**
+     * @notice Executes 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.
+     */
+    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 Express executes operations based on the payload and selector.
      * @dev This function is `payable` because non-payable functions cannot be called in a multicall that calls other `payable` functions.
@@ -646,26 +666,6 @@ contract InterchainTokenService is
         }
     }
 
-    /**
-     * @notice Executes 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.
-     */
-    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 Processes the payload data for a send token call.
      * @param commandId The unique message id.

contracts/TokenHandler.sol

@@ -4,7 +4,7 @@
 
 import { ITokenHandler } from './interfaces/ITokenHandler.sol';
 import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol';
 import { SafeTokenTransfer, SafeTokenTransferFrom, SafeTokenCall } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/SafeTransfer.sol';
 import { ReentrancyGuard } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/ReentrancyGuard.sol';
 import { Create3AddressFixed } from './utils/Create3AddressFixed.sol';
 
@@ -16,12 +16,11 @@
 
 /**
  * @title TokenHandler
- * @notice This interface is responsible for handling tokens before initiating an interchain token transfer, or after receiving one.
+ * @notice This contract is responsible for handling tokens before initiating an interchain token transfer, or after receiving one.
  */
 contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Create3AddressFixed {
     using SafeTokenTransferFrom for IERC20;
     using SafeTokenCall for IERC20;
-    using SafeTokenTransfer for IERC20;
 
     /**
      * @notice This function gives token to a specified address from the token manager.

contracts/interchain-token/ERC20Permit.sol

@@ -23,6 +23,7 @@ abstract contract ERC20Permit is IERC20, IERC20Permit, ERC20 {
      */
     bytes32 public nameHash;
 
+    // Prefix for the EIP712 structured data.
     string private constant EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA = '\x19\x01';
 
     // keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)')
@@ -71,6 +72,15 @@ abstract contract ERC20Permit is IERC20, IERC20Permit, ERC20 {
     function permit(address issuer, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s) external {
         if (block.timestamp > deadline) revert PermitExpired();
 
+        // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature
+        // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines
+        // the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1, and for v in (282): v ∈ {27, 28}. Most
+        // signatures from current libraries generate a unique signature with an s-value in the lower half order.
+        //
+        // If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value
+        // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or
+        // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
+        // these malleable signatures as well.
         if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) revert InvalidS();
 
         if (v != 27 && v != 28) revert InvalidV();

contracts/interchain-token/InterchainToken.sol

@@ -2,12 +2,9 @@
 
 pragma solidity ^0.8.0;
 
-import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol';
-
 import { IInterchainToken } from '../interfaces/IInterchainToken.sol';
 
 import { InterchainTokenStandard } from './InterchainTokenStandard.sol';
-import { ERC20 } from './ERC20.sol';
 import { ERC20Permit } from './ERC20Permit.sol';
 import { Minter } from '../utils/Minter.sol';
 
@@ -16,9 +13,7 @@ import { Minter } from '../utils/Minter.sol';
  * @notice This contract implements an interchain token which extends InterchainToken functionality.
  * @dev This contract also inherits Minter and Implementation logic.
  */
-contract InterchainToken is InterchainTokenStandard, ERC20, ERC20Permit, Minter, IInterchainToken {
-    using AddressBytes for bytes;
-
+contract InterchainToken is InterchainTokenStandard, ERC20Permit, Minter, IInterchainToken {
     string public name;
     string public symbol;
     uint8 public decimals;

contracts/interfaces/IBaseTokenManager.sol

@@ -9,12 +9,14 @@ pragma solidity ^0.8.0;
 interface IBaseTokenManager {
     /**
      * @notice A function that returns the token id.
+     * @dev This is stored in the proxy and should not be called in the implementation, but it is included here so that the interface properly tells us what functions exist.
      */
     function interchainTokenId() external view returns (bytes32);
 
     /**
      * @notice A function that should return the address of the token.
      * Must be overridden in the inheriting contract.
+     * @dev This is stored in the proxy and should not be called in the implementation, but it is included here so that the interface properly tells us what functions exist.
      * @return address address of the token.
      */
     function tokenAddress() external view returns (address);

contracts/interfaces/IInterchainTokenFactory.sol

@@ -16,11 +16,10 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
     error InvalidChainName();
     error InvalidMinter(address minter);
     error NotMinter(address minter);
-    error NotOperator(address operator);
-    error NotServiceOwner(address sender);
     error NotSupported();
     error RemoteDeploymentNotApproved();
     error InvalidTokenId(bytes32 tokenId, bytes32 expectedTokenId);
+    error EmptyInterchainToken();
 
     /// @notice Emitted when a minter approves a deployer for a remote interchain token deployment that uses a custom destinationMinter address.
     event DeployRemoteInterchainTokenApproval(

contracts/interfaces/IInterchainTokenService.sol

@@ -29,7 +29,6 @@ interface IInterchainTokenService is
     IAddressTracker,
     IUpgradable
 {
-    error InvalidTokenManagerImplementationType(address implementation);
     error InvalidChainName();
     error NotRemoteService();
     error TokenManagerDoesNotExist(bytes32 tokenId);

contracts/interfaces/ITokenManager.sol

@@ -15,16 +15,12 @@ import { IFlowLimit } from './IFlowLimit.sol';
 interface ITokenManager is IBaseTokenManager, IOperator, IFlowLimit, IImplementation {
     error TokenLinkerZeroAddress();
     error NotService(address caller);
-    error TakeTokenFailed();
-    error GiveTokenFailed();
-    error NotToken(address caller);
-    error ZeroAddress();
-    error AlreadyFlowLimiter(address flowLimiter);
-    error NotFlowLimiter(address flowLimiter);
+    error ZeroTokenAddress();
     error NotSupported();
 
     /**
      * @notice Returns implementation type of this token manager.
+     * @dev This is stored in the proxy and should not be called in the implementation, but it is included here so that the interface properly tells us what functions exist.
      * @return uint256 The implementation type of this token manager.
      */
     function implementationType() external view returns (uint256);

contracts/interfaces/ITokenManagerDeployer.sol

@@ -7,7 +7,6 @@ pragma solidity ^0.8.0;
  * @notice This interface is used to deploy new instances of the TokenManagerProxy contract.
  */
 interface ITokenManagerDeployer {
-    error AddressZero();
     error TokenManagerDeploymentFailed();
 
     /**

contracts/token-manager/TokenManager.sol

@@ -18,6 +18,7 @@ import { FlowLimit } from '../utils/FlowLimit.sol';
 /**
  * @title TokenManager
  * @notice This contract is responsible for managing tokens, such as setting locking token balances, or setting flow limits, for interchain transfers.
+ * @dev Should only be used as an implementation for TokenManagerProxy.
  */
 contract TokenManager is ITokenManager, Operator, FlowLimit, Implementation, Multicall {
     using AddressBytes for bytes;
@@ -57,8 +58,8 @@ contract TokenManager is ITokenManager, Operator, FlowLimit, Implementation, Mul
 
     /**
      * @notice Reads the token address from the proxy.
-     * @dev This function is not supported when directly called on the implementation. It
-     * must be called by the proxy.
+     * @dev This function is not supported when directly called on the implementation.
+     * It must be called by the proxy. It is included here so that the interace shows this function as existing, for better UX.
      * @return tokenAddress_ The address of the token.
      */
     function tokenAddress() external view virtual returns (address) {
@@ -68,6 +69,7 @@ contract TokenManager is ITokenManager, Operator, FlowLimit, Implementation, Mul
     /**
      * @notice A function that returns the token id.
      * @dev This will only work when implementation is called by a proxy, which stores the tokenId as an immutable.
+     * It is included here so that the interace shows this function as existing, for better UX.
      * @return bytes32 The interchain token ID.
      */
     function interchainTokenId() public pure returns (bytes32) {
@@ -89,6 +91,7 @@ contract TokenManager is ITokenManager, Operator, FlowLimit, Implementation, Mul
      */
     function getTokenAddressFromParams(bytes calldata params_) external pure returns (address tokenAddress_) {
         (, tokenAddress_) = abi.decode(params_, (bytes, address));
+        if (tokenAddress_ == address(0)) revert ZeroTokenAddress();
     }
 
     /**

contracts/types/InterchainTokenServiceTypes.sol

@@ -5,7 +5,9 @@ pragma solidity ^0.8.0;
 enum MessageType {
     INTERCHAIN_TRANSFER,
     DEPLOY_INTERCHAIN_TOKEN,
-    DEPLOY_TOKEN_MANAGER
+    DEPLOY_TOKEN_MANAGER,
+    SEND_TO_HUB,
+    RECEIVE_FROM_HUB
 }
 
 struct InterchainTransfer {

test/InterchainToken.js

@@ -147,7 +147,7 @@ describe('InterchainToken', () => {
             const contractBytecodeHash = keccak256(contractBytecode);
 
             const expected = {
-                london: '0xa01cf28b0b6ce6dc3b466e995585d69486400d671fce0ea8d06beba583e6f3bb',
+                london: '0x482146829055f052063003e9cf0ffaf798a12fb58088c2667566a135b9568355',
             }[getEVMVersion()];
 
             expect(contractBytecodeHash).to.be.equal(expected);

test/InterchainTokenFactory.js

@@ -235,18 +235,13 @@ describe('InterchainTokenFactory', () => {
         it('Should register a token if the mint amount is zero and minter is the zero address', async () => {
             const salt = keccak256('0x123456');
             tokenId = await tokenFactory.interchainTokenId(wallet.address, salt);
-            const tokenAddress = await service.interchainTokenAddress(tokenId);
-            const minterBytes = new Uint8Array();
-            const params = defaultAbiCoder.encode(['bytes', 'address'], [minterBytes, tokenAddress]);
-            const tokenManager = await getContractAt('TokenManager', await service.tokenManagerAddress(tokenId), wallet);
-
-            await expect(tokenFactory.deployInterchainToken(salt, name, symbol, decimals, 0, AddressZero))
-                .to.emit(service, 'InterchainTokenDeployed')
-                .withArgs(tokenId, tokenAddress, AddressZero, name, symbol, decimals)
-                .and.to.emit(service, 'TokenManagerDeployed')
-                .withArgs(tokenId, tokenManager.address, NATIVE_INTERCHAIN_TOKEN, params);
 
-            await checkRoles(tokenManager, AddressZero);
+            await expectRevert(
+                (gasOptions) => tokenFactory.deployInterchainToken(salt, name, symbol, decimals, 0, AddressZero, { gasOptions }),
+                tokenFactory,
+                'EmptyInterchainToken',
+                [],
+            );
         });
 
         it('Should register a token if the mint amount is greater than zero and the minter is the zero address', async () => {

test/InterchainTokenService.js

@@ -909,13 +909,13 @@ describe('Interchain Token Service', () => {
             );
         });
 
-        it('Should revert on deploying a token manager if token handler post deploy fails', async () => {
+        it('Should revert on deploying a token manager with an empty token', async () => {
             const params = defaultAbiCoder.encode(['bytes', 'address'], [wallet.address, AddressZero]);
 
             await expectRevert(
                 (gasOptions) => service.deployTokenManager(salt, '', LOCK_UNLOCK, params, 0, gasOptions),
                 service,
-                'PostDeployFailed',
+                'TokenManagerDeploymentFailed',
             );
         });
 

test/TokenManager.js

@@ -92,7 +92,7 @@ describe('Token Manager', () => {
             const proxyBytecodeHash = keccak256(proxyBytecode);
 
             const expected = {
-                london: '0x8080880884e00735cc1a34bdf5c1ea6c023db60a01cfa1e951ca41ecf5fd8836',
+                london: '0x3b336208cc75ca67bdd39bdeed72871ce795e6e9cd28e20f811599ea51973ebf',
             }[getEVMVersion()];
 
             expect(proxyBytecodeHash).to.be.equal(expected);