From f97ea0f0862679917ac30575f398150f8af2e7ce Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 13 Sep 2024 13:32:29 -0400 Subject: [PATCH 1/4] feat: fix light account 1271 signing --- .../src/light-account/accounts/base.ts | 26 +++++++- .../src/light-account/clients/client.test.ts | 62 ++++++++++--------- 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/account-kit/smart-contracts/src/light-account/accounts/base.ts b/account-kit/smart-contracts/src/light-account/accounts/base.ts index 95aeb9b24..71a5eecd5 100644 --- a/account-kit/smart-contracts/src/light-account/accounts/base.ts +++ b/account-kit/smart-contracts/src/light-account/accounts/base.ts @@ -145,7 +145,7 @@ export async function createLightAccountBase< // https://github.com/alchemyplatform/light-account/blob/main/src/LightAccount.sol#L236 domain: { chainId: Number(client.chain.id), - name: type, + name: "LightAccount", verifyingContract: accountAddress, version: "1", }, @@ -159,6 +159,26 @@ export async function createLightAccountBase< }); }; + const signWith1271WrapperV2 = async (hashedMessage: Hex): Promise => { + return signer.signTypedData({ + // EIP712Domain(string name,string version,uint256 chainId,address verifyingContract) + // https://github.com/alchemyplatform/light-account/blob/main/src/LightAccount.sol#L236 + domain: { + chainId: Number(client.chain.id), + name: "LightAccount", + verifyingContract: accountAddress, + version: "2", + }, + types: { + LightAccountMessage: [{ name: "message", type: "bytes" }], + }, + message: { + message: hashedMessage, + }, + primaryType: "LightAccountMessage", + }); + }; + const account = await toSmartContractAccount({ transport, chain, @@ -209,7 +229,7 @@ export async function createLightAccountBase< case "v1.1.0": return signWith1271WrapperV1(hashMessage(message)); case "v2.0.0": - const signature = await signWith1271WrapperV1(hashMessage(message)); + const signature = await signWith1271WrapperV2(hashMessage(message)); // TODO: handle case where signer is an SCA. return concat([SignatureType.EOA, signature]); default: @@ -229,7 +249,7 @@ export async function createLightAccountBase< case "v1.1.0": return signWith1271WrapperV1(hashTypedData(params)); case "v2.0.0": - const signature = await signWith1271WrapperV1(hashTypedData(params)); + const signature = await signWith1271WrapperV2(hashTypedData(params)); // TODO: handle case where signer is an SCA. return concat([SignatureType.EOA, signature]); default: diff --git a/account-kit/smart-contracts/src/light-account/clients/client.test.ts b/account-kit/smart-contracts/src/light-account/clients/client.test.ts index 239576df0..0c58a7c14 100644 --- a/account-kit/smart-contracts/src/light-account/clients/client.test.ts +++ b/account-kit/smart-contracts/src/light-account/clients/client.test.ts @@ -9,7 +9,7 @@ import { type UserOperationOverrides, type UserOperationStruct, } from "@aa-sdk/core"; -import { custom, parseEther, type Address } from "viem"; +import { custom, parseEther, type Address, publicActions } from "viem"; import { setBalance } from "viem/actions"; import { resetBalance } from "~test/accounts.js"; import { accounts } from "~test/constants.js"; @@ -65,32 +65,37 @@ describe("Light Account Tests", () => { ); it.each(versions)("should correctly sign the message", async (version) => { - const { account } = await givenConnectedProvider({ + const provider = await givenConnectedProvider({ signer, version, }); - const message = - "0xa70d0af2ebb03a44dcd0714a8724f622e3ab876d0aa312f0ee04823285d6fb1b"; + + const { account } = provider; + + const message = "hello world"; switch (version) { - case "v1.0.1": - expect(await account.signMessage({ message })).toBe( - "0x53f3169bd14b5ade19a0488bf04ba94a1afa46549014febf3d20d2850a92c98f200c729d723b0592c8b4cb64c0423bb40c5a7fb49f69f4ef5f71ecc672e782e71b" - ); - break; case "v1.0.2": await expect(account.signMessage({ message })).rejects.toThrowError( "LightAccount v1.0.2 doesn't support 1271" ); break; + case "v1.0.1": case "v1.1.0": - expect(await account.signMessage({ message })).toBe( - "0xddb694a4f22cb929476a39f63aae047c2ccd6a12df17c91ff2068d8679f7d2e57b019263691eae350ce53bc6f958d4f2200571614eab8343af9c382aa90579371c" - ); - break; case "v2.0.0": - expect(await account.signMessage({ message })).toBe( - "0x00823ee659d8f86abaee06bb36c4d84bfefa2938af2835ff62564e9559bc671d0c0e8603a5aa3ed9603b28a137eaf491b64c55ed73007c12efebcfd9a82cb181221b" - ); + { + const signature = await account.signMessageWith6492({ + message, + }); + + // We must use a public client, rather than an account client, to verify the message, because AA-SDK incorrectly attaches the account address as a "from" field to all actions taken by that client, including the `eth_call` used internally by viem's signature verifier logic. Per EIP-684, contract creation reverts on non-zero nonce, and the `eth_call`'s from field implicitly increases the nonce of the account contract, causing the contract creation to revert. + expect( + await client.extend(publicActions).verifyMessage({ + address: account.address, + message, + signature, + }) + ).toBe(true); + } break; default: throw new Error(`Unknown version ${version}`); @@ -112,25 +117,26 @@ describe("Light Account Tests", () => { }, } as const; switch (version) { - case "v1.0.1": - expect(await account.signTypedData(typedData)).toBe( - "0x974c40bb8038696abf26fc9134676b9afd44bcfbe6821acd523193d80a354cfc7f3e8fdc28358c5551231156cec29e1c234ffe9cad09d51d633736d582d997811b" - ); - break; case "v1.0.2": await expect(account.signTypedData(typedData)).rejects.toThrowError( "Version v1.0.2 of LightAccount doesn't support 1271" ); break; case "v1.1.0": - expect(await account.signTypedData(typedData)).toBe( - "0x7d42ec333209a34a4a64d5bcb40ba67bc8ed3751f30898be60c1eba78d127e0e06b426a1faef69aa683f92aecc6a8e2292689b03e19f41417dcfa875410bd09d1c" - ); - break; + case "v1.0.1": case "v2.0.0": - expect(await account.signTypedData(typedData)).toBe( - "0x00813b860dce6079750e9383e932412620f6bc206261fed24bc450a1c858acb35912eb082c6fe69daf47f662b88941a9f67da425fa7084f2d93eb5bd2d2705905c1b" - ); + { + const signature = await account.signTypedDataWith6492(typedData); + + // See above comment for context on the duplicate client. + expect( + await client.extend(publicActions).verifyTypedData({ + address: account.address, + signature, + ...typedData, + }) + ).toBe(true); + } break; default: throw new Error(`Unknown version ${version}`); From 26599c3d93051238c80d319b2b0c5841c7c74e7b Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 13 Sep 2024 15:29:49 -0400 Subject: [PATCH 2/4] feat: fix signature generation for MultiOwnerLightAccount --- .../src/__tests__/provider-adapter.test.ts | 34 ++++++++++----- .../src/light-account/accounts/base.ts | 43 +++++++------------ .../clients/multiOwnerLightAccount.test.ts | 43 ++++++++++++------- 3 files changed, 66 insertions(+), 54 deletions(-) diff --git a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts index f7241e38d..013a3f9f3 100644 --- a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts +++ b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts @@ -1,15 +1,15 @@ import { createLightAccount } from "@account-kit/smart-contracts"; import { Wallet } from "@ethersproject/wallet"; import { Alchemy, Network, type AlchemyProvider } from "alchemy-sdk"; -import { http } from "viem"; -import { polygonMumbai } from "viem/chains"; +import { createPublicClient, http, type Hex } from "viem"; +import { sepolia } from "viem/chains"; import { EthersProviderAdapter } from "../../src/provider-adapter.js"; import { convertWalletToAccountSigner } from "../utils.js"; describe("Simple Account Tests", async () => { const alchemy = new Alchemy({ apiKey: "test", - network: Network.MATIC_MUMBAI, + network: Network.ETH_SEPOLIA, }); // demo mnemonic from viem docs const dummyMnemonic = @@ -19,13 +19,26 @@ describe("Simple Account Tests", async () => { it("should correctly sign the message", async () => { const provider = await givenConnectedProvider({ alchemyProvider, signer }); + const message = "hello world"; + // todo: why does this provider's version of signMessage return a string, instead of Hex? + const signature = (await provider.signMessage(message)) as Hex; + + // We must use a public client, rather than an account client, to verify the message, because AA-SDK incorrectly attaches the account address as a "from" field to all actions taken by that client, including the `eth_call` used internally by viem's signature verifier logic. Per EIP-684, contract creation reverts on non-zero nonce, and the `eth_call`'s from field implicitly increases the nonce of the account contract, causing the contract creation to revert. + const publicClient = createPublicClient({ + chain: sepolia, + transport: http(`${alchemyProvider.connection.url}`), + }); + expect( - await provider.signMessage( - "0xa70d0af2ebb03a44dcd0714a8724f622e3ab876d0aa312f0ee04823285d6fb1b" - ) - ).toBe( - "0x007ecc361d63ab82d89faeecfb79d40127f376c1ef3d545aeec3578eadce9d0c405a4d1ae6177bdebdc8413065014f735ee98da9643cc0e25c07a7423b694f8ae71b" - ); + await publicClient.verifyMessage({ + address: provider.account.address, + message, + signature, + // todo: This provider doesn't support signMessageWith6492, so we have to manually provider the factory data to the verifier. Add support here for 6492 signature generation. + factory: await provider.account.getFactoryAddress(), + factoryData: await provider.account.getFactoryData(), + }) + ).toBe(true); }); }); @@ -36,7 +49,7 @@ const givenConnectedProvider = async ({ alchemyProvider: AlchemyProvider; signer: Wallet; }) => { - const chain = polygonMumbai; + const chain = sepolia; return EthersProviderAdapter.fromEthersProvider( alchemyProvider, @@ -44,7 +57,6 @@ const givenConnectedProvider = async ({ ).connectToAccount( await createLightAccount({ chain, - accountAddress: "0x856185aedfab56809e6686d2d6d0c039d615bd9c", signer: convertWalletToAccountSigner(signer), transport: http(`${alchemyProvider.connection.url}`), }) diff --git a/account-kit/smart-contracts/src/light-account/accounts/base.ts b/account-kit/smart-contracts/src/light-account/accounts/base.ts index 71a5eecd5..e58a86bdd 100644 --- a/account-kit/smart-contracts/src/light-account/accounts/base.ts +++ b/account-kit/smart-contracts/src/light-account/accounts/base.ts @@ -139,35 +139,18 @@ export async function createLightAccountBase< }); }; - const signWith1271WrapperV1 = async (hashedMessage: Hex): Promise => { + const signWith1271Wrapper = async ( + hashedMessage: Hex, + version: string + ): Promise => { return signer.signTypedData({ // EIP712Domain(string name,string version,uint256 chainId,address verifyingContract) // https://github.com/alchemyplatform/light-account/blob/main/src/LightAccount.sol#L236 domain: { chainId: Number(client.chain.id), - name: "LightAccount", + name: type, verifyingContract: accountAddress, - version: "1", - }, - types: { - LightAccountMessage: [{ name: "message", type: "bytes" }], - }, - message: { - message: hashedMessage, - }, - primaryType: "LightAccountMessage", - }); - }; - - const signWith1271WrapperV2 = async (hashedMessage: Hex): Promise => { - return signer.signTypedData({ - // EIP712Domain(string name,string version,uint256 chainId,address verifyingContract) - // https://github.com/alchemyplatform/light-account/blob/main/src/LightAccount.sol#L236 - domain: { - chainId: Number(client.chain.id), - name: "LightAccount", - verifyingContract: accountAddress, - version: "2", + version, }, types: { LightAccountMessage: [{ name: "message", type: "bytes" }], @@ -227,9 +210,12 @@ export async function createLightAccountBase< case "v1.0.2": throw new Error(`${type} ${String(version)} doesn't support 1271`); case "v1.1.0": - return signWith1271WrapperV1(hashMessage(message)); + return signWith1271Wrapper(hashMessage(message), "1"); case "v2.0.0": - const signature = await signWith1271WrapperV2(hashMessage(message)); + const signature = await signWith1271Wrapper( + hashMessage(message), + "2" + ); // TODO: handle case where signer is an SCA. return concat([SignatureType.EOA, signature]); default: @@ -247,9 +233,12 @@ export async function createLightAccountBase< `Version ${String(version)} of LightAccount doesn't support 1271` ); case "v1.1.0": - return signWith1271WrapperV1(hashTypedData(params)); + return signWith1271Wrapper(hashTypedData(params), "1"); case "v2.0.0": - const signature = await signWith1271WrapperV2(hashTypedData(params)); + const signature = await signWith1271Wrapper( + hashTypedData(params), + "2" + ); // TODO: handle case where signer is an SCA. return concat([SignatureType.EOA, signature]); default: diff --git a/account-kit/smart-contracts/src/light-account/clients/multiOwnerLightAccount.test.ts b/account-kit/smart-contracts/src/light-account/clients/multiOwnerLightAccount.test.ts index 0878fc951..0b48893a4 100644 --- a/account-kit/smart-contracts/src/light-account/clients/multiOwnerLightAccount.test.ts +++ b/account-kit/smart-contracts/src/light-account/clients/multiOwnerLightAccount.test.ts @@ -6,7 +6,7 @@ import { type Address, type SmartAccountSigner, } from "@aa-sdk/core"; -import { custom, parseEther } from "viem"; +import { custom, parseEther, publicActions } from "viem"; import { generatePrivateKey } from "viem/accounts"; import { mine, setBalance } from "viem/actions"; import { accounts } from "~test/constants.js"; @@ -102,30 +102,41 @@ describe("MultiOwner Light Account Tests", () => { signer: undeployedSigner, }); + const typedData = { + types: { + Request: [{ name: "hello", type: "string" }], + }, + primaryType: "Request", + message: { + hello: "world", + }, + } as const; + + const signature = await account.signTypedDataWith6492(typedData); + expect( - await account.signTypedDataWith6492({ - types: { - Request: [{ name: "hello", type: "string" }], - }, - primaryType: "Request", - message: { - hello: "world", - }, + await client.extend(publicActions).verifyTypedData({ + address: account.address, + signature, + ...typedData, }) - ).toMatchInlineSnapshot( - `"0x000000000000000000000000000000000019d2ee9f2729a65afe20bb0020aefc000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000001200000000000000000000000000000000000000000000000000000000000000084b54c02f20000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000008391de4cacfb91f1cf953cf345948d92e137b6b900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004200e6821f95b4baff474a8f469467ac7c8f41c916932db259b8c1ca2d58ba327c4116fe018da508b7a0e65646a1c7b0ea6b8718ffbd6e1b184f79c846d817d727941c0000000000000000000000000000000000000000000000000000000000006492649264926492649264926492649264926492649264926492649264926492"` - ); + ).toBe(true); }); it("should sign message with 6492 successfully for undeployed account", async () => { const { account } = await givenConnectedProvider({ signer: undeployedSigner, }); + const message = "test"; + + const signature = await account.signMessageWith6492({ message }); expect( - await account.signMessageWith6492({ message: "test" }) - ).toMatchInlineSnapshot( - `"0x000000000000000000000000000000000019d2ee9f2729a65afe20bb0020aefc000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000001200000000000000000000000000000000000000000000000000000000000000084b54c02f20000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000008391de4cacfb91f1cf953cf345948d92e137b6b90000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000420014415f75bc2032cac0daac2ac988561fbb27e8f936eb730a920bb34003d4c2a23a1e9356aabf1eeda097e8af5f107fe0222742bce6ca57709fe240f9e1bb2a991b0000000000000000000000000000000000000000000000000000000000006492649264926492649264926492649264926492649264926492649264926492"` - ); + await client.extend(publicActions).verifyMessage({ + address: account.address, + message, + signature, + }) + ).toBe(true); }); it("should get on-chain owner addresses successfully", async () => { From 7b90c692a0f662aeb457fbc2c2f0909671e61a01 Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 13 Sep 2024 15:48:04 -0400 Subject: [PATCH 3/4] fix: use real RPC endpoint --- aa-sdk/ethers/src/__tests__/provider-adapter.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts index 013a3f9f3..dbb828829 100644 --- a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts +++ b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts @@ -1,3 +1,5 @@ +import dotenv from "dotenv"; + import { createLightAccount } from "@account-kit/smart-contracts"; import { Wallet } from "@ethersproject/wallet"; import { Alchemy, Network, type AlchemyProvider } from "alchemy-sdk"; @@ -5,6 +7,11 @@ import { createPublicClient, http, type Hex } from "viem"; import { sepolia } from "viem/chains"; import { EthersProviderAdapter } from "../../src/provider-adapter.js"; import { convertWalletToAccountSigner } from "../utils.js"; +dotenv.config(); + +const endpoint = + process.env.VITEST_SEPOLIA_FORK_URL ?? + "https://ethereum-sepolia-rpc.publicnode.com"; describe("Simple Account Tests", async () => { const alchemy = new Alchemy({ @@ -26,7 +33,7 @@ describe("Simple Account Tests", async () => { // We must use a public client, rather than an account client, to verify the message, because AA-SDK incorrectly attaches the account address as a "from" field to all actions taken by that client, including the `eth_call` used internally by viem's signature verifier logic. Per EIP-684, contract creation reverts on non-zero nonce, and the `eth_call`'s from field implicitly increases the nonce of the account contract, causing the contract creation to revert. const publicClient = createPublicClient({ chain: sepolia, - transport: http(`${alchemyProvider.connection.url}`), + transport: http(`${endpoint}`), }); expect( @@ -58,7 +65,7 @@ const givenConnectedProvider = async ({ await createLightAccount({ chain, signer: convertWalletToAccountSigner(signer), - transport: http(`${alchemyProvider.connection.url}`), + transport: http(`${endpoint}`), }) ); }; From 8bd39a653ceb3c1f569cd761a3e57a7466991ccf Mon Sep 17 00:00:00 2001 From: adam Date: Mon, 16 Sep 2024 17:17:38 -0400 Subject: [PATCH 4/4] chore: remove unneeded todo comment --- aa-sdk/ethers/src/__tests__/provider-adapter.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts index dbb828829..a73f31ac3 100644 --- a/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts +++ b/aa-sdk/ethers/src/__tests__/provider-adapter.test.ts @@ -27,7 +27,6 @@ describe("Simple Account Tests", async () => { it("should correctly sign the message", async () => { const provider = await givenConnectedProvider({ alchemyProvider, signer }); const message = "hello world"; - // todo: why does this provider's version of signMessage return a string, instead of Hex? const signature = (await provider.signMessage(message)) as Hex; // We must use a public client, rather than an account client, to verify the message, because AA-SDK incorrectly attaches the account address as a "from" field to all actions taken by that client, including the `eth_call` used internally by viem's signature verifier logic. Per EIP-684, contract creation reverts on non-zero nonce, and the `eth_call`'s from field implicitly increases the nonce of the account contract, causing the contract creation to revert.