diff --git a/examples/testapp/src/pages/auto-sub-account/index.page.tsx b/examples/testapp/src/pages/auto-sub-account/index.page.tsx index f774c1d997..7d51186aaa 100644 --- a/examples/testapp/src/pages/auto-sub-account/index.page.tsx +++ b/examples/testapp/src/pages/auto-sub-account/index.page.tsx @@ -81,6 +81,22 @@ export default function AutoSubAccount() { } }; + const handleEthAccounts = async () => { + if (!provider) return; + + try { + const response = await provider.request({ + method: 'eth_accounts', + params: [], + }); + setAccounts(response as string[]); + setLastResult(JSON.stringify(response, null, 2)); + } catch (e) { + console.error('error', e); + setLastResult(JSON.stringify(e, null, 2)); + } + }; + const handleSendTransaction = async () => { if (!provider || !accounts.length) return; @@ -385,6 +401,9 @@ export default function AutoSubAccount() { + diff --git a/packages/wallet-sdk/src/sign/scw/SCWSigner.test.ts b/packages/wallet-sdk/src/sign/scw/SCWSigner.test.ts index c0435746ad..18cef796b5 100644 --- a/packages/wallet-sdk/src/sign/scw/SCWSigner.test.ts +++ b/packages/wallet-sdk/src/sign/scw/SCWSigner.test.ts @@ -139,6 +139,7 @@ describe('SCWSigner', () => { store.keys.clear(); store.spendPermissions.clear(); store.subAccounts.clear(); + store.subAccountsConfig.clear(); store.setState({}); }); @@ -194,8 +195,8 @@ describe('SCWSigner', () => { await expect(signer.request({ method: 'eth_requestAccounts' })).resolves.toEqual([ '0xAddress', ]); + expect(mockCallback).toHaveBeenCalledWith('chainChanged', '0x1'); expect(mockCallback).toHaveBeenCalledWith('accountsChanged', ['0xAddress']); - expect(mockCallback).toHaveBeenCalledWith('connect', { chainId: '0x1' }); }); it('should perform a successful handshake for handshake', async () => { @@ -452,6 +453,64 @@ describe('SCWSigner', () => { }); }); + describe('eth_accounts', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should return accounts in correct order based on enableAutoSubAccounts', async () => { + // Set up the signer with a global account + signer['accounts'] = [globalAccountAddress]; + signer['chain'] = { id: 1, rpcUrl: 'https://eth-rpc.example.com/1' }; + + // Set a sub account in the store + const subAccountsSpy = vi.spyOn(store.subAccounts, 'get').mockReturnValue({ + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }); + + // Test with enableAutoSubAccounts = false + const configSpy = vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({ + enableAutoSubAccounts: false, + }); + + let accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); + + // Test with enableAutoSubAccounts = true + configSpy.mockReturnValue({ + enableAutoSubAccounts: true, + }); + + accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + + // Test when enableAutoSubAccounts is undefined (should default to false behavior) + configSpy.mockReturnValue(undefined); + + accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); + + subAccountsSpy.mockRestore(); + configSpy.mockRestore(); + }); + + it('should return only global account when no sub account exists', async () => { + // Set up the signer with only a global account + signer['accounts'] = [globalAccountAddress]; + signer['chain'] = { id: 1, rpcUrl: 'https://eth-rpc.example.com/1' }; + + // No sub account in the store + const subAccountsSpy = vi.spyOn(store.subAccounts, 'get').mockReturnValue(undefined); + + const accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([globalAccountAddress]); + + subAccountsSpy.mockRestore(); + }); + }); + describe('wallet_connect', () => { beforeEach(async () => { await signer.cleanup(); @@ -463,6 +522,10 @@ describe('SCWSigner', () => { await signer.handshake({ method: 'handshake' }); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should handle wallet_connect with no capabilities', async () => { expect(signer['accounts']).toEqual([]); const mockRequest: RequestArguments = { @@ -508,9 +571,9 @@ describe('SCWSigner', () => { factoryData: '0x', }); - // eth_accounts should return only global account + // eth_accounts should return both accounts with global account first const accounts = await signer.request({ method: 'eth_accounts' }); - expect(accounts).toEqual([globalAccountAddress]); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); }); it('should handle wallet_connect with addSubAccount capability', async () => { @@ -569,10 +632,10 @@ describe('SCWSigner', () => { factoryData: '0x', }); - // eth_accounts should return [subAccount, globalAccount] + // eth_accounts should return [globalAccount, subAccount] when enableAutoSubAccounts is not true const accounts = await signer.request({ method: 'eth_accounts' }); - expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); }); it('should handle wallet_addSubAccount creating new sub account', async () => { @@ -635,9 +698,9 @@ describe('SCWSigner', () => { factoryData: '0x', }); - // eth_accounts should return [subAccount, globalAccount] + // eth_accounts should return [globalAccount, subAccount] when enableAutoSubAccounts is not true const accounts = await signer.request({ method: 'eth_accounts' }); - expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); }); it('should route eth_requestAccounts through wallet_connect', async () => { @@ -819,9 +882,71 @@ describe('SCWSigner', () => { ], }); }); + + it('should always return sub account first when enableAutoSubAccounts is true', async () => { + expect(signer['accounts']).toEqual([]); + + // Enable auto sub accounts + vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({ + enableAutoSubAccounts: true, + }); + + const mockRequest: RequestArguments = { + method: 'wallet_connect', + params: [], + }; + + const mockSetAccount = vi.spyOn(store.account, 'set'); + const mockSetSubAccounts = vi.spyOn(store.subAccounts, 'set'); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + accounts: [ + { + address: globalAccountAddress, + capabilities: { + subAccounts: [ + { + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }, + ], + }, + }, + ], + }, + }, + }); + + await signer.request(mockRequest); + + // Should persist accounts correctly + expect(mockSetAccount).toHaveBeenCalledWith({ + accounts: [globalAccountAddress], + }); + expect(mockSetSubAccounts).toHaveBeenCalledWith({ + address: subAccountAddress, + factory: globalAccountAddress, + factoryData: '0x', + }); + + // When enableAutoSubAccounts is true, sub account should be first + const accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + + // Test with eth_requestAccounts as well + const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' }); + expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]); + }); }); describe('wallet_addSubAccount', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should update internal state for successful wallet_addSubAccount', async () => { await signer.cleanup(); @@ -884,7 +1009,7 @@ describe('SCWSigner', () => { }); const accounts = await signer.request({ method: 'eth_accounts' }); - expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); expect(mockSetAccount).toHaveBeenCalledWith({ accounts: [globalAccountAddress], @@ -974,8 +1099,82 @@ describe('SCWSigner', () => { mockCryptoKey ); + const accounts = await signer.request({ method: 'eth_accounts' }); + expect(accounts).toEqual([globalAccountAddress, subAccountAddress]); + }); + + it('should always return sub account first when enableAutoSubAccounts is true', async () => { + await signer.cleanup(); + + // Enable auto sub accounts + vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({ + enableAutoSubAccounts: true, + }); + + const mockRequest: RequestArguments = { + method: 'wallet_connect', + params: [], + }; + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: null, + }, + }); + + await signer.handshake({ method: 'handshake' }); + expect(signer['accounts']).toEqual([]); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + accounts: [ + { + address: globalAccountAddress, + capabilities: {}, + }, + ], + }, + }, + }); + + await signer.request(mockRequest); + + (decryptContent as Mock).mockResolvedValueOnce({ + result: { + value: { + address: subAccountAddress, + factory: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54', + factoryData: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54', + }, + }, + }); + + await signer.request({ + method: 'wallet_addSubAccount', + params: [ + { + version: '1', + account: { + type: 'create', + keys: [ + { + publicKey: '0x123', + type: 'p256', + }, + ], + }, + }, + ], + }); + + // wallet_addSubAccount now respects enableAutoSubAccounts, so sub account should be first const accounts = await signer.request({ method: 'eth_accounts' }); expect(accounts).toEqual([subAccountAddress, globalAccountAddress]); + + // However, eth_requestAccounts will reorder based on enableAutoSubAccounts + const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' }); + expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]); }); }); @@ -1024,6 +1223,10 @@ describe('SCWSigner', () => { }); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should create a sub account when eth_requestAccounts is called', async () => { const mockRequest: RequestArguments = { method: 'eth_requestAccounts', @@ -1042,6 +1245,13 @@ describe('SCWSigner', () => { }); (findOwnerIndex as Mock).mockResolvedValueOnce(-1); + (handleAddSubAccountOwner as Mock).mockResolvedValueOnce(0); + + // Ensure createSubAccountSigner returns the expected shape + (createSubAccountSigner as Mock).mockResolvedValueOnce({ + request: vi.fn().mockResolvedValue('0xResult'), + }); + (decryptContent as Mock).mockResolvedValueOnce({ result: { value: null, diff --git a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts index 6844352bc2..612c9a36a6 100644 --- a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts +++ b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts @@ -7,7 +7,7 @@ import { RPCRequestMessage, RPCResponseMessage } from ':core/message/RPCMessage. import { RPCResponse } from ':core/message/RPCResponse.js'; import { AppMetadata, ProviderEventCallback, RequestArguments } from ':core/provider/interface.js'; import { FetchPermissionsResponse } from ':core/rpc/coinbase_fetchSpendPermissions.js'; -import { WalletConnectRequest, WalletConnectResponse } from ':core/rpc/wallet_connect.js'; +import { WalletConnectResponse } from ':core/rpc/wallet_connect.js'; import { GetSubAccountsResponse } from ':core/rpc/wallet_getSubAccount.js'; import { logHandshakeCompleted, @@ -48,6 +48,7 @@ import { Signer } from '../interface.js'; import { SCWKeyManager } from './SCWKeyManager.js'; import { addSenderToRequest, + appendWithoutDuplicates, assertFetchPermissionsRequest, assertGetCapabilitiesParams, assertParamsChainId, @@ -223,18 +224,20 @@ export class SCWSigner implements Signer { } switch (request.method) { - case 'eth_requestAccounts': { - // if auto sub accounts are enabled and we have a sub account, we need to return it as a top level account + case 'eth_requestAccounts': + case 'eth_accounts': { const subAccount = store.subAccounts.get(); + const subAccountsConfig = store.subAccountsConfig.get(); if (subAccount?.address) { - this.accounts = prependWithoutDuplicates(this.accounts, subAccount.address); + // if auto sub accounts are enabled and we have a sub account, we need to return it as a top level account + // otherwise, we just append it to the accounts array + this.accounts = subAccountsConfig?.enableAutoSubAccounts + ? prependWithoutDuplicates(this.accounts, subAccount.address) + : appendWithoutDuplicates(this.accounts, subAccount.address); } - this.callback?.('connect', { chainId: numberToHex(this.chain.id) }); return this.accounts; } - case 'eth_accounts': - return this.accounts; case 'eth_coinbase': return this.accounts[0]; case 'net_version': @@ -367,7 +370,6 @@ export class SCWSigner implements Signer { const account = response.accounts.at(0); const capabilities = account?.capabilities; - const requestCapabilities = (request as WalletConnectRequest).params?.[0]?.capabilities; if (capabilities?.subAccounts) { const capabilityResponse = capabilities?.subAccounts; @@ -384,12 +386,11 @@ export class SCWSigner implements Signer { const subAccount = store.subAccounts.get(); const subAccountsConfig = store.subAccountsConfig.get(); - // Sub account should be returned as a top level account if auto sub accounts are enabled - const shouldUseSubAccount = - subAccountsConfig?.enableAutoSubAccounts || !!requestCapabilities?.addSubAccount; - - if (subAccount?.address && shouldUseSubAccount) { - this.accounts = prependWithoutDuplicates(this.accounts, subAccount.address); + if (subAccount?.address) { + // Sub account should be returned as a top level account if auto sub accounts are enabled + this.accounts = subAccountsConfig?.enableAutoSubAccounts + ? prependWithoutDuplicates(this.accounts, subAccount.address) + : appendWithoutDuplicates(this.accounts, subAccount.address); } const spendPermissions = response?.accounts?.[0].capabilities?.spendPermissions; @@ -405,9 +406,10 @@ export class SCWSigner implements Signer { assertSubAccount(result.value); const subAccount = result.value; store.subAccounts.set(subAccount); - - // Sub account becomes the active account - this.accounts = prependWithoutDuplicates(this.accounts, subAccount.address); + const subAccountsConfig = store.subAccountsConfig.get(); + this.accounts = subAccountsConfig?.enableAutoSubAccounts + ? prependWithoutDuplicates(this.accounts, subAccount.address) + : appendWithoutDuplicates(this.accounts, subAccount.address); this.callback?.('accountsChanged', this.accounts); break; } @@ -596,9 +598,11 @@ export class SCWSigner implements Signer { }> { const state = store.getState(); const subAccount = state.subAccount; + const subAccountsConfig = store.subAccountsConfig.get(); if (subAccount?.address) { - // Move the sub account to the top level accounts to make it active - this.accounts = prependWithoutDuplicates(this.accounts, subAccount.address); + this.accounts = subAccountsConfig?.enableAutoSubAccounts + ? prependWithoutDuplicates(this.accounts, subAccount.address) + : appendWithoutDuplicates(this.accounts, subAccount.address); this.callback?.('accountsChanged', this.accounts); return subAccount; } @@ -639,7 +643,6 @@ export class SCWSigner implements Signer { const response = await this.sendRequestToPopup(request); assertSubAccount(response); - return response; } diff --git a/packages/wallet-sdk/src/sign/scw/utils.test.ts b/packages/wallet-sdk/src/sign/scw/utils.test.ts index afb2a5fddb..03ee2b0989 100644 --- a/packages/wallet-sdk/src/sign/scw/utils.test.ts +++ b/packages/wallet-sdk/src/sign/scw/utils.test.ts @@ -3,6 +3,7 @@ import { hashTypedData, hexToBigInt, numberToHex } from 'viem'; import { SpendPermissionBatch, addSenderToRequest, + appendWithoutDuplicates, assertFetchPermissionsRequest, assertGetCapabilitiesParams, assertParamsChainId, @@ -120,9 +121,15 @@ describe('assertGetCapabilitiesParams', () => { expect(() => assertGetCapabilitiesParams(['0x123'])).toThrow(); // Too short expect(() => assertGetCapabilitiesParams(['0x123abc'])).toThrow(); // Too short expect(() => assertGetCapabilitiesParams(['xyz123'])).toThrow(); // No 0x prefix - expect(() => assertGetCapabilitiesParams(['0x12345678901234567890123456789012345678gg'])).toThrow(); // Invalid hex characters - expect(() => assertGetCapabilitiesParams(['0x123456789012345678901234567890123456789'])).toThrow(); // Too short (39 chars) - expect(() => assertGetCapabilitiesParams(['0x12345678901234567890123456789012345678901'])).toThrow(); // Too long (41 chars) + expect(() => + assertGetCapabilitiesParams(['0x12345678901234567890123456789012345678gg']) + ).toThrow(); // Invalid hex characters + expect(() => + assertGetCapabilitiesParams(['0x123456789012345678901234567890123456789']) + ).toThrow(); // Too short (39 chars) + expect(() => + assertGetCapabilitiesParams(['0x12345678901234567890123456789012345678901']) + ).toThrow(); // Too long (41 chars) }); it('should not throw for valid single parameter (valid Ethereum address)', () => { @@ -148,7 +155,9 @@ describe('assertGetCapabilitiesParams', () => { it('should not throw for valid parameters with filter array', () => { expect(() => assertGetCapabilitiesParams([VALID_ADDRESS_1, []])).not.toThrow(); expect(() => assertGetCapabilitiesParams([VALID_ADDRESS_1, ['0x1']])).not.toThrow(); - expect(() => assertGetCapabilitiesParams([VALID_ADDRESS_1, ['0x1', '0x2', '0x3']])).not.toThrow(); + expect(() => + assertGetCapabilitiesParams([VALID_ADDRESS_1, ['0x1', '0x2', '0x3']]) + ).not.toThrow(); expect(() => assertGetCapabilitiesParams([VALID_ADDRESS_1, ['0xabcdef', '0x0']])).not.toThrow(); expect(() => assertGetCapabilitiesParams([VALID_ADDRESS_2, ['0x1', '0xa']])).not.toThrow(); }); @@ -476,6 +485,16 @@ describe('prependWithoutDuplicates', () => { }); }); +describe('appendWithoutDuplicates', () => { + it('should append an item to an array without duplicates', () => { + expect(appendWithoutDuplicates(['1', '2', '3'], '4')).toEqual(['1', '2', '3', '4']); + }); + + it('should move an existing item to the end of the array', () => { + expect(appendWithoutDuplicates(['1', '2', '3'], '2')).toEqual(['1', '3', '2']); + }); +}); + describe('getCachedWalletConnectResponse', () => { beforeEach(() => { vi.spyOn(store.spendPermissions, 'get').mockReturnValue([]); diff --git a/packages/wallet-sdk/src/sign/scw/utils.ts b/packages/wallet-sdk/src/sign/scw/utils.ts index 8be8eb05ab..cc3a3e7727 100644 --- a/packages/wallet-sdk/src/sign/scw/utils.ts +++ b/packages/wallet-sdk/src/sign/scw/utils.ts @@ -577,6 +577,17 @@ export function prependWithoutDuplicates(array: T[], item: T): T[] { return [item, ...filtered]; } +/** + * Appends an item to an array without duplicates + * @param array The array to append to + * @param item The item to append + * @returns The array with the item appended + */ +export function appendWithoutDuplicates(array: T[], item: T): T[] { + const filtered = array.filter((i) => i !== item); + return [...filtered, item]; +} + export async function getCachedWalletConnectResponse(): Promise { const spendPermissions = store.spendPermissions.get(); const subAccount = store.subAccounts.get(); @@ -591,7 +602,8 @@ export async function getCachedWalletConnectResponse(): Promise 0 ? { permissions: spendPermissions } : undefined, + spendPermissions: + spendPermissions.length > 0 ? { permissions: spendPermissions } : undefined, }, }) );