From 3daf662c8e3209d9f1f9dc9539f31168b67be702 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Fri, 18 Oct 2024 17:41:15 -0400 Subject: [PATCH] add lock check --- .forge-snapshots/PositionManager_subscribe.snap | 2 +- .forge-snapshots/PositionManager_unsubscribe.snap | 2 +- src/PositionManager.sol | 9 ++++++++- src/base/Notifier.sol | 12 +++++++++++- src/interfaces/IPositionManager.sol | 3 +++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/.forge-snapshots/PositionManager_subscribe.snap b/.forge-snapshots/PositionManager_subscribe.snap index e4eada75..b0cbf20a 100644 --- a/.forge-snapshots/PositionManager_subscribe.snap +++ b/.forge-snapshots/PositionManager_subscribe.snap @@ -1 +1 @@ -84348 \ No newline at end of file +88168 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_unsubscribe.snap b/.forge-snapshots/PositionManager_unsubscribe.snap index 0151c604..ef5e368e 100644 --- a/.forge-snapshots/PositionManager_unsubscribe.snap +++ b/.forge-snapshots/PositionManager_unsubscribe.snap @@ -1 +1 @@ -59238 \ No newline at end of file +63058 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index d5721e34..47ebf5df 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -153,6 +153,13 @@ contract PositionManager is _; } + /// @notice Enforces that the PoolManager is locked. + /// @dev Reverts if the caller tries to transfer, subscribe, or unsubscribe the position while the PoolManager is unlocked. + modifier onlyIfPoolManagerLocked() override { + if (poolManager.isUnlocked()) revert PoolManagerMustBeLocked(); + _; + } + function tokenURI(uint256 tokenId) public view override returns (string memory) { return IPositionDescriptor(tokenDescriptor).tokenURI(this, tokenId); } @@ -431,7 +438,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 { + 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); } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 558e85b7..d847f03e 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -29,6 +29,10 @@ 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. + /// @dev Reverts if the caller tries to transfer, subscribe, or unsubscribe the position while the PoolManager is unlocked. + modifier onlyIfPoolManagerLocked() virtual; + function _setUnsubscribed(uint256 tokenId) internal virtual; function _setSubscribed(uint256 tokenId) internal virtual; @@ -37,6 +41,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 +61,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); } diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index fbdfdd5c..53a2efbe 100644 --- a/src/interfaces/IPositionManager.sol +++ b/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