diff --git a/.changeset/short-geese-run.md b/.changeset/short-geese-run.md new file mode 100644 index 00000000..25c87c94 --- /dev/null +++ b/.changeset/short-geese-run.md @@ -0,0 +1,5 @@ +--- +'@axelar-network/interchain-token-service': patch +--- + +Don't use the smaller allowance for hedera diff --git a/contracts/TokenHandler.sol b/contracts/TokenHandler.sol index 0149e899..01fa1d34 100644 --- a/contracts/TokenHandler.sol +++ b/contracts/TokenHandler.sol @@ -44,9 +44,9 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Crea ) { _mintToken(ITokenManager(tokenManager), tokenAddress, to, amount); } else if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK)) { - _transferTokenFrom(tokenAddress, tokenManager, to, amount); + _transferTokenOut(tokenAddress, tokenManager, to, amount); } else if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { - amount = _transferTokenFromWithFee(tokenAddress, tokenManager, to, amount); + amount = _transferTokenOutWithFee(tokenAddress, tokenManager, to, amount); } else { revert UnsupportedTokenManagerType(tokenManagerType); } @@ -122,21 +122,34 @@ contract TokenHandler is ITokenHandler, ITokenManagerType, ReentrancyGuard, Crea /** * @notice This function prepares a token manager after it is deployed - * @param tokenManagerType The token manager type. - * @param tokenManager The address of the token manager. */ // slither-disable-next-line locked-ether - function postTokenManagerDeploy(uint256 tokenManagerType, ITokenManager tokenManager) external payable { - if (tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK) || tokenManagerType == uint256(TokenManagerType.LOCK_UNLOCK_FEE)) { - tokenManager.approveService(); - } - } + function postTokenManagerDeploy(uint256 /* tokenManagerType */, ITokenManager /* tokenManager */) external payable {} function _transferTokenFrom(address tokenAddress, address from, address to, uint256 amount) internal { // slither-disable-next-line arbitrary-send-erc20 IERC20(tokenAddress).safeTransferFrom(from, to, amount); } + function _transferTokenOut(address tokenAddress, address tokenManager, address to, uint256 amount) internal { + ITokenManager(tokenManager).transferTokenOut(tokenAddress, to, amount); + } + + function _transferTokenOutWithFee( + address tokenAddress, + address tokenManager, + address to, + uint256 amount + ) internal noReEntrancy returns (uint256) { + uint256 balanceBefore = IERC20(tokenAddress).balanceOf(to); + + _transferTokenOut(tokenAddress, tokenManager, to, amount); + + uint256 diff = IERC20(tokenAddress).balanceOf(to) - balanceBefore; + + return diff < amount ? diff : amount; + } + function _transferTokenFromWithFee( address tokenAddress, address from, diff --git a/contracts/interfaces/ITokenManager.sol b/contracts/interfaces/ITokenManager.sol index c4870609..5c6bb3fe 100644 --- a/contracts/interfaces/ITokenManager.sol +++ b/contracts/interfaces/ITokenManager.sol @@ -73,9 +73,13 @@ interface ITokenManager is IBaseTokenManager, IMinter, ITokenManagerType, IOpera function setFlowLimit(uint256 flowLimit_) external; /** - * @notice A function to renew approval to the service if we need to. + * @notice External function to allow the service to transfer tokens out of the token manager. + * @dev The token manager pushes its own balance, so this consumes no allowance from the service. + * @param tokenAddress_ The address of the token, since its cheaper to pass it in instead of reading it as the token manager. + * @param to The recipient. + * @param amount The amount to transfer out. */ - function approveService() external; + function transferTokenOut(address tokenAddress_, address to, uint256 amount) external; /** * @notice Getter function for the parameters of a lock/unlock TokenManager. diff --git a/contracts/token-manager/TokenManager.sol b/contracts/token-manager/TokenManager.sol index cc49a924..a2712b74 100644 --- a/contracts/token-manager/TokenManager.sol +++ b/contracts/token-manager/TokenManager.sol @@ -15,7 +15,7 @@ import { IERC20MintableBurnable } from '../interfaces/IERC20MintableBurnable.sol import { Operator } from '../utils/Operator.sol'; import { FlowLimit } from '../utils/FlowLimit.sol'; -import { HTS, IHederaTokenService } from '../hedera/HTS.sol'; +import { HTS } from '../hedera/HTS.sol'; import { Minter } from '../utils/Minter.sol'; /** @@ -26,9 +26,6 @@ contract TokenManager is ITokenManager, Minter, Operator, FlowLimit, Implementat using AddressBytes for bytes; using SafeTokenCall for IERC20; - uint256 internal constant UINT256_MAX = type(uint256).max; - uint256 internal constant INT64_MAX = uint256(uint64(type(int64).max)); - address public immutable interchainTokenService; bytes32 private constant CONTRACT_ID = keccak256('token-manager'); @@ -231,34 +228,18 @@ contract TokenManager is ITokenManager, Minter, Operator, FlowLimit, Implementat } /** - * @notice A function to renew approval to the service if we need to. + * @notice External function to allow the service to transfer tokens out of this token manager. + * @dev This token manager pushes its own balance, so no allowance to the service is consumed. + * @param tokenAddress_ The address of the token, since its cheaper to pass it in instead of reading it as the token manager. + * @param to The recipient. + * @param amount The amount to transfer out. */ - function approveService() external onlyService { - address tokenAddress_ = this.tokenAddress(); - bool isHTSToken = HTS.isToken(tokenAddress_); - uint256 amount; - if (isHTSToken) { - IHederaTokenService.FungibleTokenInfo memory info = HTS.getFungibleTokenInfo(tokenAddress_); - uint256 maxSupply = uint256(uint64(info.tokenInfo.token.maxSupply)); - - // If maxSupply is 0, the token has no max supply - // thus we approve the maximum value - if (maxSupply != 0 && maxSupply < INT64_MAX) { - amount = maxSupply; - } else { - amount = INT64_MAX; - } + function transferTokenOut(address tokenAddress_, address to, uint256 amount) external onlyService { + if (HTS.isToken(tokenAddress_)) { + HTS.transferToken(tokenAddress_, address(this), to, amount); } else { - amount = UINT256_MAX; + IERC20(tokenAddress_).safeCall(abi.encodeWithSelector(IERC20.transfer.selector, to, amount)); } - /** - * @dev Some tokens may not obey the infinite approval. - * Even so, it is unexpected to run out of allowance in practice. - * If needed, we can upgrade to allow replenishing the allowance in the future. - * - * @notice HTS tokens have a maximum supply of 2^63-1 (int64.max). - */ - IERC20(tokenAddress_).safeCall(abi.encodeWithSelector(IERC20.approve.selector, interchainTokenService, amount)); } /** diff --git a/test/TokenManager.js b/test/TokenManager.js index a384c897..8a933c88 100644 --- a/test/TokenManager.js +++ b/test/TokenManager.js @@ -58,8 +58,13 @@ describe('Token Manager', () => { await expectRevert((gasOptions) => TestTokenManager.addFlowOut(0, gasOptions), TestTokenManager, 'NotService', [owner.address]); }); - it('Should revert on approveService when calling directly', async () => { - await expectRevert((gasOptions) => TestTokenManager.approveService(gasOptions), TestTokenManager, 'NotService', [owner.address]); + it('Should revert on transferTokenOut when calling directly', async () => { + await expectRevert( + (gasOptions) => TestTokenManager.transferTokenOut(other.address, owner.address, 1234, gasOptions), + TestTokenManager, + 'NotService', + [owner.address], + ); }); it('Should revert on mintToken when calling directly', async () => {