refactor: inter-chain-token service events and errors [AXE-2064] (#125)

* refactor: contracts and interfaces for better error and event names, with additional args for faster debugging * fix: tests for changes in errors and events * refactor: test/utils.js for expecting custom args with revert if provided in funciton args * refactor: update index.md for changes in errors and events * chore: add operator address in event FlowLimitSet * chore: refactor error argument names * Added roles, tests pending * fix: tests * Added some tests and fixed constants * made lint happy * Chnaged roles constants to enum to make slither happy * Fixed tests * chore: also check error arguments in tests while reverting * chore: resolve pr comments for including tokenId in some errors * Added getter for distributor and opearator. * made lint happy * fixed a spelling error * chore: add tokenManager address in deployment event and update tests * chore: add deployed token address in event * chore: fix slither failing pipeline by adding comment * chore: resolve pr comments by adding params in erorrs and events, adding spaces * fix: test after merging branch feat/roles * fix: tests after resolving merge conflicts with main --------- Co-authored-by: Foivos <[email protected]> Co-authored-by: Dean Amiel <[email protected]>
axelarnetwork · Nov 1, 2023 · 9f89c14 · 9f89c14
1 parent b7bc804
commit 9f89c14
Show file tree

Hide file tree

Showing 23 changed files with 330 additions and 167 deletions.
diff --git a/contracts/executable/InterchainTokenExecutable.sol b/contracts/executable/InterchainTokenExecutable.sol
@@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
 import { IInterchainTokenExecutable } from '../interfaces/IInterchainTokenExecutable.sol';
 
 abstract contract InterchainTokenExecutable is IInterchainTokenExecutable {
-    error NotService();
+    error NotService(address caller);
 
     address public immutable interchainTokenService;
 
@@ -16,7 +16,7 @@ abstract contract InterchainTokenExecutable is IInterchainTokenExecutable {
     }
 
     modifier onlyService() {
-        if (msg.sender != interchainTokenService) revert NotService();
+        if (msg.sender != interchainTokenService) revert NotService(msg.sender);
         _;
     }
 

diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol
@@ -130,7 +130,9 @@ contract InterchainTokenService is
      * @param tokenId the `tokenId` of the TokenManager trying to perform the call.
      */
     modifier onlyTokenManager(bytes32 tokenId) {
-        if (msg.sender != getTokenManagerAddress(tokenId)) revert NotTokenManager();
+        address tokenManager = getTokenManagerAddress(tokenId);
+        if (msg.sender != tokenManager) revert NotTokenManager(msg.sender, tokenManager);
+
         _;
     }
 
@@ -279,10 +281,14 @@ contract InterchainTokenService is
      * @dev `gasValue` exists because this function can be part of a multicall involving multiple functions that could make remote contract calls.
      */
     function deployRemoteCanonicalToken(bytes32 tokenId, string calldata destinationChain, uint256 gasValue) public payable notPaused {
-        address tokenAddress = getValidTokenManagerAddress(tokenId);
-        tokenAddress = ITokenManager(tokenAddress).tokenAddress();
+        address tokenAddress;
+        {
+            tokenAddress = getValidTokenManagerAddress(tokenId);
+            tokenAddress = ITokenManager(tokenAddress).tokenAddress();
+            bytes32 canonicalTokenId = getCanonicalTokenId(tokenAddress);
 
-        if (getCanonicalTokenId(tokenAddress) != tokenId) revert NotCanonicalTokenManager();
+            if (canonicalTokenId != tokenId) revert InvalidCanonicalTokenId(canonicalTokenId);
+        }
 
         (string memory tokenName, string memory tokenSymbol, uint8 tokenDecimals) = _validateToken(tokenAddress);
         _deployRemoteStandardizedToken(tokenId, tokenName, tokenSymbol, tokenDecimals, '', '', 0, '', destinationChain, gasValue);
@@ -408,7 +414,7 @@ contract InterchainTokenService is
         (uint256 selector, bytes32 tokenId, , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, uint256));
 
         if (selector != SELECTOR_RECEIVE_TOKEN && selector != SELECTOR_RECEIVE_TOKEN_WITH_DATA) {
-            revert InvalidExpressSelector();
+            revert InvalidExpressSelector(selector);
         }
 
         ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
@@ -423,7 +429,7 @@ contract InterchainTokenService is
     ) external payable notPaused {
         uint256 selector = abi.decode(payload, (uint256));
         if (selector != SELECTOR_RECEIVE_TOKEN && selector != SELECTOR_RECEIVE_TOKEN_WITH_DATA) {
-            revert InvalidExpressSelector();
+            revert InvalidExpressSelector(selector);
         }
         if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted();
 
@@ -572,7 +578,8 @@ contract InterchainTokenService is
     ) internal pure returns (address implementation_) {
         implementation_ = tokenManagerImplementations[uint256(tokenManagerType)];
         if (implementation_ == address(0)) revert ZeroAddress();
-        if (ITokenManager(implementation_).implementationType() != uint256(tokenManagerType)) revert InvalidTokenManagerImplementation();
+        if (ITokenManager(implementation_).implementationType() != uint256(tokenManagerType))
+            revert InvalidTokenManagerImplementationType(implementation_);
     }
 
     /**
@@ -605,7 +612,7 @@ contract InterchainTokenService is
         if (selector == SELECTOR_DEPLOY_TOKEN_MANAGER) return _processDeployTokenManagerPayload(payload);
         if (selector == SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN) return _processDeployStandardizedTokenAndManagerPayload(payload);
 
-        revert SelectorUnknown();
+        revert SelectorUnknown(selector);
     }
 
     function contractCallWithTokenValue(
@@ -857,14 +864,19 @@ 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(
+        (bool success, bytes memory returnData) = tokenManagerDeployer.delegatecall(
             abi.encodeWithSelector(ITokenManagerDeployer.deployTokenManager.selector, tokenId, tokenManagerType, params)
         );
-        if (!success) revert TokenManagerDeploymentFailed();
+        if (!success) revert TokenManagerDeploymentFailed(returnData);
+
+        address tokenManager;
+        assembly {
+            tokenManager := mload(add(returnData, 0x20))
+        }
+
+        // slither-disable-next-line reentrancy-events
+        emit TokenManagerDeployed(tokenId, tokenManager, tokenManagerType, params);
     }
 
     /**
@@ -895,13 +907,11 @@ 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(
+        (bool success, bytes memory returnData) = standardizedTokenDeployer.delegatecall(
             abi.encodeWithSelector(
                 IStandardizedTokenDeployer.deployStandardizedToken.selector,
                 salt,
@@ -915,8 +925,16 @@ contract InterchainTokenService is
             )
         );
         if (!success) {
-            revert StandardizedTokenDeploymentFailed();
+            revert StandardizedTokenDeploymentFailed(returnData);
+        }
+
+        address tokenAddress;
+        assembly {
+            tokenAddress := mload(add(returnData, 0x20))
         }
+
+        // slither-disable-next-line reentrancy-events
+        emit StandardizedTokenDeployed(tokenId, tokenAddress, distributor, name, symbol, decimals, mintAmount, mintTo);
     }
 
     function _decodeMetadata(bytes memory metadata) internal pure returns (uint32 version, bytes memory data) {

diff --git a/contracts/interfaces/IFlowLimit.sol b/contracts/interfaces/IFlowLimit.sol
@@ -3,9 +3,9 @@
 pragma solidity ^0.8.0;
 
 interface IFlowLimit {
-    error FlowLimitExceeded();
+    error FlowLimitExceeded(uint256 limit, uint256 flowAmount);
 
-    event FlowLimitSet(uint256 flowLimit);
+    event FlowLimitSet(bytes32 indexed tokenId, address operator, uint256 flowLimit);
 
     /**
      * @notice Returns the current flow limit

diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol
@@ -13,21 +13,20 @@ import { IRemoteAddressValidator } from './IRemoteAddressValidator.sol';
 interface IInterchainTokenService is ITokenManagerType, IAxelarValuedExpressExecutable, IPausable, IMulticall, IContractIdentifier {
     error ZeroAddress();
     error LengthMismatch();
-    error InvalidTokenManagerImplementation();
+    error InvalidTokenManagerImplementationType(address implementation);
     error NotRemoteService();
     error TokenManagerDoesNotExist(bytes32 tokenId);
-    error NotTokenManager();
+    error NotTokenManager(address caller, address tokenManager);
     error ExecuteWithInterchainTokenFailed(address contractAddress);
+    error InvalidCanonicalTokenId(bytes32 expectedCanonicalTokenId);
     error ExpressExecuteWithInterchainTokenFailed(address contractAddress);
-    error NotCanonicalTokenManager();
     error GatewayToken();
-    error TokenManagerDeploymentFailed();
-    error StandardizedTokenDeploymentFailed();
-    error DoesNotAcceptExpressExecute(address contractAddress);
-    error SelectorUnknown();
+    error TokenManagerDeploymentFailed(bytes error);
+    error StandardizedTokenDeploymentFailed(bytes error);
+    error SelectorUnknown(uint256 selector);
     error InvalidMetadataVersion(uint32 version);
     error ExecuteWithTokenNotSupported();
-    error InvalidExpressSelector();
+    error InvalidExpressSelector(uint256 selector);
 
     event TokenSent(bytes32 indexed tokenId, string destinationChain, bytes destinationAddress, uint256 indexed amount);
     event TokenSentWithData(
@@ -71,9 +70,10 @@ interface IInterchainTokenService is ITokenManagerType, IAxelarValuedExpressExec
         string destinationChain,
         uint256 indexed gasValue
     );
-    event TokenManagerDeployed(bytes32 indexed tokenId, TokenManagerType indexed tokenManagerType, bytes params);
+    event TokenManagerDeployed(bytes32 indexed tokenId, address tokenManager, TokenManagerType indexed tokenManagerType, bytes params);
     event StandardizedTokenDeployed(
         bytes32 indexed tokenId,
+        address tokenAddress,
         address indexed distributor,
         string name,
         string symbol,

diff --git a/contracts/interfaces/IStandardizedTokenDeployer.sol b/contracts/interfaces/IStandardizedTokenDeployer.sol
@@ -31,6 +31,7 @@ interface IStandardizedTokenDeployer {
      * @param decimals Decimals of the token
      * @param mintAmount Amount of tokens to mint initially
      * @param mintTo Address to mint initial tokens to
+     * @return tokenAddress Address of the deployed token
      */
     function deployStandardizedToken(
         bytes32 salt,
@@ -41,5 +42,5 @@ interface IStandardizedTokenDeployer {
         uint8 decimals,
         uint256 mintAmount,
         address mintTo
-    ) external payable;
+    ) external payable returns (address tokenAddress);
 }
diff --git a/contracts/interfaces/ITokenManager.sol b/contracts/interfaces/ITokenManager.sol
@@ -13,10 +13,10 @@ import { IImplementation } from './IImplementation.sol';
  */
 interface ITokenManager is ITokenManagerType, IOperatable, IFlowLimit, IImplementation {
     error TokenLinkerZeroAddress();
-    error NotService();
+    error NotService(address caller);
     error TakeTokenFailed();
     error GiveTokenFailed();
-    error NotToken();
+    error NotToken(address caller);
     error ZeroAddress();
     error AlreadyFlowLimiter(address flowLimiter);
     error NotFlowLimiter(address flowLimiter);

diff --git a/contracts/interfaces/ITokenManagerDeployer.sol b/contracts/interfaces/ITokenManagerDeployer.sol
@@ -15,6 +15,11 @@ interface ITokenManagerDeployer {
      * @param tokenId The unique identifier for the token
      * @param implementationType Token manager implementation type
      * @param params Additional parameters used in the setup of the token manager
+     * @return tokenManager Address of the deployed tokenManager
      */
-    function deployTokenManager(bytes32 tokenId, uint256 implementationType, bytes calldata params) external payable;
+    function deployTokenManager(
+        bytes32 tokenId,
+        uint256 implementationType,
+        bytes calldata params
+    ) external payable returns (address tokenManager);
 }
diff --git a/contracts/interfaces/ITokenManagerProxy.sol b/contracts/interfaces/ITokenManagerProxy.sol
@@ -9,7 +9,7 @@ pragma solidity ^0.8.0;
  */
 interface ITokenManagerProxy {
     error ImplementationLookupFailed();
-    error SetupFailed();
+    error SetupFailed(bytes returnData);
     error NativeTokenNotAccepted();
 
     /**

diff --git a/contracts/proxies/TokenManagerProxy.sol b/contracts/proxies/TokenManagerProxy.sol
@@ -28,8 +28,8 @@ contract TokenManagerProxy is ITokenManagerProxy {
         tokenId = tokenId_;
         address impl = _getImplementation(IInterchainTokenService(interchainTokenServiceAddress_), implementationType_);
 
-        (bool success, ) = impl.delegatecall(abi.encodeWithSelector(TokenManagerProxy.setup.selector, params));
-        if (!success) revert SetupFailed();
+        (bool success, bytes memory returnData) = impl.delegatecall(abi.encodeWithSelector(TokenManagerProxy.setup.selector, params));
+        if (!success) revert SetupFailed(returnData);
     }
 
     /**

diff --git a/contracts/test/utils/FlowLimitTest.sol b/contracts/test/utils/FlowLimitTest.sol
@@ -5,8 +5,10 @@ pragma solidity ^0.8.0;
 import { FlowLimit } from '../../utils/FlowLimit.sol';
 
 contract FlowLimitTest is FlowLimit {
+    bytes32 public constant TOKEN_ID = 0x0;
+
     function setFlowLimit(uint256 flowLimit) external {
-        _setFlowLimit(flowLimit);
+        _setFlowLimit(flowLimit, TOKEN_ID);
     }
 
     function addFlowIn(uint256 flowInAmount) external {

diff --git a/contracts/test/utils/FlowLimitTestLiveNetwork.sol b/contracts/test/utils/FlowLimitTestLiveNetwork.sol
@@ -8,6 +8,7 @@ contract FlowLimitTestLiveNetwork is IFlowLimit {
     uint256 internal constant FLOW_LIMIT_SLOT = 0x201b7a0b7c19aaddc4ce9579b7df8d2db123805861bc7763627f13e04d8af42f;
     uint256 internal constant PREFIX_FLOW_OUT_AMOUNT = uint256(keccak256('flow-out-amount'));
     uint256 internal constant PREFIX_FLOW_IN_AMOUNT = uint256(keccak256('flow-in-amount'));
+    bytes32 public constant TOKEN_ID = 0x0;
 
     uint256 internal constant EPOCH_TIME = 60;
 
@@ -22,7 +23,7 @@ contract FlowLimitTestLiveNetwork is IFlowLimit {
             sstore(FLOW_LIMIT_SLOT, flowLimit)
         }
 
-        emit FlowLimitSet(flowLimit);
+        emit FlowLimitSet(TOKEN_ID, msg.sender, flowLimit);
     }
 
     function _getFlowOutSlot(uint256 epoch) internal pure returns (uint256 slot) {
@@ -60,8 +61,10 @@ contract FlowLimitTestLiveNetwork is IFlowLimit {
             flowToCompare := sload(slotToCompare)
         }
 
-        if (flowToAdd + flowAmount > flowToCompare + flowLimit) revert FlowLimitExceeded();
-        if (flowAmount > flowLimit) revert FlowLimitExceeded();
+        uint256 maxFlowLimit = flowToCompare + flowLimit;
+        uint256 netFlowAmount = flowToAdd + flowAmount;
+        if (netFlowAmount > maxFlowLimit) revert FlowLimitExceeded(maxFlowLimit, netFlowAmount);
+        if (flowAmount > flowLimit) revert FlowLimitExceeded(flowLimit, flowAmount);
 
         assembly {
             sstore(slotToAdd, add(flowToAdd, flowAmount))

diff --git a/contracts/token-manager/TokenManager.sol b/contracts/token-manager/TokenManager.sol
@@ -35,15 +35,15 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen
      * @dev A modifier that allows only the interchain token service to execute the function.
      */
     modifier onlyService() {
-        if (msg.sender != address(interchainTokenService)) revert NotService();
+        if (msg.sender != address(interchainTokenService)) revert NotService(msg.sender);
         _;
     }
 
     /**
      * @dev A modifier that allows only the token to execute the function.
      */
     modifier onlyToken() {
-        if (msg.sender != tokenAddress()) revert NotToken();
+        if (msg.sender != tokenAddress()) revert NotToken(msg.sender);
         _;
     }
 
@@ -224,7 +224,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen
      * @param flowLimit the maximum difference between the tokens flowing in and/or out at any given interval of time (6h)
      */
     function setFlowLimit(uint256 flowLimit) external onlyRole(uint8(Roles.FLOW_LIMITER)) {
-        _setFlowLimit(flowLimit);
+        _setFlowLimit(flowLimit, tokenId());
     }
 
     /**

diff --git a/contracts/utils/FlowLimit.sol b/contracts/utils/FlowLimit.sol
@@ -31,12 +31,12 @@ contract FlowLimit is IFlowLimit {
      * @dev Internal function to set the flow limit
      * @param flowLimit The value to set the flow limit to
      */
-    function _setFlowLimit(uint256 flowLimit) internal {
+    function _setFlowLimit(uint256 flowLimit, bytes32 tokenId) internal {
         assembly {
             sstore(FLOW_LIMIT_SLOT, flowLimit)
         }
 
-        emit FlowLimitSet(flowLimit);
+        emit FlowLimitSet(tokenId, msg.sender, flowLimit);
     }
 
     /**
@@ -99,8 +99,9 @@ contract FlowLimit is IFlowLimit {
             flowToCompare := sload(slotToCompare)
         }
 
-        if (flowToAdd + flowAmount > flowToCompare + flowLimit) revert FlowLimitExceeded();
-        if (flowAmount > flowLimit) revert FlowLimitExceeded();
+        if (flowToAdd + flowAmount > flowToCompare + flowLimit)
+            revert FlowLimitExceeded((flowToCompare + flowLimit), flowToAdd + flowAmount);
+        if (flowAmount > flowLimit) revert FlowLimitExceeded(flowLimit, flowAmount);
 
         assembly {
             sstore(slotToAdd, add(flowToAdd, flowAmount))

diff --git a/contracts/utils/Multicall.sol b/contracts/utils/Multicall.sol
@@ -28,7 +28,7 @@ contract Multicall is IMulticall {
             (success, result) = address(this).delegatecall(data[i]);
 
             if (!success) {
-                revert(string(result));
+                revert MulticallFailed(result);
             }
 
             results[i] = result;

diff --git a/contracts/utils/StandardizedTokenDeployer.sol b/contracts/utils/StandardizedTokenDeployer.sol
@@ -34,6 +34,7 @@ contract StandardizedTokenDeployer is IStandardizedTokenDeployer, Create3 {
      * @param decimals Decimals of the token
      * @param mintAmount Amount of tokens to mint initially
      * @param mintTo Address to mint initial tokens to
+     * @return tokenAddress Address of the deployed token
      */
     // slither-disable-next-line locked-ether
     function deployStandardizedToken(
@@ -45,12 +46,12 @@ contract StandardizedTokenDeployer is IStandardizedTokenDeployer, Create3 {
         uint8 decimals,
         uint256 mintAmount,
         address mintTo
-    ) external payable {
+    ) external payable returns (address tokenAddress) {
         bytes memory params = abi.encode(tokenManager, distributor, name, symbol, decimals, mintAmount, mintTo);
         // slither-disable-next-line too-many-digits
         bytes memory bytecode = bytes.concat(type(StandardizedTokenProxy).creationCode, abi.encode(implementationAddress, params));
 
-        address tokenAddress = _create3(bytecode, salt);
+        tokenAddress = _create3(bytecode, salt);
         if (tokenAddress.code.length == 0) revert TokenDeploymentFailed();
     }