fix: fix paymaster override for multisig plugin (#676)

* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test

packages/alchemy/e2e-tests/multisig-account.e2e.test.ts

@@ -166,6 +166,62 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
     await expect(txnHash).resolves.not.toThrowError();
   }, 100000);
 
+  it("should successfully execute 3/3", async () => {
+    const higherThreshold = 3n;
+
+    const provider1 = await givenConnectedProvider({
+      signer: signer1,
+      chain,
+      owners,
+      threshold: higherThreshold,
+    });
+    const provider2 = await givenConnectedProvider({
+      signer: signer2,
+      chain,
+      owners,
+      threshold: higherThreshold,
+    });
+    const provider3 = await givenConnectedProvider({
+      signer: signer3,
+      chain,
+      owners,
+      threshold: higherThreshold,
+    });
+
+    const {
+      account: { address },
+    } = provider1;
+    expect(address).toBe("0xDAcFC8de3c63579BA8aF72a0b73262a85c176b3F");
+
+    const { request, signatureObj: signature1 } =
+      await provider1.proposeUserOperation({
+        uo: {
+          target: provider1.getAddress(),
+          data: "0x",
+        },
+      });
+
+    const { aggregatedSignature, signatureObj: signature2 } =
+      await provider2.signMultisigUserOperation({
+        account: provider2.account,
+        signatures: [signature1],
+        userOperationRequest: request,
+      });
+
+    const result = await provider3.sendUserOperation({
+      uo: request.callData,
+      context: {
+        aggregatedSignature,
+        signatures: [signature1, signature2],
+        userOpSignatureType: "ACTUAL",
+      },
+    });
+
+    const txnHash = provider2.waitForUserOperationTransaction(result);
+
+    await expect(txnHash).resolves.not.toThrowError();
+  }, 100000);
+
   it("should successfully execute 3/3 with alchemy paymaster info", async () => {
     const higherThreshold = 3n;
 
@@ -174,6 +230,9 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
       chain,
       owners,
       threshold: higherThreshold,
+      gasManagerConfig: {
+        policyId: PAYMASTER_POLICY_ID,
+      },
     });
     const provider2 = await givenConnectedProvider({
       signer: signer2,
@@ -207,13 +266,13 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
         },
       });
 
-    const { aggregatedSignature, signatureObj } =
+    const { aggregatedSignature, signatureObj: signature2 } =
       await provider2.signMultisigUserOperation({
         account: provider2.account,
         signatures: [signature1],
         userOperationRequest: request,
       });
-
+ 
     const result = await provider3.sendUserOperation({
       uo: request.callData,
       overrides: {
@@ -223,10 +282,12 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
         maxFeePerGas: request.maxFeePerGas,
         maxPriorityFeePerGas: request.maxPriorityFeePerGas,
         nonceKey: fromHex(`0x${pad(request.nonce).slice(2, 26)}`, "bigint"), // Nonce key is the first 24 bytes of the nonce
+        // @ts-ignore
+        paymasterAndData: request.paymasterAndData,
       },
       context: {
         aggregatedSignature,
-        signatures: [signatureObj],
+        signatures: [signature1, signature2],
         userOpSignatureType: "ACTUAL",
       },
     });
@@ -237,6 +298,7 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
   }, 100000);
 });
 
+
 const givenConnectedProvider = async ({
   signer,
   chain,

packages/alchemy/src/middleware/gasManager.ts

@@ -9,7 +9,7 @@ import type {
   UserOperationRequest,
 } from "@alchemy/aa-core";
 import {
-  bypassPaymasterAndData,
+  bypassPaymasterAndDataEmptyHex,
   deepHexlify,
   defaultGasEstimator,
   filterUndefined,
@@ -197,7 +197,7 @@ export function alchemyGasManagerMiddleware<C extends ClientWithAlchemyMethods>(
       : async (struct, { overrides, account, feeOptions }) => {
           // if user is bypassing paymaster to fallback to having the account to pay the gas (one-off override),
           // we cannot delegate gas estimation to the bundler because paymaster middleware will not be called
-          if (bypassPaymasterAndData(overrides)) {
+          if (bypassPaymasterAndDataEmptyHex(overrides)) {
             return {
               ...struct,
               ...(await fallbackGasEstimator(struct, {
@@ -220,7 +220,7 @@ export function alchemyGasManagerMiddleware<C extends ClientWithAlchemyMethods>(
 
           // if user is bypassing paymaster to fallback to having the account to pay the gas (one-off override),
           // we cannot delegate gas estimation to the bundler because paymaster middleware will not be called
-          if (bypassPaymasterAndData(overrides)) {
+          if (bypassPaymasterAndDataEmptyHex(overrides)) {
             const result = await fallbackFeeDataGetter(struct, {
               overrides,
               feeOptions,

packages/core/src/actions/smartAccount/internal/runMiddlewareStack.ts

@@ -86,7 +86,14 @@ export async function _runMiddlewareStack<
           return {
             ...uo,
             ...("paymasterAndData" in overrides!
-              ? { paymasterAndData: "0x" }
+              ? { paymasterAndData: overrides.paymasterAndData }
+              : "paymasterData" in overrides! &&
+                "paymaster" in overrides &&
+                overrides.paymasterData !== "0x"
+              ? {
+                  paymasterData: overrides.paymasterData,
+                  paymaster: overrides.paymaster,
+                }
               : // At this point, nothing has run so no fields are set
                 // for 0.7 when not using a paymaster, all fields should be undefined
                 undefined),

packages/core/src/index.ts

@@ -153,6 +153,7 @@ export {
   bigIntMax,
   bigIntMultiply,
   bypassPaymasterAndData,
+  bypassPaymasterAndDataEmptyHex,
   concatPaymasterAndData,
   deepHexlify,
   defineReadOnly,

packages/core/src/types.ts

@@ -72,14 +72,15 @@ export type UserOperationPaymasterOverrides<
   TEntryPointVersion extends EntryPointVersion = EntryPointVersion
 > = TEntryPointVersion extends "0.6.0"
   ? {
-      // paymasterAndData overrides only allows '0x' for bypassing paymaster middleware
-      paymasterAndData: EmptyHex;
+      // paymasterData overrides to bypass paymaster middleware
+      paymasterAndData: Hex;
     }
   : TEntryPointVersion extends "0.7.0"
   ? {
-      // paymasterData overrides only allows '0x' for bypassing the paymaster middleware
+      // paymasterData overrides to bypass paymaster middleware
       // if set to '0x', all paymaster related fields are omitted from the user op request
-      paymasterData: EmptyHex;
+      paymasterData: Hex;
+      paymaster: Address;
       paymasterVerificationGasLimit:
         | NoUndefined<
             UserOperationStruct<"0.7.0">["paymasterVerificationGasLimit"]

packages/core/src/utils/userop.ts

@@ -156,7 +156,8 @@ export function applyUserOpOverrideOrFeeOption<
 
 /**
  * Utility method for checking whether the middleware pipeline should
- * bypass the paymaster middleware for the user operation with the given overrides
+ * bypass the paymaster middleware for the user operation with the given overrides,
+ * either because the UserOp is paying for its own gas, or passing a specific paymaster
  *
  * @template EntryPointVersion TEntryPointVersion
  * @param overrides the user operation overrides to check
@@ -168,6 +169,23 @@ export const bypassPaymasterAndData = <
   overrides: UserOperationOverrides<TEntryPointVersion> | undefined
 ): boolean =>
   !!overrides &&
+  ("paymasterAndData" in overrides || "paymasterData" in overrides);
+
+/**
+ * An alternative to `bypassPaymasterAndData` which only returns true if the data parameter
+ * is "0x," this is useful for cases when middleware should be bypassed ONLY IF the UserOp will
+ * pay for its own gas
+ *
+ * @template EntryPointVersion TEntryPointVersion
+ * @param overrides the user operation overrides to check
+ * @returns whether the paymaster middleware should be bypassed
+ */
+export const bypassPaymasterAndDataEmptyHex = <
+  TEntryPointVersion extends EntryPointVersion = EntryPointVersion
+>(
+  overrides: UserOperationOverrides<TEntryPointVersion> | undefined
+): boolean =>
+  overrides !== undefined &&
   (("paymasterAndData" in overrides && overrides.paymasterAndData === "0x") ||
     ("paymasterData" in overrides && overrides.paymasterData === "0x"));