Merge branch 'main' into update-native-snaps

snapshots/BaseActionsRouterTest.json

@@ -1,3 +1,3 @@
 {
-  "BaseActionsRouter_mock10commands": "33725"
+  "BaseActionsRouter_mock10commands": "60677"
 }

snapshots/PaymentsTests.json

@@ -1,6 +1,6 @@
 {
-  "Payments_swap_settleFromCaller_takeAllToMsgSender": "104210",
-  "Payments_swap_settleFromCaller_takeAllToSpecifiedAddress": "104961",
-  "Payments_swap_settleWithBalance_takeAllToMsgSender": "95138",
-  "Payments_swap_settleWithBalance_takeAllToSpecifiedAddress": "95052"
+  "Payments_swap_settleFromCaller_takeAllToMsgSender": "129642",
+  "Payments_swap_settleFromCaller_takeAllToSpecifiedAddress": "131705",
+  "Payments_swap_settleWithBalance_takeAllToMsgSender": "123910",
+  "Payments_swap_settleWithBalance_takeAllToSpecifiedAddress": "124052"
 }

snapshots/PosMGasTest.json

@@ -1,41 +1,41 @@
 {
-  "PositionManager_burn_empty": "15061",
-  "PositionManager_burn_empty_native": "15061",
-  "PositionManager_burn_nonEmpty_native_withClose": "51029",
-  "PositionManager_burn_nonEmpty_native_withTakePair": "50330",
-  "PositionManager_burn_nonEmpty_withClose": "47267",
-  "PositionManager_burn_nonEmpty_withTakePair": "46568",
-  "PositionManager_collect_native": "77416",
-  "PositionManager_collect_sameRange": "73654",
-  "PositionManager_collect_withClose": "73654",
-  "PositionManager_collect_withTakePair": "72955",
-  "PositionManager_decreaseLiquidity_native": "44201",
-  "PositionManager_decreaseLiquidity_withClose": "40439",
-  "PositionManager_decreaseLiquidity_withTakePair": "39740",
-  "PositionManager_decrease_burnEmpty": "49571",
-  "PositionManager_decrease_burnEmpty_native": "53333",
-  "PositionManager_decrease_sameRange_allLiquidity": "38726",
-  "PositionManager_decrease_take_take": "40547",
-  "PositionManager_increaseLiquidity_erc20_withClose": "49647",
-  "PositionManager_increaseLiquidity_erc20_withSettlePair": "48903",
-  "PositionManager_increaseLiquidity_native": "47802",
-  "PositionManager_increase_autocompoundExactUnclaimedFees": "62979",
-  "PositionManager_increase_autocompoundExcessFeesCredit": "78466",
-  "PositionManager_increase_autocompound_clearExcess": "72892",
-  "PositionManager_mint_native": "338359",
-  "PositionManager_mint_nativeWithSweep_withClose": "346306",
-  "PositionManager_mint_nativeWithSweep_withSettlePair": "345389",
-  "PositionManager_mint_onSameTickLower": "256851",
-  "PositionManager_mint_onSameTickUpper": "261521",
-  "PositionManager_mint_sameRange": "162190",
-  "PositionManager_mint_settleWithBalance_sweep": "384846",
-  "PositionManager_mint_warmedPool_differentRange": "262082",
-  "PositionManager_mint_withClose": "393304",
-  "PositionManager_mint_withSettlePair": "392474",
-  "PositionManager_multicall_initialize_mint": "426688",
-  "PositionManager_permit": "53780",
-  "PositionManager_permit_secondPosition": "29380",
-  "PositionManager_permit_twice": "7480",
-  "PositionManager_subscribe": "55708",
-  "PositionManager_unsubscribe": "26756"
+  "PositionManager_burn_empty": "50481",
+  "PositionManager_burn_empty_native": "50481",
+  "PositionManager_burn_nonEmpty_native_withClose": "125659",
+  "PositionManager_burn_nonEmpty_native_withTakePair": "125141",
+  "PositionManager_burn_nonEmpty_withClose": "132521",
+  "PositionManager_burn_nonEmpty_withTakePair": "132004",
+  "PositionManager_collect_native": "146388",
+  "PositionManager_collect_sameRange": "154966",
+  "PositionManager_collect_withClose": "154966",
+  "PositionManager_collect_withTakePair": "154331",
+  "PositionManager_decreaseLiquidity_native": "112056",
+  "PositionManager_decreaseLiquidity_withClose": "119847",
+  "PositionManager_decreaseLiquidity_withTakePair": "119212",
+  "PositionManager_decrease_burnEmpty": "135318",
+  "PositionManager_decrease_burnEmpty_native": "128456",
+  "PositionManager_decrease_sameRange_allLiquidity": "132534",
+  "PositionManager_decrease_take_take": "120467",
+  "PositionManager_increaseLiquidity_erc20_withClose": "159127",
+  "PositionManager_increaseLiquidity_erc20_withSettlePair": "158079",
+  "PositionManager_increaseLiquidity_native": "140942",
+  "PositionManager_increase_autocompoundExactUnclaimedFees": "136403",
+  "PositionManager_increase_autocompoundExcessFeesCredit": "177458",
+  "PositionManager_increase_autocompound_clearExcess": "148084",
+  "PositionManager_mint_native": "364815",
+  "PositionManager_mint_nativeWithSweep_withClose": "373334",
+  "PositionManager_mint_nativeWithSweep_withSettlePair": "372569",
+  "PositionManager_mint_onSameTickLower": "317687",
+  "PositionManager_mint_onSameTickUpper": "318357",
+  "PositionManager_mint_sameRange": "243926",
+  "PositionManager_mint_settleWithBalance_sweep": "419134",
+  "PositionManager_mint_warmedPool_differentRange": "323718",
+  "PositionManager_mint_withClose": "420240",
+  "PositionManager_mint_withSettlePair": "419310",
+  "PositionManager_multicall_initialize_mint": "456088",
+  "PositionManager_permit": "79076",
+  "PositionManager_permit_secondPosition": "61976",
+  "PositionManager_permit_twice": "44876",
+  "PositionManager_subscribe": "88168",
+  "PositionManager_unsubscribe": "63080"
 }

snapshots/PositionDescriptorTest.json

@@ -1,3 +1,3 @@
 {
-  "positionDescriptor bytecode size": "31236"
+  "positionDescriptor bytecode size": "31336"
 }

snapshots/QuoterTest.json

@@ -1,15 +1,15 @@
 {
-  "Quoter_exactInputSingle_oneForZero_multiplePositions": "121454",
-  "Quoter_exactInputSingle_zeroForOne_multiplePositions": "126894",
-  "Quoter_exactOutputSingle_oneForZero": "55727",
-  "Quoter_exactOutputSingle_zeroForOne": "60138",
-  "Quoter_quoteExactInput_oneHop_1TickLoaded": "97723",
-  "Quoter_quoteExactInput_oneHop_initializedAfter": "122646",
-  "Quoter_quoteExactInput_oneHop_startingInitialized": "46181",
-  "Quoter_quoteExactInput_twoHops": "177431",
-  "Quoter_quoteExactOutput_oneHop_1TickLoaded": "97014",
-  "Quoter_quoteExactOutput_oneHop_2TicksLoaded": "127151",
-  "Quoter_quoteExactOutput_oneHop_initializedAfter": "97082",
-  "Quoter_quoteExactOutput_oneHop_startingInitialized": "52493",
-  "Quoter_quoteExactOutput_twoHops": "176882"
+  "Quoter_exactInputSingle_oneForZero_multiplePositions": "143930",
+  "Quoter_exactInputSingle_zeroForOne_multiplePositions": "149382",
+  "Quoter_exactOutputSingle_oneForZero": "78203",
+  "Quoter_exactOutputSingle_zeroForOne": "82626",
+  "Quoter_quoteExactInput_oneHop_1TickLoaded": "120491",
+  "Quoter_quoteExactInput_oneHop_initializedAfter": "145414",
+  "Quoter_quoteExactInput_oneHop_startingInitialized": "79437",
+  "Quoter_quoteExactInput_twoHops": "201179",
+  "Quoter_quoteExactOutput_oneHop_1TickLoaded": "119782",
+  "Quoter_quoteExactOutput_oneHop_2TicksLoaded": "149919",
+  "Quoter_quoteExactOutput_oneHop_initializedAfter": "119850",
+  "Quoter_quoteExactOutput_oneHop_startingInitialized": "96549",
+  "Quoter_quoteExactOutput_twoHops": "200630"
 }

snapshots/V4RouterTest.json

@@ -1,26 +1,26 @@
 {
   "V4Router_Bytecode": "7063",
-  "V4Router_ExactIn1Hop_nativeIn": "90557",
-  "V4Router_ExactIn1Hop_nativeOut": "90874",
-  "V4Router_ExactIn1Hop_oneForZero": "99212",
-  "V4Router_ExactIn1Hop_zeroForOne": "104935",
-  "V4Router_ExactIn2Hops": "152841",
-  "V4Router_ExactIn2Hops_nativeIn": "144178",
-  "V4Router_ExactIn3Hops": "200750",
-  "V4Router_ExactIn3Hops_nativeIn": "192087",
-  "V4Router_ExactInputSingle": "104210",
-  "V4Router_ExactInputSingle_nativeIn": "89832",
-  "V4Router_ExactInputSingle_nativeOut": "90129",
-  "V4Router_ExactOut1Hop_nativeIn_sweepETH": "96628",
-  "V4Router_ExactOut1Hop_nativeOut": "91746",
-  "V4Router_ExactOut1Hop_oneForZero": "100084",
-  "V4Router_ExactOut1Hop_zeroForOne": "104029",
-  "V4Router_ExactOut2Hops": "152767",
-  "V4Router_ExactOut2Hops_nativeIn": "149311",
-  "V4Router_ExactOut3Hops": "201536",
-  "V4Router_ExactOut3Hops_nativeIn": "198080",
-  "V4Router_ExactOut3Hops_nativeOut": "193198",
-  "V4Router_ExactOutputSingle": "103301",
-  "V4Router_ExactOutputSingle_nativeIn_sweepETH": "95900",
-  "V4Router_ExactOutputSingle_nativeOut": "91104"
+  "V4Router_ExactIn1Hop_nativeIn": "115753",
+  "V4Router_ExactIn1Hop_nativeOut": "116070",
+  "V4Router_ExactIn1Hop_oneForZero": "124888",
+  "V4Router_ExactIn1Hop_zeroForOne": "130611",
+  "V4Router_ExactIn2Hops": "185452",
+  "V4Router_ExactIn2Hops_nativeIn": "170594",
+  "V4Router_ExactIn3Hops": "240296",
+  "V4Router_ExactIn3Hops_nativeIn": "225438",
+  "V4Router_ExactInputSingle": "129642",
+  "V4Router_ExactInputSingle_nativeIn": "114784",
+  "V4Router_ExactInputSingle_nativeOut": "115069",
+  "V4Router_ExactOut1Hop_nativeIn_sweepETH": "122016",
+  "V4Router_ExactOut1Hop_nativeOut": "117134",
+  "V4Router_ExactOut1Hop_oneForZero": "125952",
+  "V4Router_ExactOut1Hop_zeroForOne": "129897",
+  "V4Router_ExactOut2Hops": "183800",
+  "V4Router_ExactOut2Hops_nativeIn": "175919",
+  "V4Router_ExactOut3Hops": "237734",
+  "V4Router_ExactOut3Hops_nativeIn": "229853",
+  "V4Router_ExactOut3Hops_nativeOut": "217089",
+  "V4Router_ExactOutputSingle": "128925",
+  "V4Router_ExactOutputSingle_nativeIn_sweepETH": "121044",
+  "V4Router_ExactOutputSingle_nativeOut": "116236"
 }

src/PositionManager.sol

@@ -158,6 +158,12 @@ contract PositionManager is
         _;
     }
 
+    /// @notice Enforces that the PoolManager is locked.
+    modifier onlyIfPoolManagerLocked() override {
+        if (poolManager.isUnlocked()) revert PoolManagerMustBeLocked();
+        _;
+    }
+
     function tokenURI(uint256 tokenId) public view override returns (string memory) {
         return IPositionDescriptor(tokenDescriptor).tokenURI(this, tokenId);
     }
@@ -444,7 +450,8 @@ 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 {
+    /// @dev will revert if pool manager is locked
+    function transferFrom(address from, address to, uint256 id) public virtual override onlyIfPoolManagerLocked {
         super.transferFrom(from, to, id);
         if (positionInfo[id].hasSubscriber()) _notifyTransfer(id, from, to);
     }

src/base/Notifier.sol

@@ -29,6 +29,9 @@ abstract contract Notifier is INotifier {
     /// @param tokenId the tokenId of the position
     modifier onlyIfApproved(address caller, uint256 tokenId) virtual;
 
+    /// @notice Enforces that the PoolManager is locked.
+    modifier onlyIfPoolManagerLocked() virtual;
+
     function _setUnsubscribed(uint256 tokenId) internal virtual;
 
     function _setSubscribed(uint256 tokenId) internal virtual;
@@ -37,6 +40,7 @@ abstract contract Notifier is INotifier {
     function subscribe(uint256 tokenId, address newSubscriber, bytes calldata data)
         external
         payable
+        onlyIfPoolManagerLocked
         onlyIfApproved(msg.sender, tokenId)
     {
         ISubscriber _subscriber = subscriber[tokenId];
@@ -56,7 +60,12 @@ abstract contract Notifier is INotifier {
     }
 
     /// @inheritdoc INotifier
-    function unsubscribe(uint256 tokenId) external payable onlyIfApproved(msg.sender, tokenId) {
+    function unsubscribe(uint256 tokenId)
+        external
+        payable
+        onlyIfPoolManagerLocked
+        onlyIfApproved(msg.sender, tokenId)
+    {
         _unsubscribe(tokenId);
     }
 

src/interfaces/INotifier.sol

@@ -36,13 +36,15 @@ interface INotifier {
     /// @param data caller-provided data that's forwarded to the subscriber contract
     /// @dev Calling subscribe when a position is already subscribed will revert
     /// @dev payable so it can be multicalled with NATIVE related actions
+    /// @dev will revert if pool manager is locked
     function subscribe(uint256 tokenId, address newSubscriber, bytes calldata data) external payable;
 
     /// @notice Removes the subscriber from receiving notifications for a respective position
     /// @param tokenId the ERC721 tokenId
     /// @dev Callers must specify a high gas limit (remaining gas should be higher than unsubscriberGasLimit) such that the subscriber can be notified
     /// @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.
+    /// @dev will revert if pool manager is locked
     function unsubscribe(uint256 tokenId) external payable;
 
     /// @notice Returns and determines the maximum allowable gas-used for notifying unsubscribe

src/interfaces/IPositionManager.sol

@@ -14,6 +14,9 @@ interface IPositionManager is INotifier, IImmutableState {
     error NotApproved(address caller);
     /// @notice Thrown when the block.timestamp exceeds the user-provided deadline
     error DeadlinePassed(uint256 deadline);
+    /// @notice Thrown when calling transfer, subscribe, or unsubscribe when the PoolManager is unlocked.
+    /// @dev This is to prevent hooks from being able to trigger notifications at the same time the position is being modified.
+    error PoolManagerMustBeLocked();
 
     /// @notice Unlocks Uniswap v4 PoolManager and batches actions for modifying liquidity
     /// @dev This is the standard entrypoint for the PositionManager

test/mocks/MockReenterHook.sol

@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.20;
+
+import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
+import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
+import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol";
+import {BaseTestHooks} from "@uniswap/v4-core/src/test/BaseTestHooks.sol";
+import {PositionManager} from "../../src/PositionManager.sol";
+
+contract MockReenterHook is BaseTestHooks {
+    PositionManager posm;
+
+    function beforeAddLiquidity(
+        address,
+        PoolKey calldata,
+        IPoolManager.ModifyLiquidityParams calldata,
+        bytes calldata functionSelector
+    ) external override returns (bytes4) {
+        if (functionSelector.length == 0) {
+            return this.beforeAddLiquidity.selector;
+        }
+        (bytes4 selector, address owner, uint256 tokenId) = abi.decode(functionSelector, (bytes4, address, uint256));
+
+        if (selector == posm.transferFrom.selector) {
+            posm.transferFrom(owner, address(this), tokenId);
+        } else if (selector == posm.subscribe.selector) {
+            posm.subscribe(tokenId, address(this), "");
+        } else if (selector == posm.unsubscribe.selector) {
+            posm.unsubscribe(tokenId);
+        }
+        return this.beforeAddLiquidity.selector;
+    }
+
+    function setPosm(PositionManager _posm) external {
+        posm = _posm;
+    }
+}