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.

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

feat: added additional checks for call contract with token #249

Merged
merged 36 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
48f60e5
Added additional checks but code size is too large
Foivos Feb 6, 2024
fc8cb1a
Added value for contractCallWithToken and optimized contract size
Foivos Feb 7, 2024
ead719d
trying to fix tests
Foivos Feb 7, 2024
800ae3d
Update the create3address of ITS to use a custom bytecodehash.
Foivos Feb 9, 2024
cf016b0
prettier
Foivos Feb 12, 2024
ad53bc2
fix tests
Foivos Feb 12, 2024
7f610e7
Added a few tests
Foivos Feb 12, 2024
7a926fc
fixed one more test
Foivos Feb 12, 2024
83a396a
fixed all tests
Foivos Feb 13, 2024
c326d57
prettier
Foivos Feb 13, 2024
ca2bd91
made lint happy
Foivos Feb 13, 2024
1468d7e
working on slither
Foivos Feb 13, 2024
dccaf3c
made slither happy
Foivos Feb 14, 2024
927e1ce
prettier
Foivos Feb 14, 2024
1fc47a8
Using constant for the hash as well
Foivos Feb 14, 2024
f5823cc
addressed comments
Foivos Feb 14, 2024
c562c6a
added some tests
Foivos Feb 14, 2024
684fda9
Merge remote-tracking branch 'origin/main' into feat/additional-check…
Foivos Feb 15, 2024
f133cc9
added some coverage tests, found a bug too!
Foivos Feb 15, 2024
76a2545
a small style fix
Foivos Feb 16, 2024
74f09c6
fixed a bug
Foivos Feb 16, 2024
e896028
addressed some comments
Foivos Feb 20, 2024
b57cefc
prettier
Foivos Feb 20, 2024
5e311f2
fixed a test
Foivos Feb 20, 2024
f92443b
Merge branch 'main' into feat/additional-checks-for-execute-with-token
milapsheth Feb 21, 2024
0fcba29
remove modifier that should not exist
Foivos Feb 21, 2024
c5a5ba5
rename a function
Foivos Feb 21, 2024
dbc605d
Update contracts/InterchainTokenService.sol
Foivos Feb 21, 2024
b1c9266
reinteroduce the modifiers since they are needed after all
Foivos Feb 21, 2024
515be9c
Update contracts/utils/Create3AddressFixed.sol
milapsheth Feb 23, 2024
f787f13
add a docstring
Foivos Feb 23, 2024
10ed394
prettier and fixed tests
Foivos Mar 7, 2024
52c75e6
Merge remote-tracking branch 'origin/main' into feat/additional-check…
Foivos Mar 26, 2024
72fd642
Merge branch 'main' into feat/additional-checks-for-execute-with-token
milapsheth Apr 12, 2024
a2b9e47
address comments
milapsheth Apr 12, 2024
9d5c152
fix factory import
milapsheth Apr 12, 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
6 changes: 3 additions & 3 deletions contracts/InterchainTokenFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ contract InterchainTokenFactory is IInterchainTokenFactory, ITokenManagerType, M
* @param chainNameHash_ The hash of the chain name.
* @param deployer The address of the deployer.
* @param salt A unique identifier to generate the salt.
* @return bytes32 The calculated salt for the interchain token.
* @return tokenSalt The calculated salt for the interchain token.
*/
function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32) {
return keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt));
function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) public pure returns (bytes32 tokenSalt) {
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
tokenSalt = keccak256(abi.encode(PREFIX_INTERCHAIN_TOKEN_SALT, chainNameHash_, deployer, salt));
}

/**
Expand Down
142 changes: 111 additions & 31 deletions contracts/InterchainTokenService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { IAxelarGasService } from '@axelar-network/axelar-gmp-sdk-solidity/contr
import { IAxelarGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol';
import { ExpressExecutorTracker } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/express/ExpressExecutorTracker.sol';
import { Upgradable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Upgradable.sol';
import { Create3Address } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3Address.sol';
import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol';
import { Multicall } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Multicall.sol';
import { Pausable } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/Pausable.sol';
Expand All @@ -22,6 +21,7 @@ import { IInterchainTokenExecutable } from './interfaces/IInterchainTokenExecuta
import { IInterchainTokenExpressExecutable } from './interfaces/IInterchainTokenExpressExecutable.sol';
import { ITokenManager } from './interfaces/ITokenManager.sol';
import { IERC20Named } from './interfaces/IERC20Named.sol';
import { Create3AddressFixed } from './utils/Create3AddressFixed.sol';
milapsheth marked this conversation as resolved.
Show resolved Hide resolved

import { Operator } from './utils/Operator.sol';

Expand All @@ -37,7 +37,7 @@ contract InterchainTokenService is
Operator,
Pausable,
Multicall,
Create3Address,
Create3AddressFixed,
ExpressExecutorTracker,
InterchainAddressTracker,
IInterchainTokenService
Expand Down Expand Up @@ -343,13 +343,7 @@ contract InterchainTokenService is
string calldata sourceAddress,
bytes calldata payload
) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) {
(uint256 messageType, bytes32 tokenId, , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, uint256));

if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
revert InvalidExpressMessageType(messageType);
}

return (validTokenAddress(tokenId), amount);
return _contractCallValue(payload);
}

/**
Expand Down Expand Up @@ -626,32 +620,59 @@ contract InterchainTokenService is
* @param sourceAddress The address of the remote ITS where the transaction originates from.
* @param payload The encoded data payload for the transaction.
*/
function execute(bytes32 commandId, string calldata sourceChain, string calldata sourceAddress, bytes calldata payload) public {
function execute(
bytes32 commandId,
string calldata sourceChain,
string calldata sourceAddress,
bytes calldata payload
) external onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
bytes32 payloadHash = keccak256(payload);

if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash)) revert NotApprovedByGateway();

_execute(commandId, sourceChain, sourceAddress, payload, payloadHash);
}

/**
* @notice Returns the amount of token that this call is worth.
* @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address.
* @param sourceChain The source chain.
* @param sourceAddress The source address on the source chain.
* @param payload The payload sent with the call.
* @param symbol The symbol symbol for the call.
* @param amount The amount for the call.
* @return address The token address.
* @return uint256 The value the call is worth.
*/
function contractCallWithTokenValue(
string calldata /*sourceChain*/,
string calldata /*sourceAddress*/,
bytes calldata /*payload*/,
string calldata /*symbol*/,
uint256 /*amount*/
) public view virtual returns (address, uint256) {
revert ExecuteWithTokenNotSupported();
string calldata sourceChain,
string calldata sourceAddress,
bytes calldata payload,
string calldata symbol,
uint256 amount
) public view virtual onlyRemoteService(sourceChain, sourceAddress) whenNotPaused returns (address, uint256) {
_checkPayloadAgainstGatewayData(payload, symbol, amount);
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
return _contractCallValue(payload);
}

/**
* @notice Express executes with a gateway token operations based on the payload and selector.
* @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.
* @param payload The encoded data payload for the transaction.
* @param tokenSymbol The symbol symbol for the call.
* @param amount The amount for the call.
*/
function expressExecuteWithToken(
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
bytes32 commandId,
string calldata sourceChain,
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
string calldata sourceAddress,
bytes calldata payload,
string calldata /*tokenSymbol*/,
uint256 /*amount*/
string calldata tokenSymbol,
uint256 amount
) external payable {
_checkPayloadAgainstGatewayData(payload, tokenSymbol, amount);
// It should be ok to ignore the symbol and amount since this info exists on the payload.
expressExecute(commandId, sourceChain, sourceAddress, payload);
}
Expand All @@ -663,13 +684,22 @@ contract InterchainTokenService is
bytes calldata payload,
string calldata tokenSymbol,
uint256 amount
) external {
bytes32 payloadHash = keccak256(payload);
) external onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
_executeWithToken(commandId, sourceChain, sourceAddress, payload, tokenSymbol, amount);
}

if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount))
revert NotApprovedByGateway();
/**
* @notice Check that the tokenId from the payload is a token that is registered in the gateway with the proper tokenSymbol, with the right amount from the payload.
* Also check that the amount in the payload matches the one for the call.
* @param payload The payload for the call contract with token.
* @param tokenSymbol The tokenSymbol for the call contract with token.
* @param amount The amount for the call contract with token.
*/
function _checkPayloadAgainstGatewayData(bytes calldata payload, string calldata tokenSymbol, uint256 amount) internal view {
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
(, bytes32 tokenId, , , uint256 amountInPayload) = abi.decode(payload, (uint256, bytes32, uint256, uint256, uint256));

_execute(commandId, sourceChain, sourceAddress, payload, payloadHash);
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
if (validTokenAddress(tokenId) != gateway.tokenAddresses(tokenSymbol) || amount != amountInPayload)
revert InvalidGatewayTokenTransfer(tokenId, payload, tokenSymbol, amount);
}

/**
Expand Down Expand Up @@ -857,15 +887,10 @@ contract InterchainTokenService is
string calldata sourceAddress,
bytes calldata payload,
bytes32 payloadHash
) internal onlyRemoteService(sourceChain, sourceAddress) whenNotPaused {
) internal {
uint256 messageType = abi.decode(payload, (uint256));
if (messageType == MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
address expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash);

if (expressExecutor != address(0)) {
emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor);
}

address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash);
_processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload);
} else if (messageType == MESSAGE_TYPE_DEPLOY_TOKEN_MANAGER) {
_processDeployTokenManagerPayload(payload);
Expand All @@ -876,6 +901,32 @@ contract InterchainTokenService is
}
}

function _executeWithToken(
bytes32 commandId,
string calldata sourceChain,
string calldata sourceAddress,
bytes calldata payload,
string calldata tokenSymbol,
uint256 amount
) internal {
bytes32 payloadHash = keccak256(payload);

if (!gateway.validateContractCallAndMint(commandId, sourceChain, sourceAddress, payloadHash, tokenSymbol, amount))
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
revert NotApprovedByGateway();

uint256 messageType = abi.decode(payload, (uint256));
if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
revert InvalidMessageType(messageType);
}

_checkPayloadAgainstGatewayData(payload, tokenSymbol, amount);

// slither-disable-next-line reentrancy-events
address expressExecutor = _getExpressExecutorAndEmitEvent(commandId, sourceChain, sourceAddress, payloadHash);

_processInterchainTransferPayload(commandId, expressExecutor, sourceChain, payload);
}

/**
* @notice Deploys a token manager on a destination chain.
* @param tokenId The ID of the token.
Expand Down Expand Up @@ -1114,4 +1165,33 @@ contract InterchainTokenService is

return (amount, tokenAddress);
}

/**
* @notice Returns the amount of token that this call is worth.
* @dev If `tokenAddress` is `0`, then value is in terms of the native token, otherwise it's in terms of the token address.
* @param payload The payload sent with the call.
* @return address The token address.
* @return uint256 The value the call is worth.
*/
function _contractCallValue(bytes calldata payload) internal view returns (address, uint256) {
(uint256 messageType, bytes32 tokenId, , , uint256 amount) = abi.decode(payload, (uint256, bytes32, bytes, bytes, uint256));
if (messageType != MESSAGE_TYPE_INTERCHAIN_TRANSFER) {
revert InvalidExpressMessageType(messageType);
}

return (validTokenAddress(tokenId), amount);
}

function _getExpressExecutorAndEmitEvent(
bytes32 commandId,
string calldata sourceChain,
string calldata sourceAddress,
bytes32 payloadHash
) internal returns (address expressExecutor) {
expressExecutor = _popExpressExecutor(commandId, sourceChain, sourceAddress, payloadHash);

if (expressExecutor != address(0)) {
emit ExpressExecutionFulfilled(commandId, sourceChain, sourceAddress, payloadHash, expressExecutor);
}
}
}
8 changes: 4 additions & 4 deletions contracts/interfaces/IInterchainTokenFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
* @param chainNameHash_ The hash of the chain name.
* @param deployer The address of the deployer.
* @param salt A unique identifier to generate the salt.
* @return bytes32 The calculated salt for the interchain token.
* @return tokenSalt The calculated salt for the interchain token.
*/
function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32);
function interchainTokenSalt(bytes32 chainNameHash_, address deployer, bytes32 salt) external view returns (bytes32 tokenSalt);

/**
* @notice Computes the ID for an interchain token based on the deployer and a salt.
Expand Down Expand Up @@ -97,9 +97,9 @@ interface IInterchainTokenFactory is IUpgradable, IMulticall {
* @notice Calculates the salt for a canonical interchain token.
* @param chainNameHash_ The hash of the chain name.
* @param tokenAddress The address of the token.
* @return salt The calculated salt for the interchain token.
* @return tokenSalt The calculated salt for the interchain token.
*/
function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 salt);
function canonicalInterchainTokenSalt(bytes32 chainNameHash_, address tokenAddress) external view returns (bytes32 tokenSalt);

/**
* @notice Computes the ID for a canonical interchain token based on its address.
Expand Down
1 change: 1 addition & 0 deletions contracts/interfaces/IInterchainTokenService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ interface IInterchainTokenService is
error EmptyData();
error PostDeployFailed(bytes data);
error ZeroAmount();
error InvalidGatewayTokenTransfer(bytes32 tokenId, bytes payload, string tokenSymbol, uint256 amount);

event InterchainTransfer(
bytes32 indexed tokenId,
Expand Down
19 changes: 19 additions & 0 deletions contracts/test/utils/TestCreate3Fixed.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import { Create3Fixed } from '../../utils/Create3Fixed.sol';

contract TestCreate3Fixed is Create3Fixed {
event Deployed(address addr);

function deploy(bytes memory code, bytes32 salt) public payable returns (address addr) {
addr = _create3(code, salt);

emit Deployed(addr);
}

function deployedAddress(bytes32 salt) public view returns (address addr) {
addr = _create3Address(salt);
}
}
29 changes: 29 additions & 0 deletions contracts/utils/Create3AddressFixed.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

/**
* @title Create3AddressFixed contract
* @notice This contract can be used to predict the deterministic deployment address of a contract deployed with the `CREATE3` technique.
* It is equivalent to the Create3Address found in axelar-gmp-sdk-solidity repo but uses a fixed bytecode for CreateDeploy,
* which allows changing compilation options (like number of runs) without affecting the future deployment addresses.
*/
contract Create3AddressFixed {
// slither-disable-next-line too-many-digits
bytes internal constant CREATE_DEPLOY_BYTECODE =
hex'608060405234801561001057600080fd5b50610162806100206000396000f3fe60806040526004361061001d5760003560e01c806277436014610022575b600080fd5b61003561003036600461007b565b610037565b005b8051602082016000f061004957600080fd5b50565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b60006020828403121561008d57600080fd5b813567ffffffffffffffff808211156100a557600080fd5b818401915084601f8301126100b957600080fd5b8135818111156100cb576100cb61004c565b604051601f8201601f19908116603f011681019083821181831017156100f3576100f361004c565b8160405282815287602084870101111561010c57600080fd5b82602086016020830137600092810160200192909252509594505050505056fea264697066735822122094780ce55d28f1d568f4e0ab1b9dc230b96e952b73d2e06456fbff2289fa27f464736f6c63430008150033';
bytes32 internal constant CREATE_DEPLOY_BYTECODE_HASH = keccak256(CREATE_DEPLOY_BYTECODE);

/**
* @notice Compute the deployed address that will result from the `CREATE3` method.
* @param deploySalt A salt to influence the contract address
* @return deployed The deterministic contract address if it was deployed
*/
function _create3Address(bytes32 deploySalt) internal view returns (address deployed) {
address deployer = address(
uint160(uint256(keccak256(abi.encodePacked(hex'ff', address(this), deploySalt, CREATE_DEPLOY_BYTECODE_HASH))))
);

deployed = address(uint160(uint256(keccak256(abi.encodePacked(hex'd6_94', deployer, hex'01')))));
}
}
46 changes: 46 additions & 0 deletions contracts/utils/Create3Fixed.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import { IDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IDeploy.sol';
import { ContractAddress } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/ContractAddress.sol';
import { CreateDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/CreateDeploy.sol';
import { Create3AddressFixed } from './Create3AddressFixed.sol';

/**
* @title Create3Fixed contract
* @notice This contract can be used to deploy a contract with a deterministic address that depends only on
* the deployer address and deployment salt, not the contract bytecode and constructor parameters.
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
* It uses a fixed bytecode to allow changing the compilation settings without affecting the deployment address in the future.
*/
contract Create3Fixed is Create3AddressFixed, IDeploy {
using ContractAddress for address;

/**
* @notice Deploys a new contract using the `CREATE3` method.
* @dev This function first deploys the CreateDeploy contract using
* the `CREATE2` opcode and then utilizes the CreateDeploy to deploy the
* new contract with the `CREATE` opcode.
* @param bytecode The bytecode of the contract to be deployed
* @param deploySalt A salt to influence the contract address
* @return deployed The address of the deployed contract
*/
function _create3(bytes memory bytecode, bytes32 deploySalt) internal returns (address deployed) {
deployed = _create3Address(deploySalt);

if (bytecode.length == 0) revert EmptyBytecode();
if (deployed.isContract()) revert AlreadyDeployed();

// Deploy using create2
CreateDeploy createDeploy;
bytes memory createDeployBytecode_ = CREATE_DEPLOY_BYTECODE;
uint256 length = createDeployBytecode_.length;
assembly {
createDeploy := create2(0, add(createDeployBytecode_, 0x20), length, deploySalt)
}

if (address(createDeploy) == address(0)) revert DeployFailed();
// Deploy using create
createDeploy.deploy(bytecode);
}
}
4 changes: 2 additions & 2 deletions contracts/utils/InterchainTokenDeployer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pragma solidity ^0.8.0;

import { Create3 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3.sol';
import { Create3Fixed } from './Create3Fixed.sol';

import { IInterchainTokenDeployer } from '../interfaces/IInterchainTokenDeployer.sol';
import { IInterchainToken } from '../interfaces/IInterchainToken.sol';
Expand All @@ -11,7 +11,7 @@ import { IInterchainToken } from '../interfaces/IInterchainToken.sol';
* @title InterchainTokenDeployer
* @notice This contract is used to deploy new instances of the InterchainTokenProxy contract.
*/
contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3 {
contract InterchainTokenDeployer is IInterchainTokenDeployer, Create3Fixed {
address public immutable implementationAddress;

/**
Expand Down
Loading
Loading