fix tests

Uniswap · Jul 15, 2024 · 1ba63de · 1ba63de
1 parent 3fbe616
commit 1ba63de
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 65 deletions.
diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap
@@ -1 +1 @@
-177823
+179921
diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap
@@ -1 +1 @@
-192626
+194724
diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap
@@ -1 +1 @@
-192327
+194425
diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap
@@ -1 +1 @@
-135099
+137197
diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap
@@ -1 +1 @@
-135099
+137197
diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap
@@ -1 +1 @@
-165128
+167226
diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap
@@ -1 +1 @@
-165128
+167226
diff --git a/.forge-snapshots/mint.snap b/.forge-snapshots/mint.snap
@@ -1 +1 @@
-444628
+441402
diff --git a/.forge-snapshots/permit.snap b/.forge-snapshots/permit.snap
@@ -1 +1 @@
-75071
+75049
diff --git a/.forge-snapshots/permit_secondPosition.snap b/.forge-snapshots/permit_secondPosition.snap
@@ -1 +1 @@
-57971
+57937
diff --git a/.forge-snapshots/permit_twice.snap b/.forge-snapshots/permit_twice.snap
@@ -1 +1 @@
-40871
+40849
diff --git 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
@@ -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
@@ -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
@@ -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,46 +60,36 @@ 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
     function _decreaseLiquidity(
         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();
+    }
 }