The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`swapFee` not Used in `SmartVaultV3::swap` to Calculate `minimumAmountOut` Resulting in Potential Swap Failures

n0kto

Description

In the provided code snippet, the swapFee is not considered in the calculation of minimumAmountOut within the SmartVaultV3::swap function. While it is correctly applied to the amountIn, this inconsistency can lead to inaccurate calculations of the minimum amount to be received (amountOutMinimum) for the Uniswap router.

function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
.
.
.
@> 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
});
​

Impact

In most cases, minimumAmountOut will be higher than the actual conversion of amountIn (as it is equal to _amount - swapFee) to the _outToken. This inconsistency will cause the swap to revert because amountOutMinimum is set too high.

Recommended Mitigation

To address this issue, it is recommended to subtract the swapFee from the amount in the SmartVaultV3::calculateMinimumAmountOut function to calculate a more accurate minimumAmountOut. Additionally, it is advised to rewrite calculateMinimumAmountOut to only calculate a real value and create a separate function (wontBeUndercollateralised) to check that the vault will not be undercollateralized.

Below is a possible example:

function swap(
bytes32 _inToken,
bytes32 _outToken,
uint256 _amount
) external onlyOwner {
.
.
.
​
uint256 minimumAmountOut = calculateMinimumAmountOut(
_inToken,
_outToken,
- _amount
+ _amount - swapFee
);
+ require(wontBeUndercollateralised(_inToken, _outToken, minimumAmountOut))
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
});
.
.
.
}
​
+ function wontBeUndercollateralised(
+ bytes32 _inTokenSymbol,
+ bytes32 _outTokenSymbol,
+ uint256 _amount,
+ uint256 minimumAmountOut
+ ) private view returns (uint256) {
+ ISmartVaultManagerV3 _manager = ISmartVaultManagerV3(manager);
+
+ uint256 requiredCollateralValue = (minted * _manager.collateralRate()) /
+ _manager.HUNDRED_PC();
+
+ uint256 collateralValueMinusSwapValue = euroCollateral() -
+ calculator.tokenToEur(getToken(_inTokenSymbol), _amount);
+
+ return
+ collateralValueMinusSwapValue + minimumAmountOut >= requiredCollateralValue
+ }
+
+ function calculateMinimumAmountOut(
+ bytes32 _inTokenSymbol,
+ bytes32 _outTokenSymbol,
+ uint256 _amount
+ ) private view returns (uint256) {
+
+ amountInEuro = calculator.tokenToEurAvg(
+ getToken(_inTokenSymbol),
+ _amount
+ );
+ return calculator.eurToToken(
+ getToken(_outTokenSymbol),
+ amountInEuro
+ );
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

swapfee-incorrect-calc

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

swapfee-incorrect-calc

Support

FAQs

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

Give us feedback!