Improve forge tests

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

@@ -1 +1 @@
-85384
+197590

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

@@ -1 +1 @@
-82539
+184225

.forge-snapshots/simple swap.snap

@@ -1 +1 @@
-49771
+151412

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

@@ -1 +1 @@
-125033
+125053

.forge-snapshots/swap against liquidity.snap

@@ -1 +1 @@
-109721
+109741

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

@@ -1 +1 @@
-169533
+169553

.forge-snapshots/swap with 1155 as input.snap

@@ -1 +1 @@
-155963
+155983

.forge-snapshots/swap with dynamic fee.snap

@@ -1 +1 @@
-84493
+196825

.forge-snapshots/swap with hooks.snap

@@ -1 +1 @@
-49742
+109719

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

@@ -1 +1 @@
-91231
+203442

src/test/PoolSwapTest.sol

@@ -12,6 +12,8 @@ import {PoolKey} from "../types/PoolKey.sol";
 contract PoolSwapTest is ILockCallback {
     using CurrencyLibrary for Currency;
 
+    error NoSwapOccurred();
+
     IPoolManager public immutable manager;
 
     constructor(IPoolManager _manager) {
@@ -52,6 +54,9 @@ contract PoolSwapTest is ILockCallback {
 
         BalanceDelta delta = manager.swap(data.key, data.params, data.hookData);
 
+        // Make sure youve added liquidity to the test pool!
+        if (BalanceDelta.unwrap(delta) == 0) revert NoSwapOccurred();
+
         if (data.params.zeroForOne) {
             if (delta.amount0() > 0) {
                 if (data.testSettings.settleUsingTransfer) {

test/DynamicFees.t.sol

@@ -16,6 +16,9 @@ import {Deployers} from "./utils/Deployers.sol";
 import {IDynamicFeeManager} from "././../src/interfaces/IDynamicFeeManager.sol";
 import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
 import {DynamicFeesTest} from "../src/test/DynamicFeesTest.sol";
+import {PoolModifyPositionTest} from "../src/test/PoolModifyPositionTest.sol";
+import {Currency, CurrencyLibrary} from "../src/types/Currency.sol";
+import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
 
 contract TestDynamicFees is Test, Deployers, GasSnapshot {
     using PoolIdLibrary for PoolKey;
@@ -46,23 +49,45 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
     PoolKey key;
     PoolKey key2;
     PoolSwapTest swapRouter;
+    PoolModifyPositionTest modifyPositionRouter;
 
     function setUp() public {
         DynamicFeesTest impl = new DynamicFeesTest();
         vm.etch(address(dynamicFees), address(impl).code);
         vm.etch(address(dynamicFeesNoHook), address(impl).code);
 
         (manager, key,) =
-            Deployers.createFreshPool(IHooks(address(dynamicFees)), FeeLibrary.DYNAMIC_FEE_FLAG, SQRT_RATIO_1_1);
+            Deployers.createAndInitFreshPool(IHooks(address(dynamicFees)), FeeLibrary.DYNAMIC_FEE_FLAG, SQRT_RATIO_1_1);
         dynamicFees.setManager(IPoolManager(manager));
 
-        PoolId id2;
-        (key2, id2) = Deployers.createPool(
+        (key2,) = Deployers.createAndInitPool(
             manager, IHooks(address(dynamicFeesNoHook)), FeeLibrary.DYNAMIC_FEE_FLAG, SQRT_RATIO_1_1
         );
         dynamicFeesNoHook.setManager(IPoolManager(manager));
 
         swapRouter = new PoolSwapTest(manager);
+        modifyPositionRouter = new PoolModifyPositionTest(manager);
+
+        MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ether);
+        MockERC20(Currency.unwrap(key.currency1)).mint(address(this), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency0)).mint(address(this), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency1)).mint(address(this), 10 ether);
+
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ether);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(swapRouter), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency0)).approve(address(swapRouter), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency1)).approve(address(swapRouter), 10 ether);
+
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ether);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(modifyPositionRouter), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency0)).approve(address(modifyPositionRouter), 10 ether);
+        MockERC20(Currency.unwrap(key2.currency1)).approve(address(modifyPositionRouter), 10 ether);
+
+        // add liquidity for the 2 new pools
+        IPoolManager.ModifyPositionParams memory liqParams =
+            IPoolManager.ModifyPositionParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18});
+        modifyPositionRouter.modifyPosition(key, liqParams, ZERO_BYTES);
+        modifyPositionRouter.modifyPosition(key2, liqParams, ZERO_BYTES);
     }
 
     function testPoolInitializeFailsWithTooLargeFee() public {
@@ -72,7 +97,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
         manager.initialize(key0, SQRT_RATIO_1_1, ZERO_BYTES);
     }
 
-    function testSwapFailsWithTooLargeFee() public {
+    function testUpdateFailsWithTooLargeFee() public {
         dynamicFees.setFee(1000000);
         vm.expectRevert(IFees.FeeTooLarge.selector);
         manager.updateDynamicSwapFee(key);
@@ -92,62 +117,76 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
     function testSwapWorks() public {
         dynamicFees.setFee(123);
         manager.updateDynamicSwapFee(key);
+
+        IPoolManager.SwapParams memory params =
+            IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2});
+        PoolSwapTest.TestSettings memory testSettings =
+            PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true});
+
         vm.expectEmit(true, true, true, true, address(manager));
-        emit Swap(key.toId(), address(swapRouter), 0, 0, SQRT_RATIO_1_1 + 1, 0, 0, 123);
+        emit Swap(key.toId(), address(swapRouter), 100, -98, 79228162514264329749955861424, 1e18, -1, 123);
+
         snapStart("swap with dynamic fee");
-        swapRouter.swap(
-            key,
-            IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1),
-            PoolSwapTest.TestSettings(false, false),
-            ZERO_BYTES
-        );
+        swapRouter.swap(key, params, testSettings, ZERO_BYTES);
         snapEnd();
     }
 
     function testCacheDynamicFeeAndSwap() public {
         dynamicFees.setFee(123);
         manager.updateDynamicSwapFee(key);
+
+        IPoolManager.SwapParams memory params =
+            IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2});
+        PoolSwapTest.TestSettings memory testSettings =
+            PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true});
+
         vm.expectEmit(true, true, true, true, address(manager));
-        emit Swap(key.toId(), address(swapRouter), 0, 0, SQRT_RATIO_1_1 + 1, 0, 0, 456);
-        snapStart("update dynamic fee in before swap");
+        emit Swap(key.toId(), address(swapRouter), 100, -98, 79228162514264329749955861424, 1e18, -1, 456);
         bytes memory data = abi.encode(true, uint24(456));
-        swapRouter.swap(
-            key, IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1), PoolSwapTest.TestSettings(false, false), data
-        );
+
+        snapStart("update dynamic fee in before swap");
+        swapRouter.swap(key, params, testSettings, data);
         snapEnd();
     }
 
     function testDynamicFeeAndBeforeSwapHook() public {
         dynamicFees.setFee(123);
         manager.updateDynamicSwapFee(key);
+
+        IPoolManager.SwapParams memory params =
+            IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2});
+        PoolSwapTest.TestSettings memory testSettings =
+            PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true});
+
         vm.expectEmit(true, true, true, true, address(manager));
-        emit Swap(key.toId(), address(swapRouter), 0, 0, SQRT_RATIO_1_1 + 1, 0, 0, 123);
-        snapStart("before swap hook, already cached dynamic fee");
+        emit Swap(key.toId(), address(swapRouter), 100, -98, 79228162514264329749955861424, 1e18, -1, 123);
         bytes memory data = abi.encode(false, uint24(0));
-        swapRouter.swap(
-            key, IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1), PoolSwapTest.TestSettings(false, false), data
-        );
+
+        snapStart("before swap hook, already cached dynamic fee");
+        swapRouter.swap(key, params, testSettings, data);
         snapEnd();
     }
 
     function testUpdateRevertsOnStaticFeePool() public {
-        (PoolKey memory staticPoolKey,) = Deployers.createPool(manager, IHooks(address(0)), 3000, SQRT_RATIO_1_1);
+        (PoolKey memory staticPoolKey,) = Deployers.createAndInitPool(manager, IHooks(address(0)), 3000, SQRT_RATIO_1_1);
         vm.expectRevert(IFees.FeeNotDynamic.selector);
         manager.updateDynamicSwapFee(staticPoolKey);
     }
 
     function testDynamicFeesCacheNoOtherHooks() public {
         dynamicFeesNoHook.setFee(123);
         manager.updateDynamicSwapFee(key2);
+
+        IPoolManager.SwapParams memory params =
+            IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2});
+        PoolSwapTest.TestSettings memory testSettings =
+            PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true});
+
         vm.expectEmit(true, true, true, true, address(manager));
-        emit Swap(key2.toId(), address(swapRouter), 0, 0, SQRT_RATIO_1_1 + 1, 0, 0, 123);
+        emit Swap(key.toId(), address(swapRouter), 100, -99, 79228162514264329670727698910, 1e18, -1, 0);
+
         snapStart("cached dynamic fee, no hooks");
-        swapRouter.swap(
-            key2,
-            IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1),
-            PoolSwapTest.TestSettings(false, false),
-            ZERO_BYTES
-        );
+        swapRouter.swap(key, params, testSettings, ZERO_BYTES);
         snapEnd();
     }
 }

test/Hooks.t.sol

@@ -11,7 +11,6 @@ import {IPoolManager} from "../src/interfaces/IPoolManager.sol";
 import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
 import {IHooks} from "../src/interfaces/IHooks.sol";
 import {Currency} from "../src/types/Currency.sol";
-import {IERC20Minimal} from "../src/interfaces/external/IERC20Minimal.sol";
 import {PoolManager} from "../src/PoolManager.sol";
 import {PoolModifyPositionTest} from "../src/test/PoolModifyPositionTest.sol";
 import {PoolSwapTest} from "../src/test/PoolSwapTest.sol";
@@ -34,14 +33,15 @@ contract HooksTest is Test, Deployers, GasSnapshot {
         MockHooks impl = new MockHooks();
         vm.etch(ALL_HOOKS_ADDRESS, address(impl).code);
         mockHooks = MockHooks(ALL_HOOKS_ADDRESS);
-        (manager, key,) = Deployers.createFreshPool(mockHooks, 3000, SQRT_RATIO_1_1);
+        (manager, key,) = Deployers.createAndInitFreshPool(mockHooks, 3000, SQRT_RATIO_1_1);
         modifyPositionRouter = new PoolModifyPositionTest(IPoolManager(address(manager)));
         swapRouter = new PoolSwapTest(IPoolManager(address(manager)));
         donateRouter = new PoolDonateTest(IPoolManager(address(manager)));
     }
 
     function testInitializeSucceedsWithHook() public {
-        (PoolManager _manager,, PoolId id) = Deployers.createFreshPool(mockHooks, 3000, SQRT_RATIO_1_1, new bytes(123));
+        (PoolManager _manager,, PoolId id) =
+            Deployers.createAndInitFreshPool(mockHooks, 3000, SQRT_RATIO_1_1, new bytes(123));
         (uint160 sqrtPriceX96,,,) = _manager.getSlot0(id);
         assertEq(sqrtPriceX96, SQRT_RATIO_1_1);
         assertEq(mockHooks.beforeInitializeData(), new bytes(123));
@@ -66,7 +66,7 @@ contract HooksTest is Test, Deployers, GasSnapshot {
 
     function testModifyPositionSucceedsWithHook() public {
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
         modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100), new bytes(111));
         assertEq(mockHooks.beforeModifyPositionData(), new bytes(111));
         assertEq(mockHooks.afterModifyPositionData(), new bytes(111));
@@ -75,36 +75,46 @@ contract HooksTest is Test, Deployers, GasSnapshot {
     function testBeforeModifyPositionInvalidReturn() public {
         mockHooks.setReturnValue(mockHooks.beforeModifyPosition.selector, bytes4(0xdeadbeef));
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100), ZERO_BYTES);
     }
 
     function testAfterModifyPositionInvalidReturn() public {
         mockHooks.setReturnValue(mockHooks.afterModifyPosition.selector, bytes4(0xdeadbeef));
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100), ZERO_BYTES);
     }
 
     function testSwapSucceedsWithHook() public {
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
-        swapRouter.swap(
-            key,
-            IPoolManager.SwapParams(false, 100, SQRT_RATIO_1_1 + 60),
-            PoolSwapTest.TestSettings(false, false),
-            new bytes(222)
-        );
+        MockERC20(Currency.unwrap(key.currency1)).mint(address(this), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(swapRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(modifyPositionRouter), 10 ** 18);
+
+        IPoolManager.ModifyPositionParams memory liqParams =
+            IPoolManager.ModifyPositionParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18});
+
+        IPoolManager.SwapParams memory swapParams =
+            IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2});
+
+        PoolSwapTest.TestSettings memory testSettings =
+            PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true});
+
+        modifyPositionRouter.modifyPosition(key, liqParams, new bytes(111));
+        swapRouter.swap(key, swapParams, testSettings, new bytes(222));
         assertEq(mockHooks.beforeSwapData(), new bytes(222));
         assertEq(mockHooks.afterSwapData(), new bytes(222));
     }
 
     function testBeforeSwapInvalidReturn() public {
         mockHooks.setReturnValue(mockHooks.beforeSwap.selector, bytes4(0xdeadbeef));
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         swapRouter.swap(
             key,
@@ -117,7 +127,7 @@ contract HooksTest is Test, Deployers, GasSnapshot {
     function testAfterSwapInvalidReturn() public {
         mockHooks.setReturnValue(mockHooks.afterSwap.selector, bytes4(0xdeadbeef));
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         swapRouter.swap(
             key,
@@ -130,8 +140,8 @@ contract HooksTest is Test, Deployers, GasSnapshot {
     function testDonateSucceedsWithHook() public {
         addLiquidity(0, 60, 100);
 
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
-        IERC20Minimal(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
         donateRouter.donate(key, 100, 200, new bytes(333));
         assertEq(mockHooks.beforeDonateData(), new bytes(333));
         assertEq(mockHooks.afterDonateData(), new bytes(333));
@@ -141,8 +151,8 @@ contract HooksTest is Test, Deployers, GasSnapshot {
         mockHooks.setReturnValue(mockHooks.beforeDonate.selector, bytes4(0xdeadbeef));
         addLiquidity(0, 60, 100);
 
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
-        IERC20Minimal(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         donateRouter.donate(key, 100, 200, ZERO_BYTES);
     }
@@ -151,8 +161,8 @@ contract HooksTest is Test, Deployers, GasSnapshot {
         mockHooks.setReturnValue(mockHooks.beforeDonate.selector, bytes4(0xdeadbeef));
         addLiquidity(0, 60, 100);
 
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
-        IERC20Minimal(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(donateRouter), 100);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(donateRouter), 200);
         vm.expectRevert(Hooks.InvalidHookResponse.selector);
         donateRouter.donate(key, 100, 200, ZERO_BYTES);
     }
@@ -635,8 +645,8 @@ contract HooksTest is Test, Deployers, GasSnapshot {
     function addLiquidity(int24 tickLower, int24 tickUpper, int256 amount) internal {
         MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
         MockERC20(Currency.unwrap(key.currency1)).mint(address(this), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
-        IERC20Minimal(Currency.unwrap(key.currency1)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
+        MockERC20(Currency.unwrap(key.currency1)).approve(address(modifyPositionRouter), 10 ** 18);
         modifyPositionRouter.modifyPosition(
             key, IPoolManager.ModifyPositionParams(tickLower, tickUpper, amount), ZERO_BYTES
         );