gas optimize unsubscribe check

Uniswap · Sep 4, 2024 · dc3a623 · dc3a623
1 parent f151630
commit dc3a623
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/.forge-snapshots/PositionManager_unsubscribe.snap b/.forge-snapshots/PositionManager_unsubscribe.snap
@@ -1 +1 @@
-62617
+62559
diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol
@@ -64,9 +64,11 @@ abstract contract Notifier is INotifier {
     }
 
     function _unsubscribe(uint256 tokenId, PositionConfig calldata config) internal {
-        _positionConfigs(tokenId).setUnsubscribe();
+        PositionConfigId storage _positionConfig = _positionConfigs(tokenId);
+        if (!_positionConfig.hasSubscriber()) NotSubscribed.selector.revertWith();
+
+        _positionConfig.setUnsubscribe();
         ISubscriber _subscriber = subscriber[tokenId];
-        if (_subscriber == NO_SUBSCRIBER) NotSubscribed.selector.revertWith();
 
         delete subscriber[tokenId];
 

diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol
@@ -441,6 +441,23 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot {
         lpm.unsubscribe(tokenId, config);
     }
 
+    function test_unsubscribe_twice_reverts() public {
+        uint256 tokenId = lpm.nextTokenId();
+        mint(config, 100e18, alice, ZERO_BYTES);
+
+        // approve this contract to operate on alices liq
+        vm.startPrank(alice);
+        lpm.approve(address(this), tokenId);
+        vm.stopPrank();
+
+        lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES);
+
+        lpm.unsubscribe(tokenId, config);
+
+        vm.expectRevert(INotifier.NotSubscribed.selector);
+        lpm.unsubscribe(tokenId, config);
+    }
+
     function test_subscribe_withData() public {
         uint256 tokenId = lpm.nextTokenId();
         mint(config, 100e18, alice, ZERO_BYTES);