Skip to content

Commit c2cb539

Browse files
ffmcgee725ffmcgee725
authored
fix: enhance session comparison (SDK V2) (#1365)
* refactor: enhance session comparison * revert: lint changes * test: fix * fix: code review
1 parent 7587a0e commit c2cb539

File tree

4 files changed

+259
-6
lines changed

4 files changed

+259
-6
lines changed

‎packages/sdk-multichain/src/multichain/transports/default/index.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { type CreateSessionParams, getDefaultTransport, type Transport, type TransportRequest, type TransportResponse } from '@metamask/multichain-api-client';
22
import type { CaipAccountId } from '@metamask/utils';
33
import type { ExtendedTransport, RPCAPI, Scope, SessionData } from 'src/domain';
4-
import { addValidAccounts, getOptionalScopes, getValidAccounts } from 'src/multichain/utils';
4+
import { addValidAccounts, getOptionalScopes, getValidAccounts, isSameScopesAndAccounts } from 'src/multichain/utils';
55

66
const DEFAULT_REQUEST_TIMEOUT = 60 * 1000;
77

@@ -34,8 +34,9 @@ export class DefaultTransport implements ExtendedTransport {
3434
if (walletSession && options) {
3535
const currentScopes = Object.keys(walletSession?.sessionScopes ?? {}) as Scope[];
3636
const proposedScopes = options?.scopes ?? [];
37-
const isSameScopes = currentScopes.every((scope) => proposedScopes.includes(scope)) && proposedScopes.every((scope) => currentScopes.includes(scope));
38-
if (!isSameScopes) {
37+
const proposedCaipAccountIds = options?.caipAccountIds ?? [];
38+
const hasSameScopesAndAccounts = isSameScopesAndAccounts(currentScopes, proposedScopes, walletSession, proposedCaipAccountIds);
39+
if (!hasSameScopesAndAccounts) {
3940
await this.request({ method: 'wallet_revokeSession', params: walletSession }, this.#defaultRequestOptions);
4041
const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
4142
const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };

‎packages/sdk-multichain/src/multichain/transports/mwp/index.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { DappClient } from '@metamask/mobile-wallet-protocol-dapp-client';
55

66
import { createLogger, type ExtendedTransport, type RPCAPI, type Scope, type SessionData, type StoreAdapter } from '../../../domain';
77
import type { CaipAccountId } from '@metamask/utils';
8-
import { addValidAccounts, getOptionalScopes, getValidAccounts } from '../../utils';
8+
import { addValidAccounts, getOptionalScopes, getValidAccounts, isSameScopesAndAccounts } from '../../utils';
99

1010
const DEFAULT_REQUEST_TIMEOUT = 60 * 1000;
1111
const CONNECTION_GRACE_PERIOD = 60 * 1000;
@@ -111,8 +111,9 @@ export class MWPTransport implements ExtendedTransport {
111111
if (walletSession && options) {
112112
const currentScopes = Object.keys(walletSession?.sessionScopes ?? {}) as Scope[];
113113
const proposedScopes = options?.scopes ?? [];
114-
const isSameScopes = currentScopes.every((scope) => proposedScopes.includes(scope)) && proposedScopes.every((scope) => currentScopes.includes(scope));
115-
if (!isSameScopes) {
114+
const proposedCaipAccountIds = options?.caipAccountIds ?? [];
115+
const hasSameScopesAndAccounts = isSameScopesAndAccounts(currentScopes, proposedScopes, walletSession, proposedCaipAccountIds);
116+
if (!hasSameScopesAndAccounts) {
116117
const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
117118
const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };
118119
const response = await this.request({ method: 'wallet_createSession', params: sessionRequest });

‎packages/sdk-multichain/src/multichain/utils/index.test.ts‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
/** biome-ignore-all lint/style/noNonNullAssertion: Tests require it */
33
import * as t from 'vitest';
44
import { vi } from 'vitest';
5+
import type { CaipAccountId } from '@metamask/utils';
56
import packageJson from '../../../package.json';
67
import type { MultichainOptions } from '../../domain/multichain';
78
import { getPlatformType, isMetamaskExtensionInstalled, PlatformType } from '../../domain/platform';
9+
import type { Scope } from '../../domain';
810
import * as utils from '.';
911

1012
vi.mock('../../domain/platform', async () => {
@@ -277,4 +279,220 @@ t.describe('Utils', () => {
277279
t.expect(isMetamaskExtensionInstalled()).toBe(false);
278280
});
279281
});
282+
283+
t.describe('isSameScopesAndAccounts', () => {
284+
const mockWalletSession = {
285+
sessionScopes: {
286+
'eip155:1': {
287+
methods: ['eth_sendTransaction'],
288+
notifications: ['chainChanged'],
289+
accounts: ['eip155:1:0x1234567890123456789012345678901234567890'],
290+
},
291+
'eip155:137': {
292+
methods: ['eth_sendTransaction'],
293+
notifications: ['chainChanged'],
294+
accounts: ['eip155:137:0xabcdefabcdefabcdefabcdefabcdefabcdefabcd'],
295+
},
296+
},
297+
} as any;
298+
299+
t.it('should return true when scopes and accounts match exactly', () => {
300+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
301+
const proposedScopes: Scope[] = ['eip155:1', 'eip155:137'];
302+
const proposedCaipAccountIds = [
303+
'eip155:1:0x1234567890123456789012345678901234567890',
304+
'eip155:137:0xabcdefabcdefabcdefabcdefabcdefabcdefabcd',
305+
] as CaipAccountId[];
306+
307+
const result = utils.isSameScopesAndAccounts(
308+
currentScopes,
309+
proposedScopes,
310+
mockWalletSession,
311+
proposedCaipAccountIds,
312+
);
313+
314+
t.expect(result).toBe(true);
315+
});
316+
317+
t.it('should return false when scopes do not match', () => {
318+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
319+
const proposedScopes: Scope[] = ['eip155:1', 'eip155:56']; // Different scope
320+
const proposedCaipAccountIds = [
321+
'eip155:1:0x1234567890123456789012345678901234567890',
322+
] as CaipAccountId[];
323+
324+
const result = utils.isSameScopesAndAccounts(
325+
currentScopes,
326+
proposedScopes,
327+
mockWalletSession,
328+
proposedCaipAccountIds,
329+
);
330+
331+
t.expect(result).toBe(false);
332+
});
333+
334+
t.it('should return false when proposed accounts are not included in existing session', () => {
335+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
336+
const proposedScopes: Scope[] = ['eip155:1', 'eip155:137'];
337+
const proposedCaipAccountIds = [
338+
'eip155:1:0x1234567890123456789012345678901234567890',
339+
'eip155:1:0x9999999999999999999999999999999999999999', // Not in session
340+
] as CaipAccountId[];
341+
342+
const result = utils.isSameScopesAndAccounts(
343+
currentScopes,
344+
proposedScopes,
345+
mockWalletSession,
346+
proposedCaipAccountIds,
347+
);
348+
349+
t.expect(result).toBe(false);
350+
});
351+
352+
t.it('should return true when proposed accounts are subset of existing session accounts', () => {
353+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
354+
const proposedScopes: Scope[] = ['eip155:1', 'eip155:137'];
355+
const proposedCaipAccountIds = [
356+
'eip155:1:0x1234567890123456789012345678901234567890', // Only one account
357+
] as CaipAccountId[];
358+
359+
const result = utils.isSameScopesAndAccounts(
360+
currentScopes,
361+
proposedScopes,
362+
mockWalletSession,
363+
proposedCaipAccountIds,
364+
);
365+
366+
t.expect(result).toBe(true);
367+
});
368+
369+
t.it('should return true when no accounts are proposed and scopes match', () => {
370+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
371+
const proposedScopes: Scope[] = ['eip155:1', 'eip155:137'];
372+
const proposedCaipAccountIds: CaipAccountId[] = [];
373+
374+
const result = utils.isSameScopesAndAccounts(
375+
currentScopes,
376+
proposedScopes,
377+
mockWalletSession,
378+
proposedCaipAccountIds,
379+
)
380+
381+
t.expect(result).toBe(true);
382+
});
383+
384+
t.it('should handle empty session scopes', () => {
385+
const emptySession = { sessionScopes: {} } as any;
386+
const currentScopes: Scope[] = [];
387+
const proposedScopes: Scope[] = [];
388+
const proposedCaipAccountIds: CaipAccountId[] = [];
389+
390+
const result = utils.isSameScopesAndAccounts(
391+
currentScopes,
392+
proposedScopes,
393+
emptySession,
394+
proposedCaipAccountIds,
395+
);
396+
397+
t.expect(result).toBe(true);
398+
});
399+
400+
t.it('should handle scope objects without accounts property', () => {
401+
const sessionWithoutAccounts = {
402+
sessionScopes: {
403+
'eip155:1': {
404+
methods: ['eth_sendTransaction'],
405+
notifications: ['chainChanged'],
406+
// No accounts property
407+
},
408+
},
409+
} as any;
410+
411+
const currentScopes: Scope[] = ['eip155:1'];
412+
const proposedScopes: Scope[] = ['eip155:1'];
413+
const proposedCaipAccountIds = ['eip155:1:0x1234567890123456789012345678901234567890'] as CaipAccountId[];
414+
415+
const result = utils.isSameScopesAndAccounts(
416+
currentScopes,
417+
proposedScopes,
418+
sessionWithoutAccounts,
419+
proposedCaipAccountIds,
420+
);
421+
422+
t.expect(result).toBe(false);
423+
});
424+
425+
t.it('should handle scope objects with empty accounts array', () => {
426+
const sessionWithEmptyAccounts = {
427+
sessionScopes: {
428+
'eip155:1': {
429+
methods: ['eth_sendTransaction'],
430+
notifications: ['chainChanged'],
431+
accounts: [],
432+
},
433+
},
434+
} as any;
435+
436+
const currentScopes: Scope[] = ['eip155:1'];
437+
const proposedScopes: Scope[] = ['eip155:1'];
438+
const proposedCaipAccountIds = ['eip155:1:0x1234567890123456789012345678901234567890'] as CaipAccountId[];
439+
440+
const result = utils.isSameScopesAndAccounts(
441+
currentScopes,
442+
proposedScopes,
443+
sessionWithEmptyAccounts,
444+
proposedCaipAccountIds,
445+
);
446+
447+
t.expect(result).toBe(false);
448+
});
449+
450+
t.it('should return true when scopes have different order but same content', () => {
451+
const currentScopes: Scope[] = ['eip155:1', 'eip155:137'];
452+
const proposedScopes: Scope[] = ['eip155:137', 'eip155:1']; // Different order
453+
const proposedCaipAccountIds = [
454+
'eip155:1:0x1234567890123456789012345678901234567890',
455+
'eip155:137:0xabcdefabcdefabcdefabcdefabcdefabcdefabcd',
456+
] as CaipAccountId[];
457+
458+
const result = utils.isSameScopesAndAccounts(
459+
currentScopes,
460+
proposedScopes,
461+
mockWalletSession,
462+
proposedCaipAccountIds,
463+
);
464+
465+
t.expect(result).toBe(true);
466+
});
467+
468+
t.it('should handle multiple accounts in same scope', () => {
469+
const sessionWithMultipleAccounts = {
470+
sessionScopes: {
471+
'eip155:1': {
472+
methods: ['eth_sendTransaction'],
473+
notifications: ['chainChanged'],
474+
accounts: [
475+
'eip155:1:0x1234567890123456789012345678901234567890',
476+
'eip155:1:0xabcdefabcdefabcdefabcdefabcdefabcdefabcd',
477+
],
478+
},
479+
},
480+
} as any;
481+
482+
const currentScopes: Scope[] = ['eip155:1'];
483+
const proposedScopes: Scope[] = ['eip155:1'];
484+
const proposedCaipAccountIds = [
485+
'eip155:1:0x1234567890123456789012345678901234567890',
486+
] as CaipAccountId[];
487+
488+
const result = utils.isSameScopesAndAccounts(
489+
currentScopes,
490+
proposedScopes,
491+
sessionWithMultipleAccounts,
492+
proposedCaipAccountIds,
493+
);
494+
495+
t.expect(result).toBe(true);
496+
});
497+
});
280498
});

‎packages/sdk-multichain/src/multichain/utils/index.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,39 @@ export function setupDappMetadata(options: MultichainOptions): MultichainOptions
141141
return options;
142142
}
143143

144+
/**
145+
* Enhanced scope checking function that validates both scopes and accounts
146+
* @param currentScopes - Current scopes from the existing session
147+
* @param proposedScopes - Proposed scopes from the connect options
148+
* @param walletSession - The existing wallet session data
149+
* @param proposedCaipAccountIds - Proposed account IDs from the connect options
150+
* @returns true if scopes and accounts match, false otherwise
151+
*/
152+
export function isSameScopesAndAccounts(
153+
currentScopes: Scope[],
154+
proposedScopes: Scope[],
155+
walletSession: SessionData,
156+
proposedCaipAccountIds: CaipAccountId[],
157+
): boolean {
158+
const isSameScopes =
159+
currentScopes.every((scope) => proposedScopes.includes(scope)) &&
160+
proposedScopes.every((scope) => currentScopes.includes(scope));
161+
162+
if (!isSameScopes) {
163+
return false;
164+
}
165+
166+
const existingAccountIds: CaipAccountId[] = Object.values(walletSession.sessionScopes)
167+
.filter(({ accounts }) => Boolean(accounts))
168+
.flatMap(({ accounts }) => accounts ?? []);
169+
170+
const allProposedAccountsIncluded = proposedCaipAccountIds.every(
171+
(proposedAccountId) => existingAccountIds.includes(proposedAccountId),
172+
);
173+
174+
return allProposedAccountsIncluded;
175+
}
176+
144177
export function getValidAccounts(caipAccountIds: CaipAccountId[]) {
145178
return caipAccountIds.reduce<ReturnType<typeof parseCaipAccountId>[]>((caipAccounts, caipAccountId) => {
146179
try {

0 commit comments

Comments
 (0)