Submitted by 0xDING99YA
https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1049-L1066
https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1093-L1122
https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1200-L1248
Currently, when computing the fee a user can collect, Uniswap V3 uses the ways as (currFeeGrowth - prevFeeGrowth) * liquidity / Q128, in Panoptic it uses (currFeeGrowth * liquidity / Q128) - (prevFeeGrowth * liquidity / Q128). However, this slightly difference can result in a user to collect more fees than he should have, thus break the main invariant that Fees paid to a given user should not exceed the amount of fees earned by the liquidity owned by that user.
In Panoptic the fee collection mechanism is as follows:
To show why the slightly difference in calculation can result in the consequence, lets consider some values:
Based on UniswapV3, the collect fee should be (3 * 2^124 - 1 * 2^124 - 1) * 2^4 / 2^128 = (2 * 2^124 - 1) * 2^4/2^128 = (2 * 2^128 - 2^4) / 2^128 = 1.
However, based on Panoptic, the fee to be collected is (3 * 2^124 * 2^4)/2^128 - (1 * 2^124 + 1) * 2^4/2^128 = 3 - 1 = 2, which is larger than the fee that the user should obtain. If in the same position there is any other users also collecting the fee, that user may lose his fee due to this error.
The difference between the collected fee is 1; seems non-trivial, but it can be a problem in some cases, especially for those token with low decimals. In the documentation, Panoptic claims that it supports any ERC20 with no transfer fee, if there are some tokens with low decimals but worth a lot, this issue can be very serious. Also note, this issue will exist at any case, especially seriously for those pools where deltaFeeGrowth * liquidity is around Q128 level and the severity varies case by case.
To show this issue really exists, I will take Gemini USD - USDC pool in Uniswap V3 as an example. Below is the coded POC to show the different calculation indeed causes user collect more fees:
In this case, that user can collect 1 more Gemini USD which is around 0.01 USD. Also note that these are just two normal swaps with thousands of dollars and in reality this effect can accumulate due to much more trading volume per day. Imagine there may be some similar tokens but pegged to ETH or even BTC instead; in that case, will be 0.01 ETH or even 0.01 BTC which can be much more value! Also, in Panoptic there can be multiple persons for the same option position. Lets say 10 users for the same option position, the first 9 users who collect the fee will get more, and the last user can get much less fee because each of the 9 person will take part of the fee from him!
Foundry
In the comments, it seems Panoptic team has the reason to use this way to calculate the collected fee after much consideration. I think instead of supporting any ERC20 token, they may introduce a whitelist to only add those pool which will not affected by this issue significantly.
dyedm1 (Panoptic) confirmed and commented:
Picodes (judge) decreased severity to Medium and commented:
