Skip to content

Commit cceea58

Browse files
stephancillstephancill
authored
fix: update manual sub account odering behavior (#1681)
* fix: update manual sub account odering behavior * fix: tests
1 parent 4eba891 commit cceea58

File tree

5 files changed

+282
-28
lines changed

5 files changed

+282
-28
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
@@ -80,6 +80,22 @@ export default function AutoSubAccount() {
8080
}
8181
};
8282

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

@@ -354,6 +370,9 @@ export default function AutoSubAccount() {
354370
<Button w="full" onClick={handleRequestAccounts}>
355371
eth_requestAccounts
356372
</Button>
373+
<Button w="full" onClick={handleEthAccounts}>
374+
eth_accounts
375+
</Button>
357376
<Button w="full" onClick={handleSendTransaction} isDisabled={!accounts.length}>
358377
eth_sendTransaction
359378
</Button>

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

Lines changed: 218 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,64 @@ 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+
455514
describe('wallet_connect', () => {
456515
beforeEach(async () => {
457516
await signer.cleanup();
@@ -463,6 +522,10 @@ describe('SCWSigner', () => {
463522
await signer.handshake({ method: 'handshake' });
464523
});
465524

525+
afterEach(() => {
526+
vi.restoreAllMocks();
527+
});
528+
466529
it('should handle wallet_connect with no capabilities', async () => {
467530
expect(signer['accounts']).toEqual([]);
468531
const mockRequest: RequestArguments = {
@@ -508,9 +571,9 @@ describe('SCWSigner', () => {
508571
factoryData: '0x',
509572
});
510573

511-
// eth_accounts should return only global account
574+
// eth_accounts should return both accounts with global account first
512575
const accounts = await signer.request({ method: 'eth_accounts' });
513-
expect(accounts).toEqual([globalAccountAddress]);
576+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
514577
});
515578

516579
it('should handle wallet_connect with addSubAccount capability', async () => {
@@ -569,10 +632,10 @@ describe('SCWSigner', () => {
569632
factoryData: '0x',
570633
});
571634

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

575-
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
638+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
576639
});
577640

578641
it('should handle wallet_addSubAccount creating new sub account', async () => {
@@ -635,9 +698,9 @@ describe('SCWSigner', () => {
635698
factoryData: '0x',
636699
});
637700

638-
// eth_accounts should return [subAccount, globalAccount]
701+
// eth_accounts should return [globalAccount, subAccount] when enableAutoSubAccounts is not true
639702
const accounts = await signer.request({ method: 'eth_accounts' });
640-
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
703+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
641704
});
642705

643706
it('should route eth_requestAccounts through wallet_connect', async () => {
@@ -819,9 +882,71 @@ describe('SCWSigner', () => {
819882
],
820883
});
821884
});
885+
886+
it('should always return sub account first when enableAutoSubAccounts is true', async () => {
887+
expect(signer['accounts']).toEqual([]);
888+
889+
// Enable auto sub accounts
890+
vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
891+
enableAutoSubAccounts: true,
892+
});
893+
894+
const mockRequest: RequestArguments = {
895+
method: 'wallet_connect',
896+
params: [],
897+
};
898+
899+
const mockSetAccount = vi.spyOn(store.account, 'set');
900+
const mockSetSubAccounts = vi.spyOn(store.subAccounts, 'set');
901+
902+
(decryptContent as Mock).mockResolvedValueOnce({
903+
result: {
904+
value: {
905+
accounts: [
906+
{
907+
address: globalAccountAddress,
908+
capabilities: {
909+
subAccounts: [
910+
{
911+
address: subAccountAddress,
912+
factory: globalAccountAddress,
913+
factoryData: '0x',
914+
},
915+
],
916+
},
917+
},
918+
],
919+
},
920+
},
921+
});
922+
923+
await signer.request(mockRequest);
924+
925+
// Should persist accounts correctly
926+
expect(mockSetAccount).toHaveBeenCalledWith({
927+
accounts: [globalAccountAddress],
928+
});
929+
expect(mockSetSubAccounts).toHaveBeenCalledWith({
930+
address: subAccountAddress,
931+
factory: globalAccountAddress,
932+
factoryData: '0x',
933+
});
934+
935+
// When enableAutoSubAccounts is true, sub account should be first
936+
const accounts = await signer.request({ method: 'eth_accounts' });
937+
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
938+
939+
// Test with eth_requestAccounts as well
940+
const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' });
941+
expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]);
942+
});
822943
});
823944

824945
describe('wallet_addSubAccount', () => {
946+
afterEach(() => {
947+
vi.restoreAllMocks();
948+
});
949+
825950
it('should update internal state for successful wallet_addSubAccount', async () => {
826951
await signer.cleanup();
827952

@@ -884,7 +1009,7 @@ describe('SCWSigner', () => {
8841009
});
8851010

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

8891014
expect(mockSetAccount).toHaveBeenCalledWith({
8901015
accounts: [globalAccountAddress],
@@ -974,8 +1099,82 @@ describe('SCWSigner', () => {
9741099
mockCryptoKey
9751100
);
9761101

1102+
const accounts = await signer.request({ method: 'eth_accounts' });
1103+
expect(accounts).toEqual([globalAccountAddress, subAccountAddress]);
1104+
});
1105+
1106+
it('should always return sub account first when enableAutoSubAccounts is true', async () => {
1107+
await signer.cleanup();
1108+
1109+
// Enable auto sub accounts
1110+
vi.spyOn(store.subAccountsConfig, 'get').mockReturnValue({
1111+
enableAutoSubAccounts: true,
1112+
});
1113+
1114+
const mockRequest: RequestArguments = {
1115+
method: 'wallet_connect',
1116+
params: [],
1117+
};
1118+
1119+
(decryptContent as Mock).mockResolvedValueOnce({
1120+
result: {
1121+
value: null,
1122+
},
1123+
});
1124+
1125+
await signer.handshake({ method: 'handshake' });
1126+
expect(signer['accounts']).toEqual([]);
1127+
1128+
(decryptContent as Mock).mockResolvedValueOnce({
1129+
result: {
1130+
value: {
1131+
accounts: [
1132+
{
1133+
address: globalAccountAddress,
1134+
capabilities: {},
1135+
},
1136+
],
1137+
},
1138+
},
1139+
});
1140+
1141+
await signer.request(mockRequest);
1142+
1143+
(decryptContent as Mock).mockResolvedValueOnce({
1144+
result: {
1145+
value: {
1146+
address: subAccountAddress,
1147+
factory: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54',
1148+
factoryData: '0xe6c7D51b0d5ECC217BE74019447aeac4580Afb54',
1149+
},
1150+
},
1151+
});
1152+
1153+
await signer.request({
1154+
method: 'wallet_addSubAccount',
1155+
params: [
1156+
{
1157+
version: '1',
1158+
account: {
1159+
type: 'create',
1160+
keys: [
1161+
{
1162+
publicKey: '0x123',
1163+
type: 'p256',
1164+
},
1165+
],
1166+
},
1167+
},
1168+
],
1169+
});
1170+
1171+
// wallet_addSubAccount now respects enableAutoSubAccounts, so sub account should be first
9771172
const accounts = await signer.request({ method: 'eth_accounts' });
9781173
expect(accounts).toEqual([subAccountAddress, globalAccountAddress]);
1174+
1175+
// However, eth_requestAccounts will reorder based on enableAutoSubAccounts
1176+
const requestedAccounts = await signer.request({ method: 'eth_requestAccounts' });
1177+
expect(requestedAccounts).toEqual([subAccountAddress, globalAccountAddress]);
9791178
});
9801179
});
9811180

@@ -1024,6 +1223,10 @@ describe('SCWSigner', () => {
10241223
});
10251224
});
10261225

1226+
afterEach(() => {
1227+
vi.restoreAllMocks();
1228+
});
1229+
10271230
it('should create a sub account when eth_requestAccounts is called', async () => {
10281231
const mockRequest: RequestArguments = {
10291232
method: 'eth_requestAccounts',
@@ -1042,6 +1245,13 @@ describe('SCWSigner', () => {
10421245
});
10431246

10441247
(findOwnerIndex as Mock).mockResolvedValueOnce(-1);
1248+
(handleAddSubAccountOwner as Mock).mockResolvedValueOnce(0);
1249+
1250+
// Ensure createSubAccountSigner returns the expected shape
1251+
(createSubAccountSigner as Mock).mockResolvedValueOnce({
1252+
request: vi.fn().mockResolvedValue('0xResult'),
1253+
});
1254+
10451255
(decryptContent as Mock).mockResolvedValueOnce({
10461256
result: {
10471257
value: null,

0 commit comments

Comments
 (0)