edge case of using cached fees for autocompound

Uniswap · Jun 25, 2024 · 7e49d96 · 7e49d96
1 parent c27ad15
commit 7e49d96
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/contracts/base/BaseLiquidityManagement.sol b/contracts/base/BaseLiquidityManagement.sol
@@ -133,6 +133,32 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
             ? _calculateCallerDeltas(liquidityDelta, totalFeesAccrued, callerFeesAccrued)
             : (liquidityDelta.amount0(), liquidityDelta.amount1());
 
+        // An edge case:
+        // assume alice and bob are on the same range
+        // and 20 token fee revenue is posted (i.e. swap revenue or donate)
+
+        // bob calls collects()
+        //   liquidityDelta:    20
+        //   totalFeesAccrued:  20
+        //   callerFeesAccrued:  5
+        //   (5 tokens sent to bob, and posm caches the 15 tokens for alice)
+
+        // assume another 20 token fee revenue is posted (a net new 5 tokens for bob and 15 new tokens for alice)
+        // alice now has 30 tokens of fee revenue, half of custodied by posm and the other half unclaimed in the PM
+
+        // when alice increases her liquidity, using exactly 30 tokens (autocompound):
+        //   liquidityDelta:   -10
+        //   totalFeesAccrued:  20 (new fees: 5 for bob, 15 for alice)
+        //   callerFeesAccrued: 30 (alice's fees, per feeGrowthInside)
+
+        // naively resolving deltas:
+        // posm:  take 5 tokens (bob's fees), liquidityDelta is now: -15
+        // posm:  pay PM the 15 tokens (alice's cached fees)
+        // alice: pay nothing, as its a pure and exact autocompound
+
+        // to solve:
+        // we need a way to discern cached fees and use them against liquidityDelta
+
         // Update position storage, sanitizing the tokensOwed and callerDelta values first.
         // if callerDelta > 0, then even after re-investing old fees, the caller still has some amount to collect that were not added into the position so they are accounted.
         // if callerDelta <= 0, then tokensOwed0 and tokensOwed1 should be zero'd out as all fees were re-invested into a new position.

diff --git a/test/position-managers/IncreaseLiquidity.t.sol b/test/position-managers/IncreaseLiquidity.t.sol
@@ -412,7 +412,6 @@ contract IncreaseLiquidityTest is Test, Deployers, GasSnapshot, Fuzzers {
 
         // alice will use ALL of her fees to increase liquidity
         {
-            console2.log(newToken0Owed, newToken1Owed);
             (uint160 sqrtPriceX96,,,) = StateLibrary.getSlot0(manager, range.poolKey.toId());
             uint256 liquidityDelta = LiquidityAmounts.getLiquidityForAmounts(
                 sqrtPriceX96,
@@ -427,8 +426,8 @@ contract IncreaseLiquidityTest is Test, Deployers, GasSnapshot, Fuzzers {
         }
 
         // alice did not spend any tokens, approximately
-        assertApproxEqAbs(balance0AliceBefore, currency0.balanceOf(alice), 0.00001 ether);
-        assertApproxEqAbs(balance1AliceBefore, currency1.balanceOf(alice), 0.00001 ether);
+        assertEq(balance0AliceBefore, currency0.balanceOf(alice), "alice spent token0");
+        assertEq(balance1AliceBefore, currency1.balanceOf(alice), "alice spent token1");
 
         (token0Owed, token1Owed) = lpm.feesOwed(tokenIdAlice);
         assertEq(token0Owed, 0);