Skip to content

Commit

Permalink
Spearbit Gas-08 (#844)
Browse files Browse the repository at this point in the history
* spearbit gas-08

* use simpleMulDiv function

* use simpleMulDiv in swap function too

* unsafe math comments

* comment fix
  • Loading branch information
dianakocsis authored Aug 27, 2024
1 parent 0fc5a0b commit 5951b35
Show file tree
Hide file tree
Showing 22 changed files with 64 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106476
106338
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
146058
145776
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24268
24066
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
109048
108760
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123891
123603
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA fee on unspecified.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155331
155043
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106057
105913
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
117148
117004
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129249
129105
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn native 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118702
118558
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint native output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140094
139950
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155731
155443
Original file line number Diff line number Diff line change
@@ -1 +1 @@
207443
207011
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139923
139635
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132785
132641
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with lp fee and protocol fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
169667
169379
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with return dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
146249
145961
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
148526
148238
10 changes: 6 additions & 4 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.0;
import {SafeCast} from "./SafeCast.sol";
import {TickBitmap} from "./TickBitmap.sol";
import {Position} from "./Position.sol";
import {FullMath} from "./FullMath.sol";
import {UnsafeMath} from "./UnsafeMath.sol";
import {FixedPoint128} from "./FixedPoint128.sol";
import {TickMath} from "./TickMath.sol";
import {SqrtPriceMath} from "./SqrtPriceMath.sol";
Expand Down Expand Up @@ -394,7 +394,8 @@ library Pool {
// update global fee tracker
if (result.liquidity > 0) {
unchecked {
feeGrowthGlobalX128 += FullMath.mulDiv(step.feeAmount, FixedPoint128.Q128, result.liquidity);
// FullMath.mulDiv isn't needed as the numerator can't overflow uint256 since tokens have a max supply of type(uint128).max
feeGrowthGlobalX128 += UnsafeMath.simpleMulDiv(step.feeAmount, FixedPoint128.Q128, result.liquidity);
}
}

Expand Down Expand Up @@ -457,11 +458,12 @@ library Pool {
unchecked {
// negation safe as amount0 and amount1 are always positive
delta = toBalanceDelta(-(amount0.toInt128()), -(amount1.toInt128()));
// FullMath.mulDiv is unnecessary because the numerator is bounded by type(int128).max * Q128, which is less than type(uint256).max
if (amount0 > 0) {
state.feeGrowthGlobal0X128 += FullMath.mulDiv(amount0, FixedPoint128.Q128, liquidity);
state.feeGrowthGlobal0X128 += UnsafeMath.simpleMulDiv(amount0, FixedPoint128.Q128, liquidity);
}
if (amount1 > 0) {
state.feeGrowthGlobal1X128 += FullMath.mulDiv(amount1, FixedPoint128.Q128, liquidity);
state.feeGrowthGlobal1X128 += UnsafeMath.simpleMulDiv(amount1, FixedPoint128.Q128, liquidity);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/SwapMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ library SwapMath {
/// @return sqrtPriceNextX96 The price after swapping the amount in/out, not to exceed the price target
/// @return amountIn The amount to be swapped in, of either currency0 or currency1, based on the direction of the swap
/// @return amountOut The amount to be received, of either currency0 or currency1, based on the direction of the swap
/// @return feeAmount The amount of input that will be taken as a fee/
/// @return feeAmount The amount of input that will be taken as a fee
/// @dev feePips must be no larger than MAX_SWAP_FEE for this function. We ensure that before setting a fee using LPFeeLibrary.isValid.
function computeSwapStep(
uint160 sqrtPriceCurrentX96,
Expand Down
12 changes: 12 additions & 0 deletions src/libraries/UnsafeMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,16 @@ library UnsafeMath {
}
}
}

/// @notice Calculates floor(a×b÷denominator)
/// @dev division by 0 has unspecified behavior, and must be checked externally
/// @param a The multiplicand
/// @param b The multiplier
/// @param denominator The divisor
/// @return result The 256-bit result, floor(a×b÷denominator)
function simpleMulDiv(uint256 a, uint256 b, uint256 denominator) internal pure returns (uint256 result) {
assembly ("memory-safe") {
result := div(mul(a, b), denominator)
}
}
}
27 changes: 27 additions & 0 deletions test/libraries/UnsafeMath.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,31 @@ contract UnsafeMathTest is Test {
assertEq(diff, 1);
}
}

function test_simpleMulDiv_zeroDoesNotRevert(uint256 a, uint256 b) public pure {
a.simpleMulDiv(b, 0);
}

function test_simpleMulDiv_succeeds() public pure {
assertEq(Q128.simpleMulDiv(3, 2), Q128 * 3 / 2);
}

function test_simpleMulDiv_noOverflow() public pure {
assertLe(uint256(int256(type(int128).max)) * Q128, type(uint256).max);
}

function test_fuzz_simpleMulDiv_succeeds(uint256 a, uint256 b, uint256 denominator) public pure {
vm.assume(denominator != 0);
uint256 result = a.simpleMulDiv(b, denominator);
unchecked {
assertTrue(result == a * b / denominator);
}
}

function test_fuzz_simpleMulDiv_bounded(uint256 a, uint256 denominator) public pure {
a = bound(a, 0, uint256(int256(type(int128).max)));
denominator = bound(denominator, 1, uint256(type(uint128).max));
uint256 result = a.simpleMulDiv(Q128, denominator);
assertTrue(result == a * Q128 / denominator);
}
}

0 comments on commit 5951b35

Please sign in to comment.