Fix batching in safes

Author: sembrestels

apps/evmcrispr-terminal/src/hooks/useTransactionBatcher.ts

@@ -1,4 +1,4 @@
-import type { TransactionAction } from "@evmcrispr/core";
+import type { BatchedAction } from "@evmcrispr/core";
 import type SafeAppProvider from "@safe-global/safe-apps-sdk";
 import { useCallback } from "react";
 import type { Account, Chain, Transport, WalletClient } from "viem";
@@ -8,18 +8,24 @@ import { config } from "../config/wagmi";
 export function useTransactionBatcher(safeConnector?: any) {
   const executeBatchedActions = useCallback(
     async (
-      actions: TransactionAction[],
+      batch: BatchedAction,
       currentWalletClient: WalletClient<Transport, Chain, Account>,
     ) => {
+      const { actions, chainId } = batch;
       if (actions.length === 0) return;
 
-      const chainId = actions[0].chainId;
+      if (
+        actions.find(
+          (action) =>
+            action.chainId !== undefined && action.chainId !== chainId,
+        )
+      ) {
+        throw new Error("Batch contains transactions for multiple chains");
+      }
 
-      if (chainId !== undefined) {
-        const chain = config.chains.find((c) => c.id === chainId);
-        if (chain) {
-          await currentWalletClient.switchChain({ id: chainId });
-        }
+      const chain = config.chains.find((c) => c.id === chainId);
+      if (chain) {
+        await currentWalletClient.switchChain({ id: chainId });
       }
 
       // Filter out contract deployments (no 'to' address) as they cannot be batched
@@ -69,7 +75,7 @@ export function useTransactionBatcher(safeConnector?: any) {
   );
 
   const executeSafeBatchedActions = useCallback(
-    async (actions: TransactionAction[]) => {
+    async (batch: BatchedAction) => {
       if (!safeConnector)
         throw new Error(
           "Safe connector not available for Safe batched actions.",
@@ -80,8 +86,15 @@ export function useTransactionBatcher(safeConnector?: any) {
       if (!sdk) throw new Error("Safe SDK not available");
 
       const chainId = await safeConnector.getChainId();
+      const { actions } = batch;
 
-      if (actions.find((action) => action.chainId !== chainId)) {
+      if (
+        batch.chainId !== chainId ||
+        actions.find(
+          (action) =>
+            action.chainId !== undefined && action.chainId !== chainId,
+        )
+      ) {
         throw new Error("Safe does not support switching chains");
       }
 

apps/evmcrispr-terminal/src/hooks/useTransactionExecutor.ts

@@ -126,9 +126,9 @@ export function useTransactionExecutor(
               `Executing batch of ${action.actions.length} transactions from ${truncateAddress(action.from)}`,
             );
             if (safeConnector) {
-              await executeSafeBatchedActions(action.actions);
+              await executeSafeBatchedActions(action);
             } else {
-              return await executeBatchedActions(action.actions, walletClient);
+              return await executeBatchedActions(action, walletClient);
             }
             break;
           }

apps/evmcrispr-terminal/test/hooks/useTransactionBatcher.test.tsx

@@ -0,0 +1,58 @@
+import { describe, expect, mock, test } from "bun:test";
+import type { BatchedAction } from "@evmcrispr/core";
+import { renderHook } from "@testing-library/react";
+import { useTransactionBatcher } from "../../src/hooks/useTransactionBatcher";
+
+const batch: BatchedAction = {
+  type: "batched",
+  chainId: 100,
+  from: "0x0000000000000000000000000000000000000001",
+  actions: [
+    {
+      to: "0x0000000000000000000000000000000000000002",
+      data: "0x1234",
+      value: 1n,
+    },
+  ],
+};
+
+describe("useTransactionBatcher", () => {
+  test("submits Safe batches using the outer batch chain", async () => {
+    const send = mock(async () => undefined);
+    const safeConnector = {
+      getChainId: mock(async () => 100),
+      getProvider: mock(async () => ({
+        sdk: { txs: { send } },
+      })),
+    };
+
+    const { result } = renderHook(() => useTransactionBatcher(safeConnector));
+
+    await result.current.executeSafeBatchedActions(batch);
+
+    expect(send).toHaveBeenCalledWith({
+      txs: [
+        {
+          to: "0x0000000000000000000000000000000000000002",
+          data: "0x1234",
+          value: "1",
+        },
+      ],
+    });
+  });
+
+  test("rejects Safe batches targeting a different chain", async () => {
+    const safeConnector = {
+      getChainId: mock(async () => 1),
+      getProvider: mock(async () => ({
+        sdk: { txs: { send: mock(async () => undefined) } },
+      })),
+    };
+
+    const { result } = renderHook(() => useTransactionBatcher(safeConnector));
+
+    await expect(
+      result.current.executeSafeBatchedActions(batch),
+    ).rejects.toThrow("Safe does not support switching chains");
+  });
+});