Skip to content

Commit

Permalink
move sub unsub
Browse files Browse the repository at this point in the history
  • Loading branch information
snreynolds committed Aug 6, 2024
1 parent af688af commit df9c726
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 53 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
340631
340665
Original file line number Diff line number Diff line change
@@ -1 +1 @@
349123
349157
Original file line number Diff line number Diff line change
@@ -1 +1 @@
348425
348459
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickLower.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
318613
318647
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
319255
319289
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
244837
244871
Original file line number Diff line number Diff line change
@@ -1 +1 @@
374655
374689
Original file line number Diff line number Diff line change
@@ -1 +1 @@
324631
324665
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_withClose.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
375931
375965
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_withSettlePair.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
375071
375105
Original file line number Diff line number Diff line change
@@ -1 +1 @@
420405
420439
50 changes: 13 additions & 37 deletions src/PositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ contract PositionManager is
/// @dev The ID of the next token that will be minted. Skips 0
uint256 public nextTokenId = 1;

mapping(uint256 tokenId => bytes32 config) private positionConfigs;
mapping(uint256 tokenId => bytes32 config) private _positionConfigs;

function positionConfigs() internal view override returns (mapping(uint256 tokenId => bytes32 config) storage) {
return _positionConfigs;
}

constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2)
BaseActionsRouter(_poolManager)
Expand All @@ -75,16 +79,16 @@ contract PositionManager is
/// @param tokenId the unique identifier of the ERC721 token
/// @dev either msg.sender or _msgSender() is passed in as the caller
/// _msgSender() should ONLY be used if this is being called from within the unlockCallback
modifier onlyIfApproved(address caller, uint256 tokenId) {
modifier onlyIfApproved(address caller, uint256 tokenId) override {
if (!_isApprovedOrOwner(caller, tokenId)) revert NotApproved(caller);
_;
}

/// @notice Reverts if the hash of the config does not equal the saved hash
/// @param tokenId the unique identifier of the ERC721 token
/// @param config the PositionConfig to check against
modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) {
if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) override {
if (_positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
_;
}

Expand All @@ -107,29 +111,6 @@ contract PositionManager is
_executeActionsWithoutUnlock(actions, params);
}

/// @inheritdoc INotifier
function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber, bytes calldata data)
external
payable
onlyIfApproved(msg.sender, tokenId)
onlyValidConfig(tokenId, config)
{
// call to _subscribe will revert if the user already has a sub
positionConfigs.setSubscribe(tokenId);
_subscribe(tokenId, config, subscriber, data);
}

/// @inheritdoc INotifier
function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data)
external
payable
onlyIfApproved(msg.sender, tokenId)
onlyValidConfig(tokenId, config)
{
positionConfigs.setUnsubscribe(tokenId);
_unsubscribe(tokenId, config, data);
}

function msgSender() public view override returns (address) {
return _getLocker();
}
Expand Down Expand Up @@ -254,7 +235,7 @@ contract PositionManager is
// _beforeModify is not called here because the tokenId is newly minted
BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
liquidityDelta.validateMaxIn(amount0Max, amount1Max);
positionConfigs.setConfigId(tokenId, config);
_positionConfigs.setConfigId(tokenId, config);

emit MintPosition(tokenId, config);
}
Expand All @@ -276,7 +257,7 @@ contract PositionManager is
liquidityDelta.validateMinOut(amount0Min, amount1Min);
}

delete positionConfigs[tokenId];
delete _positionConfigs[tokenId];
// Burn the token.
_burn(tokenId);
}
Expand Down Expand Up @@ -344,7 +325,7 @@ contract PositionManager is
hookData
);

if (positionConfigs.hasSubscriber(uint256(salt))) {
if (_positionConfigs.hasSubscriber(uint256(salt))) {
_notifyModifyLiquidity(uint256(salt), config, liquidityChange);
}
}
Expand All @@ -362,7 +343,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 {
super.transferFrom(from, to, id);
if (positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to);
if (_positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to);
}

/// @inheritdoc IPositionManager
Expand All @@ -378,11 +359,6 @@ contract PositionManager is

/// @inheritdoc IPositionManager
function getPositionConfigId(uint256 tokenId) external view returns (bytes32) {
return positionConfigs.getConfigId(tokenId);
}

/// @inheritdoc INotifier
function hasSubscriber(uint256 tokenId) external view returns (bool) {
return positionConfigs.hasSubscriber(tokenId);
return _positionConfigs.getConfigId(tokenId);
}
}
32 changes: 27 additions & 5 deletions src/base/Notifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.24;

import {ISubscriber} from "../interfaces/ISubscriber.sol";
import {PositionConfig} from "../libraries/PositionConfig.sol";
import {PositionConfig, PositionConfigLibrary} from "../libraries/PositionConfig.sol";
import {BipsLibrary} from "../libraries/BipsLibrary.sol";
import {INotifier} from "../interfaces/INotifier.sol";
import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol";
Expand All @@ -11,6 +11,7 @@ import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol";
abstract contract Notifier is INotifier {
using BipsLibrary for uint256;
using CustomRevert for bytes4;
using PositionConfigLibrary for mapping(uint256 => bytes32);

error AlreadySubscribed(address subscriber);

Expand All @@ -26,9 +27,19 @@ abstract contract Notifier is INotifier {

mapping(uint256 tokenId => ISubscriber subscriber) public subscriber;

function _subscribe(uint256 tokenId, PositionConfig memory config, address newSubscriber, bytes memory data)
internal
modifier onlyIfApproved(address caller, uint256 tokenId) virtual;
modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) virtual;
function positionConfigs() internal view virtual returns (mapping(uint256 tokenId => bytes32 config) storage);

/// @inheritdoc INotifier
function subscribe(uint256 tokenId, PositionConfig calldata config, address newSubscriber, bytes calldata data)
external
payable
onlyIfApproved(msg.sender, tokenId)
onlyValidConfig(tokenId, config)
{
// will revert below if the user already has a subcriber
positionConfigs().setSubscribe(tokenId);
ISubscriber _subscriber = subscriber[tokenId];

if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(address(_subscriber));
Expand All @@ -45,8 +56,14 @@ abstract contract Notifier is INotifier {
emit Subscribed(tokenId, address(newSubscriber));
}

/// @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.
function _unsubscribe(uint256 tokenId, PositionConfig memory config, bytes memory data) internal {
/// @inheritdoc INotifier
function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data)
external
payable
onlyIfApproved(msg.sender, tokenId)
onlyValidConfig(tokenId, config)
{
positionConfigs().setUnsubscribe(tokenId);
ISubscriber _subscriber = subscriber[tokenId];

uint256 subscriberGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS);
Expand Down Expand Up @@ -88,4 +105,9 @@ abstract contract Notifier is INotifier {
success := call(gas(), target, 0, add(encodedCall, 0x20), mload(encodedCall), 0, 0)
}
}

/// @inheritdoc INotifier
function hasSubscriber(uint256 tokenId) external view returns (bool) {
return positionConfigs().hasSubscriber(tokenId);
}
}
1 change: 1 addition & 0 deletions src/interfaces/INotifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface INotifier {
/// @param config the corresponding PositionConfig for the tokenId
/// @param data caller-provided data that's forwarded to the subscriber contract
/// @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.
function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data) external payable;

/// @notice Returns whether a a position should call out to notify a subscribing contract on modification or transfer
Expand Down

0 comments on commit df9c726

Please sign in to comment.