Skip to content

Commit

Permalink
fix(BaseTokenManager): ReentrancyGuard for all managers
Browse files Browse the repository at this point in the history
  • Loading branch information
re1ro committed Nov 5, 2023
1 parent 237907e commit b49ef61
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity ^0.8.0;
import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol';
import { IImplementation } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IImplementation.sol';
import { Implementation } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/upgradable/Implementation.sol';
import { ReentrancyGuard } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/ReentrancyGuard.sol';

import { ITokenManager } from '../interfaces/ITokenManager.sol';
import { IInterchainTokenService } from '../interfaces/IInterchainTokenService.sol';
Expand All @@ -16,7 +17,7 @@ import { FlowLimit } from '../utils/FlowLimit.sol';
* @title The main functionality of TokenManagers.
* @notice This contract is responsible for handling tokens before initiating a cross chain token transfer, or after receiving one.
*/
abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implementation {
abstract contract BaseTokenManager is ITokenManager, Operatable, FlowLimit, ReentrancyGuard, Implementation {
using AddressBytes for bytes;

IInterchainTokenService public immutable interchainTokenService;
Expand Down Expand Up @@ -203,7 +204,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen
* @param amount the amount of token to give.
* @return the amount of token actually given, which will only be different than `amount` in cases where the token takes some on-transfer fee.
*/
function giveToken(address destinationAddress, uint256 amount) external onlyService returns (uint256) {
function giveToken(address destinationAddress, uint256 amount) external onlyService noReEntrancy returns (uint256) {
// rate limit the incoming amount from source
_addFlowIn(amount);
amount = _giveToken(destinationAddress, amount);
Expand All @@ -216,7 +217,7 @@ abstract contract TokenManager is ITokenManager, Operatable, FlowLimit, Implemen
* @param amount the amount of token to give.
* @return the amount of token actually given, which will onle be differen than `amount` in cases where the token takes some on-transfer fee.
*/
function takeToken(address sourceAddress, uint256 amount) external onlyService returns (uint256) {
function takeToken(address sourceAddress, uint256 amount) external onlyService noReEntrancy returns (uint256) {
amount = _takeToken(sourceAddress, amount);
// rate limit the outgoing amount to destination
_addFlowOut(amount);
Expand Down
11 changes: 5 additions & 6 deletions contracts/token-manager/TokenManagerLiquidityPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ pragma solidity ^0.8.0;

import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol';
import { SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/SafeTransfer.sol';
import { ReentrancyGuard } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/ReentrancyGuard.sol';

import { ITokenManagerLiquidityPool } from '../interfaces/ITokenManagerLiquidityPool.sol';
import { TokenManager } from './TokenManager.sol';
import { BaseTokenManager } from './BaseTokenManager.sol';

/**
* @title TokenManagerLiquidityPool
Expand All @@ -16,7 +15,7 @@ import { TokenManager } from './TokenManager.sol';
* @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods.
* It uses the Axelar SDK to safely transfer tokens.
*/
contract TokenManagerLiquidityPool is TokenManager, ReentrancyGuard, ITokenManagerLiquidityPool {
contract TokenManagerLiquidityPool is BaseTokenManager, ITokenManagerLiquidityPool {
using SafeTokenTransferFrom for IERC20;

error NotSupported();
Expand All @@ -29,7 +28,7 @@ contract TokenManagerLiquidityPool is TokenManager, ReentrancyGuard, ITokenManag
* of TokenManagerAddressStorage which calls the constructor of TokenManager.
* @param interchainTokenService_ The address of the interchain token service contract
*/
constructor(address interchainTokenService_) TokenManager(interchainTokenService_) {}
constructor(address interchainTokenService_) BaseTokenManager(interchainTokenService_) {}

function implementationType() external pure returns (uint256) {
revert NotSupported();
Expand Down Expand Up @@ -80,7 +79,7 @@ contract TokenManagerLiquidityPool is TokenManager, ReentrancyGuard, ITokenManag
* @param amount The amount of tokens to transfer
* @return uint The actual amount of tokens transferred. This allows support for fee-on-transfer tokens.
*/
function _takeToken(address from, uint256 amount) internal override noReEntrancy returns (uint256) {
function _takeToken(address from, uint256 amount) internal override returns (uint256) {
IERC20 token = IERC20(tokenAddress());
address liquidityPool_ = liquidityPool();
uint256 balance = token.balanceOf(liquidityPool_);
Expand All @@ -100,7 +99,7 @@ contract TokenManagerLiquidityPool is TokenManager, ReentrancyGuard, ITokenManag
* @param amount The amount of tokens to transfer
* @return uint The actual amount of tokens transferred
*/
function _giveToken(address to, uint256 amount) internal override noReEntrancy returns (uint256) {
function _giveToken(address to, uint256 amount) internal override returns (uint256) {
IERC20 token = IERC20(tokenAddress());
uint256 balance = token.balanceOf(to);

Expand Down
6 changes: 3 additions & 3 deletions contracts/token-manager/TokenManagerLockUnlock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interf
import { SafeTokenTransfer, SafeTokenTransferFrom } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/SafeTransfer.sol';

import { ITokenManagerLockUnlock } from '..//interfaces/ITokenManagerLockUnlock.sol';
import { TokenManager } from './TokenManager.sol';
import { BaseTokenManager } from './BaseTokenManager.sol';

/**
* @title TokenManagerLockUnlock
* @notice This contract is an implementation of TokenManager that locks and unlocks a specific token on behalf of the interchain token service.
* @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods.
* It uses the Axelar SDK to safely transfer tokens.
*/
contract TokenManagerLockUnlock is TokenManager, ITokenManagerLockUnlock {
contract TokenManagerLockUnlock is BaseTokenManager, ITokenManagerLockUnlock {
using SafeTokenTransfer for IERC20;
using SafeTokenTransferFrom for IERC20;

Expand All @@ -23,7 +23,7 @@ contract TokenManagerLockUnlock is TokenManager, ITokenManagerLockUnlock {
* of TokenManagerAddressStorage which calls the constructor of TokenManager.
* @param interchainTokenService_ The address of the interchain token service contract
*/
constructor(address interchainTokenService_) TokenManager(interchainTokenService_) {}
constructor(address interchainTokenService_) BaseTokenManager(interchainTokenService_) {}

function implementationType() external pure returns (uint256) {
return uint256(TokenManagerType.LOCK_UNLOCK);
Expand Down
11 changes: 5 additions & 6 deletions contracts/token-manager/TokenManagerLockUnlockFee.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@ pragma solidity ^0.8.0;

import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol';
import { SafeTokenTransferFrom, SafeTokenTransfer } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/SafeTransfer.sol';
import { ReentrancyGuard } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/utils/ReentrancyGuard.sol';

import { ITokenManagerLockUnlock } from '../interfaces/ITokenManagerLockUnlock.sol';
import { TokenManager } from './TokenManager.sol';
import { BaseTokenManager } from './BaseTokenManager.sol';

/**
* @title TokenManagerLockUnlock
* @notice This contract is an implementation of TokenManager that locks and unlocks a specific token on behalf of the interchain token service.
* @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods.
* It uses the Axelar SDK to safely transfer tokens.
*/
contract TokenManagerLockUnlockFee is TokenManager, ReentrancyGuard, ITokenManagerLockUnlock {
contract TokenManagerLockUnlockFee is BaseTokenManager, ITokenManagerLockUnlock {
using SafeTokenTransfer for IERC20;
using SafeTokenTransferFrom for IERC20;

Expand All @@ -24,7 +23,7 @@ contract TokenManagerLockUnlockFee is TokenManager, ReentrancyGuard, ITokenManag
* of TokenManagerAddressStorage which calls the constructor of TokenManager.
* @param interchainTokenService_ The address of the interchain token service contract
*/
constructor(address interchainTokenService_) TokenManager(interchainTokenService_) {}
constructor(address interchainTokenService_) BaseTokenManager(interchainTokenService_) {}

function implementationType() external pure returns (uint256) {
return uint256(TokenManagerType.LOCK_UNLOCK_FEE);
Expand All @@ -46,7 +45,7 @@ contract TokenManagerLockUnlockFee is TokenManager, ReentrancyGuard, ITokenManag
* @param amount The amount of tokens to transfer
* @return uint The actual amount of tokens transferred. This allows support for fee-on-transfer tokens.
*/
function _takeToken(address from, uint256 amount) internal override noReEntrancy returns (uint256) {
function _takeToken(address from, uint256 amount) internal override returns (uint256) {
IERC20 token = IERC20(tokenAddress());
uint256 balanceBefore = token.balanceOf(address(this));

Expand All @@ -66,7 +65,7 @@ contract TokenManagerLockUnlockFee is TokenManager, ReentrancyGuard, ITokenManag
* @param amount The amount of tokens to transfer
* @return uint The actual amount of tokens transferred
*/
function _giveToken(address to, uint256 amount) internal override noReEntrancy returns (uint256) {
function _giveToken(address to, uint256 amount) internal override returns (uint256) {
IERC20 token = IERC20(tokenAddress());
uint256 balanceBefore = token.balanceOf(to);

Expand Down
6 changes: 3 additions & 3 deletions contracts/token-manager/TokenManagerMintBurn.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
import { IERC20 } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IERC20.sol';
import { SafeTokenCall } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/SafeTransfer.sol';

import { TokenManager } from './TokenManager.sol';
import { BaseTokenManager } from './BaseTokenManager.sol';
import { IERC20MintableBurnable } from '../interfaces/IERC20MintableBurnable.sol';
import { ITokenManagerMintBurn } from '../interfaces/ITokenManagerMintBurn.sol';

Expand All @@ -15,15 +15,15 @@ import { ITokenManagerMintBurn } from '../interfaces/ITokenManagerMintBurn.sol';
* @dev This contract extends TokenManagerAddressStorage and provides implementation for its abstract methods.
* It uses the Axelar SDK to safely transfer tokens.
*/
contract TokenManagerMintBurn is TokenManager, ITokenManagerMintBurn {
contract TokenManagerMintBurn is BaseTokenManager, ITokenManagerMintBurn {
using SafeTokenCall for IERC20;

/**
* @dev Constructs an instance of TokenManagerMintBurn. Calls the constructor
* of TokenManagerAddressStorage which calls the constructor of TokenManager.
* @param interchainTokenService_ The address of the interchain token service contract
*/
constructor(address interchainTokenService_) TokenManager(interchainTokenService_) {}
constructor(address interchainTokenService_) BaseTokenManager(interchainTokenService_) {}

function implementationType() external pure virtual returns (uint256) {
return uint256(TokenManagerType.MINT_BURN);
Expand Down
12 changes: 5 additions & 7 deletions test/InterchainTokenFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ describe('InterchainTokenFactory', () => {
tokenId = await tokenFactory.interchainTokenId(wallet.address, salt);
const tokenAddress = await tokenFactory.interchainTokenAddress(wallet.address, salt);
const params = defaultAbiCoder.encode(['bytes', 'address'], [tokenFactory.address, tokenAddress]);
const tokenManager = await getContractAt('TokenManager', await service.tokenManagerAddress(tokenId), wallet);
const tokenManager = await getContractAt('BaseTokenManager', await service.tokenManagerAddress(tokenId), wallet);
const token = await getContractAt('InterchainToken', tokenAddress, wallet);

await expect(tokenFactory.deployInterchainToken(salt, name, symbol, decimals, mintAmount, distributor))
Expand Down Expand Up @@ -220,8 +220,8 @@ describe('InterchainTokenFactory', () => {
const salt = keccak256('0x12');
tokenId = await tokenFactory.interchainTokenId(wallet.address, salt);
const tokenAddress = await tokenFactory.interchainTokenAddress(wallet.address, salt);
let params = defaultAbiCoder.encode(['bytes', 'address'], [tokenFactory.address, tokenAddress]);
const tokenManager = await getContractAt('TokenManager', await service.tokenManagerAddress(tokenId), wallet);
const params = defaultAbiCoder.encode(['bytes', 'address'], [tokenFactory.address, tokenAddress]);
const tokenManager = await getContractAt('BaseTokenManager', await service.tokenManagerAddress(tokenId), wallet);
const token = await getContractAt('InterchainToken', tokenAddress, wallet);

await expect(tokenFactory.deployInterchainToken(salt, name, symbol, decimals, mintAmount, wallet.address))
Expand All @@ -244,7 +244,6 @@ describe('InterchainTokenFactory', () => {
.and.to.emit(tokenManager, 'RolesRemoved')
.withArgs(tokenFactory.address, 1 << FLOW_LIMITER_ROLE);

params = defaultAbiCoder.encode(['bytes', 'address'], ['0x', token.address]);
const payload = defaultAbiCoder.encode(
['uint256', 'bytes32', 'string', 'string', 'uint8', 'bytes'],
[MESSAGE_TYPE_DEPLOY_INTERCHAIN_TOKEN, tokenId, name, symbol, decimals, wallet.address.toLowerCase()],
Expand All @@ -269,8 +268,8 @@ describe('InterchainTokenFactory', () => {
const salt = keccak256('0x1245');
tokenId = await tokenFactory.interchainTokenId(wallet.address, salt);
const tokenAddress = await tokenFactory.interchainTokenAddress(wallet.address, salt);
let params = defaultAbiCoder.encode(['bytes', 'address'], [tokenFactory.address, tokenAddress]);
const tokenManager = await getContractAt('TokenManager', await service.tokenManagerAddress(tokenId), wallet);
const params = defaultAbiCoder.encode(['bytes', 'address'], [tokenFactory.address, tokenAddress]);
const tokenManager = await getContractAt('BaseTokenManager', await service.tokenManagerAddress(tokenId), wallet);
const token = await getContractAt('InterchainToken', tokenAddress, wallet);

await expect(tokenFactory.deployInterchainToken(salt, name, symbol, decimals, mintAmount, wallet.address))
Expand All @@ -293,7 +292,6 @@ describe('InterchainTokenFactory', () => {
.and.to.emit(tokenManager, 'RolesRemoved')
.withArgs(tokenFactory.address, 1 << FLOW_LIMITER_ROLE);

params = defaultAbiCoder.encode(['bytes', 'address'], ['0x', token.address]);
const payload = defaultAbiCoder.encode(
['uint256', 'bytes32', 'string', 'string', 'uint8', 'bytes'],
[MESSAGE_TYPE_DEPLOY_INTERCHAIN_TOKEN, tokenId, name, symbol, decimals, '0x'],
Expand Down
Loading

0 comments on commit b49ef61

Please sign in to comment.