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

Part 1: Improve forge tests #407

Merged
merged 19 commits into from
Nov 16, 2023
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
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_oneForZero_exactInCapped.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2222
2228
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_oneForZero_exactInPartial.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3050
3058
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1980
1986
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3050
3058
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2211
2217
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3201
3215
2 changes: 1 addition & 1 deletion .forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1969
1975
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3201
3215
Original file line number Diff line number Diff line change
@@ -1 +1 @@
172021
189798
2 changes: 1 addition & 1 deletion .forge-snapshots/cached dynamic fee, no hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
158656
145314
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 @@
76947
134915
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 @@
134745
186636
2 changes: 1 addition & 1 deletion .forge-snapshots/erc20 collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
25033
27033
Original file line number Diff line number Diff line change
@@ -1 +1 @@
517
523
Original file line number Diff line number Diff line change
@@ -1 +1 @@
668
674
Original file line number Diff line number Diff line change
@@ -1 +1 @@
598
600
Original file line number Diff line number Diff line change
@@ -1 +1 @@
784
786
Original file line number Diff line number Diff line change
@@ -1 +1 @@
882
890
Original file line number Diff line number Diff line change
@@ -1 +1 @@
542
473
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
38101
53843
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
264055
319184
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
232085
198926
2 changes: 1 addition & 1 deletion .forge-snapshots/mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
250721
202041
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
36699
38699
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24630
24805
1 change: 1 addition & 0 deletions .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
190722
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129248
202425
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106413
121318
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
91101
109143
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn claim for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
107809
127882
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as claim.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147831
212594
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 @@
171256
189038
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
91079
109122
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 @@
177851
195628
320 changes: 160 additions & 160 deletions .gas-snapshot

Large diffs are not rendered by default.

12 changes: 8 additions & 4 deletions src/libraries/SafeCast.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,37 @@ pragma solidity ^0.8.20;
/// @title Safe casting methods
/// @notice Contains methods for safely casting between types
library SafeCast {
error SafeCastOverflow();
hensha256 marked this conversation as resolved.
Show resolved Hide resolved

/// @notice Cast a uint256 to a uint160, revert on overflow
/// @param y The uint256 to be downcasted
/// @return z The downcasted integer, now type uint160
function toUint160(uint256 y) internal pure returns (uint160 z) {
require((z = uint160(y)) == y);
z = uint160(y);
if (z != y) revert SafeCastOverflow();
}

/// @notice Cast a int256 to a int128, revert on overflow or underflow
/// @param y The int256 to be downcasted
/// @return z The downcasted integer, now type int128
function toInt128(int256 y) internal pure returns (int128 z) {
require((z = int128(y)) == y);
z = int128(y);
if (z != y) revert SafeCastOverflow();
}

/// @notice Cast a uint256 to a int256, revert on overflow
/// @param y The uint256 to be casted
/// @return z The casted integer, now type int256
function toInt256(uint256 y) internal pure returns (int256 z) {
require(y <= uint256(type(int256).max));
if (y > uint256(type(int256).max)) revert SafeCastOverflow();
z = int256(y);
}

/// @notice Cast a uint256 to a int128, revert on overflow
/// @param y The uint256 to be downcasted
/// @return z The downcasted integer, now type int128
function toInt128(uint256 y) internal pure returns (int128 z) {
require(y <= uint128(type(int128).max));
if (y > uint128(type(int128).max)) revert SafeCastOverflow();
z = int128(int256(y));
}
}
27 changes: 16 additions & 11 deletions src/libraries/SqrtPriceMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import {FixedPoint96} from "./FixedPoint96.sol";
library SqrtPriceMath {
using SafeCast for uint256;

error InvalidPriceOrLiquidity();
error InvalidPrice();
error NotEnoughLiquidity();
error PriceOverflow();

/// @notice Gets the next sqrt price given a delta of currency0
/// @dev Always rounds up, because in the exact output case (increasing price) we need to move the price at least
/// far enough to get the desired output amount, and in the exact input case (decreasing price) we need to move the
Expand All @@ -34,8 +39,8 @@ library SqrtPriceMath {

if (add) {
unchecked {
uint256 product;
if ((product = amount * sqrtPX96) / amount == sqrtPX96) {
uint256 product = amount * sqrtPX96;
if (product / amount == sqrtPX96) {
uint256 denominator = numerator1 + product;
if (denominator >= numerator1) {
// always fits in 160 bits
Expand All @@ -47,10 +52,10 @@ library SqrtPriceMath {
return uint160(UnsafeMath.divRoundingUp(numerator1, (numerator1 / sqrtPX96) + amount));
} else {
unchecked {
uint256 product;
uint256 product = amount * sqrtPX96;
// if the product overflows, we know the denominator underflows
// in addition, we must check that the denominator does not underflow
require((product = amount * sqrtPX96) / amount == sqrtPX96 && numerator1 > product);
if (product / amount != sqrtPX96 || numerator1 <= product) revert PriceOverflow();
Copy link
Contributor

Choose a reason for hiding this comment

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

any concerns if amount is 0 for div?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dont think so - thats in the V3 core code too

uint256 denominator = numerator1 - product;
return FullMath.mulDivRoundingUp(numerator1, sqrtPX96, denominator).toUint160();
}
Expand Down Expand Up @@ -89,9 +94,11 @@ library SqrtPriceMath {
: FullMath.mulDivRoundingUp(amount, FixedPoint96.Q96, liquidity)
);

require(sqrtPX96 > quotient);
if (sqrtPX96 <= quotient) revert NotEnoughLiquidity();
// always fits 160 bits
return uint160(sqrtPX96 - quotient);
unchecked {
return uint160(sqrtPX96 - quotient);
}
}
}

Expand All @@ -107,8 +114,7 @@ library SqrtPriceMath {
pure
returns (uint160 sqrtQX96)
{
require(sqrtPX96 > 0);
require(liquidity > 0);
if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();

// round to make sure that we don't pass the target price
return zeroForOne
Expand All @@ -128,8 +134,7 @@ library SqrtPriceMath {
pure
returns (uint160 sqrtQX96)
{
require(sqrtPX96 > 0);
require(liquidity > 0);
if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();

// round to make sure that we pass the target price
return zeroForOne
Expand All @@ -156,7 +161,7 @@ library SqrtPriceMath {
uint256 numerator1 = uint256(liquidity) << FixedPoint96.RESOLUTION;
uint256 numerator2 = sqrtRatioBX96 - sqrtRatioAX96;

require(sqrtRatioAX96 > 0);
if (sqrtRatioAX96 == 0) revert InvalidPrice();

return roundUp
? UnsafeMath.divRoundingUp(FullMath.mulDivRoundingUp(numerator1, numerator2, sqrtRatioBX96), sqrtRatioAX96)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {PoolKey} from "../types/PoolKey.sol";
import {IPoolManager} from "../interfaces/IPoolManager.sol";
import {IHooks} from "../interfaces/IHooks.sol";

contract DynamicFeesTest is BaseTestHooks, IDynamicFeeManager {
contract DynamicFeesTestHook is BaseTestHooks, IDynamicFeeManager {
uint24 internal fee;
IPoolManager manager;

Expand Down
50 changes: 20 additions & 30 deletions src/test/PoolDonateTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,16 @@
pragma solidity ^0.8.20;

import {Currency, CurrencyLibrary} from "../types/Currency.sol";
import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";

import {Currency} from "../types/Currency.sol";
import {ILockCallback} from "../interfaces/callback/ILockCallback.sol";
import {IPoolManager} from "../interfaces/IPoolManager.sol";
import {PoolKey} from "../types/PoolKey.sol";
import {BalanceDelta} from "../types/BalanceDelta.sol";
import {PoolTestBase} from "./PoolTestBase.sol";
import {Test} from "forge-std/Test.sol";

contract PoolDonateTest is ILockCallback {
contract PoolDonateTest is PoolTestBase, Test {
using CurrencyLibrary for Currency;

IPoolManager public immutable manager;

constructor(IPoolManager _manager) {
manager = _manager;
}
constructor(IPoolManager _manager) PoolTestBase(_manager) {}

struct CallbackData {
address sender;
Expand Down Expand Up @@ -47,28 +41,24 @@ contract PoolDonateTest is ILockCallback {

CallbackData memory data = abi.decode(rawData, (CallbackData));

(,, uint256 reserveBefore0, int256 deltaBefore0) = _fetchBalances(data.key.currency0, data.sender);
(,, uint256 reserveBefore1, int256 deltaBefore1) = _fetchBalances(data.key.currency1, data.sender);

assertEq(deltaBefore0, 0);
assertEq(deltaBefore1, 0);

BalanceDelta delta = manager.donate(data.key, data.amount0, data.amount1, data.hookData);

if (delta.amount0() > 0) {
if (data.key.currency0.isNative()) {
manager.settle{value: uint128(delta.amount0())}(data.key.currency0);
} else {
IERC20Minimal(Currency.unwrap(data.key.currency0)).transferFrom(
data.sender, address(manager), uint128(delta.amount0())
);
manager.settle(data.key.currency0);
}
}
if (delta.amount1() > 0) {
if (data.key.currency1.isNative()) {
manager.settle{value: uint128(delta.amount1())}(data.key.currency1);
} else {
IERC20Minimal(Currency.unwrap(data.key.currency1)).transferFrom(
data.sender, address(manager), uint128(delta.amount1())
);
manager.settle(data.key.currency1);
}
}
(,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender);
(,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender);

assertEq(reserveBefore0, reserveAfter0);
assertEq(reserveBefore1, reserveAfter1);
assertEq(deltaAfter0, int256(data.amount0));
assertEq(deltaAfter1, int256(data.amount1));

if (data.amount0 > 0) _settle(data.key.currency0, data.sender, delta.amount0(), true);
if (data.amount1 > 0) _settle(data.key.currency1, data.sender, delta.amount1(), true);

return abi.encode(delta);
}
Expand Down
Loading