Skip to content

Commit

Permalink
fix: Replace shr with sar for integer types (#586)
Browse files Browse the repository at this point in the history
* fix: Replace `shr` with `sar` for integer types

This commit updates the `shr` instruction to `sar` in the BalanceDelta and Pool contracts, as `sar` is the correct way to shift right when dealing with signed integers. This change ensures that the sign is preserved during bit shifting operations, preventing any potential bugs related to negative amounts or liquidity values.

* Add a fuzz test for `Pool.updateTick`

This commit introduces a fuzz test for the `Pool.updateTick` function to ensure proper functionality even when fed with diverse/random data. It also includes the creation of a `LiquidityMathRef` contract which houses the helper methods utilized in this fuzz testing. Any occurrence of an arithmetic error during testing is adequately caught and asserted.

* Use `signextend` in `BalanceDeltaLibrary.amount1`
  • Loading branch information
shuhuiluo authored May 11, 2024
1 parent 830dfff commit 616e988
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,8 @@ library Pool {
let liquidity := sload(info.slot)
// slice off top 128 bits of liquidity (liquidityNet) to get just liquidityGross
liquidityGrossBefore := shr(128, shl(128, liquidity))
// shift right 128 bits to get just liquidityNet
liquidityNetBefore := shr(128, liquidity)
// signed shift right 128 bits to get just liquidityNet
liquidityNetBefore := sar(128, liquidity)
}

liquidityGrossAfter = LiquidityMath.addDelta(liquidityGrossBefore, liquidityDelta);
Expand Down
55 changes: 55 additions & 0 deletions test/Tick.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,27 @@
pragma solidity ^0.8.20;

import {Test} from "forge-std/Test.sol";
import {stdError} from "forge-std/StdError.sol";
import {GasSnapshot} from "../lib/forge-gas-snapshot/src/GasSnapshot.sol";
import {Constants} from "./utils/Constants.sol";
import {Pool} from "../src/libraries/Pool.sol";
import {TickMath} from "../src/libraries/TickMath.sol";
import {PoolGetters} from "../src/libraries/PoolGetters.sol";

contract LiquidityMathRef {
function addDelta(uint128 x, int128 y) external pure returns (uint128) {
return y < 0 ? x - uint128(-y) : x + uint128(y);
}

function addDelta(bool upper, int128 liquidityNetBefore, int128 liquidityDelta)
external
pure
returns (int128 liquidityNet)
{
liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;
}
}

contract TickTest is Test, GasSnapshot {
using PoolGetters for Pool.State;
using Pool for Pool.State;
Expand All @@ -18,6 +33,12 @@ contract TickTest is Test, GasSnapshot {

Pool.State public pool;

LiquidityMathRef internal liquidityMath;

function setUp() public {
liquidityMath = new LiquidityMathRef();
}

function ticks(int24 tick) internal view returns (Pool.TickInfo memory) {
return pool.ticks[tick];
}
Expand Down Expand Up @@ -408,6 +429,40 @@ contract TickTest is Test, GasSnapshot {
assertEq(info.liquidityNet, int128(Constants.MAX_UINT128 / 2));
}

function testTick_update_fuzz(uint128 liquidityGross, int128 liquidityNet, int128 liquidityDelta, bool upper)
public
{
try liquidityMath.addDelta(liquidityGross, liquidityDelta) returns (uint128 liquidityGrossAfter) {
try liquidityMath.addDelta(upper, liquidityNet, liquidityDelta) returns (int128 liquidityNetAfter) {
Pool.TickInfo memory info = Pool.TickInfo({
liquidityGross: liquidityGross,
liquidityNet: liquidityNet,
feeGrowthOutside0X128: 0,
feeGrowthOutside1X128: 0
});

setTick(2, info);
update({
tick: 2,
tickCurrent: 1,
liquidityDelta: liquidityDelta,
feeGrowthGlobal0X128: 0,
feeGrowthGlobal1X128: 0,
upper: upper
});

info = ticks(2);

assertEq(info.liquidityGross, liquidityGrossAfter);
assertEq(info.liquidityNet, liquidityNetAfter);
} catch (bytes memory reason) {
assertEq(reason, stdError.arithmeticError);
}
} catch (bytes memory reason) {
assertEq(reason, stdError.arithmeticError);
}
}

function testTick_clear_deletesAllTheDataInTheTick() public {
Pool.TickInfo memory info;

Expand Down

0 comments on commit 616e988

Please sign in to comment.