Rip Out Vanilla

.forge-snapshots/autocompound_exactUnclaimedFees.snap

@@ -1 +1 @@
-300126
+293482

.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap

@@ -1 +1 @@
-239588
+225841

.forge-snapshots/autocompound_excessFeesCredit.snap

@@ -1 +1 @@
-320665
+314021

.forge-snapshots/decreaseLiquidity_erc20.snap

@@ -1 +1 @@
-215187
+208541

.forge-snapshots/decreaseLiquidity_erc6909.snap

@@ -1 +1 @@
-215199
+208553

.forge-snapshots/increaseLiquidity_erc20.snap

@@ -1 +1 @@
-195029
+194298

.forge-snapshots/increaseLiquidity_erc6909.snap

@@ -1 +1 @@
-195041
+194310

.forge-snapshots/mintWithLiquidity.snap

@@ -1 +1 @@
-512049
+513250

contracts/NonfungiblePositionManager.sol

@@ -38,8 +38,9 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
     // maps the ERC721 tokenId to the keys that uniquely identify a liquidity position (owner, range)
     mapping(uint256 tokenId => TokenPosition position) public tokenPositions;
 
-    // TODO: TSTORE this jawn
+    // TODO: TSTORE these jawns
     address internal msgSender;
+    bool internal unlockedByThis;
 
     // TODO: Context is inherited through ERC721 and will be not useful to use _msgSender() which will be address(this) with our current mutlicall.
     function _msgSenderInternal() internal override returns (address) {
@@ -51,35 +52,34 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         ERC721Permit("Uniswap V4 Positions NFT-V1", "UNI-V4-POS", "1")
     {}
 
-    function unlockAndExecute(bytes[] memory data, Currency[] memory currencies) public returns (bytes memory) {
+    function unlockAndExecute(bytes[] memory data, Currency[] memory currencies) public returns (int128[] memory) {
         msgSender = msg.sender;
-        return manager.unlock(abi.encode(data, currencies));
+        unlockedByThis = true;
+        return abi.decode(manager.unlock(abi.encode(data, currencies)), (int128[]));
     }
 
     function _unlockCallback(bytes calldata payload) internal override returns (bytes memory) {
         (bytes[] memory data, Currency[] memory currencies) = abi.decode(payload, (bytes[], Currency[]));
 
         bool success;
-        bytes memory returnData;
+
         for (uint256 i; i < data.length; i++) {
             // TODO: bubble up the return
-            (success, returnData) = address(this).call(data[i]);
+            (success,) = address(this).call(data[i]);
             if (!success) revert("EXECUTE_FAILED");
         }
 
         // close the deltas
+        int128[] memory returnData = new int128[](currencies.length);
         for (uint256 i; i < currencies.length; i++) {
-            currencies[i].close(manager, msgSender);
-            currencies[i].close(manager, address(this));
+            returnData[i] = currencies[i].close(manager, msgSender, false); // TODO: support claims
+            currencies[i].close(manager, address(this), true); // position manager always takes 6909
         }
 
         // Should just be returning the netted amount that was settled on behalf of the caller (msgSender)
-        // And any recipient deltas settled earlier.
-
-        // TODO: @sara handle the return
-        // vanilla: return int128[2]
-        // batch: return int128[data.length]
-        return returnData;
+        // TODO: any recipient deltas settled earlier.
+        // @comment sauce: i dont think we can return recipient deltas since we cant parse the payload
+        return abi.encode(returnData);
     }
 
     // NOTE: more gas efficient as LiquidityAmounts is used offchain
@@ -90,26 +90,13 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         uint256 deadline,
         address owner,
         bytes calldata hookData
-    ) public payable returns (BalanceDelta delta) {
-        // TODO: optimization, read/write manager.isUnlocked to avoid repeated external calls for batched execution
-        if (manager.isUnlocked()) {
-            _increaseLiquidity(owner, range, liquidity, hookData);
-
-            // mint receipt token
-            uint256 tokenId;
-            _mint(owner, (tokenId = nextTokenId++));
-            tokenPositions[tokenId] = TokenPosition({owner: owner, range: range});
-        } else {
-            msgSender = msg.sender;
-            bytes[] memory data = new bytes[](1);
-            data[0] = abi.encodeWithSelector(this.mint.selector, range, liquidity, deadline, owner, hookData);
-
-            Currency[] memory currencies = new Currency[](2);
-            currencies[0] = range.poolKey.currency0;
-            currencies[1] = range.poolKey.currency1;
-            bytes memory result = unlockAndExecute(data, currencies);
-            delta = abi.decode(result, (BalanceDelta));
-        }
+    ) external payable onlyIfUnlocked {
+        _increaseLiquidity(owner, range, liquidity, hookData);
+
+        // mint receipt token
+        uint256 tokenId;
+        _mint(owner, (tokenId = nextTokenId++));
+        tokenPositions[tokenId] = TokenPosition({owner: owner, range: range});
     }
 
     // NOTE: more expensive since LiquidityAmounts is used onchain
@@ -135,43 +122,21 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
     function increaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
         external
         isAuthorizedForToken(tokenId)
-        returns (BalanceDelta delta)
+        onlyIfUnlocked
     {
         TokenPosition memory tokenPos = tokenPositions[tokenId];
 
-        if (manager.isUnlocked()) {
-            _increaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
-        } else {
-            bytes[] memory data = new bytes[](1);
-            data[0] = abi.encodeWithSelector(this.increaseLiquidity.selector, tokenId, liquidity, hookData, claims);
-
-            Currency[] memory currencies = new Currency[](2);
-            currencies[0] = tokenPos.range.poolKey.currency0;
-            currencies[1] = tokenPos.range.poolKey.currency1;
-            bytes memory result = unlockAndExecute(data, currencies);
-            delta = abi.decode(result, (BalanceDelta));
-        }
+        _increaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
     }
 
     function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
-        public
+        external
         isAuthorizedForToken(tokenId)
-        returns (BalanceDelta delta)
+        onlyIfUnlocked
     {
         TokenPosition memory tokenPos = tokenPositions[tokenId];
 
-        if (manager.isUnlocked()) {
-            _decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
-        } else {
-            bytes[] memory data = new bytes[](1);
-            data[0] = abi.encodeWithSelector(this.decreaseLiquidity.selector, tokenId, liquidity, hookData, claims);
-
-            Currency[] memory currencies = new Currency[](2);
-            currencies[0] = tokenPos.range.poolKey.currency0;
-            currencies[1] = tokenPos.range.poolKey.currency1;
-            bytes memory result = unlockAndExecute(data, currencies);
-            delta = abi.decode(result, (BalanceDelta));
-        }
+        _decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
     }
 
     // TODO return type?
@@ -188,22 +153,12 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
 
     // TODO: in v3, we can partially collect fees, but what was the usecase here?
     function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims)
-        public
-        returns (BalanceDelta delta)
+        external
+        onlyIfUnlocked
     {
         TokenPosition memory tokenPos = tokenPositions[tokenId];
-        if (manager.isUnlocked()) {
-            _collect(recipient, tokenPos.range, hookData);
-        } else {
-            bytes[] memory data = new bytes[](1);
-            data[0] = abi.encodeWithSelector(this.collect.selector, tokenId, recipient, hookData, claims);
-
-            Currency[] memory currencies = new Currency[](2);
-            currencies[0] = tokenPos.range.poolKey.currency0;
-            currencies[1] = tokenPos.range.poolKey.currency1;
-            bytes memory result = unlockAndExecute(data, currencies);
-            delta = abi.decode(result, (BalanceDelta));
-        }
+
+        _collect(recipient, tokenPos.owner, tokenPos.range, hookData);
     }
 
     function feesOwed(uint256 tokenId) external view returns (uint256 token0Owed, uint256 token1Owed) {
@@ -234,4 +189,9 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         require(msg.sender == address(this) || _isApprovedOrOwner(msg.sender, tokenId), "Not approved");
         _;
     }
+
+    modifier onlyIfUnlocked() {
+        if (!unlockedByThis) revert MustBeUnlockedByThisContract();
+        _;
+    }
 }

contracts/base/BaseLiquidityManagement.sol

@@ -51,27 +51,6 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
 
     function _msgSenderInternal() internal virtual returns (address);
 
-    function _closeCallerDeltas(
-        BalanceDelta callerDeltas,
-        Currency currency0,
-        Currency currency1,
-        address owner,
-        bool claims
-    ) internal {
-        int128 callerDelta0 = callerDeltas.amount0();
-        int128 callerDelta1 = callerDeltas.amount1();
-        // On liquidity increase, the deltas should never be > 0.
-        //  We always 0 out a caller positive delta because it is instead accounted for in position.tokensOwed.
-
-        if (callerDelta0 < 0) currency0.settle(manager, owner, uint256(int256(-callerDelta0)), claims);
-        else if (callerDelta0 > 0) currency0.take(manager, owner, uint128(callerDelta0), claims);
-
-        if (callerDelta1 < 0) currency1.settle(manager, owner, uint256(int256(-callerDelta1)), claims);
-        else if (callerDelta1 > 0) currency1.take(manager, owner, uint128(callerDelta1), claims);
-
-        owner.close(currency0, currency1);
-    }
-
     function _modifyLiquidity(address owner, LiquidityRange memory range, int256 liquidityChange, bytes memory hookData)
         internal
         returns (BalanceDelta liquidityDelta, BalanceDelta totalFeesAccrued)
@@ -131,22 +110,6 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
         position.addLiquidity(liquidityToAdd);
     }
 
-    // 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 _closeThisDeltas(BalanceDelta delta, Currency currency0, Currency currency1) internal {
-        int128 delta0 = delta.amount0();
-        int128 delta1 = delta.amount1();
-
-        // 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);
-
-        address(this).close(currency0, currency1);
-    }
-
     function _moveCallerDeltaToTokensOwed(
         bool useAmount0,
         BalanceDelta tokensOwed,
@@ -206,7 +169,7 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
     }
 
     // The recipient may not be the original owner.
-    function _collect(address owner, LiquidityRange memory range, bytes memory hookData) internal {
+    function _collect(address recipient, address owner, LiquidityRange memory range, bytes memory hookData) internal {
         BalanceDelta callerDelta;
         BalanceDelta thisDelta;
         Position storage position = positions[owner][range.toId()];
@@ -233,7 +196,11 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
         callerDelta = callerDelta + tokensOwed;
         thisDelta = thisDelta - tokensOwed;
 
-        callerDelta.flush(owner, range.poolKey.currency0, range.poolKey.currency1);
+        if (recipient == _msgSenderInternal()) {
+            callerDelta.flush(recipient, range.poolKey.currency0, range.poolKey.currency1);
+        } else {
+            callerDelta.closeDelta(manager, recipient, range.poolKey.currency0, range.poolKey.currency1, false); // TODO: allow recipient to receive claims, and add test!
+        }
         thisDelta.flush(address(this), range.poolKey.currency0, range.poolKey.currency1);
 
         position.clearTokensOwed();

contracts/interfaces/INonfungiblePositionManager.sol

@@ -11,14 +11,16 @@ interface INonfungiblePositionManager {
         LiquidityRange range;
     }
 
+    error MustBeUnlockedByThisContract();
+
     // NOTE: more gas efficient as LiquidityAmounts is used offchain
     function mint(
         LiquidityRange calldata position,
         uint256 liquidity,
         uint256 deadline,
         address recipient,
         bytes calldata hookData
-    ) external payable returns (BalanceDelta delta);
+    ) external payable;
 
     // NOTE: more expensive since LiquidityAmounts is used onchain
     // function mint(MintParams calldata params) external payable returns (uint256 tokenId, BalanceDelta delta);
@@ -28,20 +30,14 @@ interface INonfungiblePositionManager {
     /// @param liquidity The amount of liquidity to add
     /// @param hookData Arbitrary data passed to the hook
     /// @param claims Whether the liquidity increase uses ERC-6909 claim tokens
-    /// @return delta Corresponding balance changes as a result of increasing liquidity
-    function increaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
-        external
-        returns (BalanceDelta delta);
+    function increaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims) external;
 
     /// @notice Decrease liquidity for an existing position
     /// @param tokenId The ID of the position
     /// @param liquidity The amount of liquidity to remove
     /// @param hookData Arbitrary data passed to the hook
     /// @param claims Whether the removed liquidity is sent as ERC-6909 claim tokens
-    /// @return delta Corresponding balance changes as a result of decreasing liquidity applied to user (number of tokens credited to tokensOwed)
-    function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
-        external
-        returns (BalanceDelta delta);
+    function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims) external;
 
     // TODO Can decide if we want burn to auto encode a decrease/collect.
     /// @notice Burn a position and delete the tokenId
@@ -56,15 +52,12 @@ interface INonfungiblePositionManager {
     /// @param recipient The address to send the collected tokens to
     /// @param hookData Arbitrary data passed to the hook
     /// @param claims Whether the collected fees are sent as ERC-6909 claim tokens
-    /// @return delta Corresponding balance changes as a result of collecting fees
-    function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims)
-        external
-        returns (BalanceDelta delta);
+    function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims) external;
 
     /// @notice Execute a batch of external calls by unlocking the PoolManager
     /// @param data an array of abi.encodeWithSelector(<selector>, <args>) for each call
     /// @return delta The final delta changes of the caller
-    function unlockAndExecute(bytes[] memory data, Currency[] memory currencies) external returns (bytes memory);
+    function unlockAndExecute(bytes[] memory data, Currency[] memory currencies) external returns (int128[] memory);
 
     /// @notice Returns the fees owed for a position. Includes unclaimed fees + custodied fees + claimable fees
     /// @param tokenId The ID of the position