change add liq accounting

.forge-snapshots/FullRangeAddInitialLiquidity.snap

@@ -1 +1 @@
-354477
+311181

.forge-snapshots/FullRangeAddLiquidity.snap

@@ -1 +1 @@
-161786
+122990

.forge-snapshots/FullRangeFirstSwap.snap

@@ -1 +1 @@
-146400
+80220

.forge-snapshots/FullRangeInitialize.snap

@@ -1 +1 @@
-1039616
+1016976

.forge-snapshots/FullRangeRemoveLiquidity.snap

@@ -1 +1 @@
-146394
+110566

.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap

@@ -1 +1 @@
-281672
+240044

.forge-snapshots/FullRangeSecondSwap.snap

@@ -1 +1 @@
-116110
+45930

.forge-snapshots/FullRangeSwap.snap

@@ -1 +1 @@
-145819
+79351

.forge-snapshots/OracleGrow10Slots.snap

@@ -1 +1 @@
-254164
+232960

.forge-snapshots/OracleGrow10SlotsCardinalityGreater.snap

@@ -1 +1 @@
-249653
+223649

.forge-snapshots/OracleGrow1Slot.snap

@@ -1 +1 @@
-54049
+32845

.forge-snapshots/OracleGrow1SlotCardinalityGreater.snap

@@ -1 +1 @@
-49549
+23545

.forge-snapshots/OracleInitialize.snap

@@ -1 +1 @@
-72794
+51310

.forge-snapshots/TWAMMSubmitOrder.snap

@@ -1 +1 @@
-156828
+122336

.forge-snapshots/decreaseLiquidity_erc20.snap

@@ -1 +1 @@
-187556
+118349

.forge-snapshots/decreaseLiquidity_erc6909.snap

@@ -1 +1 @@
-166551
+116100

.forge-snapshots/increaseLiquidity_erc20.snap

@@ -1 +1 @@
-183251
+59472

.forge-snapshots/increaseLiquidity_erc6909.snap

@@ -1 +1 @@
-158833
+63240

.forge-snapshots/mintWithLiquidity.snap

@@ -1 +1 @@
-478540
+443489

contracts/NonfungiblePositionManager.sol

@@ -34,7 +34,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
 
     constructor(IPoolManager _manager)
         BaseLiquidityManagement(_manager)
-        ERC721Permit("Uniswap V4 Positions NFT-V1", "UNI-V3-POS", "1")
+        ERC721Permit("Uniswap V4 Positions NFT-V1", "UNI-V4-POS", "1")
     {}
 
     // NOTE: more gas efficient as LiquidityAmounts is used offchain
@@ -56,7 +56,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
 
     // NOTE: more expensive since LiquidityAmounts is used onchain
     // function mint(MintParams calldata params) external payable returns (uint256 tokenId, BalanceDelta delta) {
-    //     (uint160 sqrtPriceX96,,,) = manager.getSlot0(params.range.key.toId());
+    //     (uint160 sqrtPriceX96,,,) = manager.getSlot0(params.range.poolKey.toId());
     //     (tokenId, delta) = mint(
     //         params.range,
     //         LiquidityAmounts.getLiquidityForAmounts(

contracts/base/BaseLiquidityManagement.sol

@@ -22,6 +22,9 @@ import {CurrencyDeltas} from "../libraries/CurrencyDeltas.sol";
 import {FeeMath} from "../libraries/FeeMath.sol";
 import {LiquiditySaltLibrary} from "../libraries/LiquiditySaltLibrary.sol";
 import {IBaseLiquidityManagement} from "../interfaces/IBaseLiquidityManagement.sol";
+import {PositionLibrary} from "../libraries/Position.sol";
+
+import "forge-std/console2.sol";
 
 contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
     using LiquidityRangeIdLibrary for LiquidityRange;
@@ -34,17 +37,27 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
     using TransientStateLibrary for IPoolManager;
     using SafeCast for uint256;
     using LiquiditySaltLibrary for IHooks;
+    using PositionLibrary for IBaseLiquidityManagement.Position;
 
     mapping(address owner => mapping(LiquidityRangeId rangeId => Position)) public positions;
 
     constructor(IPoolManager _manager) ImmutableState(_manager) {}
 
-    function zeroOut(BalanceDelta delta, Currency currency0, Currency currency1, address owner, bool claims) public {
-        if (delta.amount0() < 0) currency0.settle(manager, owner, uint256(int256(-delta.amount0())), claims);
-        else if (delta.amount0() > 0) currency0.send(manager, owner, uint128(delta.amount0()), claims);
+    function _closeCallerDeltas(
+        BalanceDelta callerDeltas,
+        Currency currency0,
+        Currency currency1,
+        address owner,
+        bool claims
+    ) internal {
+        int128 callerDelta0 = callerDeltas.amount0();
+        int128 callerDelta1 = callerDeltas.amount1();
 
-        if (delta.amount1() < 0) currency1.settle(manager, owner, uint256(int256(-delta.amount1())), claims);
-        else if (delta.amount1() > 0) currency1.send(manager, owner, uint128(delta.amount1()), claims);
+        if (callerDelta0 < 0) currency0.settle(manager, owner, uint256(int256(-callerDelta0)), claims);
+        else if (callerDelta0 > 0) currency0.send(manager, owner, uint128(callerDelta0), claims);
+
+        if (callerDelta1 < 0) currency1.settle(manager, owner, uint256(int256(-callerDelta1)), claims);
+        else if (callerDelta1 > 0) currency1.send(manager, owner, uint128(callerDelta1), claims);
     }
 
     function _unlockCallback(bytes calldata data) internal override returns (bytes memory) {
@@ -73,17 +86,18 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         returns (BalanceDelta liquidityDelta, BalanceDelta totalFeesAccrued)
     {
         (liquidityDelta, totalFeesAccrued) = manager.modifyLiquidity(
-            range.key,
+            range.poolKey,
             IPoolManager.ModifyLiquidityParams({
                 tickLower: range.tickLower,
                 tickUpper: range.tickUpper,
                 liquidityDelta: liquidityChange,
-                salt: range.key.hooks.getLiquiditySalt(owner)
+                salt: range.poolKey.hooks.getLiquiditySalt(owner)
             }),
             hookData
         );
     }
 
+    /// @dev The open delta on the position manager is "safe" for auto-settle after this call. ie. any amounts that the caller does not owe or is not allowed to take is accounted for.
     function _increaseLiquidity(
         address owner,
         LiquidityRange memory range,
@@ -96,39 +110,76 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
 
         Position storage position = positions[owner][range.toId()];
 
-        // Account for fees that were potentially collected to other users on the same range.
-        BalanceDelta callerFeesAccrued = _updateFeeGrowth(range, position);
-        BalanceDelta feesToCollect = totalFeesAccrued - callerFeesAccrued;
-        range.key.currency0.take(manager, address(this), uint128(feesToCollect.amount0()), true);
-        range.key.currency1.take(manager, address(this), uint128(feesToCollect.amount1()), true);
-
-        // the delta applied from the above actions is liquidityDelta - feesToCollect, note that the actual total delta for the caller may be different because actions can be chained
-        BalanceDelta callerDelta = liquidityDelta - feesToCollect;
-
-        // update liquidity after feeGrowth is updated
-        position.liquidity += liquidityToAdd;
-
-        // Update the tokensOwed0 and tokensOwed1 values for the caller.
-        // if callerDelta < 0, existing fees were re-invested AND net new tokens are required for the liquidity increase
-        // if callerDelta == 0, existing fees were reinvested (autocompounded)
-        // if callerDelta > 0, some but not all existing fees were used to increase liquidity. Any remainder is added to the position's owed tokens
-        if (callerDelta.amount0() > 0) {
-            position.tokensOwed0 += uint128(callerDelta.amount0());
-            range.key.currency0.take(manager, address(this), uint128(callerDelta.amount0()), true);
-            callerDelta = toBalanceDelta(0, callerDelta.amount1());
-        } else {
-            position.tokensOwed0 = 0;
-        }
+        // Calculate the portion of the liquidityDelta that is attributable to the caller.
+        // We must account for fees that might be owed to other users on the same range.
+        (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) =
+            manager.getFeeGrowthInside(range.poolKey.toId(), range.tickLower, range.tickUpper);
 
-        if (callerDelta.amount1() > 0) {
-            position.tokensOwed1 += uint128(callerDelta.amount1());
-            range.key.currency1.take(manager, address(this), uint128(callerDelta.amount1()), true);
-            callerDelta = toBalanceDelta(callerDelta.amount0(), 0);
-        } else {
-            position.tokensOwed1 = 0;
+        BalanceDelta callerFeesAccrued = FeeMath.getFeesOwed(
+            feeGrowthInside0X128,
+            feeGrowthInside1X128,
+            position.feeGrowthInside0LastX128,
+            position.feeGrowthInside1LastX128,
+            position.liquidity
+        );
+
+        // 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.
+        (int128 callerDelta0, int128 callerDelta1) = totalFeesAccrued != callerFeesAccrued
+            ? _calculateCallerDeltas(liquidityDelta, totalFeesAccrued, callerFeesAccrued)
+            : (liquidityDelta.amount0(), liquidityDelta.amount1());
+
+        // Update position storage, sanitizing the tokensOwed values based on the actual funds owed to the caller.
+        // 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.
+        uint128 tokensOwed0 = 0;
+        uint128 tokensOwed1 = 0;
+        if (callerDelta0 > 0) {
+            tokensOwed0 = uint128(callerDelta0);
+            callerDelta0 = 0;
+        }
+        if (callerDelta1 > 0) {
+            tokensOwed1 = uint128(callerDelta1);
+            callerDelta1 = 0;
         }
 
-        return callerDelta;
+        position.updateTokensOwed(tokensOwed0, tokensOwed1);
+        position.addLiquidity(liquidityToAdd);
+        position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128);
+
+        return toBalanceDelta(callerDelta0, callerDelta1);
+    }
+
+    // Returns the new sanitized delta for 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 = totalFeesAccrued0 > callerFeesAccrued0
+            ? _calculateCallerDelta(liquidityDelta.amount0(), totalFeesAccrued0, callerFeesAccrued0)
+            : liquidityDelta0;
+
+        callerDelta1 = totalFeesAccrued1 > callerFeesAccrued1
+            ? _calculateCallerDelta(liquidityDelta.amount1(), totalFeesAccrued1, callerFeesAccrued1)
+            : liquidityDelta1;
+    }
+
+    function _calculateCallerDelta(int128 liquidityDelta, int128 totalFeesAccrued, int128 callerFeesAccrued)
+        private
+        pure
+        returns (int128 callerDelta)
+    {
+        unchecked {
+            int128 feesAccruedOutsideCaller = totalFeesAccrued - callerFeesAccrued;
+            callerDelta = liquidityDelta - feesAccruedOutsideCaller;
+        }
     }
 
     function _increaseLiquidityAndZeroOut(
@@ -137,9 +188,24 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         uint256 liquidityToAdd,
         bytes memory hookData,
         bool claims
-    ) internal returns (BalanceDelta delta) {
-        delta = _increaseLiquidity(owner, range, liquidityToAdd, hookData);
-        zeroOut(delta, range.key.currency0, range.key.currency1, owner, claims);
+    ) internal returns (BalanceDelta callerDelta) {
+        callerDelta = _increaseLiquidity(owner, range, liquidityToAdd, hookData);
+        _closeCallerDeltas(callerDelta, range.poolKey.currency0, range.poolKey.currency1, owner, claims);
+        _closeAllDeltas(range.poolKey.currency0, range.poolKey.currency1);
+    }
+
+    // When chaining many actions, this should be called at the very end to close out any open deltas owed to or by this contract for other users on the same range.
+    // This is safe because any amounts the caller should not pay or take have already been accounted for in closeCallerDeltas.
+    function _closeAllDeltas(Currency currency0, Currency currency1) internal {
+        int128 delta0 = int128(manager.currencyDelta(address(this), currency0));
+        int128 delta1 = int128(manager.currencyDelta(address(this), currency1));
+
+        // Mint a receipt for the tokens owed to this address.
+        if (delta0 > 0) currency0.take(manager, address(this), uint128(delta0), true);
+        if (delta1 > 0) currency1.take(manager, address(this), uint128(delta1), true);
+        // Burn the receipt for tokens owed to this address.
+        if (delta0 < 0) currency0.settle(manager, address(this), uint256(int256(-delta0)), true);
+        if (delta1 < 0) currency1.settle(manager, address(this), uint256(int256(-delta1)), true);
     }
 
     function _lockAndIncreaseLiquidity(
@@ -168,10 +234,10 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         // do NOT take tokens directly to the owner because this contract might be holding fees
         // that need to be paid out (position.tokensOwed)
         if (liquidityDelta.amount0() > 0) {
-            range.key.currency0.take(manager, address(this), uint128(liquidityDelta.amount0()), true);
+            range.poolKey.currency0.take(manager, address(this), uint128(liquidityDelta.amount0()), true);
         }
         if (liquidityDelta.amount1() > 0) {
-            range.key.currency1.take(manager, address(this), uint128(liquidityDelta.amount1()), true);
+            range.poolKey.currency1.take(manager, address(this), uint128(liquidityDelta.amount1()), true);
         }
 
         // when decreasing liquidity, the user collects: 1) principal liquidity, 2) new fees, 3) old fees (position.tokensOwed)
@@ -200,7 +266,8 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         bool claims
     ) internal returns (BalanceDelta delta) {
         delta = _decreaseLiquidity(owner, range, liquidityToRemove, hookData);
-        zeroOut(delta, range.key.currency0, range.key.currency1, owner, claims);
+        _closeCallerDeltas(delta, range.poolKey.currency0, range.poolKey.currency1, owner, claims);
+        _closeAllDeltas(range.poolKey.currency0, range.poolKey.currency1);
     }
 
     function _lockAndDecreaseLiquidity(
@@ -222,7 +289,7 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
     {
         (, BalanceDelta totalFeesAccrued) = _modifyLiquidity(owner, range, 0, hookData);
 
-        PoolKey memory key = range.key;
+        PoolKey memory key = range.poolKey;
         Position storage position = positions[owner][range.toId()];
 
         // take all fees first then distribute
@@ -249,7 +316,8 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         returns (BalanceDelta delta)
     {
         delta = _collect(owner, range, hookData);
-        zeroOut(delta, range.key.currency0, range.key.currency1, owner, claims);
+        _closeCallerDeltas(delta, range.poolKey.currency0, range.poolKey.currency1, owner, claims);
+        _closeAllDeltas(range.poolKey.currency0, range.poolKey.currency1);
     }
 
     function _lockAndCollect(address owner, LiquidityRange memory range, bytes memory hookData, bool claims)
@@ -261,21 +329,22 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         );
     }
 
+    // TODO: I deprecated this bc I liked to see the accounting in line in the top level function... and I like to do all the position updates at once.
+    //  can keep but should at at least use the position library in here.
     function _updateFeeGrowth(LiquidityRange memory range, Position storage position)
         internal
         returns (BalanceDelta _feesOwed)
     {
         (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) =
-            manager.getFeeGrowthInside(range.key.toId(), range.tickLower, range.tickUpper);
+            manager.getFeeGrowthInside(range.poolKey.toId(), range.tickLower, range.tickUpper);
 
-        (uint128 token0Owed, uint128 token1Owed) = FeeMath.getFeesOwed(
+        _feesOwed = FeeMath.getFeesOwed(
             feeGrowthInside0X128,
             feeGrowthInside1X128,
             position.feeGrowthInside0LastX128,
             position.feeGrowthInside1LastX128,
             position.liquidity
         );
-        _feesOwed = toBalanceDelta(uint256(token0Owed).toInt128(), uint256(token1Owed).toInt128());
 
         position.feeGrowthInside0LastX128 = feeGrowthInside0X128;
         position.feeGrowthInside1LastX128 = feeGrowthInside1X128;
@@ -290,15 +359,10 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback {
         Position memory position = positions[owner][range.toId()];
 
         (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) =
-            manager.getFeeGrowthInside(range.key.toId(), range.tickLower, range.tickUpper);
+            manager.getFeeGrowthInside(range.poolKey.toId(), range.tickLower, range.tickUpper);
 
-        (token0Owed, token1Owed) = FeeMath.getFeesOwed(
-            feeGrowthInside0X128,
-            feeGrowthInside1X128,
-            position.feeGrowthInside0LastX128,
-            position.feeGrowthInside1LastX128,
-            position.liquidity
-        );
+        (token0Owed) = FeeMath.getFeeOwed(feeGrowthInside0X128, position.feeGrowthInside0LastX128, position.liquidity);
+        (token1Owed) = FeeMath.getFeeOwed(feeGrowthInside1X128, position.feeGrowthInside1LastX128, position.liquidity);
         token0Owed += position.tokensOwed0;
         token1Owed += position.tokensOwed1;
     }