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

Loss of Withdrawal Fees Due to Incorrect Scaling of `lpTokenDepositValueChange` in `UpliftOnlyExample.onAfterRemoveLiquidity` Function

0xaadi

Summary

The UpliftOnlyExample contract contains a miscalculation in the lpTokenDepositValueChange variable within the onAfterRemoveLiquidity() function. The calculation fails to correctly apply fixed-point arithmetic by not scaling the change in deposit value with 1e18 before division, leading to a loss of precision. This results in incorrect fee calculations, potentially charging minWithdrawalFeeBps for changes less than 100% and a lower fee than intended for changes greater than or equal to 100%.

Vulnerability Details

The issue arises from performing division before multiplication in the calculation of lpTokenDepositValueChange. This leads to a loss of precision due to integer division rounding down.

@> 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#L474C1-L484C14

The current calculation

localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);

Here we can see that the division operation rounds down the result, causing lpTokenDepositValueChange to be zero for changes between 0% and 100%, and one for changes between 100% and 200%.

feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;

The subsequent multiplication by 1e18 in the fee calculation does not recover the lost precision from the initial division.

For Changes < 100%:

The lpTokenDepositValueChange becomes 0, leading to the application of minWithdrawalFeeBps instead of the intended uplift fee.

For Changes > 100% (Not a multiple of 100):

The lpTokenDepositValueChange rounds down, resulting in a much smaller fee than intended, potentially even less than minWithdrawalFeeBps.

Impact

Incorrect Fee Calculation: The miscalculation can lead to incorrect fee assessments, potentially undercharging users. If there is a positive growth (>=100%) in the deposit amount, the protocol may charge a very small fee for the remove liquidity operation. Conversely, it only applies the minWithdrawalFeeBps for growths less than 100%.

Due to this calculation issue, the protocol misses out on collecting appropriate fees for positive growth scenarios, leading to a loss of revenue.

Tools Used

Manual Review

Recommendations

Modify the calculation to apply multiplication by 1e18 before division to preserve precision:

localData.lpTokenDepositValueChange =
- (int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
+ (int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) * 1e18 /
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)) /
+ (uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps))) /
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;
}
Updates

Lead Judging Commences

n0kto Lead Judge 11 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.

Give us feedback!