Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix paymaster override for multisig plugin #676

Merged
merged 9 commits into from
Jun 4, 2024
68 changes: 65 additions & 3 deletions packages/alchemy/e2e-tests/multisig-account.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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: {
Expand All @@ -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",
},
});
Expand All @@ -237,6 +298,7 @@ describe("Multisig Modular Account Alchemy Client Tests", async () => {
}, 100000);
});


const givenConnectedProvider = async ({
signer,
chain,
Expand Down
6 changes: 3 additions & 3 deletions packages/alchemy/src/middleware/gasManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
UserOperationRequest,
} from "@alchemy/aa-core";
import {
bypassPaymasterAndData,
bypassPaymasterAndDataEmptyHex,
deepHexlify,
defaultGasEstimator,
filterUndefined,
Expand Down Expand Up @@ -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, {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
moldy530 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export {
bigIntMax,
bigIntMultiply,
bypassPaymasterAndData,
bypassPaymasterAndDataEmptyHex,
concatPaymasterAndData,
deepHexlify,
defineReadOnly,
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
moldy530 marked this conversation as resolved.
Show resolved Hide resolved
paymaster: Address;
paymasterVerificationGasLimit:
| NoUndefined<
UserOperationStruct<"0.7.0">["paymasterVerificationGasLimit"]
Expand Down
20 changes: 19 additions & 1 deletion packages/core/src/utils/userop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"));

Expand Down
Loading