From fa5164a1e26b3b15366816a40e4e0ed6d740850b Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Tue, 21 May 2024 15:21:34 -0500 Subject: [PATCH] Optimize `absTick` calculation in `TickMath.getSqrtPriceAtTick` (#663) * Optimize `absTick` calculation in `TickMath.getSqrtPriceAtTick` The getSqrtPriceAtTick function in the TickMath library has been refactored with a more efficient assembly implementation. This optimization significantly reduces the gas usage by using bitwise operations for computing absolute value of a tick. * Add test case for `getSqrtPriceAtTick` with `type(int24).min` The new test case asserts that the function `getSqrtPriceAtTick` throws an error when called with the minimum valid input. This ensures that the function behaves as expected for all possible input values, enhancing the overall test coverage. * Add comments about rounding in `getSqrtPriceAtTick` --- .forge-snapshots/TickMathGetSqrtPriceAtTick.snap | 2 +- .forge-snapshots/TickMathGetTickAtSqrtPrice.snap | 2 +- ...idity to already existing position with salt.snap | 2 +- .forge-snapshots/addLiquidity CA fee.snap | 2 +- .forge-snapshots/addLiquidity with empty hook.snap | 2 +- .forge-snapshots/addLiquidity with native token.snap | 2 +- ...create new liquidity to a position with salt.snap | 2 +- .forge-snapshots/initialize.snap | 2 +- .forge-snapshots/poolManager bytecode size.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- ...mple addLiquidity second addition same range.snap | 2 +- .forge-snapshots/simple addLiquidity.snap | 2 +- ...imple removeLiquidity some liquidity remains.snap | 2 +- .forge-snapshots/simple removeLiquidity.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .forge-snapshots/swap CA fee on unspecified.snap | 2 +- .../swap against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .forge-snapshots/swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .forge-snapshots/swap mint output as 6909.snap | 2 +- .../swap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .forge-snapshots/swap with return dynamic fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/libraries/TickMath.sol | 12 +++++++++++- test/libraries/TickMath.t.sol | 5 +++++ 33 files changed, 47 insertions(+), 32 deletions(-) diff --git a/.forge-snapshots/TickMathGetSqrtPriceAtTick.snap b/.forge-snapshots/TickMathGetSqrtPriceAtTick.snap index 3f9b14365..a389bef6a 100644 --- a/.forge-snapshots/TickMathGetSqrtPriceAtTick.snap +++ b/.forge-snapshots/TickMathGetSqrtPriceAtTick.snap @@ -1 +1 @@ -80082 \ No newline at end of file +76732 \ No newline at end of file diff --git a/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap b/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap index 6e3ae1235..611c1ba05 100644 --- a/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap +++ b/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap @@ -1 +1 @@ -227576 \ No newline at end of file +224621 \ No newline at end of file diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap index 243f23c8a..70cb20284 100644 --- a/.forge-snapshots/add liquidity to already existing position with salt.snap +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -1 +1 @@ -153494 \ No newline at end of file +153427 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 885c6a535..927bf0ecc 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -331160 \ No newline at end of file +331093 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index 0d0b571fd..75c65f10a 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -286153 \ No newline at end of file +286086 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index c5edfa171..25ba858d4 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -143704 \ No newline at end of file +143637 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap index d633601b5..2f53eb249 100644 --- a/.forge-snapshots/create new liquidity to a position with salt.snap +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -1 +1 @@ -301672 \ No newline at end of file +301605 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index f6e21d211..32826ad3b 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -61811 \ No newline at end of file +61775 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index c6b439c0f..c94d9db46 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -21667 \ No newline at end of file +21647 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index bd25f2f9d..29e8612d9 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -186626 \ No newline at end of file +186559 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index 9aebeeca5..ee41dab80 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -122994 \ No newline at end of file +122927 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 57ce483d8..4e26e8532 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -119781 \ No newline at end of file +119714 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap index 56be2edd4..c63746e60 100644 --- a/.forge-snapshots/simple addLiquidity second addition same range.snap +++ b/.forge-snapshots/simple addLiquidity second addition same range.snap @@ -1 +1 @@ -104808 \ No newline at end of file +104746 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap index cb67d6f19..230487a70 100644 --- a/.forge-snapshots/simple addLiquidity.snap +++ b/.forge-snapshots/simple addLiquidity.snap @@ -1 +1 @@ -167300 \ No newline at end of file +167238 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap index a0071306b..d525fb645 100644 --- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap +++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap @@ -1 +1 @@ -98405 \ No newline at end of file +98343 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap index f1d44602e..aabdfdaea 100644 --- a/.forge-snapshots/simple removeLiquidity.snap +++ b/.forge-snapshots/simple removeLiquidity.snap @@ -1 +1 @@ -90445 \ No newline at end of file +90383 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 261cf66ae..4dde4a638 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -116663 \ No newline at end of file +116560 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 5ee86dce6..235ed690c 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -131824 \ No newline at end of file +131721 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index a75595573..bee51022c 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -182962 \ No newline at end of file +182859 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index e5f8861df..9bda3178e 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -112435 \ No newline at end of file +112368 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index d0890ccf3..6bbd2798a 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -123778 \ No newline at end of file +123711 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index d997832ef..38f8c1eea 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -135785 \ No newline at end of file +135713 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 324ba79d2..8551647d0 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -125038 \ No newline at end of file +124966 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index c290ce5bd..26292e4a0 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -147042 \ No newline at end of file +146970 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index 873d723a3..197a9f85e 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -163678 \ No newline at end of file +163575 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index 1f83ba701..82ca5681c 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -221922 \ No newline at end of file +221752 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index b6f7b05d8..981054d4f 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -147879 \ No newline at end of file +147776 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 939a6cc8a..43d06fb97 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -123790 \ No newline at end of file +123723 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index 52d7b3932..aa2a23a21 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -180059 \ No newline at end of file +179956 \ No newline at end of file diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap index cb4e4e791..586e9f078 100644 --- a/.forge-snapshots/swap with return dynamic fee.snap +++ b/.forge-snapshots/swap with return dynamic fee.snap @@ -1 +1 @@ -155738 \ No newline at end of file +155635 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index b9ffd5fe9..f55f8c4c4 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -158341 \ No newline at end of file +158238 \ No newline at end of file diff --git a/src/libraries/TickMath.sol b/src/libraries/TickMath.sol index 9f8b108ac..7498f9952 100644 --- a/src/libraries/TickMath.sol +++ b/src/libraries/TickMath.sol @@ -46,7 +46,15 @@ library TickMath { /// at the given tick function getSqrtPriceAtTick(int24 tick) internal pure returns (uint160 sqrtPriceX96) { unchecked { - uint256 absTick = tick < 0 ? uint256(-int256(tick)) : uint256(int256(tick)); + uint256 absTick; + assembly { + // mask = 0 if tick >= 0 else -1 + let mask := sar(255, tick) + // If tick >= 0, |tick| = tick = 0 ^ tick + // If tick < 0, |tick| = ~~|tick| = ~(-|tick| - 1) = ~(tick - 1) = (-1) ^ (tick - 1) + // Either case, |tick| = mask ^ (tick + mask) + absTick := xor(mask, add(mask, tick)) + } if (absTick > uint256(int256(MAX_TICK))) revert InvalidTick(); uint256 price = @@ -78,6 +86,8 @@ library TickMath { // this divides by 1<<32 rounding up to go from a Q128.128 to a Q128.96. // we then downcast because we know the result always fits within 160 bits due to our tick input constraint // we round up in the division so getTickAtSqrtPrice of the output price is always consistent + // `sub(shl(32, 1), 1)` is `type(uint32).max` + // `price + type(uint32).max` will not overflow because `price` fits in 192 bits sqrtPriceX96 := shr(32, add(price, sub(shl(32, 1), 1))) } } diff --git a/test/libraries/TickMath.t.sol b/test/libraries/TickMath.t.sol index 93714f97a..fa04fc015 100644 --- a/test/libraries/TickMath.t.sol +++ b/test/libraries/TickMath.t.sol @@ -45,6 +45,11 @@ contract TickMathTestTest is Test, JavascriptFfi, GasSnapshot { assertEq(maxTick, MAX_TICK); } + function test_getSqrtPriceAtTick_throwsForInt24Min() public { + vm.expectRevert(TickMath.InvalidTick.selector); + tickMath.getSqrtPriceAtTick(type(int24).min); + } + function test_getSqrtPriceAtTick_throwsForTooLow() public { vm.expectRevert(TickMath.InvalidTick.selector); tickMath.getSqrtPriceAtTick(MIN_TICK - 1);