diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees.snap index 40ad7ac8..8e881fb8 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees.snap @@ -1 +1 @@ -258477 \ No newline at end of file +258575 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap index e2e7eb05..f44837b7 100644 --- a/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap +++ b/.forge-snapshots/autocompound_exactUnclaimedFees_exactCustodiedFees.snap @@ -1 +1 @@ -190850 \ No newline at end of file +190948 \ No newline at end of file diff --git a/.forge-snapshots/autocompound_excessFeesCredit.snap b/.forge-snapshots/autocompound_excessFeesCredit.snap index bcf9757d..81d04dab 100644 --- a/.forge-snapshots/autocompound_excessFeesCredit.snap +++ b/.forge-snapshots/autocompound_excessFeesCredit.snap @@ -1 +1 @@ -279016 \ No newline at end of file +279114 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc20.snap b/.forge-snapshots/decreaseLiquidity_erc20.snap index ae013013..461e5928 100644 --- a/.forge-snapshots/decreaseLiquidity_erc20.snap +++ b/.forge-snapshots/decreaseLiquidity_erc20.snap @@ -1 +1 @@ -190026 \ No newline at end of file +177014 \ No newline at end of file diff --git a/.forge-snapshots/decreaseLiquidity_erc6909.snap b/.forge-snapshots/decreaseLiquidity_erc6909.snap index 4d5e683a..1a5a1ce2 100644 --- a/.forge-snapshots/decreaseLiquidity_erc6909.snap +++ b/.forge-snapshots/decreaseLiquidity_erc6909.snap @@ -1 +1 @@ -168894 \ No newline at end of file +177026 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc20.snap b/.forge-snapshots/increaseLiquidity_erc20.snap index 4ea517e8..786ac121 100644 --- a/.forge-snapshots/increaseLiquidity_erc20.snap +++ b/.forge-snapshots/increaseLiquidity_erc20.snap @@ -1 +1 @@ -171241 \ No newline at end of file +171339 \ No newline at end of file diff --git a/.forge-snapshots/increaseLiquidity_erc6909.snap b/.forge-snapshots/increaseLiquidity_erc6909.snap index c2e421fa..24ec8e92 100644 --- a/.forge-snapshots/increaseLiquidity_erc6909.snap +++ b/.forge-snapshots/increaseLiquidity_erc6909.snap @@ -1 +1 @@ -146823 \ No newline at end of file +146921 \ No newline at end of file diff --git a/.forge-snapshots/mintWithLiquidity.snap b/.forge-snapshots/mintWithLiquidity.snap index d2591995..ee03852e 100644 --- a/.forge-snapshots/mintWithLiquidity.snap +++ b/.forge-snapshots/mintWithLiquidity.snap @@ -1 +1 @@ -466530 \ No newline at end of file +466628 \ No newline at end of file diff --git a/contracts/NonfungiblePositionManager.sol b/contracts/NonfungiblePositionManager.sol index a461d1db..ab1670cd 100644 --- a/contracts/NonfungiblePositionManager.sol +++ b/contracts/NonfungiblePositionManager.sol @@ -86,10 +86,10 @@ 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]; - delta = _lockAndDecreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData, claims); + (delta, thisDelta) = _lockAndDecreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData, claims); } function burn(uint256 tokenId, address recipient, bytes calldata hookData, bool claims) @@ -97,13 +97,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]; @@ -114,7 +118,7 @@ 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]; diff --git a/contracts/base/BaseLiquidityManagement.sol b/contracts/base/BaseLiquidityManagement.sol index bc9ab1da..df54345a 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 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { 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 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { 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 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { // 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 _unlockCallback(bytes calldata data) internal override returns (bytes memory) { @@ -77,7 +77,9 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { if (op == LiquidityOperation.INCREASE) { return abi.encode(_increaseLiquidityAndZeroOut(owner, range, liquidityChange, hookData, claims)); } else if (op == LiquidityOperation.DECREASE) { - return abi.encode(_decreaseLiquidityAndZeroOut(owner, range, liquidityChange, hookData, claims)); + (BalanceDelta callerDelta, BalanceDelta thisDelta) = + _decreaseLiquidityAndZeroOut(owner, range, liquidityChange, hookData, claims); + return abi.encode(callerDelta, thisDelta); } else if (op == LiquidityOperation.COLLECT) { return abi.encode(_collectAndZeroOut(owner, range, 0, hookData, claims)); } else { @@ -115,33 +117,13 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { 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. @@ -158,7 +140,6 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { position.addTokensOwed(tokensOwed); position.addLiquidity(liquidityToAdd); - position.updateFeeGrowthInside(feeGrowthInside0X128, feeGrowthInside1X128); } function _increaseLiquidityAndZeroOut( @@ -189,20 +170,6 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { 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, @@ -236,41 +203,40 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { ); } + /// 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); - // new fees = new fees + old fees + principal liquidity - callerFeesAccrued = callerFeesAccrued - + toBalanceDelta(uint256(position.tokensOwed0).toInt128(), uint256(position.tokensOwed1).toInt128()) - + principalDelta; + BalanceDelta tokensOwed; - position.tokensOwed0 = 0; - position.tokensOwed1 = 0; - position.liquidity -= liquidityToRemove; + // 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); + } - return callerFeesAccrued; + if (callerDelta.amount1() > 0) { + (tokensOwed, callerDelta, thisDelta) = + _moveCallerDeltaToTokensOwed(false, tokensOwed, callerDelta, thisDelta); + } + + position.addTokensOwed(tokensOwed); + position.subtractLiquidity(liquidityToRemove); } function _decreaseLiquidityAndZeroOut( @@ -279,10 +245,10 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { uint256 liquidityToRemove, bytes memory hookData, bool claims - ) internal returns (BalanceDelta delta) { - delta = _decreaseLiquidity(owner, range, liquidityToRemove, hookData); - _closeCallerDeltas(delta, range.poolKey.currency0, range.poolKey.currency1, owner, claims); - _closeAllDeltas(range.poolKey.currency0, range.poolKey.currency1); + ) internal returns (BalanceDelta callerDelta, BalanceDelta thisDelta) { + (callerDelta, thisDelta) = _decreaseLiquidity(owner, range, liquidityToRemove, hookData); + _closeCallerDeltas(callerDelta, range.poolKey.currency0, range.poolKey.currency1, owner, claims); + _closeThisDeltas(thisDelta, range.poolKey.currency0, range.poolKey.currency1); } function _lockAndDecreaseLiquidity( @@ -291,48 +257,52 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { uint256 liquidityToRemove, bytes memory hookData, bool claims - ) internal returns (BalanceDelta) { + ) internal returns (BalanceDelta, BalanceDelta) { return abi.decode( manager.unlock(abi.encode(LiquidityOperation.DECREASE, owner, range, liquidityToRemove, hookData, claims)), - (BalanceDelta) + (BalanceDelta, BalanceDelta) ); } 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); + + // Also updates the position's the feeGrowthInsideLast variables in storage. + (BalanceDelta callerFeesAccrued) = _updateFeeGrowth(range, position); - // collecting fees: new fees and old fees - BalanceDelta callerFeesAccrued = _updateFeeGrowth(range, position); - callerFeesAccrued = callerFeesAccrued - + toBalanceDelta(uint256(position.tokensOwed0).toInt128(), uint256(position.tokensOwed1).toInt128()); + // Account for fees accrued to other users on the same range. + // TODO: Opt when liquidityDelta == 0 + (callerDelta, thisDelta) = liquidityDelta.split(callerFeesAccrued, totalFeesAccrued); + } - position.tokensOwed0 = 0; - position.tokensOwed1 = 0; + // 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(); } function _collectAndZeroOut(address owner, LiquidityRange memory range, uint256, bytes memory hookData, bool claims) internal - returns (BalanceDelta delta) + returns (BalanceDelta callerDelta) { - delta = _collect(owner, range, hookData); - _closeCallerDeltas(delta, range.poolKey.currency0, range.poolKey.currency1, owner, claims); - _closeAllDeltas(range.poolKey.currency0, range.poolKey.currency1); + BalanceDelta thisDelta; + (callerDelta, thisDelta) = _collect(owner, range, hookData); + _closeCallerDeltas(callerDelta, range.poolKey.currency0, range.poolKey.currency1, owner, claims); + _closeThisDeltas(thisDelta, range.poolKey.currency0, range.poolKey.currency1); } function _lockAndCollect(address owner, LiquidityRange memory range, bytes memory hookData, bool claims) @@ -344,16 +314,14 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { ); } - // 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, @@ -361,8 +329,7 @@ contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallback { 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 6b09efe5..af011df5 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);