QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: medium
Valid

Rounding Down in Fee Calculation Allows Users to Pay Less Withdraw Fee

aksoy

Summary

UpliftOnlyExample:nAfterRemoveLiquidity function contains an issue in the calculation logic for applying fees when the LP token deposit value increases. Specifically, the uplift fee (upliftFeeBps) is not applied in scenarios where the value increase is less than 2x the deposit value, resulting in users being undercharged. This can cause a loss of potential fee revenue for the pool.

Vulnerability Details

When users remove liquidity if their deposit value increases they should have pay fee to pool calculated with upliftFeeBps ratio. However because of rounding down issue users can pay less fee.

The calculation of lpTokenDepositValueChange determines the proportional change in LP token value by dividing the difference between the current value (lpTokenDepositValueNow) and the deposit value (lpTokenDepositValue). However, this division can result in the value rounding down to zero when the increase is less than 2x the deposit value. As a result:

  • When lpTokenDepositValueNow is greater than the deposit value but not sufficiently large, the change rounds to zero.

  • The function defaults to applying the minWithdrawalFeeBps instead of the uplift fee (upliftFeeBps).

  • This prevents the pool from collecting the appropriate fee, leading to revenue loss.

localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
​
uint256 feePerLP;
// if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
if (localData.lpTokenDepositValueChange > 0) {
feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;
}
// if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
else {
//in most cases this should be a normal swap fee amount.
//there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
}

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L474

Test:

function testRemoveLiquidityPositivePrice() public {
// Add liquidity so bob has BPT to remove liquidity.
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
​
int256[] memory prices = new int256[]();
for (uint256 i = 0; i < tokens.length; ++i) {
//@audit Price is increased by 90%
prices[i] = (int256(i) * 2e18 * 9) / 10;
}
updateWeightRunner.setMockPrices(pool, prices);
​
uint256 nftTokenId = 0;
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
BaseVaultTest.Balances memory balancesBefore = getBalances(bob);
​
vm.startPrank(bob);
upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
vm.stopPrank();
BaseVaultTest.Balances memory balancesAfter = getBalances(bob);
​
//@audit Price incrased but because of rounding only minWithdrawalFeeBps is applied
uint256 feeAmountAmountPercent = ((bptAmount / 2) *
((uint256(upliftOnlyRouter.minWithdrawalFeeBps()) * 1e18) / 10000)) / ((bptAmount / 2));
​
uint256 amountOut = (bptAmount / 2).mulDown((1e18 - feeAmountAmountPercent));
​
assertEq(
balancesAfter.bobTokens[daiIdx] - balancesBefore.bobTokens[daiIdx],
amountOut,
"bob's DAI amount is wrong"
);
​
assertEq(
balancesAfter.bobTokens[usdcIdx] - balancesBefore.bobTokens[usdcIdx],
amountOut,
"bob's USDC amount is wrong"
);
​
}

Impact

The current logic allows users to avoid paying the uplift fee unless the value increase is substantial (2x or more of the deposit value)

Tools Used

Manual

Recommendations

Update the lpTokenDepositValueChange calculation to prevent rounding down by increasing precision

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_onAfterRemoveLiquidity_lpTokenDepositValueChange_rounding_error_100%_minimum

Likelihood: High, every call to the function (withdraw) Impact: Low/Medium, uplift fees will be applied only when the price of one asset is doubled but fixed fees will still be collected.

Support

FAQs

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