From bf3b8adffd2dff33807a70177f84048ae253fcc0 Mon Sep 17 00:00:00 2001 From: saucepoint <98790946+saucepoint@users.noreply.github.com> Date: Tue, 6 Aug 2024 18:04:19 -0400 Subject: [PATCH 1/2] Provide feesAccrued to subscriber.notifyModifyLiquidity (#282) * provide delta info to subscibers * notifyModifyLiquidity parameters provided correctly * do not provide liquidity delta to subscribers --- ...anager_burn_nonEmpty_native_withClose.snap | 2 +- ...ger_burn_nonEmpty_native_withTakePair.snap | 2 +- ...sitionManager_burn_nonEmpty_withClose.snap | 2 +- ...ionManager_burn_nonEmpty_withTakePair.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_collect_withClose.snap | 2 +- .../PositionManager_collect_withTakePair.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- ...onManager_decreaseLiquidity_withClose.snap | 2 +- ...anager_decreaseLiquidity_withTakePair.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- .../PositionManager_decrease_take_take.snap | 2 +- ...ger_increaseLiquidity_erc20_withClose.snap | 2 +- ...ncreaseLiquidity_erc20_withSettlePair.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- ...ger_increase_autocompound_clearExcess.snap | 2 +- .../PositionManager_mint_native.snap | 2 +- ...anager_mint_nativeWithSweep_withClose.snap | 2 +- ...r_mint_nativeWithSweep_withSettlePair.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 +- .../PositionManager_mint_withClose.snap | 2 +- .../PositionManager_mint_withSettlePair.snap | 2 +- ...tionManager_multicall_initialize_mint.snap | 2 +- src/PositionManager.sol | 5 ++-- src/base/Notifier.sol | 12 ++++++-- src/interfaces/ISubscriber.sol | 9 +++++- test/mocks/MockBadSubscribers.sol | 5 ++-- test/mocks/MockSubscriber.sol | 10 ++++++- .../PositionManager.notifier.t.sol | 29 +++++++++++++++++++ 38 files changed, 94 insertions(+), 40 deletions(-) diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap index 6fde3500..0c46e3d1 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap @@ -1 +1 @@ -123220 \ No newline at end of file +123226 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap index 3ce8a31d..4daa5df5 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap @@ -1 +1 @@ -122727 \ No newline at end of file +122733 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap index e6c9c985..4e8dd523 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap @@ -1 +1 @@ -130298 \ No newline at end of file +130304 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap index d879fbb6..695f0ba1 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap @@ -1 +1 @@ -129805 \ No newline at end of file +129812 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 91993146..a29149e6 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -141690 \ No newline at end of file +141698 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index 76901910..d0c0d21a 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -150538 \ No newline at end of file +150546 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withClose.snap b/.forge-snapshots/PositionManager_collect_withClose.snap index 76901910..d0c0d21a 100644 --- a/.forge-snapshots/PositionManager_collect_withClose.snap +++ b/.forge-snapshots/PositionManager_collect_withClose.snap @@ -1 +1 @@ -150538 \ No newline at end of file +150546 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withTakePair.snap b/.forge-snapshots/PositionManager_collect_withTakePair.snap index 0e2be24e..6654e101 100644 --- a/.forge-snapshots/PositionManager_collect_withTakePair.snap +++ b/.forge-snapshots/PositionManager_collect_withTakePair.snap @@ -1 +1 @@ -149910 \ No newline at end of file +149918 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index db298b27..bc5857fa 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -108827 \ No newline at end of file +108833 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap index fc743866..d4154674 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap @@ -1 +1 @@ -116081 \ No newline at end of file +116089 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap index 80e95ab1..cfe262e2 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap @@ -1 +1 @@ -115453 \ No newline at end of file +115461 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index c0bfde57..bf8a89f1 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -134377 \ No newline at end of file +134384 \ 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 db56dc9c..64fa2092 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -127116 \ No newline at end of file +127123 \ 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 1c3eba40..3df0c73c 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -128797 \ No newline at end of file +128805 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_take_take.snap b/.forge-snapshots/PositionManager_decrease_take_take.snap index dd61f1d3..afeb3089 100644 --- a/.forge-snapshots/PositionManager_decrease_take_take.snap +++ b/.forge-snapshots/PositionManager_decrease_take_take.snap @@ -1 +1 @@ -116614 \ No newline at end of file +116622 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap index 9435d15a..8ed74edd 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap @@ -1 +1 @@ -152488 \ No newline at end of file +152496 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap index e7d1e62a..768175ec 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap @@ -1 +1 @@ -151490 \ No newline at end of file +151498 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index b17d1e2c..21ace784 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -134288 \ No newline at end of file +134296 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index e65873e3..7eea1984 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -130387 \ No newline at end of file +130395 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 32c25b70..3be3e37d 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -171303 \ No newline at end of file +171311 \ 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 888d36a1..c1a17b31 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -141259 \ No newline at end of file +141267 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 1fff7e94..b9627da9 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -340631 \ No newline at end of file +340639 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap index 7ec221a4..5c77d4aa 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap @@ -1 +1 @@ -349123 \ No newline at end of file +349131 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap index 80f6525a..aa3817d2 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap @@ -1 +1 @@ -348425 \ No newline at end of file +348433 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index 14dd1b79..2d86e061 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -318613 \ No newline at end of file +318621 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index aba6356f..ff727a45 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -319255 \ No newline at end of file +319263 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index a6cf5393..d8c6c3a6 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -244837 \ No newline at end of file +244845 \ 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 095bb62b..99790fc7 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -374655 \ No newline at end of file +374663 \ 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 62ccd91f..780d8a42 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -324631 \ No newline at end of file +324639 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap index 27da45cd..8dab44ba 100644 --- a/.forge-snapshots/PositionManager_mint_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_withClose.snap @@ -1 +1 @@ -375931 \ No newline at end of file +375939 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap index 4ee5b345..6a830e87 100644 --- a/.forge-snapshots/PositionManager_mint_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_withSettlePair.snap @@ -1 +1 @@ -375071 \ No newline at end of file +375079 \ 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 0a30edb5..5a4056a9 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -420405 \ No newline at end of file +420413 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 9be9413f..b19a0425 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -333,7 +333,8 @@ contract PositionManager is bytes32 salt, bytes calldata hookData ) internal returns (BalanceDelta liquidityDelta) { - (liquidityDelta,) = poolManager.modifyLiquidity( + BalanceDelta feesAccrued; + (liquidityDelta, feesAccrued) = poolManager.modifyLiquidity( config.poolKey, IPoolManager.ModifyLiquidityParams({ tickLower: config.tickLower, @@ -345,7 +346,7 @@ contract PositionManager is ); if (positionConfigs.hasSubscriber(uint256(salt))) { - _notifyModifyLiquidity(uint256(salt), config, liquidityChange); + _notifyModifyLiquidity(uint256(salt), config, liquidityChange, feesAccrued); } } diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 2d6669a8..09a542d3 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -6,6 +6,7 @@ import {PositionConfig} from "../libraries/PositionConfig.sol"; import {BipsLibrary} from "../libraries/BipsLibrary.sol"; import {INotifier} from "../interfaces/INotifier.sol"; import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; /// @notice Notifier is used to opt in to sending updates to external contracts about position modifications or transfers abstract contract Notifier is INotifier { @@ -57,12 +58,19 @@ abstract contract Notifier is INotifier { emit Unsubscribed(tokenId, address(_subscriber)); } - function _notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) internal { + function _notifyModifyLiquidity( + uint256 tokenId, + PositionConfig memory config, + int256 liquidityChange, + BalanceDelta feesAccrued + ) internal { ISubscriber _subscriber = subscriber[tokenId]; bool success = _call( address(_subscriber), - abi.encodeWithSelector(ISubscriber.notifyModifyLiquidity.selector, tokenId, config, liquidityChange) + abi.encodeWithSelector( + ISubscriber.notifyModifyLiquidity.selector, tokenId, config, liquidityChange, feesAccrued + ) ); if (!success) { diff --git a/src/interfaces/ISubscriber.sol b/src/interfaces/ISubscriber.sol index 81a68c0d..18d428c7 100644 --- a/src/interfaces/ISubscriber.sol +++ b/src/interfaces/ISubscriber.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.24; import {PositionConfig} from "../libraries/PositionConfig.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; /// @notice Interface that a Subscriber contract should implement to receive updates from the v4 position manager interface ISubscriber { @@ -16,7 +17,13 @@ interface ISubscriber { /// @param tokenId the token ID of the position /// @param config details about the position /// @param liquidityChange the change in liquidity on the underlying position - function notifyModifyLiquidity(uint256 tokenId, PositionConfig memory config, int256 liquidityChange) external; + /// @param feesAccrued the fees to be collected from the position as a result of the modifyLiquidity call + function notifyModifyLiquidity( + uint256 tokenId, + PositionConfig memory config, + int256 liquidityChange, + BalanceDelta feesAccrued + ) external; /// @param tokenId the token ID of the position /// @param previousOwner address of the old owner /// @param newOwner address of the new owner diff --git a/test/mocks/MockBadSubscribers.sol b/test/mocks/MockBadSubscribers.sol index b8db89aa..30bafba7 100644 --- a/test/mocks/MockBadSubscribers.sol +++ b/test/mocks/MockBadSubscribers.sol @@ -4,6 +4,7 @@ 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"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; /// @notice A subscriber contract that returns values from the subscriber entrypoints contract MockReturnDataSubscriber is ISubscriber { @@ -44,7 +45,7 @@ contract MockReturnDataSubscriber is ISubscriber { } } - function notifyModifyLiquidity(uint256, PositionConfig memory, int256) external onlyByPosm { + function notifyModifyLiquidity(uint256, PositionConfig memory, int256, BalanceDelta) external onlyByPosm { notifyModifyLiquidityCount++; } @@ -86,7 +87,7 @@ contract MockRevertSubscriber is ISubscriber { revert TestRevert("notifyUnsubscribe"); } - function notifyModifyLiquidity(uint256, PositionConfig memory, int256) external view onlyByPosm { + function notifyModifyLiquidity(uint256, PositionConfig memory, int256, BalanceDelta) external view onlyByPosm { revert TestRevert("notifyModifyLiquidity"); } diff --git a/test/mocks/MockSubscriber.sol b/test/mocks/MockSubscriber.sol index 1e317ad1..032bea86 100644 --- a/test/mocks/MockSubscriber.sol +++ b/test/mocks/MockSubscriber.sol @@ -4,6 +4,7 @@ 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"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; /// @notice A subscriber contract that ingests updates from the v4 position manager contract MockSubscriber is ISubscriber { @@ -13,6 +14,8 @@ contract MockSubscriber is ISubscriber { uint256 public notifyUnsubscribeCount; uint256 public notifyModifyLiquidityCount; uint256 public notifyTransferCount; + int256 public liquidityChange; + BalanceDelta public feesAccrued; bytes public subscribeData; bytes public unsubscribeData; @@ -40,8 +43,13 @@ contract MockSubscriber is ISubscriber { unsubscribeData = data; } - function notifyModifyLiquidity(uint256, PositionConfig memory, int256) external onlyByPosm { + function notifyModifyLiquidity(uint256, PositionConfig memory, int256 _liquidityChange, BalanceDelta _feesAccrued) + external + onlyByPosm + { notifyModifyLiquidityCount++; + liquidityChange = _liquidityChange; + feesAccrued = _feesAccrued; } function notifyTransfer(uint256, address, address) external onlyByPosm { diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index ab008a6c..28f5813f 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -18,6 +18,7 @@ import {Plan, Planner} from "../shared/Planner.sol"; import {Actions} from "../../src/libraries/Actions.sol"; import {INotifier} from "../../src/interfaces/INotifier.sol"; import {MockReturnDataSubscriber, MockRevertSubscriber} from "../mocks/MockBadSubscribers.sol"; +import {BalanceDelta, toBalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { using PoolIdLibrary for PoolKey; @@ -125,6 +126,34 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyModifyLiquidityCount(), 10); } + function test_notifyModifyLiquidity_args() public { + uint256 tokenId = lpm.nextTokenId(); + mint(config, 100e18, alice, ZERO_BYTES); + + // donate to generate fee revenue, to be checked in subscriber + uint256 feeRevenue0 = 1e18; + uint256 feeRevenue1 = 0.1e18; + donateRouter.donate(config.poolKey, feeRevenue0, feeRevenue1, ZERO_BYTES); + + // approve this contract to operate on alices liq + vm.startPrank(alice); + lpm.approve(address(this), tokenId); + vm.stopPrank(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + + uint256 liquidityToAdd = 10e18; + increaseLiquidity(tokenId, config, liquidityToAdd, ZERO_BYTES); + + assertEq(sub.notifyModifyLiquidityCount(), 1); + assertEq(sub.liquidityChange(), int256(liquidityToAdd)); + assertEq(int256(sub.feesAccrued().amount0()), int256(feeRevenue0) - 1 wei); + assertEq(int256(sub.feesAccrued().amount1()), int256(feeRevenue1) - 1 wei); + } + function test_notifyTransfer_withTransferFrom_succeeds() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); From 17f1a49e10ed58915b0e14cb07543343820bb109 Mon Sep 17 00:00:00 2001 From: saucepoint <98790946+saucepoint@users.noreply.github.com> Date: Wed, 7 Aug 2024 18:17:44 -0400 Subject: [PATCH 2/2] OZ: posm - restore permissioning on increase (#290) * restore permissioning on increase * fix comment * fix code comments --- ...ger_increaseLiquidity_erc20_withClose.snap | 2 +- ...ncreaseLiquidity_erc20_withSettlePair.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- ...ger_increase_autocompound_clearExcess.snap | 2 +- src/PositionManager.sol | 2 +- .../position-managers/IncreaseLiquidity.t.sol | 24 ----------- test/position-managers/Permit.t.sol | 40 +++++++++++++++++-- .../PositionManager.modifyLiquidities.t.sol | 28 ++++++++++++- 10 files changed, 70 insertions(+), 36 deletions(-) diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap index 8ed74edd..b3a45767 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap @@ -1 +1 @@ -152496 \ No newline at end of file +154942 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap index 768175ec..6423f6ee 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap @@ -1 +1 @@ -151498 \ No newline at end of file +153944 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 21ace784..9aed0319 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -134296 \ No newline at end of file +136742 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index 7eea1984..d1ccfa88 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -130395 \ No newline at end of file +132841 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 3be3e37d..55dddada 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -171311 \ No newline at end of file +173757 \ 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 c1a17b31..9bac5046 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -141267 \ No newline at end of file +143713 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index b19a0425..d76b08ff 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -215,7 +215,7 @@ contract PositionManager is uint128 amount0Max, uint128 amount1Max, bytes calldata hookData - ) internal onlyValidConfig(tokenId, config) { + ) internal onlyIfApproved(msgSender(), tokenId) onlyValidConfig(tokenId, config) { // 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); liquidityDelta.validateMaxInNegative(amount0Max, amount1Max); diff --git a/test/position-managers/IncreaseLiquidity.t.sol b/test/position-managers/IncreaseLiquidity.t.sol index f1a65efd..e9aa4946 100644 --- a/test/position-managers/IncreaseLiquidity.t.sol +++ b/test/position-managers/IncreaseLiquidity.t.sol @@ -294,30 +294,6 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { assertEq(currency1.balanceOf(alice), balance1BeforeAlice); } - function test_increaseLiquidity_withUnapprovedCaller() public { - // Alice provides liquidity - // Bob increases Alice's liquidity without being approved - uint256 liquidityAlice = 3_000e18; - - // alice provides liquidity - vm.startPrank(alice); - uint256 tokenIdAlice = lpm.nextTokenId(); - mint(config, liquidityAlice, alice, ZERO_BYTES); - vm.stopPrank(); - - uint128 oldLiquidity = lpm.getPositionLiquidity(tokenIdAlice, config); - - // bob can increase liquidity for alice even though he is not the owner / not approved - vm.startPrank(bob); - increaseLiquidity(tokenIdAlice, config, 100e18, ZERO_BYTES); - vm.stopPrank(); - - uint128 newLiquidity = lpm.getPositionLiquidity(tokenIdAlice, config); - - // assert liqudity increased by the correct amount - assertEq(newLiquidity, oldLiquidity + uint128(100e18)); - } - function test_increaseLiquidity_sameRange_withExcessFees() public { // Alice and Bob provide liquidity on the same range // Alice uses half her fees to increase liquidity. The other half are collected to her wallet. diff --git a/test/position-managers/Permit.t.sol b/test/position-managers/Permit.t.sol index 22f887f0..6cf03c3f 100644 --- a/test/position-managers/Permit.t.sol +++ b/test/position-managers/Permit.t.sol @@ -72,6 +72,27 @@ contract PermitTest is Test, PosmTestSetup { ); } + function test_permit_increaseLiquidity() public { + uint256 liquidityAlice = 1e18; + uint256 tokenIdAlice = lpm.nextTokenId(); + vm.prank(alice); + mint(config, liquidityAlice, alice, ZERO_BYTES); + + // alice gives bob permissions + permit(alicePK, tokenIdAlice, bob, 1); + + // bob can increase liquidity on alice's token + uint256 liquidityToAdd = 0.4444e18; + vm.startPrank(bob); + increaseLiquidity(tokenIdAlice, config, liquidityToAdd, ZERO_BYTES); + vm.stopPrank(); + + // alice's position increased liquidity + uint256 liquidity = lpm.getPositionLiquidity(tokenIdAlice, config); + + assertEq(liquidity, liquidityAlice + liquidityToAdd); + } + function test_permit_decreaseLiquidity() public { uint256 liquidityAlice = 1e18; vm.prank(alice); @@ -143,9 +164,22 @@ contract PermitTest is Test, PosmTestSetup { vm.stopPrank(); } - // unapproved callers can increase others' positions - // see `test_increaseLiquidity_withUnapprovedCaller()` - // function test_noPermit_increaseLiquidityRevert() public {} + /// @dev unapproved callers CANNOT increase others' positions + function test_noPermit_increaseLiquidityRevert() public { + // increase fails if the owner did not permit + uint256 liquidityAlice = 1e18; + vm.prank(alice); + mint(config, liquidityAlice, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + // bob cannot increase liquidity on alice's token + uint256 liquidityToAdd = 0.4444e18; + bytes memory decrease = getIncreaseEncoded(tokenIdAlice, config, liquidityToAdd, ZERO_BYTES); + vm.startPrank(bob); + vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(bob))); + lpm.modifyLiquidities(decrease, _deadline); + vm.stopPrank(); + } function test_noPermit_decreaseLiquidityRevert() public { // decreaseLiquidity fails if the owner did not permit diff --git a/test/position-managers/PositionManager.modifyLiquidities.t.sol b/test/position-managers/PositionManager.modifyLiquidities.t.sol index 702b0952..33a8fff7 100644 --- a/test/position-managers/PositionManager.modifyLiquidities.t.sol +++ b/test/position-managers/PositionManager.modifyLiquidities.t.sol @@ -84,12 +84,15 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF assertEq(lpm.ownerOf(hookTokenId), address(hookModifyLiquidities)); // hook position owned by hook } - /// @dev increasing liquidity without approval is allowable + /// @dev hook must be approved to increase liquidity function test_hook_increaseLiquidity() public { uint256 initialLiquidity = 100e18; uint256 tokenId = lpm.nextTokenId(); mint(config, initialLiquidity, address(this), ZERO_BYTES); + // approve the hook for increasing liquidity + lpm.approve(address(hookModifyLiquidities), tokenId); + // hook increases liquidity in beforeSwap via hookData uint256 newLiquidity = 10e18; bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES); @@ -195,7 +198,28 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF } // --- Revert Scenarios --- // - /// @dev Hook does not have approval so decreasingly liquidity should revert + /// @dev Hook does not have approval so increasing liquidity should revert + function test_hook_increaseLiquidity_revert() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // hook decreases liquidity in beforeSwap via hookData + uint256 liquidityToAdd = 10e18; + bytes memory calls = getIncreaseEncoded(tokenId, config, liquidityToAdd, ZERO_BYTES); + + // should revert because hook is not approved + vm.expectRevert( + abi.encodeWithSelector( + Hooks.Wrap__FailedHookCall.selector, + address(hookModifyLiquidities), + abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities)) + ) + ); + swap(key, true, -1e18, calls); + } + + /// @dev Hook does not have approval so decreasing liquidity should revert function test_hook_decreaseLiquidity_revert() public { uint256 initialLiquidity = 100e18; uint256 tokenId = lpm.nextTokenId();