Skip to content

Override signMessage for SmartWallet class #1953

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

Merged
merged 6 commits into from
Nov 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/spicy-boats-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@thirdweb-dev/wallets": patch
---

Patched smart wallet signatures for updated eip-1271 sig verification
1 change: 1 addition & 0 deletions packages/sdk/src/evm/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export { fetchSnapshotEntryForAddress } from "./common/claim-conditions/fetchSna
export { getCachedAbiForContract } from "./common/abi";
export * from "./common/ens/resolveEns";
export * from "./common/ens/resolveAddress";
export * from "./common/sign";
//#endregion @r/packages/sdk/src/evm/common/*

//#region @r/packages/sdk/src/evm/constants/*
Expand Down
65 changes: 64 additions & 1 deletion packages/wallets/src/evm/wallets/smart-wallet.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AbstractClientWallet, WalletOptions } from "./base";
import { checkContractWalletSignature } from "./abstract";
import type { ConnectParams } from "../interfaces/connector";
import type {
SmartWalletConfig,
Expand All @@ -14,7 +15,8 @@ import {
} from "@thirdweb-dev/sdk";
import { walletIds } from "../constants/walletIds";
import { getValidChainRPCs } from "@thirdweb-dev/chains";
import { providers, utils } from "ethers";
import { providers, utils, Bytes, Signer } from "ethers";
import { signTypedDataInternal } from "@thirdweb-dev/sdk";

// export types and utils for convenience
export type * from "../connectors/smart-wallet/types";
Expand Down Expand Up @@ -76,6 +78,67 @@ export class SmartWallet extends AbstractClientWallet<
return this.connector?.personalWallet;
}

/**
* @returns the signature of the message
*/
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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to validate the signature here, will also slow down the process quite a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's another way to check whether to fallback to legacy method of signing.

So this seems necessary to maintain backward compatibility with existing smart wallets out there.

message as string,
result.signature,
address,
chainId,
);

if (!isValid) {
throw new Error("Invalid signature");
}

return result.signature;
} catch {
return await this.signMessageLegacy(erc4337Signer, message);
}
}

/**
* @returns the signature of the message (for legacy EIP-1271 signature verification)
*/
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.
Expand Down