QuantAMM

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

Incorrect Exit Fee Calculation Due to Precision Loss in Division Operations

Summary

The exit fee calculation in onAfterRemoveLiquidity https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L474-L476 suffers from precision loss due to division operations being performed before multiplication, potentially leading to incorrect fee calculations and loss of funds for LPs.

Vulnerability Details

The issue occurs in the fee calculation logic:

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;
}

Impact

feePerLP will always be 0 when localData.lpTokenDepositValueChange > 0

Poc

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity ^0.8.24;
import "forge-std/Test.sol";
contract ExitFeePrecisionTest is Test {
function testExitFeePrecisionLoss() public {
// Setup test values
uint256 lpTokenDepositValueNow = 1100 ether; // Current value: 1100 tokens due to depeg (Luna case)
uint256 lpTokenDepositValue = 1000 ether; // Original deposit value: 1000 tokens
uint256 upliftFeeBps = 1000; // 10% fee
// Current implementation (incorrect)
int256 valueChangeIncorrect = (int256(lpTokenDepositValueNow) - int256(lpTokenDepositValue)) /
int256(lpTokenDepositValue);
uint256 feePerLPIncorrect = uint256(valueChangeIncorrect) * (upliftFeeBps * 1e18) / 10000;
// Correct implementation (performing multiplication before division)
int256 valueChangeCorrect = ((int256(lpTokenDepositValueNow) - int256(lpTokenDepositValue)) * 1e18) /
int256(lpTokenDepositValue);
uint256 feePerLPCorrect = uint256(valueChangeCorrect) * upliftFeeBps / 10000;
// Print results
console.log("Incorrect fee calculation:", feePerLPIncorrect);
console.log("Correct fee calculation:", feePerLPCorrect);
// Example with small values to demonstrate precision loss
lpTokenDepositValueNow = 11 ether; // 11 tokens
lpTokenDepositValue = 10 ether; // 10 tokens
valueChangeIncorrect = (int256(lpTokenDepositValueNow) - int256(lpTokenDepositValue)) /
int256(lpTokenDepositValue);
feePerLPIncorrect = uint256(valueChangeIncorrect) * (upliftFeeBps * 1e18) / 10000;
valueChangeCorrect = ((int256(lpTokenDepositValueNow) - int256(lpTokenDepositValue)) * 1e18) /
int256(lpTokenDepositValue);
feePerLPCorrect = uint256(valueChangeCorrect) * upliftFeeBps / 10000;
console.log("Small values:");
console.log("Incorrect fee calculation:", feePerLPIncorrect);
console.log("Correct fee calculation:", feePerLPCorrect);
// Assert the difference
assertTrue(feePerLPCorrect > feePerLPIncorrect, "Precision loss detected");
}
}

Tools Used

Manual, Foundry

Recommendations

Reorder calculations to perform multiplications before division

Updates

Lead Judging Commences

n0kto Lead Judge 10 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!