move sub unsub (#287)

* move sub unsub

* use struct

* pass in bytes to setConfigId

* ...

.forge-snapshots/PositionManager_mint_native.snap

@@ -1 +1 @@
-341062
+341067

.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap

@@ -1 +1 @@
-349554
+349559

.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap

@@ -1 +1 @@
-348856
+348861

.forge-snapshots/PositionManager_mint_onSameTickLower.snap

@@ -1 +1 @@
-319044
+319049

.forge-snapshots/PositionManager_mint_onSameTickUpper.snap

@@ -1 +1 @@
-319686
+319691

.forge-snapshots/PositionManager_mint_sameRange.snap

@@ -1 +1 @@
-245268
+245273

.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap

@@ -1 +1 @@
-375086
+375091

.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap

@@ -1 +1 @@
-325062
+325067

.forge-snapshots/PositionManager_mint_withClose.snap

@@ -1 +1 @@
-376362
+376367

.forge-snapshots/PositionManager_mint_withSettlePair.snap

@@ -1 +1 @@
-375502
+375507

.forge-snapshots/PositionManager_multicall_initialize_mint.snap

@@ -1 +1 @@
-420836
+420841

src/PositionManager.sol

@@ -10,7 +10,6 @@ import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
 import {Position} from "@uniswap/v4-core/src/libraries/Position.sol";
 import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";
 import {TransientStateLibrary} from "@uniswap/v4-core/src/libraries/TransientStateLibrary.sol";
-import {Position} from "@uniswap/v4-core/src/libraries/Position.sol";
 import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
 import {ERC20} from "solmate/src/tokens/ERC20.sol";
 import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol";
@@ -29,6 +28,7 @@ import {CalldataDecoder} from "./libraries/CalldataDecoder.sol";
 import {INotifier} from "./interfaces/INotifier.sol";
 import {Permit2Forwarder} from "./base/Permit2Forwarder.sol";
 import {SlippageCheckLibrary} from "./libraries/SlippageCheck.sol";
+import {PositionConfigId, PositionConfigIdLibrary} from "./libraries/PositionConfigId.sol";
 
 contract PositionManager is
     IPositionManager,
@@ -44,18 +44,24 @@ contract PositionManager is
     using SafeTransferLib for *;
     using CurrencyLibrary for Currency;
     using PoolIdLibrary for PoolKey;
-    using PositionConfigLibrary for *;
+    using PositionConfigLibrary for PositionConfig;
     using StateLibrary for IPoolManager;
     using TransientStateLibrary for IPoolManager;
     using SafeCast for uint256;
     using SafeCast for int256;
     using CalldataDecoder for bytes;
     using SlippageCheckLibrary for BalanceDelta;
+    using PositionConfigIdLibrary for PositionConfigId;
 
     /// @dev The ID of the next token that will be minted. Skips 0
     uint256 public nextTokenId = 1;
 
-    mapping(uint256 tokenId => bytes32 config) private positionConfigs;
+    mapping(uint256 tokenId => PositionConfigId configId) internal positionConfigs;
+
+    /// @notice an internal getter for PositionConfigId to be used by Notifier
+    function _positionConfigs(uint256 tokenId) internal view override returns (PositionConfigId storage) {
+        return positionConfigs[tokenId];
+    }
 
     constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2)
         BaseActionsRouter(_poolManager)
@@ -75,16 +81,16 @@ contract PositionManager is
     /// @param tokenId the unique identifier of the ERC721 token
     /// @dev either msg.sender or _msgSender() is passed in as the caller
     /// _msgSender() should ONLY be used if this is being called from within the unlockCallback
-    modifier onlyIfApproved(address caller, uint256 tokenId) {
+    modifier onlyIfApproved(address caller, uint256 tokenId) override {
         if (!_isApprovedOrOwner(caller, tokenId)) revert NotApproved(caller);
         _;
     }
 
     /// @notice Reverts if the hash of the config does not equal the saved hash
     /// @param tokenId the unique identifier of the ERC721 token
     /// @param config the PositionConfig to check against
-    modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) {
-        if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
+    modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) override {
+        if (positionConfigs[tokenId].getConfigId() != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
         _;
     }
 
@@ -107,29 +113,6 @@ contract PositionManager is
         _executeActionsWithoutUnlock(actions, params);
     }
 
-    /// @inheritdoc INotifier
-    function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber, bytes calldata data)
-        external
-        payable
-        onlyIfApproved(msg.sender, tokenId)
-        onlyValidConfig(tokenId, config)
-    {
-        // call to _subscribe will revert if the user already has a sub
-        positionConfigs.setSubscribe(tokenId);
-        _subscribe(tokenId, config, subscriber, data);
-    }
-
-    /// @inheritdoc INotifier
-    function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data)
-        external
-        payable
-        onlyIfApproved(msg.sender, tokenId)
-        onlyValidConfig(tokenId, config)
-    {
-        positionConfigs.setUnsubscribe(tokenId);
-        _unsubscribe(tokenId, config, data);
-    }
-
     function msgSender() public view override returns (address) {
         return _getLocker();
     }
@@ -260,7 +243,7 @@ contract PositionManager is
             _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
         // Slippage checks should be done on the principal liquidityDelta which is the liquidityDelta - feesAccrued
         (liquidityDelta - feesAccrued).validateMaxIn(amount0Max, amount1Max);
-        positionConfigs.setConfigId(tokenId, config);
+        positionConfigs[tokenId].setConfigId(config.toId());
 
         emit MintPosition(tokenId, config);
     }
@@ -351,7 +334,7 @@ contract PositionManager is
             hookData
         );
 
-        if (positionConfigs.hasSubscriber(uint256(salt))) {
+        if (positionConfigs[uint256(salt)].hasSubscriber()) {
             _notifyModifyLiquidity(uint256(salt), config, liquidityChange, feesAccrued);
         }
     }
@@ -369,7 +352,7 @@ contract PositionManager is
     /// @dev overrides solmate transferFrom in case a notification to subscribers is needed
     function transferFrom(address from, address to, uint256 id) public virtual override {
         super.transferFrom(from, to, id);
-        if (positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to);
+        if (positionConfigs[id].hasSubscriber()) _notifyTransfer(id, from, to);
     }
 
     /// @inheritdoc IPositionManager
@@ -385,11 +368,6 @@ contract PositionManager is
 
     /// @inheritdoc IPositionManager
     function getPositionConfigId(uint256 tokenId) external view returns (bytes32) {
-        return positionConfigs.getConfigId(tokenId);
-    }
-
-    /// @inheritdoc INotifier
-    function hasSubscriber(uint256 tokenId) external view returns (bool) {
-        return positionConfigs.hasSubscriber(tokenId);
+        return positionConfigs[tokenId].getConfigId();
     }
 }

src/base/Notifier.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.24;
 
 import {ISubscriber} from "../interfaces/ISubscriber.sol";
 import {PositionConfig} from "../libraries/PositionConfig.sol";
+import {PositionConfigId, PositionConfigIdLibrary} from "../libraries/PositionConfigId.sol";
 import {BipsLibrary} from "../libraries/BipsLibrary.sol";
 import {INotifier} from "../interfaces/INotifier.sol";
 import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol";
@@ -12,6 +13,7 @@ import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
 abstract contract Notifier is INotifier {
     using BipsLibrary for uint256;
     using CustomRevert for bytes4;
+    using PositionConfigIdLibrary for PositionConfigId;
 
     error AlreadySubscribed(address subscriber);
 
@@ -27,9 +29,20 @@ abstract contract Notifier is INotifier {
 
     mapping(uint256 tokenId => ISubscriber subscriber) public subscriber;
 
-    function _subscribe(uint256 tokenId, PositionConfig memory config, address newSubscriber, bytes memory data)
-        internal
+    modifier onlyIfApproved(address caller, uint256 tokenId) virtual;
+    modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) virtual;
+
+    function _positionConfigs(uint256 tokenId) internal view virtual returns (PositionConfigId storage);
+
+    /// @inheritdoc INotifier
+    function subscribe(uint256 tokenId, PositionConfig calldata config, address newSubscriber, bytes calldata data)
+        external
+        payable
+        onlyIfApproved(msg.sender, tokenId)
+        onlyValidConfig(tokenId, config)
     {
+        // will revert below if the user already has a subcriber
+        _positionConfigs(tokenId).setSubscribe();
         ISubscriber _subscriber = subscriber[tokenId];
 
         if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(address(_subscriber));
@@ -46,8 +59,14 @@ abstract contract Notifier is INotifier {
         emit Subscribed(tokenId, address(newSubscriber));
     }
 
-    /// @dev Must always allow a user to unsubscribe. In the case of a malicious subscriber, a user can always unsubscribe safely, ensuring liquidity is always modifiable.
-    function _unsubscribe(uint256 tokenId, PositionConfig memory config, bytes memory data) internal {
+    /// @inheritdoc INotifier
+    function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data)
+        external
+        payable
+        onlyIfApproved(msg.sender, tokenId)
+        onlyValidConfig(tokenId, config)
+    {
+        _positionConfigs(tokenId).setUnsubscribe();
         ISubscriber _subscriber = subscriber[tokenId];
 
         uint256 subscriberGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS);
@@ -96,4 +115,9 @@ abstract contract Notifier is INotifier {
             success := call(gas(), target, 0, add(encodedCall, 0x20), mload(encodedCall), 0, 0)
         }
     }
+
+    /// @inheritdoc INotifier
+    function hasSubscriber(uint256 tokenId) external view returns (bool) {
+        return _positionConfigs(tokenId).hasSubscriber();
+    }
 }

src/interfaces/INotifier.sol

@@ -28,6 +28,7 @@ interface INotifier {
     /// @param config the corresponding PositionConfig for the tokenId
     /// @param data caller-provided data that's forwarded to the subscriber contract
     /// @dev payable so it can be multicalled with NATIVE related actions
+    /// @dev Must always allow a user to unsubscribe. In the case of a malicious subscriber, a user can always unsubscribe safely, ensuring liquidity is always modifiable.
     function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data) external payable;
 
     /// @notice Returns whether a a position should call out to notify a subscribing contract on modification or transfer

src/libraries/PositionConfig.sol

@@ -10,49 +10,8 @@ struct PositionConfig {
     int24 tickUpper;
 }
 
-/// @notice Library to get and set the PositionConfigId and subscriber status for a given tokenId
+/// @notice Library to calculate the PositionConfigId from the PositionConfig struct
 library PositionConfigLibrary {
-    using PositionConfigLibrary for PositionConfig;
-
-    bytes32 constant MASK_UPPER_BIT = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
-    bytes32 constant DIRTY_UPPER_BIT = 0x8000000000000000000000000000000000000000000000000000000000000000;
-
-    /// @notice returns the truncated hash of the PositionConfig for a given tokenId
-    function getConfigId(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId)
-        internal
-        view
-        returns (bytes32 configId)
-    {
-        configId = positionConfigs[tokenId] & MASK_UPPER_BIT;
-    }
-
-    function setConfigId(
-        mapping(uint256 => bytes32) storage positionConfigs,
-        uint256 tokenId,
-        PositionConfig calldata config
-    ) internal {
-        positionConfigs[tokenId] = config.toId();
-    }
-
-    function setSubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal {
-        positionConfigs[tokenId] |= DIRTY_UPPER_BIT;
-    }
-
-    function setUnsubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal {
-        positionConfigs[tokenId] &= MASK_UPPER_BIT;
-    }
-
-    function hasSubscriber(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId)
-        internal
-        view
-        returns (bool subscribed)
-    {
-        bytes32 _config = positionConfigs[tokenId];
-        assembly ("memory-safe") {
-            subscribed := shr(255, _config)
-        }
-    }
-
     function toId(PositionConfig calldata config) internal pure returns (bytes32 id) {
         // id = keccak256(abi.encodePacked(currency0, currency1, fee, tickSpacing, hooks, tickLower, tickUpper))) >> 1
         assembly ("memory-safe") {

src/libraries/PositionConfigId.sol

@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+pragma solidity ^0.8.24;
+
+/// @notice A configId is set per tokenId
+/// The lower 255 bits are used to store the truncated hash of the corresponding PositionConfig
+/// The upper bit is used to signal if the tokenId has a subscriber
+struct PositionConfigId {
+    bytes32 id;
+}
+
+library PositionConfigIdLibrary {
+    bytes32 constant MASK_UPPER_BIT = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
+    bytes32 constant DIRTY_UPPER_BIT = 0x8000000000000000000000000000000000000000000000000000000000000000;
+
+    /// @notice returns the truncated hash of the PositionConfig for a given tokenId
+    function getConfigId(PositionConfigId storage _configId) internal view returns (bytes32 configId) {
+        configId = _configId.id & MASK_UPPER_BIT;
+    }
+
+    /// @dev We only set the config on mint, guaranteeing that the most significant bit is unset, so we can just assign the entire 32 bytes to the id.
+    function setConfigId(PositionConfigId storage _configId, bytes32 configId) internal {
+        _configId.id = configId;
+    }
+
+    function setSubscribe(PositionConfigId storage configId) internal {
+        configId.id |= DIRTY_UPPER_BIT;
+    }
+
+    function setUnsubscribe(PositionConfigId storage configId) internal {
+        configId.id &= MASK_UPPER_BIT;
+    }
+
+    function hasSubscriber(PositionConfigId storage configId) internal view returns (bool subscribed) {
+        bytes32 _id = configId.id;
+        assembly ("memory-safe") {
+            subscribed := shr(255, _id)
+        }
+    }
+}