Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: preventing original chain name & destination chain name are the same #277

Merged
merged 12 commits into from
Sep 27, 2024
Merged

fix: preventing original chain name & destination chain name are the same #277

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
855ab1e
fix: preventing original chain name and destination chan name are the…
ahramy Sep 10, 2024
baceae4
updated
ahramy Sep 10, 2024
06b1847
updated
ahramy Sep 16, 2024
ae08d80
removed unnecessary error
ahramy Sep 16, 2024
f3f8f98
updated
ahramy Sep 18, 2024
a100d8c
updated
ahramy Sep 18, 2024
6252141
updated
ahramy Sep 18, 2024
ecdddde
updated interface
ahramy Sep 18, 2024
69bda6a
updated
ahramy Sep 25, 2024
4cef49f
Merge branch 'main' into fix/check_chain_name
milapsheth Sep 27, 2024
2e4de66
updated
ahramy Sep 27, 2024
759c5fb
Merge branch 'fix/check_chain_name' of https://github.com/axelarnetwo…
ahramy Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 18 additions & 67 deletions contracts/InterchainTokenFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ 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 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.
Expand All @@ -175,27 +174,19 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
* @return tokenId The tokenId corresponding to the deployed InterchainToken.
*/
function deployRemoteInterchainToken(
string calldata originalChainName,
bytes32 salt,
address minter,
string memory destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId) {
) public payable returns (bytes32 tokenId) {
string memory tokenName;
string memory tokenSymbol;
uint8 tokenDecimals;
bytes memory minter_ = new bytes(0);

{
ahramy marked this conversation as resolved.
Show resolved Hide resolved
bytes32 chainNameHash_;
if (bytes(originalChainName).length == 0) {
chainNameHash_ = chainNameHash;
} else {
chainNameHash_ = keccak256(bytes(originalChainName));
}

address sender = msg.sender;
salt = interchainTokenSalt(chainNameHash_, sender, salt);
salt = interchainTokenSalt(chainNameHash, sender, salt);
tokenId = interchainTokenService.interchainTokenId(TOKEN_FACTORY_DEPLOYER, salt);

IInterchainToken token = IInterchainToken(interchainTokenService.interchainTokenAddress(tokenId));
Expand All @@ -216,8 +207,10 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
}

/**
* @notice The deployRemoteInterchainToken with originalChainName allows deploying a remote interchain token back to the same chain, which could cause issues with ITS Hub balance tracking.
* To fix this, the deployRemoteInterchainToken function was overloaded to prevent self-deployemnt while still allowing self-transfers.
* @notice Deploys a remote interchain token on a specified destination chain.
ahramy marked this conversation as resolved.
Show resolved Hide resolved
* @dev originalChainName is only allowed to be '', i.e the current chain.
* Other source chains are not supported anymore to simplify ITS deployment behaviour.
ahramy marked this conversation as resolved.
Show resolved Hide resolved
* @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 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.
Expand All @@ -226,36 +219,14 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
* @return tokenId The tokenId corresponding to the deployed InterchainToken.
*/
function deployRemoteInterchainToken(
string calldata originalChainName,
bytes32 salt,
address minter,
string memory destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId) {
string memory tokenName;
string memory tokenSymbol;
uint8 tokenDecimals;
bytes memory minter_ = new bytes(0);

{
address sender = msg.sender;
salt = interchainTokenSalt(chainNameHash, sender, salt);
tokenId = interchainTokenService.interchainTokenId(TOKEN_FACTORY_DEPLOYER, salt);

IInterchainToken token = IInterchainToken(interchainTokenService.interchainTokenAddress(tokenId));

tokenName = token.name();
tokenSymbol = token.symbol();
tokenDecimals = token.decimals();

if (minter != address(0)) {
if (!token.isMinter(minter)) revert NotMinter(minter);
if (minter == address(interchainTokenService)) revert InvalidMinter(minter);

minter_ = minter.toBytes();
}
}

tokenId = _deployInterchainToken(salt, destinationChain, tokenName, tokenSymbol, tokenDecimals, minter_, gasValue);
if (bytes(originalChainName).length != 0) revert NotSupported();
tokenId = deployRemoteInterchainToken(salt, minter, destinationChain, gasValue);
ahramy marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -306,31 +277,22 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M

/**
* @notice Deploys a canonical interchain token on a remote chain.
* @param originalChain The name of the chain where the token originally exists.
* @param originalTokenAddress The address of the original token on the original chain.
* @param destinationChain The name of the chain where the token will be deployed.
* @param gasValue The gas amount to be sent for deployment.
* @return tokenId The tokenId corresponding to the deployed InterchainToken.
*/
function deployRemoteCanonicalInterchainToken(
string calldata originalChain,
address originalTokenAddress,
string calldata destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId) {
) public payable returns (bytes32 tokenId) {
bytes32 salt;
IInterchainToken token;

{
ahramy marked this conversation as resolved.
Show resolved Hide resolved
bytes32 chainNameHash_;
if (bytes(originalChain).length == 0) {
chainNameHash_ = chainNameHash;
} else {
chainNameHash_ = keccak256(bytes(originalChain));
}

// This ensures that the token manager has been deployed by this address, so it's safe to trust it.
salt = canonicalInterchainTokenSalt(chainNameHash_, originalTokenAddress);
salt = canonicalInterchainTokenSalt(chainNameHash, originalTokenAddress);
tokenId = interchainTokenService.interchainTokenId(TOKEN_FACTORY_DEPLOYER, salt);
token = IInterchainToken(interchainTokenService.validTokenAddress(tokenId));
}
Expand All @@ -344,34 +306,23 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
}

/**
* @notice The deployRemoteCanonicalInterchainToken with originalChain allows deploying a canonical interchain token back to the same chain, which could cause issues with ITS Hub balance tracking.
* To fix this, the deployRemoteCanonicalInterchainToken function was overloaded to prevent self-deployemnt while still allowing self-transfers.
* @notice Deploys a canonical interchain token on a remote chain.
ahramy marked this conversation as resolved.
Show resolved Hide resolved
* @dev originalChain is only allowed to be '', i.e the current chain.
* Other source chains are not supported anymore to simplify ITS deployment behaviour.
* @param originalChain The name of the chain where the token originally exists.
* @param originalTokenAddress The address of the original token on the original chain.
* @param destinationChain The name of the chain where the token will be deployed.
* @param gasValue The gas amount to be sent for deployment.
* @return tokenId The tokenId corresponding to the deployed InterchainToken.
*/
function deployRemoteCanonicalInterchainToken(
ahramy marked this conversation as resolved.
Show resolved Hide resolved
string calldata originalChain,
address originalTokenAddress,
string calldata destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId) {
bytes32 salt;
IInterchainToken token;

{
// This ensures that the token manager has been deployed by this address, so it's safe to trust it.
salt = canonicalInterchainTokenSalt(chainNameHash, originalTokenAddress);
tokenId = interchainTokenService.interchainTokenId(TOKEN_FACTORY_DEPLOYER, salt);
token = IInterchainToken(interchainTokenService.validTokenAddress(tokenId));
}

// The 3 lines below will revert if the token does not exist.
string memory tokenName = token.name();
string memory tokenSymbol = token.symbol();
uint8 tokenDecimals = token.decimals();

tokenId = _deployInterchainToken(salt, destinationChain, tokenName, tokenSymbol, tokenDecimals, '', gasValue);
if (bytes(originalChain).length != 0) revert NotSupported();
tokenId = deployRemoteCanonicalInterchainToken(originalTokenAddress, destinationChain, gasValue);
}

/**
Expand Down
19 changes: 11 additions & 8 deletions contracts/interfaces/IInterchainTokenFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
error GatewayToken(address tokenAddress);
error NotServiceOwner(address sender);
error NotGatewayToken(string symbol);
error NotSupported();

/**
* @notice Returns the address of the interchain token service.
Expand Down Expand Up @@ -79,31 +80,32 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {

/**
* @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 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.
*/
function deployRemoteInterchainToken(
string calldata originalChainName,
bytes32 salt,
address minter,
string memory destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId);

/**
* @notice The deployRemoteInterchainToken with originalChainName allows deploying a remote interchain token back to the same chain, which could cause issues with ITS Hub balance tracking.
* To fix this, the deployRemoteInterchainToken function was overloaded to prevent self-deployemnt while still allowing self-transfers.
* @notice Deploys a remote interchain token on a specified destination chain.
* @dev originalChainName is only allowed to be '', i.e the current chain.
* Other source chains are not supported anymore to simplify ITS deployment behaviour.
* @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 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.
*/
function deployRemoteInterchainToken(
string calldata originalChainName,
bytes32 salt,
address minter,
string memory destinationChain,
Expand Down Expand Up @@ -134,28 +136,29 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {

/**
* @notice Deploys a canonical interchain token on a remote chain.
* @param originalChain The name of the chain where the token originally exists.
* @param originalTokenAddress The address of the original token on the original chain.
* @param destinationChain The name of the chain where the token will be deployed.
* @param gasValue The gas amount to be sent for deployment.
* @return tokenId The tokenId corresponding to the deployed canonical InterchainToken.
*/
function deployRemoteCanonicalInterchainToken(
string calldata originalChain,
address originalTokenAddress,
string calldata destinationChain,
uint256 gasValue
) external payable returns (bytes32 tokenId);

/**
* @notice The deployRemoteCanonicalInterchainToken with originalChain allows deploying a canonical interchain token back to the same chain, which could cause issues with ITS Hub balance tracking.
* To fix this, the deployRemoteCanonicalInterchainToken function was overloaded to prevent self-deployemnt while still allowing self-transfers.
* @notice Deploys a canonical interchain token on a remote chain.
* @dev originalChain is only allowed to be '', i.e the current chain.
* Other source chains are not supported anymore to simplify ITS deployment behaviour.
* @param originalChain The name of the chain where the token originally exists.
* @param originalTokenAddress The address of the original token on the original chain.
* @param destinationChain The name of the chain where the token will be deployed.
* @param gasValue The gas amount to be sent for deployment.
* @return tokenId The tokenId corresponding to the deployed canonical InterchainToken.
*/
function deployRemoteCanonicalInterchainToken(
string calldata originalChain,
address originalTokenAddress,
string calldata destinationChain,
uint256 gasValue
Expand Down
89 changes: 33 additions & 56 deletions test/InterchainTokenFactory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,23 +126,20 @@ describe('InterchainTokenFactory', () => {
[MESSAGE_TYPE_DEPLOY_INTERCHAIN_TOKEN, tokenId, name, symbol, decimals, '0x'],
);

await expect(
tokenFactory['deployRemoteCanonicalInterchainToken(string,address,string,uint256)'](
chainName,
token.address,
destinationChain,
gasValue,
{
value: gasValue,
},
),
)
.to.emit(service, 'InterchainTokenDeploymentStarted')
.withArgs(tokenId, name, symbol, decimals, '0x', destinationChain)
.and.to.emit(gasService, 'NativeGasPaidForContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), gasValue, wallet.address)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), payload);
await expectRevert(
(gasOptions) =>
tokenFactory['deployRemoteCanonicalInterchainToken(string,address,string,uint256)'](
chainName,
token.address,
destinationChain,
gasValue,
{
value: gasValue,
},
),
tokenFactory,
'NotSupported',
);

await expect(
tokenFactory['deployRemoteCanonicalInterchainToken(address,string,uint256)'](token.address, destinationChain, gasValue, {
Expand Down Expand Up @@ -449,14 +446,31 @@ describe('InterchainTokenFactory', () => {
},
),
tokenFactory,
'NotSupported',
);

await expectRevert(
(gasOptions) =>
tokenFactory['deployRemoteInterchainToken(string,bytes32,address,string,uint256)'](
'',
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
salt,
otherWallet.address,
destinationChain,
gasValue,
{
...gasOptions,
value: gasValue,
},
),
tokenFactory,
'NotMinter',
[otherWallet.address],
);

await expectRevert(
(gasOptions) =>
tokenFactory['deployRemoteInterchainToken(string,bytes32,address,string,uint256)'](
chainName,
'',
salt,
service.address,
destinationChain,
Expand Down Expand Up @@ -490,25 +504,6 @@ describe('InterchainTokenFactory', () => {
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), payload);

await expect(
tokenFactory['deployRemoteInterchainToken(string,bytes32,address,string,uint256)'](
chainName,
salt,
wallet.address,
destinationChain,
gasValue,
{
value: gasValue,
},
),
)
.to.emit(service, 'InterchainTokenDeploymentStarted')
.withArgs(tokenId, name, symbol, decimals, wallet.address.toLowerCase(), destinationChain)
.and.to.emit(gasService, 'NativeGasPaidForContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), gasValue, wallet.address)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), payload);

await expectRevert(
(gasOptions) =>
tokenFactory['deployRemoteInterchainToken(bytes32,address,string,uint256)'](
Expand Down Expand Up @@ -560,24 +555,6 @@ describe('InterchainTokenFactory', () => {
.withArgs(service.address, destinationChain, service.address, keccak256(payload), gasValue, wallet.address)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), payload);

await expect(
tokenFactory['deployRemoteInterchainToken(bytes32,address,string,uint256)'](
salt,
wallet.address,
destinationChain,
gasValue,
{
value: gasValue,
},
),
)
.to.emit(service, 'InterchainTokenDeploymentStarted')
.withArgs(tokenId, name, symbol, decimals, wallet.address.toLowerCase(), destinationChain)
.and.to.emit(gasService, 'NativeGasPaidForContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), gasValue, wallet.address)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, keccak256(payload), payload);
});

it('Should initiate a remote interchain token deployment without the same minter', async () => {
Expand Down Expand Up @@ -617,7 +594,7 @@ describe('InterchainTokenFactory', () => {

await expect(
tokenFactory['deployRemoteInterchainToken(string,bytes32,address,string,uint256)'](
chainName,
'',
salt,
AddressZero,
destinationChain,
Expand Down
Loading
Loading