From 2ed153a3d87e8c86125edd19bdc3e48cbdee2b46 Mon Sep 17 00:00:00 2001 From: Foivos Date: Fri, 12 Apr 2024 12:38:01 +0300 Subject: [PATCH 1/2] feat: added additional checks for call contract with token (#249) * Added additional checks but code size is too large * Added value for contractCallWithToken and optimized contract size * trying to fix tests * Update the create3address of ITS to use a custom bytecodehash. * prettier * fix tests * Added a few tests * fixed one more test * fixed all tests * prettier * made lint happy * working on slither * made slither happy * prettier * Using constant for the hash as well * addressed comments * added some tests * added some coverage tests, found a bug too! * a small style fix * fixed a bug * addressed some comments * prettier * fixed a test * remove modifier that should not exist * rename a function * Update contracts/InterchainTokenService.sol Co-authored-by: Milap Sheth * reinteroduce the modifiers since they are needed after all * Update contracts/utils/Create3AddressFixed.sol * add a docstring * prettier and fixed tests * address comments * fix factory import --------- Co-authored-by: Milap Sheth Co-authored-by: Milap Sheth --- contracts/InterchainTokenFactory.sol | 6 +- contracts/InterchainTokenService.sol | 142 +++++-- .../interfaces/IInterchainTokenFactory.sol | 8 +- .../interfaces/IInterchainTokenService.sol | 1 + contracts/test/utils/TestCreate3Fixed.sol | 19 + contracts/utils/Create3AddressFixed.sol | 29 ++ contracts/utils/Create3Fixed.sol | 46 +++ contracts/utils/InterchainTokenDeployer.sol | 4 +- contracts/utils/TokenManagerDeployer.sol | 4 +- hardhat.config.js | 12 + test/InterchainTokenService.js | 380 ++++++++++++++++-- test/UtilsTest.js | 70 ++++ 12 files changed, 640 insertions(+), 81 deletions(-) create mode 100644 contracts/test/utils/TestCreate3Fixed.sol create mode 100644 contracts/utils/Create3AddressFixed.sol create mode 100644 contracts/utils/Create3Fixed.sol diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index 1d3209fc..ab85101e 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -55,10 +55,10 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M * @param chainNameHash_ The hash of the chain name. * @param deployer The address of the deployer. * @param salt A unique identifier to generate the salt. - * @return bytes32 The calculated salt for the interchain token. + * @return tokenSalt The calculated salt for the interchain token. */ - function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32) { - return keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt)); + function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32 tokenSalt) { + tokenSalt = keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt)); } /** diff --git a/contracts/InterchainTokenService.sol b/contracts/InterchainTokenService.sol index 923f1ad5..d62c73a9 100644 --- a/contracts/InterchainTokenService.sol +++ b/contracts/InterchainTokenService.sol @@ -7,7 +7,6 @@ import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contr import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; import { ExpressExecutorTracker } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/express/ExpressExecutorTracker.sol'; import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol'; -import { Create3Address } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3Address.sol'; import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol'; import { Multicall } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Multicall.sol'; import { Pausable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Pausable.sol'; @@ -22,6 +21,7 @@ import { IInterchainTokenExecutable } from './interfaces/IInterchainTokenExecuta 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'; @@ -37,7 +37,7 @@ contract InterchainTokenService is Operator, Pausable, Multicall, - Create3Address, + Create3AddressFixed, ExpressExecutorTracker, InterchainAddressTracker, IInterchainTokenService @@ -343,13 +343,7 @@ contract InterchainTokenService is string calldata sourceAddress, bytes calldata payload ) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) { - (uint256 messageType, bytes32 tokenId, , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, uint256)); - - if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) { - revert InvalidExpressMessageType(messageType); - } - - return (validTokenAddress(tokenId), amount); + return _contractCallValue(payload); } /** @@ -626,7 +620,12 @@ contract InterchainTokenService is * @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) public { + 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(); @@ -634,24 +633,46 @@ contract InterchainTokenService is _execute(commandId, sourceChain, sourceAddress, payload, payloadHash); } + /** + * @notice Returns the amount of token that this call is worth. + * @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address. + * @param sourceChain The source chain. + * @param sourceAddress The source address on the source chain. + * @param payload The payload sent with the call. + * @param symbol The symbol symbol for the call. + * @param amount The amount for the call. + * @return address The token address. + * @return uint256 The value the call is worth. + */ function contractCallWithTokenValue( - string calldata /*sourceChain*/, - string calldata /*sourceAddress*/, - bytes calldata /*payload*/, - string calldata /*symbol*/, - uint256 /*amount*/ - ) public view virtual returns (address, uint256) { - revert ExecuteWithTokenNotSupported(); + string calldata sourceChain, + string calldata sourceAddress, + bytes calldata payload, + string calldata symbol, + uint256 amount + ) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) { + _checkPayloadAgainstGatewayData(payload, symbol, amount); + return _contractCallValue(payload); } + /** + * @notice Express executes with a gateway token 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. + * @param tokenSymbol The symbol symbol for the call. + * @param amount The amount for the call. + */ function expressExecuteWithToken( bytes32 commandId, string calldata sourceChain, string calldata sourceAddress, bytes calldata payload, - string calldata /*tokenSymbol*/, - uint256 /*amount*/ + string calldata tokenSymbol, + uint256 amount ) external payable { + _checkPayloadAgainstGatewayData(payload, tokenSymbol, amount); // It should be ok to ignore the symbol and amount since this info exists on the payload. expressExecute(commandId, sourceChain, sourceAddress, payload); } @@ -663,13 +684,22 @@ contract InterchainTokenService is bytes calldata payload, string calldata tokenSymbol, uint256 amount - ) external { - bytes32 payloadHash = keccak256(payload); + ) external onlyRemoteService(sourceChain, sourceAddress) whenNotPaused { + _executeWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount); + } - if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount)) - revert NotApprovedByGateway(); + /** + * @notice Check that the tokenId from the payload is a token that is registered in the gateway with the proper tokenSymbol, with the right amount from the payload. + * Also check that the amount in the payload matches the one for the call. + * @param payload The payload for the call contract with token. + * @param tokenSymbol The tokenSymbol for the call contract with token. + * @param amount The amount for the call contract with token. + */ + function _checkPayloadAgainstGatewayData(bytes calldata payload, string calldata tokenSymbol, uint256 amount) internal view { + (, bytes32 tokenId, , , uint256 amountInPayload) = abi.decode(payload, (uint256, bytes32, uint256, uint256, uint256)); - _execute(commandId, sourceChain, sourceAddress, payload, payloadHash); + if (validTokenAddress(tokenId) != gateway.tokenAddresses(tokenSymbol) || amount != amountInPayload) + revert InvalidGatewayTokenTransfer(tokenId, payload, tokenSymbol, amount); } /** @@ -857,15 +887,10 @@ contract InterchainTokenService is string calldata sourceAddress, bytes calldata payload, bytes32 payloadHash - ) internal onlyRemoteService(sourceChain, sourceAddress) whenNotPaused { + ) internal { uint256 messageType = abi.decode(payload, (uint256)); if (messageType == MESSAGE_TYPE_INTERCHAIN_TRANSFER) { - address expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash); - - if (expressExecutor != address(0)) { - emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor); - } - + address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash); _processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload); } else if (messageType == MESSAGE_TYPE_DEPLOY_TOKEN_MANAGER) { _processDeployTokenManagerPayload(payload); @@ -876,6 +901,32 @@ contract InterchainTokenService is } } + function _executeWithToken( + bytes32 commandId, + string calldata sourceChain, + string calldata sourceAddress, + bytes calldata payload, + string calldata tokenSymbol, + uint256 amount + ) internal { + bytes32 payloadHash = keccak256(payload); + + if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount)) + revert NotApprovedByGateway(); + + uint256 messageType = abi.decode(payload, (uint256)); + if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) { + revert InvalidMessageType(messageType); + } + + _checkPayloadAgainstGatewayData(payload, tokenSymbol, amount); + + // slither-disable-next-line reentrancy-events + address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash); + + _processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload); + } + /** * @notice Deploys a token manager on a destination chain. * @param tokenId The ID of the token. @@ -1114,4 +1165,33 @@ contract InterchainTokenService is return (amount, tokenAddress); } + + /** + * @notice Returns the amount of token that this call is worth. + * @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address. + * @param payload The payload sent with the call. + * @return address The token address. + * @return uint256 The value the call is worth. + */ + function _contractCallValue(bytes calldata payload) internal view returns (address, uint256) { + (uint256 messageType, bytes32 tokenId, , , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, bytes, uint256)); + if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) { + revert InvalidExpressMessageType(messageType); + } + + return (validTokenAddress(tokenId), amount); + } + + function _getExpressExecutorAndEmitEvent( + bytes32 commandId, + string calldata sourceChain, + string calldata sourceAddress, + bytes32 payloadHash + ) internal returns (address expressExecutor) { + expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash); + + if (expressExecutor != address(0)) { + emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor); + } + } } diff --git a/contracts/interfaces/IInterchainTokenFactory.sol b/contracts/interfaces/IInterchainTokenFactory.sol index 7e55efbf..65301562 100644 --- a/contracts/interfaces/IInterchainTokenFactory.sol +++ b/contracts/interfaces/IInterchainTokenFactory.sol @@ -37,9 +37,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall { * @param chainNameHash_ The hash of the chain name. * @param deployer The address of the deployer. * @param salt A unique identifier to generate the salt. - * @return bytes32 The calculated salt for the interchain token. + * @return tokenSalt The calculated salt for the interchain token. */ - function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32); + function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32 tokenSalt); /** * @notice Computes the ID for an interchain token based on the deployer and a salt. @@ -97,9 +97,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall { * @notice Calculates the salt for a canonical interchain token. * @param chainNameHash_ The hash of the chain name. * @param tokenAddress The address of the token. - * @return salt The calculated salt for the interchain token. + * @return tokenSalt The calculated salt for the interchain token. */ - function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 salt); + function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 tokenSalt); /** * @notice Computes the ID for a canonical interchain token based on its address. diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index d069871e..c9fa9cc3 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -48,6 +48,7 @@ interface IInterchainTokenService is error EmptyData(); error PostDeployFailed(bytes data); error ZeroAmount(); + error InvalidGatewayTokenTransfer(bytes32 tokenId, bytes payload, string tokenSymbol, uint256 amount); event InterchainTransfer( bytes32 indexed tokenId, diff --git a/contracts/test/utils/TestCreate3Fixed.sol b/contracts/test/utils/TestCreate3Fixed.sol new file mode 100644 index 00000000..3b31656d --- /dev/null +++ b/contracts/test/utils/TestCreate3Fixed.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import { Create3Fixed } from '../../utils/Create3Fixed.sol'; + +contract TestCreate3Fixed is Create3Fixed { + event Deployed(address addr); + + function deploy(bytes memory code, bytes32 salt) public payable returns (address addr) { + addr = _create3(code, salt); + + emit Deployed(addr); + } + + function deployedAddress(bytes32 salt) public view returns (address addr) { + addr = _create3Address(salt); + } +} diff --git a/contracts/utils/Create3AddressFixed.sol b/contracts/utils/Create3AddressFixed.sol new file mode 100644 index 00000000..4dd4e81f --- /dev/null +++ b/contracts/utils/Create3AddressFixed.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +/** + * @title Create3AddressFixed contract + * @notice This contract can be used to predict the deterministic deployment address of a contract deployed with the `CREATE3` technique. + * It is equivalent to the Create3Address found in axelar-gmp-sdk-solidity repo but uses a fixed bytecode for CreateDeploy, + * which allows changing compilation options (like number of runs) without affecting the future deployment addresses. + */ +contract Create3AddressFixed { + // slither-disable-next-line too-many-digits + bytes internal constant CREATE_DEPLOY_BYTECODE = + hex'608060405234801561001057600080fd5b50610162806100206000396000f3fe60806040526004361061001d5760003560e01c806277436014610022575b600080fd5b61003561003036600461007b565b610037565b005b8051602082016000f061004957600080fd5b50565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b60006020828403121561008d57600080fd5b813567ffffffffffffffff808211156100a557600080fd5b818401915084601f8301126100b957600080fd5b8135818111156100cb576100cb61004c565b604051601f8201601f19908116603f011681019083821181831017156100f3576100f361004c565b8160405282815287602084870101111561010c57600080fd5b82602086016020830137600092810160200192909252509594505050505056fea264697066735822122094780ce55d28f1d568f4e0ab1b9dc230b96e952b73d2e06456fbff2289fa27f464736f6c63430008150033'; + bytes32 internal constant CREATE_DEPLOY_BYTECODE_HASH = keccak256(CREATE_DEPLOY_BYTECODE); + + /** + * @notice Compute the deployed address that will result from the `CREATE3` method. + * @param deploySalt A salt to influence the contract address + * @return deployed The deterministic contract address if it was deployed + */ + function _create3Address(bytes32 deploySalt) internal view returns (address deployed) { + address deployer = address( + uint160(uint256(keccak256(abi.encodePacked(hex'ff', address(this), deploySalt, CREATE_DEPLOY_BYTECODE_HASH)))) + ); + + deployed = address(uint160(uint256(keccak256(abi.encodePacked(hex'd6_94', deployer, hex'01'))))); + } +} diff --git a/contracts/utils/Create3Fixed.sol b/contracts/utils/Create3Fixed.sol new file mode 100644 index 00000000..3d13d946 --- /dev/null +++ b/contracts/utils/Create3Fixed.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import { IDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IDeploy.sol'; +import { ContractAddress } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/ContractAddress.sol'; +import { CreateDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/CreateDeploy.sol'; +import { Create3AddressFixed } from './Create3AddressFixed.sol'; + +/** + * @title Create3Fixed contract + * @notice This contract can be used to deploy a contract with a deterministic address that depends only on + * the deployer address and deployment salt, not the contract bytecode and constructor parameters. + * It uses a fixed bytecode to allow changing the compilation settings without affecting the deployment address in the future. + */ +contract Create3Fixed is Create3AddressFixed, IDeploy { + using ContractAddress for address; + + /** + * @notice Deploys a new contract using the `CREATE3` method. + * @dev This function first deploys the CreateDeploy contract using + * the `CREATE2` opcode and then utilizes the CreateDeploy to deploy the + * new contract with the `CREATE` opcode. + * @param bytecode The bytecode of the contract to be deployed + * @param deploySalt A salt to influence the contract address + * @return deployed The address of the deployed contract + */ + function _create3(bytes memory bytecode, bytes32 deploySalt) internal returns (address deployed) { + deployed = _create3Address(deploySalt); + + if (bytecode.length == 0) revert EmptyBytecode(); + if (deployed.isContract()) revert AlreadyDeployed(); + + // Deploy using create2 + CreateDeploy createDeploy; + bytes memory createDeployBytecode_ = CREATE_DEPLOY_BYTECODE; + uint256 length = createDeployBytecode_.length; + assembly { + createDeploy := create2(0, add(createDeployBytecode_, 0x20), length, deploySalt) + } + + if (address(createDeploy) == address(0)) revert DeployFailed(); + // Deploy using create + createDeploy.deploy(bytecode); + } +} diff --git a/contracts/utils/InterchainTokenDeployer.sol b/contracts/utils/InterchainTokenDeployer.sol index 1fabe688..f39eed8c 100644 --- a/contracts/utils/InterchainTokenDeployer.sol +++ b/contracts/utils/InterchainTokenDeployer.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol'; +import { Create3Fixed } from './Create3Fixed.sol'; import { IInterchainTokenDeployer } from '../interfaces/IInterchainTokenDeployer.sol'; import { IInterchainToken } from '../interfaces/IInterchainToken.sol'; @@ -11,7 +11,7 @@ import { IInterchainToken } from '../interfaces/IInterchainToken.sol'; * @title InterchainTokenDeployer * @notice This contract is used to deploy new instances of the InterchainTokenProxy contract. */ -contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3 { +contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3Fixed { address public immutable implementationAddress; /** diff --git a/contracts/utils/TokenManagerDeployer.sol b/contracts/utils/TokenManagerDeployer.sol index b46f6e26..cdc8349b 100644 --- a/contracts/utils/TokenManagerDeployer.sol +++ b/contracts/utils/TokenManagerDeployer.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol'; +import { Create3Fixed } from './Create3Fixed.sol'; import { ITokenManagerDeployer } from '../interfaces/ITokenManagerDeployer.sol'; @@ -12,7 +12,7 @@ import { TokenManagerProxy } from '../proxies/TokenManagerProxy.sol'; * @title TokenManagerDeployer * @notice This contract is used to deploy new instances of the TokenManagerProxy contract. */ -contract TokenManagerDeployer is ITokenManagerDeployer, Create3 { +contract TokenManagerDeployer is ITokenManagerDeployer, Create3Fixed { /** * @notice Deploys a new instance of the TokenManagerProxy contract * @param tokenId The unique identifier for the token diff --git a/hardhat.config.js b/hardhat.config.js index 3702a912..c02b040f 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -34,6 +34,16 @@ const compilerSettings = { optimizer: optimizerSettings, }, }; +const itsCompilerSettings = { + version: '0.8.21', + settings: { + evmVersion: process.env.EVM_VERSION || 'london', + optimizer: { + ...optimizerSettings, + runs: 600, // Reduce runs to keep bytecode size under limit + }, + }, +}; /** * @type import('hardhat/config').HardhatUserConfig @@ -47,6 +57,8 @@ module.exports = { : { 'contracts/proxies/Proxy.sol': compilerSettings, 'contracts/proxies/TokenManagerProxy.sol': compilerSettings, + 'contracts/InterchainTokenService.sol': itsCompilerSettings, + 'contracts/test/TestInterchainTokenService.sol': itsCompilerSettings, }, }, defaultNetwork: 'hardhat', diff --git a/test/InterchainTokenService.js b/test/InterchainTokenService.js index 868b7f0e..05e1be57 100644 --- a/test/InterchainTokenService.js +++ b/test/InterchainTokenService.js @@ -82,13 +82,7 @@ describe('Interchain Token Service', () => { return [token, tokenManager, tokenId]; }; - deployFunctions.gateway = async function deployNewLockUnlock( - tokenName, - tokenSymbol, - tokenDecimals, - mintAmount = 0, - skipApprove = false, - ) { + deployFunctions.gateway = async function deployNewGateway(tokenName, tokenSymbol, tokenDecimals, mintAmount = 0, skipApprove = false) { const salt = getRandomBytes32(); const tokenId = await service.interchainTokenId(wallet.address, salt); const tokenManager = await getContractAt('TokenManager', await service.tokenManagerAddress(tokenId), wallet); @@ -354,25 +348,22 @@ describe('Interchain Token Service', () => { }); it('Should revert on invalid token manager deployer', async () => { - await expectRevert( - (gasOptions) => - deployInterchainTokenService( - wallet, - create3Deployer.address, - AddressZero, - interchainTokenDeployer.address, - gateway.address, - gasService.address, - interchainTokenFactoryAddress, - tokenManager.address, - tokenHandler.address, - chainName, - [], - deploymentKey, - gasOptions, - ), - service, - 'ZeroAddress', + await expectRevert((gasOptions) => + deployInterchainTokenService( + wallet, + create3Deployer.address, + AddressZero, + interchainTokenDeployer.address, + gateway.address, + gasService.address, + interchainTokenFactoryAddress, + tokenManager.address, + tokenHandler.address, + chainName, + [], + deploymentKey, + gasOptions, + ), ); }); @@ -1028,7 +1019,6 @@ describe('Interchain Token Service', () => { const symbol = 'GTA'; const decimals = 18; const [token] = await deployFunctions.gateway(name, symbol, decimals); - console.log(await token.allowance(service.address, gateway.address)); const salt = getRandomBytes32(); const tokenId = await service.interchainTokenId(wallet.address, salt); @@ -1361,6 +1351,90 @@ describe('Interchain Token Service', () => { }); }); + describe('Execute with token checks', () => { + const sourceChain = 'source chain'; + let sourceAddress; + const amount = 1234; + let destAddress; + const tokenName = 'Token Name'; + const tokenSymbol = 'TS'; + const tokenDecimals = 16; + + before(async () => { + sourceAddress = service.address; + destAddress = wallet.address; + await deployFunctions.gateway(tokenName, tokenSymbol, tokenDecimals); + }); + + it('Should revert on execute with token if remote address validation fails', async () => { + const commandId = await approveContractCallWithMint( + gateway, + sourceChain, + wallet.address, + service.address, + '0x', + tokenSymbol, + amount, + ); + + await expectRevert( + (gasOptions) => service.executeWithToken(commandId, sourceChain, wallet.address, '0x', tokenSymbol, amount, gasOptions), + service, + 'NotRemoteService', + ); + }); + + it('Should revert on execute with token if the service is paused', async () => { + await service.setPauseStatus(true).then((tx) => tx.wait); + + const commandId = await approveContractCallWithMint( + gateway, + sourceChain, + sourceAddress, + service.address, + '0x', + tokenSymbol, + amount, + ); + + await expectRevert( + (gasOptions) => service.executeWithToken(commandId, sourceChain, sourceAddress, '0x', tokenSymbol, amount, gasOptions), + service, + 'Pause', + ); + + await service.setPauseStatus(false).then((tx) => tx.wait); + }); + + it('Should revert on execute with token with invalid messageType', async () => { + const symbol = 'TS3'; + const [token, , tokenId] = await deployFunctions.gateway('Name', symbol, 15, amount); + + await token.transfer(gateway.address, amount).then((tx) => tx.wait); + + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256'], + [MESSAGE_TYPE_DEPLOY_TOKEN_MANAGER, tokenId, sourceAddress, destAddress, amount], + ); + const commandId = await approveContractCallWithMint( + gateway, + sourceChain, + sourceAddress, + service.address, + payload, + symbol, + amount, + ); + + await expectRevert( + (gasOptions) => service.executeWithToken(commandId, sourceChain, sourceAddress, payload, symbol, amount, gasOptions), + service, + 'InvalidMessageType', + [MESSAGE_TYPE_DEPLOY_TOKEN_MANAGER], + ); + }); + }); + describe('Receive Remote Tokens', () => { let sourceAddress; const amount = 1234; @@ -2314,6 +2388,147 @@ describe('Interchain Token Service', () => { }); }); + describe('Express Execute With Token', () => { + const commandId = getRandomBytes32(); + const sourceAddress = '0x1234'; + const amount = 1234; + const destinationAddress = new Wallet(getRandomBytes32()).address; + const tokenName = 'name'; + const tokenSymbol = 'symbol'; + const tokenDecimals = 16; + const message = 'message'; + let data; + let tokenId; + let executable; + let invalidExecutable; + let token; + + before(async () => { + [token, , tokenId] = await deployFunctions.gateway(tokenName, tokenSymbol, tokenDecimals, amount * 2, true); + await token.approve(service.address, amount * 2).then((tx) => tx.wait); + data = defaultAbiCoder.encode(['address', 'string'], [destinationAddress, message]); + executable = await deployContract(wallet, 'TestInterchainExecutable', [service.address]); + invalidExecutable = await deployContract(wallet, 'TestInvalidInterchainExecutable', [service.address]); + }); + + it('Should revert on executeWithInterchainToken when not called by the service', async () => { + await expectRevert( + (gasOptions) => + executable.executeWithInterchainToken( + commandId, + sourceChain, + sourceAddress, + data, + tokenId, + token.address, + amount, + gasOptions, + ), + executable, + 'NotService', + [wallet.address], + ); + }); + + it('Should revert on expressExecuteWithInterchainToken when not called by the service', async () => { + await expectRevert( + (gasOptions) => + executable.expressExecuteWithInterchainToken( + commandId, + sourceChain, + sourceAddress, + data, + tokenId, + token.address, + amount, + gasOptions, + ), + executable, + 'NotService', + [wallet.address], + ); + }); + + it('Should revert on express execute with token when service is paused', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, hexlify(wallet.address), destinationAddress, amount, '0x'], + ); + + await service.setPauseStatus(true).then((tx) => tx.wait); + + await expectRevert( + (gasOptions) => + service.expressExecuteWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount, gasOptions), + service, + 'Pause', + ); + + await service.setPauseStatus(false).then((tx) => tx.wait); + }); + + it('Should express execute with token', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, hexlify(wallet.address), destinationAddress, amount, '0x'], + ); + await expect(service.expressExecuteWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount)) + .to.emit(service, 'ExpressExecuted') + .withArgs(commandId, sourceChain, sourceAddress, keccak256(payload), wallet.address) + .and.to.emit(token, 'Transfer') + .withArgs(wallet.address, destinationAddress, amount); + }); + + it('Should revert on express execute if token handler transfer token from fails', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', ' bytes'], + [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, sourceAddress, AddressZero, amount, data], + ); + + const errorSignatureHash = id('TokenTransferFailed()'); + const errorData = errorSignatureHash.substring(0, 10); + + await expectRevert( + (gasOptions) => + service.expressExecuteWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount, gasOptions), + service, + 'TokenHandlerFailed', + [errorData], + ); + }); + + it('Should revert on express execute with token if token transfer fails on destination chain', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', ' bytes'], + [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, sourceAddress, invalidExecutable.address, amount, data], + ); + + await expectRevert( + (gasOptions) => + service.expressExecuteWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount, gasOptions), + service, + 'ExpressExecuteWithInterchainTokenFailed', + [invalidExecutable.address], + ); + }); + + it('Should express execute with token', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', ' bytes'], + [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, sourceAddress, executable.address, amount, data], + ); + await expect(service.expressExecuteWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount)) + .to.emit(service, 'ExpressExecuted') + .withArgs(commandId, sourceChain, sourceAddress, keccak256(payload), wallet.address) + .and.to.emit(token, 'Transfer') + .withArgs(wallet.address, executable.address, amount) + .and.to.emit(token, 'Transfer') + .withArgs(executable.address, destinationAddress, amount) + .and.to.emit(executable, 'MessageReceived') + .withArgs(commandId, sourceChain, sourceAddress, destinationAddress, message, tokenId, amount); + }); + }); + describe('Express Receive Remote Token', () => { let sourceAddress; const amount = 1234; @@ -2770,6 +2985,7 @@ describe('Interchain Token Service', () => { describe('Call contract value', () => { const trustedAddress = 'Trusted address'; + const amount = 100; it('Should revert on contractCallValue if not called by remote service', async () => { const payload = '0x'; @@ -2800,8 +3016,10 @@ describe('Interchain Token Service', () => { it('Should revert on invalid express message type', async () => { const message = 10; const tokenId = HashZero; - const amount = 100; - const payload = defaultAbiCoder.encode(['uint256', 'bytes32', 'bytes', 'uint256'], [message, tokenId, '0x', amount]); + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message, tokenId, '0x', '0x', amount, '0x'], + ); await expectRevert( (gasOptions) => service.contractCallValue(sourceChain, trustedAddress, payload, gasOptions), @@ -2815,8 +3033,10 @@ describe('Interchain Token Service', () => { const mintAmount = 1234; const [token, , tokenId] = await deployFunctions.lockUnlock(`Test Token Lock Unlock`, 'TT', 12, mintAmount); const message = 0; - const amount = 100; - const payload = defaultAbiCoder.encode(['uint256', 'bytes32', 'bytes', 'uint256'], [message, tokenId, '0x', amount]); + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message, tokenId, '0x', '0x', amount, '0x'], + ); const [tokenAddress, returnedAmount] = await service.contractCallValue(sourceChain, trustedAddress, payload); @@ -2825,20 +3045,102 @@ describe('Interchain Token Service', () => { }); }); - describe('Unsupported functions', () => { - const sourceChain = 'Source chain'; - const sourceAddress = 'Source address'; - const payload = '0x'; - const symbol = 'ABC'; + describe('Call contract with token value', () => { + const trustedAddress = 'Trusted address with token'; + const name = 'Gateway Token'; + const symbol = 'GT'; + const decimals = 18; + const message = 0; const amount = 100; + let tokenId; + + before(async () => { + [, , tokenId] = await deployFunctions.gateway(name, symbol, decimals); + }); + + it('Should revert on contractCallWithTokenValue if not called by remote service', async () => { + const payload = '0x'; + + await expectRevert( + (gasOptions) => service.contractCallWithTokenValue(sourceChain, trustedAddress, payload, symbol, 0, gasOptions), + service, + 'NotRemoteService', + ); + }); + + it('Should revert on contractCallWithTokenValue if service is paused', async () => { + const payload = '0x'; + + await service.setTrustedAddress(sourceChain, trustedAddress).then((tx) => tx.wait); + + await service.setPauseStatus(true).then((tx) => tx.wait); + + await expectRevert( + (gasOptions) => service.contractCallWithTokenValue(sourceChain, trustedAddress, payload, symbol, 0, gasOptions), + service, + 'Pause', + ); + + await service.setPauseStatus(false).then((tx) => tx.wait); + }); - it('Should revert on contractCallWithTokenValue', async () => { + it('Should revert on invalid express message type', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message + 1, tokenId, '0x', '0x', amount, '0x'], + ); await expectRevert( - (gasOptions) => service.contractCallWithTokenValue(sourceChain, sourceAddress, payload, symbol, amount, gasOptions), + (gasOptions) => service.contractCallWithTokenValue(sourceChain, trustedAddress, payload, symbol, amount, gasOptions), service, - 'ExecuteWithTokenNotSupported', + 'InvalidExpressMessageType', + [message + 1], ); }); + + it('Should revert on token missmatch', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message, tokenId, '0x', '0x', amount, '0x'], + ); + await expectRevert( + (gasOptions) => + service.contractCallWithTokenValue(sourceChain, trustedAddress, payload, 'wrong symbol', amount, gasOptions), + service, + 'InvalidGatewayTokenTransfer', + [tokenId, payload, 'wrong symbol', amount], + ); + }); + + it('Should revert on amount missmatch', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message, tokenId, '0x', '0x', amount, '0x'], + ); + await expectRevert( + (gasOptions) => service.contractCallWithTokenValue(sourceChain, trustedAddress, payload, symbol, amount + 1, gasOptions), + service, + 'InvalidGatewayTokenTransfer', + [tokenId, payload, symbol, amount + 1], + ); + }); + + it('Should return correct token address and amount', async () => { + const payload = defaultAbiCoder.encode( + ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'], + [message, tokenId, '0x', '0x', amount, '0x'], + ); + + const [tokenAddress, returnedAmount] = await service.contractCallWithTokenValue( + sourceChain, + trustedAddress, + payload, + symbol, + amount, + ); + + expect(tokenAddress).to.eq(await service.validTokenAddress(tokenId)); + expect(returnedAmount).to.eq(amount); + }); }); describe('Bytecode checks [ @skip-on-coverage ]', () => { diff --git a/test/UtilsTest.js b/test/UtilsTest.js index f368431d..0b7c64c1 100644 --- a/test/UtilsTest.js +++ b/test/UtilsTest.js @@ -306,3 +306,73 @@ describe('InterchainTokenDeployer', () => { ); }); }); + +describe('Create3Deployer', () => { + let deployerWallet; + let userWallet; + let tokenFactory; + + let deployerFactory; + let deployer; + const name = 'test'; + const symbol = 'test'; + const decimals = 16; + + before(async () => { + [deployerWallet, userWallet] = await ethers.getSigners(); + + deployerFactory = await ethers.getContractFactory('TestCreate3Fixed', deployerWallet); + tokenFactory = await ethers.getContractFactory('TestMintableBurnableERC20', deployerWallet); + }); + + beforeEach(async () => { + deployer = await deployerFactory.deploy().then((d) => d.deployed()); + }); + + describe('deploy', () => { + it('should revert on deploy with empty bytecode', async () => { + const salt = getRandomBytes32(); + const bytecode = '0x'; + + await expect(deployer.connect(userWallet).deploy(bytecode, salt)).to.be.revertedWithCustomError(deployer, 'EmptyBytecode'); + }); + + it('should deploy to the predicted address', async () => { + const salt = getRandomBytes32(); + + const address = await deployer.deployedAddress(salt); + + const bytecode = tokenFactory.getDeployTransaction(name, symbol, decimals).data; + + await expect(deployer.deploy(bytecode, salt)).to.emit(deployer, 'Deployed').withArgs(address); + }); + + // TODO: Reintroduce this test if we know the address of the deployer. + /* if (isHardhat) { + it('should deploy to the predicted address with a know salt', async () => { + const salt = '0x4943fe1231449cc1baa660716a0cb38ff09af0b2c9acb63d40d9a7ba06d33d21'; + + const address = '0x03C2D7E8Fbcc46C62B3DCBB72121818334af2565'; + + const bytecode = ERC20Factory.getDeployTransaction(name, symbol, decimals).data; + + await expect(deployer.deploy(bytecode, salt)).to.emit(deployer, 'Deployed').withArgs(address); + }); + } */ + + it('should not forward native value', async () => { + const salt = getRandomBytes32(); + + const address = await deployer.deployedAddress(salt); + + const bytecode = tokenFactory.getDeployTransaction(name, symbol, decimals).data; + + await expect(deployer.deploy(bytecode, salt, { value: 10 })) + .to.emit(deployer, 'Deployed') + .withArgs(address); + + expect(await ethers.provider.getBalance(address)).to.equal(0); + expect(await ethers.provider.getBalance(deployer.address)).to.equal(10); + }); + }); +}); From ba744cac2fcc5d50cba13138951d358f196a33dd Mon Sep 17 00:00:00 2001 From: Foivos Date: Fri, 12 Apr 2024 13:25:41 +0300 Subject: [PATCH 2/2] 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), ]); });