[Smart Wallet] Always force deploy on sign/auth, support 1271 (#2341)

0xFirekeeper · web-flow · commit e7528af48ff2 · 2024-02-26T23:03:51.000Z
diff --git a/.changeset/good-ways-listen.md b/.changeset/good-ways-listen.md
@@ -0,0 +1,6 @@
+---
+"@thirdweb-dev/unity-js-bridge": minor
+"@thirdweb-dev/wallets": patch
+---
+
+[Smart Wallet] Always force deploy on sign/auth, use 712 with 1271 where possible
diff --git a/packages/unity-js-bridge/src/thirdweb-bridge.ts b/packages/unity-js-bridge/src/thirdweb-bridge.ts
@@ -144,7 +144,7 @@ class ThirdwebBridge implements TWBridge {
       }
       (globalThis as any).X_SDK_NAME = "UnitySDK_WebGL";
       (globalThis as any).X_SDK_PLATFORM = "unity";
-      (globalThis as any).X_SDK_VERSION = "4.6.4";
+      (globalThis as any).X_SDK_VERSION = "4.7.0";
       (globalThis as any).X_SDK_OS = browser?.os ?? "unknown";
     }
     this.initializedChain = chain;
@@ -244,7 +244,6 @@ class ThirdwebBridge implements TWBridge {
             paymasterUrl: sdkOptions.smartWalletConfig?.paymasterUrl,
             // paymasterAPI: sdkOptions.smartWalletConfig?.paymasterAPI,
             entryPointAddress: sdkOptions.smartWalletConfig?.entryPointAddress,
-            deployOnSign: sdkOptions.smartWalletConfig?.deployOnSign,
           };
           walletInstance = new SmartWallet(config);
           break;
@@ -813,18 +812,19 @@ class ThirdwebBridge implements TWBridge {
     if (!this.activeWallet) {
       throw new Error("No wallet connected");
     }
-    try{
+    try {
       const smartWallet = this.activeWallet as SmartWallet;
       const signer = await smartWallet.getPersonalWallet()?.getSigner();
       const res = await signer?.getAddress();
       return JSON.stringify({ result: res }, bigNumberReplacer);
     } catch {
-      console.debug("Could not find a smart wallet, defaulting to normal signer");
+      console.debug(
+        "Could not find a smart wallet, defaulting to normal signer",
+      );
       const signer = await this.activeWallet.getSigner();
       const res = await signer.getAddress();
       return JSON.stringify({ result: res }, bigNumberReplacer);
     }
-    
   }
 
   public openPopupWindow() {
diff --git a/packages/wallets/src/evm/connectors/smart-wallet/index.ts b/packages/wallets/src/evm/connectors/smart-wallet/index.ts
@@ -54,7 +54,6 @@ export class SmartWalletConnector extends Connector<SmartWalletConnectionArgs> {
       this.config.paymasterUrl ||
       `https://${this.chainId}.bundler.thirdweb.com/v2`;
     const entryPointAddress = config.entryPointAddress || ENTRYPOINT_ADDRESS;
-    const deployOnSign = config.deployOnSign ?? true;
     const localSigner = await params.personalWallet.getSigner();
     const providerConfig: ProviderConfig = {
       chain: config.chain,
@@ -70,7 +69,6 @@ export class SmartWalletConnector extends Connector<SmartWalletConnectionArgs> {
             this.config.secretKey,
           ),
       gasless: config.gasless,
-      deployOnSign: deployOnSign,
       factoryAddress: config.factoryAddress,
       accountAddress: params.accountAddress,
       factoryInfo: config.factoryInfo || this.defaultFactoryInfo(),
@@ -316,8 +314,8 @@ export class SmartWalletConnector extends Connector<SmartWalletConnectionArgs> {
         value: await transaction.getValue(),
         gasLimit: await transaction.getOverrides().gasLimit,
         maxFeePerGas: await transaction.getOverrides().maxFeePerGas,
-        maxPriorityFeePerGas: await transaction.getOverrides()
-          .maxPriorityFeePerGas,
+        maxPriorityFeePerGas:
+          await transaction.getOverrides().maxPriorityFeePerGas,
         nonce: await transaction.getOverrides().nonce,
       },
       options,
diff --git a/packages/wallets/src/evm/connectors/smart-wallet/lib/erc4337-signer.ts b/packages/wallets/src/evm/connectors/smart-wallet/lib/erc4337-signer.ts
@@ -1,11 +1,16 @@
-import { ethers, providers, utils } from "ethers";
+import { Contract, ethers, providers, utils } from "ethers";
 
 import { Bytes, Signer } from "ethers";
 import { BaseAccountAPI } from "./base-api";
 import type { ERC4337EthersProvider } from "./erc4337-provider";
 import { HttpRpcClient } from "./http-rpc-client";
 import { hexlifyUserOp, randomNonce } from "./utils";
 import { ProviderConfig, UserOpOptions } from "../types";
+import { signTypedDataInternal } from "@thirdweb-dev/sdk";
+import {
+  checkContractWalletSignature,
+  chainIdToThirdwebRpc,
+} from "../../../wallets/abstract";
 
 export class ERC4337EthersSigner extends Signer {
   config: ProviderConfig;
@@ -138,20 +143,84 @@ Code: ${errorCode}`;
     return this.address as string;
   }
 
-  async signMessage(message: Bytes | string): Promise<string> {
-      const isNotDeployed = await this.smartAccountAPI.checkAccountPhantom();
-      if (isNotDeployed && this.config.deployOnSign) {
-        console.log(
-          "Account contract not deployed yet. Deploying account before signing message",
-        );
-        const tx = await this.sendTransaction({
-          to: await this.getAddress(),
-          data: "0x",
-        });
-        await tx.wait();
-      }
-    
-    return await this.originalSigner.signMessage(message);
+  /**
+   * Sign a message and return the signature
+   */
+  public async signMessage(message: Bytes | string): Promise<string> {
+    // Deploy smart wallet if needed
+    const isNotDeployed = await this.smartAccountAPI.checkAccountPhantom();
+    if (isNotDeployed) {
+      console.log(
+        "Account contract not deployed yet. Deploying account before signing message",
+      );
+      const tx = await this.sendTransaction({
+        to: await this.getAddress(),
+        data: "0x",
+      });
+      await tx.wait();
+    }
+
+    const [chainId, address] = await Promise.all([
+      this.getChainId(),
+      this.getAddress(),
+    ]);
+    const originalMsgHash = utils.hashMessage(message);
+
+    let factorySupports712: boolean;
+    let signature: string;
+
+    try {
+      const provider = new providers.JsonRpcProvider(
+        chainIdToThirdwebRpc(chainId, this.config.clientId),
+        chainId,
+      );
+      const walletContract = new Contract(
+        address,
+        [
+          "function getMessageHash(bytes32 _hash) public view returns (bytes32)",
+        ],
+        provider,
+      );
+      // if this fails it's a pre 712 factory
+      await walletContract.getMessageHash(originalMsgHash);
+      factorySupports712 = true;
+    } catch {
+      factorySupports712 = false;
+    }
+
+    if (factorySupports712) {
+      const result = await signTypedDataInternal(
+        this,
+        {
+          name: "Account",
+          version: "1",
+          chainId,
+          verifyingContract: address,
+        },
+        { AccountMessage: [{ name: "message", type: "bytes" }] },
+        {
+          message: utils.defaultAbiCoder.encode(["bytes32"], [originalMsgHash]),
+        },
+      );
+      signature = result.signature;
+    } else {
+      signature = await this.originalSigner.signMessage(message);
+    }
+
+    const isValid = await checkContractWalletSignature(
+      message as string,
+      signature,
+      address,
+      chainId,
+    );
+
+    if (isValid) {
+      return signature;
+    } else {
+      throw new Error(
+        "Unable to verify signature on smart account, please make sure the smart account is deployed and the signature is valid.",
+      );
+    }
   }
 
   async signTransaction(
diff --git a/packages/wallets/src/evm/connectors/smart-wallet/types.ts b/packages/wallets/src/evm/connectors/smart-wallet/types.ts
@@ -22,7 +22,6 @@ export type SmartWalletConfig = {
   paymasterUrl?: string;
   paymasterAPI?: PaymasterAPI;
   entryPointAddress?: string;
-  deployOnSign?: boolean;
 } & ContractInfoInput &
   WalletConnectReceiverConfig;
 
@@ -43,7 +42,6 @@ export interface ProviderConfig extends ContractInfo {
   accountAddress?: string;
   paymasterAPI: PaymasterAPI;
   gasless: boolean;
-  deployOnSign?: boolean;
 }
 
 export type ContractInfoInput = {
diff --git a/packages/wallets/src/evm/wallets/abstract.ts b/packages/wallets/src/evm/wallets/abstract.ts
@@ -15,7 +15,7 @@ import {
 import { createErc20 } from "../utils/currency";
 
 // TODO improve this
-function chainIdToThirdwebRpc(chainId: number, clientId?: string) {
+export function chainIdToThirdwebRpc(chainId: number, clientId?: string) {
   return `https://${chainId}.rpc.thirdweb.com${clientId ? `/${clientId}` : ""}${
     typeof globalThis !== "undefined" && "APP_BUNDLE_ID" in globalThis
       ? `?bundleId=${(globalThis as any).APP_BUNDLE_ID as string}`
@@ -66,9 +66,11 @@ export async function checkContractWalletSignature(
     skipFetchSetup: _skipFetchSetup,
   });
   const walletContract = new Contract(address, EIP1271_ABI, provider);
-  const _hashMessage = utils.hashMessage(message);
   try {
-    const res = await walletContract.isValidSignature(_hashMessage, signature);
+    const res = await walletContract.isValidSignature(
+      utils.hashMessage(message),
+      signature,
+    );
     return res === EIP1271_MAGICVALUE;
   } catch {
     return false;
diff --git a/packages/wallets/src/evm/wallets/smart-wallet.ts b/packages/wallets/src/evm/wallets/smart-wallet.ts
@@ -1,5 +1,4 @@
 import { AbstractClientWallet, WalletOptions } from "./base";
-import { checkContractWalletSignature } from "./abstract";
 import type { ConnectParams } from "../interfaces/connector";
 import type {
   SmartWalletConfig,
@@ -16,8 +15,7 @@ import {
 } from "@thirdweb-dev/sdk";
 import { walletIds } from "../constants/walletIds";
 import { getValidChainRPCs } from "@thirdweb-dev/chains";
-import { providers, utils, Bytes, Signer } from "ethers";
-import { signTypedDataInternal } from "@thirdweb-dev/sdk";
+import { providers, utils } from "ethers";
 
 // export types and utils for convenience
 export type * from "../connectors/smart-wallet/types";
@@ -246,10 +244,10 @@ export class SmartWallet extends AbstractClientWallet<
    * The entrypoint contract address. Uses v0.6 by default.
    *
    * Must be a `string`.
-   * 
+   *
    * #### deployOnSign
    * Whether to deploy the smart wallet when the user signs a message. Defaults to true.
-   * 
+   *
    * Must be a `boolean`.
    *
    * #### chains
@@ -316,68 +314,6 @@ export class SmartWallet extends AbstractClientWallet<
     return this.connector?.personalWallet;
   }
 
-  /**
-   * Sign a message and return the signature
-   */
-  public async signMessage(message: Bytes | string): Promise<string> {
-    // Deploy smart wallet if needed
-    const connector = await this.getConnector();
-    await connector.deployIfNeeded();
-
-    const erc4337Signer = await this.getSigner();
-    const chainId = await erc4337Signer.getChainId();
-    const address = await connector.getAddress();
-
-    /**
-     * We first try to sign the EIP-712 typed data i.e. the message mixed with the smart wallet's domain separator.
-     * If this fails, we fallback to the legacy signing method.
-     */
-    try {
-      const result = await signTypedDataInternal(
-        erc4337Signer,
-        {
-          name: "Account",
-          version: "1",
-          chainId,
-          verifyingContract: address,
-        },
-        { AccountMessage: [{ name: "message", type: "bytes" }] },
-        {
-          message: utils.defaultAbiCoder.encode(
-            ["bytes32"],
-            [utils.hashMessage(message)],
-          ),
-        },
-      );
-
-      const isValid = await checkContractWalletSignature(
-        message as string,
-        result.signature,
-        address,
-        chainId,
-      );
-
-      if (!isValid) {
-        throw new Error("Invalid signature");
-      }
-
-      return result.signature;
-    } catch {
-      return await this.signMessageLegacy(erc4337Signer, message);
-    }
-  }
-
-  /**
-   * This is only for for legacy EIP-1271 signature verification
-   * Sign a message and return the signature
-   */
-  private async signMessageLegacy(
-    signer: Signer,
-    message: Bytes | string,
-  ): Promise<string> {
-    return await signer.signMessage(message);
-  }
-
   /**
    * Check whether the connected signer can execute a given transaction using the smart wallet.
    * @param transaction - The transaction to execute using the smart wallet.
diff --git a/packages/wallets/test/smart-wallet-integration.test.ts b/packages/wallets/test/smart-wallet-integration.test.ts
@@ -3,6 +3,10 @@ import { SmartWallet } from "../src/evm/wallets/smart-wallet";
 import { LocalWallet } from "../src/evm/wallets/local-wallet";
 import { Mumbai } from "@thirdweb-dev/chains";
 import { ThirdwebSDK, SmartContract } from "@thirdweb-dev/sdk";
+import { checkContractWalletSignature } from "../src/evm/wallets/abstract";
+
+require("dotenv-mono").load();
+jest.setTimeout(240_000);
 
 let smartWallet: SmartWallet;
 let smartWalletAddress: string;
@@ -93,4 +97,23 @@ describeIf(!!process.env.TW_SECRET_KEY)("SmartWallet core tests", () => {
     const balance = await contract.erc1155.balance(0);
     expect(balance.toNumber()).toEqual(7);
   });
+
+  it("can sign and verify 1271", async () => {
+    const message = "0x1234";
+    const sig = await smartWallet.signMessage(message);
+    const isValidV1 = await smartWallet.verifySignature(
+      message,
+      sig,
+      smartWalletAddress,
+      chain.chainId,
+    );
+    expect(isValidV1).toEqual(true);
+    const isValidV2 = await checkContractWalletSignature(
+      message,
+      sig,
+      smartWalletAddress,
+      chain.chainId,
+    );
+    expect(isValidV2).toEqual(true);
+  });
 });
