From 1ad445ec5d8ddf3c77c0425327ce7c5ef1c9b8b8 Mon Sep 17 00:00:00 2001 From: ahramy Date: Fri, 6 Sep 2024 17:25:27 -0700 Subject: [PATCH 1/6] fix: add minter check --- contracts/InterchainTokenFactory.sol | 2 ++ .../interfaces/IInterchainTokenFactory.sol | 1 + test/InterchainTokenFactory.js | 24 +++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index 83d83cb2..93555d8c 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -140,6 +140,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (initialSupply > 0) { minterBytes = address(this).toBytes(); } else if (minter != address(0)) { + if(minter == address(interchainTokenService)) revert InvalidMinter(minter); minterBytes = minter.toBytes(); } @@ -202,6 +203,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (minter != address(0)) { if (!token.isMinter(minter)) revert NotMinter(minter); + if(minter == address(interchainTokenService)) revert InvalidMinter(minter); minter_ = minter.toBytes(); } diff --git a/contracts/interfaces/IInterchainTokenFactory.sol b/contracts/interfaces/IInterchainTokenFactory.sol index ea07bbe2..ec1fabad 100644 --- a/contracts/interfaces/IInterchainTokenFactory.sol +++ b/contracts/interfaces/IInterchainTokenFactory.sol @@ -14,6 +14,7 @@ import { IInterchainTokenService } from './IInterchainTokenService.sol'; interface IInterchainTokenFactory is IUpgradable, IMulticall { error ZeroAddress(); error InvalidChainName(); + error InvalidMinter(address minter); error NotMinter(address minter); error NotOperator(address operator); error GatewayToken(address tokenAddress); diff --git a/test/InterchainTokenFactory.js b/test/InterchainTokenFactory.js index 2c3f6caa..ff420032 100644 --- a/test/InterchainTokenFactory.js +++ b/test/InterchainTokenFactory.js @@ -270,6 +270,16 @@ describe('InterchainTokenFactory', () => { expect(await tokenManager.isFlowLimiter(service.address)).to.be.true; }; + it('Should revert an interchain token deployment with the minter as interchainTokenService', async () => { + const salt = keccak256('0x1245'); + await expectRevert( + (gasOptions) => tokenFactory.deployInterchainToken(salt, name, symbol, decimals, 0, service.address, gasOptions), + tokenFactory, + 'InvalidMinter', + [service.address], + ); + }); + it('Should register a token if the mint amount is zero', async () => { const salt = keccak256('0x1234'); tokenId = await tokenFactory.interchainTokenId(wallet.address, salt); @@ -353,6 +363,20 @@ describe('InterchainTokenFactory', () => { await checkRoles(tokenManager, minter); }); + it('Should revert a remote interchain token deployment with the minter as interchainTokenService', async () => { + const gasValue = 1234; + await expectRevert( + (gasOptions) => + tokenFactory.deployRemoteInterchainToken(chainName, salt, service.address, destinationChain, gasValue, { + ...gasOptions, + value: gasValue, + }), + tokenFactory, + 'InvalidMinter', + [service.address], + ); + }); + it('Should initiate a remote interchain token deployment with the same minter', async () => { const gasValue = 1234; const mintAmount = 5678; From af8c87eb3d2cd16198a76cbaea463162388a7a50 Mon Sep 17 00:00:00 2001 From: ahramy Date: Fri, 6 Sep 2024 17:32:00 -0700 Subject: [PATCH 2/6] ran prettier and lint, moved unit tests logic --- contracts/InterchainTokenFactory.sol | 4 ++-- test/InterchainTokenFactory.js | 25 +++++++++++-------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index 93555d8c..b35ff239 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -140,7 +140,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (initialSupply > 0) { minterBytes = address(this).toBytes(); } else if (minter != address(0)) { - if(minter == address(interchainTokenService)) revert InvalidMinter(minter); + if (minter == address(interchainTokenService)) revert InvalidMinter(minter); minterBytes = minter.toBytes(); } @@ -203,7 +203,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (minter != address(0)) { if (!token.isMinter(minter)) revert NotMinter(minter); - if(minter == address(interchainTokenService)) revert InvalidMinter(minter); + if (minter == address(interchainTokenService)) revert InvalidMinter(minter); minter_ = minter.toBytes(); } diff --git a/test/InterchainTokenFactory.js b/test/InterchainTokenFactory.js index ff420032..573c7731 100644 --- a/test/InterchainTokenFactory.js +++ b/test/InterchainTokenFactory.js @@ -363,20 +363,6 @@ describe('InterchainTokenFactory', () => { await checkRoles(tokenManager, minter); }); - it('Should revert a remote interchain token deployment with the minter as interchainTokenService', async () => { - const gasValue = 1234; - await expectRevert( - (gasOptions) => - tokenFactory.deployRemoteInterchainToken(chainName, salt, service.address, destinationChain, gasValue, { - ...gasOptions, - value: gasValue, - }), - tokenFactory, - 'InvalidMinter', - [service.address], - ); - }); - it('Should initiate a remote interchain token deployment with the same minter', async () => { const gasValue = 1234; const mintAmount = 5678; @@ -424,6 +410,17 @@ describe('InterchainTokenFactory', () => { [otherWallet.address], ); + await expectRevert( + (gasOptions) => + tokenFactory.deployRemoteInterchainToken(chainName, salt, service.address, destinationChain, gasValue, { + ...gasOptions, + value: gasValue, + }), + tokenFactory, + 'InvalidMinter', + [service.address], + ); + await expect( tokenFactory.deployRemoteInterchainToken('', salt, wallet.address, destinationChain, gasValue, { value: gasValue, From fee574fb1c5abd84141a8ad1d3f16e36e4a25e4e Mon Sep 17 00:00:00 2001 From: ahramy Date: Fri, 6 Sep 2024 23:31:25 -0700 Subject: [PATCH 3/6] addressed comments --- contracts/InterchainTokenFactory.sol | 1 - contracts/InterchainTokenService.sol | 1 + contracts/interfaces/IInterchainTokenService.sol | 1 + test/InterchainTokenFactory.js | 10 ---------- test/InterchainTokenService.js | 13 +++++++++++++ 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index b35ff239..d1e57e2d 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -140,7 +140,6 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (initialSupply > 0) { minterBytes = address(this).toBytes(); } else if (minter != address(0)) { - if (minter == address(interchainTokenService)) revert InvalidMinter(minter); minterBytes = minter.toBytes(); } diff --git a/contracts/InterchainTokenService.sol b/contracts/InterchainTokenService.sol index f96bc72a..5789c063 100644 --- a/contracts/InterchainTokenService.sol +++ b/contracts/InterchainTokenService.sol @@ -1101,6 +1101,7 @@ contract InterchainTokenService is address minter; if (bytes(minterBytes).length != 0) minter = minterBytes.toAddress(); + if (minter == address(this)) revert InvalidMinter(minter); (bool success, bytes memory returnData) = interchainTokenDeployer.delegatecall( abi.encodeWithSelector(IInterchainTokenDeployer.deployInterchainToken.selector, salt, tokenId, minter, name, symbol, decimals) diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index 150e0fba..b5d99ae4 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -51,6 +51,7 @@ interface IInterchainTokenService is error InvalidGatewayTokenTransfer(bytes32 tokenId, bytes payload, string tokenSymbol, uint256 amount); error InvalidPayload(); error GatewayCallFailed(bytes data); + error InvalidMinter(address minter); event InterchainTransfer( bytes32 indexed tokenId, diff --git a/test/InterchainTokenFactory.js b/test/InterchainTokenFactory.js index 573c7731..5beff23a 100644 --- a/test/InterchainTokenFactory.js +++ b/test/InterchainTokenFactory.js @@ -270,16 +270,6 @@ describe('InterchainTokenFactory', () => { expect(await tokenManager.isFlowLimiter(service.address)).to.be.true; }; - it('Should revert an interchain token deployment with the minter as interchainTokenService', async () => { - const salt = keccak256('0x1245'); - await expectRevert( - (gasOptions) => tokenFactory.deployInterchainToken(salt, name, symbol, decimals, 0, service.address, gasOptions), - tokenFactory, - 'InvalidMinter', - [service.address], - ); - }); - it('Should register a token if the mint amount is zero', async () => { const salt = keccak256('0x1234'); tokenId = await tokenFactory.interchainTokenId(wallet.address, salt); diff --git a/test/InterchainTokenService.js b/test/InterchainTokenService.js index 3cce96cf..2933f308 100644 --- a/test/InterchainTokenService.js +++ b/test/InterchainTokenService.js @@ -705,6 +705,19 @@ describe('Interchain Token Service', () => { .withArgs(service.address, destinationChain, service.address, keccak256(payload), payload); }); + it('Should revert an interchain token deployment', async () => { + await expectRevert( + (gasOptions) => + service.deployInterchainToken(salt, '', tokenName, tokenSymbol, tokenDecimals, service.address, gasValue, { + ...gasOptions, + value: gasValue, + }), + service, + 'InvalidMinter', + [service.address], + ); + }); + it('Should revert on remote interchain token deployment if destination chain is not trusted', async () => { await expectRevert( (gasOptions) => From fb9ca523965fb7c1f63eac80b54d7c2b09bf7a40 Mon Sep 17 00:00:00 2001 From: ahramy Date: Sun, 8 Sep 2024 19:59:03 -0700 Subject: [PATCH 4/6] updated --- contracts/InterchainTokenFactory.sol | 1 + contracts/InterchainTokenService.sol | 1 - contracts/interfaces/IInterchainTokenService.sol | 1 - test/InterchainTokenFactory.js | 10 ++++++++++ test/InterchainTokenService.js | 13 ------------- 5 files changed, 11 insertions(+), 15 deletions(-) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index d1e57e2d..b35ff239 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -140,6 +140,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M if (initialSupply > 0) { minterBytes = address(this).toBytes(); } else if (minter != address(0)) { + if (minter == address(interchainTokenService)) revert InvalidMinter(minter); minterBytes = minter.toBytes(); } diff --git a/contracts/InterchainTokenService.sol b/contracts/InterchainTokenService.sol index 5789c063..f96bc72a 100644 --- a/contracts/InterchainTokenService.sol +++ b/contracts/InterchainTokenService.sol @@ -1101,7 +1101,6 @@ contract InterchainTokenService is address minter; if (bytes(minterBytes).length != 0) minter = minterBytes.toAddress(); - if (minter == address(this)) revert InvalidMinter(minter); (bool success, bytes memory returnData) = interchainTokenDeployer.delegatecall( abi.encodeWithSelector(IInterchainTokenDeployer.deployInterchainToken.selector, salt, tokenId, minter, name, symbol, decimals) diff --git a/contracts/interfaces/IInterchainTokenService.sol b/contracts/interfaces/IInterchainTokenService.sol index b5d99ae4..150e0fba 100644 --- a/contracts/interfaces/IInterchainTokenService.sol +++ b/contracts/interfaces/IInterchainTokenService.sol @@ -51,7 +51,6 @@ interface IInterchainTokenService is error InvalidGatewayTokenTransfer(bytes32 tokenId, bytes payload, string tokenSymbol, uint256 amount); error InvalidPayload(); error GatewayCallFailed(bytes data); - error InvalidMinter(address minter); event InterchainTransfer( bytes32 indexed tokenId, diff --git a/test/InterchainTokenFactory.js b/test/InterchainTokenFactory.js index 5beff23a..573c7731 100644 --- a/test/InterchainTokenFactory.js +++ b/test/InterchainTokenFactory.js @@ -270,6 +270,16 @@ describe('InterchainTokenFactory', () => { expect(await tokenManager.isFlowLimiter(service.address)).to.be.true; }; + it('Should revert an interchain token deployment with the minter as interchainTokenService', async () => { + const salt = keccak256('0x1245'); + await expectRevert( + (gasOptions) => tokenFactory.deployInterchainToken(salt, name, symbol, decimals, 0, service.address, gasOptions), + tokenFactory, + 'InvalidMinter', + [service.address], + ); + }); + it('Should register a token if the mint amount is zero', async () => { const salt = keccak256('0x1234'); tokenId = await tokenFactory.interchainTokenId(wallet.address, salt); diff --git a/test/InterchainTokenService.js b/test/InterchainTokenService.js index 2933f308..3cce96cf 100644 --- a/test/InterchainTokenService.js +++ b/test/InterchainTokenService.js @@ -705,19 +705,6 @@ describe('Interchain Token Service', () => { .withArgs(service.address, destinationChain, service.address, keccak256(payload), payload); }); - it('Should revert an interchain token deployment', async () => { - await expectRevert( - (gasOptions) => - service.deployInterchainToken(salt, '', tokenName, tokenSymbol, tokenDecimals, service.address, gasValue, { - ...gasOptions, - value: gasValue, - }), - service, - 'InvalidMinter', - [service.address], - ); - }); - it('Should revert on remote interchain token deployment if destination chain is not trusted', async () => { await expectRevert( (gasOptions) => From f3debcf2a6ededb05fda7ad73a46beb45947b531 Mon Sep 17 00:00:00 2001 From: ahramy Date: Sun, 8 Sep 2024 20:18:18 -0700 Subject: [PATCH 5/6] updated docstring --- contracts/InterchainTokenFactory.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index b35ff239..f182cda6 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -123,6 +123,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M * @param initialSupply The amount of tokens to mint initially (can be zero), allocated to the msg.sender. * @param minter The address to receive the minter and operator role of the token, in addition to ITS. If it is set to `address(0)`, * the additional minter isn't set, and can't be added later. This allows creating tokens that are managed only by ITS, reducing trust assumptions. + * Reverts if the minter is interchainTokenService address (invalid). * @return tokenId The tokenId corresponding to the deployed InterchainToken. */ function deployInterchainToken( @@ -166,7 +167,8 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M * @notice Deploys a remote interchain token on a specified destination chain. * @param originalChainName The name of the chain where the token originally exists. * @param salt The unique salt for deploying the token. - * @param minter The address to distribute the token on the destination chain. + * @param minter The address to distribute the token on the destination chain. If the address is zero, + * no additional minter is set on the token. Reverts if the minter is not authorized or interchainTokenService address (invalid). * @param destinationChain The name of the destination chain. * @param gasValue The amount of gas to send for the deployment. * @return tokenId The tokenId corresponding to the deployed InterchainToken. From 88b40e2176468fd8730b234fa1411ccc4cde53d5 Mon Sep 17 00:00:00 2001 From: ahramy Date: Sun, 8 Sep 2024 21:27:24 -0700 Subject: [PATCH 6/6] updated comments --- contracts/InterchainTokenFactory.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contracts/InterchainTokenFactory.sol b/contracts/InterchainTokenFactory.sol index f182cda6..cb726f76 100644 --- a/contracts/InterchainTokenFactory.sol +++ b/contracts/InterchainTokenFactory.sol @@ -123,7 +123,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M * @param initialSupply The amount of tokens to mint initially (can be zero), allocated to the msg.sender. * @param minter The address to receive the minter and operator role of the token, in addition to ITS. If it is set to `address(0)`, * the additional minter isn't set, and can't be added later. This allows creating tokens that are managed only by ITS, reducing trust assumptions. - * Reverts if the minter is interchainTokenService address (invalid). + * Reverts if the minter is the ITS address since it's already added as a minter. * @return tokenId The tokenId corresponding to the deployed InterchainToken. */ function deployInterchainToken( @@ -142,6 +142,7 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M minterBytes = address(this).toBytes(); } else if (minter != address(0)) { if (minter == address(interchainTokenService)) revert InvalidMinter(minter); + minterBytes = minter.toBytes(); } @@ -167,8 +168,8 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M * @notice Deploys a remote interchain token on a specified destination chain. * @param originalChainName The name of the chain where the token originally exists. * @param salt The unique salt for deploying the token. - * @param minter The address to distribute the token on the destination chain. If the address is zero, - * no additional minter is set on the token. Reverts if the minter is not authorized or interchainTokenService address (invalid). + * @param minter The address to receive the minter and operator role of the token, in addition to ITS. If the address is `address(0)`, + * no additional minter is set on the token. Reverts if the minter does not have mint permission for the token. * @param destinationChain The name of the destination chain. * @param gasValue The amount of gas to send for the deployment. * @return tokenId The tokenId corresponding to the deployed InterchainToken.