diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap index e8b620bd..2d491c86 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees.snap @@ -1 +1 @@ -262398 \ No newline at end of file +262492 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap index e46fce64..3f663c44 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap @@ -1 +1 @@ -194771 \ No newline at end of file +194865 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap index 853c5353..7da1ea95 100644 --- a/.forge-snapshots/autocompound_excessFeesCredit.snap +++ b/.forge-snapshots/autocompound_excessFeesCredit.snap @@ -1 +1 @@ -282937 \ No newline at end of file +283031 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap index 0808a085..9d65fe6b 100644 --- a/.forge-snapshots/decreaseLiquidity_erc20.snap +++ b/.forge-snapshots/decreaseLiquidity_erc20.snap @@ -1 +1 @@ -193172 \ No newline at end of file +180845 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap index df7eb4c5..f729ab62 100644 --- a/.forge-snapshots/decreaseLiquidity_erc6909.snap +++ b/.forge-snapshots/decreaseLiquidity_erc6909.snap @@ -1 +1 @@ -172032 \ No newline at end of file +180857 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap index 1f74a831..0c7dfe6a 100644 --- a/.forge-snapshots/increaseLiquidity_erc20.snap +++ b/.forge-snapshots/increaseLiquidity_erc20.snap @@ -1 +1 @@ -175155 \ No newline at end of file +175249 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap index 0f37872d..af512087 100644 --- a/.forge-snapshots/increaseLiquidity_erc6909.snap +++ b/.forge-snapshots/increaseLiquidity_erc6909.snap @@ -1 +1 @@ -150744 \ No newline at end of file +150838 \ No newline at end of file diff --git a/.forge-snapshots/mintWithLiquidity.snap b/.forge-snapshots/mintWithLiquidity.snap index ac89220c..46d7cd3c 100644 --- a/.forge-snapshots/mintWithLiquidity.snap +++ b/.forge-snapshots/mintWithLiquidity.snap @@ -1 +1 @@ -600083 \ No newline at end of file +600177 \ No newline at end of file diff --git a/contracts/NonfungiblePositionManager.sol b/contracts/NonfungiblePositionManager.sol index c34028d3..d78726a6 100644 --- a/contracts/NonfungiblePositionManager.sol +++ b/contracts/NonfungiblePositionManager.sol @@ -132,16 +132,16 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims) public isAuthorizedForToken(tokenId) - returns (BalanceDelta delta) + returns (BalanceDelta delta, BalanceDelta thisDelta) { TokenPosition memory tokenPos = tokenPositions[tokenId]; if (manager.isUnlocked()) { - delta = _decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData); + (delta, thisDelta) = _decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData); _closeCallerDeltas( delta, tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1, tokenPos.owner, claims ); - _closeAllDeltas(tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1); + _closeThisDeltas(thisDelta, tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1); } else { bytes[] memory data = new bytes[](1); data[0] = abi.encodeWithSelector(this.decreaseLiquidity.selector, tokenId, liquidity, hookData, claims); @@ -154,13 +154,17 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit isAuthorizedForToken(tokenId) returns (BalanceDelta delta) { + // TODO: Burn currently decreases and collects. However its done under different locks. + // Replace once we have the execute multicall. // remove liquidity TokenPosition storage tokenPosition = tokenPositions[tokenId]; LiquidityRangeId rangeId = tokenPosition.range.toId(); Position storage position = positions[msg.sender][rangeId]; - if (0 < position.liquidity) { - delta = decreaseLiquidity(tokenId, position.liquidity, hookData, claims); + if (position.liquidity > 0) { + (delta,) = decreaseLiquidity(tokenId, position.liquidity, hookData, claims); } + + collect(tokenId, recipient, hookData, claims); require(position.tokensOwed0 == 0 && position.tokensOwed1 == 0, "NOT_EMPTY"); delete positions[msg.sender][rangeId]; delete tokenPositions[tokenId]; @@ -171,16 +175,17 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit // TODO: in v3, we can partially collect fees, but what was the usecase here? function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims) - external + public returns (BalanceDelta delta) { TokenPosition memory tokenPos = tokenPositions[tokenId]; if (manager.isUnlocked()) { - delta = _collect(tokenPos.owner, tokenPos.range, hookData); + BalanceDelta thisDelta; + (delta, thisDelta) = _collect(tokenPos.owner, tokenPos.range, hookData); _closeCallerDeltas( delta, tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1, tokenPos.owner, claims ); - _closeAllDeltas(tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1); + _closeThisDeltas(thisDelta, tokenPos.range.poolKey.currency0, tokenPos.range.poolKey.currency1); } else { bytes[] memory data = new bytes[](1); data[0] = abi.encodeWithSelector(this.collect.selector, tokenId, recipient, hookData, claims); diff --git a/contracts/base/BaseLiquidityManagement.sol b/contracts/base/BaseLiquidityManagement.sol index a6af4ed3..63d325de 100644 --- a/contracts/base/BaseLiquidityManagement.sol +++ b/contracts/base/BaseLiquidityManagement.sol @@ -16,7 +16,6 @@ import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; import {CurrencySettleTake} from "../libraries/CurrencySettleTake.sol"; -import {CurrencySenderLibrary} from "../libraries/CurrencySenderLibrary.sol"; import {CurrencyDeltas} from "../libraries/CurrencyDeltas.sol"; import {FeeMath} from "../libraries/FeeMath.sol"; @@ -24,6 +23,7 @@ import {LiquiditySaltLibrary} from "../libraries/LiquiditySaltLibrary.sol"; import {IBaseLiquidityManagement} from "../interfaces/IBaseLiquidityManagement.sol"; import {PositionLibrary} from "../libraries/Position.sol"; import {BalanceDeltaExtensionLibrary} from "../libraries/BalanceDeltaExtensionLibrary.sol"; +import {LiquidityDeltaAccounting} from "../libraries/LiquidityDeltaAccounting.sol"; import "forge-std/console2.sol"; @@ -31,7 +31,6 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb using LiquidityRangeIdLibrary for LiquidityRange; using CurrencyLibrary for Currency; using CurrencySettleTake for Currency; - using CurrencySenderLibrary for Currency; using CurrencyDeltas for IPoolManager; using PoolIdLibrary for PoolKey; using StateLibrary for IPoolManager; @@ -40,6 +39,7 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb using LiquiditySaltLibrary for IHooks; using PositionLibrary for IBaseLiquidityManagement.Position; using BalanceDeltaExtensionLibrary for BalanceDelta; + using LiquidityDeltaAccounting for BalanceDelta; mapping(address owner => mapping(LiquidityRangeId rangeId => Position)) public positions; @@ -58,10 +58,10 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb // We always 0 out a caller positive delta because it is instead accounted for in position.tokensOwed. if (callerDelta0 < 0) currency0.settle(manager, owner, uint256(int256(-callerDelta0)), claims); - else if (callerDelta0 > 0) currency0.send(manager, owner, uint128(callerDelta0), claims); + else if (callerDelta0 > 0) currency0.take(manager, owner, uint128(callerDelta0), claims); if (callerDelta1 < 0) currency1.settle(manager, owner, uint256(int256(-callerDelta1)), claims); - else if (callerDelta1 > 0) currency1.send(manager, owner, uint128(callerDelta1), claims); + else if (callerDelta1 > 0) currency1.take(manager, owner, uint128(callerDelta1), claims); } function _modifyLiquidity(address owner, LiquidityRange memory range, int256 liquidityChange, bytes memory hookData) @@ -94,33 +94,13 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb Position storage position = positions[owner][range.toId()]; + // Calculates the fee growth since the last time the positions feeGrowthInside was updated. + // Also updates the feeGrowthInsideLast variables in storage. + (BalanceDelta callerFeesAccrued) = _updateFeeGrowth(range, position); + // Calculate the portion of the liquidityDelta that is attributable to the caller. // We must account for fees that might be owed to other users on the same range. - (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) = - manager.getFeeGrowthInside(range.poolKey.toId(), range.tickLower, range.tickUpper); - - BalanceDelta callerFeesAccrued = FeeMath.getFeesOwed( - feeGrowthInside0X128, - feeGrowthInside1X128, - position.feeGrowthInside0LastX128, - position.feeGrowthInside1LastX128, - position.liquidity - ); - - if (totalFeesAccrued == callerFeesAccrued) { - // when totalFeesAccrued == callerFeesAccrued, the caller is not sharing the range - // therefore, the caller is responsible for the entire liquidityDelta - callerDelta = liquidityDelta; - } else { - // the delta for increasing liquidity assuming that totalFeesAccrued was not applied - BalanceDelta principalDelta = liquidityDelta - totalFeesAccrued; - - // outstanding deltas the caller is responsible for, after their fees are credited to the principal delta - callerDelta = principalDelta + callerFeesAccrued; - - // outstanding deltas this contract is responsible for, intuitively the contract is responsible for taking fees external to the caller's accrued fees - thisDelta = totalFeesAccrued - callerFeesAccrued; - } + (callerDelta, thisDelta) = liquidityDelta.split(callerFeesAccrued, totalFeesAccrued); // Update position storage, flushing the callerDelta value to tokensOwed first if necessary. // If callerDelta > 0, then even after investing callerFeesAccrued, the caller still has some amount to collect that were not added into the position so they are accounted to tokensOwed and removed from the final callerDelta returned. @@ -137,7 +117,6 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb position.addTokensOwed(tokensOwed); position.addLiquidity(liquidityToAdd); - position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128); } // When chaining many actions, this should be called at the very end to close out any open deltas owed to or by this contract for other users on the same range. @@ -154,20 +133,6 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb if (delta1 < 0) currency1.settle(manager, address(this), uint256(int256(-delta1)), true); } - //TODO @sara deprecate when moving to _closeThisDeltas for decreaes and collect - function _closeAllDeltas(Currency currency0, Currency currency1) internal { - (BalanceDelta delta) = manager.currencyDeltas(address(this), currency0, currency1); - int128 delta0 = delta.amount0(); - int128 delta1 = delta.amount1(); - - // Mint a receipt for the tokens owed to this address. - if (delta0 > 0) currency0.take(manager, address(this), uint128(delta0), true); - if (delta1 > 0) currency1.take(manager, address(this), uint128(delta1), true); - // Burn the receipt for tokens owed to this address. - if (delta0 < 0) currency0.settle(manager, address(this), uint256(int256(-delta0)), true); - if (delta1 < 0) currency1.settle(manager, address(this), uint256(int256(-delta1)), true); - } - function _moveCallerDeltaToTokensOwed( bool useAmount0, BalanceDelta tokensOwed, @@ -187,82 +152,82 @@ 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, LiquidityRange memory range, uint256 liquidityToRemove, bytes memory hookData - ) internal returns (BalanceDelta delta) { + ) internal returns (BalanceDelta callerDelta, BalanceDelta thisDelta) { (BalanceDelta liquidityDelta, BalanceDelta totalFeesAccrued) = _modifyLiquidity(owner, range, -(liquidityToRemove.toInt256()), hookData); - // take all tokens first - // do NOT take tokens directly to the owner because this contract might be holding fees - // that need to be paid out (position.tokensOwed) - if (liquidityDelta.amount0() > 0) { - range.poolKey.currency0.take(manager, address(this), uint128(liquidityDelta.amount0()), true); - } - if (liquidityDelta.amount1() > 0) { - range.poolKey.currency1.take(manager, address(this), uint128(liquidityDelta.amount1()), true); - } + Position storage position = positions[owner][range.toId()]; - // when decreasing liquidity, the user collects: 1) principal liquidity, 2) new fees, 3) old fees (position.tokensOwed) + // Calculates the fee growth since the last time the positions feeGrowthInside was updated + // Also updates the position's the feeGrowthInsideLast variables in storage. + (BalanceDelta callerFeesAccrued) = _updateFeeGrowth(range, position); - Position storage position = positions[owner][range.toId()]; - BalanceDelta callerFeesAccrued = _updateFeeGrowth(range, position); - BalanceDelta principalDelta = liquidityDelta - totalFeesAccrued; + // Account for fees accrued to other users on the same range. + (callerDelta, thisDelta) = liquidityDelta.split(callerFeesAccrued, totalFeesAccrued); + + BalanceDelta tokensOwed; - // new fees = new fees + old fees + principal liquidity - callerFeesAccrued = callerFeesAccrued - + toBalanceDelta(uint256(position.tokensOwed0).toInt128(), uint256(position.tokensOwed1).toInt128()) - + principalDelta; + // Flush the callerDelta, incrementing the tokensOwed to the user and the amount claimable to this contract. + if (callerDelta.amount0() > 0) { + (tokensOwed, callerDelta, thisDelta) = + _moveCallerDeltaToTokensOwed(true, tokensOwed, callerDelta, thisDelta); + } - position.tokensOwed0 = 0; - position.tokensOwed1 = 0; - position.liquidity -= liquidityToRemove; + if (callerDelta.amount1() > 0) { + (tokensOwed, callerDelta, thisDelta) = + _moveCallerDeltaToTokensOwed(false, tokensOwed, callerDelta, thisDelta); + } - return callerFeesAccrued; + position.addTokensOwed(tokensOwed); + position.subtractLiquidity(liquidityToRemove); } function _collect(address owner, LiquidityRange memory range, bytes memory hookData) internal - returns (BalanceDelta) + returns (BalanceDelta callerDelta, BalanceDelta thisDelta) { - (, BalanceDelta totalFeesAccrued) = _modifyLiquidity(owner, range, 0, hookData); - - PoolKey memory key = range.poolKey; Position storage position = positions[owner][range.toId()]; - // take all fees first then distribute - if (totalFeesAccrued.amount0() > 0) { - key.currency0.take(manager, address(this), uint128(totalFeesAccrued.amount0()), true); - } - if (totalFeesAccrued.amount1() > 0) { - key.currency1.take(manager, address(this), uint128(totalFeesAccrued.amount1()), true); - } + // Only call modify if there is still liquidty in this position. + if (position.liquidity != 0) { + // Do not add or decrease liquidity, just trigger fee updates. + (BalanceDelta liquidityDelta, BalanceDelta totalFeesAccrued) = _modifyLiquidity(owner, range, 0, hookData); - // collecting fees: new fees and old fees - BalanceDelta callerFeesAccrued = _updateFeeGrowth(range, position); - callerFeesAccrued = callerFeesAccrued - + toBalanceDelta(uint256(position.tokensOwed0).toInt128(), uint256(position.tokensOwed1).toInt128()); + // Also updates the position's the feeGrowthInsideLast variables in storage. + (BalanceDelta callerFeesAccrued) = _updateFeeGrowth(range, position); - position.tokensOwed0 = 0; - position.tokensOwed1 = 0; + // Account for fees accrued to other users on the same range. + // TODO: Opt when liquidityDelta == 0 + (callerDelta, thisDelta) = liquidityDelta.split(callerFeesAccrued, totalFeesAccrued); + } + + // Allow the caller to collect the tokens owed. + // Tokens owed that the caller collects is paid for by this contract. + // ie. Transfer the tokensOwed amounts to the caller from the position manager through the pool manager. + // TODO case where this contract does not have enough credits to pay the caller? + BalanceDelta tokensOwed = + toBalanceDelta(uint256(position.tokensOwed0).toInt128(), uint256(position.tokensOwed1).toInt128()); + callerDelta = callerDelta + tokensOwed; + thisDelta = thisDelta - tokensOwed; - return callerFeesAccrued; + position.clearTokensOwed(); } - // TODO: I deprecated this bc I liked to see the accounting in line in the top level function... and I like to do all the position updates at once. - // can keep but should at at least use the position library in here. function _updateFeeGrowth(LiquidityRange memory range, Position storage position) internal - returns (BalanceDelta _feesOwed) + returns (BalanceDelta callerFeesAccrued) { (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) = manager.getFeeGrowthInside(range.poolKey.toId(), range.tickLower, range.tickUpper); - _feesOwed = FeeMath.getFeesOwed( + callerFeesAccrued = FeeMath.getFeesOwed( feeGrowthInside0X128, feeGrowthInside1X128, position.feeGrowthInside0LastX128, @@ -270,8 +235,7 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb position.liquidity ); - position.feeGrowthInside0LastX128 = feeGrowthInside0X128; - position.feeGrowthInside1LastX128 = feeGrowthInside1X128; + position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128); } // --- View Functions --- // diff --git a/contracts/interfaces/INonfungiblePositionManager.sol b/contracts/interfaces/INonfungiblePositionManager.sol index 17c2fc45..b937f10b 100644 --- a/contracts/interfaces/INonfungiblePositionManager.sol +++ b/contracts/interfaces/INonfungiblePositionManager.sol @@ -37,10 +37,11 @@ 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 + /// @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 function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims) external - returns (BalanceDelta delta); + returns (BalanceDelta delta, BalanceDelta thisDelta); /// @notice Burn a position and delete the tokenId /// @dev It removes liquidity and collects fees if the position is not empty diff --git a/contracts/libraries/CurrencySenderLibrary.sol b/contracts/libraries/CurrencySenderLibrary.sol deleted file mode 100644 index 656a9439..00000000 --- a/contracts/libraries/CurrencySenderLibrary.sol +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; - -import {Currency, CurrencyLibrary} from "v4-core/types/Currency.sol"; -import {CurrencySettleTake} from "./CurrencySettleTake.sol"; -import {IPoolManager} from "v4-core/interfaces/IPoolManager.sol"; - -/// @notice Library used to send Currencies from address to address -library CurrencySenderLibrary { - using CurrencyLibrary for Currency; - using CurrencySettleTake for Currency; - - /// @notice Send a custodied Currency to a recipient - /// @dev If sending ERC20 or native, the PoolManager must be unlocked - /// @param currency The Currency to send - /// @param manager The PoolManager - /// @param recipient The recipient address - /// @param amount The amount to send - /// @param useClaims If true, transfer ERC-6909 tokens - function send(Currency currency, IPoolManager manager, address recipient, uint256 amount, bool useClaims) - internal - { - if (useClaims) { - manager.transfer(recipient, currency.toId(), amount); - } else { - // currency.settle(manager, address(this), amount, true); // sends in tokens into PM from this address - currency.take(manager, recipient, amount, false); // takes out tokens from PM to recipient - } - } -} diff --git a/contracts/libraries/LiquidityDeltaAccounting.sol b/contracts/libraries/LiquidityDeltaAccounting.sol new file mode 100644 index 00000000..b6c99b10 --- /dev/null +++ b/contracts/libraries/LiquidityDeltaAccounting.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; + +import "forge-std/console2.sol"; + +library LiquidityDeltaAccounting { + function split(BalanceDelta liquidityDelta, BalanceDelta callerFeesAccrued, BalanceDelta totalFeesAccrued) + internal + returns (BalanceDelta callerDelta, BalanceDelta thisDelta) + { + if (totalFeesAccrued == callerFeesAccrued) { + // when totalFeesAccrued == callerFeesAccrued, the caller is not sharing the range + // therefore, the caller is responsible for the entire liquidityDelta + callerDelta = liquidityDelta; + } else { + // the delta for increasing liquidity assuming that totalFeesAccrued was not applied + BalanceDelta principalDelta = liquidityDelta - totalFeesAccrued; + + // outstanding deltas the caller is responsible for, after their fees are credited to the principal delta + callerDelta = principalDelta + callerFeesAccrued; + + // outstanding deltas this contract is responsible for, intuitively the contract is responsible for taking fees external to the caller's accrued fees + thisDelta = totalFeesAccrued - callerFeesAccrued; + } + } +} diff --git a/contracts/libraries/Position.sol b/contracts/libraries/Position.sol index 79cd02c0..11ef1771 100644 --- a/contracts/libraries/Position.sol +++ b/contracts/libraries/Position.sol @@ -6,18 +6,32 @@ import {BalanceDelta} from "v4-core/types/BalanceDelta.sol"; // Updates Position storage library PositionLibrary { + error InsufficientLiquidity(); + // TODO ensure this is one sstore. function addTokensOwed(IBaseLiquidityManagement.Position storage position, BalanceDelta tokensOwed) internal { position.tokensOwed0 += uint128(tokensOwed.amount0()); position.tokensOwed1 += uint128(tokensOwed.amount1()); } + function clearTokensOwed(IBaseLiquidityManagement.Position storage position) internal { + position.tokensOwed0 = 0; + position.tokensOwed1 = 0; + } + function addLiquidity(IBaseLiquidityManagement.Position storage position, uint256 liquidity) internal { unchecked { position.liquidity += liquidity; } } + function subtractLiquidity(IBaseLiquidityManagement.Position storage position, uint256 liquidity) internal { + if (position.liquidity < liquidity) revert InsufficientLiquidity(); + unchecked { + position.liquidity -= liquidity; + } + } + // TODO ensure this is one sstore. function updateFeeGrowthInside( IBaseLiquidityManagement.Position storage position, diff --git a/test/position-managers/FeeCollection.t.sol b/test/position-managers/FeeCollection.t.sol index 643f6303..e89ff68a 100644 --- a/test/position-managers/FeeCollection.t.sol +++ b/test/position-managers/FeeCollection.t.sol @@ -216,49 +216,10 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers { function test_collect_donate() public {} function test_collect_donate_sameRange() public {} - function test_decreaseLiquidity_sameRange( - 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 - - liquidityDeltaBob = bound(liquidityDeltaBob, 100e18, 100_000e18); - - 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); - - vm.prank(bob); - (tokenIdBob,) = lpm.mint(range, liquidityDeltaBob, block.timestamp + 1, bob, ZERO_BYTES); - - // swap to create fees - uint256 swapAmount = 0.001e18; - swap(key, true, -int256(swapAmount), ZERO_BYTES); - - // alice removes all of her liquidity - vm.prank(alice); - BalanceDelta aliceDelta = lpm.decreaseLiquidity(tokenIdAlice, uint256(params.liquidityDelta), ZERO_BYTES, true); - assertEq(uint256(uint128(aliceDelta.amount0())), manager.balanceOf(alice, currency0.toId())); - assertEq(uint256(uint128(aliceDelta.amount1())), manager.balanceOf(alice, currency1.toId())); - - // bob removes half of his liquidity - vm.prank(bob); - BalanceDelta bobDelta = lpm.decreaseLiquidity(tokenIdBob, liquidityDeltaBob / 2, ZERO_BYTES, true); - assertEq(uint256(uint128(bobDelta.amount0())), manager.balanceOf(bob, currency0.toId())); - assertEq(uint256(uint128(bobDelta.amount1())), manager.balanceOf(bob, currency1.toId())); - - // position manager holds no fees now - assertApproxEqAbs(manager.balanceOf(address(lpm), currency0.toId()), 0, 1 wei); - assertApproxEqAbs(manager.balanceOf(address(lpm), currency1.toId()), 0, 1 wei); - } - /// @dev Alice and bob create liquidity on the same range /// when alice decreases liquidity, she should only collect her fees + /// TODO Add back fuzz test on liquidityDeltaBob + /// TODO Assert state changes for lpm balance, position state, and return values function test_decreaseLiquidity_sameRange_exact() public { // alice and bob create liquidity on the same range [-120, 120] LiquidityRange memory range = LiquidityRange({poolKey: key, tickLower: -120, tickUpper: 120}); @@ -281,39 +242,38 @@ contract FeeCollectionTest is Test, Deployers, GasSnapshot, LiquidityFuzzers { // alice decreases liquidity vm.prank(alice); - BalanceDelta aliceDelta = lpm.decreaseLiquidity(tokenIdAlice, liquidityAlice, ZERO_BYTES, true); + BalanceDelta aliceDelta; + BalanceDelta thisDelta; + (aliceDelta, thisDelta) = lpm.decreaseLiquidity(tokenIdAlice, liquidityAlice, ZERO_BYTES, true); uint256 tolerance = 0.000000001 ether; - // alice claims original principal + her fees + uint256 lpmBalance0 = manager.balanceOf(address(lpm), currency0.toId()); + uint256 lpmBalance1 = manager.balanceOf(address(lpm), currency1.toId()); + + // lpm collects alice's principal + all fees accrued on the range assertApproxEqAbs( - manager.balanceOf(alice, currency0.toId()), - uint256(int256(-lpDeltaAlice.amount0())) - + swapAmount.mulWadDown(FEE_WAD).mulDivDown(liquidityAlice, liquidityAlice + liquidityBob), - tolerance + lpmBalance0, uint256(int256(-lpDeltaAlice.amount0())) + swapAmount.mulWadDown(FEE_WAD), tolerance ); assertApproxEqAbs( - manager.balanceOf(alice, currency1.toId()), - uint256(int256(-lpDeltaAlice.amount1())) - + swapAmount.mulWadDown(FEE_WAD).mulDivDown(liquidityAlice, liquidityAlice + liquidityBob), - tolerance + lpmBalance1, uint256(int256(-lpDeltaAlice.amount1())) + swapAmount.mulWadDown(FEE_WAD), tolerance ); // bob decreases half of his liquidity vm.prank(bob); - BalanceDelta bobDelta = lpm.decreaseLiquidity(tokenIdBob, liquidityBob / 2, ZERO_BYTES, true); + BalanceDelta bobDelta; + (bobDelta, thisDelta) = lpm.decreaseLiquidity(tokenIdBob, liquidityBob / 2, ZERO_BYTES, true); - // bob claims half of the original principal + his fees + // lpm collects half of bobs principal + // the fee amount has already been collected with alice's calls assertApproxEqAbs( - manager.balanceOf(bob, currency0.toId()), - uint256(int256(-lpDeltaBob.amount0()) / 2) - + swapAmount.mulWadDown(FEE_WAD).mulDivDown(liquidityBob, liquidityAlice + liquidityBob), + manager.balanceOf(address(lpm), currency0.toId()) - lpmBalance0, + uint256(int256(-lpDeltaBob.amount0()) / 2), tolerance ); assertApproxEqAbs( - manager.balanceOf(bob, currency1.toId()), - uint256(int256(-lpDeltaBob.amount1()) / 2) - + swapAmount.mulWadDown(FEE_WAD).mulDivDown(liquidityBob, liquidityAlice + liquidityBob), + manager.balanceOf(address(lpm), currency1.toId()) - lpmBalance1, + uint256(int256(-lpDeltaBob.amount1()) / 2), tolerance ); } diff --git a/test/position-managers/NonfungiblePositionManager.t.sol b/test/position-managers/NonfungiblePositionManager.t.sol index c1cad0c1..3d59572b 100644 --- a/test/position-managers/NonfungiblePositionManager.t.sol +++ b/test/position-managers/NonfungiblePositionManager.t.sol @@ -247,7 +247,8 @@ contract NonfungiblePositionManagerTest is Test, Deployers, GasSnapshot, Liquidi uint256 balance0Before = currency0.balanceOfSelf(); uint256 balance1Before = currency1.balanceOfSelf(); - BalanceDelta delta = lpm.decreaseLiquidity(tokenId, decreaseLiquidityDelta, ZERO_BYTES, false); + (BalanceDelta delta, BalanceDelta thisDelta) = + lpm.decreaseLiquidity(tokenId, decreaseLiquidityDelta, ZERO_BYTES, false); (,, uint256 liquidity,,,,) = lpm.positions(address(this), range.toId()); assertEq(liquidity, uint256(params.liquidityDelta) - decreaseLiquidityDelta);