use notifyBurn (#399)

* use notifyBurn

* rename

* update bytecode

.forge-snapshots/PositionManager_burn_empty.snap

@@ -1 +1 @@
-50540
+50845

.forge-snapshots/PositionManager_burn_empty_native.snap

@@ -1 +1 @@
-50540
+50845

.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap

@@ -1 +1 @@
-125788
+126075

.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap

@@ -1 +1 @@
-125220
+125508

.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap

@@ -1 +1 @@
-132648
+132935

.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap

@@ -1 +1 @@
-132080
+132368

.forge-snapshots/PositionManager_collect_native.snap

@@ -1 +1 @@
-146518
+146517

.forge-snapshots/PositionManager_collect_sameRange.snap

@@ -1 +1 @@
-155093
+155092

.forge-snapshots/PositionManager_collect_withClose.snap

@@ -1 +1 @@
-155093
+155092

.forge-snapshots/PositionManager_collect_withTakePair.snap

@@ -1 +1 @@
-154396
+154395

.forge-snapshots/PositionManager_decreaseLiquidity_native.snap

@@ -1 +1 @@
-112160
+112159

.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap

@@ -1 +1 @@
-119974
+119973

.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap

@@ -1 +1 @@
-119277
+119276

.forge-snapshots/PositionManager_decrease_burnEmpty.snap

@@ -1 +1 @@
-135480
+135785

.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap

@@ -1 +1 @@
-128620
+128925

.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap

@@ -1 +1 @@
-132661
+132660

.forge-snapshots/PositionManager_decrease_take_take.snap

@@ -1 +1 @@
-120564
+120563

.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap

@@ -1 +1 @@
-158973
+158972

.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap

@@ -1 +1 @@
-157884
+157883

.forge-snapshots/PositionManager_increaseLiquidity_native.snap

@@ -1 +1 @@
-142237
+142236

.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap

@@ -1 +1 @@
-136418
+136417

.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap

@@ -1 +1 @@
-177562
+177561

.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap

@@ -1 +1 @@
-148338
+148337

.forge-snapshots/PositionManager_mint_native.snap

@@ -1 +1 @@
-366111
+366110

.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap

@@ -1 +1 @@
-374685
+374684

.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap

@@ -1 +1 @@
-373880
+373879

.forge-snapshots/PositionManager_mint_onSameTickLower.snap

@@ -1 +1 @@
-317534
+317533

.forge-snapshots/PositionManager_mint_onSameTickUpper.snap

@@ -1 +1 @@
-318204
+318203

.forge-snapshots/PositionManager_mint_sameRange.snap

@@ -1 +1 @@
-243773
+243772

.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap

@@ -1 +1 @@
-419063
+419062

.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap

@@ -1 +1 @@
-323565
+323564

.forge-snapshots/PositionManager_mint_withClose.snap

@@ -1 +1 @@
-420087
+420086

.forge-snapshots/PositionManager_mint_withSettlePair.snap

@@ -1 +1 @@
-419116
+419115

.forge-snapshots/PositionManager_multicall_initialize_mint.snap

@@ -1 +1 @@
-455917
+455916

.forge-snapshots/PositionManager_unsubscribe.snap

@@ -1 +1 @@
-63036
+63058

src/PositionManager.sol

@@ -411,20 +411,34 @@ contract PositionManager is
 
         uint256 liquidity = uint256(_getLiquidity(tokenId, poolKey, info.tickLower(), info.tickUpper()));
 
+        address owner = ownerOf(tokenId);
+
         // Clear the position info.
         positionInfo[tokenId] = PositionInfoLibrary.EMPTY_POSITION_INFO;
         // Burn the token.
         _burn(tokenId);
 
         // Can only call modify if there is non zero liquidity.
+        BalanceDelta feesAccrued;
+        BalanceDelta liquidityDelta;
         if (liquidity > 0) {
-            (BalanceDelta liquidityDelta, BalanceDelta feesAccrued) =
-                _modifyLiquidity(info, poolKey, -(liquidity.toInt256()), bytes32(tokenId), hookData);
+            // do not use _modifyLiquidity as we do not need to notify on modification for burns.
+            (liquidityDelta, feesAccrued) = poolManager.modifyLiquidity(
+                poolKey,
+                IPoolManager.ModifyLiquidityParams({
+                    tickLower: info.tickLower(),
+                    tickUpper: info.tickUpper(),
+                    liquidityDelta: -(liquidity.toInt256()),
+                    salt: bytes32(tokenId)
+                }),
+                hookData
+            );
             // Slippage checks should be done on the principal liquidityDelta which is the liquidityDelta - feesAccrued
             (liquidityDelta - feesAccrued).validateMinOut(amount0Min, amount1Min);
         }
 
-        if (info.hasSubscriber()) _unsubscribe(tokenId);
+        // deletes then notifies the subscriber
+        if (info.hasSubscriber()) _removeSubscriberAndNotifyBurn(tokenId, owner, info, liquidity, feesAccrued);
     }
 
     function _settlePair(Currency currency0, Currency currency1) internal {
@@ -475,6 +489,7 @@ contract PositionManager is
         if (balance > 0) currency.transfer(to, balance);
     }
 
+    /// @dev if there is a subscriber attached to the position, this function will notify the subscriber
     function _modifyLiquidity(
         PositionInfo info,
         PoolKey memory poolKey,

src/base/Notifier.sol

@@ -88,6 +88,30 @@ abstract contract Notifier is INotifier {
         emit Unsubscription(tokenId, address(_subscriber));
     }
 
+    /// @dev note this function also deletes the subscriber address from the mapping
+    function _removeSubscriberAndNotifyBurn(
+        uint256 tokenId,
+        address owner,
+        PositionInfo info,
+        uint256 liquidity,
+        BalanceDelta feesAccrued
+    ) internal {
+        ISubscriber _subscriber = subscriber[tokenId];
+
+        // remove the subscriber
+        delete subscriber[tokenId];
+
+        bool success = _call(
+            address(_subscriber), abi.encodeCall(ISubscriber.notifyBurn, (tokenId, owner, info, liquidity, feesAccrued))
+        );
+
+        if (!success) {
+            address(_subscriber).bubbleUpAndRevertWith(
+                ISubscriber.notifyBurn.selector, BurnNotificationReverted.selector
+            );
+        }
+    }
+
     function _notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, BalanceDelta feesAccrued) internal {
         ISubscriber _subscriber = subscriber[tokenId];
 

src/interfaces/INotifier.sol

@@ -15,6 +15,8 @@ interface INotifier {
     error SubscriptionReverted(address subscriber, bytes reason);
     /// @notice Wraps the revert message of the subscriber contract on a reverting modify liquidity notification
     error ModifyLiquidityNotificationReverted(address subscriber, bytes reason);
+    /// @notice Wraps the revert message of the subscriber contract on a reverting burn notification
+    error BurnNotificationReverted(address subscriber, bytes reason);
     /// @notice Wraps the revert message of the subscriber contract on a reverting transfer notification
     error TransferNotificationReverted(address subscriber, bytes reason);
     /// @notice Thrown when a tokenId already has a subscriber

src/interfaces/ISubscriber.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.0;
 
 import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
+import {PositionInfo} from "../libraries/PositionInfoLibrary.sol";
 
 /// @notice Interface that a Subscriber contract should implement to receive updates from the v4 position manager
 interface ISubscriber {
@@ -13,6 +14,14 @@ interface ISubscriber {
     /// @dev Because of EIP-150, solidity may only allocate 63/64 of gasleft()
     /// @param tokenId the token ID of the position
     function notifyUnsubscribe(uint256 tokenId) external;
+    /// @notice Called when a position is burned
+    /// @param tokenId the token ID of the position
+    /// @param owner the current owner of the tokenId
+    /// @param info information about the position
+    /// @param liquidity the amount of liquidity decreased in the position, may be 0
+    /// @param feesAccrued the fees accrued by the position if liquidity was decreased
+    function notifyBurn(uint256 tokenId, address owner, PositionInfo info, uint256 liquidity, BalanceDelta feesAccrued)
+        external;
     /// @param tokenId the token ID of the position
     /// @param liquidityChange the change in liquidity on the underlying position
     /// @param feesAccrued the fees to be collected from the position as a result of the modifyLiquidity call

test/mocks/MockBadSubscribers.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.20;
 import {ISubscriber} from "../../src/interfaces/ISubscriber.sol";
 import {PositionManager} from "../../src/PositionManager.sol";
 import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
+import {PositionInfo} from "../../src/libraries/PositionInfoLibrary.sol";
 
 /// @notice A subscriber contract that returns values from the subscriber entrypoints
 contract MockReturnDataSubscriber is ISubscriber {
@@ -47,6 +48,12 @@ contract MockReturnDataSubscriber is ISubscriber {
         notifyModifyLiquidityCount++;
     }
 
+    function notifyBurn(uint256 tokenId, address owner, PositionInfo info, uint256 liquidity, BalanceDelta feesAccrued)
+        external
+    {
+        return;
+    }
+
     function setReturnDataSize(uint256 _value) external {
         memPtr = _value;
     }
@@ -85,6 +92,12 @@ contract MockRevertSubscriber is ISubscriber {
         revert TestRevert("notifyModifyLiquidity");
     }
 
+    function notifyBurn(uint256 tokenId, address owner, PositionInfo info, uint256 liquidity, BalanceDelta feesAccrued)
+        external
+    {
+        return;
+    }
+
     function setRevert(bool _shouldRevert) external {
         shouldRevert = _shouldRevert;
     }

test/mocks/MockSubscriber.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.20;
 import {ISubscriber} from "../../src/interfaces/ISubscriber.sol";
 import {PositionManager} from "../../src/PositionManager.sol";
 import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
+import {PositionInfo} from "../../src/libraries/PositionInfoLibrary.sol";
 
 /// @notice A subscriber contract that ingests updates from the v4 position manager
 contract MockSubscriber is ISubscriber {
@@ -12,6 +13,7 @@ contract MockSubscriber is ISubscriber {
     uint256 public notifySubscribeCount;
     uint256 public notifyUnsubscribeCount;
     uint256 public notifyModifyLiquidityCount;
+    uint256 public notifyBurnCount;
     int256 public liquidityChange;
     BalanceDelta public feesAccrued;
 
@@ -44,4 +46,11 @@ contract MockSubscriber is ISubscriber {
         liquidityChange = _liquidityChange;
         feesAccrued = _feesAccrued;
     }
+
+    function notifyBurn(uint256 tokenId, address owner, PositionInfo info, uint256 liquidity, BalanceDelta feesAccrued)
+        external
+        onlyByPosm
+    {
+        notifyBurnCount++;
+    }
 }

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

@@ -566,8 +566,8 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot {
         lpm.modifyLiquidities(calls, _deadline);
     }
 
-    /// @notice burning a position will automatically notify unsubscribe
-    function test_burn_unsubscribe() public {
+    /// @notice burning a position will automatically notify burn
+    function test_notifyBurn_succeeds() public {
         uint256 tokenId = lpm.nextTokenId();
         mint(config, 100e18, alice, ZERO_BYTES);
 
@@ -583,12 +583,13 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot {
         assertEq(lpm.positionInfo(tokenId).hasSubscriber(), true);
         assertEq(sub.notifyUnsubscribeCount(), 0);
 
-        // burn the position, causing an unsubscribe
+        // burn the position, causing a notifyBurn
         burn(tokenId, config, ZERO_BYTES);
 
         // position is now unsubscribed
         assertEq(lpm.positionInfo(tokenId).hasSubscriber(), false);
-        assertEq(sub.notifyUnsubscribeCount(), 1);
+        assertEq(sub.notifyUnsubscribeCount(), 0);
+        assertEq(sub.notifyBurnCount(), 1);
     }
 
     /// @notice Test that users cannot forcibly avoid unsubscribe logic via gas limits