OZ: posm - restore permissioning on increase (#290)

* restore permissioning on increase

* fix comment

* fix code comments

.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap

@@ -1 +1 @@
-152496
+154942

.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap

@@ -1 +1 @@
-151498
+153944

.forge-snapshots/PositionManager_increaseLiquidity_native.snap

@@ -1 +1 @@
-134296
+136742

.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap

@@ -1 +1 @@
-130395
+132841

.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap

@@ -1 +1 @@
-171311
+173757

.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap

@@ -1 +1 @@
-141267
+143713

src/PositionManager.sol

@@ -215,7 +215,7 @@ contract PositionManager is
         uint128 amount0Max,
         uint128 amount1Max,
         bytes calldata hookData
-    ) internal onlyValidConfig(tokenId, config) {
+    ) internal onlyIfApproved(msgSender(), tokenId) onlyValidConfig(tokenId, config) {
         // Note: The tokenId is used as the salt for this position, so every minted position has unique storage in the pool manager.
         BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
         liquidityDelta.validateMaxInNegative(amount0Max, amount1Max);

test/position-managers/IncreaseLiquidity.t.sol

@@ -294,30 +294,6 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
         assertEq(currency1.balanceOf(alice), balance1BeforeAlice);
     }
 
-    function test_increaseLiquidity_withUnapprovedCaller() public {
-        // Alice provides liquidity
-        // Bob increases Alice's liquidity without being approved
-        uint256 liquidityAlice = 3_000e18;
-
-        // alice provides liquidity
-        vm.startPrank(alice);
-        uint256 tokenIdAlice = lpm.nextTokenId();
-        mint(config, liquidityAlice, alice, ZERO_BYTES);
-        vm.stopPrank();
-
-        uint128 oldLiquidity = lpm.getPositionLiquidity(tokenIdAlice, config);
-
-        // bob can increase liquidity for alice even though he is not the owner / not approved
-        vm.startPrank(bob);
-        increaseLiquidity(tokenIdAlice, config, 100e18, ZERO_BYTES);
-        vm.stopPrank();
-
-        uint128 newLiquidity = lpm.getPositionLiquidity(tokenIdAlice, config);
-
-        // assert liqudity increased by the correct amount
-        assertEq(newLiquidity, oldLiquidity + uint128(100e18));
-    }
-
     function test_increaseLiquidity_sameRange_withExcessFees() public {
         // Alice and Bob provide liquidity on the same range
         // Alice uses half her fees to increase liquidity. The other half are collected to her wallet.

test/position-managers/Permit.t.sol

@@ -72,6 +72,27 @@ contract PermitTest is Test, PosmTestSetup {
         );
     }
 
+    function test_permit_increaseLiquidity() public {
+        uint256 liquidityAlice = 1e18;
+        uint256 tokenIdAlice = lpm.nextTokenId();
+        vm.prank(alice);
+        mint(config, liquidityAlice, alice, ZERO_BYTES);
+
+        // alice gives bob permissions
+        permit(alicePK, tokenIdAlice, bob, 1);
+
+        // bob can increase liquidity on alice's token
+        uint256 liquidityToAdd = 0.4444e18;
+        vm.startPrank(bob);
+        increaseLiquidity(tokenIdAlice, config, liquidityToAdd, ZERO_BYTES);
+        vm.stopPrank();
+
+        // alice's position increased liquidity
+        uint256 liquidity = lpm.getPositionLiquidity(tokenIdAlice, config);
+
+        assertEq(liquidity, liquidityAlice + liquidityToAdd);
+    }
+
     function test_permit_decreaseLiquidity() public {
         uint256 liquidityAlice = 1e18;
         vm.prank(alice);
@@ -143,9 +164,22 @@ contract PermitTest is Test, PosmTestSetup {
         vm.stopPrank();
     }
 
-    // unapproved callers can increase others' positions
-    // see `test_increaseLiquidity_withUnapprovedCaller()`
-    // function test_noPermit_increaseLiquidityRevert() public {}
+    /// @dev unapproved callers CANNOT increase others' positions
+    function test_noPermit_increaseLiquidityRevert() public {
+        // increase fails if the owner did not permit
+        uint256 liquidityAlice = 1e18;
+        vm.prank(alice);
+        mint(config, liquidityAlice, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
+
+        // bob cannot increase liquidity on alice's token
+        uint256 liquidityToAdd = 0.4444e18;
+        bytes memory decrease = getIncreaseEncoded(tokenIdAlice, config, liquidityToAdd, ZERO_BYTES);
+        vm.startPrank(bob);
+        vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(bob)));
+        lpm.modifyLiquidities(decrease, _deadline);
+        vm.stopPrank();
+    }
 
     function test_noPermit_decreaseLiquidityRevert() public {
         // decreaseLiquidity fails if the owner did not permit

test/position-managers/PositionManager.modifyLiquidities.t.sol

@@ -84,12 +84,15 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF
         assertEq(lpm.ownerOf(hookTokenId), address(hookModifyLiquidities)); // hook position owned by hook
     }
 
-    /// @dev increasing liquidity without approval is allowable
+    /// @dev hook must be approved to increase liquidity
     function test_hook_increaseLiquidity() public {
         uint256 initialLiquidity = 100e18;
         uint256 tokenId = lpm.nextTokenId();
         mint(config, initialLiquidity, address(this), ZERO_BYTES);
 
+        // approve the hook for increasing liquidity
+        lpm.approve(address(hookModifyLiquidities), tokenId);
+
         // hook increases liquidity in beforeSwap via hookData
         uint256 newLiquidity = 10e18;
         bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES);
@@ -195,7 +198,28 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF
     }
 
     // --- Revert Scenarios --- //
-    /// @dev Hook does not have approval so decreasingly liquidity should revert
+    /// @dev Hook does not have approval so increasing liquidity should revert
+    function test_hook_increaseLiquidity_revert() public {
+        uint256 initialLiquidity = 100e18;
+        uint256 tokenId = lpm.nextTokenId();
+        mint(config, initialLiquidity, address(this), ZERO_BYTES);
+
+        // hook decreases liquidity in beforeSwap via hookData
+        uint256 liquidityToAdd = 10e18;
+        bytes memory calls = getIncreaseEncoded(tokenId, config, liquidityToAdd, ZERO_BYTES);
+
+        // should revert because hook is not approved
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                Hooks.Wrap__FailedHookCall.selector,
+                address(hookModifyLiquidities),
+                abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities))
+            )
+        );
+        swap(key, true, -1e18, calls);
+    }
+
+    /// @dev Hook does not have approval so decreasing liquidity should revert
     function test_hook_decreaseLiquidity_revert() public {
         uint256 initialLiquidity = 100e18;
         uint256 tokenId = lpm.nextTokenId();