v4-periphery icon indicating copy to clipboard operation
v4-periphery copied to clipboard

[Bug]: FullRange.addLiquidity: wrong amount of tokens minted

Open ferencdg opened this issue 1 year ago • 1 comments

Describe the bug

https://github.com/Uniswap/v4-periphery/blob/581d96dfd8b281cafe351205bf6d2d65efb4df90/contracts/hooks/examples/FullRange.sol#L152

I am not sure this can be called bug or a design decision, but imagine the following scenario:

  1. January 1st userA addLiquidity and gets 100 shares minted
  2. Between January 1 - June 30th lots of swaps are happening, but no one removes liquidity
  3. On June 30th the pool price is the same as it was on January 1st, and userB adds the same liquidity as userA and gets the same 100 shares minted
  4. Now if userB redeems, he can run away with all the fees generated between January 1st - June 30th

The problem is that 'rebalancing' is only called during removing liquidity from the pool, so the fees are not accounted for unless someone withdraws liquidity. So the fairness of the fee distribution depends on how often the withdrawLiquidity method is called.

Expected Behavior

easiest fix would be to call rebalance before depositing too, then mint: userSuppliedLiquidity*totalAmountOfTokensMinted/allLiquidityHeldByThePool

To Reproduce

No response

Additional context

No response

ferencdg avatar Oct 22 '23 09:10 ferencdg

The problem is actually worse. Let's say:

  1. userA adds a lot of liquidity
  2. lots of swaps generate 1 ETH and 1000 USDC of fees, but no one removes liquidity yet
  3. userB adds 1.001 ETH and 1001 USDC of liquidity but it only costs 0.001 ETH and 1 USDC to do so
    • because modifyLiquidity accrues fees, manager.modifyLiquidity returns a delta of (1 - 1.001) ETH and (1000 - 1001) USDC
    • note: userB must add at least 1 ETH and 1000 USDC or else the function call will revert as manager.modifyLiquidity will return a positive delta
  4. Now userB redeems for 1.001 ETH and 1001 USDC, yoinking all the fees instantly

Jun1on avatar Jun 24 '24 22:06 Jun1on

Hi! thanks for flagging -- going to close this issue because:

hook examples no longer live in the main branch

custom accounting will allow for a more v2-style curve with v2 interfaces. reference implementation https://github.com/hensha256/v2-on-v4

saucepoint avatar Sep 04 '24 22:09 saucepoint