From 56536feebc91eb734a631cfeecd5695f84240c7e Mon Sep 17 00:00:00 2001 From: Atis Elsts Date: Sat, 9 Dec 2023 18:35:47 +0200 Subject: [PATCH] gas optimizations for the full range hook --- .../FullRangeAddInitialLiquidity.snap | 2 +- .forge-snapshots/FullRangeAddLiquidity.snap | 2 +- .forge-snapshots/FullRangeFirstSwap.snap | 2 +- .forge-snapshots/FullRangeLargeSwap.snap | 1 + .../FullRangeRemoveLiquidity.snap | 2 +- .../FullRangeRemoveLiquidityAndRebalance.snap | 2 +- .forge-snapshots/FullRangeSecondSwap.snap | 2 +- .forge-snapshots/FullRangeSwap.snap | 2 +- README.md | 1 + contracts/hooks/examples/FullRange.sol | 41 +++--- test/FullRange.t.sol | 132 ++++++++++++------ 11 files changed, 122 insertions(+), 67 deletions(-) create mode 100644 .forge-snapshots/FullRangeLargeSwap.snap diff --git a/.forge-snapshots/FullRangeAddInitialLiquidity.snap b/.forge-snapshots/FullRangeAddInitialLiquidity.snap index 2d5250a5..d0412a70 100644 --- a/.forge-snapshots/FullRangeAddInitialLiquidity.snap +++ b/.forge-snapshots/FullRangeAddInitialLiquidity.snap @@ -1 +1 @@ -412696 \ No newline at end of file +411324 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeAddLiquidity.snap b/.forge-snapshots/FullRangeAddLiquidity.snap index 032a6a3b..71b2125d 100644 --- a/.forge-snapshots/FullRangeAddLiquidity.snap +++ b/.forge-snapshots/FullRangeAddLiquidity.snap @@ -1 +1 @@ -206962 \ No newline at end of file +205590 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeFirstSwap.snap b/.forge-snapshots/FullRangeFirstSwap.snap index 9d59ac16..728d6c18 100644 --- a/.forge-snapshots/FullRangeFirstSwap.snap +++ b/.forge-snapshots/FullRangeFirstSwap.snap @@ -1 +1 @@ -154763 \ No newline at end of file +150901 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeLargeSwap.snap b/.forge-snapshots/FullRangeLargeSwap.snap new file mode 100644 index 00000000..f766fccd --- /dev/null +++ b/.forge-snapshots/FullRangeLargeSwap.snap @@ -0,0 +1 @@ +149976 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeRemoveLiquidity.snap b/.forge-snapshots/FullRangeRemoveLiquidity.snap index 920384a4..d51d705c 100644 --- a/.forge-snapshots/FullRangeRemoveLiquidity.snap +++ b/.forge-snapshots/FullRangeRemoveLiquidity.snap @@ -1 +1 @@ -200095 \ No newline at end of file +200462 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap b/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap index 5ee38978..583a1dbf 100644 --- a/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap +++ b/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap @@ -1 +1 @@ -379287 \ No newline at end of file +374704 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeSecondSwap.snap b/.forge-snapshots/FullRangeSecondSwap.snap index 436848b5..97cbf81a 100644 --- a/.forge-snapshots/FullRangeSecondSwap.snap +++ b/.forge-snapshots/FullRangeSecondSwap.snap @@ -1 +1 @@ -112303 \ No newline at end of file +112734 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeSwap.snap b/.forge-snapshots/FullRangeSwap.snap index d48620c7..35131c42 100644 --- a/.forge-snapshots/FullRangeSwap.snap +++ b/.forge-snapshots/FullRangeSwap.snap @@ -1 +1 @@ -153038 \ No newline at end of file +149176 \ No newline at end of file diff --git a/README.md b/README.md index b931bd6a..b3c34732 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ If you’re interested in contributing please see the [contribution guidelines]( contracts/ ----hooks/ ----examples/ + | FullRange.sol | GeomeanOracle.sol | LimitOrder.sol | TWAMM.sol diff --git a/contracts/hooks/examples/FullRange.sol b/contracts/hooks/examples/FullRange.sol index 6c5b08ec..cd693640 100644 --- a/contracts/hooks/examples/FullRange.sol +++ b/contracts/hooks/examples/FullRange.sol @@ -39,11 +39,19 @@ contract FullRange is BaseHook, ILockCallback { bytes internal constant ZERO_BYTES = bytes(""); - /// @dev Min tick for full range with tick spacing of 60 - int24 internal constant MIN_TICK = -887220; - /// @dev Max tick for full range with tick spacing of 60 + /// @dev Set tick spacing to a large number that's <= type(int16).max + int24 internal constant TICK_SPACING = 0x7000; + + /// @dev Min tick for full range with tick spacing of TICK_SPACING + int24 internal constant MIN_TICK = (TickMath.MIN_TICK / TICK_SPACING + 1) * TICK_SPACING; + /// @dev Max tick for full range with tick spacing of TICK_SPACING int24 internal constant MAX_TICK = -MIN_TICK; + /// @dev TickMath.getSqrtRatioAtTick(MIN_TICK), cached for optimization + uint160 internal immutable MIN_SQRT_RATIO = TickMath.getSqrtRatioAtTick(MIN_TICK); + /// @dev TickMath.getSqrtRatioAtTick(MIN_TICK), cached for optimization + uint160 internal immutable MAX_SQRT_RATIO = TickMath.getSqrtRatioAtTick(MAX_TICK); + int256 internal constant MAX_INT = type(int256).max; uint16 internal constant MINIMUM_LIQUIDITY = 1000; @@ -109,7 +117,7 @@ contract FullRange is BaseHook, ILockCallback { currency0: params.currency0, currency1: params.currency1, fee: params.fee, - tickSpacing: 60, + tickSpacing: TICK_SPACING, hooks: IHooks(address(this)) }); @@ -124,11 +132,7 @@ contract FullRange is BaseHook, ILockCallback { uint128 poolLiquidity = poolManager.getLiquidity(poolId); liquidity = LiquidityAmounts.getLiquidityForAmounts( - sqrtPriceX96, - TickMath.getSqrtRatioAtTick(MIN_TICK), - TickMath.getSqrtRatioAtTick(MAX_TICK), - params.amount0Desired, - params.amount1Desired + sqrtPriceX96, MIN_SQRT_RATIO, MAX_SQRT_RATIO, params.amount0Desired, params.amount1Desired ); if (poolLiquidity == 0 && liquidity <= MINIMUM_LIQUIDITY) { @@ -166,7 +170,7 @@ contract FullRange is BaseHook, ILockCallback { currency0: params.currency0, currency1: params.currency1, fee: params.fee, - tickSpacing: 60, + tickSpacing: TICK_SPACING, hooks: IHooks(address(this)) }); @@ -195,7 +199,7 @@ contract FullRange is BaseHook, ILockCallback { override returns (bytes4) { - if (key.tickSpacing != 60) revert TickSpacingNotDefault(); + if (key.tickSpacing != TICK_SPACING) revert TickSpacingNotDefault(); PoolId poolId = key.toId(); @@ -234,11 +238,7 @@ contract FullRange is BaseHook, ILockCallback { returns (bytes4) { PoolId poolId = key.toId(); - - if (!poolInfo[poolId].hasAccruedFees) { - PoolInfo storage pool = poolInfo[poolId]; - pool.hasAccruedFees = true; - } + poolInfo[poolId].hasAccruedFees = true; return IHooks.beforeSwap.selector; } @@ -282,6 +282,7 @@ contract FullRange is BaseHook, ILockCallback { if (pool.hasAccruedFees) { _rebalance(key); + pool.hasAccruedFees = false; } uint256 liquidityToRemove = FullMath.mulDiv( @@ -292,7 +293,6 @@ contract FullRange is BaseHook, ILockCallback { params.liquidityDelta = -(liquidityToRemove.toInt256()); delta = poolManager.modifyPosition(key, params, ZERO_BYTES); - pool.hasAccruedFees = false; } function lockAcquired(bytes calldata rawData) @@ -326,10 +326,11 @@ contract FullRange is BaseHook, ILockCallback { ZERO_BYTES ); + // The final shift by 48 is equal to multiplying by sqrt(Q96) using unchecked math uint160 newSqrtPriceX96 = ( FixedPointMathLib.sqrt( FullMath.mulDiv(uint128(-balanceDelta.amount1()), FixedPoint96.Q96, uint128(-balanceDelta.amount0())) - ) * FixedPointMathLib.sqrt(FixedPoint96.Q96) + ) << 48 ).toUint160(); (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(poolId); @@ -346,8 +347,8 @@ contract FullRange is BaseHook, ILockCallback { uint128 liquidity = LiquidityAmounts.getLiquidityForAmounts( newSqrtPriceX96, - TickMath.getSqrtRatioAtTick(MIN_TICK), - TickMath.getSqrtRatioAtTick(MAX_TICK), + MIN_SQRT_RATIO, + MAX_SQRT_RATIO, uint256(uint128(-balanceDelta.amount0())), uint256(uint128(-balanceDelta.amount1())) ); diff --git a/test/FullRange.t.sol b/test/FullRange.t.sol index fa9d13ed..d7b6771f 100644 --- a/test/FullRange.t.sol +++ b/test/FullRange.t.sol @@ -19,6 +19,7 @@ import {IHooks} from "@uniswap/v4-core/contracts/interfaces/IHooks.sol"; import {UniswapV4ERC20} from "../contracts/libraries/UniswapV4ERC20.sol"; import {FullMath} from "@uniswap/v4-core/contracts/libraries/FullMath.sol"; import {SafeCast} from "@uniswap/v4-core/contracts/libraries/SafeCast.sol"; +import {TickMath} from "@uniswap/v4-core/contracts/libraries/TickMath.sol"; contract TestFullRange is Test, Deployers, GasSnapshot { using PoolIdLibrary for PoolKey; @@ -47,17 +48,17 @@ contract TestFullRange is Test, Deployers, GasSnapshot { uint24 fee ); - /// @dev Min tick for full range with tick spacing of 60 - int24 internal constant MIN_TICK = -887220; - /// @dev Max tick for full range with tick spacing of 60 - int24 internal constant MAX_TICK = -MIN_TICK; - - int24 constant TICK_SPACING = 60; + int24 constant TICK_SPACING = 0x7000; uint16 constant LOCKED_LIQUIDITY = 1000; uint256 constant MAX_DEADLINE = 12329839823; uint256 constant MAX_TICK_LIQUIDITY = 11505069308564788430434325881101412; uint8 constant DUST = 30; + /// @dev Min tick for full range with tick spacing of TICK_SPACING + int24 internal constant MIN_TICK = (TickMath.MIN_TICK / TICK_SPACING + 1) * TICK_SPACING; + /// @dev Max tick for full range with tick spacing of TICK_SPACING + int24 internal constant MAX_TICK = -MIN_TICK; + MockERC20 token0; MockERC20 token1; MockERC20 token2; @@ -169,10 +170,10 @@ contract TestFullRange is Test, Deployers, GasSnapshot { assertEq(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether); - assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether, DUST); - assertEq(liquidityTokenBal, 10 ether - LOCKED_LIQUIDITY); + assertApproxEqAbs(liquidityTokenBal, 10 ether - LOCKED_LIQUIDITY, DUST); assertEq(hasAccruedFees, false); } @@ -235,10 +236,10 @@ contract TestFullRange is Test, Deployers, GasSnapshot { assertEq(manager.getLiquidity(idWithLiq), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(keyWithLiq.currency0.balanceOfSelf(), prevBalance0 - 10 ether); - assertEq(keyWithLiq.currency1.balanceOfSelf(), prevBalance1 - 10 ether); + assertApproxEqAbs(keyWithLiq.currency0.balanceOfSelf(), prevBalance0 - 10 ether, DUST); + assertApproxEqAbs(keyWithLiq.currency1.balanceOfSelf(), prevBalance1 - 10 ether, DUST); - assertEq(liquidityTokenBal, prevLiquidityTokenBal + 10 ether); + assertApproxEqAbs(liquidityTokenBal, prevLiquidityTokenBal + 10 ether, DUST); assertEq(hasAccruedFees, false); } @@ -266,14 +267,21 @@ contract TestFullRange is Test, Deployers, GasSnapshot { uint256 liquidityTokenBal = UniswapV4ERC20(liquidityToken).balanceOf(address(this)); - assertEq(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(liquidityTokenBal, 10 ether - LOCKED_LIQUIDITY); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether); - assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether); + assertApproxEqAbs(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY, DUST); + assertApproxEqAbs(liquidityTokenBal, 10 ether - LOCKED_LIQUIDITY, DUST); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether, DUST); vm.expectEmit(true, true, true, true); emit Swap( - id, address(swapRouter), 1 ether, -906610893880149131, 72045250990510446115798809072, 10 ether, -1901, 3000 + id, + address(swapRouter), + 1 ether, + -906610893880149131, + 72045250990510446121024169824, + 10 ether + 8, + -1901, + 3000 ); IPoolManager.SwapParams memory params = @@ -287,7 +295,7 @@ contract TestFullRange is Test, Deployers, GasSnapshot { (bool hasAccruedFees,) = fullRange.poolInfo(id); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether - 1 ether); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether - 1 ether, DUST); assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 9093389106119850869); assertEq(hasAccruedFees, true); @@ -301,7 +309,7 @@ contract TestFullRange is Test, Deployers, GasSnapshot { liquidityTokenBal = UniswapV4ERC20(liquidityToken).balanceOf(address(this)); assertEq(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(liquidityTokenBal, 14546694553059925434 - LOCKED_LIQUIDITY); + assertEq(liquidityTokenBal, 14546694553059925446 - LOCKED_LIQUIDITY); assertEq(hasAccruedFees, true); } @@ -310,7 +318,15 @@ contract TestFullRange is Test, Deployers, GasSnapshot { fullRange.addLiquidity( FullRange.AddLiquidityParams( - key.currency0, key.currency1, 3000, 10 ether, 10 ether, 10 ether, 10 ether, address(this), MAX_DEADLINE + key.currency0, + key.currency1, + 3000, + 10 ether, + 10 ether, + 10 ether - DUST, + 10 ether - DUST, + address(this), + MAX_DEADLINE ) ); @@ -324,7 +340,15 @@ contract TestFullRange is Test, Deployers, GasSnapshot { vm.expectRevert(FullRange.TooMuchSlippage.selector); fullRange.addLiquidity( FullRange.AddLiquidityParams( - key.currency0, key.currency1, 3000, 10 ether, 10 ether, 10 ether, 10 ether, address(this), MAX_DEADLINE + key.currency0, + key.currency1, + 3000, + 10 ether, + 10 ether, + 10 ether - DUST, + 10 ether - DUST, + address(this), + MAX_DEADLINE ) ); } @@ -390,6 +414,32 @@ contract TestFullRange is Test, Deployers, GasSnapshot { assertEq(hasAccruedFees, true); } + function testFullRange_swap_LargeSwap() public { + PoolKey memory testKey = key; + manager.initialize(testKey, SQRT_RATIO_1_1, ZERO_BYTES); + + fullRange.addLiquidity( + FullRange.AddLiquidityParams( + key.currency0, key.currency1, 3000, 10 ether, 10 ether, 9 ether, 9 ether, address(this), MAX_DEADLINE + ) + ); + + IPoolManager.SwapParams memory params = IPoolManager.SwapParams({ + zeroForOne: true, + amountSpecified: 100 ether, + sqrtPriceLimitX96: TickMath.MIN_SQRT_RATIO + 1 + }); + PoolSwapTest.TestSettings memory settings = + PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true}); + + snapStart("FullRangeLargeSwap"); + swapRouter.swap(testKey, params, settings, ZERO_BYTES); + snapEnd(); + + (bool hasAccruedFees,) = fullRange.poolInfo(id); + assertEq(hasAccruedFees, true); + } + function testFullRange_removeLiquidity_InitialRemoveSucceeds() public { uint256 prevBalance0 = keyWithLiq.currency0.balanceOfSelf(); uint256 prevBalance1 = keyWithLiq.currency1.balanceOfSelf(); @@ -409,9 +459,11 @@ contract TestFullRange is Test, Deployers, GasSnapshot { uint256 liquidityTokenBal = UniswapV4ERC20(liquidityToken).balanceOf(address(this)); assertEq(manager.getLiquidity(idWithLiq), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 99 ether - LOCKED_LIQUIDITY + 5); - assertEq(keyWithLiq.currency0.balanceOfSelf(), prevBalance0 + 1 ether - 1); - assertEq(keyWithLiq.currency1.balanceOfSelf(), prevBalance1 + 1 ether - 1); + assertApproxEqAbs( + UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 99 ether - LOCKED_LIQUIDITY, 3 * DUST + ); + assertApproxEqAbs(keyWithLiq.currency0.balanceOfSelf(), prevBalance0 + 1 ether, DUST); + assertApproxEqAbs(keyWithLiq.currency1.balanceOfSelf(), prevBalance1 + 1 ether, DUST); assertEq(hasAccruedFees, false); } @@ -489,10 +541,10 @@ contract TestFullRange is Test, Deployers, GasSnapshot { (, address liquidityToken) = fullRange.poolInfo(id); - assertEq(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 10 ether - LOCKED_LIQUIDITY); + assertApproxEqAbs(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 10 ether - LOCKED_LIQUIDITY, DUST); - assertEq(key.currency0.balanceOfSelf(), prevBalance0 - 10 ether); - assertEq(key.currency1.balanceOfSelf(), prevBalance1 - 10 ether); + assertApproxEqAbs(key.currency0.balanceOfSelf(), prevBalance0 - 10 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOfSelf(), prevBalance1 - 10 ether, DUST); UniswapV4ERC20(liquidityToken).approve(address(fullRange), type(uint256).max); @@ -504,9 +556,9 @@ contract TestFullRange is Test, Deployers, GasSnapshot { uint256 liquidityTokenBal = UniswapV4ERC20(liquidityToken).balanceOf(address(this)); assertEq(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(liquidityTokenBal, 5 ether - LOCKED_LIQUIDITY); - assertEq(key.currency0.balanceOfSelf(), prevBalance0 - 5 ether - 1); - assertEq(key.currency1.balanceOfSelf(), prevBalance1 - 5 ether - 1); + assertApproxEqAbs(liquidityTokenBal, 5 ether - LOCKED_LIQUIDITY, DUST); + assertApproxEqAbs(key.currency0.balanceOfSelf(), prevBalance0 - 5 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOfSelf(), prevBalance1 - 5 ether, DUST); assertEq(hasAccruedFees, false); } @@ -522,12 +574,12 @@ contract TestFullRange is Test, Deployers, GasSnapshot { ) ); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether); - assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 10 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOf(address(this)), prevBalance1 - 10 ether, DUST); (, address liquidityToken) = fullRange.poolInfo(id); - assertEq(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 10 ether - LOCKED_LIQUIDITY); + assertApproxEqAbs(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 10 ether - LOCKED_LIQUIDITY, DUST); fullRange.addLiquidity( FullRange.AddLiquidityParams( @@ -535,10 +587,10 @@ contract TestFullRange is Test, Deployers, GasSnapshot { ) ); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 12.5 ether); - assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 12.5 ether); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 12.5 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOf(address(this)), prevBalance1 - 12.5 ether, DUST); - assertEq(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 12.5 ether - LOCKED_LIQUIDITY); + assertApproxEqAbs(UniswapV4ERC20(liquidityToken).balanceOf(address(this)), 12.5 ether - LOCKED_LIQUIDITY, DUST); UniswapV4ERC20(liquidityToken).approve(address(fullRange), type(uint256).max); @@ -549,9 +601,9 @@ contract TestFullRange is Test, Deployers, GasSnapshot { uint256 liquidityTokenBal = UniswapV4ERC20(liquidityToken).balanceOf(address(this)); assertEq(manager.getLiquidity(id), liquidityTokenBal + LOCKED_LIQUIDITY); - assertEq(liquidityTokenBal, 7.5 ether - LOCKED_LIQUIDITY); - assertEq(key.currency0.balanceOf(address(this)), prevBalance0 - 7.5 ether - 1); - assertEq(key.currency1.balanceOf(address(this)), prevBalance1 - 7.5 ether - 1); + assertApproxEqAbs(liquidityTokenBal, 7.5 ether - LOCKED_LIQUIDITY, DUST); + assertApproxEqAbs(key.currency0.balanceOf(address(this)), prevBalance0 - 7.5 ether, DUST); + assertApproxEqAbs(key.currency1.balanceOf(address(this)), prevBalance1 - 7.5 ether, DUST); } function testFullRange_removeLiquidity_SwapAndRebalance() public { @@ -706,7 +758,7 @@ contract TestFullRange is Test, Deployers, GasSnapshot { // PoolManager does not have any liquidity left over assertTrue(manager.getLiquidity(id) >= LOCKED_LIQUIDITY); - assertTrue(manager.getLiquidity(id) < LOCKED_LIQUIDITY + DUST); + assertTrue(manager.getLiquidity(id) < LOCKED_LIQUIDITY + 266); assertEq(hasAccruedFees, false); }