posm : use actions router, remove return vals (#214)

* move to a base actions router

* use one Planner file

* msgSender

* use delta saving hook to get deltas

* remove return values

* add burn comment

* make gas snapshots more accurate, remove hook

* move to 1 planner, fix merge conf

* rename, add liquidityDelta return param

* rename

* comment

* comments

* remove SafeCallback

.forge-snapshots/PositionManager_burn_empty.snap

@@ -1 +1 @@
-50204
+47271

.forge-snapshots/PositionManager_burn_empty_native.snap

@@ -1 +1 @@
-50021
+47088

.forge-snapshots/PositionManager_burn_nonEmpty.snap

@@ -1 +1 @@
-135849
+130139

.forge-snapshots/PositionManager_burn_nonEmpty_native.snap

@@ -1 +1 @@
-128771
+123060

.forge-snapshots/PositionManager_collect.snap

@@ -1 +1 @@
-158136
+151223

.forge-snapshots/PositionManager_collect_native.snap

@@ -1 +1 @@
-149288
+142375

.forge-snapshots/PositionManager_collect_sameRange.snap

@@ -1 +1 @@
-158136
+151223

.forge-snapshots/PositionManager_decreaseLiquidity.snap

@@ -1 +1 @@
-123679
+116766

.forge-snapshots/PositionManager_decreaseLiquidity_native.snap

@@ -1 +1 @@
-114905
+109375

.forge-snapshots/PositionManager_decrease_burnEmpty.snap

@@ -1 +1 @@
-142176
+135178

.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap

@@ -1 +1 @@
-134916
+127917

.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap

@@ -1 +1 @@
-136395
+129482

.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap

@@ -1 +1 @@
-159575
+152726

.forge-snapshots/PositionManager_increaseLiquidity_native.snap

@@ -1 +1 @@
-141458
+134609

.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap

@@ -1 +1 @@
-142282
+135432

.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap

@@ -1 +1 @@
-178438
+171588

.forge-snapshots/PositionManager_mint.snap

@@ -1 +1 @@
-379791
+372827

.forge-snapshots/PositionManager_mint_native.snap

@@ -1 +1 @@
-344574
+337610

.forge-snapshots/PositionManager_mint_nativeWithSweep.snap

@@ -1 +1 @@
-351512
+344548

.forge-snapshots/PositionManager_mint_onSameTickLower.snap

@@ -1 +1 @@
-322473
+315509

.forge-snapshots/PositionManager_mint_onSameTickUpper.snap

@@ -1 +1 @@
-323115
+316151

.forge-snapshots/PositionManager_mint_sameRange.snap

@@ -1 +1 @@
-248697
+241733

.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap

@@ -1 +1 @@
-328491
+321527

.forge-snapshots/PositionManager_multicall_initialize_mint.snap

@@ -1 +1 @@
-424259
+417163

src/PositionManager.sol

@@ -15,21 +15,22 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"
 
 import {ERC721Permit} from "./base/ERC721Permit.sol";
 import {ReentrancyLock} from "./base/ReentrancyLock.sol";
-import {IPositionManager, Actions} from "./interfaces/IPositionManager.sol";
-import {SafeCallback} from "./base/SafeCallback.sol";
+import {IPositionManager} from "./interfaces/IPositionManager.sol";
 import {Multicall} from "./base/Multicall.sol";
 import {PoolInitializer} from "./base/PoolInitializer.sol";
 import {DeltaResolver} from "./base/DeltaResolver.sol";
 import {PositionConfig, PositionConfigLibrary} from "./libraries/PositionConfig.sol";
+import {BaseActionsRouter} from "./base/BaseActionsRouter.sol";
+import {Actions} from "./libraries/Actions.sol";
 
 contract PositionManager is
     IPositionManager,
     ERC721Permit,
     PoolInitializer,
     Multicall,
-    SafeCallback,
     DeltaResolver,
-    ReentrancyLock
+    ReentrancyLock,
+    BaseActionsRouter
 {
     using SafeTransferLib for *;
     using CurrencyLibrary for Currency;
@@ -48,7 +49,7 @@ contract PositionManager is
     IAllowanceTransfer public immutable permit2;
 
     constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2)
-        SafeCallback(_poolManager)
+        BaseActionsRouter(_poolManager)
         ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1")
     {
         permit2 = _permit2;
@@ -60,80 +61,63 @@ contract PositionManager is
     }
 
     /// @param unlockData is an encoding of actions, params, and currencies
-    /// @return returnData is the endocing of each actions return information
+    /// @param deadline is the timestamp at which the unlockData will no longer be valid
     function modifyLiquidities(bytes calldata unlockData, uint256 deadline)
         external
         payable
         isNotLocked
         checkDeadline(deadline)
-        returns (bytes[] memory)
     {
-        // TODO: Edit the encoding/decoding.
-        return abi.decode(poolManager.unlock(unlockData), (bytes[]));
+        _executeActions(unlockData);
     }
 
-    function _unlockCallback(bytes calldata payload) internal override returns (bytes memory) {
-        (Actions[] memory actions, bytes[] memory params) = abi.decode(payload, (Actions[], bytes[]));
-
-        bytes[] memory returnData = _dispatch(actions, params);
-
-        return abi.encode(returnData);
+    function _handleAction(uint256 action, bytes calldata params) internal override {
+        if (action == Actions.INCREASE_LIQUIDITY) {
+            _increase(params);
+        } else if (action == Actions.DECREASE_LIQUIDITY) {
+            _decrease(params);
+        } else if (action == Actions.MINT_POSITION) {
+            _mint(params);
+        } else if (action == Actions.CLOSE_CURRENCY) {
+            _close(params);
+        } else if (action == Actions.BURN_POSITION) {
+            // Will automatically decrease liquidity to 0 if the position is not already empty.
+            _burn(params);
+        } else {
+            revert UnsupportedAction(action);
+        }
     }
 
-    function _dispatch(Actions[] memory actions, bytes[] memory params) internal returns (bytes[] memory returnData) {
-        uint256 length = actions.length;
-        if (length != params.length) revert MismatchedLengths();
-        returnData = new bytes[](length);
-        for (uint256 i; i < length; i++) {
-            if (actions[i] == Actions.INCREASE) {
-                returnData[i] = _increase(params[i]);
-            } else if (actions[i] == Actions.DECREASE) {
-                returnData[i] = _decrease(params[i]);
-            } else if (actions[i] == Actions.MINT) {
-                // TODO: Mint will be coupled with increase.
-                returnData[i] = _mint(params[i]);
-            } else if (actions[i] == Actions.CLOSE_CURRENCY) {
-                returnData[i] = _close(params[i]);
-            } else if (actions[i] == Actions.BURN) {
-                // Will automatically decrease liquidity to 0 if the position is not already empty.
-                returnData[i] = _burn(params[i]);
-            } else {
-                revert UnsupportedAction();
-            }
-        }
+    function _msgSender() internal view override returns (address) {
+        return _getLocker();
     }
 
     /// @param params is an encoding of uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes hookData
-    /// @return returns an encoding of the BalanceDelta applied by this increase call, including credited fees.
     /// @dev Calling increase with 0 liquidity will credit the caller with any underlying fees of the position
-    function _increase(bytes memory params) internal returns (bytes memory) {
+    function _increase(bytes memory params) internal {
         (uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) =
             abi.decode(params, (uint256, PositionConfig, uint256, bytes));
 
         if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
         // Note: The tokenId is used as the salt for this position, so every minted position has unique storage in the pool manager.
-        BalanceDelta delta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
-        return abi.encode(delta);
+        BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
     }
 
     /// @param params is an encoding of uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes hookData
-    /// @return returns an encoding of the BalanceDelta applied by this increase call, including credited fees.
     /// @dev Calling decrease with 0 liquidity will credit the caller with any underlying fees of the position
-    function _decrease(bytes memory params) internal returns (bytes memory) {
+    function _decrease(bytes memory params) internal {
         (uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) =
             abi.decode(params, (uint256, PositionConfig, uint256, bytes));
 
-        if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker());
+        if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender());
         if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
 
         // Note: the tokenId is used as the salt.
-        BalanceDelta delta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
-        return abi.encode(delta);
+        BalanceDelta liquidityDelta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
     }
 
     /// @param params is an encoding of PositionConfig memory config, uint256 liquidity, address recipient, bytes hookData where recipient is the receiver / owner of the ERC721
-    /// @return returns an encoding of the BalanceDelta from the initial increase
-    function _mint(bytes memory params) internal returns (bytes memory) {
+    function _mint(bytes memory params) internal {
         (PositionConfig memory config, uint256 liquidity, address owner, bytes memory hookData) =
             abi.decode(params, (PositionConfig, uint256, address, bytes));
 
@@ -146,23 +130,20 @@ contract PositionManager is
         _mint(owner, tokenId);
 
         // _beforeModify is not called here because the tokenId is newly minted
-        BalanceDelta delta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
+        BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
 
         positionConfigs[tokenId] = config.toId();
-
-        return abi.encode(delta);
     }
 
     /// @param params is an encoding of the Currency to close
-    /// @return bytes an encoding of int256 the balance of the currency being settled by this call
-    function _close(bytes memory params) internal returns (bytes memory) {
+    function _close(bytes memory params) internal {
         (Currency currency) = abi.decode(params, (Currency));
         // this address has applied all deltas on behalf of the user/owner
         // it is safe to close this entire delta because of slippage checks throughout the batched calls.
         int256 currencyDelta = poolManager.currencyDelta(address(this), currency);
 
         // the locker is the payer or receiver
-        address caller = _getLocker();
+        address caller = _msgSender();
         if (currencyDelta < 0) {
             _settle(currency, caller, uint256(-currencyDelta));
 
@@ -171,29 +152,27 @@ contract PositionManager is
         } else if (currencyDelta > 0) {
             _take(currency, caller, uint256(currencyDelta));
         }
-
-        return abi.encode(currencyDelta);
     }
 
     /// @param params is an encoding of uint256 tokenId, PositionConfig memory config, bytes hookData
     /// @dev this is overloaded with ERC721Permit._burn
-    function _burn(bytes memory params) internal returns (bytes memory) {
+    function _burn(bytes memory params) internal {
         (uint256 tokenId, PositionConfig memory config, bytes memory hookData) =
             abi.decode(params, (uint256, PositionConfig, bytes));
 
-        if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker());
+        if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender());
         if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
         uint256 liquidity = uint256(_getPositionLiquidity(config, tokenId));
 
+        BalanceDelta liquidityDelta;
         // Can only call modify if there is non zero liquidity.
-        BalanceDelta delta;
-
-        if (liquidity > 0) delta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
+        if (liquidity > 0) {
+            liquidityDelta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
+        }
 
         delete positionConfigs[tokenId];
         // Burn the token.
         _burn(tokenId);
-        return abi.encode(delta);
     }
 
     function _modifyLiquidity(PositionConfig memory config, int256 liquidityChange, bytes32 salt, bytes memory hookData)

src/base/BaseActionsRouter.sol

@@ -20,8 +20,8 @@ abstract contract BaseActionsRouter is SafeCallback {
 
     /// @notice internal function that triggers the execution of a set of actions on v4
     /// @dev inheriting contracts should call this function to trigger execution
-    function _executeActions(bytes calldata params) internal {
-        poolManager.unlock(params);
+    function _executeActions(bytes calldata unlockData) internal {
+        poolManager.unlock(unlockData);
     }
 
     /// @notice function that is called by the PoolManager through the SafeCallback.unlockCallback
@@ -38,7 +38,6 @@ abstract contract BaseActionsRouter is SafeCallback {
             _handleAction(action, params[actionIndex]);
         }
 
-        // TODO do we want to return anything?
         return "";
     }
 

src/interfaces/IPositionManager.sol

@@ -3,20 +3,9 @@ pragma solidity ^0.8.24;
 
 import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
 
-enum Actions {
-    MINT,
-    BURN,
-    INCREASE,
-    DECREASE,
-    // Any positive delta on a currency will be sent to specified address
-    CLOSE_CURRENCY
-}
-
 interface IPositionManager {
-    error MismatchedLengths();
     error NotApproved(address caller);
     error DeadlinePassed();
-    error UnsupportedAction();
     error IncorrectPositionConfigForTokenId(uint256 tokenId);
 
     /// @notice Maps the ERC721 tokenId to a configId, which is a keccak256 hash of the position's pool key, and range (tickLower, tickUpper)
@@ -28,8 +17,7 @@ interface IPositionManager {
     /// @notice Batches many liquidity modification calls to pool manager
     /// @param payload is an encoding of actions, and parameters for those actions
     /// @param deadline is the deadline for the batched actions to be executed
-    /// @return returnData is the endocing of each actions return information
-    function modifyLiquidities(bytes calldata payload, uint256 deadline) external payable returns (bytes[] memory);
+    function modifyLiquidities(bytes calldata payload, uint256 deadline) external payable;
 
     function nextTokenId() external view returns (uint256);
 }

src/libraries/Actions.sol

@@ -28,4 +28,8 @@ library Actions {
     // minting/burning 6909s to close deltas
     uint256 constant MINT_6909 = 0x20;
     uint256 constant BURN_6909 = 0x21;
+
+    // mint + burn ERC721 position
+    uint256 constant MINT_POSITION = 0x22;
+    uint256 constant BURN_POSITION = 0x23;
 }