Skip to content

Commit d9f686b

Browse files
stephancillstephancill
committed
fix: update manual sub account odering behavior
1 parent 4539500 commit d9f686b

File tree

5 files changed

+330
-33
lines changed

5 files changed

+330
-33
lines changed

‎examples/testapp/src/pages/auto-sub-account/index.page.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,22 @@ export default function AutoSubAccount() {
8181
}
8282
};
8383

84+
const handleEthAccounts = async () => {
85+
if (!provider) return;
86+
87+
try {
88+
const response = await provider.request({
89+
method: 'eth_accounts',
90+
params: [],
91+
});
92+
setAccounts(response as string[]);
93+
setLastResult(JSON.stringify(response, null, 2));
94+
} catch (e) {
95+
console.error('error', e);
96+
setLastResult(JSON.stringify(e, null, 2));
97+
}
98+
};
99+
84100
const handleSendTransaction = async () => {
85101
if (!provider || !accounts.length) return;
86102

@@ -385,6 +401,9 @@ export default function AutoSubAccount() {
385401
<Button w="full" onClick={handleRequestAccounts}>
386402
eth_requestAccounts
387403
</Button>
404+
<Button w="full" onClick={handleEthAccounts}>
405+
eth_accounts
406+
</Button>
388407
<Button w="full" onClick={handleSendTransaction} isDisabled={!accounts.length}>
389408
eth_sendTransaction
390409
</Button>

‎packages/wallet-sdk/src/sign/scw/SCWSigner.test.ts‎

Lines changed: 252 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ describe('SCWSigner', () => {
139139
store.keys.clear();
140140
store.spendPermissions.clear();
141141
store.subAccounts.clear();
142+
store.subAccountsConfig.clear();
142143
store.setState({});
143144
});
144145

@@ -194,8 +195,8 @@ describe('SCWSigner', () => {
194195
await expect(signer.request({ method: 'eth_requestAccounts' })).resolves.toEqual([
195196
'0xAddress',
196197
]);
198+
expect(mockCallback).toHaveBeenCalledWith('chainChanged', '0x1');
197199
expect(mockCallback).toHaveBeenCalledWith('accountsChanged', ['0xAddress']);
198-
expect(mockCallback).toHaveBeenCalledWith('connect', { chainId: '0x1' });
199200
});
200201

201202
it('should perform a successful handshake for handshake', async () => {
@@ -452,6 +453,98 @@ describe('SCWSigner', () => {
452453
});
453454
});
454455

456+
describe('eth_accounts', () => {
457+
afterEach(() => {
458+
vi.restoreAllMocks();
459+
});
460+
461+
it('should return accounts in correct order based on enableAutoSubAccounts', async () => {
462+
// Set up the signer with a global account
463+
signer['accounts'] = [globalAccountAddress];
464+
signer['chain'] = { id: 1, rpcUrl: 'https://eth-rpc.example.com/1' };
465+
466+
// Set a sub account in the store
467+
const subAccountsSpy = vi.spyOn(store.subAccounts, 'get').mockReturnValue({
468+
address: subAccountAddress,
469+
factory: globalAccountAddress,
470+
factoryData: '0x',
471+
});
472+
473+
// Test with enableAutoSubAccounts = false
474+
const configSpy = vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
475+
enableAutoSubAccounts: false,
476+
});
477+
478+
let accounts = await signer.request({ method: 'eth_accounts' });
479+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
480+
481+
// Test with enableAutoSubAccounts = true
482+
configSpy.mockReturnValue({
483+
enableAutoSubAccounts: true,
484+
});
485+
486+
accounts = await signer.request({ method: 'eth_accounts' });
487+
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
488+
489+
// Test when enableAutoSubAccounts is undefined (should default to false behavior)
490+
configSpy.mockReturnValue(undefined);
491+
492+
accounts = await signer.request({ method: 'eth_accounts' });
493+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
494+
495+
subAccountsSpy.mockRestore();
496+
configSpy.mockRestore();
497+
});
498+
499+
it('should return only global account when no sub account exists', async () => {
500+
// Set up the signer with only a global account
501+
signer['accounts'] = [globalAccountAddress];
502+
signer['chain'] = { id: 1, rpcUrl: 'https://eth-rpc.example.com/1' };
503+
504+
// No sub account in the store
505+
const subAccountsSpy = vi.spyOn(store.subAccounts, 'get').mockReturnValue(undefined);
506+
507+
const accounts = await signer.request({ method: 'eth_accounts' });
508+
expect(accounts).toEqual([globalAccountAddress]);
509+
510+
subAccountsSpy.mockRestore();
511+
});
512+
513+
it('should handle multiple accounts correctly', async () => {
514+
const secondGlobalAccount = '0x1234567890123456789012345678901234567890';
515+
516+
// Set up the signer with multiple global accounts
517+
signer['accounts'] = [globalAccountAddress, secondGlobalAccount];
518+
signer['chain'] = { id: 1, rpcUrl: 'https://eth-rpc.example.com/1' };
519+
520+
// Set a sub account in the store
521+
const subAccountsSpy = vi.spyOn(store.subAccounts, 'get').mockReturnValue({
522+
address: subAccountAddress,
523+
factory: globalAccountAddress,
524+
factoryData: '0x',
525+
});
526+
527+
// Test with enableAutoSubAccounts = false
528+
const configSpy = vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
529+
enableAutoSubAccounts: false,
530+
});
531+
532+
let accounts = await signer.request({ method: 'eth_accounts' });
533+
expect(accounts).toEqual([globalAccountAddress, secondGlobalAccount, subAccountAddress]);
534+
535+
// Test with enableAutoSubAccounts = true
536+
configSpy.mockReturnValue({
537+
enableAutoSubAccounts: true,
538+
});
539+
540+
accounts = await signer.request({ method: 'eth_accounts' });
541+
expect(accounts).toEqual([subAccountAddress, globalAccountAddress, secondGlobalAccount]);
542+
543+
subAccountsSpy.mockRestore();
544+
configSpy.mockRestore();
545+
});
546+
});
547+
455548
describe('wallet_connect', () => {
456549
beforeEach(async () => {
457550
await signer.cleanup();
@@ -463,6 +556,10 @@ describe('SCWSigner', () => {
463556
await signer.handshake({ method: 'handshake' });
464557
});
465558

559+
afterEach(() => {
560+
vi.restoreAllMocks();
561+
});
562+
466563
it('should handle wallet_connect with no capabilities', async () => {
467564
expect(signer['accounts']).toEqual([]);
468565
const mockRequest: RequestArguments = {
@@ -508,9 +605,9 @@ describe('SCWSigner', () => {
508605
factoryData: '0x',
509606
});
510607

511-
// eth_accounts should return only global account
608+
// eth_accounts should return both accounts with global account first
512609
const accounts = await signer.request({ method: 'eth_accounts' });
513-
expect(accounts).toEqual([globalAccountAddress]);
610+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
514611
});
515612

516613
it('should handle wallet_connect with addSubAccount capability', async () => {
@@ -569,10 +666,10 @@ describe('SCWSigner', () => {
569666
factoryData: '0x',
570667
});
571668

572-
// eth_accounts should return [subAccount, globalAccount]
669+
// eth_accounts should return [globalAccount, subAccount] when enableAutoSubAccounts is not true
573670
const accounts = await signer.request({ method: 'eth_accounts' });
574671

575-
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
672+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
576673
});
577674

578675
it('should handle wallet_addSubAccount creating new sub account', async () => {
@@ -635,9 +732,9 @@ describe('SCWSigner', () => {
635732
factoryData: '0x',
636733
});
637734

638-
// eth_accounts should return [subAccount, globalAccount]
735+
// eth_accounts should return [globalAccount, subAccount] when enableAutoSubAccounts is not true
639736
const accounts = await signer.request({ method: 'eth_accounts' });
640-
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
737+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
641738
});
642739

643740
it('should route eth_requestAccounts through wallet_connect', async () => {
@@ -819,9 +916,71 @@ describe('SCWSigner', () => {
819916
],
820917
});
821918
});
919+
920+
it('should always return sub account first when enableAutoSubAccounts is true', async () => {
921+
expect(signer['accounts']).toEqual([]);
922+
923+
// Enable auto sub accounts
924+
vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
925+
enableAutoSubAccounts: true,
926+
});
927+
928+
const mockRequest: RequestArguments = {
929+
method: 'wallet_connect',
930+
params: [],
931+
};
932+
933+
const mockSetAccount = vi.spyOn(store.account, 'set');
934+
const mockSetSubAccounts = vi.spyOn(store.subAccounts, 'set');
935+
936+
(decryptContent as Mock).mockResolvedValueOnce({
937+
result: {
938+
value: {
939+
accounts: [
940+
{
941+
address: globalAccountAddress,
942+
capabilities: {
943+
subAccounts: [
944+
{
945+
address: subAccountAddress,
946+
factory: globalAccountAddress,
947+
factoryData: '0x',
948+
},
949+
],
950+
},
951+
},
952+
],
953+
},
954+
},
955+
});
956+
957+
await signer.request(mockRequest);
958+
959+
// Should persist accounts correctly
960+
expect(mockSetAccount).toHaveBeenCalledWith({
961+
accounts: [globalAccountAddress],
962+
});
963+
expect(mockSetSubAccounts).toHaveBeenCalledWith({
964+
address: subAccountAddress,
965+
factory: globalAccountAddress,
966+
factoryData: '0x',
967+
});
968+
969+
// When enableAutoSubAccounts is true, sub account should be first
970+
const accounts = await signer.request({ method: 'eth_accounts' });
971+
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
972+
973+
// Test with eth_requestAccounts as well
974+
const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' });
975+
expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]);
976+
});
822977
});
823978

824979
describe('wallet_addSubAccount', () => {
980+
afterEach(() => {
981+
vi.restoreAllMocks();
982+
});
983+
825984
it('should update internal state for successful wallet_addSubAccount', async () => {
826985
await signer.cleanup();
827986

@@ -884,7 +1043,7 @@ describe('SCWSigner', () => {
8841043
});
8851044

8861045
const accounts = await signer.request({ method: 'eth_accounts' });
887-
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
1046+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
8881047

8891048
expect(mockSetAccount).toHaveBeenCalledWith({
8901049
accounts: [globalAccountAddress],
@@ -974,8 +1133,82 @@ describe('SCWSigner', () => {
9741133
mockCryptoKey
9751134
);
9761135

1136+
const accounts = await signer.request({ method: 'eth_accounts' });
1137+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
1138+
});
1139+
1140+
it('should always return sub account first when enableAutoSubAccounts is true', async () => {
1141+
await signer.cleanup();
1142+
1143+
// Enable auto sub accounts
1144+
vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
1145+
enableAutoSubAccounts: true,
1146+
});
1147+
1148+
const mockRequest: RequestArguments = {
1149+
method: 'wallet_connect',
1150+
params: [],
1151+
};
1152+
1153+
(decryptContent as Mock).mockResolvedValueOnce({
1154+
result: {
1155+
value: null,
1156+
},
1157+
});
1158+
1159+
await signer.handshake({ method: 'handshake' });
1160+
expect(signer['accounts']).toEqual([]);
1161+
1162+
(decryptContent as Mock).mockResolvedValueOnce({
1163+
result: {
1164+
value: {
1165+
accounts: [
1166+
{
1167+
address: globalAccountAddress,
1168+
capabilities: {},
1169+
},
1170+
],
1171+
},
1172+
},
1173+
});
1174+
1175+
await signer.request(mockRequest);
1176+
1177+
(decryptContent as Mock).mockResolvedValueOnce({
1178+
result: {
1179+
value: {
1180+
address: subAccountAddress,
1181+
factory: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54',
1182+
factoryData: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54',
1183+
},
1184+
},
1185+
});
1186+
1187+
await signer.request({
1188+
method: 'wallet_addSubAccount',
1189+
params: [
1190+
{
1191+
version: '1',
1192+
account: {
1193+
type: 'create',
1194+
keys: [
1195+
{
1196+
publicKey: '0x123',
1197+
type: 'p256',
1198+
},
1199+
],
1200+
},
1201+
},
1202+
],
1203+
});
1204+
1205+
// wallet_addSubAccount now respects enableAutoSubAccounts, so sub account should be first
9771206
const accounts = await signer.request({ method: 'eth_accounts' });
9781207
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
1208+
1209+
// However, eth_requestAccounts will reorder based on enableAutoSubAccounts
1210+
const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' });
1211+
expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]);
9791212
});
9801213
});
9811214

@@ -1024,6 +1257,10 @@ describe('SCWSigner', () => {
10241257
});
10251258
});
10261259

1260+
afterEach(() => {
1261+
vi.restoreAllMocks();
1262+
});
1263+
10271264
it('should create a sub account when eth_requestAccounts is called', async () => {
10281265
const mockRequest: RequestArguments = {
10291266
method: 'eth_requestAccounts',
@@ -1042,6 +1279,13 @@ describe('SCWSigner', () => {
10421279
});
10431280

10441281
(findOwnerIndex as Mock).mockResolvedValueOnce(-1);
1282+
(handleAddSubAccountOwner as Mock).mockResolvedValueOnce(0);
1283+
1284+
// Ensure createSubAccountSigner returns the expected shape
1285+
(createSubAccountSigner as Mock).mockResolvedValueOnce({
1286+
request: vi.fn().mockResolvedValue('0xResult'),
1287+
});
1288+
10451289
(decryptContent as Mock).mockResolvedValueOnce({
10461290
result: {
10471291
value: null,

0 commit comments

Comments
 (0)