ci(Slither): action + fixes + comments

.github/workflows/slither.yml

@@ -0,0 +1,16 @@
+name: Slither Static Analysis
+
+on:
+  - pull_request
+
+jobs:
+  analyze:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Install Dependencies
+        run: npm ci
+
+      - name: Run Slither
+        uses: crytic/[email protected]

contracts/interchain-token-service/InterchainTokenService.sol

@@ -282,7 +282,9 @@ contract InterchainTokenService is
     function deployRemoteCanonicalToken(bytes32 tokenId, string calldata destinationChain, uint256 gasValue) public payable notPaused {
         address tokenAddress = getValidTokenManagerAddress(tokenId);
         tokenAddress = ITokenManager(tokenAddress).tokenAddress();
+
         if (getCanonicalTokenId(tokenAddress) != tokenId) revert NotCanonicalTokenManager();
+
         (string memory tokenName, string memory tokenSymbol, uint8 tokenDecimals) = _validateToken(tokenAddress);
         _deployRemoteStandardizedToken(tokenId, tokenName, tokenSymbol, tokenDecimals, '', '', 0, '', destinationChain, gasValue);
     }
@@ -300,8 +302,10 @@ contract InterchainTokenService is
     ) public payable notPaused returns (bytes32 tokenId) {
         address deployer_ = msg.sender;
         tokenId = getCustomTokenId(deployer_, salt);
-        _deployTokenManager(tokenId, tokenManagerType, params);
+
         emit CustomTokenIdClaimed(tokenId, deployer_, salt);
+
+        _deployTokenManager(tokenId, tokenManagerType, params);
     }
 
     /**
@@ -324,8 +328,10 @@ contract InterchainTokenService is
     ) external payable notPaused returns (bytes32 tokenId) {
         address deployer_ = msg.sender;
         tokenId = getCustomTokenId(deployer_, salt);
-        _deployRemoteTokenManager(tokenId, destinationChain, gasValue, tokenManagerType, params);
+
         emit CustomTokenIdClaimed(tokenId, deployer_, salt);
+
+        _deployRemoteTokenManager(tokenId, destinationChain, gasValue, tokenManagerType, params);
     }
 
     /**
@@ -361,7 +367,7 @@ contract InterchainTokenService is
      * @param distributor the address that will be able to mint and burn the deployed token.
      * @param mintTo The address where the minted tokens will be sent upon deployment
      * @param mintAmount The amount of tokens to be minted upon deployment
-     * @param operator The operator data for standardized tokens
+     * @param operator_ The operator data for standardized tokens
      * @param destinationChain the name of the destination chain to deploy to.
      * @param gasValue the amount of native tokens to be used to pay for gas for the remote deployment. At least the amount
      * specified needs to be passed to the call
@@ -375,7 +381,7 @@ contract InterchainTokenService is
         bytes memory distributor,
         bytes memory mintTo,
         uint256 mintAmount,
-        bytes memory operator,
+        bytes memory operator_,
         string calldata destinationChain,
         uint256 gasValue
     ) external payable notPaused {
@@ -388,7 +394,7 @@ contract InterchainTokenService is
             distributor,
             mintTo,
             mintAmount,
-            operator,
+            operator_,
             destinationChain,
             gasValue
         );
@@ -490,11 +496,12 @@ contract InterchainTokenService is
      * @param tokenIds an array of the token Ids of the tokenManagers to set the flow limit of.
      * @param flowLimits the flowLimits to set
      */
-    function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
+    function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
         uint256 length = tokenIds.length;
         if (length != flowLimits.length) revert LengthMismatch();
         for (uint256 i; i < length; ++i) {
             ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenIds[i]));
+            // slither-disable-next-line calls-loop
             tokenManager.setFlowLimit(flowLimits[i]);
         }
     }
@@ -580,15 +587,18 @@ contract InterchainTokenService is
             bytes memory data;
             (, , , , sourceAddress, data) = abi.decode(payload, (uint256, bytes32, bytes, uint256, bytes, bytes));
 
+            // slither-disable-next-line reentrancy-events
+            emit TokenReceivedWithData(tokenId, sourceChain, destinationAddress, amount, sourceAddress, data);
+
             IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken(
                 sourceChain,
                 sourceAddress,
                 data,
                 tokenId,
                 amount
             );
-            emit TokenReceivedWithData(tokenId, sourceChain, destinationAddress, amount, sourceAddress, data);
         } else {
+            // slither-disable-next-line reentrancy-events
             emit TokenReceived(tokenId, sourceChain, destinationAddress, amount);
         }
     }
@@ -688,9 +698,10 @@ contract InterchainTokenService is
         TokenManagerType tokenManagerType,
         bytes memory params
     ) internal {
+        emit RemoteTokenManagerDeploymentInitialized(tokenId, destinationChain, gasValue, tokenManagerType, params);
+
         bytes memory payload = abi.encode(SELECTOR_DEPLOY_TOKEN_MANAGER, tokenId, tokenManagerType, params);
         _callContract(destinationChain, payload, gasValue);
-        emit RemoteTokenManagerDeploymentInitialized(tokenId, destinationChain, gasValue, tokenManagerType, params);
     }
 
     /**
@@ -702,7 +713,7 @@ contract InterchainTokenService is
      * @param distributor The distributor address for the token
      * @param mintTo The address where the minted tokens will be sent upon deployment
      * @param mintAmount The amount of tokens to be minted upon deployment
-     * @param operator The operator data for standardized tokens
+     * @param operator_ The operator data for standardized tokens
      * @param destinationChain The destination chain where the token will be deployed
      * @param gasValue The amount of gas to be paid for the transaction
      */
@@ -714,34 +725,36 @@ contract InterchainTokenService is
         bytes memory distributor,
         bytes memory mintTo,
         uint256 mintAmount,
-        bytes memory operator,
+        bytes memory operator_,
         string calldata destinationChain,
         uint256 gasValue
     ) internal {
-        bytes memory payload = abi.encode(
-            SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN,
+        // slither-disable-next-line reentrancy-events
+        emit RemoteStandardizedTokenAndManagerDeploymentInitialized(
             tokenId,
             name,
             symbol,
             decimals,
             distributor,
             mintTo,
             mintAmount,
-            operator
+            operator_,
+            destinationChain,
+            gasValue
         );
-        _callContract(destinationChain, payload, gasValue);
-        emit RemoteStandardizedTokenAndManagerDeploymentInitialized(
+
+        bytes memory payload = abi.encode(
+            SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN,
             tokenId,
             name,
             symbol,
             decimals,
             distributor,
             mintTo,
             mintAmount,
-            operator,
-            destinationChain,
-            gasValue
+            operator_
         );
+        _callContract(destinationChain, payload, gasValue);
     }
 
     /**
@@ -751,13 +764,14 @@ contract InterchainTokenService is
      * @param params Additional parameters for the token manager deployment
      */
     function _deployTokenManager(bytes32 tokenId, TokenManagerType tokenManagerType, bytes memory params) internal {
+        // slither-disable-next-line reentrancy-events
+        emit TokenManagerDeployed(tokenId, tokenManagerType, params);
+
+        // slither-disable-next-line controlled-delegatecall
         (bool success, ) = tokenManagerDeployer.delegatecall(
             abi.encodeWithSelector(ITokenManagerDeployer.deployTokenManager.selector, tokenId, tokenManagerType, params)
         );
-        if (!success) {
-            revert TokenManagerDeploymentFailed();
-        }
-        emit TokenManagerDeployed(tokenId, tokenManagerType, params);
+        if (!success) revert TokenManagerDeploymentFailed();
     }
 
     /**
@@ -788,9 +802,12 @@ contract InterchainTokenService is
         uint256 mintAmount,
         address mintTo
     ) internal {
+        emit StandardizedTokenDeployed(tokenId, distributor, name, symbol, decimals, mintAmount, mintTo);
+
         bytes32 salt = _getStandardizedTokenSalt(tokenId);
         address tokenManagerAddress = getTokenManagerAddress(tokenId);
 
+        // slither-disable-next-line controlled-delegatecall
         (bool success, ) = standardizedTokenDeployer.delegatecall(
             abi.encodeWithSelector(
                 IStandardizedTokenDeployer.deployStandardizedToken.selector,
@@ -807,7 +824,6 @@ contract InterchainTokenService is
         if (!success) {
             revert StandardizedTokenDeploymentFailed();
         }
-        emit StandardizedTokenDeployed(tokenId, distributor, name, symbol, decimals, mintAmount, mintTo);
     }
 
     function _decodeMetadata(bytes memory metadata) internal pure returns (uint32 version, bytes memory data) {
@@ -860,16 +876,21 @@ contract InterchainTokenService is
     ) internal {
         bytes memory payload;
         if (metadata.length < 4) {
+            // slither-disable-next-line reentrancy-events
+            emit TokenSent(tokenId, destinationChain, destinationAddress, amount);
+
             payload = abi.encode(SELECTOR_SEND_TOKEN, tokenId, destinationAddress, amount);
             _callContract(destinationChain, payload, msg.value);
-            emit TokenSent(tokenId, destinationChain, destinationAddress, amount);
             return;
         }
         uint32 version;
         (version, metadata) = _decodeMetadata(metadata);
         if (version > 0) revert InvalidMetadataVersion(version);
+
+        // slither-disable-next-line reentrancy-events
+        emit TokenSentWithData(tokenId, destinationChain, destinationAddress, amount, sourceAddress, metadata);
+
         payload = abi.encode(SELECTOR_SEND_TOKEN_WITH_DATA, tokenId, destinationAddress, amount, sourceAddress.toBytes(), metadata);
         _callContract(destinationChain, payload, msg.value);
-        emit TokenSentWithData(tokenId, destinationChain, destinationAddress, amount, sourceAddress, metadata);
     }
 }

contracts/interfaces/IDistributable.sol

@@ -11,16 +11,16 @@ interface IDistributable {
 
     /**
      * @notice Get the address of the distributor
-     * @return distributor of the distributor
+     * @return distributor_ of the distributor
      */
-    function distributor() external view returns (address distributor);
+    function distributor() external view returns (address distributor_);
 
     /**
      * @notice Change the distributor of the contract
      * @dev Can only be called by the current distributor
-     * @param distributor The address of the new distributor
+     * @param distributor_ The address of the new distributor
      */
-    function transferDistributorship(address distributor) external;
+    function transferDistributorship(address distributor_) external;
 
     /**
      * @notice Proposed a change of the distributor of the contract

contracts/interfaces/IImplementation.sol

@@ -4,4 +4,11 @@ pragma solidity ^0.8.0;
 
 interface IImplementation {
     error NotProxy();
+
+    /**
+     * @notice Called by the proxy to setup itself.
+     * @dev This should be hidden by the proxy.
+     * @param params the data to be used for the initialization.
+     */
+    function setup(bytes calldata params) external;
 }

contracts/interfaces/IInterchainToken.sol

@@ -2,10 +2,19 @@
 
 pragma solidity ^0.8.0;
 
+import { ITokenManager } from './ITokenManager.sol';
+
 /**
  * @dev Interface of the ERC20 standard as defined in the EIP.
  */
 interface IInterchainToken {
+    /**
+     * @notice Getter for the tokenManager used for this token.
+     * @dev Needs to be overwitten.
+     * @return tokenManager_ the TokenManager called to facilitate cross chain transfers.
+     */
+    function tokenManager() external view returns (ITokenManager tokenManager_);
+
     /**
      * @notice Implementation of the interchainTransfer method
      * @dev We chose to either pass `metadata` as raw data on a remote contract call, or, if no data is passed, just do a transfer.

contracts/interfaces/IInterchainTokenService.sol

@@ -266,7 +266,7 @@ interface IInterchainTokenService is ITokenManagerType, IExpressCallHandler, IAx
      * @param tokenIds An array of tokenIds.
      * @param flowLimits An array of flow limits corresponding to the tokenIds.
      */
-    function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external;
+    function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external;
 
     /**
      * @notice Returns the flow limit for a specific token.

contracts/interfaces/IRemoteAddressValidator.sol

@@ -52,8 +52,8 @@ interface IRemoteAddressValidator {
 
     /**
      * @dev Fetches the interchain token service address for the specified chain
-     * @param chainName Name of the chain
+     * @param chainName_ Name of the chain
      * @return remoteAddress Interchain token service address for the specified chain
      */
-    function getRemoteAddress(string calldata chainName) external view returns (string memory remoteAddress);
+    function getRemoteAddress(string calldata chainName_) external view returns (string memory remoteAddress);
 }

contracts/interfaces/IStandardizedToken.sol

@@ -2,34 +2,23 @@
 
 pragma solidity ^0.8.0;
 
+import { IImplementation } from './IImplementation.sol';
 import { IInterchainToken } from './IInterchainToken.sol';
 import { IDistributable } from './IDistributable.sol';
 import { IERC20MintableBurnable } from './IERC20MintableBurnable.sol';
-import { ITokenManager } from './ITokenManager.sol';
 import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol';
 
 /**
  * @title StandardizedToken
  * @notice This contract implements a standardized token which extends InterchainToken functionality.
  * This contract also inherits Distributable and Implementation logic.
  */
-interface IStandardizedToken is IInterchainToken, IDistributable, IERC20MintableBurnable, IERC20 {
+interface IStandardizedToken is IImplementation, IInterchainToken, IDistributable, IERC20MintableBurnable, IERC20 {
+    error TokenManagerAddressZero();
+    error TokenNameEmpty();
+
     /**
      * @notice Returns the contract id, which a proxy can check to ensure no false implementation was used.
      */
     function contractId() external view returns (bytes32);
-
-    /**
-     * @notice Called by the proxy to setup itself.
-     * @dev This should be hidden by the proxy.
-     * @param params the data to be used for the initialization.
-     */
-    function setup(bytes calldata params) external;
-
-    /**
-     * @notice Getter for the tokenManager used for this token.
-     * @dev Needs to be overwitten.
-     * @return tokenManager_ the TokenManager called to facilitate cross chain transfers.
-     */
-    function tokenManager() external view returns (ITokenManager tokenManager_);
 }

contracts/interfaces/ITokenManagerLiquidityPool.sol

@@ -11,11 +11,12 @@ import { ITokenManager } from './ITokenManager.sol';
 interface ITokenManagerLiquidityPool is ITokenManager {
     /**
      * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends.
-     * @param operator the operator of the TokenManager.
-     * @param tokenAddress the token to be managed.
+     * @param operator_ the operator of the TokenManager.
+     * @param tokenAddress_ the token to be managed.
+     * @param liquidityPool_ he address of the liquidity pool.
      * @return params the resulting params to be passed to custom TokenManager deployments.
      */
-    function getParams(bytes memory operator, address tokenAddress, address liquidityPool) external pure returns (bytes memory params);
+    function getParams(bytes memory operator_, address tokenAddress_, address liquidityPool_) external pure returns (bytes memory params);
 
     /**
      * @dev Reads the stored liquidity pool address from the specified storage slot

contracts/interfaces/ITokenManagerLockUnlock.sol

@@ -11,9 +11,9 @@ import { ITokenManager } from './ITokenManager.sol';
 interface ITokenManagerLockUnlock is ITokenManager {
     /**
      * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends.
-     * @param operator the operator of the TokenManager.
-     * @param tokenAddress the token to be managed.
+     * @param operator_ the operator of the TokenManager.
+     * @param tokenAddress_ the token to be managed.
      * @return params the resulting params to be passed to custom TokenManager deployments.
      */
-    function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params);
+    function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params);
 }

contracts/interfaces/ITokenManagerLockUnlockFee.sol

@@ -11,9 +11,9 @@ import { ITokenManager } from './ITokenManager.sol';
 interface ITokenManagerLockUnlockFee is ITokenManager {
     /**
      * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends.
-     * @param operator the operator of the TokenManager.
-     * @param tokenAddress the token to be managed.
+     * @param operator_ the operator of the TokenManager.
+     * @param tokenAddress_ the token to be managed.
      * @return params the resulting params to be passed to custom TokenManager deployments.
      */
-    function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params);
+    function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params);
 }