diff --git a/test/position-managers/PositionManager.modifiyLiquidities.t.sol b/test/position-managers/PositionManager.modifiyLiquidities.t.sol new file mode 100644 index 00000000..a2a85f29 --- /dev/null +++ b/test/position-managers/PositionManager.modifiyLiquidities.t.sol @@ -0,0 +1,294 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; +import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; +import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; +import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; +import {Position} from "@uniswap/v4-core/src/libraries/Position.sol"; +import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; + +import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; +import {ReentrancyLock} from "../../src/base/ReentrancyLock.sol"; +import {Actions} from "../../src/libraries/Actions.sol"; +import {PositionManager} from "../../src/PositionManager.sol"; +import {PositionConfig} from "../../src/libraries/PositionConfig.sol"; + +import {LiquidityFuzzers} from "../shared/fuzz/LiquidityFuzzers.sol"; +import {Planner, Plan} from "../shared/Planner.sol"; +import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; + +contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityFuzzers { + using StateLibrary for IPoolManager; + using PoolIdLibrary for PoolKey; + + PoolId poolId; + address alice; + uint256 alicePK; + address bob; + + PositionConfig config; + + function setUp() public { + (alice, alicePK) = makeAddrAndKey("ALICE"); + (bob,) = makeAddrAndKey("BOB"); + + deployFreshManagerAndRouters(); + deployMintAndApprove2Currencies(); + + // Requires currency0 and currency1 to be set in base Deployers contract. + deployAndApprovePosm(manager); + + seedBalance(alice); + approvePosmFor(alice); + + // must deploy after posm + // Deploys a hook which can accesses IPositionManager.modifyLiquidities + deployPosmHookModifyLiquidities(); + seedBalance(address(hookModifyLiquidities)); + + (key, poolId) = initPool(currency0, currency1, IHooks(hookModifyLiquidities), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + + config = PositionConfig({poolKey: key, tickLower: -60, tickUpper: 60}); + } + + /// @dev minting liquidity without approval is allowable + function test_hook_mint() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // hook mints a new position in beforeSwap via hookData + uint256 hookTokenId = lpm.nextTokenId(); + uint256 newLiquidity = 10e18; + bytes memory calls = getMintEncoded(config, newLiquidity, address(hookModifyLiquidities), ZERO_BYTES); + + swap(key, true, -1e18, calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + // original liquidity unchanged + assertEq(liquidity, initialLiquidity); + + // hook minted its own position + positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(hookTokenId)); + (liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + assertEq(liquidity, newLiquidity); + + assertEq(lpm.ownerOf(tokenId), address(this)); // original position owned by this contract + assertEq(lpm.ownerOf(hookTokenId), address(hookModifyLiquidities)); // hook position owned by hook + } + + /// @dev increasing liquidity without approval is allowable + function test_hook_increaseLiquidity() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // hook increases liquidity in beforeSwap via hookData + uint256 newLiquidity = 10e18; + bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES); + + swap(key, true, -1e18, calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + assertEq(liquidity, initialLiquidity + newLiquidity); + } + + /// @dev hook can decrease liquidity with approval + function test_hook_decreaseLiquidity() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // approve the hook for decreasing liquidity + lpm.approve(address(hookModifyLiquidities), tokenId); + + // hook decreases liquidity in beforeSwap via hookData + uint256 liquidityToDecrease = 10e18; + bytes memory calls = getDecreaseEncoded(tokenId, config, liquidityToDecrease, ZERO_BYTES); + + swap(key, true, -1e18, calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + assertEq(liquidity, initialLiquidity - liquidityToDecrease); + } + + /// @dev hook can collect liquidity with approval + function test_hook_collect() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // approve the hook for collecting liquidity + lpm.approve(address(hookModifyLiquidities), tokenId); + + // donate to generate revenue + uint256 feeRevenue0 = 1e18; + uint256 feeRevenue1 = 0.1e18; + donateRouter.donate(config.poolKey, feeRevenue0, feeRevenue1, ZERO_BYTES); + + uint256 balance0HookBefore = currency0.balanceOf(address(hookModifyLiquidities)); + uint256 balance1HookBefore = currency1.balanceOf(address(hookModifyLiquidities)); + + // hook collects liquidity in beforeSwap via hookData + bytes memory calls = getCollectEncoded(tokenId, config, ZERO_BYTES); + swap(key, true, -1e18, calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + // liquidity unchanged + assertEq(liquidity, initialLiquidity); + + // hook collected the fee revenue + assertEq(currency0.balanceOf(address(hookModifyLiquidities)), balance0HookBefore + feeRevenue0 - 1 wei); // imprecision, core is keeping 1 wei + assertEq(currency1.balanceOf(address(hookModifyLiquidities)), balance1HookBefore + feeRevenue1 - 1 wei); + } + + /// @dev hook can burn liquidity with approval + function test_hook_burn() public { + // mint some liquidity that is NOT burned in beforeSwap + mint(config, 100e18, address(this), ZERO_BYTES); + + // the position to be burned by the hook + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + // TODO: make this less jank since HookModifyLiquidites also has delta saving capabilities + // BalanceDelta mintDelta = getLastDelta(); + BalanceDelta mintDelta = hookModifyLiquidities.deltas(hookModifyLiquidities.numberDeltasReturned() - 1); + + // approve the hook for burning liquidity + lpm.approve(address(hookModifyLiquidities), tokenId); + + uint256 balance0HookBefore = currency0.balanceOf(address(hookModifyLiquidities)); + uint256 balance1HookBefore = currency1.balanceOf(address(hookModifyLiquidities)); + + // hook burns liquidity in beforeSwap via hookData + bytes memory calls = getBurnEncoded(tokenId, config, ZERO_BYTES); + swap(key, true, -1e18, calls); + + bytes32 positionId = + Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId)); + (uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId); + + // liquidity burned + assertEq(liquidity, 0); + // 721 will revert if the token does not exist + vm.expectRevert(); + lpm.ownerOf(tokenId); + + // hook claimed the burned liquidity + assertEq( + currency0.balanceOf(address(hookModifyLiquidities)), + balance0HookBefore + uint128(-mintDelta.amount0() - 1 wei) // imprecision since core is keeping 1 wei + ); + assertEq( + currency1.balanceOf(address(hookModifyLiquidities)), + balance1HookBefore + uint128(-mintDelta.amount1() - 1 wei) + ); + } + + // --- Revert Scenarios --- // + /// @dev Hook does not have approval so decreasingly liquidity should revert + function test_hook_decreaseLiquidity_revert() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // hook decreases liquidity in beforeSwap via hookData + uint256 liquidityToDecrease = 10e18; + bytes memory calls = getDecreaseEncoded(tokenId, config, liquidityToDecrease, ZERO_BYTES); + + // should revert because hook is not approved + vm.expectRevert( + abi.encodeWithSelector( + Hooks.FailedHookCall.selector, + abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities)) + ) + ); + swap(key, true, -1e18, calls); + } + + /// @dev hook does not have approval so collecting liquidity should revert + function test_hook_collect_revert() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // donate to generate revenue + uint256 feeRevenue0 = 1e18; + uint256 feeRevenue1 = 0.1e18; + donateRouter.donate(config.poolKey, feeRevenue0, feeRevenue1, ZERO_BYTES); + + // hook collects liquidity in beforeSwap via hookData + bytes memory calls = getCollectEncoded(tokenId, config, ZERO_BYTES); + + // should revert because hook is not approved + vm.expectRevert( + abi.encodeWithSelector( + Hooks.FailedHookCall.selector, + abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities)) + ) + ); + swap(key, true, -1e18, calls); + } + + /// @dev hook does not have approval so burning liquidity should revert + function test_hook_burn_revert() public { + // the position to be burned by the hook + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + // hook burns liquidity in beforeSwap via hookData + bytes memory calls = getBurnEncoded(tokenId, config, ZERO_BYTES); + + // should revert because hook is not approved + vm.expectRevert( + abi.encodeWithSelector( + Hooks.FailedHookCall.selector, + abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities)) + ) + ); + swap(key, true, -1e18, calls); + } + + /// @dev hook cannot re-enter modifyLiquidities in beforeRemoveLiquidity + function test_hook_increaseLiquidity_reenter_revert() public { + uint256 initialLiquidity = 100e18; + uint256 tokenId = lpm.nextTokenId(); + mint(config, initialLiquidity, address(this), ZERO_BYTES); + + uint256 newLiquidity = 10e18; + + // to be provided as hookData, so beforeAddLiquidity attempts to increase liquidity + bytes memory hookCall = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES); + bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, hookCall); + + // should revert because hook is re-entering modifyLiquidities + vm.expectRevert( + abi.encodeWithSelector( + Hooks.FailedHookCall.selector, abi.encodeWithSelector(ReentrancyLock.ContractLocked.selector) + ) + ); + lpm.unlockAndModifyLiquidities(calls, _deadline); + } +} diff --git a/test/position-managers/PositionManager.t.sol b/test/position-managers/PositionManager.t.sol index 45a44f39..d9ea37b9 100644 --- a/test/position-managers/PositionManager.t.sol +++ b/test/position-managers/PositionManager.t.sol @@ -346,7 +346,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { assertEq(currency0.balanceOfSelf(), balance0BeforeBurn + uint256(int256(deltaDecrease.amount0()))); assertEq(currency1.balanceOfSelf(), balance1BeforeBurn + uint256(uint128(deltaDecrease.amount1()))); - // OZ 721 will revert if the token does not exist + // 721 will revert if the token does not exist vm.expectRevert(); lpm.ownerOf(1); diff --git a/test/shared/HookModifyLiquidities.sol b/test/shared/HookModifyLiquidities.sol new file mode 100644 index 00000000..d3b523e8 --- /dev/null +++ b/test/shared/HookModifyLiquidities.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"; + +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {BalanceDelta, BalanceDeltaLibrary} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; + +import {HookSavesDelta} from "./HookSavesDelta.sol"; +import {IERC20} from "forge-std/interfaces/IERC20.sol"; + +import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; + +/// @notice This contract is NOT a production use contract. It is meant to be used in testing to verify that external contracts can modify liquidity without a lock (IPositionManager.modifyLiquidities) +/// @dev a hook that can modify liquidity in beforeSwap +contract HookModifyLiquidities is HookSavesDelta { + IPositionManager posm; + IAllowanceTransfer permit2; + + function setAddresses(IPositionManager _posm, IAllowanceTransfer _permit2) external { + posm = _posm; + permit2 = _permit2; + } + + function beforeSwap( + address, /* sender **/ + PoolKey calldata key, /* key **/ + IPoolManager.SwapParams calldata, /* params **/ + bytes calldata hookData + ) external override returns (bytes4, BeforeSwapDelta, uint24) { + approvePosmCurrency(key.currency0); + approvePosmCurrency(key.currency1); + + (bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[])); + posm.modifyLiquidities(actions, params); + return (this.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA, 0); + } + + function beforeAddLiquidity( + address, /* sender **/ + PoolKey calldata, /* key **/ + IPoolManager.ModifyLiquidityParams calldata, /* params **/ + bytes calldata hookData + ) external override returns (bytes4) { + if (hookData.length > 0) { + (bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[])); + posm.modifyLiquidities(actions, params); + } + return this.beforeAddLiquidity.selector; + } + + function beforeRemoveLiquidity( + address, /* sender **/ + PoolKey calldata, /* key **/ + IPoolManager.ModifyLiquidityParams calldata, /* params **/ + bytes calldata hookData + ) external override returns (bytes4) { + if (hookData.length > 0) { + (bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[])); + posm.modifyLiquidities(actions, params); + } + return this.beforeRemoveLiquidity.selector; + } + + function approvePosmCurrency(Currency currency) internal { + // Because POSM uses permit2, we must execute 2 permits/approvals. + // 1. First, the caller must approve permit2 on the token. + IERC20(Currency.unwrap(currency)).approve(address(permit2), type(uint256).max); + // 2. Then, the caller must approve POSM as a spender of permit2. TODO: This could also be a signature. + permit2.approve(Currency.unwrap(currency), address(posm), type(uint160).max, type(uint48).max); + } +} diff --git a/test/shared/PosmTestSetup.sol b/test/shared/PosmTestSetup.sol index 3ae10590..345726c4 100644 --- a/test/shared/PosmTestSetup.sol +++ b/test/shared/PosmTestSetup.sol @@ -14,6 +14,7 @@ import {LiquidityOperations} from "./LiquidityOperations.sol"; import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"; import {DeployPermit2} from "permit2/test/utils/DeployPermit2.sol"; import {HookSavesDelta} from "./HookSavesDelta.sol"; +import {HookModifyLiquidities} from "./HookModifyLiquidities.sol"; import {ERC721PermitHashLibrary} from "../../src/libraries/ERC721PermitHash.sol"; /// @notice A shared test contract that wraps the v4-core deployers contract and exposes basic liquidity operations on posm. @@ -24,12 +25,30 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { HookSavesDelta hook; address hookAddr = address(uint160(Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG)); + HookModifyLiquidities hookModifyLiquidities; + address hookModifyLiquiditiesAddr = address( + uint160( + Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_ADD_LIQUIDITY_FLAG | Hooks.BEFORE_REMOVE_LIQUIDITY_FLAG + | Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG + ) + ); + function deployPosmHookSavesDelta() public { HookSavesDelta impl = new HookSavesDelta(); vm.etch(hookAddr, address(impl).code); hook = HookSavesDelta(hookAddr); } + /// @dev deploys a special test hook where beforeSwap hookData is used to modify liquidity + function deployPosmHookModifyLiquidities() public { + HookModifyLiquidities impl = new HookModifyLiquidities(); + vm.etch(hookModifyLiquiditiesAddr, address(impl).code); + hookModifyLiquidities = HookModifyLiquidities(hookModifyLiquiditiesAddr); + + // set posm address since constructor args are not easily copied by vm.etch + hookModifyLiquidities.setAddresses(lpm, permit2); + } + function deployAndApprovePosm(IPoolManager poolManager) public { deployPosm(poolManager); approvePosm();