diff --git a/.forge-snapshots/BinFungibleTokenTest#testBatchTransferFrom_FromBob.snap b/.forge-snapshots/BinFungibleTokenTest#testBatchTransferFrom_FromBob.snap index d99eabc..0ef3935 100644 --- a/.forge-snapshots/BinFungibleTokenTest#testBatchTransferFrom_FromBob.snap +++ b/.forge-snapshots/BinFungibleTokenTest#testBatchTransferFrom_FromBob.snap @@ -1 +1 @@ -54579 \ No newline at end of file +54723 \ No newline at end of file diff --git a/.forge-snapshots/BinFungibleTokenTest#testBurn.snap b/.forge-snapshots/BinFungibleTokenTest#testBurn.snap index 9ba56dd..d7585d6 100644 --- a/.forge-snapshots/BinFungibleTokenTest#testBurn.snap +++ b/.forge-snapshots/BinFungibleTokenTest#testBurn.snap @@ -1 +1 @@ -26887 \ No newline at end of file +26896 \ No newline at end of file diff --git a/.forge-snapshots/BinFungibleTokenTest#testMint.snap b/.forge-snapshots/BinFungibleTokenTest#testMint.snap index 9b41b12..4b59870 100644 --- a/.forge-snapshots/BinFungibleTokenTest#testMint.snap +++ b/.forge-snapshots/BinFungibleTokenTest#testMint.snap @@ -1 +1 @@ -67685 \ No newline at end of file +67373 \ No newline at end of file diff --git a/.forge-snapshots/NonfungiblePositionManager#collect.snap b/.forge-snapshots/NonfungiblePositionManager#collect.snap index 091b6c2..e2d4db9 100644 --- a/.forge-snapshots/NonfungiblePositionManager#collect.snap +++ b/.forge-snapshots/NonfungiblePositionManager#collect.snap @@ -1 +1 @@ -265629 \ No newline at end of file +265663 \ No newline at end of file diff --git a/.forge-snapshots/NonfungiblePositionManager#decreaseLiquidity.snap b/.forge-snapshots/NonfungiblePositionManager#decreaseLiquidity.snap index e30fc66..5e420ab 100644 --- a/.forge-snapshots/NonfungiblePositionManager#decreaseLiquidity.snap +++ b/.forge-snapshots/NonfungiblePositionManager#decreaseLiquidity.snap @@ -1 +1 @@ -176568 \ No newline at end of file +161440 \ No newline at end of file diff --git a/.forge-snapshots/NonfungiblePositionManager#increaseLiquidity.snap b/.forge-snapshots/NonfungiblePositionManager#increaseLiquidity.snap index 036527e..11769d0 100644 --- a/.forge-snapshots/NonfungiblePositionManager#increaseLiquidity.snap +++ b/.forge-snapshots/NonfungiblePositionManager#increaseLiquidity.snap @@ -1 +1 @@ -210284 \ No newline at end of file +195159 \ No newline at end of file diff --git a/.forge-snapshots/NonfungiblePositionManager#mint.snap b/.forge-snapshots/NonfungiblePositionManager#mint.snap index 8fdf5cf..c427d27 100644 --- a/.forge-snapshots/NonfungiblePositionManager#mint.snap +++ b/.forge-snapshots/NonfungiblePositionManager#mint.snap @@ -1 +1 @@ -609324 \ No newline at end of file +607379 \ No newline at end of file diff --git a/src/pool-cl/NonfungiblePositionManager.sol b/src/pool-cl/NonfungiblePositionManager.sol index f1a5c58..ba3406f 100644 --- a/src/pool-cl/NonfungiblePositionManager.sol +++ b/src/pool-cl/NonfungiblePositionManager.sol @@ -396,7 +396,7 @@ contract NonfungiblePositionManager is uint128 tokensOwed1 = nftPositionCache.tokensOwed1; if (nftPositionCache.liquidity > 0) { - resetAccumulatedFee(poolKey, nftPositionCache.tickLower, nftPositionCache.tickUpper); + mintAccumulatedPositionFee(poolKey, nftPositionCache.tickLower, nftPositionCache.tickUpper); // todo: about the salt CLPosition.Info memory poolManagerPositionInfo = poolManager.getPosition( diff --git a/src/pool-cl/base/LiquidityManagement.sol b/src/pool-cl/base/LiquidityManagement.sol index 086e504..a1c8223 100644 --- a/src/pool-cl/base/LiquidityManagement.sol +++ b/src/pool-cl/base/LiquidityManagement.sol @@ -44,32 +44,34 @@ abstract contract LiquidityManagement is CLPeripheryImmutableState, PeripheryPay uint256 amount1Min; } - /// @dev Since in v4 `modifyLiquidity` accumulated fee are claimed and - // resynced by default, which can mixup with user's actual settlement - // for update liquidity, we claim the fee before further action to avoid this. - function resetAccumulatedFee(PoolKey memory poolKey, int24 tickLower, int24 tickUpper) internal { + /// @notice Claim accumulated fees from the position and mint them to the NFP contract + function mintAccumulatedPositionFee(PoolKey memory poolKey, int24 tickLower, int24 tickUpper) internal { CLPosition.Info memory poolManagerPositionInfo = poolManager.getPosition(poolKey.toId(), address(this), tickLower, tickUpper, SALT_0); if (poolManagerPositionInfo.liquidity > 0) { - // todo: can we avoid this resetAccumulatedFee call? (, BalanceDelta feeDelta) = poolManager.modifyLiquidity( poolKey, ICLPoolManager.ModifyLiquidityParams(tickLower, tickUpper, 0, SALT_0), "" ); - if (feeDelta.amount0() > 0) { - vault.mint(address(this), poolKey.currency0, uint256(int256(feeDelta.amount0()))); - } + mintFeeDelta(poolKey, feeDelta); + } + } - if (feeDelta.amount1() > 0) { - vault.mint(address(this), poolKey.currency1, uint256(int256(feeDelta.amount1()))); - } + /// @dev Mint accumulated fee to the contract so user can perform collect() at a later stage + function mintFeeDelta(PoolKey memory poolKey, BalanceDelta feeDelta) internal { + if (feeDelta.amount0() > 0) { + vault.mint(address(this), poolKey.currency0, uint256(int256(feeDelta.amount0()))); + } + + if (feeDelta.amount1() > 0) { + vault.mint(address(this), poolKey.currency1, uint256(int256(feeDelta.amount1()))); } } + /// @return liquidity The amount of liquidity added to the position + /// @return delta The amount of token0 and token1 from liquidity additional. Does not include the fee accumulated in the position. function addLiquidity(AddLiquidityParams memory params) internal returns (uint128 liquidity, BalanceDelta delta) { - resetAccumulatedFee(params.poolKey, params.tickLower, params.tickUpper); - (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(params.poolKey.toId()); uint160 sqrtRatioAX96 = TickMath.getSqrtRatioAtTick(params.tickLower); uint160 sqrtRatioBX96 = TickMath.getSqrtRatioAtTick(params.tickUpper); @@ -77,12 +79,17 @@ abstract contract LiquidityManagement is CLPeripheryImmutableState, PeripheryPay sqrtPriceX96, sqrtRatioAX96, sqrtRatioBX96, params.amount0Desired, params.amount1Desired ); - (delta,) = poolManager.modifyLiquidity( + BalanceDelta feeDelta; + (delta, feeDelta) = poolManager.modifyLiquidity( params.poolKey, ICLPoolManager.ModifyLiquidityParams(params.tickLower, params.tickUpper, int256(uint256(liquidity)), SALT_0), "" ); + /// @dev `delta` return value of modifyLiquidity is inclusive of fee. Mint the `feeDelta` to nfp contract so subtract from `delta` + delta = delta - feeDelta; + mintFeeDelta(params.poolKey, feeDelta); + /// @dev amount0 & amount1 cant be positive here since LPing has been claimed if ( uint256(uint128(-delta.amount0())) < params.amount0Min @@ -92,10 +99,10 @@ abstract contract LiquidityManagement is CLPeripheryImmutableState, PeripheryPay } } + /// @return delta The amount of token0 and token1 from liquidity removal. Does not include the fee accumulated in the position. function removeLiquidity(RemoveLiquidityParams memory params) internal returns (BalanceDelta delta) { - resetAccumulatedFee(params.poolKey, params.tickLower, params.tickUpper); - - (delta,) = poolManager.modifyLiquidity( + BalanceDelta feeDelta; + (delta, feeDelta) = poolManager.modifyLiquidity( params.poolKey, ICLPoolManager.ModifyLiquidityParams( params.tickLower, params.tickUpper, -int256(uint256(params.liquidity)), SALT_0 @@ -103,6 +110,10 @@ abstract contract LiquidityManagement is CLPeripheryImmutableState, PeripheryPay "" ); + /// @dev `delta` return value of modifyLiquidity is inclusive of fee. Mint the `feeDelta` to nfp contract so subtract from `delta` + delta = delta - feeDelta; + mintFeeDelta(params.poolKey, feeDelta); + /// @dev amount0 & amount1 must be positive here since LPing has been claimed if ( uint256(uint128(delta.amount0())) < params.amount0Min