all operations only return a BalanceDelta type (#136)

Uniswap · Jun 29, 2024 · fae83dc · fae83dc
1 parent 406506f
commit fae83dc
Show file tree

Hide file tree

Showing 17 changed files with 83 additions and 63 deletions.
diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap
@@ -1 +1 @@
-262501
+262456
diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap
@@ -1 +1 @@
-194874
+194829
diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap
@@ -1 +1 @@
-283040
+282995
diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap
@@ -1 +1 @@
-180891
+180479
diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap
@@ -1 +1 @@
-180903
+180491
diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap
@@ -1 +1 @@
-175258
+175213
diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap
@@ -1 +1 @@
-150847
+150802
diff --git a/.forge-snapshots/mintWithLiquidity.snap b/.forge-snapshots/mintWithLiquidity.snap
@@ -1 +1 @@
-472846
+472424
diff --git a/contracts/NonfungiblePositionManager.sol b/contracts/NonfungiblePositionManager.sol
@@ -29,7 +29,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
     using SafeCast for uint256;
 
     /// @dev The ID of the next token that will be minted. Skips 0
-    uint256 private _nextId = 1;
+    uint256 public nextTokenId = 1;
 
     // maps the ERC721 tokenId to the keys that uniquely identify a liquidity position (owner, range)
     mapping(uint256 tokenId => TokenPosition position) public tokenPositions;
@@ -66,7 +66,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         uint256 deadline,
         address recipient,
         bytes calldata hookData
-    ) public payable returns (uint256 tokenId, BalanceDelta delta) {
+    ) public payable returns (BalanceDelta delta) {
         // TODO: optimization, read/write manager.isUnlocked to avoid repeated external calls for batched execution
         if (manager.isUnlocked()) {
             BalanceDelta thisDelta;
@@ -77,13 +77,14 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
             _closeThisDeltas(thisDelta, range.poolKey.currency0, range.poolKey.currency1);
 
             // mint receipt token
-            _mint(recipient, (tokenId = _nextId++));
+            uint256 tokenId;
+            _mint(recipient, (tokenId = nextTokenId++));
             tokenPositions[tokenId] = TokenPosition({owner: recipient, range: range});
         } else {
             bytes[] memory data = new bytes[](1);
             data[0] = abi.encodeWithSelector(this.mint.selector, range, liquidity, deadline, recipient, hookData);
             bytes memory result = unlockAndExecute(data);
-            (tokenId, delta) = abi.decode(result, (uint256, BalanceDelta));
+            delta = abi.decode(result, (BalanceDelta));
         }
     }
 
@@ -134,11 +135,12 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
     function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
         public
         isAuthorizedForToken(tokenId)
-        returns (BalanceDelta delta, BalanceDelta thisDelta)
+        returns (BalanceDelta delta)
     {
         TokenPosition memory tokenPos = tokenPositions[tokenId];
 
         if (manager.isUnlocked()) {
+            BalanceDelta thisDelta;
             (delta, thisDelta) = _decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
             _closeCallerDeltas(
                 delta, tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1, tokenPos.owner, claims
@@ -148,7 +150,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
             bytes[] memory data = new bytes[](1);
             data[0] = abi.encodeWithSelector(this.decreaseLiquidity.selector, tokenId, liquidity, hookData, claims);
             bytes memory result = unlockAndExecute(data);
-            (delta, thisDelta) = abi.decode(result, (BalanceDelta, BalanceDelta));
+            delta = abi.decode(result, (BalanceDelta));
         }
     }
 
@@ -164,7 +166,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
         LiquidityRangeId rangeId = tokenPosition.range.toId();
         Position storage position = positions[msg.sender][rangeId];
         if (position.liquidity > 0) {
-            (delta,) = decreaseLiquidity(tokenId, position.liquidity, hookData, claims);
+            delta = decreaseLiquidity(tokenId, position.liquidity, hookData, claims);
         }
 
         collect(tokenId, recipient, hookData, claims);

diff --git a/contracts/base/BaseLiquidityManagement.sol b/contracts/base/BaseLiquidityManagement.sol
@@ -152,7 +152,7 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
 
         return (tokensOwed, callerDelta, thisDelta);
     }
-    
+
     /// Any outstanding amounts owed to the caller from the underlying modify call must be collected explicitly with `collect`.
     function _decreaseLiquidity(
         address owner,

diff --git a/contracts/interfaces/INonfungiblePositionManager.sol b/contracts/interfaces/INonfungiblePositionManager.sol
@@ -17,7 +17,7 @@ interface INonfungiblePositionManager {
         uint256 deadline,
         address recipient,
         bytes calldata hookData
-    ) external payable returns (uint256 tokenId, BalanceDelta delta);
+    ) external payable returns (BalanceDelta delta);
 
     // NOTE: more expensive since LiquidityAmounts is used onchain
     // function mint(MintParams calldata params) external payable returns (uint256 tokenId, BalanceDelta delta);
@@ -37,11 +37,10 @@ interface INonfungiblePositionManager {
     /// @param liquidity The amount of liquidity to remove
     /// @param hookData Arbitrary data passed to the hook
     /// @param claims Whether the removed liquidity is sent as ERC-6909 claim tokens
-    /// @return delta Corresponding balance changes as a result of decreasing liquidity applied to user
-    /// @return thisDelta Corresponding balance changes as a result of decreasing liquidity applied to lpm
+    /// @return delta Corresponding balance changes as a result of decreasing liquidity applied to user (number of tokens credited to tokensOwed)
     function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
         external
-        returns (BalanceDelta delta, BalanceDelta thisDelta);
+        returns (BalanceDelta delta);
 
     /// @notice Burn a position and delete the tokenId
     /// @dev It removes liquidity and collects fees if the position is not empty
@@ -75,4 +74,6 @@ interface INonfungiblePositionManager {
     /// @return token0Owed The amount of token0 owed
     /// @return token1Owed The amount of token1 owed
     function feesOwed(uint256 tokenId) external view returns (uint256 token0Owed, uint256 token1Owed);
+
+    function nextTokenId() external view returns (uint256);
 }
diff --git a/test/position-managers/Execute.t.sol b/test/position-managers/Execute.t.sol
@@ -79,7 +79,8 @@ contract ExecuteTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
     function test_execute_increaseLiquidity_once(uint256 initialLiquidity, uint256 liquidityToAdd) public {
         initialLiquidity = bound(initialLiquidity, 1e18, 1000e18);
         liquidityToAdd = bound(liquidityToAdd, 1e18, 1000e18);
-        (uint256 tokenId,) = lpm.mint(range, initialLiquidity, 0, address(this), ZERO_BYTES);
+        lpm.mint(range, initialLiquidity, 0, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         bytes[] memory data = new bytes[](1);
         data[0] = abi.encodeWithSelector(
@@ -100,7 +101,8 @@ contract ExecuteTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
         initialiLiquidity = bound(initialiLiquidity, 1e18, 1000e18);
         liquidityToAdd = bound(liquidityToAdd, 1e18, 1000e18);
         liquidityToAdd2 = bound(liquidityToAdd2, 1e18, 1000e18);
-        (uint256 tokenId,) = lpm.mint(range, initialiLiquidity, 0, address(this), ZERO_BYTES);
+        lpm.mint(range, initialiLiquidity, 0, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         bytes[] memory data = new bytes[](2);
         data[0] = abi.encodeWithSelector(

diff --git a/test/position-managers/FeeCollection.t.sol b/test/position-managers/FeeCollection.t.sol
@@ -116,8 +116,6 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
     function test_collect_sameRange_6909(IPoolManager.ModifyLiquidityParams memory params, uint256 liquidityDeltaBob)
         public
     {
-        uint256 tokenIdAlice;
-        uint256 tokenIdBob;
         params.liquidityDelta = bound(params.liquidityDelta, 10e18, 10_000e18);
         params = createFuzzyLiquidityParams(key, params, SQRT_PRICE_1_1);
         vm.assume(params.tickLower < 0 && 0 < params.tickUpper); // require two-sided liquidity
@@ -127,10 +125,12 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
         LiquidityRange memory range =
             LiquidityRange({poolKey: key, tickLower: params.tickLower, tickUpper: params.tickUpper});
         vm.prank(alice);
-        (tokenIdAlice,) = lpm.mint(range, uint256(params.liquidityDelta), block.timestamp + 1, alice, ZERO_BYTES);
+        lpm.mint(range, uint256(params.liquidityDelta), block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         vm.prank(bob);
-        (tokenIdBob,) = lpm.mint(range, liquidityDeltaBob, block.timestamp + 1, bob, ZERO_BYTES);
+        lpm.mint(range, liquidityDeltaBob, block.timestamp + 1, bob, ZERO_BYTES);
+        uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // swap to create fees
         uint256 swapAmount = 0.01e18;
@@ -158,8 +158,6 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
     function test_collect_sameRange_erc20(IPoolManager.ModifyLiquidityParams memory params, uint256 liquidityDeltaBob)
         public
     {
-        uint256 tokenIdAlice;
-        uint256 tokenIdBob;
         params.liquidityDelta = bound(params.liquidityDelta, 10e18, 10_000e18);
         params = createFuzzyLiquidityParams(key, params, SQRT_PRICE_1_1);
         vm.assume(params.tickLower < 0 && 0 < params.tickUpper); // require two-sided liquidity
@@ -169,10 +167,12 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
         LiquidityRange memory range =
             LiquidityRange({poolKey: key, tickLower: params.tickLower, tickUpper: params.tickUpper});
         vm.prank(alice);
-        (tokenIdAlice,) = lpm.mint(range, uint256(params.liquidityDelta), block.timestamp + 1, alice, ZERO_BYTES);
+        lpm.mint(range, uint256(params.liquidityDelta), block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         vm.prank(bob);
-        (tokenIdBob,) = lpm.mint(range, liquidityDeltaBob, block.timestamp + 1, bob, ZERO_BYTES);
+        lpm.mint(range, liquidityDeltaBob, block.timestamp + 1, bob, ZERO_BYTES);
+        uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // confirm the positions are same range
         (, LiquidityRange memory rangeAlice) = lpm.tokenPositions(tokenIdAlice);
@@ -228,12 +228,12 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
         uint256 liquidityAlice = 3000e18;
         uint256 liquidityBob = 1000e18;
         vm.prank(alice);
-        (uint256 tokenIdAlice, BalanceDelta lpDeltaAlice) =
-            lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        BalanceDelta lpDeltaAlice = lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         vm.prank(bob);
-        (uint256 tokenIdBob, BalanceDelta lpDeltaBob) =
-            lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        BalanceDelta lpDeltaBob = lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // swap to create fees
         uint256 swapAmount = 0.001e18;
@@ -242,9 +242,7 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
 
         // alice decreases liquidity
         vm.prank(alice);
-        BalanceDelta aliceDelta;
-        BalanceDelta thisDelta;
-        (aliceDelta, thisDelta) = lpm.decreaseLiquidity(tokenIdAlice, liquidityAlice, ZERO_BYTES, true);
+        BalanceDelta aliceDelta = lpm.decreaseLiquidity(tokenIdAlice, liquidityAlice, ZERO_BYTES, true);
 
         uint256 tolerance = 0.000000001 ether;
 
@@ -261,8 +259,7 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers {
 
         // bob decreases half of his liquidity
         vm.prank(bob);
-        BalanceDelta bobDelta;
-        (bobDelta, thisDelta) = lpm.decreaseLiquidity(tokenIdBob, liquidityBob / 2, ZERO_BYTES, true);
+        BalanceDelta bobDelta = lpm.decreaseLiquidity(tokenIdBob, liquidityBob / 2, ZERO_BYTES, true);
 
         // lpm collects half of bobs principal
         // the fee amount has already been collected with alice's calls

diff --git a/test/position-managers/Gas.t.sol b/test/position-managers/Gas.t.sol
@@ -103,14 +103,16 @@ contract GasTest is Test, Deployers, GasSnapshot {
     }
 
     function test_gas_increaseLiquidity_erc20() public {
-        (uint256 tokenId,) = lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         lpm.increaseLiquidity(tokenId, 1000 ether, ZERO_BYTES, false);
         snapLastCall("increaseLiquidity_erc20");
     }
 
     function test_gas_increaseLiquidity_erc6909() public {
-        (uint256 tokenId,) = lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         lpm.increaseLiquidity(tokenId, 1000 ether, ZERO_BYTES, true);
         snapLastCall("increaseLiquidity_erc6909");
@@ -125,7 +127,8 @@ contract GasTest is Test, Deployers, GasSnapshot {
 
         // alice provides liquidity
         vm.prank(alice);
-        (uint256 tokenIdAlice,) = lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         // bob provides liquidity
         vm.prank(bob);
@@ -159,11 +162,13 @@ contract GasTest is Test, Deployers, GasSnapshot {
 
         // alice provides liquidity
         vm.prank(alice);
-        (uint256 tokenIdAlice,) = lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         // bob provides liquidity
         vm.prank(bob);
-        (uint256 tokenIdBob,) = lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // donate to create fees
         donateRouter.donate(key, 20e18, 20e18, ZERO_BYTES);
@@ -203,11 +208,13 @@ contract GasTest is Test, Deployers, GasSnapshot {
 
         // alice provides liquidity
         vm.prank(alice);
-        (uint256 tokenIdAlice,) = lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        lpm.mint(range, liquidityAlice, block.timestamp + 1, alice, ZERO_BYTES);
+        uint256 tokenIdAlice = lpm.nextTokenId() - 1;
 
         // bob provides liquidity
         vm.prank(bob);
-        (uint256 tokenIdBob,) = lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        lpm.mint(range, liquidityBob, block.timestamp + 1, bob, ZERO_BYTES);
+        uint256 tokenIdBob = lpm.nextTokenId() - 1;
 
         // donate to create fees
         donateRouter.donate(key, 20e18, 20e18, ZERO_BYTES);
@@ -230,14 +237,16 @@ contract GasTest is Test, Deployers, GasSnapshot {
     }
 
     function test_gas_decreaseLiquidity_erc20() public {
-        (uint256 tokenId,) = lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         lpm.decreaseLiquidity(tokenId, 10_000 ether, ZERO_BYTES, false);
         snapLastCall("decreaseLiquidity_erc20");
     }
 
     function test_gas_decreaseLiquidity_erc6909() public {
-        (uint256 tokenId,) = lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        lpm.mint(range, 10_000 ether, block.timestamp + 1, address(this), ZERO_BYTES);
+        uint256 tokenId = lpm.nextTokenId() - 1;
 
         lpm.decreaseLiquidity(tokenId, 10_000 ether, ZERO_BYTES, true);
         snapLastCall("decreaseLiquidity_erc6909");