From 84f21c0e040f7bfc6033865774e64609396787dc Mon Sep 17 00:00:00 2001 From: Stephan Cilliers Date: Tue, 28 Oct 2025 14:59:28 +0200 Subject: [PATCH 1/2] fix: sub account caching policy only cache known address --- .../src/sign/base-account/Signer.test.ts | 164 +++++++++++++++++- .../src/sign/base-account/Signer.ts | 33 +++- 2 files changed, 185 insertions(+), 12 deletions(-) diff --git a/packages/account-sdk/src/sign/base-account/Signer.test.ts b/packages/account-sdk/src/sign/base-account/Signer.test.ts index 50b37b32..b8e20627 100644 --- a/packages/account-sdk/src/sign/base-account/Signer.test.ts +++ b/packages/account-sdk/src/sign/base-account/Signer.test.ts @@ -1026,7 +1026,9 @@ describe('Signer', () => { expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); // Test with eth_requestAccounts as well - const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' }); + const requestedAccounts = await signer.request({ + method: 'eth_requestAccounts', + }); expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]); }); }); @@ -1262,9 +1264,157 @@ describe('Signer', () => { expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); // eth_requestAccounts will also order based on defaultAccount - const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' }); + const requestedAccounts = await signer.request({ + method: 'eth_requestAccounts', + }); expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]); }); + + it('should return cached sub account when requested address matches', async () => { + await signer.cleanup(); + + // Setup initial connection + (decryptContent as Mock).mockResolvedValueOnce({ + result: { value: null }, + }); + await signer.handshake({ method: 'handshake' }); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + accounts: [{ address: globalAccountAddress, capabilities: {} }], + }, + }, + }); + await signer.request({ method: 'wallet_connect', params: [] }); + + // Cache a sub account + store.subAccounts.set({ + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }); + + // Request same address (isAddressEqual handles case-insensitive comparison) + const result = await signer.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'deployed', + address: subAccountAddress, + chainId: '0x14a34', + }, + }, + ], + }); + + // Should return cached without calling backend + expect(result.address).toBe(subAccountAddress); + expect(decryptContent).toHaveBeenCalledTimes(2); // Only handshake + connect, not addSubAccount + }); + + it('should fetch from backend when requested address differs from cached', async () => { + await signer.cleanup(); + + const secondSubAccountAddress = '0x9999999999999999999999999999999999999999'; + + // Setup initial connection + (decryptContent as Mock).mockResolvedValueOnce({ + result: { value: null }, + }); + await signer.handshake({ method: 'handshake' }); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + accounts: [{ address: globalAccountAddress, capabilities: {} }], + }, + }, + }); + await signer.request({ method: 'wallet_connect', params: [] }); + + // Cache a sub account + store.subAccounts.set({ + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }); + + // Request different address + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + address: secondSubAccountAddress, + factory: globalAccountAddress, + factoryData: '0x123', + }, + }, + }); + + const result = await signer.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'deployed', + address: secondSubAccountAddress, + chainId: '0x14a34', + }, + }, + ], + }); + + // Should call backend and return new sub account + expect(result.address).toBe(secondSubAccountAddress); + expect(decryptContent).toHaveBeenCalledTimes(3); // handshake + connect + addSubAccount + }); + + it('should return cached sub account for create type (no address specified)', async () => { + await signer.cleanup(); + + // Setup initial connection + (decryptContent as Mock).mockResolvedValueOnce({ + result: { value: null }, + }); + await signer.handshake({ method: 'handshake' }); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + accounts: [{ address: globalAccountAddress, capabilities: {} }], + }, + }, + }); + await signer.request({ method: 'wallet_connect', params: [] }); + + // Cache a sub account + store.subAccounts.set({ + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }); + + // Request create type (no specific address) + const result = await signer.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'create', + keys: [{ publicKey: '0x123', type: 'p256' }], + }, + }, + ], + }); + + // Should return cached without calling backend + expect(result.address).toBe(subAccountAddress); + expect(decryptContent).toHaveBeenCalledTimes(2); // Only handshake + connect, not addSubAccount + }); }); describe('auto sub account', () => { @@ -1856,7 +2006,10 @@ describe('Signer', () => { }); // Set the chain to match the mocked client - signer['chain'] = { id: 84532, rpcUrl: 'https://eth-rpc.example.com/84532' }; + signer['chain'] = { + id: 84532, + rpcUrl: 'https://eth-rpc.example.com/84532', + }; signer['accounts'] = [globalAccountAddress]; // Setup basic handshake @@ -2144,7 +2297,10 @@ describe('Signer', () => { }); // Set the chain to match the mocked client - signer['chain'] = { id: 84532, rpcUrl: 'https://eth-rpc.example.com/84532' }; + signer['chain'] = { + id: 84532, + rpcUrl: 'https://eth-rpc.example.com/84532', + }; signer['accounts'] = [globalAccountAddress]; // Mock decryptContent for wallet_connect to set up accounts diff --git a/packages/account-sdk/src/sign/base-account/Signer.ts b/packages/account-sdk/src/sign/base-account/Signer.ts index 065ddb5f..1df1c084 100644 --- a/packages/account-sdk/src/sign/base-account/Signer.ts +++ b/packages/account-sdk/src/sign/base-account/Signer.ts @@ -603,15 +603,32 @@ export class Signer { factoryData?: Hex; }> { const state = store.getState(); - const subAccount = state.subAccount; + const cachedSubAccount = state.subAccount; const subAccountsConfig = store.subAccountsConfig.get(); - if (subAccount?.address) { - this.accounts = - subAccountsConfig?.defaultAccount === 'sub' - ? prependWithoutDuplicates(this.accounts, subAccount.address) - : appendWithoutDuplicates(this.accounts, subAccount.address); - this.callback?.('accountsChanged', this.accounts); - return subAccount; + + // Extract requested address from params (for deployed/undeployed types) + const requestedAddress = + Array.isArray(request.params) && + request.params.length > 0 && + request.params[0]?.account?.address + ? request.params[0].account.address + : undefined; + + // Only return cached if: + // 1. Cache exists AND + // 2. No specific address requested (create type) OR requested address matches cached + if (cachedSubAccount?.address) { + const shouldUseCache = + !requestedAddress || isAddressEqual(requestedAddress, cachedSubAccount.address); + + if (shouldUseCache) { + this.accounts = + subAccountsConfig?.defaultAccount === 'sub' + ? prependWithoutDuplicates(this.accounts, cachedSubAccount.address) + : appendWithoutDuplicates(this.accounts, cachedSubAccount.address); + this.callback?.('accountsChanged', this.accounts); + return cachedSubAccount; + } } // Wait for the popup to be loaded before sending the request From 12b4d81696dcb9193030d63601af5876eaedcaec Mon Sep 17 00:00:00 2001 From: Stephan Cilliers Date: Tue, 28 Oct 2025 15:11:12 +0200 Subject: [PATCH 2/2] feat: new playground --- .../components/AccountsList.tsx | 379 ++++++++++++++++++ .../components/AddGlobalOwner.tsx | 50 ++- .../components/AddSubAccountDeployed.tsx | 22 +- .../components/AddSubAccountUndeployed.tsx | 22 +- .../components/DeploySubAccount.tsx | 13 +- .../pages/import-sub-account/index.page.tsx | 166 +++++--- .../utils/unsafe_manageMultipleAccounts.ts | 69 ++++ 7 files changed, 645 insertions(+), 76 deletions(-) create mode 100644 examples/testapp/src/pages/import-sub-account/components/AccountsList.tsx create mode 100644 examples/testapp/src/utils/unsafe_manageMultipleAccounts.ts diff --git a/examples/testapp/src/pages/import-sub-account/components/AccountsList.tsx b/examples/testapp/src/pages/import-sub-account/components/AccountsList.tsx new file mode 100644 index 00000000..35dba4de --- /dev/null +++ b/examples/testapp/src/pages/import-sub-account/components/AccountsList.tsx @@ -0,0 +1,379 @@ +import { createBaseAccountSDK } from '@base-org/account'; +import { DeleteIcon } from '@chakra-ui/icons'; +import { + Badge, + Box, + Button, + Divider, + HStack, + IconButton, + Input, + Text, + VStack, + useToast, +} from '@chakra-ui/react'; +import { useCallback, useEffect, useState } from 'react'; +import { numberToHex } from 'viem'; +import { SmartAccount } from 'viem/account-abstraction'; +import { baseSepolia } from 'viem/chains'; +import { abi } from '../../../constants'; +import type { StoredAccount } from '../../../utils/unsafe_manageMultipleAccounts'; +import { AddGlobalOwner } from './AddGlobalOwner'; +import { DeploySubAccount } from './DeploySubAccount'; + +type AccountsListProps = { + accounts: Array<{ + stored: StoredAccount; + smartAccount: SmartAccount; + isDeployed: boolean; + }>; + sdk: ReturnType; + onAddAccount: () => void; + onRemoveAccount: (id: string) => void; + onAccountDeployed: (id: string) => void; +}; + +export function AccountsList({ + accounts, + sdk, + onAddAccount, + onRemoveAccount, + onAccountDeployed, +}: AccountsListProps) { + const toast = useToast(); + const [importingAccountId, setImportingAccountId] = useState(null); + const [customLabels, setCustomLabels] = useState>({}); + const [ownershipStatus, setOwnershipStatus] = useState>({}); + const [connectedAddress, setConnectedAddress] = useState(null); + + // Get connected address + useEffect(() => { + const getConnectedAddress = async () => { + if (!sdk) return; + + try { + const provider = sdk.getProvider(); + const addresses = (await provider.request({ + method: 'eth_accounts', + })) as string[]; + if (addresses && addresses.length > 0) { + setConnectedAddress(addresses[0]); + } + } catch (error) { + console.error('Failed to get connected address:', error); + } + }; + + getConnectedAddress(); + }, [sdk]); + + // Check ownership for deployed accounts + useEffect(() => { + const checkOwnership = async () => { + if (!connectedAddress) return; + + const newOwnershipStatus: Record = {}; + + await Promise.all( + accounts.map(async (account) => { + if (!account.isDeployed) { + // Undeployed accounts don't have ownership yet + newOwnershipStatus[account.stored.id] = false; + return; + } + + try { + // @ts-ignore - viem type inference issue with SmartAccount client + const isOwner = (await account.smartAccount.client.readContract({ + address: account.smartAccount.address, + abi, + functionName: 'isOwnerAddress', + args: [connectedAddress as `0x${string}`], + })) as boolean; + + newOwnershipStatus[account.stored.id] = isOwner; + } catch (error) { + console.error(`Failed to check ownership for ${account.smartAccount.address}:`, error); + newOwnershipStatus[account.stored.id] = false; + } + }) + ); + + setOwnershipStatus(newOwnershipStatus); + }; + + checkOwnership(); + }, [accounts, connectedAddress]); + + const handleImportAccount = useCallback( + async (account: (typeof accounts)[0]) => { + if (!sdk) { + toast({ + title: 'SDK not initialized', + status: 'error', + duration: 3000, + }); + return; + } + + setImportingAccountId(account.stored.id); + + try { + const provider = sdk.getProvider(); + await provider.request({ + method: 'wallet_switchEthereumChain', + params: [{ chainId: numberToHex(84532) }], + }); + + const customLabel = customLabels[account.stored.id] || ''; + + if (account.isDeployed) { + // Import as deployed + const response = (await provider.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'deployed', + address: account.smartAccount.address, + chainId: baseSepolia.id, + ...(customLabel && { label: customLabel }), + }, + }, + ], + })) as { address: string }; + + toast({ + title: 'Deployed account imported', + description: `Address: ${response.address}${ + customLabel ? `\nLabel: ${customLabel}` : '' + }`, + status: 'success', + duration: 5000, + }); + } else { + // Import as undeployed + const factoryArgs = await account.smartAccount.getFactoryArgs(); + const response = (await provider.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'undeployed', + address: account.smartAccount.address, + factory: factoryArgs?.factory, + factoryData: factoryArgs?.factoryData, + ...(customLabel && { label: customLabel }), + }, + }, + ], + })) as { address: string }; + + toast({ + title: 'Undeployed account imported', + description: `Address: ${response.address}${ + customLabel ? `\nLabel: ${customLabel}` : '' + }`, + status: 'success', + duration: 5000, + }); + } + } catch (error) { + console.error('Failed to import account:', error); + toast({ + title: 'Import failed', + description: error instanceof Error ? error.message : 'Unknown error', + status: 'error', + duration: 5000, + }); + } finally { + setImportingAccountId(null); + } + }, + [sdk, customLabels, toast] + ); + + const handleLabelChange = useCallback((accountId: string, label: string) => { + setCustomLabels((prev) => ({ + ...prev, + [accountId]: label, + })); + }, []); + + const handleOwnerAdded = useCallback( + async (accountId: string) => { + if (!connectedAddress) return; + + // Re-check ownership status for this account + const account = accounts.find((acc) => acc.stored.id === accountId); + if (!account || !account.isDeployed) return; + + try { + // @ts-ignore - viem type inference issue with SmartAccount client + const isOwner = (await account.smartAccount.client.readContract({ + address: account.smartAccount.address, + abi, + functionName: 'isOwnerAddress', + args: [connectedAddress as `0x${string}`], + })) as boolean; + + setOwnershipStatus((prev) => ({ + ...prev, + [accountId]: isOwner, + })); + + if (isOwner) { + toast({ + title: 'Owner added successfully', + description: 'You can now import this account', + status: 'success', + duration: 3000, + }); + } + } catch (error) { + console.error('Failed to re-check ownership:', error); + } + }, + [accounts, connectedAddress, toast] + ); + + const truncateAddress = (address: string) => { + return `${address.slice(0, 6)}...${address.slice(-4)}`; + }; + + return ( + + + + Test Sub Accounts ({accounts.length}) + + + + + {accounts.length === 0 ? ( + + + No test accounts yet. Click "Generate New Account" to create one. + + + ) : ( + accounts.map((account, index) => ( + + + + + + Account #{index + 1} + + + {account.isDeployed ? 'Deployed' : 'Undeployed'} + + + } + size="sm" + colorScheme="red" + variant="ghost" + onClick={() => onRemoveAccount(account.stored.id)} + /> + + + + + Address + + + {truncateAddress(account.smartAccount.address)} + + + + + + Chain + + + + Base Sepolia + + + {baseSepolia.id} + + + + + + + + handleLabelChange(account.stored.id, e.target.value)} + bg="gray.50" + _dark={{ bg: 'gray.800' }} + /> + + {!account.isDeployed && ( + onAccountDeployed(account.stored.id)} + /> + )} + + {account.isDeployed && !ownershipStatus[account.stored.id] && ( + <> + + + ⚠️ Connected address is not an owner. Add yourself as an owner first. + + + handleOwnerAdded(account.stored.id)} + /> + + )} + + + + + + )) + )} + + ); +} diff --git a/examples/testapp/src/pages/import-sub-account/components/AddGlobalOwner.tsx b/examples/testapp/src/pages/import-sub-account/components/AddGlobalOwner.tsx index 938b3e06..e0cf78d7 100644 --- a/examples/testapp/src/pages/import-sub-account/components/AddGlobalOwner.tsx +++ b/examples/testapp/src/pages/import-sub-account/components/AddGlobalOwner.tsx @@ -9,26 +9,31 @@ import { abi } from '../../../constants'; export function AddGlobalOwner({ sdk, subAccount, + onOwnerAdded, }: { sdk: ReturnType; subAccount: SmartAccount; + onOwnerAdded?: () => void; }) { const [state, setState] = useState(); + const [isAdding, setIsAdding] = useState(false); const handleAddGlobalOwner = useCallback(async () => { - if (!sdk) { + if (!sdk || isAdding) { return; } - const provider = sdk.getProvider(); - const accounts = await provider.request({ - method: 'eth_accounts', - }); - - // biome-ignore lint/suspicious/noConsole: internal logging - console.log('customlogs: accounts', accounts); + setIsAdding(true); try { + const provider = sdk.getProvider(); + const accounts = await provider.request({ + method: 'eth_accounts', + }); + + // biome-ignore lint/suspicious/noConsole: internal logging + console.log('customlogs: accounts', accounts); + const client = createPublicClient({ chain: baseSepolia, transport: http(), @@ -46,7 +51,7 @@ export function AddGlobalOwner({ ), paymaster: paymasterClient, }); - // @ts-expect-error + // @ts-ignore const hash = await bundlerClient.sendUserOperation({ calls: [ { @@ -62,26 +67,37 @@ export function AddGlobalOwner({ }); console.info('response', hash); - setState(hash as string); + setState(`Adding owner... UserOp: ${hash as string}`); + + // Wait a bit for the transaction to be processed + setTimeout(() => { + setState(`Owner added! UserOp: ${hash as string}`); + onOwnerAdded?.(); + }, 2000); } catch (e) { console.error('error', e); + setState(`Error: ${e instanceof Error ? e.message : 'Unknown error'}`); + } finally { + setIsAdding(false); } - }, [sdk, subAccount]); + }, [sdk, subAccount, onOwnerAdded, isAdding]); return ( <> )} diff --git a/examples/testapp/src/utils/unsafe_manageMultipleAccounts.ts b/examples/testapp/src/utils/unsafe_manageMultipleAccounts.ts new file mode 100644 index 00000000..6de3177f --- /dev/null +++ b/examples/testapp/src/utils/unsafe_manageMultipleAccounts.ts @@ -0,0 +1,69 @@ +import { generatePrivateKey } from 'viem/accounts'; +import type { Hex } from 'viem'; + +const STORAGE_KEY = 'base-acc-sdk.demo.sub-accounts.pks'; + +export type StoredAccount = { + id: string; + privateKey: Hex; + label?: string; +}; + +/** + * Manage multiple private keys in local storage for testing sub accounts + * + * This is not safe, this is only for testing + * In a real app you should not store/expose private keys + */ +export function unsafe_manageMultipleAccounts() { + function getAll(): StoredAccount[] { + const stored = localStorage.getItem(STORAGE_KEY); + if (!stored) { + return []; + } + try { + return JSON.parse(stored); + } catch { + return []; + } + } + + function add(label?: string): StoredAccount { + const accounts = getAll(); + const newAccount: StoredAccount = { + id: crypto.randomUUID(), + privateKey: generatePrivateKey(), + label, + }; + accounts.push(newAccount); + localStorage.setItem(STORAGE_KEY, JSON.stringify(accounts)); + return newAccount; + } + + function remove(id: string): void { + const accounts = getAll(); + const filtered = accounts.filter((acc) => acc.id !== id); + localStorage.setItem(STORAGE_KEY, JSON.stringify(filtered)); + } + + function updateLabel(id: string, label: string): void { + const accounts = getAll(); + const account = accounts.find((acc) => acc.id === id); + if (account) { + account.label = label; + localStorage.setItem(STORAGE_KEY, JSON.stringify(accounts)); + } + } + + function clear(): void { + localStorage.removeItem(STORAGE_KEY); + } + + return { + getAll, + add, + remove, + updateLabel, + clear, + }; +}