From 83ff4eefa2c011743981a1a537a87d758fda1d69 Mon Sep 17 00:00:00 2001 From: Blockchain Guy Date: Fri, 6 Oct 2023 21:37:10 +0530 Subject: [PATCH 1/6] chore: fixed spelling mistakes in design doc (#120) Co-authored-by: Ayush Tiwari --- DESIGN.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index f1ebc322..35bd761b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -21,19 +21,19 @@ Note that a lot of the design choises were made with supporting non-EVM chains i ### Canonical Bridges -Most current bridge designs aim to get a pre-existing, popular token to different chains that can benefit from the liquidity. When they do so the resulting token, called [`StandardizedToken`](./contracts/utils/StandardizedToken.sol) in this project, will only have basic functionality that enables users to transfer their token and use it with general use smart contracts like De-Fi applications. This is certainly powerfull, and has the benefit that as long as the pre-existing `ERC20` implemention and the bridge function properly everything run as expected. We wanted to include this design for the `InterchainTokenService` as well, so deployers can deploy a Canonical Bridge for any token they want, and this can be done only once per pre-existing `ERC20`. Who the deployer is does not matter for this, they just need to pay for the deployment gas, but they do not need to be trusted as they have no special powers over this kind of bridge +Most current bridge designs aim to get a pre-existing, popular token to different chains that can benefit from the liquidity. When they do so the resulting token, called [`StandardizedToken`](./contracts/utils/StandardizedToken.sol) in this project, will only have basic functionality that enables users to transfer their token and use it with general use smart contracts like De-Fi applications. This is certainly powerful, and has the benefit that as long as the pre-existing `ERC20` implementation and the bridge function properly everything run as expected. We wanted to include this design for the `InterchainTokenService` as well, so deployers can deploy a Canonical Bridge for any token they want, and this can be done only once per pre-existing `ERC20`. Who the deployer is does not matter for this, they just need to pay for the deployment gas, but they do not need to be trusted as they have no special powers over this kind of bridge ### Custom Bridges Most projects that look to go cross-chain nowadays have more complex needs that the ones covered by Canonical Bridges: they often need custom `ERC20` designs, and will sometimes want to have additional power over the bridge. This is where the `InterchainTokenService` shines, deployers can claim certain `tokenIds` only based on their `address`, and a `salt` they provide, and specify any kind of `TokenManager` to be deployed and either manage an external `ERC20` or a `StandardizedToken`. Users using Custom Bridges need to trust the deployers as they could easily confiscate the funds of users if they wanted to, same as any `ERC20` distributor could confiscate the funds of users. There are currently three kinds of possible `TokenManagers` available, but this number might increase in the future, as we find more potential uses for the `InterchainTokenService`. - Lock/Unlock: This `TokenManager` will simply transfer tokens from a user to itself or vice versa to initiate/fulfill cross-chain transfers -- Mint/Burn: This `TokenManager` will burn/mint tokens from/to the user to initiate/fulfill cross-chain transfers. Tokens used with this kind of `TokenManager` need to be properly permissioned to allow for this behaviour. -- Liquidity Pool: This `TokenManager` functions exaclty like a Lock/Unlock one, except the balance is kept at a separate, prespecified account. This allows for deployers to have more controll over the bridged funds. +- Mint/Burn: This `TokenManager` will burn/mint tokens from/to the user to initiate/fulfill cross-chain transfers. Tokens used with this kind of `TokenManager` need to be properly permissioned to allow for this behavior. +- Liquidity Pool: This `TokenManager` functions exactly like a Lock/Unlock one, except the balance is kept at a separate, pre-specified account. This allows for deployers to have more control over the bridged funds. ## Linker Router -We plan to finalize the design of the `InterchainTokenService` but we want to be able to support new chains as they get added to the Axelar Network. For this purpose, the service will ask a separate contract, the [`RemoteAddressValidator`](./contracts/remoteAddressValidator/RemoteAddressValidator.sol) to obtain the destination address for outgoing messages, and for validation of incoming messages. This contract might eventually stop being upgradable but it will probalby be able to support new addresses for new chains indefinately. +We plan to finalize the design of the `InterchainTokenService` but we want to be able to support new chains as they get added to the Axelar Network. For this purpose, the service will ask a separate contract, the [`RemoteAddressValidator`](./contracts/remoteAddressValidator/RemoteAddressValidator.sol) to obtain the destination address for outgoing messages, and for validation of incoming messages. This contract might eventually stop being upgradable but it will probably be able to support new addresses for new chains indefinitely. ## Interchain Token -We designed an [interface](./contracts/interfaces/IInterchainToken.sol) allong a [example implementation](./contracts/interchainToken/InterchainToken.sol) of an ERC20 that can use the `InterchainTokenService` internally. This has the main benefit that for `TokenManagers` that require user approval (Lock/Unlock and Liquidity Pool typically) the token can provide this approval within the same call, providing better UX for users, and saving them some gas. \ No newline at end of file +We designed an [interface](./contracts/interfaces/IInterchainToken.sol) along a [example implementation](./contracts/interchainToken/InterchainToken.sol) of an ERC20 that can use the `InterchainTokenService` internally. This has the main benefit that for `TokenManagers` that require user approval (Lock/Unlock and Liquidity Pool typically) the token can provide this approval within the same call, providing better UX for users, and saving them some gas. \ No newline at end of file From 111fed98dae66c70ff8a568832e947ccf1c0ecaf Mon Sep 17 00:00:00 2001 From: Foivos Date: Mon, 9 Oct 2023 11:50:37 +0300 Subject: [PATCH 2/6] feat: removed the default evm address from remote address validator (#112) * removed the default evm address from remote address validator * fixed a namign issue in tests --------- Co-authored-by: Milap Sheth --- .../interfaces/IRemoteAddressValidator.sol | 11 +-- .../RemoteAddressValidator.sol | 54 +--------- scripts/deploy.js | 10 +- test/RemoteAddressValidator.js | 48 +++------ test/tokenService.js | 98 ++++++++----------- test/tokenServiceFullFlow.js | 7 +- 6 files changed, 67 insertions(+), 161 deletions(-) diff --git a/contracts/interfaces/IRemoteAddressValidator.sol b/contracts/interfaces/IRemoteAddressValidator.sol index 93d6eda7..818a893b 100644 --- a/contracts/interfaces/IRemoteAddressValidator.sol +++ b/contracts/interfaces/IRemoteAddressValidator.sol @@ -10,6 +10,7 @@ interface IRemoteAddressValidator { error ZeroAddress(); error LengthMismatch(); error ZeroStringLength(); + error UntrustedChain(); event TrustedAddressAdded(string sourceChain, string sourceAddress); event TrustedAddressRemoved(string sourceChain); @@ -19,16 +20,6 @@ interface IRemoteAddressValidator { */ function chainName() external view returns (string memory); - /** - * @notice Returns the interchain token address - */ - function interchainTokenServiceAddress() external view returns (address); - - /** - * @notice Returns the interchain token address to string to lower case hash, which is used to compare with incoming calls. - */ - function interchainTokenServiceAddressHash() external view returns (bytes32); - /** * @dev Validates that the sender is a valid interchain token service address * @param sourceChain Source chain of the transaction diff --git a/contracts/remote-address-validator/RemoteAddressValidator.sol b/contracts/remote-address-validator/RemoteAddressValidator.sol index 89d7d40c..6caa0ec7 100644 --- a/contracts/remote-address-validator/RemoteAddressValidator.sol +++ b/contracts/remote-address-validator/RemoteAddressValidator.sol @@ -17,40 +17,13 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable { mapping(string => string) public remoteAddresses; string public chainName; - address public immutable interchainTokenServiceAddress; - bytes32 public immutable interchainTokenServiceAddressHash; - - /** - * @dev Store the interchain token service address as string across two immutable variables to avoid recomputation and save gas - */ - uint256 private immutable interchainTokenServiceAddress1; - uint256 private immutable interchainTokenServiceAddress2; - bytes32 private constant CONTRACT_ID = keccak256('remote-address-validator'); /** - * @dev Constructs the RemoteAddressValidator contract, both array parameters must be equal in length - * @param _interchainTokenServiceAddress Address of the interchain token service + * @dev Constructs the RemoteAddressValidator contract, both array parameters must be equal in length. + * @param chainName_ The name of the current chain. */ - constructor(address _interchainTokenServiceAddress, string memory chainName_) { - if (_interchainTokenServiceAddress == address(0)) revert ZeroAddress(); - - interchainTokenServiceAddress = _interchainTokenServiceAddress; - - string memory interchainTokenServiceAddressString = interchainTokenServiceAddress.toString(); - interchainTokenServiceAddressHash = keccak256(bytes(interchainTokenServiceAddressString)); - - uint256 p1; - uint256 p2; - - assembly { - p1 := mload(add(interchainTokenServiceAddressString, 32)) - p2 := mload(add(interchainTokenServiceAddressString, 64)) - } - - interchainTokenServiceAddress1 = p1; - interchainTokenServiceAddress2 = p2; - + constructor(string memory chainName_) { if (bytes(chainName_).length == 0) revert ZeroStringLength(); chainName = chainName_; } @@ -89,21 +62,6 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable { return s; } - /** - * @dev Return the interchain token service address as a string by constructing it from the two immutable variables caching it - */ - function _interchainTokenServiceAddressString() internal view returns (string memory interchainTokenServiceAddressString) { - interchainTokenServiceAddressString = new string(42); - - uint256 p1 = interchainTokenServiceAddress1; - uint256 p2 = interchainTokenServiceAddress2; - - assembly { - mstore(add(interchainTokenServiceAddressString, 32), p1) - mstore(add(interchainTokenServiceAddressString, 64), p2) - } - } - /** * @dev Validates that the sender is a valid interchain token service address * @param sourceChain Source chain of the transaction @@ -114,10 +72,6 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable { string memory sourceAddressNormalized = _lowerCase(sourceAddress); bytes32 sourceAddressHash = keccak256(bytes(sourceAddressNormalized)); - if (sourceAddressHash == interchainTokenServiceAddressHash) { - return true; - } - return sourceAddressHash == remoteAddressHashes[sourceChain]; } @@ -158,7 +112,7 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable { remoteAddress = remoteAddresses[chainName_]; if (bytes(remoteAddress).length == 0) { - remoteAddress = _interchainTokenServiceAddressString(); + revert UntrustedChain(); } } } diff --git a/scripts/deploy.js b/scripts/deploy.js index 87da2b26..38e7937a 100644 --- a/scripts/deploy.js +++ b/scripts/deploy.js @@ -11,9 +11,9 @@ async function deployContract(wallet, contractName, args = []) { return contract; } -async function deployRemoteAddressValidator(wallet, interchainTokenServiceAddress, chainName) { - const remoteAddressValidatorImpl = await deployContract(wallet, 'RemoteAddressValidator', [interchainTokenServiceAddress, chainName]); - const params = defaultAbiCoder.encode(['string[]', 'string[]'], [[], []]); +async function deployRemoteAddressValidator(wallet, chainName, interchainTokenServiceAddress = '', evmChains = []) { + const remoteAddressValidatorImpl = await deployContract(wallet, 'RemoteAddressValidator', [chainName]); + const params = defaultAbiCoder.encode(['string[]', 'string[]'], [evmChains, evmChains.map(() => interchainTokenServiceAddress)]); const remoteAddressValidatorProxy = await deployContract(wallet, 'RemoteAddressValidatorProxy', [ remoteAddressValidatorImpl.address, @@ -74,7 +74,7 @@ async function deployTokenManagerImplementations(wallet, interchainTokenServiceA return implementations; } -async function deployAll(wallet, chainName, deploymentKey = 'interchainTokenService') { +async function deployAll(wallet, chainName, evmChains = [], deploymentKey = 'interchainTokenService') { const create3Deployer = await deployContract(wallet, 'Create3Deployer'); const gateway = await deployMockGateway(wallet); const gasService = await deployGasService(wallet); @@ -82,7 +82,7 @@ async function deployAll(wallet, chainName, deploymentKey = 'interchainTokenServ const standardizedToken = await deployContract(wallet, 'StandardizedToken'); const standardizedTokenDeployer = await deployContract(wallet, 'StandardizedTokenDeployer', [standardizedToken.address]); const interchainTokenServiceAddress = await getCreate3Address(create3Deployer.address, wallet, deploymentKey); - const remoteAddressValidator = await deployRemoteAddressValidator(wallet, interchainTokenServiceAddress, chainName); + const remoteAddressValidator = await deployRemoteAddressValidator(wallet, chainName, interchainTokenServiceAddress, evmChains); const tokenManagerImplementations = await deployTokenManagerImplementations(wallet, interchainTokenServiceAddress); const service = await deployInterchainTokenService( diff --git a/test/RemoteAddressValidator.js b/test/RemoteAddressValidator.js index 6415bc77..c787d5c6 100644 --- a/test/RemoteAddressValidator.js +++ b/test/RemoteAddressValidator.js @@ -4,8 +4,7 @@ require('dotenv').config(); const chai = require('chai'); const { ethers } = require('hardhat'); const { - constants: { AddressZero }, - utils: { defaultAbiCoder, keccak256, toUtf8Bytes }, + utils: { defaultAbiCoder }, } = ethers; const { expect } = chai; const { deployRemoteAddressValidator, deployContract } = require('../scripts/deploy'); @@ -21,30 +20,16 @@ describe('RemoteAddressValidator', () => { ownerWallet = wallets[0]; otherWallet = wallets[1]; interchainTokenServiceAddress = wallets[2].address; - remoteAddressValidator = await deployRemoteAddressValidator(ownerWallet, interchainTokenServiceAddress, chainName); - }); - - it('Should revert on RemoteAddressValidator deployment with invalid interchain token service address', async () => { - const remoteAddressValidatorFactory = await ethers.getContractFactory('RemoteAddressValidator'); - await expect(remoteAddressValidatorFactory.deploy(AddressZero, chainName)).to.be.revertedWithCustomError( - remoteAddressValidator, - 'ZeroAddress', - ); + remoteAddressValidator = await deployRemoteAddressValidator(ownerWallet, chainName); }); it('Should revert on RemoteAddressValidator deployment with invalid chain name', async () => { const remoteAddressValidatorFactory = await ethers.getContractFactory('RemoteAddressValidator'); - await expect(remoteAddressValidatorFactory.deploy(interchainTokenServiceAddress, '')).to.be.revertedWithCustomError( - remoteAddressValidator, - 'ZeroStringLength', - ); + await expect(remoteAddressValidatorFactory.deploy('')).to.be.revertedWithCustomError(remoteAddressValidator, 'ZeroStringLength'); }); it('Should revert on RemoteAddressValidator deployment with length mismatch between chains and trusted addresses arrays', async () => { - const remoteAddressValidatorImpl = await deployContract(ownerWallet, 'RemoteAddressValidator', [ - interchainTokenServiceAddress, - chainName, - ]); + const remoteAddressValidatorImpl = await deployContract(ownerWallet, 'RemoteAddressValidator', [chainName]); const remoteAddressValidatorProxyFactory = await ethers.getContractFactory('RemoteAddressValidatorProxy'); const params = defaultAbiCoder.encode(['string[]', 'string[]'], [['Chain A'], []]); await expect( @@ -52,22 +37,16 @@ describe('RemoteAddressValidator', () => { ).to.be.revertedWithCustomError(remoteAddressValidator, 'SetupFailed'); }); - it('Should get the correct remote address for unregistered chains', async () => { - const remoteAddress = await remoteAddressValidator.getRemoteAddress(otherChain); - expect(remoteAddress).to.equal(interchainTokenServiceAddress.toLowerCase()); - }); - - it('Should get the correct interchain token service address', async () => { - const itsAddress = await remoteAddressValidator.interchainTokenServiceAddress(); - expect(itsAddress).to.equal(interchainTokenServiceAddress); - - const itsAddressHash = await remoteAddressValidator.interchainTokenServiceAddressHash(); - expect(itsAddressHash).to.equal(keccak256(toUtf8Bytes(interchainTokenServiceAddress.toLowerCase()))); + it('Should revert when querrying the remote address for unregistered chains', async () => { + await expect(remoteAddressValidator.getRemoteAddress(otherChain)).to.be.revertedWithCustomError( + remoteAddressValidator, + 'UntrustedChain', + ); }); it('Should be able to validate remote addresses properly', async () => { expect(await remoteAddressValidator.validateSender(otherChain, otherRemoteAddress)).to.equal(false); - expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(true); + expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(false); }); it('Should not be able to add a custom remote address as not the owner', async () => { @@ -112,7 +91,10 @@ describe('RemoteAddressValidator', () => { await expect(remoteAddressValidator.removeTrustedAddress(otherChain)) .to.emit(remoteAddressValidator, 'TrustedAddressRemoved') .withArgs(otherChain); - expect(await remoteAddressValidator.getRemoteAddress(otherChain)).to.equal(interchainTokenServiceAddress.toLowerCase()); + await expect(remoteAddressValidator.getRemoteAddress(otherChain)).to.be.revertedWithCustomError( + remoteAddressValidator, + 'UntrustedChain', + ); }); it('Should revert on removing a custom remote address with an empty chain name', async () => { @@ -124,6 +106,6 @@ describe('RemoteAddressValidator', () => { it('Should be able to validate remote addresses properly.', async () => { expect(await remoteAddressValidator.validateSender(otherChain, otherRemoteAddress)).to.equal(false); - expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(true); + expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(false); }); }); diff --git a/test/tokenService.js b/test/tokenService.js index f2e2a39c..93b0bca9 100644 --- a/test/tokenService.js +++ b/test/tokenService.js @@ -30,6 +30,8 @@ describe('Interchain Token Service', () => { let service, gateway, gasService; const deployFunctions = {}; + const destinationChain = 'destination chain'; + const sourceChain = 'source chain'; deployFunctions.lockUnlock = async function deployNewLockUnlock( tokenName, @@ -132,7 +134,7 @@ describe('Interchain Token Service', () => { const wallets = await ethers.getSigners(); wallet = wallets[0]; liquidityPool = wallets[1]; - [service, gateway, gasService] = await deployAll(wallet, 'Test'); + [service, gateway, gasService] = await deployAll(wallet, 'Test', [sourceChain, destinationChain]); }); describe('Register Canonical Token', () => { @@ -211,19 +213,18 @@ describe('Interchain Token Service', () => { }); it('Should be able to initiate a remote standardized token deployment', async () => { - const chain = 'chain1'; const gasValue = 1e6; const payload = defaultAbiCoder.encode( ['uint256', 'bytes32', 'string', 'string', 'uint8', 'bytes', 'bytes', 'uint256', 'bytes'], [SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN, tokenId, tokenName, tokenSymbol, tokenDecimals, '0x', '0x', 0, '0x'], ); - await expect(service.deployRemoteCanonicalToken(tokenId, chain, gasValue, { value: gasValue })) + await expect(service.deployRemoteCanonicalToken(tokenId, destinationChain, gasValue, { value: gasValue })) .to.emit(service, 'RemoteStandardizedTokenAndManagerDeploymentInitialized') - .withArgs(tokenId, tokenName, tokenSymbol, tokenDecimals, '0x', '0x', 0, '0x', chain, gasValue) + .withArgs(tokenId, tokenName, tokenSymbol, tokenDecimals, '0x', '0x', 0, '0x', destinationChain, gasValue) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), payload); + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), payload); }); // it('Should revert if token manager for given token has not be deployed', async () => {}); @@ -246,12 +247,10 @@ describe('Interchain Token Service', () => { const tokenManagerAddress = await service.getValidTokenManagerAddress(tokenId); expect(tokenManagerAddress).to.not.equal(AddressZero); - const chain = 'chain1'; const gasValue = 1e6; - await expect(service.deployRemoteCanonicalToken(tokenId, chain, gasValue, { value: gasValue })).to.be.revertedWithCustomError( - service, - 'NotCanonicalTokenManager', - ); + await expect( + service.deployRemoteCanonicalToken(tokenId, destinationChain, gasValue, { value: gasValue }), + ).to.be.revertedWithCustomError(service, 'NotCanonicalTokenManager'); }); it('Should revert on remote standardized token deployment if paused', async () => { @@ -260,12 +259,10 @@ describe('Interchain Token Service', () => { const salt = getRandomBytes32(); const tokenId = await service.getCustomTokenId(wallet.address, salt); - const chain = 'chain1'; const gasValue = 1e6; - await expect(service.deployRemoteCanonicalToken(tokenId, chain, gasValue, { value: gasValue })).to.be.revertedWithCustomError( - service, - 'Paused', - ); + await expect( + service.deployRemoteCanonicalToken(tokenId, destinationChain, gasValue, { value: gasValue }), + ).to.be.revertedWithCustomError(service, 'Paused'); tx = await service.setPaused(false); await tx.wait(); @@ -329,7 +326,6 @@ describe('Interchain Token Service', () => { const tokenSymbol = 'TN'; const tokenDecimals = 13; const distributor = '0x12345678'; - const destinationChain = 'dest'; const mintTo = '0x0abc'; const mintAmount = 123456; const operator = '0x5678'; @@ -418,7 +414,6 @@ describe('Interchain Token Service', () => { }); describe('Receive Remote Standardized Token Deployment', () => { - const sourceChain = 'source chain'; const tokenName = 'Token Name'; const tokenSymbol = 'TN'; const tokenDecimals = 13; @@ -685,7 +680,6 @@ describe('Interchain Token Service', () => { it('Should initialize a remote custom token manager deployment', async () => { const salt = getRandomBytes32(); const tokenId = await service.getCustomTokenId(wallet.address, salt); - const chain = 'chain1'; const gasValue = 1e6; const params = '0x1234'; const type = LOCK_UNLOCK; @@ -694,13 +688,13 @@ describe('Interchain Token Service', () => { [SELECTOR_DEPLOY_TOKEN_MANAGER, tokenId, type, params], ); - await expect(service.deployRemoteCustomTokenManager(salt, chain, type, params, gasValue, { value: gasValue })) + await expect(service.deployRemoteCustomTokenManager(salt, destinationChain, type, params, gasValue, { value: gasValue })) .to.emit(service, 'RemoteTokenManagerDeploymentInitialized') - .withArgs(tokenId, chain, gasValue, type, params) + .withArgs(tokenId, destinationChain, gasValue, type, params) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), payload); + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), payload); }); it('Should revert on remote custom token manager deployment if paused', async () => { @@ -708,13 +702,12 @@ describe('Interchain Token Service', () => { await tx.wait(); const salt = getRandomBytes32(); - const chain = 'chain1'; const gasValue = 1e6; const params = '0x1234'; const type = LOCK_UNLOCK; await expect( - service.deployRemoteCustomTokenManager(salt, chain, type, params, gasValue, { value: gasValue }), + service.deployRemoteCustomTokenManager(salt, destinationChain, type, params, gasValue, { value: gasValue }), ).to.be.revertedWithCustomError(service, 'Paused'); tx = await service.setPaused(false); await tx.wait(); @@ -732,7 +725,6 @@ describe('Interchain Token Service', () => { it('Should initialize a remote custom token manager deployment', async () => { const salt = getRandomBytes32(); const tokenId = await service.getCustomTokenId(wallet.address, salt); - const chain = 'chain1'; const gasValue = 1e6; const params = '0x1234'; const type = LOCK_UNLOCK; @@ -741,13 +733,13 @@ describe('Interchain Token Service', () => { [SELECTOR_DEPLOY_TOKEN_MANAGER, tokenId, type, params], ); - await expect(service.deployRemoteCustomTokenManager(salt, chain, type, params, gasValue, { value: gasValue })) + await expect(service.deployRemoteCustomTokenManager(salt, destinationChain, type, params, gasValue, { value: gasValue })) .to.emit(service, 'RemoteTokenManagerDeploymentInitialized') - .withArgs(tokenId, chain, gasValue, type, params) + .withArgs(tokenId, destinationChain, gasValue, type, params) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), gasValue, wallet.address) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, chain, service.address.toLowerCase(), keccak256(payload), payload); + .withArgs(service.address, destinationChain, service.address.toLowerCase(), keccak256(payload), payload); }); it('Should revert on remote custom token manager deployment if paused', async () => { @@ -755,13 +747,12 @@ describe('Interchain Token Service', () => { await tx.wait(); const salt = getRandomBytes32(); - const chain = 'chain1'; const gasValue = 1e6; const params = '0x1234'; const type = LOCK_UNLOCK; await expect( - service.deployRemoteCustomTokenManager(salt, chain, type, params, gasValue, { value: gasValue }), + service.deployRemoteCustomTokenManager(salt, destinationChain, type, params, gasValue, { value: gasValue }), ).to.be.revertedWithCustomError(service, 'Paused'); tx = await service.setPaused(false); await tx.wait(); @@ -769,7 +760,6 @@ describe('Interchain Token Service', () => { }); describe('Receive Remote Token Manager Deployment', () => { - const sourceChain = 'source chain'; let sourceAddress; before(async () => { @@ -848,7 +838,6 @@ describe('Interchain Token Service', () => { describe('Send Token', () => { const amount = 1234; - const destChain = 'destination Chain'; const destAddress = '0x5678'; const gasValue = 90; @@ -870,21 +859,20 @@ describe('Interchain Token Service', () => { transferToAddress = liquidityPool.address; } - await expect(tokenManager.interchainTransfer(destChain, destAddress, amount, '0x', { value: gasValue })) + await expect(tokenManager.interchainTransfer(destinationChain, destAddress, amount, '0x', { value: gasValue })) .and.to.emit(token, 'Transfer') .withArgs(wallet.address, transferToAddress, amount) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, payload) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, payload) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) .to.emit(service, 'TokenSent') - .withArgs(tokenId, destChain, destAddress, sendAmount); + .withArgs(tokenId, destinationChain, destAddress, sendAmount); }); } }); describe('Receive Remote Tokens', () => { - const sourceChain = 'source chain'; let sourceAddress; const amount = 1234; let destAddress; @@ -962,7 +950,6 @@ describe('Interchain Token Service', () => { describe('Send Token With Data', () => { const amount = 1234; - const destChain = 'destination Chain'; const destAddress = '0x5678'; const gasValue = 90; let sourceAddress; @@ -990,21 +977,20 @@ describe('Interchain Token Service', () => { transferToAddress = liquidityPool.address; } - await expect(tokenManager.callContractWithInterchainToken(destChain, destAddress, amount, data, { value: gasValue })) + await expect(tokenManager.callContractWithInterchainToken(destinationChain, destAddress, amount, data, { value: gasValue })) .and.to.emit(token, 'Transfer') .withArgs(wallet.address, transferToAddress, amount) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, payload) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, payload) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) .to.emit(service, 'TokenSentWithData') - .withArgs(tokenId, destChain, destAddress, sendAmount, sourceAddress, data); + .withArgs(tokenId, destinationChain, destAddress, sendAmount, sourceAddress, data); }); } }); describe('Receive Remote Tokens with Data', () => { - const sourceChain = 'source chain'; let sourceAddress; const sourceAddressForService = '0x1234'; const amount = 1234; @@ -1116,7 +1102,6 @@ describe('Interchain Token Service', () => { describe('Send Interchain Token', () => { const amount = 1234; - const destChain = 'destination Chain'; const destAddress = '0x5678'; const gasValue = 90; const metadata = '0x'; @@ -1139,22 +1124,21 @@ describe('Interchain Token Service', () => { transferToAddress = liquidityPool.address; } - await expect(token.interchainTransfer(destChain, destAddress, amount, metadata, { value: gasValue })) + await expect(token.interchainTransfer(destinationChain, destAddress, amount, metadata, { value: gasValue })) .and.to.emit(token, 'Transfer') .withArgs(wallet.address, transferToAddress, amount) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, payload) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, payload) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) .to.emit(service, 'TokenSent') - .withArgs(tokenId, destChain, destAddress, sendAmount); + .withArgs(tokenId, destinationChain, destAddress, sendAmount); }); } }); describe('Send Interchain Token With Data', () => { const amount = 1234; - const destChain = 'destination Chain'; const destAddress = '0x5678'; const gasValue = 90; let sourceAddress; @@ -1184,22 +1168,21 @@ describe('Interchain Token Service', () => { } const metadata = solidityPack(['uint32', 'bytes'], [0, data]); - await expect(token.interchainTransfer(destChain, destAddress, amount, metadata, { value: gasValue })) + await expect(token.interchainTransfer(destinationChain, destAddress, amount, metadata, { value: gasValue })) .and.to.emit(token, 'Transfer') .withArgs(wallet.address, transferToAddress, amount) .and.to.emit(gateway, 'ContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, payload) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, payload) .and.to.emit(gasService, 'NativeGasPaidForContractCall') - .withArgs(service.address, destChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) + .withArgs(service.address, destinationChain, service.address.toLowerCase(), payloadHash, gasValue, wallet.address) .to.emit(service, 'TokenSentWithData') - .withArgs(tokenId, destChain, destAddress, sendAmount, sourceAddress, data); + .withArgs(tokenId, destinationChain, destAddress, sendAmount, sourceAddress, data); }); } }); describe('Express Execute', () => { const commandId = getRandomBytes32(); - const sourceChain = 'source chain'; const sourceAddress = '0x1234'; const amount = 1234; const destinationAddress = new Wallet(getRandomBytes32()).address; @@ -1249,7 +1232,6 @@ describe('Interchain Token Service', () => { }); describe('Express Receive Remote Tokens', () => { - const sourceChain = 'source chain'; let sourceAddress; const amount = 1234; const destAddress = new Wallet(getRandomBytes32()).address; @@ -1339,7 +1321,6 @@ describe('Interchain Token Service', () => { }); describe('Express Receive Remote Tokens with Data', () => { - const sourceChain = 'source chain'; let sourceAddress; const sourceAddressForService = '0x1234'; const amount = 1234; @@ -1443,7 +1424,6 @@ describe('Interchain Token Service', () => { }); describe('Flow Limits', () => { - const destinationChain = 'dest'; const destinationAddress = '0x1234'; let tokenManager, tokenId; const sendAmount = 1234; diff --git a/test/tokenServiceFullFlow.js b/test/tokenServiceFullFlow.js index 1fa94921..2170c04c 100644 --- a/test/tokenServiceFullFlow.js +++ b/test/tokenServiceFullFlow.js @@ -26,22 +26,22 @@ const LOCK_UNLOCK = 2; // const LOCK_UNLOCK_FEE_ON_TRANSFER = 3; // const LIQUIDITY_POOL = 4; -describe('Interchain Token Service Flow', () => { +describe('Interchain Token Service Full Flow', () => { let wallet; let service, gateway, gasService, tokenManager, tokenId; const name = 'tokenName'; const symbol = 'tokenSymbol'; + const otherChains = ['chain 1', 'chain 2']; const decimals = 18; before(async () => { const wallets = await ethers.getSigners(); wallet = wallets[0]; - [service, gateway, gasService] = await deployAll(wallet, 'Test'); + [service, gateway, gasService] = await deployAll(wallet, 'Test', otherChains); }); describe('Full canonical token registration, remote deployment and token send', async () => { let token; - const otherChains = ['chain 1', 'chain 2']; const gasValues = [1234, 5678]; const tokenCap = BigInt(1e18); @@ -134,7 +134,6 @@ describe('Interchain Token Service Flow', () => { let token; let tokenId; const salt = getRandomBytes32(); - const otherChains = ['chain 1', 'chain 2']; const gasValues = [1234, 5678]; const tokenCap = BigInt(1e18); From 69cc47e69869b1ca793b66aeb719dca9dfe470bc Mon Sep 17 00:00:00 2001 From: Foivos Date: Mon, 9 Oct 2023 11:54:31 +0300 Subject: [PATCH 3/6] feat: using a custom implementation of AxelarExecutable (#114) * using a custom implementation of AxelarExecutable * Fixed an import bug. --------- Co-authored-by: Milap Sheth --- .../InterchainTokenService.sol | 40 ++++++++++++------- .../interfaces/IInterchainTokenService.sol | 1 + 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol index 4ba539c6..6c3392eb 100644 --- a/contracts/interchain-token-service/InterchainTokenService.sol +++ b/contracts/interchain-token-service/InterchainTokenService.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGasService.sol'; -import { AxelarExecutable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/executable/AxelarExecutable.sol'; +import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; @@ -33,15 +33,7 @@ import { Multicall } from '../utils/Multicall.sol'; * It (mostly) does not handle tokens, but is responsible for the messaging that needs to occur for cross chain transfers to happen. * @dev The only storage used here is for ExpressCalls */ -contract InterchainTokenService is - IInterchainTokenService, - AxelarExecutable, - Upgradable, - Operatable, - ExpressCallHandler, - Pausable, - Multicall -{ +contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatable, ExpressCallHandler, Pausable, Multicall { using StringToBytes32 for string; using Bytes32ToString for bytes32; using AddressBytesUtils for bytes; @@ -51,6 +43,7 @@ contract InterchainTokenService is address internal immutable implementationMintBurn; address internal immutable implementationLockUnlockFee; address internal immutable implementationLiquidityPool; + IAxelarGateway public immutable gateway; IAxelarGasService public immutable gasService; IRemoteAddressValidator public immutable remoteAddressValidator; address public immutable tokenManagerDeployer; @@ -84,13 +77,16 @@ contract InterchainTokenService is address gasService_, address remoteAddressValidator_, address[] memory tokenManagerImplementations - ) AxelarExecutable(gateway_) { + ) { if ( remoteAddressValidator_ == address(0) || gasService_ == address(0) || tokenManagerDeployer_ == address(0) || - standardizedTokenDeployer_ == address(0) + standardizedTokenDeployer_ == address(0) || + gateway_ == address(0) ) revert ZeroAddress(); + + gateway = IAxelarGateway(gateway_); remoteAddressValidator = IRemoteAddressValidator(remoteAddressValidator_); gasService = IAxelarGasService(gasService_); tokenManagerDeployer = tokenManagerDeployer_; @@ -530,11 +526,16 @@ contract InterchainTokenService is * @param sourceAddress The address where the transaction originates from * @param payload The encoded data payload for the transaction */ - function _execute( + function execute( + bytes32 commandId, string calldata sourceChain, string calldata sourceAddress, bytes calldata payload - ) internal override onlyRemoteService(sourceChain, sourceAddress) notPaused { + ) external onlyRemoteService(sourceChain, sourceAddress) notPaused { + bytes32 payloadHash = keccak256(payload); + + if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash)) revert NotApprovedByGateway(); + uint256 selector = abi.decode(payload, (uint256)); if (selector == SELECTOR_SEND_TOKEN || selector == SELECTOR_SEND_TOKEN_WITH_DATA) { _processSendTokenPayload(sourceChain, payload, selector); @@ -547,6 +548,17 @@ contract InterchainTokenService is } } + function executeWithToken( + bytes32 /*commandId*/, + string calldata /*sourceChain*/, + string calldata /*sourceAddress*/, + bytes calldata /*payload*/, + string calldata /*tokenSymbol*/, + uint256 /*amount*/ + ) external pure { + revert ExecuteWithTokenNotSupported(); + } + /** * @notice Processes the payload data for a send token call * @param sourceChain The chain where the transaction originates from diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index a46bcfd2..a9325220 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -26,6 +26,7 @@ interface IInterchainTokenService is ITokenManagerType, IExpressCallHandler, IAx error SelectorUnknown(); error InvalidMetadataVersion(uint32 version); error AlreadyExecuted(bytes32 commandId); + error ExecuteWithTokenNotSupported(); error InvalidExpressSelector(); event TokenSent(bytes32 indexed tokenId, string destinationChain, bytes destinationAddress, uint256 indexed amount); From 6e29f749633828c6982273d2efcf23e4ac73101d Mon Sep 17 00:00:00 2001 From: Kiryl Yermakou Date: Mon, 9 Oct 2023 05:36:28 -0400 Subject: [PATCH 4/6] ci(Slither): action + fixes + comments (#117) * renamed folder and changed version * npmignore * npmignore * change version * using include pattern instead. * Fixed most of the things least auhority suggested. * made lint happy * Apply suggestions from code review * fixed some bugs * added events * rename set to transfer for distributor and operator * changed standardized token to always allow token managers to mint/burn it. * using immutable storage for remoteAddressValidator address to save gas * Added some recommended changes * added milap's suggested changes * Fixed some names and some minor gas optimizations * prettier and lint * stash * import .env in hardhat.config * trying to fix .env.example * Added some getters in IRemoteAddressValidator and removed useless check for distributor in the InterchainTokenService. * removed ternary operators * made lint happy * made lint happy * Added a new token manager to handle fee on transfer and added some tests for it as well * fixed the liquidity pool check. * fix a duplication bug * lint * added some more tests * Added more tests * Added proper re-entrancy protection for fee on transfer token managers. * change to tx.origin for refunds * Added support for more kinds of addresses. * some minor gas opts * some more gas optimizations. * Added a getter for chain name to the remote address validator. * moved the tokenManager getter functionality to a separate contract which saves almost a kilobyte of codesize. * made lint happy * Removed tokenManagerGetter and put params into tokenManagers * Added separate tokenManager interfaces * addressed ackeeblockchains's 3.0 report * prettier * added interchain transfer methods to the service and unified receiving tokens a bit. * made lint happy * rename sendToken to interchainTransfer * changed sendToken everywhere * changed from uint256.max to a const * change setting to zero to delete for storage slots. * rearange storage variables to save a bit of gas. * Removed unecesairy casts * made as many event params inexed as possible * Removed unused imports * domain separator is calculated each time. * added some natspec * added an example for using pre-existing custom tokens. * added a comment * feat(TokenManager): MintBurnFrom and MintBurnFromAddress * fix(TokenManager): removed MintBurnFromAddress as deprecated * ci(Slither): action + fixes + comments * style(Solidity): prettier --------- Co-authored-by: Foivos Co-authored-by: Milap Sheth --- .github/workflows/slither.yml | 16 +++++ .../InterchainTokenService.sol | 71 ++++++++++++------- contracts/interfaces/IDistributable.sol | 8 +-- contracts/interfaces/IERC20BurnableFrom.sol | 8 --- contracts/interfaces/IImplementation.sol | 7 ++ contracts/interfaces/IInterchainToken.sol | 9 +++ .../interfaces/IInterchainTokenService.sol | 2 +- .../interfaces/IRemoteAddressValidator.sol | 4 +- contracts/interfaces/IStandardizedToken.sol | 21 ++---- .../interfaces/ITokenManagerLiquidityPool.sol | 7 +- .../interfaces/ITokenManagerLockUnlock.sol | 6 +- .../interfaces/ITokenManagerLockUnlockFee.sol | 6 +- .../interfaces/ITokenManagerMintBurn.sol | 6 +- contracts/proxies/StandardizedTokenProxy.sol | 4 +- .../token-implementations/ERC20Permit.sol | 3 +- .../StandardizedToken.sol | 10 ++- contracts/token-manager/TokenManager.sol | 9 +-- .../TokenManagerLiquidityPool.sol | 21 +++--- .../TokenManagerLockUnlock.sol | 18 ++--- .../TokenManagerLockUnlockFee.sol | 21 +++--- .../implementations/TokenManagerMintBurn.sol | 12 ++-- contracts/utils/Implementation.sol | 8 --- contracts/utils/Multicall.sol | 1 + contracts/utils/StandardizedTokenDeployer.sol | 2 + contracts/utils/TokenManagerDeployer.sol | 2 + hardhat.config.js | 2 +- slither.config.json | 4 ++ 27 files changed, 167 insertions(+), 121 deletions(-) create mode 100644 .github/workflows/slither.yml create mode 100644 slither.config.json diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml new file mode 100644 index 00000000..d807be46 --- /dev/null +++ b/.github/workflows/slither.yml @@ -0,0 +1,16 @@ +name: Slither Static Analysis + +on: + - pull_request + +jobs: + analyze: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Dependencies + run: npm ci + + - name: Run Slither + uses: crytic/slither-action@v0.3.0 diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol index 6c3392eb..1d1f8571 100644 --- a/contracts/interchain-token-service/InterchainTokenService.sol +++ b/contracts/interchain-token-service/InterchainTokenService.sol @@ -278,7 +278,9 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab function deployRemoteCanonicalToken(bytes32 tokenId, string calldata destinationChain, uint256 gasValue) public payable notPaused { address tokenAddress = getValidTokenManagerAddress(tokenId); tokenAddress = ITokenManager(tokenAddress).tokenAddress(); + if (getCanonicalTokenId(tokenAddress) != tokenId) revert NotCanonicalTokenManager(); + (string memory tokenName, string memory tokenSymbol, uint8 tokenDecimals) = _validateToken(tokenAddress); _deployRemoteStandardizedToken(tokenId, tokenName, tokenSymbol, tokenDecimals, '', '', 0, '', destinationChain, gasValue); } @@ -296,8 +298,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab ) public payable notPaused returns (bytes32 tokenId) { address deployer_ = msg.sender; tokenId = getCustomTokenId(deployer_, salt); - _deployTokenManager(tokenId, tokenManagerType, params); + emit CustomTokenIdClaimed(tokenId, deployer_, salt); + + _deployTokenManager(tokenId, tokenManagerType, params); } /** @@ -320,8 +324,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab ) external payable notPaused returns (bytes32 tokenId) { address deployer_ = msg.sender; tokenId = getCustomTokenId(deployer_, salt); - _deployRemoteTokenManager(tokenId, destinationChain, gasValue, tokenManagerType, params); + emit CustomTokenIdClaimed(tokenId, deployer_, salt); + + _deployRemoteTokenManager(tokenId, destinationChain, gasValue, tokenManagerType, params); } /** @@ -357,7 +363,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param distributor the address that will be able to mint and burn the deployed token. * @param mintTo The address where the minted tokens will be sent upon deployment * @param mintAmount The amount of tokens to be minted upon deployment - * @param operator The operator data for standardized tokens + * @param operator_ The operator data for standardized tokens * @param destinationChain the name of the destination chain to deploy to. * @param gasValue the amount of native tokens to be used to pay for gas for the remote deployment. At least the amount * specified needs to be passed to the call @@ -371,7 +377,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab bytes memory distributor, bytes memory mintTo, uint256 mintAmount, - bytes memory operator, + bytes memory operator_, string calldata destinationChain, uint256 gasValue ) external payable notPaused { @@ -384,7 +390,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab distributor, mintTo, mintAmount, - operator, + operator_, destinationChain, gasValue ); @@ -486,11 +492,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param tokenIds an array of the token Ids of the tokenManagers to set the flow limit of. * @param flowLimits the flowLimits to set */ - function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator { + function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator { uint256 length = tokenIds.length; if (length != flowLimits.length) revert LengthMismatch(); for (uint256 i; i < length; ++i) { ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenIds[i])); + // slither-disable-next-line calls-loop tokenManager.setFlowLimit(flowLimits[i]); } } @@ -592,6 +599,9 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab bytes memory data; (, , , , sourceAddress, data) = abi.decode(payload, (uint256, bytes32, bytes, uint256, bytes, bytes)); + // slither-disable-next-line reentrancy-events + emit TokenReceivedWithData(tokenId, sourceChain, destinationAddress, amount, sourceAddress, data); + IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken( sourceChain, sourceAddress, @@ -599,8 +609,8 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab tokenId, amount ); - emit TokenReceivedWithData(tokenId, sourceChain, destinationAddress, amount, sourceAddress, data); } else { + // slither-disable-next-line reentrancy-events emit TokenReceived(tokenId, sourceChain, destinationAddress, amount); } } @@ -700,9 +710,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab TokenManagerType tokenManagerType, bytes memory params ) internal { + emit RemoteTokenManagerDeploymentInitialized(tokenId, destinationChain, gasValue, tokenManagerType, params); + bytes memory payload = abi.encode(SELECTOR_DEPLOY_TOKEN_MANAGER, tokenId, tokenManagerType, params); _callContract(destinationChain, payload, gasValue); - emit RemoteTokenManagerDeploymentInitialized(tokenId, destinationChain, gasValue, tokenManagerType, params); } /** @@ -714,7 +725,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param distributor The distributor address for the token * @param mintTo The address where the minted tokens will be sent upon deployment * @param mintAmount The amount of tokens to be minted upon deployment - * @param operator The operator data for standardized tokens + * @param operator_ The operator data for standardized tokens * @param destinationChain The destination chain where the token will be deployed * @param gasValue The amount of gas to be paid for the transaction */ @@ -726,12 +737,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab bytes memory distributor, bytes memory mintTo, uint256 mintAmount, - bytes memory operator, + bytes memory operator_, string calldata destinationChain, uint256 gasValue ) internal { - bytes memory payload = abi.encode( - SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN, + // slither-disable-next-line reentrancy-events + emit RemoteStandardizedTokenAndManagerDeploymentInitialized( tokenId, name, symbol, @@ -739,10 +750,13 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab distributor, mintTo, mintAmount, - operator + operator_, + destinationChain, + gasValue ); - _callContract(destinationChain, payload, gasValue); - emit RemoteStandardizedTokenAndManagerDeploymentInitialized( + + bytes memory payload = abi.encode( + SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN, tokenId, name, symbol, @@ -750,10 +764,9 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab distributor, mintTo, mintAmount, - operator, - destinationChain, - gasValue + operator_ ); + _callContract(destinationChain, payload, gasValue); } /** @@ -763,13 +776,14 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param params Additional parameters for the token manager deployment */ function _deployTokenManager(bytes32 tokenId, TokenManagerType tokenManagerType, bytes memory params) internal { + // slither-disable-next-line reentrancy-events + emit TokenManagerDeployed(tokenId, tokenManagerType, params); + + // slither-disable-next-line controlled-delegatecall (bool success, ) = tokenManagerDeployer.delegatecall( abi.encodeWithSelector(ITokenManagerDeployer.deployTokenManager.selector, tokenId, tokenManagerType, params) ); - if (!success) { - revert TokenManagerDeploymentFailed(); - } - emit TokenManagerDeployed(tokenId, tokenManagerType, params); + if (!success) revert TokenManagerDeploymentFailed(); } /** @@ -800,9 +814,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab uint256 mintAmount, address mintTo ) internal { + emit StandardizedTokenDeployed(tokenId, distributor, name, symbol, decimals, mintAmount, mintTo); + bytes32 salt = _getStandardizedTokenSalt(tokenId); address tokenManagerAddress = getTokenManagerAddress(tokenId); + // slither-disable-next-line controlled-delegatecall (bool success, ) = standardizedTokenDeployer.delegatecall( abi.encodeWithSelector( IStandardizedTokenDeployer.deployStandardizedToken.selector, @@ -819,7 +836,6 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab if (!success) { revert StandardizedTokenDeploymentFailed(); } - emit StandardizedTokenDeployed(tokenId, distributor, name, symbol, decimals, mintAmount, mintTo); } function _decodeMetadata(bytes memory metadata) internal pure returns (uint32 version, bytes memory data) { @@ -872,16 +888,21 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab ) internal { bytes memory payload; if (metadata.length < 4) { + // slither-disable-next-line reentrancy-events + emit TokenSent(tokenId, destinationChain, destinationAddress, amount); + payload = abi.encode(SELECTOR_SEND_TOKEN, tokenId, destinationAddress, amount); _callContract(destinationChain, payload, msg.value); - emit TokenSent(tokenId, destinationChain, destinationAddress, amount); return; } uint32 version; (version, metadata) = _decodeMetadata(metadata); if (version > 0) revert InvalidMetadataVersion(version); + + // slither-disable-next-line reentrancy-events + emit TokenSentWithData(tokenId, destinationChain, destinationAddress, amount, sourceAddress, metadata); + payload = abi.encode(SELECTOR_SEND_TOKEN_WITH_DATA, tokenId, destinationAddress, amount, sourceAddress.toBytes(), metadata); _callContract(destinationChain, payload, msg.value); - emit TokenSentWithData(tokenId, destinationChain, destinationAddress, amount, sourceAddress, metadata); } } diff --git a/contracts/interfaces/IDistributable.sol b/contracts/interfaces/IDistributable.sol index 7605eccd..cfe446dc 100644 --- a/contracts/interfaces/IDistributable.sol +++ b/contracts/interfaces/IDistributable.sol @@ -11,16 +11,16 @@ interface IDistributable { /** * @notice Get the address of the distributor - * @return distributor of the distributor + * @return distributor_ of the distributor */ - function distributor() external view returns (address distributor); + function distributor() external view returns (address distributor_); /** * @notice Change the distributor of the contract * @dev Can only be called by the current distributor - * @param distributor The address of the new distributor + * @param distributor_ The address of the new distributor */ - function transferDistributorship(address distributor) external; + function transferDistributorship(address distributor_) external; /** * @notice Proposed a change of the distributor of the contract diff --git a/contracts/interfaces/IERC20BurnableFrom.sol b/contracts/interfaces/IERC20BurnableFrom.sol index 5c086542..82487bee 100644 --- a/contracts/interfaces/IERC20BurnableFrom.sol +++ b/contracts/interfaces/IERC20BurnableFrom.sol @@ -6,14 +6,6 @@ pragma solidity ^0.8.0; * @dev Interface of the ERC20 standard as defined in the EIP. */ interface IERC20BurnableFrom { - /** - * @notice Function to burn tokens from a burn deposit address - * @notice It is needed to support legacy Axelar Gateway tokens - * @dev Can only be called after token is transferred to a deposit address. - * @param salt The address that will have its tokens burnt - */ - function burn(bytes32 salt) external; - /** * @notice Function to burn tokens * @notice Requires the caller to have allowance for `amount` on `from` diff --git a/contracts/interfaces/IImplementation.sol b/contracts/interfaces/IImplementation.sol index 950c90a3..357d3b22 100644 --- a/contracts/interfaces/IImplementation.sol +++ b/contracts/interfaces/IImplementation.sol @@ -4,4 +4,11 @@ pragma solidity ^0.8.0; interface IImplementation { error NotProxy(); + + /** + * @notice Called by the proxy to setup itself. + * @dev This should be hidden by the proxy. + * @param params the data to be used for the initialization. + */ + function setup(bytes calldata params) external; } diff --git a/contracts/interfaces/IInterchainToken.sol b/contracts/interfaces/IInterchainToken.sol index c4456cd6..2a427aa3 100644 --- a/contracts/interfaces/IInterchainToken.sol +++ b/contracts/interfaces/IInterchainToken.sol @@ -2,10 +2,19 @@ pragma solidity ^0.8.0; +import { ITokenManager } from './ITokenManager.sol'; + /** * @dev Interface of the ERC20 standard as defined in the EIP. */ interface IInterchainToken { + /** + * @notice Getter for the tokenManager used for this token. + * @dev Needs to be overwitten. + * @return tokenManager_ the TokenManager called to facilitate cross chain transfers. + */ + function tokenManager() external view returns (ITokenManager tokenManager_); + /** * @notice Implementation of the interchainTransfer method * @dev We chose to either pass `metadata` as raw data on a remote contract call, or, if no data is passed, just do a transfer. diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index a9325220..730c6d31 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -267,7 +267,7 @@ interface IInterchainTokenService is ITokenManagerType, IExpressCallHandler, IAx * @param tokenIds An array of tokenIds. * @param flowLimits An array of flow limits corresponding to the tokenIds. */ - function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external; + function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external; /** * @notice Returns the flow limit for a specific token. diff --git a/contracts/interfaces/IRemoteAddressValidator.sol b/contracts/interfaces/IRemoteAddressValidator.sol index 818a893b..a609eacc 100644 --- a/contracts/interfaces/IRemoteAddressValidator.sol +++ b/contracts/interfaces/IRemoteAddressValidator.sol @@ -43,8 +43,8 @@ interface IRemoteAddressValidator { /** * @dev Fetches the interchain token service address for the specified chain - * @param chainName Name of the chain + * @param chainName_ Name of the chain * @return remoteAddress Interchain token service address for the specified chain */ - function getRemoteAddress(string calldata chainName) external view returns (string memory remoteAddress); + function getRemoteAddress(string calldata chainName_) external view returns (string memory remoteAddress); } diff --git a/contracts/interfaces/IStandardizedToken.sol b/contracts/interfaces/IStandardizedToken.sol index 9763a2ff..1fbd40bd 100644 --- a/contracts/interfaces/IStandardizedToken.sol +++ b/contracts/interfaces/IStandardizedToken.sol @@ -2,10 +2,10 @@ pragma solidity ^0.8.0; +import { IImplementation } from './IImplementation.sol'; import { IInterchainToken } from './IInterchainToken.sol'; import { IDistributable } from './IDistributable.sol'; import { IERC20MintableBurnable } from './IERC20MintableBurnable.sol'; -import { ITokenManager } from './ITokenManager.sol'; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; /** @@ -13,23 +13,12 @@ import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interf * @notice This contract implements a standardized token which extends InterchainToken functionality. * This contract also inherits Distributable and Implementation logic. */ -interface IStandardizedToken is IInterchainToken, IDistributable, IERC20MintableBurnable, IERC20 { +interface IStandardizedToken is IImplementation, IInterchainToken, IDistributable, IERC20MintableBurnable, IERC20 { + error TokenManagerAddressZero(); + error TokenNameEmpty(); + /** * @notice Returns the contract id, which a proxy can check to ensure no false implementation was used. */ function contractId() external view returns (bytes32); - - /** - * @notice Called by the proxy to setup itself. - * @dev This should be hidden by the proxy. - * @param params the data to be used for the initialization. - */ - function setup(bytes calldata params) external; - - /** - * @notice Getter for the tokenManager used for this token. - * @dev Needs to be overwitten. - * @return tokenManager_ the TokenManager called to facilitate cross chain transfers. - */ - function tokenManager() external view returns (ITokenManager tokenManager_); } diff --git a/contracts/interfaces/ITokenManagerLiquidityPool.sol b/contracts/interfaces/ITokenManagerLiquidityPool.sol index 902a0d1f..7390d428 100644 --- a/contracts/interfaces/ITokenManagerLiquidityPool.sol +++ b/contracts/interfaces/ITokenManagerLiquidityPool.sol @@ -11,11 +11,12 @@ import { ITokenManager } from './ITokenManager.sol'; interface ITokenManagerLiquidityPool is ITokenManager { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. + * @param liquidityPool_ he address of the liquidity pool. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress, address liquidityPool) external pure returns (bytes memory params); + function getParams(bytes memory operator_, address tokenAddress_, address liquidityPool_) external pure returns (bytes memory params); /** * @dev Reads the stored liquidity pool address from the specified storage slot diff --git a/contracts/interfaces/ITokenManagerLockUnlock.sol b/contracts/interfaces/ITokenManagerLockUnlock.sol index 5c009ae8..cf8688c2 100644 --- a/contracts/interfaces/ITokenManagerLockUnlock.sol +++ b/contracts/interfaces/ITokenManagerLockUnlock.sol @@ -11,9 +11,9 @@ import { ITokenManager } from './ITokenManager.sol'; interface ITokenManagerLockUnlock is ITokenManager { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params); } diff --git a/contracts/interfaces/ITokenManagerLockUnlockFee.sol b/contracts/interfaces/ITokenManagerLockUnlockFee.sol index 686b01d3..8dd3987c 100644 --- a/contracts/interfaces/ITokenManagerLockUnlockFee.sol +++ b/contracts/interfaces/ITokenManagerLockUnlockFee.sol @@ -11,9 +11,9 @@ import { ITokenManager } from './ITokenManager.sol'; interface ITokenManagerLockUnlockFee is ITokenManager { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params); } diff --git a/contracts/interfaces/ITokenManagerMintBurn.sol b/contracts/interfaces/ITokenManagerMintBurn.sol index b9e1f4d1..5f954fc2 100644 --- a/contracts/interfaces/ITokenManagerMintBurn.sol +++ b/contracts/interfaces/ITokenManagerMintBurn.sol @@ -11,9 +11,9 @@ import { ITokenManager } from './ITokenManager.sol'; interface ITokenManagerMintBurn is ITokenManager { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params); } diff --git a/contracts/proxies/StandardizedTokenProxy.sol b/contracts/proxies/StandardizedTokenProxy.sol index 7dcac348..d82d3989 100644 --- a/contracts/proxies/StandardizedTokenProxy.sol +++ b/contracts/proxies/StandardizedTokenProxy.sol @@ -3,8 +3,10 @@ pragma solidity ^0.8.0; import { FixedProxy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/FixedProxy.sol'; + import { IStandardizedToken } from '../interfaces/IStandardizedToken.sol'; import { IStandardizedTokenProxy } from '../interfaces/IStandardizedTokenProxy.sol'; +import { IImplementation } from '../interfaces/IImplementation.sol'; /** * @title StandardizedTokenProxy @@ -21,7 +23,7 @@ contract StandardizedTokenProxy is FixedProxy, IStandardizedTokenProxy { constructor(address implementationAddress, bytes memory params) FixedProxy(implementationAddress) { if (IStandardizedToken(implementationAddress).contractId() != CONTRACT_ID) revert InvalidImplementation(); - (bool success, ) = implementationAddress.delegatecall(abi.encodeWithSelector(IStandardizedToken.setup.selector, params)); + (bool success, ) = implementationAddress.delegatecall(abi.encodeWithSelector(IImplementation.setup.selector, params)); if (!success) revert SetupFailed(); } diff --git a/contracts/token-implementations/ERC20Permit.sol b/contracts/token-implementations/ERC20Permit.sol index 349bd7de..cc327535 100644 --- a/contracts/token-implementations/ERC20Permit.sol +++ b/contracts/token-implementations/ERC20Permit.sol @@ -49,8 +49,9 @@ abstract contract ERC20Permit is IERC20, IERC20Permit, ERC20 { * @notice Calculates the DOMAIN_SEPARATOR. * @dev This is not cached because chainid can change on chain forks. */ - // solhint-disable-next-line func-name-mixedcase + // slither-disable-next-line naming-convention function DOMAIN_SEPARATOR() public view returns (bytes32 domainSeparator) { + // solhint-disable-line func-name-mixedcase domainSeparator = keccak256(abi.encode(DOMAIN_TYPE_SIGNATURE_HASH, nameHash, keccak256(bytes('1')), block.chainid, address(this))); } diff --git a/contracts/token-implementations/StandardizedToken.sol b/contracts/token-implementations/StandardizedToken.sol index 769fa53e..80e1dbad 100644 --- a/contracts/token-implementations/StandardizedToken.sol +++ b/contracts/token-implementations/StandardizedToken.sol @@ -2,8 +2,9 @@ pragma solidity ^0.8.0; -import { IERC20MintableBurnable } from '../interfaces/IERC20MintableBurnable.sol'; +import { IStandardizedToken } from '../interfaces/IStandardizedToken.sol'; import { ITokenManager } from '../interfaces/ITokenManager.sol'; +import { IInterchainToken } from '../interfaces/IInterchainToken.sol'; import { InterchainToken } from '../interchain-token/InterchainToken.sol'; import { ERC20Permit } from '../token-implementations/ERC20Permit.sol'; @@ -16,7 +17,7 @@ import { Distributable } from '../utils/Distributable.sol'; * @notice This contract implements a standardized token which extends InterchainToken functionality. * This contract also inherits Distributable and Implementation logic. */ -contract StandardizedToken is IERC20MintableBurnable, InterchainToken, ERC20Permit, Implementation, Distributable { +contract StandardizedToken is InterchainToken, ERC20Permit, Implementation, Distributable, IStandardizedToken { using AddressBytesUtils for bytes; string public name; @@ -45,7 +46,7 @@ contract StandardizedToken is IERC20MintableBurnable, InterchainToken, ERC20Perm * @notice Returns the token manager for this token * @return ITokenManager The token manager contract */ - function tokenManager() public view override returns (ITokenManager) { + function tokenManager() public view override(InterchainToken, IInterchainToken) returns (ITokenManager) { return ITokenManager(tokenManager_); } @@ -64,6 +65,9 @@ contract StandardizedToken is IERC20MintableBurnable, InterchainToken, ERC20Perm (address, address, string, string, uint8) ); + if (tokenManagerAddress == address(0)) revert TokenManagerAddressZero(); + if (bytes(tokenName).length == 0) revert TokenNameEmpty(); + tokenManager_ = tokenManagerAddress; name = tokenName; diff --git a/contracts/token-manager/TokenManager.sol b/contracts/token-manager/TokenManager.sol index 516baea5..507e7c5e 100644 --- a/contracts/token-manager/TokenManager.sol +++ b/contracts/token-manager/TokenManager.sol @@ -60,9 +60,10 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen /** * @notice A function that returns the token id. - * @dev This will only work when called by a proxy, which hides this and returns the correct value. + * @dev This will only work when implementation is called by a proxy, which stores the tokenId as an immutable. */ function tokenId() public view returns (bytes32) { + // slither-disable-next-line var-read-using-this return this.tokenId(); } @@ -108,7 +109,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen _addFlowOut(amount); interchainTokenService.transmitSendToken{ value: msg.value }( - this.tokenId(), + tokenId(), sender, destinationChain, destinationAddress, @@ -135,7 +136,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen _addFlowOut(amount); uint32 version = 0; interchainTokenService.transmitSendToken{ value: msg.value }( - this.tokenId(), + tokenId(), sender, destinationChain, destinationAddress, @@ -162,7 +163,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen amount = _takeToken(sender, amount); _addFlowOut(amount); interchainTokenService.transmitSendToken{ value: msg.value }( - this.tokenId(), + tokenId(), sender, destinationChain, destinationAddress, diff --git a/contracts/token-manager/implementations/TokenManagerLiquidityPool.sol b/contracts/token-manager/implementations/TokenManagerLiquidityPool.sol index 5089a444..51ad6f1a 100644 --- a/contracts/token-manager/implementations/TokenManagerLiquidityPool.sol +++ b/contracts/token-manager/implementations/TokenManagerLiquidityPool.sol @@ -2,13 +2,13 @@ pragma solidity ^0.8.0; -import { TokenManager } from '../TokenManager.sol'; -import { NoReEntrancy } from '../../utils/NoReEntrancy.sol'; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; -import { ITokenManagerLiquidityPool } from '../../interfaces/ITokenManagerLiquidityPool.sol'; - import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; +import { ITokenManagerLiquidityPool } from '../../interfaces/ITokenManagerLiquidityPool.sol'; +import { TokenManager } from '../TokenManager.sol'; +import { NoReEntrancy } from '../../utils/NoReEntrancy.sol'; + /** * @title TokenManagerLiquidityPool * @notice This contract is a an implementation of TokenManager that stores all tokens in a separate liquity pool @@ -16,7 +16,7 @@ import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/c * @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods. * It uses the Axelar SDK to safely transfer tokens. */ -contract TokenManagerLiquidityPool is TokenManager, NoReEntrancy { +contract TokenManagerLiquidityPool is TokenManager, NoReEntrancy, ITokenManagerLiquidityPool { using SafeTokenTransferFrom for IERC20; // uint256(keccak256('liquidity-pool-slot')) - 1 @@ -102,6 +102,7 @@ contract TokenManagerLiquidityPool is TokenManager, NoReEntrancy { IERC20 token = IERC20(tokenAddress()); uint256 balance = token.balanceOf(to); + // slither-disable-next-line arbitrary-send-erc20 token.safeTransferFrom(liquidityPool(), to, amount); uint256 diff = token.balanceOf(to) - balance; @@ -113,16 +114,16 @@ contract TokenManagerLiquidityPool is TokenManager, NoReEntrancy { /** * @notice Getter function for the parameters of a liquidity pool TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @param liquidityPoolAddress the liquidity pool to be used to store the bridged tokens. * @return params the resulting params to be passed to custom TokenManager deployments. */ function getParams( - bytes memory operator, - address tokenAddress, + bytes memory operator_, + address tokenAddress_, address liquidityPoolAddress ) external pure returns (bytes memory params) { - params = abi.encode(operator, tokenAddress, liquidityPoolAddress); + params = abi.encode(operator_, tokenAddress_, liquidityPoolAddress); } } diff --git a/contracts/token-manager/implementations/TokenManagerLockUnlock.sol b/contracts/token-manager/implementations/TokenManagerLockUnlock.sol index 96ebce7e..bad398ed 100644 --- a/contracts/token-manager/implementations/TokenManagerLockUnlock.sol +++ b/contracts/token-manager/implementations/TokenManagerLockUnlock.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.0; -import { TokenManager } from '../TokenManager.sol'; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; -import { ITokenManagerLockUnlock } from '../../interfaces/ITokenManagerLockUnlock.sol'; - import { SafeTokenTransfer, SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; +import { ITokenManagerLockUnlock } from '../../interfaces/ITokenManagerLockUnlock.sol'; +import { TokenManager } from '../TokenManager.sol'; + /** * @title TokenManagerLockUnlock * @notice This contract is an implementation of TokenManager that locks and unlocks a specific token on behalf of the interchain token service. @@ -35,8 +35,8 @@ contract TokenManagerLockUnlock is TokenManager, ITokenManagerLockUnlock { */ function _setup(bytes calldata params) internal override { // The first argument is reserved for the operator. - (, address tokenAddress) = abi.decode(params, (bytes, address)); - _setTokenAddress(tokenAddress); + (, address tokenAddress_) = abi.decode(params, (bytes, address)); + _setTokenAddress(tokenAddress_); } /** @@ -69,11 +69,11 @@ contract TokenManagerLockUnlock is TokenManager, ITokenManagerLockUnlock { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params) { - params = abi.encode(operator, tokenAddress); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params) { + params = abi.encode(operator_, tokenAddress_); } } diff --git a/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol b/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol index b3b95f01..3994d0fb 100644 --- a/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol +++ b/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol @@ -2,19 +2,20 @@ pragma solidity ^0.8.0; -import { TokenManager } from '../TokenManager.sol'; -import { NoReEntrancy } from '../../utils/NoReEntrancy.sol'; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; - import { SafeTokenTransferFrom, SafeTokenTransfer } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; +import { ITokenManagerLockUnlock } from '../../interfaces/ITokenManagerLockUnlock.sol'; +import { TokenManager } from '../TokenManager.sol'; +import { NoReEntrancy } from '../../utils/NoReEntrancy.sol'; + /** * @title TokenManagerLockUnlock * @notice This contract is an implementation of TokenManager that locks and unlocks a specific token on behalf of the interchain token service. * @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods. * It uses the Axelar SDK to safely transfer tokens. */ -contract TokenManagerLockUnlockFee is TokenManager, NoReEntrancy { +contract TokenManagerLockUnlockFee is TokenManager, NoReEntrancy, ITokenManagerLockUnlock { using SafeTokenTransfer for IERC20; using SafeTokenTransferFrom for IERC20; @@ -35,8 +36,8 @@ contract TokenManagerLockUnlockFee is TokenManager, NoReEntrancy { */ function _setup(bytes calldata params) internal override { // The first argument is reserved for the operator. - (, address tokenAddress) = abi.decode(params, (bytes, address)); - _setTokenAddress(tokenAddress); + (, address tokenAddress_) = abi.decode(params, (bytes, address)); + _setTokenAddress(tokenAddress_); } /** @@ -71,11 +72,11 @@ contract TokenManagerLockUnlockFee is TokenManager, NoReEntrancy { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params) { - params = abi.encode(operator, tokenAddress); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params) { + params = abi.encode(operator_, tokenAddress_); } } diff --git a/contracts/token-manager/implementations/TokenManagerMintBurn.sol b/contracts/token-manager/implementations/TokenManagerMintBurn.sol index 779fd80f..0286293f 100644 --- a/contracts/token-manager/implementations/TokenManagerMintBurn.sol +++ b/contracts/token-manager/implementations/TokenManagerMintBurn.sol @@ -35,8 +35,8 @@ contract TokenManagerMintBurn is TokenManager, ITokenManagerMintBurn { */ function _setup(bytes calldata params) internal override { // The first argument is reserved for the operator. - (, address tokenAddress) = abi.decode(params, (bytes, address)); - _setTokenAddress(tokenAddress); + (, address tokenAddress_) = abi.decode(params, (bytes, address)); + _setTokenAddress(tokenAddress_); } /** @@ -69,11 +69,11 @@ contract TokenManagerMintBurn is TokenManager, ITokenManagerMintBurn { /** * @notice Getter function for the parameters of a lock/unlock TokenManager. Mainly to be used by frontends. - * @param operator the operator of the TokenManager. - * @param tokenAddress the token to be managed. + * @param operator_ the operator of the TokenManager. + * @param tokenAddress_ the token to be managed. * @return params the resulting params to be passed to custom TokenManager deployments. */ - function getParams(bytes memory operator, address tokenAddress) external pure returns (bytes memory params) { - params = abi.encode(operator, tokenAddress); + function getParams(bytes memory operator_, address tokenAddress_) external pure returns (bytes memory params) { + params = abi.encode(operator_, tokenAddress_); } } diff --git a/contracts/utils/Implementation.sol b/contracts/utils/Implementation.sol index 2ae3fd09..ad662978 100644 --- a/contracts/utils/Implementation.sol +++ b/contracts/utils/Implementation.sol @@ -27,12 +27,4 @@ abstract contract Implementation is IImplementation { if (implementationAddress == address(this)) revert NotProxy(); _; } - - /** - * @notice Initializes contract parameters. - * This function is intended to be overridden by derived contracts. - * The overriding function must have the onlyProxy modifier. - * @param params The parameters to be used for initialization - */ - function setup(bytes calldata params) external virtual; } diff --git a/contracts/utils/Multicall.sol b/contracts/utils/Multicall.sol index 50914d39..55fe0ad2 100644 --- a/contracts/utils/Multicall.sol +++ b/contracts/utils/Multicall.sol @@ -24,6 +24,7 @@ contract Multicall is IMulticall { bool success; bytes memory result; for (uint256 i = 0; i < data.length; ++i) { + // slither-disable-next-line calls-loop,delegatecall-loop (success, result) = address(this).delegatecall(data[i]); if (!success) { diff --git a/contracts/utils/StandardizedTokenDeployer.sol b/contracts/utils/StandardizedTokenDeployer.sol index 77536f3f..da28e244 100644 --- a/contracts/utils/StandardizedTokenDeployer.sol +++ b/contracts/utils/StandardizedTokenDeployer.sol @@ -35,6 +35,7 @@ contract StandardizedTokenDeployer is IStandardizedTokenDeployer { * @param mintAmount Amount of tokens to mint initially * @param mintTo Address to mint initial tokens to */ + // slither-disable-next-line locked-ether function deployStandardizedToken( bytes32 salt, address tokenManager, @@ -46,6 +47,7 @@ contract StandardizedTokenDeployer is IStandardizedTokenDeployer { address mintTo ) external payable { 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.deploy(salt, bytecode); diff --git a/contracts/utils/TokenManagerDeployer.sol b/contracts/utils/TokenManagerDeployer.sol index 14040795..a2fb413b 100644 --- a/contracts/utils/TokenManagerDeployer.sol +++ b/contracts/utils/TokenManagerDeployer.sol @@ -19,8 +19,10 @@ contract TokenManagerDeployer is ITokenManagerDeployer { * @param implementationType Token manager implementation type * @param params Additional parameters used in the setup of the token manager */ + // slither-disable-next-line locked-ether function deployTokenManager(bytes32 tokenId, uint256 implementationType, bytes calldata params) external payable { bytes memory args = abi.encode(address(this), implementationType, tokenId, params); + // slither-disable-next-line too-many-digits bytes memory bytecode = abi.encodePacked(type(TokenManagerProxy).creationCode, args); address tokenManagerAddress = Create3.deploy(tokenId, bytecode); if (tokenManagerAddress.code.length == 0) revert TokenManagerDeploymentFailed(); diff --git a/hardhat.config.js b/hardhat.config.js index b4a3f6ad..709fbfc6 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -13,7 +13,7 @@ const { networks, etherscan } = importNetworks(chains, keys); */ module.exports = { solidity: { - version: '0.8.18', + version: '0.8.19', settings: { evmVersion: process.env.EVM_VERSION || 'london', optimizer: { diff --git a/slither.config.json b/slither.config.json new file mode 100644 index 00000000..1a22cab9 --- /dev/null +++ b/slither.config.json @@ -0,0 +1,4 @@ +{ + "detectors_to_exclude": "assembly,low-level-calls,solc-version,timestamp,dead-code", + "filter_paths": "(contracts/test/|node_modules/)" +} From 02c3bd57ef4e26ae5e3e77d9c9054b7aecdb2cb4 Mon Sep 17 00:00:00 2001 From: Kiryl Yermakou Date: Mon, 9 Oct 2023 06:05:55 -0400 Subject: [PATCH 5/6] feat(InterchainTokenExecutable): passing token address to the executable (#118) * feat(InterchainTokenExecutable): passing token address to the executable * fixed slither --------- Co-authored-by: Milap Sheth --- .../examples/InterchainTokenExecutable.sol | 4 +- .../InterchainTokenExpressExecutable.sol | 3 +- .../InterchainTokenService.sol | 110 ++++++++---------- .../interfaces/IInterchainTokenExecutable.sol | 8 +- .../IInterchainTokenExpressExecutable.sol | 8 +- contracts/interfaces/ITokenManagerType.sol | 2 +- contracts/test/InterchainExecutableTest.sol | 4 +- .../token-implementations/ERC20Permit.sol | 11 +- .../TokenManagerLockUnlockFee.sol | 2 +- 9 files changed, 71 insertions(+), 81 deletions(-) diff --git a/contracts/examples/InterchainTokenExecutable.sol b/contracts/examples/InterchainTokenExecutable.sol index 019a0414..50124de8 100644 --- a/contracts/examples/InterchainTokenExecutable.sol +++ b/contracts/examples/InterchainTokenExecutable.sol @@ -23,9 +23,10 @@ abstract contract InterchainTokenExecutable is IInterchainTokenExecutable { bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) external onlyService { - _executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, amount); + _executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, token, amount); } function _executeWithInterchainToken( @@ -33,6 +34,7 @@ abstract contract InterchainTokenExecutable is IInterchainTokenExecutable { bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) internal virtual; } diff --git a/contracts/examples/InterchainTokenExpressExecutable.sol b/contracts/examples/InterchainTokenExpressExecutable.sol index 24649c2f..d93eacac 100644 --- a/contracts/examples/InterchainTokenExpressExecutable.sol +++ b/contracts/examples/InterchainTokenExpressExecutable.sol @@ -13,8 +13,9 @@ abstract contract InterchainTokenExpressExecutable is IInterchainTokenExpressExe bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) external onlyService { - _executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, amount); + _executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, token, amount); } } diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol index 1d1f8571..c577bf07 100644 --- a/contracts/interchain-token-service/InterchainTokenService.sol +++ b/contracts/interchain-token-service/InterchainTokenService.sol @@ -2,26 +2,25 @@ pragma solidity ^0.8.0; +import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGasService.sol'; -import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; +import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol'; +import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol'; import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; -import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; +import { StringToBytes32, Bytes32ToString } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Bytes32String.sol'; import { IInterchainTokenService } from '../interfaces/IInterchainTokenService.sol'; +import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; import { ITokenManagerDeployer } from '../interfaces/ITokenManagerDeployer.sol'; import { IStandardizedTokenDeployer } from '../interfaces/IStandardizedTokenDeployer.sol'; import { IRemoteAddressValidator } from '../interfaces/IRemoteAddressValidator.sol'; +import { IInterchainTokenExecutable } from '../interfaces/IInterchainTokenExecutable.sol'; import { IInterchainTokenExpressExecutable } from '../interfaces/IInterchainTokenExpressExecutable.sol'; import { ITokenManager } from '../interfaces/ITokenManager.sol'; import { ITokenManagerProxy } from '../interfaces/ITokenManagerProxy.sol'; import { IERC20Named } from '../interfaces/IERC20Named.sol'; import { AddressBytesUtils } from '../libraries/AddressBytesUtils.sol'; -import { StringToBytes32, Bytes32ToString } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Bytes32String.sol'; - -import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol'; -import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol'; - import { ExpressCallHandler } from '../utils/ExpressCallHandler.sol'; import { Pausable } from '../utils/Pausable.sol'; import { Operatable } from '../utils/Operatable.sol'; @@ -38,9 +37,11 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab using Bytes32ToString for bytes32; using AddressBytesUtils for bytes; using AddressBytesUtils for address; + using SafeTokenTransferFrom for IERC20; address internal immutable implementationLockUnlock; address internal immutable implementationMintBurn; + address internal immutable implementationMintBurnFrom; address internal immutable implementationLockUnlockFee; address internal immutable implementationLiquidityPool; IAxelarGateway public immutable gateway; @@ -54,8 +55,8 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab bytes32 internal constant PREFIX_STANDARDIZED_TOKEN_ID = keccak256('its-standardized-token-id'); bytes32 internal constant PREFIX_STANDARDIZED_TOKEN_SALT = keccak256('its-standardized-token-salt'); - uint256 private constant SELECTOR_SEND_TOKEN = 1; - uint256 private constant SELECTOR_SEND_TOKEN_WITH_DATA = 2; + uint256 private constant SELECTOR_RECEIVE_TOKEN = 1; + uint256 private constant SELECTOR_RECEIVE_TOKEN_WITH_DATA = 2; uint256 private constant SELECTOR_DEPLOY_TOKEN_MANAGER = 3; uint256 private constant SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN = 4; @@ -94,12 +95,10 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab if (tokenManagerImplementations.length != uint256(type(TokenManagerType).max) + 1) revert LengthMismatch(); - implementationLockUnlock = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.LOCK_UNLOCK); implementationMintBurn = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.MINT_BURN); - implementationLockUnlockFee = _sanitizeTokenManagerImplementation( - tokenManagerImplementations, - TokenManagerType.LOCK_UNLOCK_FEE_ON_TRANSFER - ); + implementationMintBurnFrom = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.MINT_BURN_FROM); + implementationLockUnlock = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.LOCK_UNLOCK); + implementationLockUnlockFee = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.LOCK_UNLOCK_FEE); implementationLiquidityPool = _sanitizeTokenManagerImplementation(tokenManagerImplementations, TokenManagerType.LIQUIDITY_POOL); string memory chainName_ = remoteAddressValidator.chainName(); chainNameHash = keccak256(bytes(chainName_)); @@ -207,17 +206,16 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param tokenManagerType the type of the TokenManager. * @return tokenManagerAddress the address of the TokenManagerImplementation. */ - function getImplementation(uint256 tokenManagerType) external view returns (address tokenManagerAddress) { + function getImplementation(uint256 tokenManagerType) external view returns (address) { if (tokenManagerType > uint256(type(TokenManagerType).max)) revert InvalidImplementation(); - if (TokenManagerType(tokenManagerType) == TokenManagerType.LOCK_UNLOCK) { - return implementationLockUnlock; - } else if (TokenManagerType(tokenManagerType) == TokenManagerType.MINT_BURN) { - return implementationMintBurn; - } else if (TokenManagerType(tokenManagerType) == TokenManagerType.LOCK_UNLOCK_FEE_ON_TRANSFER) { - return implementationLockUnlockFee; - } else if (TokenManagerType(tokenManagerType) == TokenManagerType.LIQUIDITY_POOL) { - return implementationLiquidityPool; - } + + if (TokenManagerType(tokenManagerType) == TokenManagerType.MINT_BURN) return implementationMintBurn; + if (TokenManagerType(tokenManagerType) == TokenManagerType.MINT_BURN_FROM) return implementationMintBurnFrom; + if (TokenManagerType(tokenManagerType) == TokenManagerType.LOCK_UNLOCK) return implementationLockUnlock; + if (TokenManagerType(tokenManagerType) == TokenManagerType.LOCK_UNLOCK_FEE) return implementationLockUnlockFee; + if (TokenManagerType(tokenManagerType) == TokenManagerType.LIQUIDITY_POOL) return implementationLiquidityPool; + + revert InvalidImplementation(); } /** @@ -418,18 +416,19 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId)); IERC20 token = IERC20(tokenManager.tokenAddress()); - SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount); + token.safeTransferFrom(caller, destinationAddress, amount); - if (selector == SELECTOR_SEND_TOKEN_WITH_DATA) { + if (selector == SELECTOR_RECEIVE_TOKEN_WITH_DATA) { (, , , , bytes memory sourceAddress, bytes memory data) = abi.decode(payload, (uint256, bytes32, bytes, uint256, bytes, bytes)); - IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken( + IInterchainTokenExpressExecutable(destinationAddress).expressExecuteWithInterchainToken( sourceChain, sourceAddress, data, tokenId, + address(token), amount ); - } else if (selector != SELECTOR_SEND_TOKEN) { + } else if (selector != SELECTOR_RECEIVE_TOKEN) { revert InvalidExpressSelector(); } } @@ -519,12 +518,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab } function _sanitizeTokenManagerImplementation( - address[] memory implementations, + address[] memory tokenManagerImplementations, TokenManagerType tokenManagerType - ) internal pure returns (address implementation) { - implementation = implementations[uint256(tokenManagerType)]; - if (implementation == address(0)) revert ZeroAddress(); - if (ITokenManager(implementation).implementationType() != uint256(tokenManagerType)) revert InvalidTokenManagerImplementation(); + ) internal pure returns (address implementation_) { + implementation_ = tokenManagerImplementations[uint256(tokenManagerType)]; + if (implementation_ == address(0)) revert ZeroAddress(); + if (ITokenManager(implementation_).implementationType() != uint256(tokenManagerType)) revert InvalidTokenManagerImplementation(); } /** @@ -544,15 +543,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash)) revert NotApprovedByGateway(); uint256 selector = abi.decode(payload, (uint256)); - if (selector == SELECTOR_SEND_TOKEN || selector == SELECTOR_SEND_TOKEN_WITH_DATA) { - _processSendTokenPayload(sourceChain, payload, selector); - } else if (selector == SELECTOR_DEPLOY_TOKEN_MANAGER) { - _processDeployTokenManagerPayload(payload); - } else if (selector == SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN) { - _processDeployStandardizedTokenAndManagerPayload(payload); - } else { - revert SelectorUnknown(); - } + if (selector == SELECTOR_RECEIVE_TOKEN || selector == SELECTOR_RECEIVE_TOKEN_WITH_DATA) + return _processReceiveTokenPayload(sourceChain, payload, selector); + if (selector == SELECTOR_DEPLOY_TOKEN_MANAGER) return _processDeployTokenManagerPayload(payload); + if (selector == SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN) return _processDeployStandardizedTokenAndManagerPayload(payload); + + revert SelectorUnknown(); } function executeWithToken( @@ -571,7 +567,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param sourceChain The chain where the transaction originates from * @param payload The encoded data payload to be processed */ - function _processSendTokenPayload(string calldata sourceChain, bytes calldata payload, uint256 selector) internal { + function _processReceiveTokenPayload(string calldata sourceChain, bytes calldata payload, uint256 selector) internal { bytes32 tokenId; address destinationAddress; uint256 amount; @@ -594,7 +590,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab } } amount = tokenManager.giveToken(destinationAddress, amount); - if (selector == SELECTOR_SEND_TOKEN_WITH_DATA) { + if (selector == SELECTOR_RECEIVE_TOKEN_WITH_DATA) { bytes memory sourceAddress; bytes memory data; (, , , , sourceAddress, data) = abi.decode(payload, (uint256, bytes32, bytes, uint256, bytes, bytes)); @@ -602,11 +598,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab // slither-disable-next-line reentrancy-events emit TokenReceivedWithData(tokenId, sourceChain, destinationAddress, amount, sourceAddress, data); - IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken( + IInterchainTokenExecutable(destinationAddress).executeWithInterchainToken( sourceChain, sourceAddress, data, tokenId, + tokenManager.tokenAddress(), amount ); } else { @@ -852,23 +849,6 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab } } - function _expressExecuteWithInterchainTokenToken( - bytes32 tokenId, - address destinationAddress, - string memory sourceChain, - bytes memory sourceAddress, - bytes calldata data, - uint256 amount - ) internal { - IInterchainTokenExpressExecutable(destinationAddress).expressExecuteWithInterchainToken( - sourceChain, - sourceAddress, - data, - tokenId, - amount - ); - } - /** * @notice Transmit a sendTokenWithData for the given tokenId. Only callable by a token manager. * @param tokenId the tokenId of the TokenManager (which must be the msg.sender). @@ -891,7 +871,8 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab // slither-disable-next-line reentrancy-events emit TokenSent(tokenId, destinationChain, destinationAddress, amount); - payload = abi.encode(SELECTOR_SEND_TOKEN, tokenId, destinationAddress, amount); + payload = abi.encode(SELECTOR_RECEIVE_TOKEN, tokenId, destinationAddress, amount); + _callContract(destinationChain, payload, msg.value); return; } @@ -902,7 +883,8 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab // slither-disable-next-line reentrancy-events emit TokenSentWithData(tokenId, destinationChain, destinationAddress, amount, sourceAddress, metadata); - payload = abi.encode(SELECTOR_SEND_TOKEN_WITH_DATA, tokenId, destinationAddress, amount, sourceAddress.toBytes(), metadata); + payload = abi.encode(SELECTOR_RECEIVE_TOKEN_WITH_DATA, tokenId, destinationAddress, amount, sourceAddress.toBytes(), metadata); + _callContract(destinationChain, payload, msg.value); } } diff --git a/contracts/interfaces/IInterchainTokenExecutable.sol b/contracts/interfaces/IInterchainTokenExecutable.sol index c5597708..319c84d6 100644 --- a/contracts/interfaces/IInterchainTokenExecutable.sol +++ b/contracts/interfaces/IInterchainTokenExecutable.sol @@ -9,11 +9,12 @@ pragma solidity ^0.8.0; interface IInterchainTokenExecutable { /** * @notice This will be called after the tokens arrive to this contract - * @dev You are revert unless the msg.sender is the InterchainTokenService + * @dev Executable should revert unless the msg.sender is the InterchainTokenService * @param sourceChain the name of the source chain * @param sourceAddress the address that sent the contract call - * @param data the data to be proccessed - * @param tokenId the tokenId of the token manager managing the token. You can access it's address by querying the service + * @param data the data to be processed + * @param tokenId the tokenId of the token manager managing the token. + * @param token the address of the token. * @param amount the amount of token that was sent */ function executeWithInterchainToken( @@ -21,6 +22,7 @@ interface IInterchainTokenExecutable { bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) external; } diff --git a/contracts/interfaces/IInterchainTokenExpressExecutable.sol b/contracts/interfaces/IInterchainTokenExpressExecutable.sol index 9475af22..3b78a26d 100644 --- a/contracts/interfaces/IInterchainTokenExpressExecutable.sol +++ b/contracts/interfaces/IInterchainTokenExpressExecutable.sol @@ -11,11 +11,12 @@ import { IInterchainTokenExecutable } from './IInterchainTokenExecutable.sol'; interface IInterchainTokenExpressExecutable is IInterchainTokenExecutable { /** * @notice This will be called after the tokens arrive to this contract - * @dev You are revert unless the msg.sender is the InterchainTokenService + * @dev Executable should revert unless the msg.sender is the InterchainTokenService * @param sourceChain the name of the source chain * @param sourceAddress the address that sent the contract call - * @param data the data to be proccessed - * @param tokenId the tokenId of the token manager managing the token. You can access it's address by querying the service + * @param data the data to be processed + * @param tokenId the token id of the token manager managing the token. + * @param token the address of the token. * @param amount the amount of token that was sent */ function expressExecuteWithInterchainToken( @@ -23,6 +24,7 @@ interface IInterchainTokenExpressExecutable is IInterchainTokenExecutable { bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) external; } diff --git a/contracts/interfaces/ITokenManagerType.sol b/contracts/interfaces/ITokenManagerType.sol index 925bff8c..0882d56a 100644 --- a/contracts/interfaces/ITokenManagerType.sol +++ b/contracts/interfaces/ITokenManagerType.sol @@ -11,7 +11,7 @@ interface ITokenManagerType { MINT_BURN, MINT_BURN_FROM, LOCK_UNLOCK, - LOCK_UNLOCK_FEE_ON_TRANSFER, + LOCK_UNLOCK_FEE, LIQUIDITY_POOL } } diff --git a/contracts/test/InterchainExecutableTest.sol b/contracts/test/InterchainExecutableTest.sol index bf952cb0..21c4b0d3 100644 --- a/contracts/test/InterchainExecutableTest.sol +++ b/contracts/test/InterchainExecutableTest.sol @@ -18,12 +18,12 @@ contract InterchainExecutableTest is InterchainTokenExpressExecutable { bytes calldata sourceAddress, bytes calldata data, bytes32 tokenId, + address token, uint256 amount ) internal override { (address receiver, string memory message) = abi.decode(data, (address, string)); lastMessage = message; - address tokenAddress = IInterchainTokenService(msg.sender).getTokenAddress(tokenId); emit MessageReceived(sourceChain, sourceAddress, receiver, message, tokenId, amount); - IERC20(tokenAddress).transfer(receiver, amount); + IERC20(token).transfer(receiver, amount); } } diff --git a/contracts/token-implementations/ERC20Permit.sol b/contracts/token-implementations/ERC20Permit.sol index cc327535..b692d723 100644 --- a/contracts/token-implementations/ERC20Permit.sol +++ b/contracts/token-implementations/ERC20Permit.sol @@ -21,7 +21,6 @@ abstract contract ERC20Permit is IERC20, IERC20Permit, ERC20 { /** * @dev Represents hash of the EIP-712 Domain Separator. */ - // solhint-disable-next-line var-name-mixedcase bytes32 public nameHash; string private constant EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA = '\x19\x01'; @@ -46,15 +45,17 @@ abstract contract ERC20Permit is IERC20, IERC20Permit, ERC20 { } /** - * @notice Calculates the DOMAIN_SEPARATOR. + * @notice Calculates the domain separator. * @dev This is not cached because chainid can change on chain forks. */ + // solhint-disable func-name-mixedcase // slither-disable-next-line naming-convention - function DOMAIN_SEPARATOR() public view returns (bytes32 domainSeparator) { - // solhint-disable-line func-name-mixedcase - domainSeparator = keccak256(abi.encode(DOMAIN_TYPE_SIGNATURE_HASH, nameHash, keccak256(bytes('1')), block.chainid, address(this))); + function DOMAIN_SEPARATOR() public view returns (bytes32) { + return keccak256(abi.encode(DOMAIN_TYPE_SIGNATURE_HASH, nameHash, keccak256(bytes('1')), block.chainid, address(this))); } + // solhint-enable func-name-mixedcase + /** * @notice Permit the designated spender to spend the holder's tokens * @dev The permit function is used to allow a holder to designate a spender diff --git a/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol b/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol index 3994d0fb..0f5f90fe 100644 --- a/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol +++ b/contracts/token-manager/implementations/TokenManagerLockUnlockFee.sol @@ -27,7 +27,7 @@ contract TokenManagerLockUnlockFee is TokenManager, NoReEntrancy, ITokenManagerL constructor(address interchainTokenService_) TokenManager(interchainTokenService_) {} function implementationType() external pure returns (uint256) { - return uint256(TokenManagerType.LOCK_UNLOCK_FEE_ON_TRANSFER); + return uint256(TokenManagerType.LOCK_UNLOCK_FEE); } /** From 8fc9653eeade757ef58cee7bbd86c36f4a7f1b48 Mon Sep 17 00:00:00 2001 From: Milap Sheth Date: Mon, 9 Oct 2023 06:21:53 -0400 Subject: [PATCH 6/6] fix: pass commandId explicitly (#121) --- .../InterchainTokenService.sol | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/contracts/interchain-token-service/InterchainTokenService.sol b/contracts/interchain-token-service/InterchainTokenService.sol index c577bf07..195ce6c2 100644 --- a/contracts/interchain-token-service/InterchainTokenService.sol +++ b/contracts/interchain-token-service/InterchainTokenService.sol @@ -4,13 +4,13 @@ pragma solidity ^0.8.0; import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol'; import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGasService.sol'; +import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol'; import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol'; import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/SafeTransfer.sol'; import { StringToBytes32, Bytes32ToString } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Bytes32String.sol'; import { IInterchainTokenService } from '../interfaces/IInterchainTokenService.sol'; -import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol'; import { ITokenManagerDeployer } from '../interfaces/ITokenManagerDeployer.sol'; import { IStandardizedTokenDeployer } from '../interfaces/IStandardizedTokenDeployer.sol'; import { IRemoteAddressValidator } from '../interfaces/IRemoteAddressValidator.sol'; @@ -544,7 +544,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab uint256 selector = abi.decode(payload, (uint256)); if (selector == SELECTOR_RECEIVE_TOKEN || selector == SELECTOR_RECEIVE_TOKEN_WITH_DATA) - return _processReceiveTokenPayload(sourceChain, payload, selector); + return _processReceiveTokenPayload(commandId, sourceChain, payload, selector); if (selector == SELECTOR_DEPLOY_TOKEN_MANAGER) return _processDeployTokenManagerPayload(payload); if (selector == SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN) return _processDeployStandardizedTokenAndManagerPayload(payload); @@ -567,7 +567,12 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab * @param sourceChain The chain where the transaction originates from * @param payload The encoded data payload to be processed */ - function _processReceiveTokenPayload(string calldata sourceChain, bytes calldata payload, uint256 selector) internal { + function _processReceiveTokenPayload( + bytes32 commandId, + string calldata sourceChain, + bytes calldata payload, + uint256 selector + ) internal { bytes32 tokenId; address destinationAddress; uint256 amount; @@ -576,11 +581,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab (, tokenId, destinationAddressBytes, amount) = abi.decode(payload, (uint256, bytes32, bytes, uint256)); destinationAddress = destinationAddressBytes.toAddress(); } - bytes32 commandId; - assembly { - commandId := calldataload(4) - } ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId)); { address expressCaller = _popExpressReceiveToken(payload, commandId); @@ -590,6 +591,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab } } amount = tokenManager.giveToken(destinationAddress, amount); + if (selector == SELECTOR_RECEIVE_TOKEN_WITH_DATA) { bytes memory sourceAddress; bytes memory data; @@ -621,6 +623,7 @@ contract InterchainTokenService is IInterchainTokenService, Upgradable, Operatab payload, (uint256, bytes32, TokenManagerType, bytes) ); + _deployTokenManager(tokenId, tokenManagerType, params); }