From ba744cac2fcc5d50cba13138951d358f196a33dd Mon Sep 17 00:00:00 2001 From: Foivos Date: Fri, 12 Apr 2024 13:25:41 +0300 Subject: [PATCH] feat: move more to token handler (#258) * first commmit * second commit * lint and prettier * address comments --------- Co-authored-by: Milap Sheth --- contracts/InterchainTokenService.sol | 46 ++------ contracts/TokenHandler.sol | 101 +++++++++--------- .../interfaces/IInterchainTokenService.sol | 1 - contracts/interfaces/ITokenHandler.sol | 41 +++---- test/InterchainTokenService.js | 84 ++++++--------- 5 files changed, 102 insertions(+), 171 deletions(-) diff --git a/contracts/InterchainTokenService.sol b/contracts/InterchainTokenService.sol index d62c73a9..0743c818 100644 --- a/contracts/InterchainTokenService.sol +++ b/contracts/InterchainTokenService.sol @@ -13,14 +13,12 @@ import { Pausable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/util import { InterchainAddressTracker } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/InterchainAddressTracker.sol'; import { IInterchainTokenService } from './interfaces/IInterchainTokenService.sol'; -import { ITokenManagerProxy } from './interfaces/ITokenManagerProxy.sol'; import { ITokenHandler } from './interfaces/ITokenHandler.sol'; import { ITokenManagerDeployer } from './interfaces/ITokenManagerDeployer.sol'; import { IInterchainTokenDeployer } from './interfaces/IInterchainTokenDeployer.sol'; import { IInterchainTokenExecutable } from './interfaces/IInterchainTokenExecutable.sol'; 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'; @@ -390,21 +388,11 @@ contract InterchainTokenService is IERC20 token; { - ITokenManager tokenManager_ = ITokenManager(tokenManagerAddress(tokenId)); - token = IERC20(tokenManager_.tokenAddress()); - (bool success, bytes memory returnData) = tokenHandler.delegatecall( - abi.encodeWithSelector( - ITokenHandler.transferTokenFrom.selector, - tokenManager_.implementationType(), - address(token), - msg.sender, - destinationAddress, - amount - ) + abi.encodeWithSelector(ITokenHandler.transferTokenFrom.selector, tokenId, msg.sender, destinationAddress, amount) ); if (!success) revert TokenHandlerFailed(returnData); - amount = abi.decode(returnData, (uint256)); + (amount, token) = abi.decode(returnData, (uint256, IERC20)); } // slither-disable-next-line reentrancy-events @@ -1124,44 +1112,24 @@ contract InterchainTokenService is * @dev Takes token from a sender via the token service. `tokenOnly` indicates if the caller should be restricted to the token only. */ function _takeToken(bytes32 tokenId, address from, uint256 amount, bool tokenOnly) internal returns (uint256, string memory symbol) { - address tokenManager_ = tokenManagerAddress(tokenId); - uint256 tokenManagerType; - address tokenAddress; - - (tokenManagerType, tokenAddress) = ITokenManagerProxy(tokenManager_).getImplementationTypeAndTokenAddress(); - - if (tokenOnly && msg.sender != tokenAddress) revert NotToken(msg.sender, tokenAddress); - (bool success, bytes memory data) = tokenHandler.delegatecall( - abi.encodeWithSelector(ITokenHandler.takeToken.selector, tokenManagerType, tokenAddress, tokenManager_, from, amount) + abi.encodeWithSelector(ITokenHandler.takeToken.selector, tokenId, tokenOnly, from, amount) ); if (!success) revert TakeTokenFailed(data); - amount = abi.decode(data, (uint256)); + (amount, symbol) = abi.decode(data, (uint256, string)); - /// @dev Track the flow amount being sent out as a message - ITokenManager(tokenManager_).addFlowOut(amount); - if (tokenManagerType == uint256(TokenManagerType.GATEWAY)) { - symbol = IERC20Named(tokenAddress).symbol(); - } return (amount, symbol); } /** * @dev Gives token to recipient via the token service. */ - function _giveToken(bytes32 tokenId, address to, uint256 amount) internal returns (uint256, address) { - address tokenManager_ = tokenManagerAddress(tokenId); - - (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager_).getImplementationTypeAndTokenAddress(); - - /// @dev Track the flow amount being received via the message - ITokenManager(tokenManager_).addFlowIn(amount); - + function _giveToken(bytes32 tokenId, address to, uint256 amount) internal returns (uint256, address tokenAddress) { (bool success, bytes memory data) = tokenHandler.delegatecall( - abi.encodeWithSelector(ITokenHandler.giveToken.selector, tokenManagerType, tokenAddress, tokenManager_, to, amount) + abi.encodeWithSelector(ITokenHandler.giveToken.selector, tokenId, to, amount) ); if (!success) revert GiveTokenFailed(data); - amount = abi.decode(data, (uint256)); + (amount, tokenAddress) = abi.decode(data, (uint256, address)); return (amount, tokenAddress); } diff --git a/contracts/TokenHandler.sol b/contracts/TokenHandler.sol index 0a9c432b..35c5ff5f 100644 --- a/contracts/TokenHandler.sol +++ b/contracts/TokenHandler.sol @@ -6,17 +6,20 @@ 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'; import { ITokenManagerType } from './interfaces/ITokenManagerType.sol'; import { ITokenManager } from './interfaces/ITokenManager.sol'; +import { ITokenManagerProxy } from './interfaces/ITokenManagerProxy.sol'; import { IERC20MintableBurnable } from './interfaces/IERC20MintableBurnable.sol'; import { IERC20BurnableFrom } from './interfaces/IERC20BurnableFrom.sol'; +import { IERC20Named } from './interfaces/IERC20Named.sol'; /** * @title TokenHandler * @notice This interface is responsible for handling tokens before initiating an interchain token transfer, or after receiving one. */ -contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard { +contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Create3AddressFixed { using SafeTokenTransferFrom for IERC20; using SafeTokenCall for IERC20; using SafeTokenTransfer for IERC20; @@ -32,39 +35,39 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard { /** * @notice This function gives token to a specified address from the token manager. - * @param tokenManagerType The token manager type. - * @param tokenAddress The address of the token to give. - * @param tokenManager The address of the token manager. + * @param tokenId The token id of the tokenManager. * @param to The address to give tokens to. * @param amount The amount of tokens to give. * @return uint256 The amount of token actually given, which could be different for certain token type. + * @return address the address of the token. */ // slither-disable-next-line locked-ether - function giveToken( - uint256 tokenManagerType, - address tokenAddress, - address tokenManager, - address to, - uint256 amount - ) external payable returns (uint256) { + function giveToken(bytes32 tokenId, address to, uint256 amount) external payable returns (uint256, address) { + address tokenManager = _create3Address(tokenId); + + (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager).getImplementationTypeAndTokenAddress(); + + /// @dev Track the flow amount being received via the message + ITokenManager(tokenManager).addFlowIn(amount); + if (tokenManagerType == uint256(TokenManagerType.MINT_BURN) || tokenManagerType == uint256(TokenManagerType.MINT_BURN_FROM)) { _giveTokenMintBurn(tokenAddress, to, amount); - return amount; + return (amount, tokenAddress); } if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK)) { _transferTokenFrom(tokenAddress, tokenManager, to, amount); - return amount; + return (amount, tokenAddress); } if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { amount = _transferTokenFromWithFee(tokenAddress, tokenManager, to, amount); - return amount; + return (amount, tokenAddress); } if (tokenManagerType == uint256(TokenManagerType.GATEWAY)) { _transferToken(tokenAddress, to, amount); - return amount; + return (amount, tokenAddress); } revert UnsupportedTokenManagerType(tokenManagerType); @@ -72,66 +75,62 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard { /** * @notice This function takes token from a specified address to the token manager. - * @param tokenManagerType The token manager type. - * @param tokenAddress The address of the token to give. - * @param tokenManager The address of the token manager. + * @param tokenId The tokenId for the token. + * @param tokenOnly can only be called from the token. * @param from The address to take tokens from. * @param amount The amount of token to take. * @return uint256 The amount of token actually taken, which could be different for certain token type. + * @return symbol The symbol for the token, if not empty the token is a gateway token and a callContractWith token has to be made. */ // slither-disable-next-line locked-ether function takeToken( - uint256 tokenManagerType, - address tokenAddress, - address tokenManager, + bytes32 tokenId, + bool tokenOnly, address from, uint256 amount - ) external payable returns (uint256) { + ) external payable returns (uint256, string memory symbol) { + address tokenManager = _create3Address(tokenId); + + (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager).getImplementationTypeAndTokenAddress(); + + if (tokenOnly && msg.sender != tokenAddress) revert NotToken(msg.sender, tokenAddress); + if (tokenManagerType == uint256(TokenManagerType.MINT_BURN)) { _takeTokenMintBurn(tokenAddress, from, amount); - return amount; - } - - if (tokenManagerType == uint256(TokenManagerType.MINT_BURN_FROM)) { + } else if (tokenManagerType == uint256(TokenManagerType.MINT_BURN_FROM)) { _takeTokenMintBurnFrom(tokenAddress, from, amount); - return amount; - } - - if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK)) { + } else if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK)) { _transferTokenFrom(tokenAddress, from, tokenManager, amount); - return amount; - } - - if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { + } else if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { amount = _transferTokenFromWithFee(tokenAddress, from, tokenManager, amount); - return amount; - } - - if (tokenManagerType == uint256(TokenManagerType.GATEWAY)) { + } else if (tokenManagerType == uint256(TokenManagerType.GATEWAY)) { + symbol = IERC20Named(tokenAddress).symbol(); _transferTokenFrom(tokenAddress, from, address(this), amount); - return amount; + } else { + revert UnsupportedTokenManagerType(tokenManagerType); } - revert UnsupportedTokenManagerType(tokenManagerType); + /// @dev Track the flow amount being sent out as a message + ITokenManager(tokenManager).addFlowOut(amount); + + return (amount, symbol); } /** * @notice This function transfers token from and to a specified address. - * @param tokenManagerType The token manager type. - * @param tokenAddress the address of the token to give. + * @param tokenId The token id of the token manager. * @param from The address to transfer tokens from. * @param to The address to transfer tokens to. * @param amount The amount of token to transfer. * @return uint256 The amount of token actually transferred, which could be different for certain token type. + * @return address The address of the token corresponding to the input tokenId. */ // slither-disable-next-line locked-ether - function transferTokenFrom( - uint256 tokenManagerType, - address tokenAddress, - address from, - address to, - uint256 amount - ) external payable returns (uint256) { + function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external payable returns (uint256, address) { + address tokenManager = _create3Address(tokenId); + + (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager).getImplementationTypeAndTokenAddress(); + if ( tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK) || tokenManagerType == uint256(TokenManagerType.MINT_BURN) || @@ -139,12 +138,12 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard { tokenManagerType == uint256(TokenManagerType.GATEWAY) ) { _transferTokenFrom(tokenAddress, from, to, amount); - return amount; + return (amount, tokenAddress); } if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { amount = _transferTokenFromWithFee(tokenAddress, from, to, amount); - return amount; + return (amount, tokenAddress); } revert UnsupportedTokenManagerType(tokenManagerType); diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index c9fa9cc3..f9fc592d 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -32,7 +32,6 @@ interface IInterchainTokenService is error InvalidChainName(); error NotRemoteService(); error TokenManagerDoesNotExist(bytes32 tokenId); - error NotToken(address caller, address token); error ExecuteWithInterchainTokenFailed(address contractAddress); error ExpressExecuteWithInterchainTokenFailed(address contractAddress); error GatewayToken(); diff --git a/contracts/interfaces/ITokenHandler.sol b/contracts/interfaces/ITokenHandler.sol index b2167cf5..992079c7 100644 --- a/contracts/interfaces/ITokenHandler.sol +++ b/contracts/interfaces/ITokenHandler.sol @@ -9,6 +9,7 @@ pragma solidity ^0.8.0; interface ITokenHandler { error UnsupportedTokenManagerType(uint256 tokenManagerType); error AddressZero(); + error NotToken(address caller, address token); /** * @notice Returns the address of the axelar gateway on this chain. @@ -18,54 +19,42 @@ interface ITokenHandler { /** * @notice This function gives token to a specified address from the token manager. - * @param tokenManagerType The token manager type. - * @param tokenAddress The address of the token to give. - * @param tokenManager The address of the token manager. + * @param tokenId The token id of the tokenManager. * @param to The address to give tokens to. * @param amount The amount of tokens to give. * @return uint256 The amount of token actually given, which could be different for certain token type. + * @return address the address of the token. */ - function giveToken( - uint256 tokenManagerType, - address tokenAddress, - address tokenManager, - address to, - uint256 amount - ) external payable returns (uint256); + function giveToken(bytes32 tokenId, address to, uint256 amount) external payable returns (uint256, address); /** * @notice This function takes token from a specified address to the token manager. - * @param tokenManagerType The token manager type. - * @param tokenAddress The address of the token to give. - * @param tokenManager The address of the token manager. + * @param tokenId The tokenId for the token. + * @param tokenOnly can only be called from the token. * @param from The address to take tokens from. * @param amount The amount of token to take. * @return uint256 The amount of token actually taken, which could be different for certain token type. + * @return symbol The symbol for the token, if not empty the token is a gateway token and a callContractWith token has to be made. */ + // slither-disable-next-line locked-ether function takeToken( - uint256 tokenManagerType, - address tokenAddress, - address tokenManager, + bytes32 tokenId, + bool tokenOnly, address from, uint256 amount - ) external payable returns (uint256); + ) external payable returns (uint256, string memory symbol); /** * @notice This function transfers token from and to a specified address. - * @param tokenManagerType The token manager type. - * @param tokenAddress the address of the token to give. + * @param tokenId The token id of the token manager. * @param from The address to transfer tokens from. * @param to The address to transfer tokens to. * @param amount The amount of token to transfer. * @return uint256 The amount of token actually transferred, which could be different for certain token type. + * @return address The address of the token corresponding to the input tokenId. */ - function transferTokenFrom( - uint256 tokenManagerType, - address tokenAddress, - address from, - address to, - uint256 amount - ) external payable returns (uint256); + // slither-disable-next-line locked-ether + function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external payable returns (uint256, address); /** * @notice This function prepares a token manager after it is deployed diff --git a/test/InterchainTokenService.js b/test/InterchainTokenService.js index 05e1be57..f492b1d8 100644 --- a/test/InterchainTokenService.js +++ b/test/InterchainTokenService.js @@ -544,57 +544,19 @@ describe('Interchain Token Service', () => { }); describe('Token Handler', () => { - const tokenManagerType = 5; const amount = 1234; - it('Should revert on give token with unsupported token type', async () => { - await expectRevert( - (gasOptions) => - tokenHandler.giveToken( - tokenManagerType, - otherWallet.address, - otherWallet.address, - otherWallet.address, - amount, - gasOptions, - ), - tokenHandler, - 'UnsupportedTokenManagerType', - [tokenManagerType], - ); + it('Should revert on give token with non existing token id', async () => { + await expectRevert((gasOptions) => tokenHandler.giveToken(getRandomBytes32(), otherWallet.address, amount, gasOptions)); }); - it('Should revert on take token with unsupported token type', async () => { - await expectRevert( - (gasOptions) => - tokenHandler.takeToken( - tokenManagerType, - otherWallet.address, - otherWallet.address, - otherWallet.address, - amount, - gasOptions, - ), - tokenHandler, - 'UnsupportedTokenManagerType', - [tokenManagerType], - ); + it('Should revert on take token with non existing token id', async () => { + await expectRevert((gasOptions) => tokenHandler.takeToken(getRandomBytes32(), false, otherWallet.address, amount, gasOptions)); }); - it('Should revert on transfer token from with unsupported token type', async () => { - await expectRevert( - (gasOptions) => - tokenHandler.transferTokenFrom( - tokenManagerType, - otherWallet.address, - otherWallet.address, - otherWallet.address, - amount, - gasOptions, - ), - tokenHandler, - 'UnsupportedTokenManagerType', - [tokenManagerType], + it('Should revert on transfer token from non existing token id', async () => { + await expectRevert((gasOptions) => + tokenHandler.transferTokenFrom(getRandomBytes32(), otherWallet.address, otherWallet.address, amount, gasOptions), ); }); }); @@ -1289,6 +1251,10 @@ describe('Interchain Token Service', () => { }); it(`Should revert on transmit send token when not called by interchain token`, async () => { + const errorSignatureHash = id('NotToken(address,address)'); + const selector = errorSignatureHash.substring(0, 10); + const errorData = defaultAbiCoder.encode(['address', 'address'], [wallet.address, token.address]); + await expectRevert( (gasOptions) => service.transmitInterchainTransfer(tokenId, wallet.address, destinationChain, destAddress, amount, '0x', { @@ -1296,8 +1262,8 @@ describe('Interchain Token Service', () => { value: gasValue, }), service, - 'NotToken', - [wallet.address, token.address], + 'TakeTokenFailed', + [selector + errorData.substring(2)], ); }); }); @@ -2818,11 +2784,16 @@ describe('Interchain Token Service', () => { it('Should be able to send token only if it does not trigger the mint limit', async () => { await service.interchainTransfer(tokenId, destinationChain, destinationAddress, sendAmount, '0x', 0).then((tx) => tx.wait); + + const errorSignatureHash = id('FlowLimitExceeded(uint256,uint256,address)'); + const selector = errorSignatureHash.substring(0, 10); + const errorData = defaultAbiCoder.encode(['uint256', 'uint256', 'address'], [flowLimit, 2 * sendAmount, tokenManager.address]); + await expectRevert( (gasOptions) => service.interchainTransfer(tokenId, destinationChain, destinationAddress, sendAmount, '0x', 0, gasOptions), - tokenManager, - 'FlowLimitExceeded', - [flowLimit, 2 * sendAmount, tokenManager.address], + service, + 'TakeTokenFailed', + [selector + errorData.substring(2)], ); }); @@ -2854,10 +2825,15 @@ describe('Interchain Token Service', () => { expect(flowIn).to.eq(sendAmount); expect(flowOut).to.eq(sendAmount); - await expectRevert((gasOptions) => receiveToken(2 * sendAmount, gasOptions), tokenManager, 'FlowLimitExceeded', [ - (5 * sendAmount) / 2, - 3 * sendAmount, - tokenManager.address, + const errorSignatureHash = id('FlowLimitExceeded(uint256,uint256,address)'); + const selector = errorSignatureHash.substring(0, 10); + const errorData = defaultAbiCoder.encode( + ['uint256', 'uint256', 'address'], + [(5 * sendAmount) / 2, 3 * sendAmount, tokenManager.address], + ); + + await expectRevert((gasOptions) => receiveToken(2 * sendAmount, gasOptions), service, 'GiveTokenFailed', [ + selector + errorData.substring(2), ]); });