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

refactor: inter-chain-token service events and errors [AXE-2064] #125

Merged
merged 29 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4f98632
refactor: contracts and interfaces for better error and event names, …
blockchainguyy Oct 12, 2023
c7e59a3
fix: tests for changes in errors and events
blockchainguyy Oct 12, 2023
cdd43d4
refactor: test/utils.js for expecting custom args with revert if prov…
blockchainguyy Oct 12, 2023
8fa38c0
refactor: update index.md for changes in errors and events
blockchainguyy Oct 12, 2023
5a47f53
chore: resolve conflicts with main
blockchainguyy Oct 16, 2023
f4ae099
chore: add operator address in event FlowLimitSet
blockchainguyy Oct 16, 2023
8c40a4d
chore: refactor error argument names
blockchainguyy Oct 16, 2023
df97235
Added roles, tests pending
Foivos Oct 17, 2023
8ea68ca
fix: tests
Oct 19, 2023
afdb98d
Merge remote-tracking branch 'origin/main' into feat/roles
Oct 19, 2023
fc0d72b
Added some tests and fixed constants
Foivos Oct 19, 2023
4b0586d
made lint happy
Foivos Oct 19, 2023
8c28d6d
Chnaged roles constants to enum to make slither happy
Foivos Oct 19, 2023
3ef1db2
Fixed tests
Foivos Oct 20, 2023
d2eeb46
Merge branch 'feat/roles' into refactor/errors-and-events
blockchainguyy Oct 25, 2023
53dc382
chore: also check error arguments in tests while reverting
blockchainguyy Oct 25, 2023
704fa25
chore: resolve pr comments for including tokenId in some errors
blockchainguyy Oct 25, 2023
6c4e38f
Added getter for distributor and opearator.
Foivos Oct 25, 2023
51a3ce4
made lint happy
Foivos Oct 25, 2023
68fcbdd
fixed a spelling error
Foivos Oct 25, 2023
d5ac83e
chore: add tokenManager address in deployment event and update tests
blockchainguyy Oct 25, 2023
6a27685
chore: add deployed token address in event
blockchainguyy Oct 26, 2023
3e55e98
chore: fix slither failing pipeline by adding comment
blockchainguyy Oct 26, 2023
d2db7e5
chore: resolve pr comments by adding params in erorrs and events, ad…
blockchainguyy Oct 26, 2023
fe2af9f
Merge remote-tracking branch 'origin/main' into feat/roles
Foivos Oct 27, 2023
479606e
Merge branch 'feat/roles' into refactor/errors-and-events
blockchainguyy Oct 31, 2023
7b5482b
fix: test after merging branch feat/roles
blockchainguyy Oct 31, 2023
5ab456c
Merge branch 'main' into refactor/errors-and-events
blockchainguyy Nov 1, 2023
3fcb1ad
fix: tests after resolving merge conflicts with main
blockchainguyy Nov 1, 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
4 changes: 2 additions & 2 deletions contracts/examples/InterchainTokenExecutable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
import { IInterchainTokenExecutable } from '../interfaces/IInterchainTokenExecutable.sol';

abstract contract InterchainTokenExecutable is IInterchainTokenExecutable {
error NotService();
error NotService(address caller);

address public immutable interchainTokenService;

Expand All @@ -14,7 +14,7 @@ abstract contract InterchainTokenExecutable is IInterchainTokenExecutable {
}

modifier onlyService() {
if (msg.sender != interchainTokenService) revert NotService();
if (msg.sender != interchainTokenService) revert NotService(msg.sender);
_;
}

Expand Down
54 changes: 36 additions & 18 deletions contracts/interchain-token-service/InterchainTokenService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ contract InterchainTokenService is
* @param tokenId the `tokenId` of the TokenManager trying to perform the call.
*/
modifier onlyTokenManager(bytes32 tokenId) {
if (msg.sender != getTokenManagerAddress(tokenId)) revert NotTokenManager();
address tokenManager = getTokenManagerAddress(tokenId);
if (msg.sender != tokenManager) revert NotTokenManager(msg.sender, tokenManager);

_;
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -278,10 +280,14 @@ contract InterchainTokenService is
* @dev `gasValue` exists because this function can be part of a multicall involving multiple functions that could make remote contract calls.
*/
function deployRemoteCanonicalToken(bytes32 tokenId, string calldata destinationChain, uint256 gasValue) public payable notPaused {
address tokenAddress = getValidTokenManagerAddress(tokenId);
tokenAddress = ITokenManager(tokenAddress).tokenAddress();
address tokenAddress;
{
tokenAddress = getValidTokenManagerAddress(tokenId);
tokenAddress = ITokenManager(tokenAddress).tokenAddress();
bytes32 canonicalTokenId = getCanonicalTokenId(tokenAddress);

if (getCanonicalTokenId(tokenAddress) != tokenId) revert NotCanonicalTokenManager();
if (canonicalTokenId != tokenId) revert InvalidCanonicalTokenId(canonicalTokenId);
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved
}

(string memory tokenName, string memory tokenSymbol, uint8 tokenDecimals) = _validateToken(tokenAddress);
_deployRemoteStandardizedToken(tokenId, tokenName, tokenSymbol, tokenDecimals, '', '', 0, '', destinationChain, gasValue);
Expand Down Expand Up @@ -433,7 +439,7 @@ contract InterchainTokenService is
amount
);
} else if (selector != SELECTOR_RECEIVE_TOKEN) {
revert InvalidExpressSelector();
revert InvalidExpressSelector(selector);
}
}

Expand Down Expand Up @@ -495,7 +501,7 @@ contract InterchainTokenService is
* @param tokenIds an array of the token Ids of the tokenManagers to set the flow limit of.
* @param flowLimits the flowLimits to set
*/
function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
function setFlowLimits(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyRole(uint8(Roles.OPERATOR)) {
uint256 length = tokenIds.length;
if (length != flowLimits.length) revert LengthMismatch();
for (uint256 i; i < length; ++i) {
Expand All @@ -518,7 +524,7 @@ contract InterchainTokenService is
\****************/

function _setup(bytes calldata params) internal override {
_setOperator(params.toAddress());
_addOperator(params.toAddress());
}

function _sanitizeTokenManagerImplementation(
Expand All @@ -527,7 +533,8 @@ contract InterchainTokenService is
) internal pure returns (address implementation_) {
implementation_ = tokenManagerImplementations[uint256(tokenManagerType)];
if (implementation_ == address(0)) revert ZeroAddress();
if (ITokenManager(implementation_).implementationType() != uint256(tokenManagerType)) revert InvalidTokenManagerImplementation();
if (ITokenManager(implementation_).implementationType() != uint256(tokenManagerType))
revert InvalidTokenManagerImplementationType(implementation_);
}

/**
Expand All @@ -552,7 +559,7 @@ contract InterchainTokenService is
if (selector == SELECTOR_DEPLOY_TOKEN_MANAGER) return _processDeployTokenManagerPayload(payload);
if (selector == SELECTOR_DEPLOY_AND_REGISTER_STANDARDIZED_TOKEN) return _processDeployStandardizedTokenAndManagerPayload(payload);

revert SelectorUnknown();
revert SelectorUnknown(selector);
}

function executeWithToken(
Expand Down Expand Up @@ -780,14 +787,19 @@ contract InterchainTokenService is
* @param params Additional parameters for the token manager deployment
*/
function _deployTokenManager(bytes32 tokenId, TokenManagerType tokenManagerType, bytes memory params) internal {
// slither-disable-next-line reentrancy-events
emit TokenManagerDeployed(tokenId, tokenManagerType, params);

// slither-disable-next-line controlled-delegatecall
(bool success, ) = tokenManagerDeployer.delegatecall(
(bool success, bytes memory returnData) = tokenManagerDeployer.delegatecall(
abi.encodeWithSelector(ITokenManagerDeployer.deployTokenManager.selector, tokenId, tokenManagerType, params)
);
if (!success) revert TokenManagerDeploymentFailed();
if (!success) revert TokenManagerDeploymentFailed(returnData);

address tokenManager;
assembly {
tokenManager := mload(add(returnData, 0x20))
}

// slither-disable-next-line reentrancy-events
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved
emit TokenManagerDeployed(tokenId, tokenManager, tokenManagerType, params);
}

/**
Expand Down Expand Up @@ -818,13 +830,11 @@ contract InterchainTokenService is
uint256 mintAmount,
address mintTo
) internal {
emit StandardizedTokenDeployed(tokenId, distributor, name, symbol, decimals, mintAmount, mintTo);

bytes32 salt = _getStandardizedTokenSalt(tokenId);
address tokenManagerAddress = getTokenManagerAddress(tokenId);

// slither-disable-next-line controlled-delegatecall
(bool success, ) = standardizedTokenDeployer.delegatecall(
(bool success, bytes memory returnData) = standardizedTokenDeployer.delegatecall(
abi.encodeWithSelector(
IStandardizedTokenDeployer.deployStandardizedToken.selector,
salt,
Expand All @@ -838,8 +848,16 @@ contract InterchainTokenService is
)
);
if (!success) {
revert StandardizedTokenDeploymentFailed();
revert StandardizedTokenDeploymentFailed(returnData);
}

address tokenAddress;
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved
assembly {
tokenAddress := mload(add(returnData, 0x20))
}

// slither-disable-next-line reentrancy-events
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved
emit StandardizedTokenDeployed(tokenId, tokenAddress, distributor, name, symbol, decimals, mintAmount, mintTo);
}

function _decodeMetadata(bytes memory metadata) internal pure returns (uint32 version, bytes memory data) {
Expand Down
20 changes: 7 additions & 13 deletions contracts/interfaces/IDistributable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,6 @@
pragma solidity ^0.8.0;

interface IDistributable {
error NotDistributor();
error NotProposedDistributor();

event DistributorshipTransferred(address indexed distributor);
event DistributorshipTransferStarted(address indexed distributor);

/**
* @notice Get the address of the distributor
* @return distributor_ of the distributor
*/
function distributor() external view returns (address distributor_);

/**
* @notice Change the distributor of the contract
* @dev Can only be called by the current distributor
Expand All @@ -33,5 +21,11 @@ interface IDistributable {
* @notice Accept a change of the distributor of the contract
* @dev Can only be called by the proposed distributor
*/
function acceptDistributorship() external;
function acceptDistributorship(address fromDistributor) external;

/**
* @notice Query if an address is a distributor
* @param addr the address to query for
*/
function isDistributor(address addr) external view returns (bool);
}
3 changes: 1 addition & 2 deletions contracts/interfaces/IExpressCallHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
pragma solidity ^0.8.0;

interface IExpressCallHandler {
error AlreadyExpressCalled();
error SameDestinationAsCaller();
error AlreadyExpressCalled(address prevExpressCaller, address expressCaller);

event ExpressReceive(bytes payload, bytes32 indexed sendHash, address indexed expressCaller);
event ExpressExecutionFulfilled(bytes payload, bytes32 indexed sendHash, address indexed expressCaller);
Expand Down
4 changes: 2 additions & 2 deletions contracts/interfaces/IFlowLimit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
pragma solidity ^0.8.0;

interface IFlowLimit {
error FlowLimitExceeded();
error FlowLimitExceeded(uint256 limit, uint256 flowAmount);
blockchainguyy marked this conversation as resolved.
Show resolved Hide resolved

event FlowLimitSet(uint256 flowLimit);
event FlowLimitSet(bytes32 indexed tokenId, address operator, uint256 flowLimit);

/**
* @notice Returns the current flow limit
Expand Down
18 changes: 9 additions & 9 deletions contracts/interfaces/IInterchainTokenService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,20 @@ import { IMulticall } from './IMulticall.sol';
interface IInterchainTokenService is ITokenManagerType, IExpressCallHandler, IAxelarExecutable, IPausable, IMulticall, IContractIdentifier {
error ZeroAddress();
error LengthMismatch();
error InvalidTokenManagerImplementation();
error InvalidTokenManagerImplementationType(address implementation);
error NotRemoteService();
error TokenManagerDoesNotExist(bytes32 tokenId);
error NotTokenManager();
error NotTokenManager(address caller, address tokenManager);
error ExecuteWithInterchainTokenFailed(address contractAddress);
error NotCanonicalTokenManager();
error InvalidCanonicalTokenId(bytes32 expectedCanonicalTokenId);
error GatewayToken();
error TokenManagerDeploymentFailed();
error StandardizedTokenDeploymentFailed();
error DoesNotAcceptExpressExecute(address contractAddress);
error SelectorUnknown();
error TokenManagerDeploymentFailed(bytes error);
error StandardizedTokenDeploymentFailed(bytes error);
error SelectorUnknown(uint256 selector);
error InvalidMetadataVersion(uint32 version);
error AlreadyExecuted(bytes32 commandId);
error ExecuteWithTokenNotSupported();
error InvalidExpressSelector();
error InvalidExpressSelector(uint256 selector);

event TokenSent(bytes32 indexed tokenId, string destinationChain, bytes destinationAddress, uint256 indexed amount);
event TokenSentWithData(
Expand Down Expand Up @@ -66,9 +65,10 @@ interface IInterchainTokenService is ITokenManagerType, IExpressCallHandler, IAx
string destinationChain,
uint256 indexed gasValue
);
event TokenManagerDeployed(bytes32 indexed tokenId, TokenManagerType indexed tokenManagerType, bytes params);
event TokenManagerDeployed(bytes32 indexed tokenId, address tokenManager, TokenManagerType indexed tokenManagerType, bytes params);
event StandardizedTokenDeployed(
bytes32 indexed tokenId,
address tokenAddress,
address indexed distributor,
string name,
string symbol,
Expand Down
20 changes: 7 additions & 13 deletions contracts/interfaces/IOperatable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,6 @@
pragma solidity ^0.8.0;

interface IOperatable {
error NotOperator();
error NotProposedOperator();

event OperatorshipTransferred(address indexed operator);
event OperatorChangeProposed(address indexed operator);

/**
* @notice Get the address of the operator
* @return operator_ of the operator
*/
function operator() external view returns (address operator_);

/**
* @notice Change the operator of the contract
* @dev Can only be called by the current operator
Expand All @@ -33,5 +21,11 @@ interface IOperatable {
* @notice Accept a proposed change of operatorship
* @dev Can only be called by the proposed operator
*/
function acceptOperatorship() external;
function acceptOperatorship(address fromOperator) external;

/**
* @notice Query if an address is a operator
* @param addr the address to query for
*/
function isOperator(address addr) external view returns (bool);
}
3 changes: 2 additions & 1 deletion contracts/interfaces/IStandardizedTokenDeployer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ interface IStandardizedTokenDeployer {
* @param decimals Decimals of the token
* @param mintAmount Amount of tokens to mint initially
* @param mintTo Address to mint initial tokens to
* @return tokenAddress Address of the deployed token
*/
function deployStandardizedToken(
bytes32 salt,
Expand All @@ -43,5 +44,5 @@ interface IStandardizedTokenDeployer {
uint8 decimals,
uint256 mintAmount,
address mintTo
) external payable;
) external payable returns (address tokenAddress);
}
7 changes: 5 additions & 2 deletions contracts/interfaces/ITokenManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ import { IImplementation } from './IImplementation.sol';
*/
interface ITokenManager is ITokenManagerType, IOperatable, IFlowLimit, IImplementation {
error TokenLinkerZeroAddress();
error NotService();
error NotService(address caller);
error TakeTokenFailed();
error GiveTokenFailed();
error NotToken();
error NotToken(address caller);
error ZeroAddress();
error AlreadyFlowLimiter(address flowLimiter);
error NotFlowLimiter(address flowLimiter);

/**
* @notice A function that returns the token id.
Expand Down
7 changes: 6 additions & 1 deletion contracts/interfaces/ITokenManagerDeployer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ interface ITokenManagerDeployer {
* @param tokenId The unique identifier for the token
* @param implementationType Token manager implementation type
* @param params Additional parameters used in the setup of the token manager
* @return tokenManager Address of the deployed tokenManager
*/
function deployTokenManager(bytes32 tokenId, uint256 implementationType, bytes calldata params) external payable;
function deployTokenManager(
bytes32 tokenId,
uint256 implementationType,
bytes calldata params
) external payable returns (address tokenManager);
}
2 changes: 1 addition & 1 deletion contracts/interfaces/ITokenManagerProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pragma solidity ^0.8.0;
*/
interface ITokenManagerProxy {
error ImplementationLookupFailed();
error SetupFailed();
error SetupFailed(bytes returnData);
error NativeTokenNotAccepted();

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/proxies/TokenManagerProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ contract TokenManagerProxy is ITokenManagerProxy {
tokenId = tokenId_;
address impl = _getImplementation(IInterchainTokenService(interchainTokenServiceAddress_), implementationType_);

(bool success, ) = impl.delegatecall(abi.encodeWithSelector(TokenManagerProxy.setup.selector, params));
if (!success) revert SetupFailed();
(bool success, bytes memory returnData) = impl.delegatecall(abi.encodeWithSelector(TokenManagerProxy.setup.selector, params));
if (!success) revert SetupFailed(returnData);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions contracts/test/FeeOnTransferTokenTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract FeeOnTransferTokenTest is InterchainToken, Distributable, IERC20Mintabl
name = name_;
symbol = symbol_;
decimals = decimals_;
_setDistributor(msg.sender);
_addDistributor(msg.sender);
tokenManager_ = ITokenManager(tokenManagerAddress);
}

Expand Down Expand Up @@ -50,11 +50,11 @@ contract FeeOnTransferTokenTest is InterchainToken, Distributable, IERC20Mintabl
tokenManagerRequiresApproval_ = requiresApproval;
}

function mint(address account, uint256 amount) external onlyDistributor {
function mint(address account, uint256 amount) external onlyRole(uint8(Roles.DISTRIBUTOR)) {
_mint(account, amount);
}

function burn(address account, uint256 amount) external onlyDistributor {
function burn(address account, uint256 amount) external onlyRole(uint8(Roles.DISTRIBUTOR)) {
_burn(account, amount);
}

Expand Down
10 changes: 2 additions & 8 deletions contracts/test/HardCodedConstantsTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ contract TestTokenManager is TokenManagerLiquidityPool {
contract TestDistributable is Distributable {
string public constant NAME = 'TestDistributable';

constructor() {
require(DISTRIBUTOR_SLOT == uint256(keccak256('distributor')) - 1, 'invalid constant');
require(PROPOSED_DISTRIBUTOR_SLOT == uint256(keccak256('proposed-distributor')) - 1, 'invalid constant');
}
constructor() {}
}

contract TestFlowLimit is FlowLimit {
Expand All @@ -46,10 +43,7 @@ contract TestNoReEntrancy is NoReEntrancy {
contract TestOperatable is Operatable {
string public constant NAME = 'TestOperatable';

constructor() {
require(OPERATOR_SLOT == uint256(keccak256('operator')) - 1, 'invalid constant');
require(PROPOSED_OPERATOR_SLOT == uint256(keccak256('proposed-operator')) - 1, 'invalid constant');
}
constructor() {}
}

contract TestPausable is Pausable {
Expand Down
Loading
Loading