fix fillOrder approvals to hook contract

Author: mmsaki

src/AsyncSwap.sol

@@ -8,6 +8,7 @@ import { AsyncFiller } from "@async-swap/libraries/AsyncFiller.sol";
 import { AsyncOrder } from "@async-swap/types/AsyncOrder.sol";
 import { CurrencySettler } from "@uniswap/v4-core/test/utils/CurrencySettler.sol";
 import { IPoolManager } from "v4-core/interfaces/IPoolManager.sol";
+import { IERC20Minimal } from "v4-core/interfaces/external/IERC20Minimal.sol";
 import { Hooks } from "v4-core/libraries/Hooks.sol";
 import { LPFeeLibrary } from "v4-core/libraries/LPFeeLibrary.sol";
 import { SafeCast } from "v4-core/libraries/SafeCast.sol";
@@ -131,13 +132,14 @@ contract AsyncSwap is BaseHook, IAsyncSwapAMM {
   }
 
   /// @inheritdoc IAsyncSwapAMM
-  function executeOrder(AsyncOrder calldata order, bytes calldata) external {
+  function executeOrder(AsyncOrder calldata order, bytes calldata fillerData) external {
     address owner = order.owner;
     uint256 amountIn = order.amountIn;
     bool zeroForOne = order.zeroForOne;
     Currency currency0 = order.key.currency0;
     Currency currency1 = order.key.currency1;
     PoolId poolId = order.key.toId();
+    address filler = abi.decode(fillerData, (address));
 
     if (amountIn == 0) revert ZeroFillOrder();
 
@@ -160,15 +162,15 @@ contract AsyncSwap is BaseHook, IAsyncSwapAMM {
     }
 
     asyncOrders[poolId].asyncOrderAmount[owner][zeroForOne] -= amountToFill;
-    /// TODO: check if this is needed, we could just burn
-    poolManager.transfer(owner, currencyTake.toId(), amountToFill);
+    /// we could also burn
+    poolManager.transfer(filler, currencyTake.toId(), amountToFill);
     emit AsyncOrderFilled(poolId, owner, zeroForOne, amountToFill);
 
     /// @dev Take currencyFill from filler
     /// @dev Hook may charge filler a hook fee
     /// TODO: If fee emit HookFee event
-    currencyFill.take(poolManager, address(this), amountToFill, true);
-    currencyFill.settle(poolManager, msg.sender, amountToFill, false); // transfer
+    currencyFill.take(poolManager, owner, amountToFill, true);
+    currencyFill.settle(poolManager, filler, amountToFill, false); // transfer
   }
 
   /// @inheritdoc BaseHook

src/interfaces/IAsyncSwapAMM.sol

@@ -20,7 +20,7 @@ interface IAsyncSwapAMM is IAsyncSwapOrder {
 
   /// @notice Fill an async order in an Async Swap AMM.
   /// @param order The async order to be filled.
-  /// @param userParams Additional data for the user.
+  /// @param userParams data containe adderss of the filler.
   function executeOrder(AsyncOrder calldata order, bytes calldata userParams) external;
 
   /// Fills async orders in batching mode, allowing multiple orders to be executed in a single transaction.

src/interfaces/IRouter.sol

@@ -38,12 +38,12 @@ interface IRouter {
   /// Swaps tokens using an async order.
   /// @param order The async order to be placed.
   /// @param userData Additional data for the user, allowing user to specify an executor.
-  function swap(AsyncOrder calldata order, bytes calldata userData) external;
+  function swap(AsyncOrder calldata order, bytes calldata userData) external payable;
 
   /// Fills an async order.
   /// @param order The async order to be filled.
   /// @param userData Additional data for the user, allowing user to specify an executor.
-  function fillOrder(AsyncOrder calldata order, bytes calldata userData) external;
+  function fillOrder(AsyncOrder calldata order, bytes calldata userData) external payable;
 
   function withdraw(PoolKey memory key, bool zeroForOne, uint256 amount) external;
 

src/router.sol

@@ -53,7 +53,7 @@ contract Router is IRouter {
   }
 
   /// @inheritdoc IRouter
-  function swap(AsyncOrder calldata order, bytes memory userData) external {
+  function swap(AsyncOrder calldata order, bytes memory userData) external payable {
     address onBehalf = address(this);
     IAsyncSwapAMM.UserParams memory userParams = abi.decode(userData, (IAsyncSwapAMM.UserParams));
     require(userParams.executor == address(this), "Use router as your executor!");
@@ -66,7 +66,7 @@ contract Router is IRouter {
   }
 
   /// @inheritdoc IRouter
-  function fillOrder(AsyncOrder calldata order, bytes calldata) external {
+  function fillOrder(AsyncOrder calldata order, bytes calldata) external payable {
     address onBehalf = address(this);
     assembly ("memory-safe") {
       tstore(USER_LOCATION, caller())
@@ -122,10 +122,20 @@ contract Router is IRouter {
     /// @dev FillingOrder
     if (action == 1) {
       SwapCallback memory orderData = abi.decode(data, (SwapCallback));
-      Currency currency = orderData.order.zeroForOne ? orderData.order.key.currency1 : orderData.order.key.currency0;
-      assert(IERC20Minimal(Currency.unwrap(currency)).transferFrom(user, asyncFiller, orderData.order.amountIn));
-      assert(IERC20Minimal(Currency.unwrap(currency)).approve(address(HOOK), orderData.order.amountIn));
-      HOOK.executeOrder(orderData.order, abi.encode(asyncFiller));
+      Currency currencyFill;
+      Currency currencyTake;
+      if (orderData.order.zeroForOne) {
+        currencyFill = orderData.order.key.currency1;
+        currencyTake = orderData.order.key.currency0;
+      } else {
+        currencyFill = orderData.order.key.currency0;
+        currencyTake = orderData.order.key.currency1;
+      }
+      // pay order
+      // assert(IERC20Minimal(Currency.unwrap(currencyFill)).transferFrom(user, address(HOOK),
+      // orderData.order.amountIn));
+      HOOK.executeOrder(orderData.order, abi.encode(user));
+      // currencyTake.take(POOLMANAGER, user, orderData.order.amountIn, false);
     }
 
     /// @notice Handle withdrawals

test/AsyncFillerTest.t.sol

@@ -61,8 +61,8 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token1.approve(address(router), swapAmount);
-    router.fillOrder(fillOrder, abi.encode(address(router)));
+    token1.approve(address(hook), swapAmount);
+    router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
     vm.stopPrank();
 
     // Verify order was filled
@@ -90,9 +90,9 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(nonExecutor);
-    token1.approve(address(router), swapAmount);
+    token1.approve(address(hook), swapAmount);
     vm.expectRevert("Caller is valid not excutor");
-    hook.executeOrder(fillOrder, "");
+    hook.executeOrder(fillOrder, abi.encode(nonExecutor));
     vm.stopPrank();
   }
 
@@ -115,8 +115,8 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: fillAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token1.approve(address(router), fillAmount);
-    router.fillOrder(fillOrder, abi.encode(address(router)));
+    token1.approve(address(hook), fillAmount);
+    router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
     vm.stopPrank();
 
     // Verify partial fill
@@ -142,9 +142,9 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: excessFillAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token1.approve(address(router), excessFillAmount);
+    token1.approve(address(hook), excessFillAmount);
     vm.expectRevert("Max fill order limit exceed");
-    router.fillOrder(fillOrder, abi.encode(address(router)));
+    router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
     vm.stopPrank();
   }
 
@@ -172,8 +172,8 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: fillAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token1.approve(address(router), fillAmount);
-    router.fillOrder(fillOrder, abi.encode(address(router)));
+    token1.approve(address(hook), fillAmount);
+    router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
     vm.stopPrank();
 
     // Verify fill
@@ -199,7 +199,7 @@ contract AsyncFillerTest is SetupHook {
 
     vm.startPrank(testExecutor);
     vm.expectRevert("ZeroFillOrder()");
-    hook.executeOrder(fillOrder, "");
+    hook.executeOrder(fillOrder, abi.encode(testExecutor));
     vm.stopPrank();
   }
 
@@ -233,10 +233,10 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser2, zeroForOne: true, amountIn: swapAmount2, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token1.approve(address(router), swapAmount1 + swapAmount2);
+    token1.approve(address(hook), swapAmount1 + swapAmount2);
 
-    router.fillOrder(fillOrder1, abi.encode(address(router)));
-    router.fillOrder(fillOrder2, abi.encode(address(router)));
+    router.fillOrder(fillOrder1, abi.encode(address(testExecutor)));
+    router.fillOrder(fillOrder2, abi.encode(address(testExecutor)));
     vm.stopPrank();
 
     // Verify both orders were filled
@@ -262,8 +262,8 @@ contract AsyncFillerTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: false, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testExecutor);
-    token0.approve(address(router), swapAmount);
-    router.fillOrder(fillOrder, abi.encode(address(router)));
+    token0.approve(address(hook), swapAmount);
+    router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
     vm.stopPrank();
 
     // Verify order was filled

test/AsyncSwapEdgeCases.t.sol

@@ -113,7 +113,7 @@ contract AsyncSwapEdgeCasesTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: 0, sqrtPrice: 2 ** 96 });
 
     vm.expectRevert(AsyncFiller.ZeroFillOrder.selector);
-    hook.executeOrder(order, "");
+    hook.executeOrder(order, abi.encode(address(this)));
   }
 
   function testExecuteOrdersMultiple() public {
@@ -153,8 +153,8 @@ contract AsyncSwapEdgeCasesTest is SetupHook {
         AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: amountPerOrder, sqrtPrice: 2 ** 96 });
 
       vm.startPrank(testExecutor);
-      token1.approve(address(router), amountPerOrder);
-      router.fillOrder(fillOrder, abi.encode(address(router)));
+      token1.approve(address(hook), amountPerOrder);
+      router.fillOrder(fillOrder, abi.encode(address(testExecutor)));
       vm.stopPrank();
     }
 

test/RouterTest.t.sol

@@ -69,8 +69,8 @@ contract RouterTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testUser2);
-    token1.approve(address(router), swapAmount);
-    router.fillOrder(fillOrder, "");
+    token1.approve(address(hook), swapAmount);
+    router.fillOrder(fillOrder, abi.encode(testUser2));
     vm.stopPrank();
 
     // Verify order was filled
@@ -123,8 +123,8 @@ contract RouterTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: false, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testUser2);
-    token0.approve(address(router), swapAmount);
-    router.fillOrder(fillOrder, "");
+    token0.approve(address(hook), swapAmount);
+    router.fillOrder(fillOrder, abi.encode(testUser2));
     vm.stopPrank();
 
     // Verify order was filled
@@ -171,8 +171,8 @@ contract RouterTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount1, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testUser2);
-    token1.approve(address(router), swapAmount1);
-    router.fillOrder(fillOrder, "");
+    token1.approve(address(hook), swapAmount1);
+    router.fillOrder(fillOrder, abi.encode(testUser2));
     vm.stopPrank();
 
     // Verify partial fill
@@ -183,8 +183,8 @@ contract RouterTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount2, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testUser2);
-    token1.approve(address(router), swapAmount2);
-    router.fillOrder(fillOrder2, "");
+    token1.approve(address(hook), swapAmount2);
+    router.fillOrder(fillOrder2, abi.encode(testUser2));
     vm.stopPrank();
 
     // Verify complete fill
@@ -232,14 +232,14 @@ contract RouterTest is SetupHook {
       AsyncOrder({ key: key, owner: testUser, zeroForOne: true, amountIn: swapAmount, sqrtPrice: 2 ** 96 });
 
     vm.startPrank(testUser2);
-    token1.approve(address(router), swapAmount);
-    router.fillOrder(fillOrder, "");
+    token1.approve(address(hook), swapAmount);
+    router.fillOrder(fillOrder, abi.encode(testUser2));
     vm.stopPrank();
 
     // Verify balance changes
     assertEq(token1.balanceOf(testUser2), fillerBalance1Before - swapAmount);
     assertEq(token0.balanceOf(testUser2), fillerBalance0Before); // Should be unchanged
-    assertEq(manager.balanceOf(testUser, currency0.toId()), uint256(swapAmount));
+    assertEq(manager.balanceOf(testUser, currency1.toId()), uint256(swapAmount));
   }
 
   function testFuzzSwapAmounts(uint256 amount, bool zeroForOne) public {