From b97343807ab7055aff8c76f2532265347cb66e48 Mon Sep 17 00:00:00 2001 From: diana Date: Fri, 26 Jul 2024 11:19:09 -0400 Subject: [PATCH] remove isApprovedOrOwner from increase (#207) * remove isApprovedOrOwner from increase * separate test * simplify test * extra comment --- .../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 +- src/PositionManager.sol | 13 ++++------ .../position-managers/IncreaseLiquidity.t.sol | 26 +++++++++++++++++++ 18 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index 113df2a4..ccdbe1ba 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -50237 \ No newline at end of file +50222 \ 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 7224072f..9ff61767 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -50055 \ No newline at end of file +50040 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty.snap b/.forge-snapshots/PositionManager_burn_nonEmpty.snap index 83bc56d3..b60fb958 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty.snap @@ -1 +1 @@ -135881 \ No newline at end of file +135866 \ 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 876e5b8a..6dd8b18f 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native.snap @@ -1 +1 @@ -128803 \ No newline at end of file +128788 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect.snap b/.forge-snapshots/PositionManager_collect.snap index 34bd865f..2f3bb39f 100644 --- a/.forge-snapshots/PositionManager_collect.snap +++ b/.forge-snapshots/PositionManager_collect.snap @@ -1 +1 @@ -158176 \ No newline at end of file +158157 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 64f5c1b9..2c55e112 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -149328 \ No newline at end of file +149309 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index 34bd865f..2f3bb39f 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -158176 \ No newline at end of file +158157 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity.snap b/.forge-snapshots/PositionManager_decreaseLiquidity.snap index fa5a4c1e..a73bc079 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity.snap @@ -1 +1 @@ -123719 \ No newline at end of file +123700 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index ee9d27da..601755bc 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -114937 \ No newline at end of file +114922 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index d7a4374f..4f68da3a 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -142224 \ No newline at end of file +142193 \ 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 5eb166d6..cdb7010b 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -134963 \ No newline at end of file +134932 \ 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 2097ebeb..27de42fa 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -136435 \ No newline at end of file +136416 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap index df2d5c92..f96eb326 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap @@ -1 +1 @@ -152896 \ No newline at end of file +150444 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 7b4e69aa..c6dec8eb 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -138106 \ No newline at end of file +135654 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index 58f9c1c5..eb87d8b0 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -144757 \ No newline at end of file +142305 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 87439f32..627d3cfa 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -180911 \ No newline at end of file +178459 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 4b64c8e7..6a704dac 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -105,7 +105,7 @@ contract PositionManager is (uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) = abi.decode(params, (uint256, PositionConfig, uint256, bytes)); - _validateModify(config, tokenId); + 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 delta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData); return abi.encode(delta); @@ -118,7 +118,8 @@ contract PositionManager is (uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) = abi.decode(params, (uint256, PositionConfig, uint256, bytes)); - _validateModify(config, tokenId); + if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker()); + if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); // Note: the tokenId is used as the salt. BalanceDelta delta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData); @@ -175,7 +176,8 @@ contract PositionManager is (uint256 tokenId, PositionConfig memory config, bytes memory hookData) = abi.decode(params, (uint256, PositionConfig, bytes)); - _validateModify(config, tokenId); + if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker()); + if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); uint256 liquidity = uint256(_getPositionLiquidity(config, tokenId)); // Can only call modify if there is non zero liquidity. @@ -189,11 +191,6 @@ contract PositionManager is return abi.encode(delta); } - function _validateModify(PositionConfig memory config, uint256 tokenId) private view { - if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker()); - if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); - } - function _modifyLiquidity(PositionConfig memory config, int256 liquidityChange, bytes32 salt, bytes memory hookData) internal returns (BalanceDelta liquidityDelta) diff --git a/test/position-managers/IncreaseLiquidity.t.sol b/test/position-managers/IncreaseLiquidity.t.sol index ae76c5d3..d38993a3 100644 --- a/test/position-managers/IncreaseLiquidity.t.sol +++ b/test/position-managers/IncreaseLiquidity.t.sol @@ -11,6 +11,7 @@ import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; import {LiquidityAmounts} from "@uniswap/v4-core/test/utils/LiquidityAmounts.sol"; import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; +import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {FixedPointMathLib} from "solmate/src/utils/FixedPointMathLib.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {Fuzzers} from "@uniswap/v4-core/src/test/Fuzzers.sol"; @@ -164,6 +165,31 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { assertApproxEqAbs(balance1BeforeAlice, currency1.balanceOf(alice), tolerance); } + function test_increaseLiquidity_withUnapprovedCaller() public { + // Alice provides liquidity + // Bob increases Alice's liquidity without being approved + uint256 liquidityAlice = 3_000e18; + + // alice provides liquidity + vm.prank(alice); + mint(config, liquidityAlice, alice, ZERO_BYTES); + uint256 tokenIdAlice = lpm.nextTokenId() - 1; + + bytes32 positionId = + keccak256(abi.encodePacked(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenIdAlice))); + uint128 oldLiquidity = StateLibrary.getPositionLiquidity(manager, config.poolKey.toId(), positionId); + + // 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 = StateLibrary.getPositionLiquidity(manager, config.poolKey.toId(), positionId); + + // 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.