The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: medium
Valid

`SmartVaultV3::swap()` uses hard coded pool fee tier

Summary

SmartVaultV3 uses a hard-coded fee of 0.3% to perform swaps in all of the supported tokens' pools. This is not an ideal design because Uniswap V3 has four different fee tiers and there's no one size fits it all. Uniswap V3 docs recommend that the lowest tier is used for stable pairs and the highest one for exotic pairs.

Vulnerability Details

Uniswap V3 currently has support for different pool fee tiers: 0.01%, 0.05%, 0.3%, and 1%. The general recommendation is the following:

0.01% => Best for very stable pairs

0.05% => Best for stable pairs

0.3% => Best for most pairs

1% => Best for exotic pairs

Official docs on this: https://docs.uniswap.org/concepts/protocol/fees#finding-the-right-pool-fee

If we look at the code, we will find that SmartVaultV3::swap() uses a hard-coded value of 3000 as the fee (representing the 0.3% tier):

function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
address inToken = getSwapAddressFor(_inToken);
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
tokenIn: inToken,
tokenOut: getSwapAddressFor(_outToken),
fee: 3000,
recipient: address(this),
deadline: block.timestamp,
amountIn: _amount - swapFee,
amountOutMinimum: minimumAmountOut,
sqrtPriceLimitX96: 0
});
inToken == ISmartVaultManagerV3(manager).weth() ?
executeNativeSwapAndFee(params, swapFee) :
executeERC20SwapAndFee(params, swapFee);
}

Impact

I believe this to be a bad design for the following reasons:

  1. There might be a cheaper pool for the pair to use. If that's the case, the user will be incurred with unnecessary fees on every swap.

  2. The pool might not have enough liquidity to perform the swap.

  3. The pool might not exist at all. This is not the case with the current tokens but could occur with any of the future-added ones.

  4. The Uniswap DAO could add a new tier in the future and most liquidity providers might decide to switch to that one for a pair, leading to the 0.3% pool not having sufficient liquidity.

Tools Used

Manual Analysis

Recommendations

There are two possible solutions:

  1. Add a new parameter to SmartVaultV3::swap() that allows the user to choose the fee tier on their own.

  2. Store a mapping of each pair's fee tier in the TokenManager contract and refer to it for every swap. This approach allows admins to change the fee tier as they deem necessary but is more centralized than the first one.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

fixed-uni-fee

NentoR Submitter
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

hardcoded-fee

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.