Optimize SqrtPriceMath
#258
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
by replacing `require` statements with inline assembly
This was referenced Jun 18, 2023
# Conflicts: # .forge-snapshots/mint with empty hook.snap # .forge-snapshots/mint with native token.snap # .forge-snapshots/mint.snap # .forge-snapshots/simple swap.snap # .forge-snapshots/swap against liquidity with native token.snap # .forge-snapshots/swap against liquidity.snap # .forge-snapshots/swap with hooks.snap # .forge-snapshots/swap with native.snap # src/libraries/SqrtPriceMath.sol # test/__snapshots__/PoolManager.gas.spec.ts.snap # test/__snapshots__/PoolManager.spec.ts.snap # test/__snapshots__/SqrtPriceMath.spec.ts.snap # test/__snapshots__/SwapMath.spec.ts.snap # test/libraries/SqrtPriceMath.t.sol
|
Please can you merge main so we can re-review the gas improvements 🙏 |
I did already. |
# Conflicts: # .forge-snapshots/addLiquidity with empty hook.snap # .forge-snapshots/addLiquidity with native token.snap # .forge-snapshots/addLiquidity.snap # .forge-snapshots/poolManager bytecode size.snap # .forge-snapshots/removeLiquidity with empty hook.snap # .forge-snapshots/removeLiquidity with native token.snap # .forge-snapshots/removeLiquidity.snap # .forge-snapshots/simple swap with native.snap # .forge-snapshots/simple swap.snap # .forge-snapshots/swap against liquidity with native token.snap # .forge-snapshots/swap against liquidity.snap # .forge-snapshots/swap burn 6909 for input.snap # .forge-snapshots/swap burn native 6909 for input.snap # .forge-snapshots/swap mint native output as 6909.snap # .forge-snapshots/swap mint output as 6909.snap # .forge-snapshots/swap with dynamic fee.snap # .forge-snapshots/swap with hooks.snap # .forge-snapshots/update dynamic fee in before swap.snap
Renamed the method `mulDiv96` to `mulDivQ96` in FullMath.sol and SqrtPriceMath.sol to better reflect its usage. Additionally, snapshot files were updated with new resultant values reflecting the changes in the computations.
Several function and variable names have been updated in the SqrtPriceMath.sol and FullMath.t.sol files for improved clarity. In the SqrtPriceMath file, the getNextSqrtPriceFromAmount1RoundingDown return name has been removed. In the FullMath.t.sol file, functions with 'mulDiv96' have been renamed to 'mulDivQ96'.
…ount1RoundingDown` This commit changes the way we check if an amount is within uint160 range in SqrtPriceMath.sol. Instead of comparing it with the max value of uint160, we now simply right shift by 160 and check if the result is zero. This substantially reduces execution costs, as seen in updated benchmarks and snapshots.
In the FullMath library, the variable names in the comments for the `mulDivQ96` function were updated from 'x' and 'y' to 'a' and 'b'. This change was implemented to match the actual variable names used in the function, making the code clearer and easier to understand.
The 'absDiff' function has been added in `SqrtPriceMath.sol` to replace direct subtraction in the `getAmount1Delta` method. This new function handles potential underflow by taking the absolute difference between inputs. Snapshots across different scenarios have been updated accordingly, reflecting more accurate value computations and slightly reduced gas costs.
This update includes refactoring in the SqrtPriceMath.sol, avoiding unnecessary typecasting and using assembly code for optimal performance. Most of the changes involve replacing expressions with their equivalent assembly instructions for faster execution. This code optimization resulted in reduced gas cost in forge-snapshot files.
Updated the code to improve how sign and mask values are calculated within the SqrtPriceMath library. The new implementation uses shift arithmetic right (sar) operations for more efficient and clear code. Changes have been made in absDiff, getAmount0Delta and getAmount1Delta functions.
Added new division function in `UnsafeMath` library that doesn't perform safety checks and a function for sorting two uint values in `SqrtPriceMath`. Adjusted the calculation process for `getAmount0Delta` in `SqrtPriceMath` to use the new functions for better performance and safety checks.
This commit adds a new function named 'assign' in SqrtPriceMath.sol. This function allows for direct conversion from two `uint160`s to two `uint256`s without implicit upcasting. Additionally, this newly introduced function has been utilized in line 185 where previously the conversion was done via assembly, simplifying the code and maintaining consistency.
The code in `SqrtPriceMath` has been refactored to optimize the functions `getAmount0Delta` and `getAmount1Delta`. The changes replaced previous return statements with inline assembly to handle operations on `int256` more efficiently. This has resulted in reduction in bytecode size and runtime gas.
This commit changes the FullMath.mulDiv function call in SqrtPriceMath to FullMath.mulDivQ96. This change simplifies the code and makes it more readable, without impacting the correctness or efficiency of the operation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue
Which issue does this pull request resolve?
Description of changes
The functions within
SqrtPriceMathhave been comprehensively rewritten and optimized to various degrees, with implementation ported from Aperture-Finance/uni-v3-lib. Heuristics for replacing conditional statements with bitwise operations have previously been discussed in Uniswap/universal-router#291. Additional changes primarily revolve around mathematical enhancements.A new function,
FullMath.mulDivQ96, is introduced for more efficient execution ofmulDiv(a, b, FixedPoint96.Q96). Helper functions such assort2andabsDiffimplemented in inline assembly are also introduced to reduce gas which may later be placed in a separate library.Gas snapshots have been taken after each commit to incrementally validate the effectiveness of these optimizations. The results are as follows:
Gas Usage Comparison
Note: The provided gas numbers are averages calculated from Foundry gas snapshots, taking into account both
roundUp = trueandroundUp = falsescenarios.In addition to these,
SwapMath.computeSwapStephas also seen improvements ranging from 9.1% to 15.6%.