ci(Slither): action + fixes + comments (#117)

* renamed folder and changed version * npmignore * npmignore * change version * using include pattern instead. * Fixed most of the things least auhority suggested. * made lint happy * Apply suggestions from code review * fixed some bugs * added events * rename set to transfer for distributor and operator * changed standardized token to always allow token managers to mint/burn it. * using immutable storage for remoteAddressValidator address to save gas * Added some recommended changes * added milap's suggested changes * Fixed some names and some minor gas optimizations * prettier and lint * stash * import .env in hardhat.config * trying to fix .env.example * Added some getters in IRemoteAddressValidator and removed useless check for distributor in the InterchainTokenService. * removed ternary operators * made lint happy * made lint happy * Added a new token manager to handle fee on transfer and added some tests for it as well * fixed the liquidity pool check. * fix a duplication bug * lint * added some more tests * Added more tests * Added proper re-entrancy protection for fee on transfer token managers. * change to tx.origin for refunds * Added support for more kinds of addresses. * some minor gas opts * some more gas optimizations. * Added a getter for chain name to the remote address validator. * moved the tokenManager getter functionality to a separate contract which saves almost a kilobyte of codesize. * made lint happy * Removed tokenManagerGetter and put params into tokenManagers * Added separate tokenManager interfaces * addressed ackeeblockchains's 3.0 report * prettier * added interchain transfer methods to the service and unified receiving tokens a bit. * made lint happy * rename sendToken to interchainTransfer * changed sendToken everywhere * changed from uint256.max to a const * change setting to zero to delete for storage slots. * rearange storage variables to save a bit of gas. * Removed unecesairy casts * made as many event params inexed as possible * Removed unused imports * domain separator is calculated each time. * added some natspec * added an example for using pre-existing custom tokens. * added a comment * feat(TokenManager): MintBurnFrom and MintBurnFromAddress * fix(TokenManager): removed MintBurnFromAddress as deprecated * ci(Slither): action + fixes + comments * style(Solidity): prettier --------- Co-authored-by: Foivos <[email protected]> Co-authored-by: Milap Sheth <[email protected]>
axelarnetwork · Oct 9, 2023 · 6e29f74 · 6e29f74
1 parent 69cc47e
commit 6e29f74
Show file tree

Hide file tree

Showing 27 changed files with 167 additions and 121 deletions.
diff --git a/.github/workflows/slither.yml b/.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]
diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol
@@ -278,7 +278,9 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
     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);
     }
@@ -296,8 +298,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
     ) 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);
     }
 
     /**
@@ -320,8 +324,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
     ) 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);
     }
 
     /**
@@ -357,7 +363,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
      * @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
@@ -371,7 +377,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
         bytes memory distributor,
         bytes memory mintTo,
         uint256 mintAmount,
-        bytes memory operator,
+        bytes memory operator_,
         string calldata destinationChain,
         uint256 gasValue
     ) external payable notPaused {
@@ -384,7 +390,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
             distributor,
             mintTo,
             mintAmount,
-            operator,
+            operator_,
             destinationChain,
             gasValue
         );
@@ -486,11 +492,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
      * @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]);
         }
     }
@@ -592,15 +599,18 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
             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);
         }
     }
@@ -700,9 +710,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
         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);
     }
 
     /**
@@ -714,7 +725,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
      * @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
      */
@@ -726,34 +737,36 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
         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);
     }
 
     /**
@@ -763,13 +776,14 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
      * @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();
     }
 
     /**
@@ -800,9 +814,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
         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,
@@ -819,7 +836,6 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
         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) {
@@ -872,16 +888,21 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab
     ) 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);
     }
 }
diff --git a/contracts/interfaces/IDistributable.sol b/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

diff --git a/contracts/interfaces/IERC20BurnableFrom.sol b/contracts/interfaces/IERC20BurnableFrom.sol
@@ -6,14 +6,6 @@ pragma solidity ^0.8.0;
  * @dev Interface of the ERC20 standard as defined in the EIP.
  */
 interface IERC20BurnableFrom {
-    /**
-     * @notice Function to burn tokens from a burn deposit address
-     * @notice It is needed to support legacy Axelar Gateway tokens
-     * @dev Can only be called after token is transferred to a deposit address.
-     * @param salt The address that will have its tokens burnt
-     */
-    function burn(bytes32 salt) external;
-
     /**
      * @notice Function to burn tokens
      * @notice Requires the caller to have allowance for `amount` on `from`

diff --git a/contracts/interfaces/IImplementation.sol b/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;
 }
diff --git a/contracts/interfaces/IInterchainToken.sol b/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.

diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol
@@ -267,7 +267,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.

diff --git a/contracts/interfaces/IRemoteAddressValidator.sol b/contracts/interfaces/IRemoteAddressValidator.sol
@@ -43,8 +43,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);
 }
diff --git a/contracts/interfaces/IStandardizedToken.sol b/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_);
 }
diff --git a/contracts/interfaces/ITokenManagerLiquidityPool.sol b/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

diff --git a/contracts/interfaces/ITokenManagerLockUnlock.sol b/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);
 }