Part 1: Improve forge tests (Uniswap#407)

* Revert messages

* Common take and settle contract for tests

* improving swap and take tests

* add asserts for modify position

* extra asserts in modify position

* asserts in donate test

* More deployment helpers

* native set up in deployer

* simplify initialize tests

* few more corrections

* more cleanup

* remove console logs

* snapshots and test fix post merge

* snapshots

* accidentally pushed foundry.toml

* PR comments

.forge-snapshots/SwapMath_oneForZero_exactInCapped.snap

@@ -1 +1 @@
-2222
+2228

.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap

@@ -1 +1 @@
-3050
+3058

.forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap

@@ -1 +1 @@
-1980
+1986

.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap

@@ -1 +1 @@
-3050
+3058

.forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap

@@ -1 +1 @@
-2211
+2217

.forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap

@@ -1 +1 @@
-3201
+3215

.forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap

@@ -1 +1 @@
-1969
+1975

.forge-snapshots/SwapMath_zeroForOne_exactOutPartial.snap

@@ -1 +1 @@
-3201
+3215

.forge-snapshots/before swap hook, already cached dynamic fee.snap

@@ -1 +1 @@
-172021
+189798

.forge-snapshots/cached dynamic fee, no hooks.snap

@@ -1 +1 @@
-158656
+145314

.forge-snapshots/donate gas with 1 token.snap

@@ -1 +1 @@
-76947
+134915

.forge-snapshots/donate gas with 2 tokens.snap

@@ -1 +1 @@
-134745
+186636

.forge-snapshots/erc20 collect protocol fees.snap

@@ -1 +1 @@
-25033
+27033

.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsFalse.snap

@@ -1 +1 @@
-517
+523

.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsTrue.snap

@@ -1 +1 @@
-668
+674

.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap

@@ -1 +1 @@
-598
+600

.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsTrueGas.snap

@@ -1 +1 @@
-784
+786

.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap

@@ -1 +1 @@
-882
+890

.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsTrueGas.snap

@@ -1 +1 @@
-542
+473

.forge-snapshots/initialize.snap

@@ -1 +1 @@
-38101
+53843

.forge-snapshots/mint with empty hook.snap

@@ -1 +1 @@
-264055
+319184

.forge-snapshots/mint with native token.snap

@@ -1 +1 @@
-232085
+198926

.forge-snapshots/mint.snap

@@ -1 +1 @@
-250721
+202041

.forge-snapshots/native collect protocol fees.snap

@@ -1 +1 @@
-36699
+38699

.forge-snapshots/poolManager bytecode size.snap

@@ -1 +1 @@
-24630
+24805

.forge-snapshots/simple swap with native.snap

@@ -0,0 +1 @@
+190722

.forge-snapshots/simple swap.snap

@@ -1 +1 @@
-129248
+202425

.forge-snapshots/swap against liquidity with native token.snap

@@ -1 +1 @@
-106413
+121318

.forge-snapshots/swap against liquidity.snap

@@ -1 +1 @@
-91101
+109143

.forge-snapshots/swap burn claim for input.snap

@@ -1 +1 @@
-107809
+127882

.forge-snapshots/swap mint output as claim.snap

@@ -1 +1 @@
-147831
+212594

.forge-snapshots/swap with dynamic fee.snap

@@ -1 +1 @@
-171256
+189038

.forge-snapshots/swap with hooks.snap

@@ -1 +1 @@
-91079
+109122

.forge-snapshots/update dynamic fee in before swap.snap

@@ -1 +1 @@
-177851
+195628

src/libraries/SafeCast.sol

@@ -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();
+
     /// @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));
     }
 }

src/libraries/SqrtPriceMath.sol

@@ -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
@@ -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
@@ -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();
                 uint256 denominator = numerator1 - product;
                 return FullMath.mulDivRoundingUp(numerator1, sqrtPX96, denominator).toUint160();
             }
@@ -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);
+            }
         }
     }
 
@@ -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
@@ -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
@@ -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)

src/test/DynamicFeesTest.sol → src/test/DynamicFeesTestHook.sol

@@ -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;
 

src/test/PoolDonateTest.sol

@@ -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;
@@ -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);
     }