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

Withdrawing and depositing allows avoiding uplift fees and in a bull market pool may experience high slippage

jsondoge

Summary

Due to current uplift fee scaling logic it's encouraged to withdraw and deposit before the first uplift fee boundary is hit.

Vulnerability Details

By current logic users can avoid paying high uplift fees by withdrawing before their the first uplift fee boundary is hit.

The boundary being deposit price doubling.

localData.lpTokenDepositValueChange =
int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
​
if (localData.lpTokenDepositValueChange > 0) {
// uplift fee is applied
} else {
// fixed fee
}

If we look at lpTokenDepositValueChange calculation we'll see that it will only be set to "1" and above if the new value is at least double the price at deposit time. Otherwise the division will round it back to zero and the "if" logic will apply the fixed fee instead.

At least in tests the values of minimal uplift fee and the fixed fee are 200 (2%) and 5 (0.05%), which is not an insignificant difference.

This means that if gas fees are ignored, the user could withdraw and deposit 40 times before the fees charged would get anywhere close the once charged uplift fee value.

Because uplift fees are charged on price increases, this would mean that when the market would be in a bull run the liquidity of the pool would fluctuate greatly as users withdraw and redeposit liquidity to reset the fees. Especially because the fee logic has a sharp cutoff where a 1.9999...x times price increase has 0.05% fee and as soon as it hits 2x time increase it becomes a 2% fee.

In smaller liquidity pools this would expose regular swappers to higher slippage and more arbitrage opportunities.

Impact

The fee calculation logic encourages unstable liquidity when market is in a bull run - may expose swappers to large slippage. Also reduces platform revenue by circumventing larger profit fees.

Tools Used

Manual review and foundry tests. Test case is a modified version from UpliftExample.t.sol (UpliftOnlyExampleTest)

function testWithdrawAndDepositAllowsEvadingFees() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(alice), usdc.balanceOf(alice)].toMemoryArray();
uint256 input = 1e18;
​
vm.prank(alice);
uint256[] memory amountsIn = upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, input, false, bytes(""));
​
int256[] memory prices = new int256[]();
prices[0] = 0;
prices[1] = 1.9999999e18; // 1.999..x price increase (1e18 originally)
updateWeightRunner.setMockPrices(pool, prices);
​
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
uint256 beforeDaiAlice = dai.balanceOf(alice);
uint256 beforeUsdcAlice = usdc.balanceOf(alice);
​
vm.prank(alice);
upliftOnlyRouter.removeLiquidityProportional(input, minAmountsOut, false, pool);
​
uint256 afterDaiAlice = dai.balanceOf(alice);
uint256 afterUsdcAlice = usdc.balanceOf(alice);
​
uint256 daiDiff = afterDaiAlice - beforeDaiAlice;
uint256 usdcDiff = afterUsdcAlice - beforeUsdcAlice;
​
// min withdrawal fee is 0.05%
// If Alice withdraws just before price doubles, she will pay only 0.05% withdrawal fee
assertEq(amountsIn[0] * 9995 / 10000, daiDiff, "Alice paid only min withdrawal fee (0.05%)");
assertEq(amountsIn[1] * 9995 / 10000, usdcDiff, "Alice paid only min withdrawal fee (0.05%)");
​
// After paying 0.05% withdrawal fee, Alice will re-deposit and repeat the cycle if price increases further
uint256 newInput = afterDaiAlice;
​
vm.prank(alice);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, newInput, false, bytes(""));
}

Recommendations

The following below would gradually increase the fee percentage up to 2% maximum and stop increasing when profits hit 150% of the original deposit.

constant uint256 MAX_VALUE_INCREASE_BPS = 15000; // scale fees up to 150% profit
...
​
int256 valueChangeBps = ((int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) * 10_000) /
int256(localData.lpTokenDepositValue);
uint256 feePerLp = 0;
​
if (valueChangeBps > 0) {
scaledUpliftFeeBps = uint256(valueChangeBps) >= MAX_VALUE_INCREASE_BPS ? (uint256(feeDataArray[i].upliftFeeBps) : (uint256(feeDataArray[i].upliftFeeBps) * uint256(valueChangeBps) / MAX_VALUE_INCREASE_BPS;
feePerLp = scaledUpliftFeeBps * 1e18 / 10000
}

And after the calculation use, the below code to transition:

uint256 feePerLp = Math.max(
feePerLp, // - above calculated value
(uint256(minWithdrawalFeeBps) * 1e18) / 10000
)

This would result in a much smoother transition from fixed to uplift fees. Now it still has some overlap and minWithdrawalFeeBps is going to be applied at the beggining of positive value increase, but the transition can futher be tuned to decrease that overlap.

Updates

Lead Judging Commences

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