fix tests and use BalanceDeltas

.forge-snapshots/increaseLiquidity_erc20.snap

@@ -1 +1 @@
-176386
+172027

.forge-snapshots/increaseLiquidity_erc6909.snap

@@ -1 +1 @@
-151968
+147609

.forge-snapshots/mintWithLiquidity.snap

@@ -1 +1 @@
-471675
+467316

contracts/base/BaseLiquidityManagement.sol

@@ -126,66 +126,45 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
             position.liquidity
         );
 
-        console2.log(callerFeesAccrued.amount0());
-        console2.log(callerFeesAccrued.amount1());
-        console2.log("totalFees");
-        console2.log(totalFeesAccrued.amount0());
-        console2.log(totalFeesAccrued.amount1());
-
-        // Calculate the accurate tokens owed to the caller.
-        // If the totalFeesAccrued equals the callerFeesAccrued then the total owed to the caller is just the liquidityDelta.
-        // If the totalFeesAccrued is greater than the callerFeesAccrued, we must account for the difference.
-        // TODO: If totalFeesAccrued == callerFeesAccrued, I think we can just apply the entire delta onto the caller, even if this implicitly collects on behalf of another user in the same range.
-        (int128 callerDelta0, int128 callerDelta1) = totalFeesAccrued != callerFeesAccrued
-            ? _calculateCallerDeltas(liquidityDelta, totalFeesAccrued, callerFeesAccrued)
-            : (liquidityDelta.amount0(), liquidityDelta.amount1());
+        // the delta for increase liquidity assuming that totalFeesAccrued was not applied
+        BalanceDelta principalDelta = liquidityDelta - totalFeesAccrued;
+
+        // outstanding deltas the caller is responsible for, after their fees are credited to the principal delta
+        BalanceDelta callerDelta = principalDelta + callerFeesAccrued;
+
+        // outstanding deltas this contract is responsible for, intuitively the contract is responsible for taking fees external to the caller's accrued fees
+        BalanceDelta thisDelta = totalFeesAccrued - callerFeesAccrued;
 
         // Update position storage, flushing the callerDelta value to tokensOwed first if necessary.
         // If callerDelta > 0, then even after investing callerFeesAccrued, the caller still has some amount to collect that were not added into the position so they are accounted to tokensOwed and removed from the final callerDelta returned.
-        uint128 tokensOwed0 = 0;
-        uint128 tokensOwed1 = 0;
-        (tokensOwed0, callerDelta0) = callerDelta0 > 0 ? (uint128(callerDelta0), int128(0)) : (uint128(0), callerDelta0);
-        (tokensOwed1, callerDelta1) = callerDelta1 > 0 ? (uint128(callerDelta1), int128(0)) : (uint128(0), callerDelta1);
+        BalanceDelta tokensOwed;
+        if (callerDelta.amount0() > 0) {
+            // credit the excess tokens to the position's tokensOwed
+            tokensOwed = toBalanceDelta(callerDelta.amount0(), 0);
 
-        position.addTokensOwed(tokensOwed0, tokensOwed1);
-        position.addLiquidity(liquidityToAdd);
-        position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128);
+            // this contract is responsible for custodying the excess tokens
+            thisDelta = thisDelta + toBalanceDelta(callerDelta.amount0(), 0);
 
-        // The delta owed or credited by this contract.
-        // TODO @sauce check that if callerDelta == 0 (zerod out from above), then this line just credits the posm to takes on behalf of the caller
-        int128 thisDelta0 = liquidityDelta.amount0() - callerDelta0;
-        int128 thisDelta1 = liquidityDelta.amount1() - callerDelta1;
+            // the caller is not expected to collect the excess tokens
+            callerDelta = toBalanceDelta(0, callerDelta.amount1());
+        }
 
-        return (toBalanceDelta(callerDelta0, callerDelta1), toBalanceDelta(thisDelta0, thisDelta1));
-    }
+        if (callerDelta.amount1() > 0) {
+            // credit the excess tokens to the position's tokensOwed
+            tokensOwed = toBalanceDelta(tokensOwed.amount0(), callerDelta.amount1());
 
-    // Returns the delta paid/credited by/to the caller.
-    function _calculateCallerDeltas(
-        BalanceDelta liquidityDelta,
-        BalanceDelta totalFeesAccrued,
-        BalanceDelta callerFeesAccrued
-    ) private pure returns (int128 callerDelta0, int128 callerDelta1) {
-        (int128 liquidityDelta0, int128 liquidityDelta1) = (liquidityDelta.amount0(), liquidityDelta.amount1());
-        (int128 totalFeesAccrued0, int128 totalFeesAccrued1) = (totalFeesAccrued.amount0(), totalFeesAccrued.amount1());
-        (int128 callerFeesAccrued0, int128 callerFeesAccrued1) =
-            (callerFeesAccrued.amount0(), callerFeesAccrued.amount1());
-
-        callerDelta0 = _calculateCallerDelta(liquidityDelta0, totalFeesAccrued0, callerFeesAccrued0);
-        callerDelta1 = _calculateCallerDelta(liquidityDelta1, totalFeesAccrued1, callerFeesAccrued1);
-    }
+            // this contract is responsible for custodying the excess tokens
+            thisDelta = thisDelta + toBalanceDelta(0, callerDelta.amount1());
 
-    function _calculateCallerDelta(int128 liquidityDelta, int128 totalFeesAccrued, int128 callerFeesAccrued)
-        private
-        pure
-        returns (int128 callerDelta)
-    {
-        unchecked {
-            // The principle delta owed/debited to the caller before any LP fees are deducted.
-            int128 principleDelta = liquidityDelta - totalFeesAccrued;
-            // The new caller delta is this principle delta plus the callerFeesAccrued which consists of
-            // the custodied fees by posm and unclaimed fees from the modifyLiq call.
-            callerDelta = principleDelta + callerFeesAccrued;
+            // the caller is not expected to collect the excess tokens
+            callerDelta = toBalanceDelta(callerDelta.amount0(), 0);
         }
+
+        position.addTokensOwed(tokensOwed);
+        position.addLiquidity(liquidityToAdd);
+        position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128);
+
+        return (callerDelta, thisDelta);
     }
 
     function _increaseLiquidityAndZeroOut(

contracts/libraries/Position.sol

@@ -2,15 +2,14 @@
 pragma solidity >=0.8.20;
 
 import {IBaseLiquidityManagement} from "../interfaces/IBaseLiquidityManagement.sol";
+import {BalanceDelta} from "v4-core/types/BalanceDelta.sol";
 
 // Updates Position storage
 library PositionLibrary {
     // TODO ensure this is one sstore.
-    function addTokensOwed(IBaseLiquidityManagement.Position storage position, uint128 tokensOwed0, uint128 tokensOwed1)
-        internal
-    {
-        position.tokensOwed0 += tokensOwed0;
-        position.tokensOwed1 += tokensOwed1;
+    function addTokensOwed(IBaseLiquidityManagement.Position storage position, BalanceDelta tokensOwed) internal {
+        position.tokensOwed0 += uint128(tokensOwed.amount0());
+        position.tokensOwed1 += uint128(tokensOwed.amount1());
     }
 
     function addLiquidity(IBaseLiquidityManagement.Position storage position, uint256 liquidity) internal {

test/position-managers/IncreaseLiquidity.t.sol

@@ -365,13 +365,14 @@ contract IncreaseLiquidityTest is Test, Deployers, GasSnapshot, Fuzzers {
             lpm.increaseLiquidity(tokenIdAlice, liquidityDelta, ZERO_BYTES, false);
         }
 
-        // alice did not spend any tokens, approximately
-        assertApproxEqAbs(balance0AliceBefore, currency0.balanceOf(alice), 0.00001 ether);
-        assertApproxEqAbs(balance1AliceBefore, currency1.balanceOf(alice), 0.00001 ether);
+        // alice did not spend any tokens
+        assertEq(balance0AliceBefore, currency0.balanceOf(alice));
+        assertEq(balance1AliceBefore, currency1.balanceOf(alice));
 
+        // sum dust was credited to alice's tokensOwed
         (token0Owed, token1Owed) = lpm.feesOwed(tokenIdAlice);
-        assertEq(token0Owed, 0);
-        assertEq(token1Owed, 0);
+        assertApproxEqAbs(token0Owed, 0, 80 wei);
+        assertApproxEqAbs(token1Owed, 0, 80 wei);
     }
 
     // uses donate to simulate fee revenue
@@ -428,10 +429,6 @@ contract IncreaseLiquidityTest is Test, Deployers, GasSnapshot, Fuzzers {
         // alice did not spend any tokens
         assertEq(balance0AliceBefore, currency0.balanceOf(alice), "alice spent token0");
         assertEq(balance1AliceBefore, currency1.balanceOf(alice), "alice spent token1");
-
-        // passes: but WRONG!!!
-        // assertEq(balance0AliceBefore - currency0.balanceOf(alice), 10e18);
-        // assertEq(balance1AliceBefore - currency1.balanceOf(alice), 10e18);
 
         (token0Owed, token1Owed) = lpm.feesOwed(tokenIdAlice);
         assertEq(token0Owed, 0);
@@ -441,5 +438,10 @@ contract IncreaseLiquidityTest is Test, Deployers, GasSnapshot, Fuzzers {
         (token0Owed, token1Owed) = lpm.feesOwed(tokenIdBob);
         assertApproxEqAbs(token0Owed, 5e18, 1 wei);
         assertApproxEqAbs(token1Owed, 5e18, 1 wei);
+
+        vm.prank(bob);
+        BalanceDelta result = lpm.collect(tokenIdBob, bob, ZERO_BYTES, false);
+        assertApproxEqAbs(result.amount0(), 5e18, 1 wei);
+        assertApproxEqAbs(result.amount1(), 5e18, 1 wei);
     }
 }