Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [hexen-r9] add overflow check for bin liquidity #211

Merged
merged 5 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142478
142475
Original file line number Diff line number Diff line change
@@ -1 +1 @@
188410
188265
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
311254
311495
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
189451
190107
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
410336
410577
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23287
23558
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasDonate.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118691
118546
Original file line number Diff line number Diff line change
@@ -1 +1 @@
968475
970284
Original file line number Diff line number Diff line change
@@ -1 +1 @@
327787
329605
Original file line number Diff line number Diff line change
@@ -1 +1 @@
337511
337752
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140062
140304
Original file line number Diff line number Diff line change
@@ -1 +1 @@
173098
178450
Original file line number Diff line number Diff line change
@@ -1 +1 @@
179126
184732
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133129
133785
Original file line number Diff line number Diff line change
@@ -1 +1 @@
304550
304791
Original file line number Diff line number Diff line change
@@ -1 +1 @@
109014
115965
Original file line number Diff line number Diff line change
@@ -1 +1 @@
156553
171392
Original file line number Diff line number Diff line change
@@ -1 +1 @@
62453
63109
Original file line number Diff line number Diff line change
@@ -1 +1 @@
62422
63078
Original file line number Diff line number Diff line change
@@ -1 +1 @@
151090
165863
Original file line number Diff line number Diff line change
@@ -1 +1 @@
56810
57403
Original file line number Diff line number Diff line change
@@ -1 +1 @@
56825
57442
23 changes: 20 additions & 3 deletions src/pool-bin/libraries/BinPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ library BinPool {
error BinPool__NoLiquidityToReceiveFees();
/// @dev if swap exactIn, x for y, unspecifiedToken = token y. if swap x for exact out y, unspecified token is x
error BinPool__InsufficientAmountUnSpecified();
error BinPool__MaxLiquidityPerBinExceeded();

/// @dev The state of a pool
struct State {
Expand Down Expand Up @@ -169,6 +170,14 @@ library BinPool {
}

self.reserveOfBin[swapState.activeId] = binReserves.add(amountsInWithFees).sub(amountsOutOfBin);

if (
Copy link
Collaborator

@ChefMist ChefMist Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can i trouble you to try to add some test cases for swap/mint case? would be good exercise to further our knowledge on bin related especially with #213 issue

self.reserveOfBin[swapState.activeId].getLiquidity(
swapState.activeId.getPriceFromId(params.binStep)
) > Constants.MAX_LIQUIDITY_PER_BIN
) {
revert BinPool__MaxLiquidityPerBinExceeded();
}
}
}

Expand Down Expand Up @@ -347,9 +356,12 @@ library BinPool {

/// @dev overflow check on total reserves and the resulting liquidity
uint256 price = activeId.getPriceFromId(binStep);
binReserves.add(amountIn).getLiquidity(price);
bytes32 newReserves = binReserves.add(amountIn);
if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
revert BinPool__MaxLiquidityPerBinExceeded();
}

ChefSnoopy marked this conversation as resolved.
Show resolved Hide resolved
self.reserveOfBin[activeId] = binReserves.add(amountIn);
self.reserveOfBin[activeId] = newReserves;
result = toBalanceDelta(-(amount0.safeInt128()), -(amount1.safeInt128()));
}

Expand Down Expand Up @@ -457,7 +469,12 @@ library BinPool {
if (shares == 0 || amountsInToBin == 0) revert BinPool__ZeroShares(id);
if (supply == 0) _addBinIdToTree(self, id);

self.reserveOfBin[id] = binReserves.add(amountsInToBin);
bytes32 newReserves = binReserves.add(amountsInToBin);
if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
revert BinPool__MaxLiquidityPerBinExceeded();
}

self.reserveOfBin[id] = newReserves;
}

/// @notice Subtract share from user's position and update total share supply of bin
Expand Down
4 changes: 4 additions & 0 deletions src/pool-bin/libraries/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ library Constants {
uint256 internal constant SQUARED_PRECISION = PRECISION * PRECISION;

uint256 internal constant BASIS_POINT_MAX = 10_000;

// (2^256 - 1) / (2 * log(2**128) / log(1.0001))
uint256 internal constant MAX_LIQUIDITY_PER_BIN =
65251743116719673010965625540244653191619923014385985379600384103134737;
}
11 changes: 11 additions & 0 deletions test/pool-bin/libraries/BinPoolDonate.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@ import {PackedUint128Math} from "../../../src/pool-bin/libraries/math/PackedUint
import {SafeCast} from "../../../src/pool-bin/libraries/math/SafeCast.sol";
import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolParametersHelper.sol";
import {BinTestHelper} from "../helpers/BinTestHelper.sol";
import {Constants} from "../../../src/pool-bin/libraries/Constants.sol";
import {PriceHelper} from "../../../src/pool-bin/libraries/PriceHelper.sol";

contract BinPoolDonateTest is BinTestHelper {
using PackedUint128Math for bytes32;
using BinPoolParametersHelper for bytes32;
using SafeCast for uint256;
using BinHelper for bytes32;

MockVault public vault;
BinPoolManager public poolManager;
Expand Down Expand Up @@ -119,6 +122,14 @@ contract BinPoolDonateTest is BinTestHelper {
addLiquidityToBin(key, poolManager, bob, activeId, 1e18, 1e18, 1e18, 1e18, "");
poolManager.getPosition(poolId, bob, activeId, 0).share;

bytes32 newReserves = PackedUint128Math.encode(1e18 + amt0, 1e18 + amt1);
uint256 price = PriceHelper.getPriceFromId(activeId, poolParam.getBinStep());
if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
poolManager.donate(key, amt0, amt1, "");
return;
}

poolManager.donate(key, amt0, amt1, "");

// Verify reserve after donate
Expand Down
101 changes: 101 additions & 0 deletions test/pool-bin/libraries/BinPoolLiquidity.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import {LiquidityConfigurations} from "../../../src/pool-bin/libraries/math/Liqu
import {IBinPoolManager} from "../../../src/pool-bin/interfaces/IBinPoolManager.sol";
import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolParametersHelper.sol";
import {BinTestHelper} from "../helpers/BinTestHelper.sol";
import {PriceHelper} from "../../../src/pool-bin/libraries/PriceHelper.sol";
import {BinHelper} from "../../../src/pool-bin/libraries/BinHelper.sol";

contract BinPoolLiquidityTest is BinTestHelper {
using PackedUint128Math for bytes32;
using BinPoolParametersHelper for bytes32;
using SafeCast for uint256;
using BinHelper for bytes32;

MockVault public vault;
BinPoolManager public poolManager;
Expand Down Expand Up @@ -53,6 +56,104 @@ contract BinPoolLiquidityTest is BinTestHelper {
poolId = key.toId();
}

function test_MintFuzz(uint128 amountX, uint128 amountY) external {
amountX = uint128(bound(amountX, 1 ether, uint128(type(int128).max)));
amountY = uint128(bound(amountY, 1 ether, uint128(type(int128).max)));

uint8 nbBinX = 6;
uint8 nbBinY = 6;

poolManager.initialize(key, activeId);

BinPool.MintArrays memory array;
vault.updateCurrentPoolKey(key);

// check if the new liquidity will exceed the max liquidity per bin
bool shouldRevert = false;
bytes32 newReserves = PackedUint128Math.encode(amountX / nbBinX, amountY / nbBinY);
{
uint256 total = getTotalBins(nbBinX, nbBinY);
for (uint256 i; i < total; ++i) {
uint24 id = getId(activeId, i, nbBinY);
uint256 price = PriceHelper.getPriceFromId(id, poolParam.getBinStep());
if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
shouldRevert = true;
break;
}
}
}

if (shouldRevert) {
vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
(, array) = addLiquidity(key, poolManager, bob, activeId, amountX, amountY, nbBinX, nbBinY);
return;
}

(, array) = addLiquidity(key, poolManager, bob, activeId, amountX, amountY, nbBinX, nbBinY);

{
// verify X and Y amount
uint256 amtXBalanceDelta = uint256(-int256(vault.balanceDeltaOfPool(poolId).amount0()));
uint256 amountXLeft = amountX - ((amountX * (Constants.PRECISION / nbBinX)) / 1e18) * nbBinX;
assertEq(amountX, amtXBalanceDelta + amountXLeft, "test_MintFuzz::1");

uint256 amtYBalanceDelta = uint256(-int256(vault.balanceDeltaOfPool(poolId).amount1()));
uint256 amountYLeft = amountY - ((amountY * (Constants.PRECISION / nbBinY)) / 1e18) * nbBinY;
assertEq(amountY, amtYBalanceDelta + amountYLeft, "test_MintFUzz::2");
}
{
// verify each binId has the right reserve
uint256 total = getTotalBins(nbBinX, nbBinY);
for (uint256 i; i < total; ++i) {
uint24 id = getId(activeId, i, nbBinY);

(uint128 binReserveX, uint128 binReserveY,,) = poolManager.getBin(poolId, id);

if (id < activeId) {
assertEq(binReserveX, 0, "test_MintFuzz::3");
assertEq(binReserveY, (amountY * (Constants.PRECISION / nbBinY)) / 1e18, "test_MintFuzz::4");
} else if (id == activeId) {
assertApproxEqRel(
binReserveX, (amountX * (Constants.PRECISION / nbBinX)) / 1e18, 1e15, "test_MintFuzz::5"
);
assertApproxEqRel(
binReserveY, (amountY * (Constants.PRECISION / nbBinY)) / 1e18, 1e15, "test_MintFuzz::6"
);
} else {
assertEq(binReserveX, (amountX * (Constants.PRECISION / nbBinX)) / 1e18, "test_MintFuzz::7");
assertEq(binReserveY, 0, "test_MintFuzz::8");
}

assertGt(poolManager.getPosition(poolId, bob, id, 0).share, 0, "test_MintFuzz::9");
}
}
{
uint256 total = getTotalBins(nbBinX, nbBinY);
for (uint256 i; i < total; ++i) {
uint24 id = getId(activeId, i, nbBinY);

// verify id
assertEq(id, array.ids[i]);

// verify amount
(uint128 x, uint128 y) = array.amounts[i].decode();
if (id < activeId) {
assertEq(x, 0);
assertApproxEqRel(y, amountY / 6, 1e15); // approx amount within 0.1%,
} else if (id == activeId) {
assertApproxEqRel(y, amountY / 6, 1e15); // approx amount within 0.1%
assertApproxEqRel(x, amountX / 6, 1e15); // approx amount within 0.1%
} else {
assertApproxEqRel(x, amountX / 6, 1e15); // approx amount within 0.1%
assertEq(y, 0);
}

// verify liquidity minted
assertEq(poolManager.getPosition(poolId, bob, id, 0).share, array.liquidityMinted[i]);
}
}
}

function test_SimpleMintX() external {
poolManager.initialize(key, activeId);

Expand Down
23 changes: 23 additions & 0 deletions test/pool-bin/libraries/BinPoolSwap.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolPa
import {BinTestHelper} from "../helpers/BinTestHelper.sol";
import {IProtocolFeeController} from "../../../src/interfaces/IProtocolFeeController.sol";
import {MockProtocolFeeController} from "../../../src/test/fee/MockProtocolFeeController.sol";
import {Constants} from "../../../src/pool-bin/libraries/Constants.sol";
import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";

contract BinPoolSwapTest is BinTestHelper, GasSnapshot {
Expand Down Expand Up @@ -269,6 +270,28 @@ contract BinPoolSwapTest is BinTestHelper, GasSnapshot {
poolManager.swap(key, false, -int128(amountIn), "0x");
}

function test_revert_swapMaxLiquidityPerBinfuzz(int128 amountSpecified) external {
vm.assume(amountSpecified != 0);

// Add liquidity to the point where it is close to the max liquidity per bin
poolManager.initialize(key, activeId);
addLiquidity(
key,
poolManager,
bob,
activeId,
// when price is 1:1, then Constants.MAX_LIQUIDITY_PER_BIN >> 128 / 2 is the threshold
(Constants.MAX_LIQUIDITY_PER_BIN >> 128) / 2,
(Constants.MAX_LIQUIDITY_PER_BIN >> 128) / 2,
1,
1
);

// arbitrary amount of token will trigger the revert
vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
poolManager.swap(key, false, amountSpecified, "0x");
}

function test_revert_SwapOutOfLiquidity() external {
// Add liquidity of 1e18 on each side
poolManager.initialize(key, activeId);
Expand Down
Loading