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

Jump to bottom

fix: address audit comments #234

Merged
merged 2 commits into from
Dec 15, 2023
Merged

fix: address audit comments #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89fd09a
build: bump cgp and gmp sdk dep
milapsheth Dec 15, 2023
1476bda
fix: address audit comments
milapsheth Dec 15, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/publish-to-npm.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: 'publish to npm'
name: 'Publish to npm'

on:
workflow_dispatch:
Expand Down
1 change: 1 addition & 0 deletions contracts/interchain-token/InterchainToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ contract InterchainToken is InterchainTokenStandard, ERC20, ERC20Permit, Minter,

if (tokenId_ == bytes32(0)) revert TokenIdZero();
if (bytes(tokenName).length == 0) revert TokenNameEmpty();
if (bytes(tokenSymbol).length == 0) revert TokenSymbolEmpty();

name = tokenName;
symbol = tokenSymbol;
Expand Down
1 change: 1 addition & 0 deletions contracts/interfaces/IInterchainToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface IInterchainToken is IInterchainTokenStandard, IMinter, IERC20MintableB
error InterchainTokenServiceAddressZero();
error TokenIdZero();
error TokenNameEmpty();
error TokenSymbolEmpty();
error AlreadyInitialized();

/**
Expand Down
2 changes: 2 additions & 0 deletions contracts/utils/InterchainTokenDeployer.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3 {
string calldata symbol,
uint8 decimals
) external returns (address tokenAddress) {
// Use a minimal proxy for cheap token deployment and auto-verification on explorers
// https://eips.ethereum.org/EIPS/eip-1167
// The minimal proxy bytecode is the same as https://github.com/OpenZeppelin/openzeppelin-contracts/blob/94697be8a3f0dfcd95dfb13ffbd39b5973f5c65d/contracts/proxy/Clones.sol#L28
// The minimal proxy bytecode is 0x37 = 55 bytes long
bytes memory bytecode = new bytes(0x37);
Expand Down
34 changes: 13 additions & 21 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@axelar-network/interchain-token-service",
"version": "1.0.0",
"version": "1.1.0",
"repository": {
"type": "git",
"url": "https://github.com/axelarnetwork/interchain-token-service"
Expand Down Expand Up @@ -29,11 +29,11 @@
"node": ">=16"
},
"dependencies": {
"@axelar-network/axelar-cgp-solidity": "6.2.0",
"@axelar-network/axelar-gmp-sdk-solidity": "5.6.3"
"@axelar-network/axelar-cgp-solidity": "6.2.1",
"@axelar-network/axelar-gmp-sdk-solidity": "5.6.4"
},
"devDependencies": {
"@axelar-network/axelar-chains-config": "~0.1.2",
"@axelar-network/axelar-chains-config": "^1.0.0",
"@axelarjs/evm": "^0.2.1",
"@nomicfoundation/hardhat-toolbox": "^2.0.2",
"@tsconfig/strictest": "^2.0.2",
Expand Down
21 changes: 13 additions & 8 deletions test/InterchainToken.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,37 @@ describe('InterchainToken', () => {
});

it('revert on init if tokenId is 0', async () => {
const implementationAddress = await interchainTokenDeployer.implementationAddress();
const implementation = await getContractAt('InterchainToken', implementationAddress, owner);

const salt = getRandomBytes32();
const minter = owner.address;
await expectRevert(
(gasOptions) => interchainTokenDeployer.deployInterchainToken(salt, HashZero, minter, name, symbol, decimals, gasOptions),
implementation,
interchainToken,
'TokenIdZero',
);
});

it('revert on init if token name is invalid', async () => {
const implementationAddress = await interchainTokenDeployer.implementationAddress();
const implementation = await getContractAt('InterchainToken', implementationAddress, owner);

const salt = getRandomBytes32();
const tokenId = getRandomBytes32();
const minter = owner.address;
await expectRevert(
(gasOptions) => interchainTokenDeployer.deployInterchainToken(salt, tokenId, minter, '', symbol, decimals, gasOptions),
implementation,
interchainToken,
'TokenNameEmpty',
);
});

it('revert on init if token symbol is invalid', async () => {
const salt = getRandomBytes32();
const tokenId = getRandomBytes32();
const minter = owner.address;
await expectRevert(
(gasOptions) => interchainTokenDeployer.deployInterchainToken(salt, tokenId, minter, name, '', decimals, gasOptions),
interchainToken,
'TokenSymbolEmpty',
);
});

it('should subtract from the spender allowance', async () => {
tokenTest = await deployContract(owner, 'TestInterchainToken', []);

Expand Down
55 changes: 14 additions & 41 deletions test/InterchainTokenService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('Interchain Token Service', () => {
const deployFunctions = {};
const destinationChain = 'destination chain';
const sourceChain = 'source chain';
const gasValue = 12;

deployFunctions.lockUnlock = async function deployNewLockUnlock(
tokenName,
Expand Down Expand Up @@ -376,7 +377,7 @@ describe('Interchain Token Service', () => {
);
});

it('Should revert on invalid token manager', async () => {
it('Should revert on invalid token manager implementation', async () => {
await expectRevert(
(gasOptions) =>
deployInterchainTokenService(
Expand Down Expand Up @@ -422,29 +423,6 @@ describe('Interchain Token Service', () => {
);
});

it('Should revert on invalid token manager', async () => {
await expectRevert(
(gasOptions) =>
deployInterchainTokenService(
wallet,
create3Deployer.address,
tokenManagerDeployer.address,
interchainTokenDeployer.address,
gateway.address,
gasService.address,
interchainTokenFactoryAddress,
tokenManager.address,
AddressZero,
chainName,
[],
deploymentKey,
gasOptions,
),
service,
'ZeroAddress',
);
});

it('Should return the token manager implementation', async () => {
const tokenManagerImplementation = await service.tokenManagerImplementation(getRandomInt(1000));
expect(tokenManagerImplementation).to.eq(tokenManager.address);
Expand Down Expand Up @@ -656,7 +634,6 @@ describe('Interchain Token Service', () => {
const tokenSymbol = 'TN';
const tokenDecimals = 13;
const minter = '0x12345678';
const gasValue = 1234;
let salt;

before(async () => {
Expand Down Expand Up @@ -813,15 +790,19 @@ describe('Interchain Token Service', () => {
]);
});

it('Should revert on deploying an invalid token manager', async () => {
const params = defaultAbiCoder.encode(['bytes', 'address'], [wallet.address, token.address]);

await expectRevert((gasOptions) => service.deployTokenManager(salt, '', 5, params, 0, gasOptions));
});

it('Should deploy a lock/unlock token manager', async () => {
const tokenManagerAddress = await service.tokenManagerAddress(tokenId);
const params = defaultAbiCoder.encode(['bytes', 'address'], [wallet.address, token.address]);

const expectedTokenManagerAddress = await service.tokenManagerAddress(tokenId);

await expect(reportGas(service.deployTokenManager(salt, '', LOCK_UNLOCK, params, 0), 'Call deployTokenManager on source chain'))
.to.emit(service, 'TokenManagerDeployed')
.withArgs(tokenId, expectedTokenManagerAddress, LOCK_UNLOCK, params);
.withArgs(tokenId, tokenManagerAddress, LOCK_UNLOCK, params);

expect(tokenManagerAddress).to.not.equal(AddressZero);
const tokenManager = await getContractAt('TokenManager', tokenManagerAddress, wallet);
Expand Down Expand Up @@ -1003,7 +984,6 @@ describe('Interchain Token Service', () => {
).wait();

const tokenId = await service.interchainTokenId(wallet.address, salt);
const gasValue = 1e6;
const params = '0x1234';
const type = LOCK_UNLOCK;
const payload = defaultAbiCoder.encode(
Expand Down Expand Up @@ -1034,7 +1014,6 @@ describe('Interchain Token Service', () => {
it('Should revert on a remote custom token manager deployment if the token manager does does not exist', async () => {
const salt = getRandomBytes32();
const tokenId = await service.interchainTokenId(wallet.address, salt);
const gasValue = 1e6;
const params = '0x1234';
const type = LOCK_UNLOCK;

Expand All @@ -1047,7 +1026,6 @@ describe('Interchain Token Service', () => {
await service.setPauseStatus(true).then((tx) => tx.wait());

const salt = getRandomBytes32();
const gasValue = 1e6;
const params = '0x1234';
const type = LOCK_UNLOCK;

Expand Down Expand Up @@ -1134,7 +1112,6 @@ describe('Interchain Token Service', () => {
describe('Send Token', () => {
const amount = 1234;
const destAddress = '0x5678';
const gasValue = 90;
let token, tokenId;

before(async () => {
Expand Down Expand Up @@ -1372,7 +1349,6 @@ describe('Interchain Token Service', () => {
const amount = 1234;
const destAddress = '0x5678';
const data = '0x1234';
const gasValue = 90;
let sourceAddress;
let token, tokenManager, tokenId;

Expand Down Expand Up @@ -1408,11 +1384,6 @@ describe('Interchain Token Service', () => {
const payloadHash = keccak256(payload);

const metadataExpress = '0x00000001';
const payloadExpress = defaultAbiCoder.encode(
['uint256', 'bytes32', 'bytes', 'bytes', 'uint256', 'bytes'],
[MESSAGE_TYPE_INTERCHAIN_TRANSFER, tokenId, sourceAddress, destAddress, sendAmount, '0x'],
);
const payloadHashExpress = keccak256(payloadExpress);

let transferToAddress = AddressZero;

Expand All @@ -1435,6 +1406,8 @@ describe('Interchain Token Service', () => {
.withArgs(wallet.address, transferToAddress, amount)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, payloadHash, payload)
.and.to.emit(gasService, 'NativeGasPaidForContractCall')
.withArgs(service.address, destinationChain, service.address, payloadHash, gasValue, wallet.address)
.to.emit(service, 'InterchainTransfer')
.withArgs(tokenId, sourceAddress, destinationChain, destAddress, sendAmount, HashZero);

Expand All @@ -1449,7 +1422,9 @@ describe('Interchain Token Service', () => {
.to.emit(token, 'Transfer')
.withArgs(wallet.address, transferToAddress, amount)
.and.to.emit(gateway, 'ContractCall')
.withArgs(service.address, destinationChain, service.address, payloadHashExpress, payloadExpress)
.withArgs(service.address, destinationChain, service.address, payloadHash, payload)
.and.to.emit(gasService, 'NativeGasPaidForExpressCall')
.withArgs(service.address, destinationChain, service.address, payloadHash, gasValue, wallet.address)
.to.emit(service, 'InterchainTransfer')
.withArgs(tokenId, sourceAddress, destinationChain, destAddress, sendAmount, HashZero);
});
Expand Down Expand Up @@ -1763,7 +1738,6 @@ describe('Interchain Token Service', () => {
describe('Send Interchain Token', () => {
const amount = 1234;
const destAddress = '0x5678';
const gasValue = 90;
const metadata = '0x';
let token, tokenManager, tokenId;

Expand Down Expand Up @@ -1852,7 +1826,6 @@ describe('Interchain Token Service', () => {
describe('Send Interchain Token With Data', () => {
const amount = 1234;
const destAddress = '0x5678';
const gasValue = 90;
let sourceAddress;
const data = '0x1234';

Expand Down
2 changes: 1 addition & 1 deletion test/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const getGasOptions = () => {
};

const expectRevert = async (txFunc, contract, error, args) => {
if (network.config.skipRevertTests) {
if (network.config.skipRevertTests || contract === undefined) {
await expect(txFunc(getGasOptions())).to.be.reverted;
} else {
if (args) {
Expand Down
Loading