diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap index ccc61b70a..ff43b36e8 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees.snap @@ -1 +1 @@ -177823 \ No newline at end of file +179921 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap index 349667e72..31456f879 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap @@ -1 +1 @@ -192626 \ No newline at end of file +194724 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap index 551e2c975..f7b70cace 100644 --- a/.forge-snapshots/autocompound_excessFeesCredit.snap +++ b/.forge-snapshots/autocompound_excessFeesCredit.snap @@ -1 +1 @@ -192327 \ No newline at end of file +194425 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap index 9bfafe5d7..f499ab6a9 100644 --- a/.forge-snapshots/decreaseLiquidity_erc20.snap +++ b/.forge-snapshots/decreaseLiquidity_erc20.snap @@ -1 +1 @@ -135099 \ No newline at end of file +137197 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap index 9bfafe5d7..f499ab6a9 100644 --- a/.forge-snapshots/decreaseLiquidity_erc6909.snap +++ b/.forge-snapshots/decreaseLiquidity_erc6909.snap @@ -1 +1 @@ -135099 \ No newline at end of file +137197 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap index dd91c8f55..f7e2fbd06 100644 --- a/.forge-snapshots/increaseLiquidity_erc20.snap +++ b/.forge-snapshots/increaseLiquidity_erc20.snap @@ -1 +1 @@ -165128 \ No newline at end of file +167226 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap index dd91c8f55..f7e2fbd06 100644 --- a/.forge-snapshots/increaseLiquidity_erc6909.snap +++ b/.forge-snapshots/increaseLiquidity_erc6909.snap @@ -1 +1 @@ -165128 \ No newline at end of file +167226 \ No newline at end of file diff --git a/.forge-snapshots/mint.snap b/.forge-snapshots/mint.snap index 7f0aa9ea5..334c9f545 100644 --- a/.forge-snapshots/mint.snap +++ b/.forge-snapshots/mint.snap @@ -1 +1 @@ -444628 \ No newline at end of file +441402 \ No newline at end of file diff --git a/.forge-snapshots/permit.snap b/.forge-snapshots/permit.snap index 269d80a6a..f51e74cec 100644 --- a/.forge-snapshots/permit.snap +++ b/.forge-snapshots/permit.snap @@ -1 +1 @@ -75071 \ No newline at end of file +75049 \ No newline at end of file diff --git a/.forge-snapshots/permit_secondPosition.snap b/.forge-snapshots/permit_secondPosition.snap index 15e35dee9..8977b2043 100644 --- a/.forge-snapshots/permit_secondPosition.snap +++ b/.forge-snapshots/permit_secondPosition.snap @@ -1 +1 @@ -57971 \ No newline at end of file +57937 \ No newline at end of file diff --git a/.forge-snapshots/permit_twice.snap b/.forge-snapshots/permit_twice.snap index 6ef9d7616..f1519e1c4 100644 --- a/.forge-snapshots/permit_twice.snap +++ b/.forge-snapshots/permit_twice.snap @@ -1 +1 @@ -40871 \ No newline at end of file +40849 \ No newline at end of file diff --git a/contracts/interfaces/INonfungiblePositionManager.sol b/contracts/interfaces/INonfungiblePositionManager.sol index dda1a8da0..a13380f24 100644 --- a/contracts/interfaces/INonfungiblePositionManager.sol +++ b/contracts/interfaces/INonfungiblePositionManager.sol @@ -27,7 +27,7 @@ interface INonfungiblePositionManager { error DeadlinePassed(); error UnsupportedAction(); - function tokenPositions(uint256 tokenId) external view returns (address, LiquidityRange memory); + function tokenPositions(uint256 tokenId) external view returns (address, LiquidityRange memory, address); /// @notice Batches many liquidity modification calls to pool manager /// @param payload is an encoding of actions, params, and currencies diff --git a/test/position-managers/Permit.t.sol b/test/position-managers/Permit.t.sol index 1075ebcb3..d51717d9c 100644 --- a/test/position-managers/Permit.t.sol +++ b/test/position-managers/Permit.t.sol @@ -33,6 +33,7 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation using CurrencyLibrary for Currency; using LiquidityRangeIdLibrary for LiquidityRange; using PoolIdLibrary for PoolKey; + using StateLibrary for IPoolManager; PoolId poolId; address alice; @@ -92,11 +93,14 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation uint256 newLiquidity = 2e18; uint256 balance0BobBefore = currency0.balanceOf(bob); uint256 balance1BobBefore = currency1.balanceOf(bob); - vm.prank(bob); - _increaseLiquidity(range, tokenIdAlice, newLiquidity, ZERO_BYTES, false); + vm.startPrank(bob); + _increaseLiquidity(tokenIdAlice, newLiquidity, ZERO_BYTES); + vm.stopPrank(); // alice's position has new liquidity - (uint256 liquidity,,,,) = lpm.positions(alice, range.toId()); + bytes32 positionId = + keccak256(abi.encodePacked(address(lpm), range.tickLower, range.tickUpper, bytes32(tokenIdAlice))); + (uint256 liquidity,,) = manager.getPositionInfo(range.poolKey.toId(), positionId); assertEq(liquidity, liquidityAlice + newLiquidity); // bob used his tokens to increase liquidity @@ -115,11 +119,15 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation // bob can decrease liquidity on alice's token uint256 liquidityToRemove = 0.4444e18; - vm.prank(bob); - _decreaseLiquidity(range, tokenIdAlice, liquidityToRemove, ZERO_BYTES, false); + vm.startPrank(bob); + _decreaseLiquidity(tokenIdAlice, liquidityToRemove, ZERO_BYTES); + vm.stopPrank(); // alice's position decreased liquidity - (uint256 liquidity,,,,) = lpm.positions(alice, range.toId()); + bytes32 positionId = + keccak256(abi.encodePacked(address(lpm), range.tickLower, range.tickUpper, bytes32(tokenIdAlice))); + (uint256 liquidity,,) = manager.getPositionInfo(range.poolKey.toId(), positionId); + assertEq(liquidity, liquidityAlice - liquidityToRemove); } @@ -137,16 +145,18 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation // alice gives bob operator permissions _permit(alice, alicePK, tokenIdAlice, bob, 1); - // TODO: enable once we fix recipient collection + // TODO: test collection to recipient with a permissioned operator - // bob collects fees to a recipient - // address recipient = address(0x00444400); - // vm.startPrank(bob); - // _collect(tokenIdAlice, recipient, ZERO_BYTES, false); - // vm.stopPrank(); + // bob collects fees to himself + address recipient = bob; + uint256 balance0BobBefore = currency0.balanceOf(bob); + uint256 balance1BobBefore = currency1.balanceOf(bob); + vm.startPrank(bob); + _collect(tokenIdAlice, recipient, ZERO_BYTES); + vm.stopPrank(); - // assertEq(currency0.balanceOf(recipient), currency0Revenue); - // assertEq(currency1.balanceOf(recipient), currency1Revenue); + assertApproxEqAbs(currency0.balanceOf(recipient), balance0BobBefore + currency0Revenue, 1 wei); + assertApproxEqAbs(currency1.balanceOf(recipient), balance1BobBefore + currency1Revenue, 1 wei); } // --- Fail Scenarios --- // @@ -178,11 +188,10 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation // bob cannot increase liquidity on alice's token uint256 newLiquidity = 2e18; - uint256 balance0BobBefore = currency0.balanceOf(bob); - uint256 balance1BobBefore = currency1.balanceOf(bob); + bytes memory increase = LiquidityOperations.getIncreaseEncoded(tokenIdAlice, newLiquidity, ZERO_BYTES); vm.startPrank(bob); vm.expectRevert("Not approved"); - _increaseLiquidity(range, tokenIdAlice, newLiquidity, ZERO_BYTES, false); + lpm.modifyLiquidities(increase); vm.stopPrank(); } @@ -195,9 +204,10 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation // bob cannot decrease liquidity on alice's token uint256 liquidityToRemove = 0.4444e18; + bytes memory decrease = LiquidityOperations.getDecreaseEncoded(tokenIdAlice, 0.4444e18, ZERO_BYTES); vm.startPrank(bob); vm.expectRevert("Not approved"); - _decreaseLiquidity(range, tokenIdAlice, liquidityToRemove, ZERO_BYTES, false); + lpm.modifyLiquidities(decrease); vm.stopPrank(); } @@ -215,9 +225,10 @@ contract PermitTest is Test, Deployers, GasSnapshot, Fuzzers, LiquidityOperation // bob cannot collect fees to a recipient address recipient = address(0x00444400); + bytes memory collect = LiquidityOperations.getCollectEncoded(tokenIdAlice, recipient, ZERO_BYTES); vm.startPrank(bob); vm.expectRevert("Not approved"); - _collect(range, tokenIdAlice, recipient, ZERO_BYTES, false); + lpm.modifyLiquidities(collect); vm.stopPrank(); } diff --git a/test/shared/FeeMath.sol b/test/shared/FeeMath.sol index 2c88ebce1..ad8746c06 100644 --- a/test/shared/FeeMath.sol +++ b/test/shared/FeeMath.sol @@ -27,7 +27,7 @@ library FeeMath { view returns (BalanceDelta feesOwed) { - (, LiquidityRange memory range) = posm.tokenPositions(tokenId); + (, LiquidityRange memory range,) = posm.tokenPositions(tokenId); // getPosition(poolId, owner, tL, tU, salt) // owner is the position manager diff --git a/test/shared/LiquidityOperations.sol b/test/shared/LiquidityOperations.sol index 220dd8191..5e74db073 100644 --- a/test/shared/LiquidityOperations.sol +++ b/test/shared/LiquidityOperations.sol @@ -30,29 +30,28 @@ contract LiquidityOperations { return abi.decode(result[0], (BalanceDelta)); } - function _increaseLiquidity(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData, bool claims) + // we overloaded this function because vm.prank was hitting .tokenPositions() + // TODO: now that vm.prank is hitting Planner, we can probably consolidate to a single function + function _increaseLiquidity(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData) internal returns (BalanceDelta) { (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); - return _increaseLiquidity(_range, tokenId, liquidityToAdd, hookData, claims); + return _increaseLiquidity(_range, tokenId, liquidityToAdd, hookData); } function _increaseLiquidity( LiquidityRange memory _range, uint256 tokenId, uint256 liquidityToAdd, - bytes memory hookData, - bool claims + bytes memory hookData ) internal returns (BalanceDelta) { - // cannot use Planner because it interferes with cheatcodes - Actions[] memory actions = new Actions[](1); - actions[0] = Actions.INCREASE; - bytes[] memory params = new bytes[](1); - params[0] = abi.encode(tokenId, liquidityToAdd, hookData, claims); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.INCREASE, abi.encode(tokenId, liquidityToAdd, hookData)); planner = planner.finalize(_range); // Close the currencies. - lpm.modifyLiquidities(abi.encode(planner.actions, planner.params)); + bytes[] memory result = lpm.modifyLiquidities(planner.zip()); + return abi.decode(result[0], (BalanceDelta)); } function _decreaseLiquidity(uint256 tokenId, uint256 liquidityToRemove, bytes memory hookData) @@ -61,7 +60,7 @@ contract LiquidityOperations { { (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); - return _decreaseLiquidity(_range, tokenId, liquidityToRemove, hookData, claims); + return _decreaseLiquidity(_range, tokenId, liquidityToRemove, hookData); } // do not make external call before unlockAndExecute, allows us to test reverts @@ -69,38 +68,28 @@ contract LiquidityOperations { LiquidityRange memory _range, uint256 tokenId, uint256 liquidityToRemove, - bytes memory hookData, - bool claims + bytes memory hookData ) internal returns (BalanceDelta) { - // cannot use Planner as it interferes with cheatcodes (prank / expectRevert) - Actions[] memory actions = new Actions[](1); - actions[0] = Actions.DECREASE; - bytes[] memory params = new bytes[](1); - params[0] = abi.encode(tokenId, liquidityToRemove, hookData, claims); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, liquidityToRemove, hookData)); planner = planner.finalize(_range); // Close the currencies. - bytes[] memory result = lpm.modifyLiquidities(abi.encode(planner.actions, planner.params)); + bytes[] memory result = lpm.modifyLiquidities(planner.zip()); return abi.decode(result[0], (BalanceDelta)); } function _collect(uint256 tokenId, address recipient, bytes memory hookData) internal returns (BalanceDelta) { - // Planner.Plan memory planner = Planner.init(); - // planner = planner.add(Actions.DECREASE, abi.encode(tokenId, 0, hookData)); + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + return _collect(_range, tokenId, recipient, hookData); } // do not make external call before unlockAndExecute, allows us to test reverts - function _collect( - LiquidityRange memory _range, - uint256 tokenId, - address recipient, - bytes memory hookData, - bool claims - ) internal returns (BalanceDelta) { - // cannot use Planner because it interferes with cheatcodes - Actions[] memory actions = new Actions[](1); - actions[0] = Actions.COLLECT; - bytes[] memory params = new bytes[](1); - params[0] = abi.encode(tokenId, recipient, hookData, claims); + function _collect(LiquidityRange memory _range, uint256 tokenId, address recipient, bytes memory hookData) + internal + returns (BalanceDelta) + { + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, 0, hookData)); planner = planner.finalize(_range); // Close the currencies. @@ -124,4 +113,44 @@ contract LiquidityOperations { _vm1.prank(signer); lpm.permit(operator, tokenId, block.timestamp + 1, nonce, v, r, s); } + + // Helper functions for getting encoded calldata for .modifyLiquidities + function getIncreaseEncoded(uint256 tokenId, uint256 liquidityToAdd, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.INCREASE, abi.encode(tokenId, liquidityToAdd, hookData)); + planner = planner.finalize(_range); + return planner.zip(); + } + + function getDecreaseEncoded(uint256 tokenId, uint256 liquidityToRemove, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, liquidityToRemove, hookData)); + planner = planner.finalize(_range); + return planner.zip(); + } + + function getCollectEncoded(uint256 tokenId, address recipient, bytes memory hookData) + internal + view + returns (bytes memory) + { + (, LiquidityRange memory _range,) = lpm.tokenPositions(tokenId); + Planner.Plan memory planner = Planner.init(); + planner = planner.add(Actions.DECREASE, abi.encode(tokenId, 0, hookData)); + + // TODO: allow recipient when supported on CLOSE_CURRENCY? + planner = planner.add(Actions.CLOSE_CURRENCY, abi.encode(_range.poolKey.currency0)); + planner = planner.add(Actions.CLOSE_CURRENCY, abi.encode(_range.poolKey.currency1)); + return planner.zip(); + } }