Rip Out Vanilla

.forge-snapshots/autocompound_exactUnclaimedFees.snap

@@ -1 +1 @@
-300126
+299532

.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap

@@ -1 +1 @@
-239588
+238996

.forge-snapshots/autocompound_excessFeesCredit.snap

@@ -1 +1 @@
-320665
+320071

.forge-snapshots/decreaseLiquidity_erc20.snap

@@ -1 +1 @@
-215187
+214591

.forge-snapshots/decreaseLiquidity_erc6909.snap

@@ -1 +1 @@
-215199
+214603

.forge-snapshots/increaseLiquidity_erc20.snap

@@ -1 +1 @@
-195029
+194748

.forge-snapshots/increaseLiquidity_erc6909.snap

@@ -1 +1 @@
-195041
+194760

.forge-snapshots/mintWithLiquidity.snap

@@ -1 +1 @@
-512049
+513700

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);
+            returnData[i] = currencies[i].close(manager, msgSender);
             currencies[i].close(manager, address(this));
         }
 
         // 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

@@ -206,7 +206,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 +233,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);
+        }
         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

contracts/libraries/TransientLiquidityDelta.sol

@@ -54,19 +54,18 @@ library TransientLiquidityDelta {
         }
     }
 
-    function close(Currency currency, IPoolManager manager, address holder) internal {
+    function close(Currency currency, IPoolManager manager, address holder) internal returns (int128 delta) {
         // getDelta(currency, holder);
         bytes32 hashSlot = _computeSlot(holder, currency);
-        int256 delta;
         assembly {
             delta := tload(hashSlot)
         }
 
         // TODO support claims field
         if (delta < 0) {
-            currency.settle(manager, holder, uint256(-delta), false);
+            currency.settle(manager, holder, uint256(-int256(delta)), false);
         } else {
-            currency.take(manager, holder, uint256(delta), false);
+            currency.take(manager, holder, uint256(int256(delta)), false);
         }
 
         // setDelta(0);
@@ -75,6 +74,17 @@ library TransientLiquidityDelta {
         }
     }
 
+    function closeDelta(
+        BalanceDelta delta,
+        IPoolManager manager,
+        address holder,
+        Currency currency0,
+        Currency currency1
+    ) internal {
+        close(currency0, manager, holder);
+        close(currency1, manager, holder);
+    }
+
     function getBalanceDelta(address holder, Currency currency0, Currency currency1)
         internal
         view