From db190c8b3b7143a32327f48bcd985aa314a1eec3 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Tue, 30 Jul 2024 15:34:09 -0400 Subject: [PATCH 01/14] compiling --- .../PositionManager_burn_empty.snap | 2 +- .../PositionManager_burn_empty_native.snap | 2 +- .../PositionManager_burn_nonEmpty.snap | 2 +- .../PositionManager_burn_nonEmpty_native.snap | 2 +- .forge-snapshots/PositionManager_collect.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_decreaseLiquidity.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- ...sitionManager_increaseLiquidity_erc20.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- .forge-snapshots/PositionManager_mint.snap | 2 +- .../PositionManager_mint_native.snap | 2 +- .../PositionManager_mint_nativeWithSweep.snap | 2 +- .../PositionManager_mint_onSameTickLower.snap | 2 +- .../PositionManager_mint_onSameTickUpper.snap | 2 +- .../PositionManager_mint_sameRange.snap | 2 +- ...anager_mint_warmedPool_differentRange.snap | 2 +- ...tionManager_multicall_initialize_mint.snap | 2 +- src/PositionManager.sol | 15 +++++- src/base/StakingNotifier.sol | 49 +++++++++++++++++++ src/interfaces/IStakingSubscriber.sol | 12 +++++ test/mocks/StakingSubscriber.sol | 48 ++++++++++++++++++ test/shared/PosmTestSetup.sol | 2 +- 29 files changed, 147 insertions(+), 27 deletions(-) create mode 100644 src/base/StakingNotifier.sol create mode 100644 src/interfaces/IStakingSubscriber.sol create mode 100644 test/mocks/StakingSubscriber.sol diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index 2758ccf3..f3d6d7c0 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -47271 \ No newline at end of file +47307 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap index 462cc207..cbff577e 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -47088 \ No newline at end of file +47124 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty.snap b/.forge-snapshots/PositionManager_burn_nonEmpty.snap index d1f6268f..9bb02f5c 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty.snap @@ -1 +1 @@ -130139 \ No newline at end of file +131968 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap index cd0f3868..a580c09a 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap @@ -1 +1 @@ -123060 \ No newline at end of file +124890 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect.snap b/.forge-snapshots/PositionManager_collect.snap index 0c61cc2c..81f12dc6 100644 --- a/.forge-snapshots/PositionManager_collect.snap +++ b/.forge-snapshots/PositionManager_collect.snap @@ -1 +1 @@ -151223 \ No newline at end of file +153510 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 3767f77c..1e5a5c2f 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -142375 \ No newline at end of file +144662 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index 0c61cc2c..81f12dc6 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -151223 \ No newline at end of file +153510 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity.snap b/.forge-snapshots/PositionManager_decreaseLiquidity.snap index f7274df9..2929882e 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity.snap @@ -1 +1 @@ -116766 \ No newline at end of file +119053 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index 8ef6aa1f..57fcdd61 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -109375 \ No newline at end of file +111204 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 31fc4741..cdc7728f 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -135178 \ No newline at end of file +137008 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index 4be6bbf3..63895f7c 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -127917 \ No newline at end of file +129747 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index 28c9ff85..73bf6d2e 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -129482 \ No newline at end of file +131769 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap index dfa8e719..8ff40a42 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap @@ -1 +1 @@ -152726 \ No newline at end of file +155013 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index c7b0f926..aae3436f 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -134609 \ No newline at end of file +136896 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index 32633ab4..f2aab688 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -135432 \ No newline at end of file +137719 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index c6e7ab8c..4766ba9f 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -171588 \ No newline at end of file +173875 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint.snap b/.forge-snapshots/PositionManager_mint.snap index 43f34e63..519f622c 100644 --- a/.forge-snapshots/PositionManager_mint.snap +++ b/.forge-snapshots/PositionManager_mint.snap @@ -1 +1 @@ -372827 \ No newline at end of file +375114 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 412b757a..4ac64e89 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -337610 \ No newline at end of file +339897 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap index 3ac91811..fb9011ee 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap @@ -1 +1 @@ -344548 \ No newline at end of file +346835 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index 68be3acc..87502097 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -315509 \ No newline at end of file +317796 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index ead48271..6e353286 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -316151 \ No newline at end of file +318438 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index ee368a59..a08cc1c6 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -241733 \ No newline at end of file +244020 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap index 54413555..6ea59de9 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -321527 \ No newline at end of file +323814 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index ebe66360..57ab2638 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -417163 \ No newline at end of file +419520 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index f4cf09b8..9fcc1379 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -22,6 +22,7 @@ import {DeltaResolver} from "./base/DeltaResolver.sol"; import {PositionConfig, PositionConfigLibrary} from "./libraries/PositionConfig.sol"; import {BaseActionsRouter} from "./base/BaseActionsRouter.sol"; import {Actions} from "./libraries/Actions.sol"; +import {StakingNotifier, StakingConfig} from "./base/StakingNotifier.sol"; contract PositionManager is IPositionManager, @@ -30,7 +31,8 @@ contract PositionManager is Multicall, DeltaResolver, ReentrancyLock, - BaseActionsRouter + BaseActionsRouter, + StakingNotifier { using SafeTransferLib for *; using CurrencyLibrary for Currency; @@ -48,8 +50,9 @@ contract PositionManager is IAllowanceTransfer public immutable permit2; - constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2) + constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2, uint256 _stakingGasLimit) BaseActionsRouter(_poolManager) + StakingNotifier(_stakingGasLimit) ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1") { permit2 = _permit2; @@ -189,6 +192,14 @@ contract PositionManager is }), hookData ); + + _notifyModifyLiquidity(uint256(salt), liquidityChange, config); + } + + function stake(uint256 tokenId, StakingConfig memory stakingConfig, PositionConfig memory config) external { + if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender()); + if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); + _notifyStake(tokenId, stakingConfig, config); } function _getPositionLiquidity(PositionConfig memory config, uint256 tokenId) diff --git a/src/base/StakingNotifier.sol b/src/base/StakingNotifier.sol new file mode 100644 index 00000000..c3d8ea0c --- /dev/null +++ b/src/base/StakingNotifier.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.24; + +import {IStakingSubscriber} from "../interfaces/IStakingSubscriber.sol"; +import {PositionConfig} from "../libraries/PositionConfig.sol"; + +struct StakingConfig { + IStakingSubscriber subscriber; +} + +contract StakingNotifier { + error StakingSubscriberCannotBeNotified(); + + event Staked(uint256 tokenId, address subscriber); + + uint256 private immutable stakingGasLimit; + IStakingSubscriber private constant NO_STAKING_SUBSCRIBER = IStakingSubscriber(address(0)); + + mapping(uint256 tokenId => StakingConfig config) public stakingConfig; + + constructor(uint256 _stakingGasLimit) { + stakingGasLimit = _stakingGasLimit; + } + + function _notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) internal { + IStakingSubscriber subscriber = stakingConfig[tokenId].subscriber; + if (subscriber != NO_STAKING_SUBSCRIBER) { + if (gasleft() < stakingGasLimit) revert StakingSubscriberCannotBeNotified(); + subscriber.notifyModifyLiquidity{gas: stakingGasLimit}(tokenId, liquidityChange, config); + } + } + + function _notifyStake(uint256 tokenId, StakingConfig memory _stakingConfig, PositionConfig memory config) + internal + { + stakingConfig[tokenId] = _stakingConfig; + _stakingConfig.subscriber.notifyStake(tokenId, 0, config); + emit Staked(tokenId, address(_stakingConfig.subscriber)); + } + + // TODO + // on transfer, notify subscriber. do we need to reset staking permissions? + // on stake, pass through position currenct liq? + // support unstake functionality + // support hooks auto-enrolling positions in their staking program + // use percentage/fraction for gasLimit + // other gasLimit concerns? + // can we optimize in the no stake case (reduce from 1 SLOAD :/) +} diff --git a/src/interfaces/IStakingSubscriber.sol b/src/interfaces/IStakingSubscriber.sol new file mode 100644 index 00000000..60b9621b --- /dev/null +++ b/src/interfaces/IStakingSubscriber.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {PositionConfig} from "../libraries/PositionConfig.sol"; + +interface IStakingSubscriber { + function notifyStake(uint256 tokenId, uint256 liquidity, PositionConfig memory config) external; + function notifyUnstake(uint256 tokenId, PositionConfig memory config) external; + function notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) external; + function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner, PositionConfig memory config) + external; +} diff --git a/test/mocks/StakingSubscriber.sol b/test/mocks/StakingSubscriber.sol new file mode 100644 index 00000000..0d31b27c --- /dev/null +++ b/test/mocks/StakingSubscriber.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.20; + +import {IStakingSubscriber} from "../../src/interfaces/IStakingSubscriber.sol"; +import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; +import {PositionManager} from "../../src/PositionManager.sol"; + +/// @notice A staking subscriber contract that ingests updates from the v4 position manager +contract StakingSubscriber is IStakingSubscriber { + PositionManager posm; + + error NotAuthorizedNotifer(address sender); + + error NotImplemented(); + + constructor(PositionManager _posm) { + posm = _posm; + } + + modifier onlyByPosm() { + if (msg.sender != address(posm)) revert NotAuthorizedNotifer(msg.sender); + _; + } + + // liquidity param? in case there is already liquidity in a position that is now being staked? + // owner is lookup able + function notifyStake(uint256 tokenId, uint256 liquidity, PositionConfig memory config) external onlyByPosm { + revert NotImplemented(); + } + + function notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) + external + onlyByPosm + { + revert NotImplemented(); + } + + function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner, PositionConfig memory config) + external + onlyByPosm + { + revert NotImplemented(); + } + + function notifyUnstake(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + revert NotImplemented(); + } +} diff --git a/test/shared/PosmTestSetup.sol b/test/shared/PosmTestSetup.sol index 1f945ccb..1b1a3dd1 100644 --- a/test/shared/PosmTestSetup.sol +++ b/test/shared/PosmTestSetup.sol @@ -37,7 +37,7 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { function deployPosm(IPoolManager poolManager) internal { // We use deployPermit2() to prevent having to use via-ir in this repository. permit2 = IAllowanceTransfer(deployPermit2()); - lpm = new PositionManager(poolManager, permit2); + lpm = new PositionManager(poolManager, permit2, 200_000); } function seedBalance(address to) internal { From 147f235af57ed938205ae8f7865e3fc7d2772bd6 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Wed, 31 Jul 2024 18:46:29 -0400 Subject: [PATCH 02/14] compiling, gas snaps added --- .../PositionManager_burn_empty.snap | 2 +- .../PositionManager_burn_empty_native.snap | 2 +- .../PositionManager_burn_nonEmpty.snap | 2 +- .../PositionManager_burn_nonEmpty_native.snap | 2 +- .forge-snapshots/PositionManager_collect.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_decreaseLiquidity.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- ...sitionManager_increaseLiquidity_erc20.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- .forge-snapshots/PositionManager_mint.snap | 2 +- .../PositionManager_mint_native.snap | 2 +- .../PositionManager_mint_nativeWithSweep.snap | 2 +- .../PositionManager_mint_onSameTickLower.snap | 2 +- .../PositionManager_mint_onSameTickUpper.snap | 2 +- .../PositionManager_mint_sameRange.snap | 2 +- ...nManager_mint_settleWithBalance_sweep.snap | 2 +- ...anager_mint_warmedPool_differentRange.snap | 2 +- ...tionManager_multicall_initialize_mint.snap | 2 +- src/PositionManager.sol | 66 +++++++++++++------ src/base/Notifier.sol | 53 +++++++++++++++ src/base/StakingNotifier.sol | 49 -------------- src/interfaces/IPositionManager.sol | 4 +- src/interfaces/IStakingSubscriber.sol | 12 ---- src/interfaces/ISubscriber.sol | 12 ++++ src/libraries/PositionConfig.sol | 50 +++++++++++++- test/libraries/PositionConfig.t.sol | 2 + test/mocks/StakingSubscriber.sol | 22 +++---- 34 files changed, 199 insertions(+), 121 deletions(-) create mode 100644 src/base/Notifier.sol delete mode 100644 src/base/StakingNotifier.sol delete mode 100644 src/interfaces/IStakingSubscriber.sol create mode 100644 src/interfaces/ISubscriber.sol diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index 637bc1f0..06d1fe79 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -46864 \ No newline at end of file +46889 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap index 122463b6..dcd12a0f 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -46681 \ No newline at end of file +46707 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty.snap b/.forge-snapshots/PositionManager_burn_nonEmpty.snap index 78556cf9..9ff7061a 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty.snap @@ -1 +1 @@ -132224 \ No newline at end of file +129907 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap index 6329ffb5..6f0d98f1 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap @@ -1 +1 @@ -125145 \ No newline at end of file +122828 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect.snap b/.forge-snapshots/PositionManager_collect.snap index 0bcdd3dc..e15a91f0 100644 --- a/.forge-snapshots/PositionManager_collect.snap +++ b/.forge-snapshots/PositionManager_collect.snap @@ -1 +1 @@ -152889 \ No newline at end of file +149992 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 3fca37bf..71eaa6f1 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -144041 \ No newline at end of file +141144 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index 0bcdd3dc..e15a91f0 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -152889 \ No newline at end of file +149992 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity.snap b/.forge-snapshots/PositionManager_decreaseLiquidity.snap index 07620f5d..0adcd423 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity.snap @@ -1 +1 @@ -118432 \ No newline at end of file +115535 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index bf0f5aae..ff16dd93 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -110708 \ No newline at end of file +108390 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 7feacdc1..89af5ae9 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -136102 \ No newline at end of file +133798 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index ad7d951c..a82525a3 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -128841 \ No newline at end of file +126537 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index 6772976d..e98923a4 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -131148 \ No newline at end of file +128251 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap index 0647cc74..93c5d91d 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap @@ -1 +1 @@ -154374 \ No newline at end of file +151461 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 229e8148..568794ae 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -136174 \ No newline at end of file +133261 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index d6b384f9..37c1d2e4 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -137095 \ No newline at end of file +134188 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 860f2794..59c2bd77 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -173251 \ No newline at end of file +170344 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint.snap b/.forge-snapshots/PositionManager_mint.snap index 2df70fcd..b7017a38 100644 --- a/.forge-snapshots/PositionManager_mint.snap +++ b/.forge-snapshots/PositionManager_mint.snap @@ -1 +1 @@ -374409 \ No newline at end of file +371380 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 4322f052..8107db81 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -339109 \ No newline at end of file +336080 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap index c70b99e3..141f9455 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap @@ -1 +1 @@ -347739 \ No newline at end of file +344710 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index 839fcaa4..c522c314 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -317091 \ No newline at end of file +314062 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index 2a0691d0..77cebbb9 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -317733 \ No newline at end of file +314704 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index bb99e63f..9dda11bb 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -243315 \ No newline at end of file +240286 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap index d6415c90..ff6488b4 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -372583 \ No newline at end of file +369548 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap index 0893f494..d217575d 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -323109 \ No newline at end of file +320080 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index f82d06fe..6184f1f4 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -418855 \ No newline at end of file +415822 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 3b97a565..01411104 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -24,7 +24,7 @@ import {DeltaResolver} from "./base/DeltaResolver.sol"; import {PositionConfig, PositionConfigLibrary} from "./libraries/PositionConfig.sol"; import {BaseActionsRouter} from "./base/BaseActionsRouter.sol"; import {Actions} from "./libraries/Actions.sol"; -import {StakingNotifier, StakingConfig} from "./base/StakingNotifier.sol"; +import {Notifier} from "./base/Notifier.sol"; import {CalldataDecoder} from "./libraries/CalldataDecoder.sol"; contract PositionManager is @@ -35,12 +35,12 @@ contract PositionManager is DeltaResolver, ReentrancyLock, BaseActionsRouter, - StakingNotifier + Notifier { using SafeTransferLib for *; using CurrencyLibrary for Currency; using PoolIdLibrary for PoolKey; - using PositionConfigLibrary for PositionConfig; + using PositionConfigLibrary for *; using StateLibrary for IPoolManager; using TransientStateLibrary for IPoolManager; using SafeCast for uint256; @@ -50,13 +50,13 @@ contract PositionManager is uint256 public nextTokenId = 1; /// @inheritdoc IPositionManager - mapping(uint256 tokenId => bytes32 configId) public positionConfigs; + mapping(uint256 tokenId => bytes32 config) public positionConfigs; IAllowanceTransfer public immutable permit2; - constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2, uint256 _stakingGasLimit) + constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2, uint256 _subscriberGasLimit) BaseActionsRouter(_poolManager) - StakingNotifier(_stakingGasLimit) + Notifier(_subscriberGasLimit) ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1") { permit2 = _permit2; @@ -67,6 +67,16 @@ contract PositionManager is _; } + modifier onlyIfApproved(address sender, uint256 tokenId) { + if (!_isApprovedOrOwner(sender, tokenId)) revert NotApproved(sender); + _; + } + + modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) { + if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); + _; + } + /// @param unlockData is an encoding of actions, params, and currencies /// @param deadline is the timestamp at which the unlockData will no longer be valid function modifyLiquidities(bytes calldata unlockData, uint256 deadline) @@ -78,6 +88,25 @@ contract PositionManager is _executeActions(unlockData); } + function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) + external + onlyIfApproved(msg.sender, tokenId) + onlyValidConfig(tokenId, config) + { + if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); + _subscribe(tokenId, config, subscriber); + positionConfigs.setSubscribe(tokenId); + } + + function unsubscribe(uint256 tokenId, PositionConfig calldata config) + external + onlyIfApproved(msg.sender, tokenId) + { + if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); + _unsubscribe(tokenId, config); + positionConfigs.setUnsubscribe(tokenId); + } + function _handleAction(uint256 action, bytes calldata params) internal override { if (action == Actions.INCREASE_LIQUIDITY) { (uint256 tokenId, PositionConfig calldata config, uint256 liquidity, bytes calldata hookData) = @@ -116,8 +145,8 @@ contract PositionManager is /// @dev Calling increase with 0 liquidity will credit the caller with any underlying fees of the position function _increase(uint256 tokenId, PositionConfig calldata config, uint256 liquidity, bytes calldata hookData) internal + onlyValidConfig(tokenId, config) { - if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); // 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); } @@ -125,10 +154,9 @@ contract PositionManager is /// @dev Calling decrease with 0 liquidity will credit the caller with any underlying fees of the position function _decrease(uint256 tokenId, PositionConfig calldata config, uint256 liquidity, bytes calldata hookData) internal + onlyIfApproved(_msgSender(), tokenId) + onlyValidConfig(tokenId, config) { - if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender()); - if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); - // Note: the tokenId is used as the salt. BalanceDelta liquidityDelta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData); } @@ -171,9 +199,11 @@ contract PositionManager is } /// @dev this is overloaded with ERC721Permit._burn - function _burn(uint256 tokenId, PositionConfig calldata config, bytes calldata hookData) internal { - if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender()); - if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); + function _burn(uint256 tokenId, PositionConfig calldata config, bytes calldata hookData) + internal + onlyIfApproved(_msgSender(), tokenId) + onlyValidConfig(tokenId, config) + { uint256 liquidity = uint256(_getPositionLiquidity(config, tokenId)); BalanceDelta liquidityDelta; @@ -204,13 +234,9 @@ contract PositionManager is hookData ); - _notifyModifyLiquidity(uint256(salt), liquidityChange, config); - } - - function stake(uint256 tokenId, StakingConfig calldata stakingConfig, PositionConfig calldata config) external { - if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender()); - if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); - _notifyStake(tokenId, stakingConfig, config); + if (positionConfigs.getSubscribed(uint256(salt))) { + _notifyModifyLiquidity(uint256(salt), config, liquidityChange); + } } function _getPositionLiquidity(PositionConfig calldata config, uint256 tokenId) diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol new file mode 100644 index 00000000..6ef0c31c --- /dev/null +++ b/src/base/Notifier.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.24; + +import {ISubscriber} from "../interfaces/ISubscriber.sol"; +import {PositionConfig} from "../libraries/PositionConfig.sol"; + +contract Notifier { + error SubscriberCannotBeNotified(); + error AlreadySubscribed(address subscriber); + + event Subscribed(uint256 tokenId, address subscriber); + event Unsubscribed(uint256 tokenId, address subscriber); + + uint256 private immutable subscriberGasLimit; + ISubscriber private constant NO_SUBSCRIBER = ISubscriber(address(0)); + + mapping(uint256 tokenId => ISubscriber subscriber) public subscriber; + + constructor(uint256 _subscriberGasLimit) { + subscriberGasLimit = _subscriberGasLimit; + } + + function _subscribe(uint256 tokenId, PositionConfig memory config, address newSubscriber) internal { + ISubscriber _subscriber = subscriber[tokenId]; + + if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(address(_subscriber)); + subscriber[tokenId] = ISubscriber(newSubscriber); + + ISubscriber(newSubscriber).notifySubscribe(tokenId, config); + 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) internal { + ISubscriber _subscriber = subscriber[tokenId]; + + if (gasleft() < subscriberGasLimit) revert SubscriberCannotBeNotified(); + try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} + + delete subscriber[tokenId]; + emit Unsubscribed(tokenId, address(_subscriber)); + } + + function _notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) internal { + subscriber[tokenId].notifyModifyLiquidity(tokenId, config, liquidityChange); + } + + function _notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) + internal + { + subscriber[tokenId].notifyTransfer(tokenId, config, previousOwner, newOwner); + } +} diff --git a/src/base/StakingNotifier.sol b/src/base/StakingNotifier.sol deleted file mode 100644 index c3d8ea0c..00000000 --- a/src/base/StakingNotifier.sol +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; - -import {IStakingSubscriber} from "../interfaces/IStakingSubscriber.sol"; -import {PositionConfig} from "../libraries/PositionConfig.sol"; - -struct StakingConfig { - IStakingSubscriber subscriber; -} - -contract StakingNotifier { - error StakingSubscriberCannotBeNotified(); - - event Staked(uint256 tokenId, address subscriber); - - uint256 private immutable stakingGasLimit; - IStakingSubscriber private constant NO_STAKING_SUBSCRIBER = IStakingSubscriber(address(0)); - - mapping(uint256 tokenId => StakingConfig config) public stakingConfig; - - constructor(uint256 _stakingGasLimit) { - stakingGasLimit = _stakingGasLimit; - } - - function _notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) internal { - IStakingSubscriber subscriber = stakingConfig[tokenId].subscriber; - if (subscriber != NO_STAKING_SUBSCRIBER) { - if (gasleft() < stakingGasLimit) revert StakingSubscriberCannotBeNotified(); - subscriber.notifyModifyLiquidity{gas: stakingGasLimit}(tokenId, liquidityChange, config); - } - } - - function _notifyStake(uint256 tokenId, StakingConfig memory _stakingConfig, PositionConfig memory config) - internal - { - stakingConfig[tokenId] = _stakingConfig; - _stakingConfig.subscriber.notifyStake(tokenId, 0, config); - emit Staked(tokenId, address(_stakingConfig.subscriber)); - } - - // TODO - // on transfer, notify subscriber. do we need to reset staking permissions? - // on stake, pass through position currenct liq? - // support unstake functionality - // support hooks auto-enrolling positions in their staking program - // use percentage/fraction for gasLimit - // other gasLimit concerns? - // can we optimize in the no stake case (reduce from 1 SLOAD :/) -} diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index ebf0b482..7f6f20c0 100644 --- a/src/interfaces/IPositionManager.sol +++ b/src/interfaces/IPositionManager.sol @@ -11,7 +11,9 @@ interface IPositionManager { /// @notice Maps the ERC721 tokenId to a configId, which is a keccak256 hash of the position's pool key, and range (tickLower, tickUpper) /// Enforces that a minted ERC721 token is tied to one range on one pool. /// @param tokenId the ERC721 tokenId, assigned at mint - /// @return configId the hash of the position's poolkey, tickLower, and tickUpper + /// @return configId a truncated hash of the position's poolkey, tickLower, and tickUpper and a reserved upper bit for the isSubscribed flag + /// @dev the highest bit of the configId is used to signal if the position is subscribed + /// and the lower bits contain the truncated hash of the PositionConfig function positionConfigs(uint256 tokenId) external view returns (bytes32 configId); /// @notice Batches many liquidity modification calls to pool manager diff --git a/src/interfaces/IStakingSubscriber.sol b/src/interfaces/IStakingSubscriber.sol deleted file mode 100644 index 60b9621b..00000000 --- a/src/interfaces/IStakingSubscriber.sol +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; - -import {PositionConfig} from "../libraries/PositionConfig.sol"; - -interface IStakingSubscriber { - function notifyStake(uint256 tokenId, uint256 liquidity, PositionConfig memory config) external; - function notifyUnstake(uint256 tokenId, PositionConfig memory config) external; - function notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) external; - function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner, PositionConfig memory config) - external; -} diff --git a/src/interfaces/ISubscriber.sol b/src/interfaces/ISubscriber.sol new file mode 100644 index 00000000..f68a3e97 --- /dev/null +++ b/src/interfaces/ISubscriber.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {PositionConfig} from "../libraries/PositionConfig.sol"; + +interface ISubscriber { + function notifySubscribe(uint256 tokenId, PositionConfig memory config) external; + function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external; + function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) external; + function notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) + external; +} diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index d3bd8e24..93fbca7f 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -3,15 +3,59 @@ pragma solidity ^0.8.24; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; -// A PositionConfig is the input for creating and modifying a Position in core, set per tokenId +// A PositionConfig is the input for creating and modifying a Position in core, whos truncated hash is set per tokenId struct PositionConfig { PoolKey poolKey; int24 tickLower; int24 tickUpper; } -/// @notice Library for computing the configId given a PositionConfig +/// @notice Library to get and set the PositionConfigId and subscriber status for a given tokenId library PositionConfigLibrary { + using PositionConfigLibrary for PositionConfig; + + bytes32 constant MASK_UPPER_BIT = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF; + bytes32 constant DIRTY_UPPER_BIT = 0x8000000000000000000000000000000000000000000000000000000000000000; + + /// @notice returns the truncated hash of the PositionConfig for a given tokenId + function getConfigId(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) + internal + view + returns (bytes32 configId) + { + bytes32 _config = positionConfigs[tokenId]; + configId = _config & MASK_UPPER_BIT; + } + + function setConfigId( + mapping(uint256 => bytes32) storage positionConfigs, + uint256 tokenId, + PositionConfig calldata config + ) internal { + positionConfigs[tokenId] = config.toId(); + } + + function setSubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal { + bytes32 _config = positionConfigs[tokenId]; + positionConfigs[tokenId] = _config | DIRTY_UPPER_BIT; + } + + function setUnsubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal { + bytes32 _config = positionConfigs[tokenId]; + positionConfigs[tokenId] = _config & MASK_UPPER_BIT; + } + + function getSubscribed(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) + internal + view + returns (bool subscribed) + { + bytes32 _config = positionConfigs[tokenId]; + assembly ("memory-safe") { + subscribed := shr(255, _config) + } + } + function toId(PositionConfig calldata config) internal pure returns (bytes32 id) { // id = keccak256(abi.encodePacked(currency0, currency1, fee, tickSpacing, hooks, tickLower, tickUpper))) assembly ("memory-safe") { @@ -24,7 +68,7 @@ library PositionConfigLibrary { mstore(add(fmp, 0x14), calldataload(add(config, 0x20))) // currency1: [0x20, 0x34) mstore(fmp, calldataload(config)) // currency0: [0x0c, 0x20) - id := keccak256(add(fmp, 0x0c), 0x48) // len is 72 bytes + id := shr(1, keccak256(add(fmp, 0x0c), 0x48)) // len is 72 bytes, truncate upper bit of the hash // now clean the memory we used mstore(add(fmp, 0x40), 0) // fmp+0x40 held hooks (14 bytes), tickLower, tickUpper diff --git a/test/libraries/PositionConfig.t.sol b/test/libraries/PositionConfig.t.sol index 1eeedc18..4fbfb684 100644 --- a/test/libraries/PositionConfig.t.sol +++ b/test/libraries/PositionConfig.t.sol @@ -19,6 +19,8 @@ contract PositionConfigTest is Test { config.tickUpper ) ); + // the id is shifted over 1 + expectedId = expectedId >> 1; assertEq(expectedId, config.toId()); } } diff --git a/test/mocks/StakingSubscriber.sol b/test/mocks/StakingSubscriber.sol index 0d31b27c..c57ab17b 100644 --- a/test/mocks/StakingSubscriber.sol +++ b/test/mocks/StakingSubscriber.sol @@ -1,12 +1,12 @@ // SPDX-License-Identifier: GPL-2.0-or-later pragma solidity ^0.8.20; -import {IStakingSubscriber} from "../../src/interfaces/IStakingSubscriber.sol"; +import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; import {PositionManager} from "../../src/PositionManager.sol"; /// @notice A staking subscriber contract that ingests updates from the v4 position manager -contract StakingSubscriber is IStakingSubscriber { +contract StakingSubscriber is ISubscriber { PositionManager posm; error NotAuthorizedNotifer(address sender); @@ -22,27 +22,27 @@ contract StakingSubscriber is IStakingSubscriber { _; } - // liquidity param? in case there is already liquidity in a position that is now being staked? - // owner is lookup able - function notifyStake(uint256 tokenId, uint256 liquidity, PositionConfig memory config) external onlyByPosm { + function notifySubscribe(uint256 tokenId, PositionConfig memory config) external view onlyByPosm { revert NotImplemented(); } - function notifyModifyLiquidity(uint256 tokenId, int256 liquidityChange, PositionConfig memory config) - external - onlyByPosm - { + function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external view onlyByPosm { revert NotImplemented(); } - function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner, PositionConfig memory config) + function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) external + view onlyByPosm { revert NotImplemented(); } - function notifyUnstake(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + function notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) + external + view + onlyByPosm + { revert NotImplemented(); } } From 7848510f67b9cedc1651471b3310bd3a11e67849 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Wed, 31 Jul 2024 19:31:04 -0400 Subject: [PATCH 03/14] add unit tests for PositionConfigLibrary --- src/libraries/PositionConfig.sol | 2 +- test/libraries/PositionConfig.t.sol | 102 ++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index 93fbca7f..a1cb834f 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -57,7 +57,7 @@ library PositionConfigLibrary { } function toId(PositionConfig calldata config) internal pure returns (bytes32 id) { - // id = keccak256(abi.encodePacked(currency0, currency1, fee, tickSpacing, hooks, tickLower, tickUpper))) + // id = keccak256(abi.encodePacked(currency0, currency1, fee, tickSpacing, hooks, tickLower, tickUpper))) >> 1 assembly ("memory-safe") { let fmp := mload(0x40) mstore(add(fmp, 0x34), calldataload(add(config, 0xc0))) // tickUpper: [0x51, 0x54) diff --git a/test/libraries/PositionConfig.t.sol b/test/libraries/PositionConfig.t.sol index 4fbfb684..04ae4a0b 100644 --- a/test/libraries/PositionConfig.t.sol +++ b/test/libraries/PositionConfig.t.sol @@ -2,13 +2,108 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; + import {PositionConfig, PositionConfigLibrary} from "../../src/libraries/PositionConfig.sol"; contract PositionConfigTest is Test { - using PositionConfigLibrary for PositionConfig; + using PositionConfigLibrary for *; + + mapping(uint256 => bytes32) internal testConfigs; + + bytes32 public constant UPPER_BIT_SET = 0x8000000000000000000000000000000000000000000000000000000000000000; function test_fuzz_toId(PositionConfig calldata config) public pure { - bytes32 expectedId = keccak256( + bytes32 expectedId = _calculateExpectedId(config); + assertEq(expectedId, config.toId()); + } + + function test_fuzz_setConfigId(uint256 tokenId, PositionConfig calldata config) public { + testConfigs.setConfigId(tokenId, config); + + bytes32 expectedConfigId = _calculateExpectedId(config); + + bytes32 actualConfigId = testConfigs[tokenId]; + assertEq(expectedConfigId, actualConfigId); + } + + function test_fuzz_getConfigId(uint256 tokenId, PositionConfig calldata config) public { + bytes32 expectedId = _calculateExpectedId(config); + // set + testConfigs[tokenId] = expectedId; + + assertEq(expectedId, testConfigs.getConfigId(tokenId)); + } + + function test_fuzz_setConfigId_getConfigId(uint256 tokenId, PositionConfig calldata config) public { + testConfigs.setConfigId(tokenId, config); + + bytes32 expectedId = _calculateExpectedId(config); + + assertEq(testConfigs.getConfigId(tokenId), testConfigs[tokenId]); + assertEq(testConfigs.getConfigId(tokenId), expectedId); + } + + function test_fuzz_getConfigId_equal_afterSubscribe(uint256 tokenId, PositionConfig calldata config) public { + testConfigs.setConfigId(tokenId, config); + testConfigs.setSubscribe(tokenId); + + assertEq(testConfigs.getConfigId(tokenId), config.toId()); + } + + function test_fuzz_setSubscribe(uint256 tokenId) public { + testConfigs.setSubscribe(tokenId); + bytes32 upperBitSet = testConfigs[tokenId]; + + assertEq(upperBitSet, UPPER_BIT_SET); + } + + function test_fuzz_setConfigId_setSubscribe(uint256 tokenId, PositionConfig calldata config) public { + testConfigs.setConfigId(tokenId, config); + testConfigs.setSubscribe(tokenId); + + bytes32 expectedConfig = _calculateExpectedId(config) | UPPER_BIT_SET; + + bytes32 _config = testConfigs[tokenId]; + + assertEq(_config, expectedConfig); + } + + function test_fuzz_setUnsubscribe(uint256 tokenId) public { + testConfigs.setSubscribe(tokenId); + bytes32 _config = testConfigs[tokenId]; + assertEq(_config, UPPER_BIT_SET); + testConfigs.setUnsubscribe(tokenId); + _config = testConfigs[tokenId]; + assertEq(_config, 0); + } + + function test_getSubscribed(uint256 tokenId) public { + testConfigs.setSubscribe(tokenId); + assert(testConfigs.getSubscribed(tokenId)); + testConfigs.setUnsubscribe(tokenId); + assert(!testConfigs.getSubscribed(tokenId)); + } + + function test_fuzz_setConfigId_setSubscribe_setUnsubscribe_getConfigId( + uint256 tokenId, + PositionConfig calldata config + ) public { + assertEq(testConfigs.getConfigId(tokenId), 0); + + testConfigs.setConfigId(tokenId, config); + assertEq(testConfigs.getConfigId(tokenId), config.toId()); + + testConfigs.setSubscribe(tokenId); + assertEq(testConfigs.getConfigId(tokenId), config.toId()); + assertEq(testConfigs.getSubscribed(tokenId), true); + + testConfigs.setUnsubscribe(tokenId); + assertEq(testConfigs.getConfigId(tokenId), config.toId()); + assertEq(testConfigs.getSubscribed(tokenId), false); + } + + function _calculateExpectedId(PositionConfig calldata config) internal pure returns (bytes32 expectedId) { + expectedId = keccak256( abi.encodePacked( config.poolKey.currency0, config.poolKey.currency1, @@ -19,8 +114,7 @@ contract PositionConfigTest is Test { config.tickUpper ) ); - // the id is shifted over 1 + // truncate the upper bit expectedId = expectedId >> 1; - assertEq(expectedId, config.toId()); } } From 159b1ca91d9356717b47269a14f37b65b60dfe88 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Wed, 31 Jul 2024 21:10:27 -0400 Subject: [PATCH 04/14] subscriber tests --- .../PositionManager_burn_nonEmpty.snap | 2 +- .../PositionManager_burn_nonEmpty_native.snap | 2 +- .forge-snapshots/PositionManager_collect.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_decreaseLiquidity.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- src/PositionManager.sol | 6 + src/base/Notifier.sol | 6 +- src/interfaces/ISubscriber.sol | 3 +- ...akingSubscriber.sol => MockSubscriber.sol} | 28 ++-- .../PositionManager.notifier.t.sol | 129 ++++++++++++++++++ 16 files changed, 163 insertions(+), 31 deletions(-) rename test/mocks/{StakingSubscriber.sol => MockSubscriber.sol} (59%) create mode 100644 test/position-managers/PositionManager.notifier.t.sol diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty.snap b/.forge-snapshots/PositionManager_burn_nonEmpty.snap index 9ff7061a..dd591329 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty.snap @@ -1 +1 @@ -129907 \ No newline at end of file +129924 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap index 6f0d98f1..400c003a 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap @@ -1 +1 @@ -122828 \ No newline at end of file +122846 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect.snap b/.forge-snapshots/PositionManager_collect.snap index e15a91f0..d3d1eb65 100644 --- a/.forge-snapshots/PositionManager_collect.snap +++ b/.forge-snapshots/PositionManager_collect.snap @@ -1 +1 @@ -149992 \ No newline at end of file +150014 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 71eaa6f1..fd547f65 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -141144 \ No newline at end of file +141166 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index e15a91f0..d3d1eb65 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -149992 \ No newline at end of file +150014 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity.snap b/.forge-snapshots/PositionManager_decreaseLiquidity.snap index 0adcd423..2a0c4e9a 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity.snap @@ -1 +1 @@ -115535 \ No newline at end of file +115557 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index ff16dd93..f17d9c56 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -108390 \ No newline at end of file +108408 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 89af5ae9..5b746a43 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -133798 \ No newline at end of file +133816 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index a82525a3..ba291b49 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -126537 \ No newline at end of file +126555 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index e98923a4..5e10fac8 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -128251 \ No newline at end of file +128273 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 59c2bd77..ca605b2d 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -170344 \ No newline at end of file +170366 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 01411104..bec6af76 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -264,4 +264,10 @@ contract PositionManager is permit2.transferFrom(payer, address(poolManager), uint160(amount), Currency.unwrap(currency)); } } + + /// @dev overrides solmate transferFrom in case a notification to subscribers is needed + function transferFrom(address from, address to, uint256 id) public override { + super.transferFrom(from, to, id); + if (positionConfigs.getSubscribed(id)) _notifyTransfer(id, from, to); + } } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 6ef0c31c..2ec83489 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -45,9 +45,7 @@ contract Notifier { subscriber[tokenId].notifyModifyLiquidity(tokenId, config, liquidityChange); } - function _notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) - internal - { - subscriber[tokenId].notifyTransfer(tokenId, config, previousOwner, newOwner); + function _notifyTransfer(uint256 tokenId, address previousOwner, address newOwner) internal { + subscriber[tokenId].notifyTransfer(tokenId, previousOwner, newOwner); } } diff --git a/src/interfaces/ISubscriber.sol b/src/interfaces/ISubscriber.sol index f68a3e97..a74efeeb 100644 --- a/src/interfaces/ISubscriber.sol +++ b/src/interfaces/ISubscriber.sol @@ -7,6 +7,5 @@ interface ISubscriber { function notifySubscribe(uint256 tokenId, PositionConfig memory config) external; function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external; function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) external; - function notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) - external; + function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner) external; } diff --git a/test/mocks/StakingSubscriber.sol b/test/mocks/MockSubscriber.sol similarity index 59% rename from test/mocks/StakingSubscriber.sol rename to test/mocks/MockSubscriber.sol index c57ab17b..b6c92cad 100644 --- a/test/mocks/StakingSubscriber.sol +++ b/test/mocks/MockSubscriber.sol @@ -5,10 +5,15 @@ import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; import {PositionManager} from "../../src/PositionManager.sol"; -/// @notice A staking subscriber contract that ingests updates from the v4 position manager -contract StakingSubscriber is ISubscriber { +/// @notice A subscriber contract that ingests updates from the v4 position manager +contract MockSubscriber is ISubscriber { PositionManager posm; + uint256 public notifySubscribeCount; + uint256 public notifyUnsubscribeCount; + uint256 public notifyModifyLiquidityCount; + uint256 public notifyTransferCount; + error NotAuthorizedNotifer(address sender); error NotImplemented(); @@ -22,27 +27,22 @@ contract StakingSubscriber is ISubscriber { _; } - function notifySubscribe(uint256 tokenId, PositionConfig memory config) external view onlyByPosm { - revert NotImplemented(); + function notifySubscribe(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + notifySubscribeCount++; } - function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external view onlyByPosm { - revert NotImplemented(); + function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + notifyUnsubscribeCount++; } function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) external - view onlyByPosm { - revert NotImplemented(); + notifyModifyLiquidityCount++; } - function notifyTransfer(uint256 tokenId, PositionConfig memory config, address previousOwner, address newOwner) - external - view - onlyByPosm - { - revert NotImplemented(); + function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner) external onlyByPosm { + notifyTransferCount++; } } diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol new file mode 100644 index 00000000..030ea00f --- /dev/null +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; +import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; +import {MockSubscriber} from "../mocks/MockSubscriber.sol"; +import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; +import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; +import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; +import {Plan, Planner} from "../shared/Planner.sol"; +import {Actions} from "../../src/libraries/Actions.sol"; + +contract PositionManagerNotifierTest is Test, PosmTestSetup { + using Planner for Plan; + + MockSubscriber sub; + PositionConfig config; + + address alice = makeAddr("ALICE"); + address bob = makeAddr("BOB"); + + function setUp() public { + deployFreshManagerAndRouters(); + deployMintAndApprove2Currencies(); + + (key,) = initPool(currency0, currency1, IHooks(hook), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + + // Requires currency0 and currency1 to be set in base Deployers contract. + deployAndApprovePosm(manager); + + sub = new MockSubscriber(lpm); + config = PositionConfig({poolKey: key, tickLower: -300, tickUpper: 300}); + + // TODO: Test NATIVE poolKey + } + + function test_subscribe_revertsWithEmptyPositionConfig() public { + uint256 tokenId = lpm.nextTokenId(); + vm.expectRevert("NOT_MINTED"); + lpm.subscribe(tokenId, config, address(sub)); + } + + function test_subscribe_revertsWhenNotApproved() public { + uint256 tokenId = lpm.nextTokenId(); + mint(config, 100e18, alice, ZERO_BYTES); + + // this contract is not approved to operate on alice's liq + + vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(this))); + lpm.subscribe(tokenId, config, address(sub)); + } + + function test_subscribe_reverts_withIncorrectConfig() 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(); + + PositionConfig memory incorrectConfig = PositionConfig({poolKey: key, tickLower: -300, tickUpper: 301}); + + vm.expectRevert(abi.encodeWithSelector(IPositionManager.IncorrectPositionConfigForTokenId.selector, tokenId)); + lpm.subscribe(tokenId, incorrectConfig, address(sub)); + } + + function test_subscribe_succeeds() 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)); + + assertEq(_getSubscribed(lpm.positionConfigs(tokenId)), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + assertEq(sub.notifySubscribeCount(), 1); + } + + function test_notifyModifyLiquidity_succeeds() 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)); + + Plan memory plan = Planner.init(); + for (uint256 i = 0; i < 10; i++) { + plan.add(Actions.INCREASE_LIQUIDITY, abi.encode(tokenId, config, 10e18, ZERO_BYTES)); + } + + bytes memory calls = plan.finalizeModifyLiquidity(config.poolKey); + lpm.modifyLiquidities(calls, _deadline); + + assertEq(sub.notifySubscribeCount(), 1); + assertEq(sub.notifyModifyLiquidityCount(), 10); + } + + function test_notifyTransfer_withTransferFrom_succeeds() 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)); + + lpm.transferFrom(alice, bob, tokenId); + + assertEq(sub.notifyTransferCount(), 1); + } + + function _getSubscribed(bytes32 config) internal pure returns (bool subscribed) { + assembly { + subscribed := shr(255, config) + } + } +} From 0f5943319eebfc4cab220bb74fe3eb1f94257cae Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 13:02:42 -0400 Subject: [PATCH 05/14] pr comments --- src/libraries/PositionConfig.sol | 9 +++------ test/position-managers/PositionManager.notifier.t.sol | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index a1cb834f..582941c9 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -23,8 +23,7 @@ library PositionConfigLibrary { view returns (bytes32 configId) { - bytes32 _config = positionConfigs[tokenId]; - configId = _config & MASK_UPPER_BIT; + configId = positionConfigs[tokenId] & MASK_UPPER_BIT; } function setConfigId( @@ -36,13 +35,11 @@ library PositionConfigLibrary { } function setSubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal { - bytes32 _config = positionConfigs[tokenId]; - positionConfigs[tokenId] = _config | DIRTY_UPPER_BIT; + positionConfigs[tokenId] |= DIRTY_UPPER_BIT; } function setUnsubscribe(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal { - bytes32 _config = positionConfigs[tokenId]; - positionConfigs[tokenId] = _config & MASK_UPPER_BIT; + positionConfigs[tokenId] &= MASK_UPPER_BIT; } function getSubscribed(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index 030ea00f..dcc227cd 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -121,9 +121,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup { assertEq(sub.notifyTransferCount(), 1); } - function _getSubscribed(bytes32 config) internal pure returns (bool subscribed) { + function _getSubscribed(bytes32 _config) internal pure returns (bool subscribed) { assembly { - subscribed := shr(255, config) + subscribed := shr(255, _config) } } } From 510dff1e3f57e015ae0a4c1a5d55d13791221f80 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 13:36:04 -0400 Subject: [PATCH 06/14] use gas limit calcualtor --- src/PositionManager.sol | 3 +-- src/base/Notifier.sol | 14 +++++++++----- src/libraries/GasLimitCalculator.sol | 12 ++++++++++++ test/libraries/GasLimitCalculator.t.sol | 22 ++++++++++++++++++++++ test/shared/PosmTestSetup.sol | 2 +- 5 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 src/libraries/GasLimitCalculator.sol create mode 100644 test/libraries/GasLimitCalculator.t.sol diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 55043445..3667ae23 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -54,9 +54,8 @@ contract PositionManager is IAllowanceTransfer public immutable permit2; - constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2, uint256 _subscriberGasLimit) + constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2) BaseActionsRouter(_poolManager) - Notifier(_subscriberGasLimit) ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1") { permit2 = _permit2; diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 2ec83489..c3d6460f 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -3,22 +3,26 @@ pragma solidity ^0.8.24; import {ISubscriber} from "../interfaces/ISubscriber.sol"; import {PositionConfig} from "../libraries/PositionConfig.sol"; +import {GasLimitCalculator} from "../libraries/GasLimitCalculator.sol"; contract Notifier { + using GasLimitCalculator for uint256; + error SubscriberCannotBeNotified(); error AlreadySubscribed(address subscriber); event Subscribed(uint256 tokenId, address subscriber); event Unsubscribed(uint256 tokenId, address subscriber); - uint256 private immutable subscriberGasLimit; ISubscriber private constant NO_SUBSCRIBER = ISubscriber(address(0)); + // a percentage of the block.gaslimit denoted in BPS, used as the gas limit for subscriber calls + // 1 BP is 0.01% + uint256 private constant BLOCK_LIMIT_BPS = 1; + mapping(uint256 tokenId => ISubscriber subscriber) public subscriber; - constructor(uint256 _subscriberGasLimit) { - subscriberGasLimit = _subscriberGasLimit; - } + constructor() {} function _subscribe(uint256 tokenId, PositionConfig memory config, address newSubscriber) internal { ISubscriber _subscriber = subscriber[tokenId]; @@ -34,7 +38,7 @@ contract Notifier { function _unsubscribe(uint256 tokenId, PositionConfig memory config) internal { ISubscriber _subscriber = subscriber[tokenId]; - if (gasleft() < subscriberGasLimit) revert SubscriberCannotBeNotified(); + uint256 subscriberGasLimit = BLOCK_LIMIT_BPS.toGasLimit(); try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} delete subscriber[tokenId]; diff --git a/src/libraries/GasLimitCalculator.sol b/src/libraries/GasLimitCalculator.sol new file mode 100644 index 00000000..9da976bb --- /dev/null +++ b/src/libraries/GasLimitCalculator.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.24; + +// TODO: Post-audit move to core, as v4-core will use something similar. +library GasLimitCalculator { + uint256 constant BPS_DENOMINATOR = 10_000; + + /// calculates a gas limit as a percentage of the currenct block's gas limit + function toGasLimit(uint256 bps) internal returns (uint256 gasLimit) { + return block.gaslimit * bps / BPS_DENOMINATOR; + } +} diff --git a/test/libraries/GasLimitCalculator.t.sol b/test/libraries/GasLimitCalculator.t.sol new file mode 100644 index 00000000..47129537 --- /dev/null +++ b/test/libraries/GasLimitCalculator.t.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import {GasLimitCalculator} from "../../src/libraries/GasLimitCalculator.sol"; + +contract GasLimitCalculatorTest is Test { + function test_gasLimit_100_percent() public { + assertEq(block.gaslimit, GasLimitCalculator.toGasLimit(10_000)); + } + + function test_gasLimit_1_percent() public { + /// 100 bps = 1% + // 1% of 3_000_000_000 is 30_000_000 + assertEq(30_000_000, GasLimitCalculator.toGasLimit(100)); + } + + function test_gasLimit_1BP() public { + /// 1bp is 0.01% + assertEq(300_000, GasLimitCalculator.toGasLimit(1)); + } +} diff --git a/test/shared/PosmTestSetup.sol b/test/shared/PosmTestSetup.sol index 798a0879..4cfe740f 100644 --- a/test/shared/PosmTestSetup.sol +++ b/test/shared/PosmTestSetup.sol @@ -37,7 +37,7 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { function deployPosm(IPoolManager poolManager) internal { // We use deployPermit2() to prevent having to use via-ir in this repository. permit2 = IAllowanceTransfer(deployPermit2()); - lpm = new PositionManager(poolManager, permit2, 200_000); + lpm = new PositionManager(poolManager, permit2); } function seedBalance(address to) internal { From e8d8a7c35c2b019ca9fc8d1c18b0377416ad69c4 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 18:16:06 -0400 Subject: [PATCH 07/14] test return data --- foundry.toml | 3 + src/PositionManager.sol | 11 ++-- src/base/Notifier.sol | 15 ++++- src/libraries/PositionConfig.sol | 2 +- test/libraries/PositionConfig.t.sol | 10 +-- test/mocks/MockBadSubscribers.sol | 64 +++++++++++++++++++ .../PositionManager.notifier.t.sol | 28 +++++++- 7 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 test/mocks/MockBadSubscribers.sol diff --git a/foundry.toml b/foundry.toml index 54e2ac0c..002bc8be 100644 --- a/foundry.toml +++ b/foundry.toml @@ -11,4 +11,7 @@ fuzz_runs = 10_000 [profile.ci] fuzz_runs = 100_000 +[profile.gas] +gas_limit=30_000_000 + # See more config options https://github.com/foundry-rs/foundry/tree/master/config diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 3667ae23..dc57fb7e 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -92,18 +92,17 @@ contract PositionManager is onlyIfApproved(msg.sender, tokenId) onlyValidConfig(tokenId, config) { - if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); - _subscribe(tokenId, config, subscriber); positionConfigs.setSubscribe(tokenId); + _subscribe(tokenId, config, subscriber); } function unsubscribe(uint256 tokenId, PositionConfig calldata config) external onlyIfApproved(msg.sender, tokenId) + onlyValidConfig(tokenId, config) { - if (positionConfigs.getConfigId(tokenId) != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); - _unsubscribe(tokenId, config); positionConfigs.setUnsubscribe(tokenId); + _unsubscribe(tokenId, config); } function _handleAction(uint256 action, bytes calldata params) internal virtual override { @@ -233,7 +232,7 @@ contract PositionManager is hookData ); - if (positionConfigs.getSubscribed(uint256(salt))) { + if (positionConfigs.hasSubscriber(uint256(salt))) { _notifyModifyLiquidity(uint256(salt), config, liquidityChange); } } @@ -267,6 +266,6 @@ contract PositionManager is /// @dev overrides solmate transferFrom in case a notification to subscribers is needed function transferFrom(address from, address to, uint256 id) public override { super.transferFrom(from, to, id); - if (positionConfigs.getSubscribed(id)) _notifyTransfer(id, from, to); + if (positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to); } } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index c3d6460f..d023e4e9 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -5,6 +5,8 @@ import {ISubscriber} from "../interfaces/ISubscriber.sol"; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {GasLimitCalculator} from "../libraries/GasLimitCalculator.sol"; +import "forge-std/console2.sol"; + contract Notifier { using GasLimitCalculator for uint256; @@ -17,8 +19,9 @@ contract Notifier { ISubscriber private constant NO_SUBSCRIBER = ISubscriber(address(0)); // a percentage of the block.gaslimit denoted in BPS, used as the gas limit for subscriber calls - // 1 BP is 0.01% - uint256 private constant BLOCK_LIMIT_BPS = 1; + // 100 bps is 1% + // at 30M gas, the limit is 300K + uint256 private constant BLOCK_LIMIT_BPS = 100; mapping(uint256 tokenId => ISubscriber subscriber) public subscriber; @@ -39,7 +42,13 @@ contract Notifier { ISubscriber _subscriber = subscriber[tokenId]; uint256 subscriberGasLimit = BLOCK_LIMIT_BPS.toGasLimit(); - try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} + + bytes memory data = abi.encodeWithSelector(ISubscriber.notifyUnsubscribe.selector, tokenId, config); + + //try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} + assembly ("memory-safe") { + pop(call(subscriberGasLimit, _subscriber, 0, add(data, 0x20), mload(data), 0, 0)) + } delete subscriber[tokenId]; emit Unsubscribed(tokenId, address(_subscriber)); diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index 582941c9..bc90aea7 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -42,7 +42,7 @@ library PositionConfigLibrary { positionConfigs[tokenId] &= MASK_UPPER_BIT; } - function getSubscribed(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) + function hasSubscriber(mapping(uint256 => bytes32) storage positionConfigs, uint256 tokenId) internal view returns (bool subscribed) diff --git a/test/libraries/PositionConfig.t.sol b/test/libraries/PositionConfig.t.sol index 04ae4a0b..cfec1a54 100644 --- a/test/libraries/PositionConfig.t.sol +++ b/test/libraries/PositionConfig.t.sol @@ -77,11 +77,11 @@ contract PositionConfigTest is Test { assertEq(_config, 0); } - function test_getSubscribed(uint256 tokenId) public { + function test_hasSubscriber(uint256 tokenId) public { testConfigs.setSubscribe(tokenId); - assert(testConfigs.getSubscribed(tokenId)); + assert(testConfigs.hasSubscriber(tokenId)); testConfigs.setUnsubscribe(tokenId); - assert(!testConfigs.getSubscribed(tokenId)); + assert(!testConfigs.hasSubscriber(tokenId)); } function test_fuzz_setConfigId_setSubscribe_setUnsubscribe_getConfigId( @@ -95,11 +95,11 @@ contract PositionConfigTest is Test { testConfigs.setSubscribe(tokenId); assertEq(testConfigs.getConfigId(tokenId), config.toId()); - assertEq(testConfigs.getSubscribed(tokenId), true); + assertEq(testConfigs.hasSubscriber(tokenId), true); testConfigs.setUnsubscribe(tokenId); assertEq(testConfigs.getConfigId(tokenId), config.toId()); - assertEq(testConfigs.getSubscribed(tokenId), false); + assertEq(testConfigs.hasSubscriber(tokenId), false); } function _calculateExpectedId(PositionConfig calldata config) internal pure returns (bytes32 expectedId) { diff --git a/test/mocks/MockBadSubscribers.sol b/test/mocks/MockBadSubscribers.sol new file mode 100644 index 00000000..aee0e2b2 --- /dev/null +++ b/test/mocks/MockBadSubscribers.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.20; + +import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; +import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; +import {PositionManager} from "../../src/PositionManager.sol"; + +/// This file contains a few malicious Subscriber contract +/// 1. A subscriber contract that returns values, can cause a OOG revert with large return data. + +/// @notice A subscriber contract that returns values from the subscriber entrypoints +contract MockReturnDataSubscriber is ISubscriber { + PositionManager posm; + + uint256 public notifySubscribeCount; + uint256 public notifyUnsubscribeCount; + uint256 public notifyModifyLiquidityCount; + uint256 public notifyTransferCount; + + error NotAuthorizedNotifer(address sender); + + error NotImplemented(); + + uint256 memPtr; + + constructor(PositionManager _posm) { + posm = _posm; + } + + modifier onlyByPosm() { + if (msg.sender != address(posm)) revert NotAuthorizedNotifer(msg.sender); + _; + } + + function notifySubscribe(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + notifySubscribeCount++; + } + + function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external onlyByPosm { + notifyUnsubscribeCount++; + uint256 _memPtr = memPtr; + assembly { + let fmp := mload(0x40) + mstore(fmp, 0xBEEF) + mstore(add(fmp, 0x20), 0xCAFE) + return(fmp, _memPtr) + } + } + + function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) + external + onlyByPosm + { + notifyModifyLiquidityCount++; + } + + function notifyTransfer(uint256 tokenId, address previousOwner, address newOwner) external onlyByPosm { + notifyTransferCount++; + } + + function setReturnDataSize(uint256 _value) external { + memPtr = _value; + } +} diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index dcc227cd..620cdf90 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; import {MockSubscriber} from "../mocks/MockSubscriber.sol"; import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; @@ -10,11 +11,13 @@ import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; import {Plan, Planner} from "../shared/Planner.sol"; import {Actions} from "../../src/libraries/Actions.sol"; +import {MockReturnDataSubscriber} from "../mocks/MockBadSubscribers.sol"; -contract PositionManagerNotifierTest is Test, PosmTestSetup { +contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { using Planner for Plan; MockSubscriber sub; + MockReturnDataSubscriber badSubscriber; PositionConfig config; address alice = makeAddr("ALICE"); @@ -30,6 +33,7 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup { deployAndApprovePosm(manager); sub = new MockSubscriber(lpm); + badSubscriber = new MockReturnDataSubscriber(lpm); config = PositionConfig({poolKey: key, tickLower: -300, tickUpper: 300}); // TODO: Test NATIVE poolKey @@ -77,7 +81,7 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup { lpm.subscribe(tokenId, config, address(sub)); - assertEq(_getSubscribed(lpm.positionConfigs(tokenId)), true); + assertEq(_hasSubscriber(lpm.positionConfigs(tokenId)), true); assertEq(address(lpm.subscriber(tokenId)), address(sub)); assertEq(sub.notifySubscribeCount(), 1); } @@ -121,7 +125,25 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup { assertEq(sub.notifyTransferCount(), 1); } - function _getSubscribed(bytes32 _config) internal pure returns (bool subscribed) { + function test_unsubscribe_isSuccessfulWithBadSubscriber() 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(badSubscriber)); + + MockReturnDataSubscriber(badSubscriber).setReturnDataSize(0x600000); + lpm.unsubscribe(tokenId, config); + + // the subscriber contract call failed bc it used too much gas + assertEq(MockReturnDataSubscriber(badSubscriber).notifyUnsubscribeCount(), 0); + } + + function _hasSubscriber(bytes32 _config) internal pure returns (bool subscribed) { assembly { subscribed := shr(255, _config) } From 724e0c8521f63d9f52ef06d2d3f7716bc83c1c1f Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 18:17:22 -0400 Subject: [PATCH 08/14] use solidity --- src/base/Notifier.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index d023e4e9..0f5f041b 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -45,10 +45,7 @@ contract Notifier { bytes memory data = abi.encodeWithSelector(ISubscriber.notifyUnsubscribe.selector, tokenId, config); - //try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} - assembly ("memory-safe") { - pop(call(subscriberGasLimit, _subscriber, 0, add(data, 0x20), mload(data), 0, 0)) - } + try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} delete subscriber[tokenId]; emit Unsubscribed(tokenId, address(_subscriber)); From 7c0f9712853cb7489306e29d6e9a21cf3a57a1b0 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 18:18:49 -0400 Subject: [PATCH 09/14] comments --- src/libraries/PositionConfig.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index bc90aea7..7c5d632a 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.24; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; -// A PositionConfig is the input for creating and modifying a Position in core, whos truncated hash is set per tokenId +// A PositionConfig is the input for creating and modifying a Position in core, whose truncated hash is set per tokenId struct PositionConfig { PoolKey poolKey; int24 tickLower; @@ -65,7 +65,7 @@ library PositionConfigLibrary { mstore(add(fmp, 0x14), calldataload(add(config, 0x20))) // currency1: [0x20, 0x34) mstore(fmp, calldataload(config)) // currency0: [0x0c, 0x20) - id := shr(1, keccak256(add(fmp, 0x0c), 0x48)) // len is 72 bytes, truncate upper bit of the hash + id := shr(1, keccak256(add(fmp, 0x0c), 0x48)) // len is 72 bytes, truncate lower bit of the hash // now clean the memory we used mstore(add(fmp, 0x40), 0) // fmp+0x40 held hooks (14 bytes), tickLower, tickUpper From e26a09e8e6ecd70a9c4000b83d3e9eeabfe41277 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Thu, 1 Aug 2024 23:34:16 -0400 Subject: [PATCH 10/14] natspec & more tests --- src/PositionManager.sol | 13 +++++ src/base/Notifier.sol | 4 +- src/interfaces/INotifier.sol | 18 +++++++ src/interfaces/IPositionManager.sol | 4 +- test/mocks/MockBadSubscribers.sol | 3 -- .../PositionManager.notifier.t.sol | 48 +++++++++++++++++++ 6 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 src/interfaces/INotifier.sol diff --git a/src/PositionManager.sol b/src/PositionManager.sol index dc57fb7e..5a2c54af 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -26,6 +26,7 @@ import {BaseActionsRouter} from "./base/BaseActionsRouter.sol"; import {Actions} from "./libraries/Actions.sol"; import {Notifier} from "./base/Notifier.sol"; import {CalldataDecoder} from "./libraries/CalldataDecoder.sol"; +import {INotifier} from "./interfaces/INotifier.sol"; contract PositionManager is IPositionManager, @@ -61,16 +62,26 @@ contract PositionManager is permit2 = _permit2; } + /// @notice Reverts if the deadline has passed + /// @param deadline The timestamp at which the call is no longer valid, passed in by the caller modifier checkDeadline(uint256 deadline) { if (block.timestamp > deadline) revert DeadlinePassed(); _; } + /// @notice Reverts if the caller is not the owner or approved for the ERC721 token + /// @param sender The address of the caller + /// @param tokenId the unique identifier of the ERC721 token + /// @dev either msg.sender or _msgSender() is passed in as the sender + /// _msgSender() should ONLY be used if this is being called from within the unlockCallback modifier onlyIfApproved(address sender, uint256 tokenId) { if (!_isApprovedOrOwner(sender, tokenId)) revert NotApproved(sender); _; } + /// @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); _; @@ -87,6 +98,7 @@ contract PositionManager is _executeActions(unlockData); } + /// @inheritdoc INotifier function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external onlyIfApproved(msg.sender, tokenId) @@ -96,6 +108,7 @@ contract PositionManager is _subscribe(tokenId, config, subscriber); } + /// @inheritdoc INotifier function unsubscribe(uint256 tokenId, PositionConfig calldata config) external onlyIfApproved(msg.sender, tokenId) diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 0f5f041b..7e6dfc96 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -5,9 +5,9 @@ import {ISubscriber} from "../interfaces/ISubscriber.sol"; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {GasLimitCalculator} from "../libraries/GasLimitCalculator.sol"; -import "forge-std/console2.sol"; +import "../interfaces/INotifier.sol"; -contract Notifier { +abstract contract Notifier is INotifier { using GasLimitCalculator for uint256; error SubscriberCannotBeNotified(); diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol new file mode 100644 index 00000000..64cba19e --- /dev/null +++ b/src/interfaces/INotifier.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {PositionConfig} from "../libraries/PositionConfig.sol"; + +/// @notice This interface is used to opt in to sending updates to external contracts about position modifications or transfers +interface INotifier { + /// @notice Enables the subscriber to receive notifications for a respective position + /// @param tokenId the ERC721 tokenId + /// @param config the corresponding PositionConfig for the tokenId + /// @param subscriber the address to notify + function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external; + + /// @notice Removes the subscriber from receiving notifications for a respective position + /// @param tokenId the ERC721 tokenId + /// @param config the corresponding PositionConfig for the tokenId + function unsubscribe(uint256 tokenId, PositionConfig calldata config) external; +} diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index 7f6f20c0..3d931bff 100644 --- a/src/interfaces/IPositionManager.sol +++ b/src/interfaces/IPositionManager.sol @@ -3,7 +3,9 @@ pragma solidity ^0.8.24; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; -interface IPositionManager { +import {INotifier} from "./INotifier.sol"; + +interface IPositionManager is INotifier { error NotApproved(address caller); error DeadlinePassed(); error IncorrectPositionConfigForTokenId(uint256 tokenId); diff --git a/test/mocks/MockBadSubscribers.sol b/test/mocks/MockBadSubscribers.sol index aee0e2b2..2fa517a1 100644 --- a/test/mocks/MockBadSubscribers.sol +++ b/test/mocks/MockBadSubscribers.sol @@ -5,9 +5,6 @@ import {ISubscriber} from "../../src/interfaces/ISubscriber.sol"; import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; import {PositionManager} from "../../src/PositionManager.sol"; -/// This file contains a few malicious Subscriber contract -/// 1. A subscriber contract that returns values, can cause a OOG revert with large return data. - /// @notice A subscriber contract that returns values from the subscriber entrypoints contract MockReturnDataSubscriber is ISubscriber { PositionManager posm; diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index 620cdf90..9776ddf5 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -125,6 +125,54 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyTransferCount(), 1); } + function test_notifyTransfer_withSafeTransferFrom_succeeds() 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)); + + lpm.safeTransferFrom(alice, bob, tokenId); + + assertEq(sub.notifyTransferCount(), 1); + } + + function test_notifyTransfer_withSafeTransferFromData_succeeds() 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)); + + lpm.safeTransferFrom(alice, bob, tokenId, ""); + + assertEq(sub.notifyTransferCount(), 1); + } + + function test_unsubscribe_succeeds() 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)); + + lpm.unsubscribe(tokenId, config); + + assertEq(sub.notifyUnsubscribeCount(), 1); + } + function test_unsubscribe_isSuccessfulWithBadSubscriber() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); From c6da12a2f131811dcad9865aa52342025875c8b8 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Fri, 2 Aug 2024 00:13:07 -0400 Subject: [PATCH 11/14] payable --- src/PositionManager.sol | 2 + src/interfaces/INotifier.sol | 4 +- test/position-managers/NativeToken.t.sol | 33 +++++++++++ .../PositionManager.notifier.t.sol | 59 +++++++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 5a2c54af..f30eccc8 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -101,6 +101,7 @@ contract PositionManager is /// @inheritdoc INotifier function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external + payable onlyIfApproved(msg.sender, tokenId) onlyValidConfig(tokenId, config) { @@ -111,6 +112,7 @@ contract PositionManager is /// @inheritdoc INotifier function unsubscribe(uint256 tokenId, PositionConfig calldata config) external + payable onlyIfApproved(msg.sender, tokenId) onlyValidConfig(tokenId, config) { diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol index 64cba19e..4de0d148 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -9,10 +9,10 @@ interface INotifier { /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId /// @param subscriber the address to notify - function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external; + function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external payable; /// @notice Removes the subscriber from receiving notifications for a respective position /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId - function unsubscribe(uint256 tokenId, PositionConfig calldata config) external; + function unsubscribe(uint256 tokenId, PositionConfig calldata config) external payable; } diff --git a/test/position-managers/NativeToken.t.sol b/test/position-managers/NativeToken.t.sol index f8d23c54..38a1194a 100644 --- a/test/position-managers/NativeToken.t.sol +++ b/test/position-managers/NativeToken.t.sol @@ -28,6 +28,7 @@ import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; import {Actions} from "../../src/libraries/Actions.sol"; import {PositionManager} from "../../src/PositionManager.sol"; +import {MockSubscriber} from "../mocks/MockSubscriber.sol"; import {LiquidityFuzzers} from "../shared/fuzz/LiquidityFuzzers.sol"; import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; import {Planner, Plan} from "../shared/Planner.sol"; @@ -44,6 +45,8 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { PoolId poolId; + MockSubscriber sub; + function setUp() public { deployFreshManagerAndRouters(); deployMintAndApprove2Currencies(); @@ -58,6 +61,8 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { // currency0 is the native token so only execute approvals for currency1. approvePosmCurrency(currency1); + sub = new MockSubscriber(lpm); + vm.deal(address(this), type(uint256).max); } @@ -396,4 +401,32 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { assertEq(currency0.balanceOfSelf() - balance0Before, uint128(delta.amount0())); assertEq(currency1.balanceOfSelf() - balance1Before, uint128(delta.amount1())); } + + // this test fails unless subscribe is payable + function test_mutlicall_mint_subscribe_native() public { + uint256 tokenId = lpm.nextTokenId(); + + PositionConfig memory config = PositionConfig({poolKey: nativeKey, tickLower: -60, tickUpper: 60}); + + Plan memory plan = Planner.init(); + plan.add(Actions.MINT_POSITION, abi.encode(config, 10e18, address(this), ZERO_BYTES)); + plan.add(Actions.CLOSE_CURRENCY, abi.encode(config.poolKey.currency0)); + plan.add(Actions.CLOSE_CURRENCY, abi.encode(config.poolKey.currency1)); + plan.add(Actions.SWEEP, abi.encode(CurrencyLibrary.NATIVE, address(this))); + bytes memory actions = plan.encode(); + + bytes[] memory calls = new bytes[](2); + + calls[0] = abi.encodeWithSelector(lpm.modifyLiquidities.selector, actions, _deadline); + calls[1] = abi.encodeWithSelector(lpm.subscribe.selector, tokenId, config, sub); + + lpm.multicall{value: 10e18}(calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + // assertEq(liquidity, 100e18); + // assertEq(sub.notifySubscribeCount(), 1); + } } diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index 9776ddf5..095da7ef 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -3,6 +3,11 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; +import {Position} from "@uniswap/v4-core/src/libraries/Position.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; +import {PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; import {MockSubscriber} from "../mocks/MockSubscriber.sol"; @@ -14,6 +19,8 @@ import {Actions} from "../../src/libraries/Actions.sol"; import {MockReturnDataSubscriber} from "../mocks/MockBadSubscribers.sol"; contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { + using PoolIdLibrary for PoolKey; + using StateLibrary for IPoolManager; using Planner for Plan; MockSubscriber sub; @@ -191,6 +198,58 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(MockReturnDataSubscriber(badSubscriber).notifyUnsubscribeCount(), 0); } + function test_multicall_mint_subscribe() public { + uint256 tokenId = lpm.nextTokenId(); + + Plan memory plan = Planner.init(); + plan.add(Actions.MINT_POSITION, abi.encode(config, 100e18, address(this), ZERO_BYTES)); + bytes memory actions = plan.finalizeModifyLiquidity(config.poolKey); + + bytes[] memory calls = new bytes[](2); + + calls[0] = abi.encodeWithSelector(lpm.modifyLiquidities.selector, actions, _deadline); + calls[1] = abi.encodeWithSelector(lpm.subscribe.selector, tokenId, config, sub); + + lpm.multicall(calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + assertEq(liquidity, 100e18); + assertEq(sub.notifySubscribeCount(), 1); + } + + function test_multicall_mint_subscribe_increase() public { + uint256 tokenId = lpm.nextTokenId(); + + // Encode mint. + Plan memory plan = Planner.init(); + plan.add(Actions.MINT_POSITION, abi.encode(config, 100e18, address(this), ZERO_BYTES)); + bytes memory actions = plan.finalizeModifyLiquidity(config.poolKey); + + // Encode increase separately. + plan = Planner.init(); + plan.add(Actions.INCREASE_LIQUIDITY, abi.encode(tokenId, config, 10e18, ZERO_BYTES)); + bytes memory actions2 = plan.finalizeModifyLiquidity(config.poolKey); + + bytes[] memory calls = new bytes[](3); + + calls[0] = abi.encodeWithSelector(lpm.modifyLiquidities.selector, actions, _deadline); + calls[1] = abi.encodeWithSelector(lpm.subscribe.selector, tokenId, config, sub); + calls[2] = abi.encodeWithSelector(lpm.modifyLiquidities.selector, actions2, _deadline); + + lpm.multicall(calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + assertEq(liquidity, 110e18); + assertEq(sub.notifySubscribeCount(), 1); + assertEq(sub.notifyModifyLiquidityCount(), 1); + } + function _hasSubscriber(bytes32 _config) internal pure returns (bool subscribed) { assembly { subscribed := shr(255, _config) From b3e02a7d4e312b89ebe730cf5e6c04e3998a925e Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Fri, 2 Aug 2024 10:54:17 -0400 Subject: [PATCH 12/14] pr comments --- src/PositionManager.sol | 7 ++++--- src/base/Notifier.sol | 5 +---- src/interfaces/IERC721Permit.sol | 1 + src/interfaces/INotifier.sol | 3 +++ test/position-managers/NativeToken.t.sol | 6 +++--- .../PositionManager.notifier.t.sol | 13 +++++++++++++ 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/PositionManager.sol b/src/PositionManager.sol index c41f2568..ff73f682 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -72,12 +72,12 @@ contract PositionManager is } /// @notice Reverts if the caller is not the owner or approved for the ERC721 token - /// @param sender The address of the caller + /// @param caller The address of the caller /// @param tokenId the unique identifier of the ERC721 token /// @dev either msg.sender or _msgSender() is passed in as the sender /// _msgSender() should ONLY be used if this is being called from within the unlockCallback - modifier onlyIfApproved(address sender, uint256 tokenId) { - if (!_isApprovedOrOwner(sender, tokenId)) revert NotApproved(sender); + modifier onlyIfApproved(address caller, uint256 tokenId) { + if (!_isApprovedOrOwner(caller, tokenId)) revert NotApproved(caller); _; } @@ -107,6 +107,7 @@ contract PositionManager is 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); } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 7e6dfc96..d878336c 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -7,6 +7,7 @@ import {GasLimitCalculator} from "../libraries/GasLimitCalculator.sol"; import "../interfaces/INotifier.sol"; +/// @notice Notifier is used to opt in to sending updates to external contracts about position modifications or transfers abstract contract Notifier is INotifier { using GasLimitCalculator for uint256; @@ -25,8 +26,6 @@ abstract contract Notifier is INotifier { mapping(uint256 tokenId => ISubscriber subscriber) public subscriber; - constructor() {} - function _subscribe(uint256 tokenId, PositionConfig memory config, address newSubscriber) internal { ISubscriber _subscriber = subscriber[tokenId]; @@ -43,8 +42,6 @@ abstract contract Notifier is INotifier { uint256 subscriberGasLimit = BLOCK_LIMIT_BPS.toGasLimit(); - bytes memory data = abi.encodeWithSelector(ISubscriber.notifyUnsubscribe.selector, tokenId, config); - try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} delete subscriber[tokenId]; diff --git a/src/interfaces/IERC721Permit.sol b/src/interfaces/IERC721Permit.sol index 213bca2a..a2dec88b 100644 --- a/src/interfaces/IERC721Permit.sol +++ b/src/interfaces/IERC721Permit.sol @@ -21,6 +21,7 @@ interface IERC721Permit { /// @param v Must produce valid secp256k1 signature from the holder along with `r` and `s` /// @param r Must produce valid secp256k1 signature from the holder along with `v` and `s` /// @param s Must produce valid secp256k1 signature from the holder along with `r` and `v` + /// @dev payable so it can be multicalled with NATIVE related acitons function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, uint8 v, bytes32 r, bytes32 s) external payable; diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol index 4de0d148..edd2c3cf 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -9,10 +9,13 @@ interface INotifier { /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId /// @param subscriber the address to notify + /// @dev Calling subscribe when a position is already subscribed will revert + /// @dev payable so it can be multicalled with NATIVE related acitons function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external payable; /// @notice Removes the subscriber from receiving notifications for a respective position /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId + /// @dev payable so it can be multicalled with NATIVE related acitons function unsubscribe(uint256 tokenId, PositionConfig calldata config) external payable; } diff --git a/test/position-managers/NativeToken.t.sol b/test/position-managers/NativeToken.t.sol index a5347554..83e3d120 100644 --- a/test/position-managers/NativeToken.t.sol +++ b/test/position-managers/NativeToken.t.sol @@ -417,7 +417,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { Plan memory plan = Planner.init(); plan.add( Actions.MINT_POSITION, - abi.encode(config, 10e18, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, address(this), ZERO_BYTES) + abi.encode(config, 100e18, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, address(this), ZERO_BYTES) ); plan.add(Actions.CLOSE_CURRENCY, abi.encode(config.poolKey.currency0)); plan.add(Actions.CLOSE_CURRENCY, abi.encode(config.poolKey.currency1)); @@ -435,7 +435,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); - // assertEq(liquidity, 100e18); - // assertEq(sub.notifySubscribeCount(), 1); + assertEq(liquidity, 100e18); + assertEq(sub.notifySubscribeCount(), 1); } } diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index d1249149..fd6f6725 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -262,6 +262,19 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyModifyLiquidityCount(), 1); } + function test_unsubscribe_revertsWhenNotSubscribed() 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(); + + vm.expectRevert(); + lpm.unsubscribe(tokenId, config); + } + function _hasSubscriber(bytes32 _config) internal pure returns (bool subscribed) { assembly { subscribed := shr(255, _config) From 989b54709c0dd6ebe6efc014d7d154e9a316725b Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Fri, 2 Aug 2024 17:38:33 -0400 Subject: [PATCH 13/14] pr comments --- .../PositionManager_burn_empty.snap | 2 +- .../PositionManager_burn_empty_native.snap | 2 +- .../PositionManager_burn_nonEmpty.snap | 2 +- .../PositionManager_burn_nonEmpty_native.snap | 2 +- .forge-snapshots/PositionManager_collect.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_decreaseLiquidity.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- ...sitionManager_increaseLiquidity_erc20.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- ...ger_increase_autocompound_clearExcess.snap | 2 +- .forge-snapshots/PositionManager_mint.snap | 2 +- .../PositionManager_mint_native.snap | 2 +- .../PositionManager_mint_nativeWithSweep.snap | 2 +- .../PositionManager_mint_onSameTickLower.snap | 2 +- .../PositionManager_mint_onSameTickUpper.snap | 2 +- .../PositionManager_mint_sameRange.snap | 2 +- ...nManager_mint_settleWithBalance_sweep.snap | 2 +- ...anager_mint_warmedPool_differentRange.snap | 2 +- ...tionManager_multicall_initialize_mint.snap | 2 +- src/PositionManager.sol | 15 +++++++--- src/base/Notifier.sol | 1 - src/interfaces/IERC721Permit.sol | 2 +- src/interfaces/INotifier.sol | 9 ++++-- src/interfaces/IPositionManager.sol | 13 ++++----- test/position-managers/NativeToken.t.sol | 2 +- .../PositionManager.notifier.t.sol | 29 ++++++++++++++----- 33 files changed, 73 insertions(+), 50 deletions(-) diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index d4a6cbf6..b4f9b770 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -47187 \ No newline at end of file +47168 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap index a04aaf5c..58f0bac9 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -47004 \ No newline at end of file +46986 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty.snap b/.forge-snapshots/PositionManager_burn_nonEmpty.snap index 042ea26f..274fdc43 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty.snap @@ -1 +1 @@ -130137 \ No newline at end of file +130119 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap index 4e92f5cc..0621747c 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap @@ -1 +1 @@ -123059 \ No newline at end of file +123040 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect.snap b/.forge-snapshots/PositionManager_collect.snap index ea302fda..d68e06f1 100644 --- a/.forge-snapshots/PositionManager_collect.snap +++ b/.forge-snapshots/PositionManager_collect.snap @@ -1 +1 @@ -150258 \ No newline at end of file +150235 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index b6682a84..edac16c2 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -141410 \ No newline at end of file +141387 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index ea302fda..d68e06f1 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -150258 \ No newline at end of file +150235 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity.snap b/.forge-snapshots/PositionManager_decreaseLiquidity.snap index 8d3abeda..ff261d83 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity.snap @@ -1 +1 @@ -115801 \ No newline at end of file +115778 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index 843b75cf..c3def0ba 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -108603 \ No newline at end of file +108584 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index f2edca7c..bcbd95de 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -134196 \ No newline at end of file +134178 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index f964f8b2..be94d9c7 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -126936 \ No newline at end of file +126917 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index bd1c4424..d53ad25c 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -128517 \ No newline at end of file +128494 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap index 2eca3a97..7be08370 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap @@ -1 +1 @@ -152364 \ No newline at end of file +152341 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index c75835ce..3bdd6761 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -134164 \ No newline at end of file +134141 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index a0924e27..e09bd90f 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -130329 \ No newline at end of file +130306 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 6011c5e4..4aed0cee 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -171023 \ No newline at end of file +171000 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap index b4c32e05..32491bda 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -140867 \ No newline at end of file +140844 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint.snap b/.forge-snapshots/PositionManager_mint.snap index 35234885..e1810838 100644 --- a/.forge-snapshots/PositionManager_mint.snap +++ b/.forge-snapshots/PositionManager_mint.snap @@ -1 +1 @@ -372160 \ No newline at end of file +372168 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 2e58abf2..6209adc6 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -336860 \ No newline at end of file +336868 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap index 1f422fd1..ca54cfb2 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep.snap @@ -1 +1 @@ -345392 \ No newline at end of file +345400 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index 2cb1620a..c22cda78 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -314842 \ No newline at end of file +314850 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index 67f269a6..57477a9b 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -315484 \ No newline at end of file +315492 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index 3bcaf9ad..df6d10d6 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -241066 \ No newline at end of file +241074 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap index 239e9372..7ac43dbb 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -370166 \ No newline at end of file +370174 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap index 3936f4eb..94308d3f 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -320860 \ No newline at end of file +320868 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index 92f68ac1..3498eadd 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -416602 \ No newline at end of file +416521 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index f90f93c8..44f98c66 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -52,8 +52,7 @@ contract PositionManager is /// @dev The ID of the next token that will be minted. Skips 0 uint256 public nextTokenId = 1; - /// @inheritdoc IPositionManager - mapping(uint256 tokenId => bytes32 config) public positionConfigs; + mapping(uint256 tokenId => bytes32 config) private positionConfigs; IAllowanceTransfer public immutable permit2; @@ -74,7 +73,7 @@ contract PositionManager is /// @notice Reverts if the caller is not the owner or approved for the ERC721 token /// @param caller The address of the caller /// @param tokenId the unique identifier of the ERC721 token - /// @dev either msg.sender or _msgSender() is passed in as the sender + /// @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) { if (!_isApprovedOrOwner(caller, tokenId)) revert NotApproved(caller); @@ -232,7 +231,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[tokenId] = config.toId(); + positionConfigs.setConfigId(tokenId, config); } function _close(Currency currency) internal { @@ -338,4 +337,12 @@ contract PositionManager is super.transferFrom(from, to, id); if (positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to); } + + function getPositionConfigId(uint256 tokenId) external view returns (bytes32) { + return positionConfigs.getConfigId(tokenId); + } + + function hasSubscriber(uint256 tokenId) external view returns (bool) { + return positionConfigs.hasSubscriber(tokenId); + } } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index d878336c..97510888 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -11,7 +11,6 @@ import "../interfaces/INotifier.sol"; abstract contract Notifier is INotifier { using GasLimitCalculator for uint256; - error SubscriberCannotBeNotified(); error AlreadySubscribed(address subscriber); event Subscribed(uint256 tokenId, address subscriber); diff --git a/src/interfaces/IERC721Permit.sol b/src/interfaces/IERC721Permit.sol index a2dec88b..875a5568 100644 --- a/src/interfaces/IERC721Permit.sol +++ b/src/interfaces/IERC721Permit.sol @@ -21,7 +21,7 @@ interface IERC721Permit { /// @param v Must produce valid secp256k1 signature from the holder along with `r` and `s` /// @param r Must produce valid secp256k1 signature from the holder along with `v` and `s` /// @param s Must produce valid secp256k1 signature from the holder along with `r` and `v` - /// @dev payable so it can be multicalled with NATIVE related acitons + /// @dev payable so it can be multicalled with NATIVE related actions function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, uint8 v, bytes32 r, bytes32 s) external payable; diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol index edd2c3cf..6fced60e 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -10,12 +10,17 @@ interface INotifier { /// @param config the corresponding PositionConfig for the tokenId /// @param subscriber the address to notify /// @dev Calling subscribe when a position is already subscribed will revert - /// @dev payable so it can be multicalled with NATIVE related acitons + /// @dev payable so it can be multicalled with NATIVE related actions function subscribe(uint256 tokenId, PositionConfig calldata config, address subscriber) external payable; /// @notice Removes the subscriber from receiving notifications for a respective position /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId - /// @dev payable so it can be multicalled with NATIVE related acitons + /// @dev payable so it can be multicalled with NATIVE related actions function unsubscribe(uint256 tokenId, PositionConfig calldata config) external payable; + + /// @notice Returns whether a a position should call out to notify a subscribing contract on modification or transfer + /// @param tokenId the ERC721 tokenId + /// @return bool whether or not the position has a subscriber + function hasSubscriber(uint256 tokenId) external view returns (bool); } diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index 2f67a18d..c2ae7201 100644 --- a/src/interfaces/IPositionManager.sol +++ b/src/interfaces/IPositionManager.sol @@ -12,18 +12,15 @@ interface IPositionManager is INotifier { error IncorrectPositionConfigForTokenId(uint256 tokenId); error ClearExceedsMaxAmount(Currency currency, int256 amount, uint256 maxAmount); - /// @notice Maps the ERC721 tokenId to a configId, which is a keccak256 hash of the position's pool key, and range (tickLower, tickUpper) - /// Enforces that a minted ERC721 token is tied to one range on one pool. - /// @param tokenId the ERC721 tokenId, assigned at mint - /// @return configId a truncated hash of the position's poolkey, tickLower, and tickUpper and a reserved upper bit for the isSubscribed flag - /// @dev the highest bit of the configId is used to signal if the position is subscribed - /// and the lower bits contain the truncated hash of the PositionConfig - function positionConfigs(uint256 tokenId) external view returns (bytes32 configId); - /// @notice Batches many liquidity modification calls to pool manager /// @param payload is an encoding of actions, and parameters for those actions /// @param deadline is the deadline for the batched actions to be executed function modifyLiquidities(bytes calldata payload, uint256 deadline) external payable; function nextTokenId() external view returns (uint256); + + /// @param tokenId the ERC721 tokenId + /// @return configId a truncated hash of the position's poolkey, tickLower, and tickUpper + /// @dev truncates the least significant bit of the hash + function getPositionConfigId(uint256 tokenId) external view returns (bytes32 configId); } diff --git a/test/position-managers/NativeToken.t.sol b/test/position-managers/NativeToken.t.sol index 83e3d120..72c38920 100644 --- a/test/position-managers/NativeToken.t.sol +++ b/test/position-managers/NativeToken.t.sol @@ -409,7 +409,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { } // this test fails unless subscribe is payable - function test_mutlicall_mint_subscribe_native() public { + function test_multicall_mint_subscribe_native() public { uint256 tokenId = lpm.nextTokenId(); PositionConfig memory config = PositionConfig({poolKey: nativeKey, tickLower: -60, tickUpper: 60}); diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index fd6f6725..9119be52 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -88,7 +88,7 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub)); - assertEq(_hasSubscriber(lpm.positionConfigs(tokenId)), true); + assertEq(lpm.hasSubscriber(tokenId), true); assertEq(address(lpm.subscriber(tokenId)), address(sub)); assertEq(sub.notifySubscribeCount(), 1); } @@ -104,6 +104,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub)); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + Plan memory plan = Planner.init(); for (uint256 i = 0; i < 10; i++) { plan.add( @@ -130,6 +133,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub)); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + lpm.transferFrom(alice, bob, tokenId); assertEq(sub.notifyTransferCount(), 1); @@ -146,6 +152,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub)); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + lpm.safeTransferFrom(alice, bob, tokenId); assertEq(sub.notifyTransferCount(), 1); @@ -162,6 +171,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub)); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + lpm.safeTransferFrom(alice, bob, tokenId, ""); assertEq(sub.notifyTransferCount(), 1); @@ -181,6 +193,8 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.unsubscribe(tokenId, config); assertEq(sub.notifyUnsubscribeCount(), 1); + assertEq(lpm.hasSubscriber(tokenId), false); + assertEq(address(lpm.subscriber(tokenId)), address(0)); } function test_unsubscribe_isSuccessfulWithBadSubscriber() public { @@ -199,6 +213,8 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { // the subscriber contract call failed bc it used too much gas assertEq(MockReturnDataSubscriber(badSubscriber).notifyUnsubscribeCount(), 0); + assertEq(lpm.hasSubscriber(tokenId), false); + assertEq(address(lpm.subscriber(tokenId)), address(0)); } function test_multicall_mint_subscribe() public { @@ -224,6 +240,9 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(liquidity, 100e18); assertEq(sub.notifySubscribeCount(), 1); + + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); } function test_multicall_mint_subscribe_increase() public { @@ -260,6 +279,8 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(liquidity, 110e18); assertEq(sub.notifySubscribeCount(), 1); assertEq(sub.notifyModifyLiquidityCount(), 1); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); } function test_unsubscribe_revertsWhenNotSubscribed() public { @@ -274,10 +295,4 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { vm.expectRevert(); lpm.unsubscribe(tokenId, config); } - - function _hasSubscriber(bytes32 _config) internal pure returns (bool subscribed) { - assembly { - subscribed := shr(255, _config) - } - } } From b0b8031a92bf5af512423426e5a2b04b557e2ee9 Mon Sep 17 00:00:00 2001 From: Sara Reynolds Date: Fri, 2 Aug 2024 18:11:12 -0400 Subject: [PATCH 14/14] inheritdoc --- src/PositionManager.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 44f98c66..c3b2ba3b 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -338,10 +338,12 @@ contract PositionManager is if (positionConfigs.hasSubscriber(id)) _notifyTransfer(id, from, to); } + /// @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); }