fix: signTypedData verification for smart accounts (#2906)

.changeset/afraid-pianos-cross.md

@@ -0,0 +1,5 @@
+---
+"thirdweb": patch
+---
+
+Fix signTypedData external verification for smart accounts

legacy_packages/auth/test/smart-wallet.test.ts

@@ -9,9 +9,6 @@ require("dotenv-mono").load();
 
 describe.runIf(process.env.TW_SECRET_KEY)(
   "Wallet Authentication - EVM - Smart Wallet",
-  {
-    timeout: 240000,
-  },
   async () => {
     let adminWallet: any, signerWallet: any, attackerWallet: any;
     let auth: ThirdwebAuth;
@@ -42,7 +39,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       auth.updateWallet(signerWallet);
     });
 
-    it("Should verify logged in wallet", async () => {
+    it("Should verify logged in wallet", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -51,7 +50,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should verify logged in wallet with chain ID and expiration", async () => {
+    it("Should verify logged in wallet with chain ID and expiration", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         expirationTime: new Date(Date.now() + 1000 * 60 * 5),
         chainId: "84532",
@@ -65,7 +66,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should verify payload with resources", async () => {
+    it("Should verify payload with resources", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         resources: ["https://example.com", "https://test.com"],
       });
@@ -78,7 +81,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should reject payload without necessary resources", async () => {
+    it("Should reject payload without necessary resources", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         resources: ["https://example.com"],
       });
@@ -96,7 +101,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should verify payload with customized statement", async () => {
+    it("Should verify payload with customized statement", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         statement: "Please sign!",
       });
@@ -109,7 +116,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should reject payload with incorrect statement", async () => {
+    it("Should reject payload with incorrect statement", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         statement: "Please sign!",
       });
@@ -127,7 +136,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject invalid nonce", async () => {
+    it("Should reject invalid nonce", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -145,7 +156,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should accept valid nonce", async () => {
+    it("Should accept valid nonce", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -160,7 +173,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should reject payload with incorrect domain", async () => {
+    it("Should reject payload with incorrect domain", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -174,7 +189,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject expired login payload", async () => {
+    it("Should reject expired login payload", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         expirationTime: new Date(Date.now() - 1000 * 60 * 5),
       });
@@ -188,7 +205,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject payload with incorrect chain ID", async () => {
+    it("Should reject payload with incorrect chain ID", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login({
         chainId: "1",
       });
@@ -206,7 +225,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject payload with incorrect signer", async () => {
+    it("Should reject payload with incorrect signer", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
       payload.payload.address = await attackerWallet.getAddress();
 
@@ -219,7 +240,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should generate valid authentication token", async () => {
+    it("Should generate valid authentication token", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -229,7 +252,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(user.address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should reject token with incorrect domain", async () => {
+    it("Should reject token with incorrect domain", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -245,7 +270,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject token before invalid before", async () => {
+    it("Should reject token before invalid before", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -261,7 +288,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject expired authentication token", async () => {
+    it("Should reject expired authentication token", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -277,7 +306,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should reject if admin address is not connected wallet address", async () => {
+    it("Should reject if admin address is not connected wallet address", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -294,7 +325,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should accept token with valid token ID", async () => {
+    it("Should accept token with valid token ID", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -313,7 +346,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(user.address).to.equal(await signerWallet.getAddress());
     });
 
-    it("Should reject token with invalid token ID", async () => {
+    it("Should reject token with invalid token ID", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -335,7 +370,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       }
     });
 
-    it("Should propagate session on token", async () => {
+    it("Should propagate session on token", {
+      timeout: 240000,
+    },  async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -349,7 +386,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       expect(user.session).to.deep.equal({ role: "admin" });
     });
 
-    it("Should call session callback function", async () => {
+    it("Should call session callback function", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);
@@ -368,7 +407,9 @@ describe.runIf(process.env.TW_SECRET_KEY)(
       });
     });
 
-    it("Should authenticate with issuer address", async () => {
+    it("Should authenticate with issuer address", {
+      timeout: 240000,
+    }, async () => {
       const payload = await auth.login();
 
       auth.updateWallet(adminWallet);

legacy_packages/wallets/src/evm/connectors/smart-wallet/lib/check-contract-wallet-signature.ts

@@ -74,7 +74,7 @@ export async function checkContractWalletSignature(
 
   const provider = new providers.StaticJsonRpcProvider(
     {
-      url: chainIdToThirdwebRpc(chainId),
+      url: rpcUrl,
       skipFetchSetup: _skipFetchSetup,
       headers,
     },

legacy_packages/wallets/src/evm/connectors/smart-wallet/lib/erc4337-signer.ts

@@ -227,7 +227,7 @@ Code: ${errorCode}`;
     try {
       const provider = new providers.StaticJsonRpcProvider(
         {
-          url: chainIdToThirdwebRpc(chainId, this.config.clientId),
+          url: rpcUrl,
           headers,
         },
         chainId,

legacy_packages/wallets/src/evm/wallets/abstract.ts

@@ -17,7 +17,7 @@ import { createErc20 } from "../utils/currency";
 // TODO improve this
 export function chainIdToThirdwebRpc(chainId: number, clientId?: string) {
   return `https://${chainId}.rpc.thirdweb.com${clientId ? `/${clientId}` : ""}${
-    typeof globalThis !== "undefined" && "APP_BUNDLE_ID" in globalThis
+    typeof globalThis !== "undefined" && "APP_BUNDLE_ID" in globalThis && !!(globalThis as any).APP_BUNDLE_ID
       ? `?bundleId=${(globalThis as any).APP_BUNDLE_ID as string}`
       : ""
   }`;

legacy_packages/wallets/test/smart-wallet-integration.test.ts

@@ -117,7 +117,7 @@ describeIf(!!SECRET_KEY)(
     });
 
     it("can sign and verify 1271 old factory", {
-      timeout: 120_000,
+      timeout: 240_000,
     }, async () => {
       const message = "0x1234";
       const sig = await smartWallet.signMessage(message);

packages/thirdweb/src/extensions/erc1271/checkContractWalletSignedTypedData.ts

@@ -0,0 +1,53 @@
+import { type TypedData, type TypedDataDefinition, hashTypedData } from "viem";
+import type { ThirdwebContract } from "../../contract/contract.js";
+import { isHex } from "../../utils/encoding/hex.js";
+import { isValidSignature } from "./__generated__/isValidSignature/read/isValidSignature.js";
+
+export type CheckContractWalletSignTypedDataOptions<
+  typedData extends TypedData | Record<string, unknown>,
+  primaryType extends keyof typedData | "EIP712Domain" = keyof typedData,
+> = {
+  contract: ThirdwebContract;
+  data: TypedDataDefinition<typedData, primaryType>;
+  signature: string;
+};
+const MAGIC_VALUE = "0x1626ba7e";
+
+/**
+ * Checks if a contract wallet signature is valid.
+ * @param options - The options for the checkContractWalletSignature function.
+ * @param options.contract - The contract to check the signature against.
+ * @param options.message - The message to check the signature against.
+ * @param options.signature - The signature to check.
+ * @extension ERC1271
+ * @example
+ * ```ts
+ * import { checkContractWalletSignedTypedData } from "thirdweb/extensions/erc1271";
+ * const isValid = await checkContractWalletSignedTypedData({
+ *  contract: myContract,
+ *  data: {
+ *   primaryType: "EIP712Domain",
+ *   domain: {
+ *     name: "Example",
+ *     version: "1",
+ *     chainId: 1,
+ *     verifyingContract: myContract.address,
+ *   },
+ * });
+ * ```
+ * @returns A promise that resolves with a boolean indicating if the signature is valid.
+ */
+export async function checkContractWalletSignedTypedData<
+  typedData extends TypedData | Record<string, unknown>,
+  primaryType extends keyof typedData | "EIP712Domain" = keyof typedData,
+>(options: CheckContractWalletSignTypedDataOptions<typedData, primaryType>) {
+  if (!isHex(options.signature)) {
+    throw new Error("The signature must be a valid hex string.");
+  }
+  const result = await isValidSignature({
+    contract: options.contract,
+    hash: hashTypedData(options.data),
+    signature: options.signature,
+  });
+  return result === MAGIC_VALUE;
+}

packages/thirdweb/src/rpc/watchBlockNumber.test.ts

@@ -36,8 +36,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 
@@ -59,8 +59,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
     expect(onNewBlockNumber2).toHaveBeenCalledTimes(0);
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
     expect(onNewBlockNumber2).toHaveBeenCalled();
@@ -80,8 +80,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 
@@ -89,8 +89,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     unwatch();
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
   });
@@ -104,8 +104,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 
@@ -115,8 +115,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     unwatch();
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
@@ -126,8 +126,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
       onNewBlockNumber,
     });
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 
@@ -148,8 +148,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 
@@ -159,8 +159,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
 
     unwatch();
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalledTimes(0);
 
@@ -171,8 +171,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("watch block number", () => {
       latestBlockNumber: lastBlockNumber,
     });
 
-    // wait for 5 seconds which should always be sufficient for a new block to be mined
-    await wait(5000);
+    // wait for 10 seconds which should always be sufficient for a new block to be mined
+    await wait(10000);
 
     expect(onNewBlockNumber).toHaveBeenCalled();
 

packages/thirdweb/src/wallets/smart/index.ts

@@ -1,4 +1,9 @@
-import type { SignableMessage } from "viem";
+import type {
+  SignableMessage,
+  TypedData,
+  TypedDataDefinition,
+  TypedDataDomain,
+} from "viem";
 import type { Chain } from "../../chains/types.js";
 import type { ThirdwebClient } from "../../client/client.js";
 import { type ThirdwebContract, getContract } from "../../contract/contract.js";
@@ -219,9 +224,97 @@ async function createSmartAccount(
         "Unable to verify signature on smart account, please make sure the smart account is deployed and the signature is valid.",
       );
     },
-    // biome-ignore lint/suspicious/noExplicitAny: TODO: fix any
-    async signTypedData(typedData: any) {
-      return options.personalAccount.signTypedData(typedData);
+    async signTypedData<
+      const typedData extends TypedData | Record<string, unknown>,
+      primaryType extends keyof typedData | "EIP712Domain" = keyof typedData,
+    >(_typedData: TypedDataDefinition<typedData, primaryType>) {
+      const [
+        { isContractDeployed },
+        { readContract },
+        { encodeAbiParameters },
+        { hashTypedData },
+        { checkContractWalletSignedTypedData },
+      ] = await Promise.all([
+        import("../../utils/bytecode/is-contract-deployed.js"),
+        import("../../transaction/read-contract.js"),
+        import("../../utils/abi/encodeAbiParameters.js"),
+        import("viem"),
+        import(
+          "../../extensions/erc1271/checkContractWalletSignedTypedData.js"
+        ),
+      ]);
+      const isSelfVerifyingContract =
+        (
+          _typedData.domain as TypedDataDomain
+        )?.verifyingContract?.toLowerCase() ===
+        accountContract.address?.toLowerCase();
+
+      if (isSelfVerifyingContract) {
+        // if the contract is self-verifying, we can just sign the message with the EOA (ie. adding a session key)
+        return options.personalAccount.signTypedData(_typedData);
+      }
+
+      const isDeployed = await isContractDeployed(accountContract);
+      if (!isDeployed) {
+        console.log(
+          "Account contract not deployed yet. Deploying account before signing message",
+        );
+        await _deployAccount({
+          options,
+          account,
+          accountContract,
+        });
+      }
+
+      const originalMsgHash = hashTypedData(_typedData);
+      // check if the account contract supports EIP721 domain separator based signing
+      let factorySupports712 = false;
+      try {
+        // this will throw if the contract does not support it (old factories)
+        await readContract({
+          contract: accountContract,
+          method:
+            "function getMessageHash(bytes32 _hash) public view returns (bytes32)",
+          params: [originalMsgHash],
+        });
+        factorySupports712 = true;
+      } catch (e) {
+        // ignore
+      }
+
+      let sig: `0x${string}`;
+      if (factorySupports712) {
+        const wrappedMessageHash = encodeAbiParameters(
+          [{ type: "bytes32" }],
+          [originalMsgHash],
+        );
+        sig = await options.personalAccount.signTypedData({
+          domain: {
+            name: "Account",
+            version: "1",
+            chainId: options.chain.id,
+            verifyingContract: accountContract.address,
+          },
+          primaryType: "AccountMessage",
+          types: { AccountMessage: [{ name: "message", type: "bytes" }] },
+          message: { message: wrappedMessageHash },
+        });
+      } else {
+        sig = await options.personalAccount.signTypedData(_typedData);
+      }
+
+      const isValid = await checkContractWalletSignedTypedData({
+        contract: accountContract,
+        data: _typedData,
+        signature: sig,
+      });
+
+      if (isValid) {
+        return sig;
+      }
+      throw new Error(
+        "Unable to verify signature on smart account, please make sure the smart account is deployed and the signature is valid.",
+      );
     },
   };
   return account;

packages/thirdweb/src/wallets/smart/smart-wallet-integration.test.ts

@@ -1,8 +1,14 @@
 import { beforeAll, describe, expect, it } from "vitest";
 import { TEST_CLIENT } from "../../../test/src/test-clients.js";
+import { typedData } from "../../../test/src/typed-data.js";
 import { verifySignature } from "../../auth/verifySignature.js";
 import { arbitrumSepolia } from "../../chains/chain-definitions/arbitrum-sepolia.js";
 import { type ThirdwebContract, getContract } from "../../contract/contract.js";
+import { parseEventLogs } from "../../event/actions/parse-logs.js";
+import {
+  addAdmin,
+  adminUpdatedEvent,
+} from "../../exports/extensions/erc4337.js";
 import { balanceOf } from "../../extensions/erc1155/__generated__/IERC1155/read/balanceOf.js";
 import { claimTo } from "../../extensions/erc1155/drops/write/claimTo.js";
 import { checkContractWalletSignature } from "../../extensions/erc1271/checkContractWalletSignature.js";
@@ -156,6 +162,35 @@ describe.runIf(process.env.TW_SECRET_KEY)(
         contract: accountContract,
       });
       expect(isValidV2).toEqual(true);
+
+      // sign typed data
+      const signatureTyped = await smartAccount.signTypedData({
+        ...typedData.basic,
+        primaryType: "Mail",
+      });
+      expect(signatureTyped.length).toBe(132);
+
+      // add admin
+      const newAdmin = await generateAccount({ client });
+      const receipt = await sendAndConfirmTransaction({
+        account: smartAccount,
+        transaction: addAdmin({
+          account: smartAccount,
+          adminAddress: newAdmin.address,
+          contract: getContract({
+            address: smartAccount.address,
+            chain,
+            client,
+          }),
+        }),
+      });
+      const logs = parseEventLogs({
+        events: [adminUpdatedEvent()],
+        logs: receipt.logs,
+      });
+      expect(logs.length).toBe(1);
+      expect(logs[0]?.args.signer).toBe(newAdmin.address);
+      expect(logs[0]?.args.isAdmin).toBe(true);
     });
 
     it("can sign and verify 1271 new factory", async () => {
@@ -185,6 +220,35 @@ describe.runIf(process.env.TW_SECRET_KEY)(
         }),
       });
       expect(isValidV2).toEqual(true);
+
+      // sign typed data
+      const signatureTyped = await newAccount.signTypedData({
+        ...typedData.basic,
+        primaryType: "Mail",
+      });
+      expect(signatureTyped.length).toBe(132);
+
+      // add admin
+      const newAdmin = await generateAccount({ client });
+      const receipt = await sendAndConfirmTransaction({
+        account: newAccount,
+        transaction: addAdmin({
+          account: newAccount,
+          adminAddress: newAdmin.address,
+          contract: getContract({
+            address: newAccount.address,
+            chain,
+            client,
+          }),
+        }),
+      });
+      const logs = parseEventLogs({
+        events: [adminUpdatedEvent()],
+        logs: receipt.logs,
+      });
+      expect(logs.length).toBe(1);
+      expect(logs[0]?.args.signer).toBe(newAdmin.address);
+      expect(logs[0]?.args.isAdmin).toBe(true);
     });
   },
 );