Merge branch 'main' into feat/prevent-deploy-token-manager-on-its-hub

contracts/InterchainTokenFactory.sol

@@ -104,6 +104,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
     /**
      * @notice Deploys a new interchain token with specified parameters.
      * @dev Creates a new token and optionally mints an initial amount to a specified minter.
+     * This function is `payable` because non-payable functions cannot be called in a multicall that calls other `payable` functions.
      * @param salt The unique salt for deploying the token.
      * @param name The name of the token.
      * @param symbol The symbol of the token.
@@ -250,6 +251,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
 
     /**
      * @notice Registers a canonical token as an interchain token and deploys its token manager.
+     * @dev This function is `payable` because non-payable functions cannot be called in a multicall that calls other `payable` functions.
      * @param tokenAddress The address of the canonical token.
      * @return tokenId The tokenId corresponding to the registered canonical token.
      */

contracts/InterchainTokenService.sol

@@ -302,6 +302,8 @@ contract InterchainTokenService is
         bytes calldata params,
         uint256 gasValue
     ) external payable whenNotPaused returns (bytes32 tokenId) {
+        if (bytes(params).length == 0) revert EmptyParams();
+
         // Custom token managers can't be deployed with native interchain token type, which is reserved for interchain tokens
         if (tokenManagerType == TokenManagerType.NATIVE_INTERCHAIN_TOKEN) revert CannotDeploy(tokenManagerType);
 
@@ -390,6 +392,7 @@ contract InterchainTokenService is
 
     /**
      * @notice Express executes operations based on the payload and selector.
+     * @dev This function is `payable` because non-payable functions cannot be called in a multicall that calls other `payable` functions.
      * @param commandId The unique message id.
      * @param sourceChain The chain where the transaction originates from.
      * @param sourceAddress The address of the remote ITS where the transaction originates from.
@@ -523,6 +526,7 @@ contract InterchainTokenService is
         uint256 gasValue
     ) external payable whenNotPaused {
         if (data.length == 0) revert EmptyData();
+
         amount = _takeToken(tokenId, msg.sender, amount, false);
 
         _transmitInterchainTransfer(
@@ -921,6 +925,9 @@ contract InterchainTokenService is
         string calldata destinationChain,
         uint256 gasValue
     ) internal {
+        if (bytes(name).length == 0) revert EmptyTokenName();
+        if (bytes(symbol).length == 0) revert EmptyTokenSymbol();
+
         // slither-disable-next-line unused-return
         deployedTokenManager(tokenId);
 
@@ -982,6 +989,9 @@ contract InterchainTokenService is
         string memory symbol,
         uint8 decimals
     ) internal returns (address tokenAddress) {
+        if (bytes(name).length == 0) revert EmptyTokenName();
+        if (bytes(symbol).length == 0) revert EmptyTokenSymbol();
+
         bytes32 salt = _getInterchainTokenSalt(tokenId);
 
         address minter;
@@ -1043,6 +1053,7 @@ contract InterchainTokenService is
         bytes memory data,
         uint256 gasValue
     ) internal {
+        if (destinationAddress.length == 0) revert EmptyDestinationAddress();
         if (amount == 0) revert ZeroAmount();
 
         // slither-disable-next-line reentrancy-events

contracts/TokenHandler.sol

@@ -31,8 +31,7 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Crea
      * @return uint256 The amount of token actually given, which could be different for certain token type.
      * @return address the address of the token.
      */
-    // slither-disable-next-line locked-ether
-    function giveToken(bytes32 tokenId, address to, uint256 amount) external payable returns (uint256, address) {
+    function giveToken(bytes32 tokenId, address to, uint256 amount) external returns (uint256, address) {
         address tokenManager = _create3Address(tokenId);
 
         (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager).getImplementationTypeAndTokenAddress();
@@ -107,8 +106,7 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Crea
      * @return uint256 The amount of token actually transferred, which could be different for certain token type.
      * @return address The address of the token corresponding to the input tokenId.
      */
-    // slither-disable-next-line locked-ether
-    function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external payable returns (uint256, address) {
+    function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external returns (uint256, address) {
         address tokenManager = _create3Address(tokenId);
         (uint256 tokenManagerType, address tokenAddress) = ITokenManagerProxy(tokenManager).getImplementationTypeAndTokenAddress();
 

contracts/interfaces/IInterchainTokenService.sol

@@ -50,6 +50,10 @@ interface IInterchainTokenService is
     error CannotDeployRemotelyToSelf();
     error InvalidPayload();
     error GatewayCallFailed(bytes data);
+    error EmptyTokenName();
+    error EmptyTokenSymbol();
+    error EmptyParams();
+    error EmptyDestinationAddress();
     error NotSupported();
 
     event InterchainTransfer(

contracts/interfaces/ITokenHandler.sol

@@ -18,7 +18,7 @@ interface ITokenHandler {
      * @return uint256 The amount of token actually given, which could be different for certain token type.
      * @return address the address of the token.
      */
-    function giveToken(bytes32 tokenId, address to, uint256 amount) external payable returns (uint256, address);
+    function giveToken(bytes32 tokenId, address to, uint256 amount) external returns (uint256, address);
 
     /**
      * @notice This function takes token from a specified address to the token manager.
@@ -40,8 +40,7 @@ interface ITokenHandler {
      * @return uint256 The amount of token actually transferred, which could be different for certain token type.
      * @return address The address of the token corresponding to the input tokenId.
      */
-    // slither-disable-next-line locked-ether
-    function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external payable returns (uint256, address);
+    function transferTokenFrom(bytes32 tokenId, address from, address to, uint256 amount) external returns (uint256, address);
 
     /**
      * @notice This function prepares a token manager after it is deployed

test/InterchainTokenService.js

@@ -675,6 +675,26 @@ describe('Interchain Token Service', () => {
 
             await service.setPauseStatus(false).then((tx) => tx.wait);
         });
+
+        it('Should revert when registering an interchain token with empty token name', async () => {
+            expect(await tokenManager.hasRole(wallet.address, OPERATOR_ROLE)).to.be.true;
+
+            await expectRevert(
+                (gasOptions) => service.deployInterchainToken(salt, '', '', tokenSymbol, tokenDecimals, wallet.address, 0, gasOptions),
+                service,
+                'EmptyTokenName',
+            );
+        });
+
+        it('Should revert when registering an interchain token with empty token symbol', async () => {
+            expect(await tokenManager.hasRole(wallet.address, OPERATOR_ROLE)).to.be.true;
+
+            await expectRevert(
+                (gasOptions) => service.deployInterchainToken(salt, '', tokenName, '', tokenDecimals, wallet.address, 0, gasOptions),
+                service,
+                'EmptyTokenSymbol',
+            );
+        });
     });
 
     describe('Deploy and Register remote Interchain Token', () => {
@@ -728,6 +748,30 @@ describe('Interchain Token Service', () => {
             );
         });
 
+        it('Should revert on remote interchain token deployment with invalid token name', async () => {
+            await expectRevert(
+                (gasOptions) =>
+                    service.deployInterchainToken(salt, destinationChain, '', tokenSymbol, tokenDecimals, minter, gasValue, {
+                        ...gasOptions,
+                        value: gasValue,
+                    }),
+                service,
+                'EmptyTokenName',
+            );
+        });
+
+        it('Should revert on remote interchain token deployment with invalid token symbol', async () => {
+            await expectRevert(
+                (gasOptions) =>
+                    service.deployInterchainToken(salt, destinationChain, tokenName, '', tokenDecimals, minter, gasValue, {
+                        ...gasOptions,
+                        value: gasValue,
+                    }),
+                service,
+                'EmptyTokenSymbol',
+            );
+        });
+
         it('Should revert on remote interchain token deployment if paused', async () => {
             await service.setPauseStatus(true).then((tx) => tx.wait);
 
@@ -820,6 +864,14 @@ describe('Interchain Token Service', () => {
             await expectRevert((gasOptions) => service.deployTokenManager(salt, '', 6, params, 0, gasOptions));
         });
 
+        it('Should revert on deploying a local token manager with invalid params', async () => {
+            await expectRevert(
+                (gasOptions) => service.deployTokenManager(salt, '', NATIVE_INTERCHAIN_TOKEN, '0x', 0, gasOptions),
+                service,
+                'EmptyParams',
+            );
+        });
+
         it('Should revert on deploying a local token manager with interchain token manager type', async () => {
             const params = defaultAbiCoder.encode(['bytes', 'address'], [wallet.address, token.address]);
 
@@ -1315,6 +1367,18 @@ describe('Interchain Token Service', () => {
             );
         });
 
+        it('Should revert on initiate interchain token transfer with invalid destination address', async () => {
+            await expectRevert(
+                (gasOptions) =>
+                    service.interchainTransfer(tokenId, destinationChain, '0x', amount, '0x', gasValue, {
+                        ...gasOptions,
+                        value: gasValue,
+                    }),
+                service,
+                'EmptyDestinationAddress',
+            );
+        });
+
         it('Should revert on initiate interchain token transfer when service is paused', async () => {
             await service.setPauseStatus(true).then((tx) => tx.wait);
 
@@ -1343,6 +1407,18 @@ describe('Interchain Token Service', () => {
             await service.setPauseStatus(false).then((tx) => tx.wait);
         });
 
+        it('Should revert on transmit send token when destination address is zero address', async () => {
+            await expectRevert(
+                (gasOptions) =>
+                    service.transmitInterchainTransfer(tokenId, wallet.address, destinationChain, '0x', amount, '0x', {
+                        ...gasOptions,
+                        value: gasValue,
+                    }),
+                service,
+                'TakeTokenFailed',
+            );
+        });
+
         it('Should revert on transmit send token when not called by interchain token', async () => {
             const errorSignatureHash = id('NotToken(address,address)');
             const selector = errorSignatureHash.substring(0, 10);
@@ -1682,6 +1758,16 @@ describe('Interchain Token Service', () => {
             );
         });
 
+        it('Should revert on callContractWithInterchainToken function on the service with invalid destination address', async () => {
+            const [, , tokenId] = await deployFunctions.lockUnlock(service, 'Test Token', 'TT', 12, amount);
+
+            await expectRevert(
+                (gasOptions) => service.callContractWithInterchainToken(tokenId, destinationChain, '0x', amount, data, 0, gasOptions),
+                service,
+                'EmptyDestinationAddress',
+            );
+        });
+
         for (const type of ['lockUnlock', 'lockUnlockFee']) {
             it(`Should be able to initiate an interchain token transfer via the interchainTransfer function on the service when the service is approved as well [${type}]`, async () => {
                 const [token, tokenManager, tokenId] = await deployFunctions[type](service, `Test Token ${type}`, 'TT', 12, amount);
@@ -1731,7 +1817,9 @@ describe('Interchain Token Service', () => {
 
                 await expect(
                     reportGas(
-                        service.callContractWithInterchainToken(tokenId, destinationChain, destAddress, amount, data, 0),
+                        service.callContractWithInterchainToken(tokenId, destinationChain, destAddress, amount, data, 0, {
+                            value: gasValue,
+                        }),
                         `Call service.callContractWithInterchainToken ${type}`,
                     ),
                 )
@@ -2168,6 +2256,37 @@ describe('Interchain Token Service', () => {
             });
         }
 
+        for (const type of ['mintBurn', 'mintBurnFrom', 'lockUnlockFee', 'lockUnlock']) {
+            it(`Should be able to initiate an interchain token transfer via interchainTransfer [${type}] without native gas`, async () => {
+                [token, tokenManager, tokenId] = await deployFunctions[type](service, `Test Token ${type}`, 'TT', 12, amount * 3, true);
+                const sendAmount = type === 'lockUnlockFee' ? amount - 10 : amount;
+                const payload = defaultAbiCoder.encode(
+                    ['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'],
+                    [MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, hexlify(wallet.address), destAddress, sendAmount, '0x'],
+                );
+                const payloadHash = keccak256(payload);
+
+                let transferToAddress = AddressZero;
+
+                if (type === 'lockUnlock' || type === 'lockUnlockFee') {
+                    transferToAddress = tokenManager.address;
+                }
+
+                await expect(
+                    reportGas(
+                        token.connect(wallet).interchainTransfer(destinationChain, destAddress, amount, metadata),
+                        `Call token.interchainTransfer ${type}`,
+                    ),
+                )
+                    .and.to.emit(token, 'Transfer')
+                    .withArgs(wallet.address, transferToAddress, amount)
+                    .and.to.emit(gateway, 'ContractCall')
+                    .withArgs(service.address, destinationChain, service.address, payloadHash, payload)
+                    .to.emit(service, 'InterchainTransfer')
+                    .withArgs(tokenId, wallet.address, destinationChain, destAddress, sendAmount, HashZero);
+            });
+        }
+
         it('Should be able to initiate an interchain token transfer using interchainTransferFrom with max possible allowance', async () => {
             const sendAmount = amount;
             const payload = defaultAbiCoder.encode(