From 241e025bfb45e031aaac8b3d59a12f98c7559612 Mon Sep 17 00:00:00 2001 From: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Date: Thu, 9 May 2024 15:58:57 -0400 Subject: [PATCH] Add salt to modifyLiquidity params (#608) * add salt poc and tests * update gas test * fmt * gas snaps * use lastCallGas * update * add .snap * use external * use external * use overloaded func * use isolate * assertEq, update gas snaps * use scratch space, add multi router test * undo lib/forge-std * comments: * update gas --- ...o already existing position with salt.snap | 1 + .../addLiquidity with empty hook.snap | 2 +- .../addLiquidity with native token.snap | 2 +- .forge-snapshots/addLiquidity.snap | 2 +- ...new liquidity to a position with salt.snap | 1 + .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- .../erc20 collect protocol fees.snap | 2 +- .../native collect protocol fees.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- .forge-snapshots/removeLiquidity.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/PoolManager.sol | 11 +- src/interfaces/IPoolManager.sol | 6 +- src/libraries/Pool.sol | 4 +- src/libraries/Position.sol | 10 +- src/test/PoolNestedActionsTest.sol | 4 +- src/test/SkipCallsTestHook.sol | 2 +- test/ModifyLiquidity.t.sol | 186 ++++++++++++++++++ test/PoolManager.t.sol | 4 +- test/Sync.t.sol | 2 +- test/libraries/Hooks.t.sol | 14 +- test/libraries/Pool.t.sol | 3 +- test/libraries/Position.t.sol | 6 +- test/utils/Deployers.sol | 4 +- 39 files changed, 252 insertions(+), 54 deletions(-) create mode 100644 .forge-snapshots/add liquidity to already existing position with salt.snap create mode 100644 .forge-snapshots/create new liquidity to a position with salt.snap create mode 100644 test/ModifyLiquidity.t.sol diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap new file mode 100644 index 000000000..d48dd491a --- /dev/null +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -0,0 +1 @@ +146357 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index a54c413f5..449ae03c2 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -278788 \ No newline at end of file +279342 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index 36e97649b..97f85c617 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -136156 \ No newline at end of file +136584 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap index df3667106..e63c54318 100644 --- a/.forge-snapshots/addLiquidity.snap +++ b/.forge-snapshots/addLiquidity.snap @@ -1 +1 @@ -145947 \ No newline at end of file +146333 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap new file mode 100644 index 000000000..bd6fabdec --- /dev/null +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -0,0 +1 @@ +294885 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index d03053ee3..89ae48f28 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -108839 \ No newline at end of file +108818 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index f99b4be6f..c5366c4b3 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -149324 \ No newline at end of file +149260 \ No newline at end of file diff --git a/.forge-snapshots/erc20 collect protocol fees.snap b/.forge-snapshots/erc20 collect protocol fees.snap index 9d85816b6..975b23465 100644 --- a/.forge-snapshots/erc20 collect protocol fees.snap +++ b/.forge-snapshots/erc20 collect protocol fees.snap @@ -1 +1 @@ -57332 \ No newline at end of file +57354 \ No newline at end of file diff --git a/.forge-snapshots/native collect protocol fees.snap b/.forge-snapshots/native collect protocol fees.snap index f704d0c34..86d096ded 100644 --- a/.forge-snapshots/native collect protocol fees.snap +++ b/.forge-snapshots/native collect protocol fees.snap @@ -1 +1 @@ -59565 \ No newline at end of file +59587 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index ea6b47a93..1f2f82624 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -22548 \ No newline at end of file +22710 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index 4a890fee8..9b7f305da 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -115868 \ No newline at end of file +116427 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index ab9f1f9e9..12c0317b0 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -113155 \ No newline at end of file +113605 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap index c11cc8c15..c67fcb47d 100644 --- a/.forge-snapshots/removeLiquidity.snap +++ b/.forge-snapshots/removeLiquidity.snap @@ -1 +1 @@ -115856 \ No newline at end of file +116415 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index bc2fb8634..b63e5c735 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -130538 \ No newline at end of file +130629 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 2cdead364..44ba60386 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -148889 \ No newline at end of file +148937 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index aae292123..4f51848d5 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -113433 \ No newline at end of file +113524 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index b01815060..9f50530b3 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -124803 \ No newline at end of file +124851 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index 1ab75f9b4..5df1673f6 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -136807 \ No newline at end of file +136898 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 2634c653d..84508f65d 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -125922 \ No newline at end of file +126013 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 1a060b5e4..60e6f7a84 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -148001 \ No newline at end of file +147983 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index a4154b13b..73b0bfbf5 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -164904 \ No newline at end of file +164886 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index 858c46065..7c98774e1 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -224124 \ No newline at end of file +224212 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index a74236b3b..e1eba0bc6 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -149105 \ No newline at end of file +149153 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 05f7243a9..23c7648dd 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -124815 \ No newline at end of file +124863 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index 5d31b2e86..be735dedf 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -181394 \ No newline at end of file +181496 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 1c9388d2b..222663c68 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -159641 \ No newline at end of file +159743 \ No newline at end of file diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 050ca55a8..0e1d1d78a 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -116,22 +116,22 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim } /// @inheritdoc IPoolManager - function getLiquidity(PoolId id, address _owner, int24 tickLower, int24 tickUpper) + function getLiquidity(PoolId id, address _owner, int24 tickLower, int24 tickUpper, bytes32 salt) external view override returns (uint128 liquidity) { - return pools[id].positions.get(_owner, tickLower, tickUpper).liquidity; + return pools[id].positions.get(_owner, tickLower, tickUpper, salt).liquidity; } - function getPosition(PoolId id, address _owner, int24 tickLower, int24 tickUpper) + function getPosition(PoolId id, address _owner, int24 tickLower, int24 tickUpper, bytes32 salt) external view override returns (Position.Info memory position) { - return pools[id].positions.get(_owner, tickLower, tickUpper); + return pools[id].positions.get(_owner, tickLower, tickUpper, salt); } /// @inheritdoc IPoolManager @@ -239,7 +239,8 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim tickLower: params.tickLower, tickUpper: params.tickUpper, liquidityDelta: params.liquidityDelta.toInt128(), - tickSpacing: key.tickSpacing + tickSpacing: key.tickSpacing, + salt: params.salt }) ); diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol index 1ba47b813..0755288b9 100644 --- a/src/interfaces/IPoolManager.sol +++ b/src/interfaces/IPoolManager.sol @@ -97,7 +97,7 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload { function getLiquidity(PoolId id) external view returns (uint128 liquidity); /// @notice Get the current value of liquidity for the specified pool and position - function getLiquidity(PoolId id, address owner, int24 tickLower, int24 tickUpper) + function getLiquidity(PoolId id, address owner, int24 tickLower, int24 tickUpper, bytes32 salt) external view returns (uint128 liquidity); @@ -115,7 +115,7 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload { returns (uint256 feeGrowthGlobal0, uint256 feeGrowthGlobal1); /// @notice Get the position struct for a specified pool and position - function getPosition(PoolId id, address owner, int24 tickLower, int24 tickUpper) + function getPosition(PoolId id, address owner, int24 tickLower, int24 tickUpper, bytes32 salt) external view returns (Position.Info memory position); @@ -152,6 +152,8 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload { int24 tickUpper; // how to modify the liquidity int256 liquidityDelta; + // a value to set if you want unique liquidity positions at the same range + bytes32 salt; } /// @notice Modify the liquidity for the given pool diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index e7b6420c0..d61f17525 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -143,6 +143,8 @@ library Pool { int128 liquidityDelta; // the spacing between ticks int24 tickSpacing; + // used to distinguish positions of the same owner, at the same tick range + bytes32 salt; } struct ModifyLiquidityState { @@ -200,7 +202,7 @@ library Pool { (state.feeGrowthInside0X128, state.feeGrowthInside1X128) = getFeeGrowthInside(self, tickLower, tickUpper); - Position.Info storage position = self.positions.get(params.owner, tickLower, tickUpper); + Position.Info storage position = self.positions.get(params.owner, tickLower, tickUpper, params.salt); (feesOwed0, feesOwed1) = position.update(liquidityDelta, state.feeGrowthInside0X128, state.feeGrowthInside1X128); diff --git a/src/libraries/Position.sol b/src/libraries/Position.sol index 3b08fbd69..13563b63f 100644 --- a/src/libraries/Position.sol +++ b/src/libraries/Position.sol @@ -26,20 +26,24 @@ library Position { /// @param owner The address of the position owner /// @param tickLower The lower tick boundary of the position /// @param tickUpper The upper tick boundary of the position + /// @param salt A unique value to differentiate between multiple positions in the same range /// @return position The position info struct of the given owners' position - function get(mapping(bytes32 => Info) storage self, address owner, int24 tickLower, int24 tickUpper) + function get(mapping(bytes32 => Info) storage self, address owner, int24 tickLower, int24 tickUpper, bytes32 salt) internal view returns (Info storage position) { - // positionKey = keccak256(abi.encodePacked(owner, tickLower, tickUpper)) + // positionKey = keccak256(abi.encodePacked(owner, tickLower, tickUpper, salt)) bytes32 positionKey; + /// @solidity memory-safe-assembly assembly { + mstore(0x26, salt) // [0x26, 0x46) mstore(0x06, tickUpper) // [0x23, 0x26) mstore(0x03, tickLower) // [0x20, 0x23) mstore(0, owner) // [0x0c, 0x20) - positionKey := keccak256(0x0c, 0x1a) + positionKey := keccak256(0x0c, 0x3a) // len is 58 bytes + mstore(0x26, 0) // rewrite 0x26 to 0 } position = self[positionKey]; } diff --git a/src/test/PoolNestedActionsTest.sol b/src/test/PoolNestedActionsTest.sol index 07fbdd081..7cba77aa8 100644 --- a/src/test/PoolNestedActionsTest.sol +++ b/src/test/PoolNestedActionsTest.sol @@ -68,10 +68,10 @@ contract NestedActionExecutor is Test, PoolTestBase { error KeyNotSet(); IPoolManager.ModifyLiquidityParams internal ADD_LIQ_PARAMS = - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18}); + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, salt: 0}); IPoolManager.ModifyLiquidityParams internal REMOVE_LIQ_PARAMS = - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: -1e18}); + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: -1e18, salt: 0}); IPoolManager.SwapParams internal SWAP_PARAMS = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -100, sqrtPriceLimitX96: Constants.SQRT_RATIO_1_2}); diff --git a/src/test/SkipCallsTestHook.sol b/src/test/SkipCallsTestHook.sol index c98a7975c..3c85dac19 100644 --- a/src/test/SkipCallsTestHook.sol +++ b/src/test/SkipCallsTestHook.sol @@ -176,7 +176,7 @@ contract SkipCallsTestHook is BaseTestHooks, Test { ) public { // first hook needs to add liquidity for itself IPoolManager.ModifyLiquidityParams memory newParams = - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18}); + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, salt: 0}); IPoolManager(manager).modifyLiquidity(key, newParams, hookData); // hook removes liquidity IPoolManager(manager).modifyLiquidity(key, params, hookData); diff --git a/test/ModifyLiquidity.t.sol b/test/ModifyLiquidity.t.sol new file mode 100644 index 000000000..5bbbef4df --- /dev/null +++ b/test/ModifyLiquidity.t.sol @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Deployers} from "./utils/Deployers.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; +import {PoolKey} from "src/types/PoolKey.sol"; +import {IPoolManager} from "src/interfaces/IPoolManager.sol"; +import {IHooks} from "src/interfaces/IHooks.sol"; +import {Position} from "src/libraries/Position.sol"; +import {PoolId} from "src/types/PoolId.sol"; +import {PoolModifyLiquidityTest} from "../src/test/PoolModifyLiquidityTest.sol"; +import {Constants} from "./utils/Constants.sol"; +import {Currency} from "src/types/Currency.sol"; +import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; + +contract ModifyLiquidityTest is Test, Deployers, GasSnapshot { + PoolKey simpleKey; // vanilla pool key + PoolId simplePoolId; // id for vanilla pool key + + bytes32 SALT = hex"CAFF"; + + IPoolManager.ModifyLiquidityParams public LIQ_PARAM_NO_SALT = + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, salt: 0}); + + IPoolManager.ModifyLiquidityParams public LIQ_PARAM_SALT = + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, salt: SALT}); + + function setUp() public { + deployFreshManagerAndRouters(); + deployMintAndApprove2Currencies(); + (simpleKey, simplePoolId) = initPool(currency0, currency1, IHooks(address(0)), 3000, SQRT_RATIO_1_1, ZERO_BYTES); + } + + function test_modifyLiquidity_samePosition_zeroSalt_isUpdated() public { + Position.Info memory position = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + assertEq(position.liquidity, 0); + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_NO_SALT, ZERO_BYTES); + position = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + + assertEq(position.liquidity, uint128(uint256(LIQ_PARAM_NO_SALT.liquidityDelta))); + + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_NO_SALT, ZERO_BYTES); + Position.Info memory updated = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + assertEq(updated.liquidity, position.liquidity + uint128(uint256(LIQ_PARAM_NO_SALT.liquidityDelta))); + } + + function test_modifyLiquidity_samePosition_withSalt_isUpdated() public { + Position.Info memory position = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + assertEq(position.liquidity, 0); + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + position = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + assertEq(position.liquidity, uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + Position.Info memory updated = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + assertEq(updated.liquidity, position.liquidity + uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + } + + function test_modifyLiquidity_sameTicks_withDifferentSalt_isNotUpdated() public { + Position.Info memory positionNoSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + + Position.Info memory positionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + assertEq(positionNoSalt.liquidity, 0); + assertEq(positionSalt.liquidity, 0); + + // Modify the liquidity with the salt. + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + + positionNoSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + + positionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + assertEq(positionNoSalt.liquidity, 0); // This position does not have liquidity. + assertEq(positionSalt.liquidity, uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); // This position does. + + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_NO_SALT, ZERO_BYTES); // Now the positions should have the same liquidity. + + positionNoSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + + positionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + // positionSalt should still only have the original liquidity deposited to it + assertEq(positionSalt.liquidity, uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + assertEq(positionNoSalt.liquidity, positionSalt.liquidity); + + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + Position.Info memory updatedWithSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + Position.Info memory updatedNoSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_NO_SALT.tickLower, LIQ_PARAM_NO_SALT.tickUpper, 0 + ); + + assertEq(updatedWithSalt.liquidity, positionSalt.liquidity + uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + assertGt(updatedWithSalt.liquidity, updatedNoSalt.liquidity); + assertEq(updatedNoSalt.liquidity, positionNoSalt.liquidity); + } + + function test_modifyLiquidity_sameSalt_differentLiquidityRouters_doNotEditSamePosition() public { + // Set up new router. + PoolModifyLiquidityTest modifyLiquidityRouter2 = new PoolModifyLiquidityTest(manager); + + MockERC20(Currency.unwrap(currency0)).approve(address(modifyLiquidityRouter2), Constants.MAX_UINT256); + MockERC20(Currency.unwrap(currency1)).approve(address(modifyLiquidityRouter2), Constants.MAX_UINT256); + + IPoolManager.ModifyLiquidityParams memory LIQ_PARAM_SALT_2 = + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 2e18, salt: SALT}); + + // Get the uninitialized positions and assert they have no liquidity. + Position.Info memory positionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + Position.Info memory positionSalt2 = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter2), LIQ_PARAM_SALT_2.tickLower, LIQ_PARAM_SALT_2.tickUpper, SALT + ); + + assertEq(positionSalt.liquidity, 0); + assertEq(positionSalt2.liquidity, 0); + + // Modify the liquidity with the salt with the first router. + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + + Position.Info memory updatedPositionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + Position.Info memory updatedPositionSalt2 = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter2), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + // Assert only the liquidity from the first router is updated. + assertEq(updatedPositionSalt.liquidity, uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + assertEq(updatedPositionSalt2.liquidity, 0); + + // Modify the liquidity with the second router. + modifyLiquidityRouter2.modifyLiquidity(simpleKey, LIQ_PARAM_SALT_2, ZERO_BYTES); + + updatedPositionSalt2 = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter2), LIQ_PARAM_SALT_2.tickLower, LIQ_PARAM_SALT_2.tickUpper, SALT + ); + updatedPositionSalt = manager.getPosition( + simplePoolId, address(modifyLiquidityRouter), LIQ_PARAM_SALT.tickLower, LIQ_PARAM_SALT.tickUpper, SALT + ); + + // Assert only the liquidity from the second router is updated. + assertEq(updatedPositionSalt2.liquidity, uint128(uint256(LIQ_PARAM_SALT_2.liquidityDelta))); + assertEq(updatedPositionSalt.liquidity, uint128(uint256(LIQ_PARAM_SALT.liquidityDelta))); + } + + function test_gas_modifyLiquidity_newPosition() public { + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + snapLastCall("create new liquidity to a position with salt"); + } + + function test_gas_modifyLiquidity_updateSamePosition_withSalt() public { + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + modifyLiquidityRouter.modifyLiquidity(simpleKey, LIQ_PARAM_SALT, ZERO_BYTES); + snapLastCall("add liquidity to already existing position with salt"); + } +} diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index a2fa50ce4..7d8fcce8b 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -1164,7 +1164,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { // manager.initialize(key, SQRT_RATIO_1_1, ZERO_BYTES); // // populate feeGrowthGlobalX128 struct w/ modify + swap - // modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(-120, 120, 5 ether)); + // modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(-120, 120, 5 ether, 0)); // swapRouter.swap( // key, // IPoolManager.SwapParams(false, 1 ether, TickMath.MAX_SQRT_RATIO - 1), @@ -1193,7 +1193,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { function test_getPosition() public view { Position.Info memory managerPosition = - manager.getPosition(key.toId(), address(modifyLiquidityRouter), -120, 120); + manager.getPosition(key.toId(), address(modifyLiquidityRouter), -120, 120, 0); assert(LIQ_PARAMS.liquidityDelta > 0); assertEq(managerPosition.liquidity, uint128(uint256(LIQ_PARAMS.liquidityDelta))); } diff --git a/test/Sync.t.sol b/test/Sync.t.sol index 7c4305cc2..fe18abf53 100644 --- a/test/Sync.t.sol +++ b/test/Sync.t.sol @@ -80,7 +80,7 @@ contract SyncTest is Test, Deployers, GasSnapshot { // Sync has not been called. vm.expectRevert(Reserves.ReservesMustBeSynced.selector); manager.getReserves(currency2); - modifyLiquidityRouter.modifyLiquidity(key2, IPoolManager.ModifyLiquidityParams(-60, 60, 100), new bytes(0)); + modifyLiquidityRouter.modifyLiquidity(key2, IPoolManager.ModifyLiquidityParams(-60, 60, 100, 0), new bytes(0)); (uint256 balanceCurrency2) = currency2.balanceOf(address(manager)); assertEq(manager.getReserves(currency2), balanceCurrency2); } diff --git a/test/libraries/Hooks.t.sol b/test/libraries/Hooks.t.sol index 1c4acdb9b..a64992673 100644 --- a/test/libraries/Hooks.t.sol +++ b/test/libraries/Hooks.t.sol @@ -67,11 +67,11 @@ contract HooksTest is Test, Deployers, GasSnapshot { function test_beforeAfterAddLiquidity_beforeAfterRemoveLiquidity_succeedsWithHook() public { MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 1e18); MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyLiquidityRouter), 1e18); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), new bytes(111)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18, 0), new bytes(111)); assertEq(mockHooks.beforeAddLiquidityData(), new bytes(111)); assertEq(mockHooks.afterAddLiquidityData(), new bytes(111)); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, -1e18), new bytes(222)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, -1e18, 0), new bytes(222)); assertEq(mockHooks.beforeRemoveLiquidityData(), new bytes(222)); assertEq(mockHooks.afterRemoveLiquidityData(), new bytes(222)); } @@ -79,7 +79,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { function test_beforeAfterAddLiquidity_calledWithPositiveLiquidityDelta() public { MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 1e18); MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyLiquidityRouter), 1e18); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 100), new bytes(111)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 100, 0), new bytes(111)); assertEq(mockHooks.beforeAddLiquidityData(), new bytes(111)); assertEq(mockHooks.afterAddLiquidityData(), new bytes(111)); } @@ -87,11 +87,11 @@ contract HooksTest is Test, Deployers, GasSnapshot { function test_beforeAfterRemoveLiquidity_calledWithZeroLiquidityDelta() public { MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 1e18); MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyLiquidityRouter), 1e18); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), new bytes(111)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18, 0), new bytes(111)); assertEq(mockHooks.beforeAddLiquidityData(), new bytes(111)); assertEq(mockHooks.afterAddLiquidityData(), new bytes(111)); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 0), new bytes(222)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 0, 0), new bytes(222)); assertEq(mockHooks.beforeAddLiquidityData(), new bytes(111)); assertEq(mockHooks.afterAddLiquidityData(), new bytes(111)); assertEq(mockHooks.beforeRemoveLiquidityData(), new bytes(222)); @@ -99,10 +99,10 @@ contract HooksTest is Test, Deployers, GasSnapshot { } function test_beforeAfterRemoveLiquidity_calledWithPositiveLiquidityDelta() public { - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), new bytes(111)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18, 0), new bytes(111)); MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 1e18); MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyLiquidityRouter), 1e18); - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, -1e18), new bytes(111)); + modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, -1e18, 0), new bytes(111)); assertEq(mockHooks.beforeRemoveLiquidityData(), new bytes(111)); assertEq(mockHooks.afterRemoveLiquidityData(), new bytes(111)); } diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 25f9bd7f2..187bf1312 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -110,7 +110,8 @@ contract PoolTest is Test { tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, - tickSpacing: 60 + tickSpacing: 60, + salt: 0 }) ); Pool.Slot0 memory slot0 = state.slot0; diff --git a/test/libraries/Position.t.sol b/test/libraries/Position.t.sol index 66c09ce43..6a87bff4e 100644 --- a/test/libraries/Position.t.sol +++ b/test/libraries/Position.t.sol @@ -9,10 +9,10 @@ contract PositionTest is Test { mapping(bytes32 => Position.Info) internal positions; - function test_get_fuzz(address owner, int24 tickLower, int24 tickUpper) public view { - bytes32 positionKey = keccak256(abi.encodePacked(owner, tickLower, tickUpper)); + function test_get_fuzz(address owner, int24 tickLower, int24 tickUpper, bytes32 salt) public view { + bytes32 positionKey = keccak256(abi.encodePacked(owner, tickLower, tickUpper, salt)); Position.Info storage expectedPosition = positions[positionKey]; - Position.Info storage position = positions.get(owner, tickLower, tickUpper); + Position.Info storage position = positions.get(owner, tickLower, tickUpper, salt); bytes32 expectedPositionSlot; bytes32 positionSlot; assembly ("memory-safe") { diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index d0d4d8f8e..996af80f3 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -46,9 +46,9 @@ contract Deployers { uint160 public constant MAX_PRICE_LIMIT = TickMath.MAX_SQRT_RATIO - 1; IPoolManager.ModifyLiquidityParams public LIQ_PARAMS = - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18}); + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18, salt: 0}); IPoolManager.ModifyLiquidityParams public REMOVE_LIQ_PARAMS = - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: -1e18}); + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: -1e18, salt: 0}); // Global variables Currency internal currency0;