From 76c00c93397e134f9f1e0451c97668700869d9ea Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Fri, 31 Jul 2026 13:04:04 +0700 Subject: [PATCH 1/5] feat: migrate keyring handler to Keyring API v2 --- package.json | 1 + packages/snap/snap.manifest.json | 12 +- .../snap/src/handlers/keyring/exceptions.ts | 22 - .../snap/src/handlers/keyring/keyring.test.ts | 642 +++++++----------- packages/snap/src/handlers/keyring/keyring.ts | 269 +++----- packages/snap/src/permissions.ts | 40 +- .../services/account/AccountService.test.ts | 151 +--- .../src/services/account/AccountService.ts | 93 --- .../snap/src/utils/requestResponse.test.ts | 30 +- packages/snap/tsconfig.json | 4 +- yarn.lock | 110 +-- 11 files changed, 471 insertions(+), 903 deletions(-) diff --git a/package.json b/package.json index 6e3a7416..a527e74c 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ }, "resolutions": { "@metamask/snaps-sdk": "11.1.1", + "@metamask/snaps-utils": "12.2.0", "@stellar/stellar-sdk/axios@npm:1.14.0": "1.18.1", "axios/form-data@npm:^4.0.5": "^4.0.6", "@metamask/snaps-utils/fast-xml-parser@npm:^5.3.4": "^5.7.0", diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 97b74c41..f8c93cec 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "1ftcq5ewpbXIRZpVifhbpo4kHtdvXB4MEoo+bgjMWGQ=", + "shasum": "R5GFkwtIWH9mNc989eLsAxLU+12Ffd2JR/N1Sqatirw=", "location": { "npm": { "filePath": "dist/bundle.js", @@ -23,7 +23,15 @@ }, "initialPermissions": { "endowment:keyring": { - "allowedOrigins": ["https://portfolio.metamask.io"] + "allowedOrigins": ["https://portfolio.metamask.io"], + "capabilities": { + "scopes": ["stellar:pubnet"], + "bip44": { + "deriveIndex": true, + "deriveIndexRange": true, + "discover": true + } + } }, "snap_getBip32Entropy": [ { diff --git a/packages/snap/src/handlers/keyring/exceptions.ts b/packages/snap/src/handlers/keyring/exceptions.ts index 6e81023e..4fec7fda 100644 --- a/packages/snap/src/handlers/keyring/exceptions.ts +++ b/packages/snap/src/handlers/keyring/exceptions.ts @@ -14,7 +14,6 @@ import { TransactionValidationException, } from '../../services/transaction/exceptions'; import { HttpException } from '../../utils'; -import type { StellarSnapExceptionOptions } from '../../utils/errors'; import { StellarSnapException } from '../../utils/errors'; export class KeyringException extends StellarSnapException {} @@ -131,24 +130,3 @@ export function toSep43Error(error: unknown): Sep43Error { return new Sep43Error({ code: Sep43ErrorCode.Internal }); } - -export class KeyringAccountRollbackException extends KeyringException { - constructor(accountId: string, options?: StellarSnapExceptionOptions) { - super( - `Failed to rollback account creation for account ${accountId}`, - options, - ); - } -} - -export class KeyringEmitAccountCreatedEventException extends KeyringException { - constructor(options?: StellarSnapExceptionOptions) { - super('Failed to emit account created event', options); - } -} - -export class KeyringEmitAccountDeletedEventException extends KeyringException { - constructor(options?: StellarSnapExceptionOptions) { - super('Failed to emit account deleted event', options); - } -} diff --git a/packages/snap/src/handlers/keyring/keyring.test.ts b/packages/snap/src/handlers/keyring/keyring.test.ts index cc8e85e1..d7f8b0ea 100644 --- a/packages/snap/src/handlers/keyring/keyring.test.ts +++ b/packages/snap/src/handlers/keyring/keyring.test.ts @@ -2,18 +2,16 @@ import type { KeyringAccount } from '@metamask/keyring-api'; import { AccountCreationType, DiscoveredAccountType, - KeyringEvent, KeyringRpcMethod, } from '@metamask/keyring-api'; import { emitSnapKeyringEvent, - handleKeyringRequest, - MethodNotSupportedError, + handleKeyringRequest as handleKeyringRequestV1, } from '@metamask/keyring-snap-sdk'; +import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError } from '@metamask/snaps-sdk'; import type { JsonRpcRequest } from '@metamask/snaps-sdk'; import { create } from '@metamask/superstruct'; -import type { Json } from '@metamask/utils'; import { MultichainMethod, @@ -22,14 +20,9 @@ import { SignTransactionResponseStruct, } from './api'; import type { IKeyringRequestHandler } from './base'; -import { - KeyringAccountRollbackException, - KeyringEmitAccountCreatedEventException, - KeyringEmitAccountDeletedEventException, -} from './exceptions'; import { KeyringHandler } from './keyring'; import { KnownCaip2ChainId } from '../../api'; -import { KEYRING_ACCOUNT_TYPE, METAMASK_ORIGIN } from '../../constants'; +import { METAMASK_ORIGIN } from '../../constants'; import { AccountService } from '../../services/account'; import type { StellarKeyringAccount } from '../../services/account'; import { @@ -53,7 +46,6 @@ import { import { getSlip44AssetId, getDefaultEntropySource, - getSnapProvider, Duration, } from '../../utils'; import { bufferToUint8Array } from '../../utils/buffer'; @@ -67,12 +59,18 @@ jest.mock('../../utils/requestResponse', () => ({ validateOrigin: jest.fn(), })); jest.mock('@metamask/keyring-snap-sdk', () => ({ - handleKeyringRequest: jest.fn(), emitSnapKeyringEvent: jest.fn(), + handleKeyringRequest: jest.fn(), + MethodNotSupportedError: jest.requireActual('@metamask/keyring-snap-sdk') + .MethodNotSupportedError, +})); +jest.mock('@metamask/keyring-snap-sdk/v2', () => ({ + handleKeyringRequest: jest.fn(), })); describe('KeyringHandler', () => { const entropySourceId = 'entropy-source-1'; + const NON_EXISTENT_ID = '00000000-0000-4000-8000-000000000000'; let keyringHandler: KeyringHandler; let mockAccount: StellarKeyringAccount; let mockAccountId: string; @@ -92,17 +90,11 @@ describe('KeyringHandler', () => { }; }; - const mockCreateAccountResult = ( - account: StellarKeyringAccount, - isNewAccount = true, - ) => ({ account, isNewAccount }); - const getAccountServiceSpies = () => ({ listAccountsSpy: jest.spyOn(AccountService.prototype, 'listAccounts'), findByIdSpy: jest.spyOn(AccountService.prototype, 'findById'), deleteSpy: jest.spyOn(AccountService.prototype, 'delete'), resolveAccountSpy: jest.spyOn(AccountService.prototype, 'resolveAccount'), - createAccountSpy: jest.spyOn(AccountService.prototype, 'create'), batchCreateAccountSpy: jest.spyOn(AccountService.prototype, 'batchCreate'), findByIdsSpy: jest.spyOn(AccountService.prototype, 'findByIds'), }); @@ -150,91 +142,89 @@ describe('KeyringHandler', () => { }); describe('handle', () => { - const request = { + const v2Request = { + method: 'keyring_getAccounts', + id: '1', + jsonrpc: '2.0', + } as JsonRpcRequest; + const v1Request = { method: KeyringRpcMethod.ListAccounts, id: '1', jsonrpc: '2.0', } as JsonRpcRequest; - it('calls handleKeyringRequest', async () => { - const handleKeyringRequestSpy = jest.mocked(handleKeyringRequest); - handleKeyringRequestSpy.mockResolvedValue([]); + it('routes Keyring API v2 methods to the v2 dispatcher', async () => { + jest.mocked(handleKeyringRequest).mockResolvedValue([]); - const result = await keyringHandler.handle(METAMASK_ORIGIN, request); + const result = await keyringHandler.handle(METAMASK_ORIGIN, v2Request); - expect(handleKeyringRequestSpy).toHaveBeenCalledWith( + expect(handleKeyringRequest).toHaveBeenCalledWith( keyringHandler, - request, + v2Request, ); + expect(handleKeyringRequestV1).not.toHaveBeenCalled(); expect(result).toStrictEqual([]); }); - it('returns null if handleKeyringRequest returns null', async () => { - const handleKeyringRequestSpy = jest.mocked(handleKeyringRequest); - handleKeyringRequestSpy.mockResolvedValue(null); + it.each([KeyringRpcMethod.ListAccounts, KeyringRpcMethod.DiscoverAccounts])( + 'routes the v1-only method %s to the v1 dispatcher', + async (method) => { + jest.mocked(handleKeyringRequestV1).mockResolvedValue([]); - const result = await keyringHandler.handle(METAMASK_ORIGIN, request); + const request = { ...v1Request, method } as JsonRpcRequest; + const result = await keyringHandler.handle(METAMASK_ORIGIN, request); - expect(handleKeyringRequestSpy).toHaveBeenCalledWith( - keyringHandler, - request, - ); - expect(result).toBeNull(); - }); - }); - - describe('listAccounts', () => { - it('lists all accounts', async () => { - const expectedAccounts = generateMockStellarKeyringAccounts( - 5, - 'entropy-source-1', - ); - jest - .spyOn(AccountService.prototype, 'listAccounts') - .mockResolvedValue(expectedAccounts); + expect(handleKeyringRequestV1).toHaveBeenCalledWith( + keyringHandler, + request, + ); + expect(handleKeyringRequest).not.toHaveBeenCalled(); + expect(result).toStrictEqual([]); + }, + ); - const result = await keyringHandler.listAccounts(); + it.each([ + KeyringRpcMethod.ListAccountAssets, + KeyringRpcMethod.ListAccountTransactions, + ])( + 'routes the deprecated v1 alias %s to the v2 dispatcher', + async (method) => { + jest.mocked(handleKeyringRequest).mockResolvedValue([]); + + const request = { ...v1Request, method } as JsonRpcRequest; + await keyringHandler.handle(METAMASK_ORIGIN, request); + + expect(handleKeyringRequest).toHaveBeenCalledWith( + keyringHandler, + request, + ); + expect(handleKeyringRequestV1).not.toHaveBeenCalled(); + }, + ); - expect(result).toStrictEqual( - expectedAccounts.map((account) => toKeyringAccount(account)), - ); - }); + it('returns null if the dispatcher returns null', async () => { + jest.mocked(handleKeyringRequest).mockResolvedValue(null); - it('propagates errors when account listing fails', async () => { - jest - .spyOn(AccountService.prototype, 'listAccounts') - .mockRejectedValue(new Error('Account listing failed')); + const result = await keyringHandler.handle(METAMASK_ORIGIN, v2Request); - await expect(keyringHandler.listAccounts()).rejects.toThrow( - 'Account listing failed', - ); + expect(result).toBeNull(); }); }); describe('getAccount', () => { it('gets an account by its ID', async () => { - jest - .spyOn(AccountService.prototype, 'findById') - .mockResolvedValue(mockAccount); - - const result = await keyringHandler.getAccount(mockAccountId); - expect(result).toStrictEqual(toKeyringAccount(mockAccount)); - }); - - it('returns undefined if the account is not found', async () => { - jest - .spyOn(AccountService.prototype, 'findById') - .mockResolvedValue(undefined); + const { findByIdSpy } = getAccountServiceSpies(); + findByIdSpy.mockResolvedValue(mockAccount); const result = await keyringHandler.getAccount(mockAccountId); - expect(result).toBeUndefined(); + expect(findByIdSpy).toHaveBeenCalledWith(mockAccountId); + expect(result).toStrictEqual(toKeyringAccount(mockAccount)); }); it('propagates errors when account retrieval fails', async () => { - jest - .spyOn(AccountService.prototype, 'findById') - .mockRejectedValue(new Error('Account retrieval failed')); + const { findByIdSpy } = getAccountServiceSpies(); + findByIdSpy.mockRejectedValue(new Error('Account retrieval failed')); await expect(keyringHandler.getAccount(mockAccountId)).rejects.toThrow( 'Account retrieval failed', @@ -248,95 +238,120 @@ describe('KeyringHandler', () => { }); }); - describe('createAccount', () => { - it('creates an account', async () => { - const { createAccountSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue(mockCreateAccountResult(mockAccount)); - - const result = await keyringHandler.createAccount(); + describe('getAccount (v2 semantics)', () => { + it('throws for an unknown account id instead of returning undefined', async () => { + const { findByIdSpy } = getAccountServiceSpies(); + findByIdSpy.mockResolvedValue(undefined); - expect(createAccountSpy).toHaveBeenCalledTimes(1); - expect(result).toStrictEqual(toKeyringAccount(mockAccount)); + await expect(keyringHandler.getAccount(NON_EXISTENT_ID)).rejects.toThrow( + AccountNotFoundException, + ); }); + }); - it('emits the account-created event', async () => { - const { createAccountSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue(mockCreateAccountResult(mockAccount)); - const emitSnapKeyringEventSpy = jest.mocked(emitSnapKeyringEvent); - emitSnapKeyringEventSpy.mockResolvedValue(); + describe('getAccounts', () => { + it('lists all accounts', async () => { + const expectedAccounts = generateMockStellarKeyringAccounts( + 5, + 'entropy-source-1', + ); + const { listAccountsSpy } = getAccountServiceSpies(); + listAccountsSpy.mockResolvedValue(expectedAccounts); - await keyringHandler.createAccount({ - metamask: { - correlationId: '123', - }, - }); + const result = await keyringHandler.getAccounts(); - expect(emitSnapKeyringEventSpy).toHaveBeenCalledWith( - getSnapProvider(), - KeyringEvent.AccountCreated, - expect.objectContaining({ - account: toKeyringAccount(mockAccount), - displayConfirmation: false, - metamask: { correlationId: '123' }, - }), + expect(result).toStrictEqual( + expectedAccounts.map((account) => toKeyringAccount(account)), ); }); - it('does not emit account-created event for an existing account', async () => { - const { createAccountSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue( - mockCreateAccountResult(mockAccount, false), - ); - const emitSnapKeyringEventSpy = jest.mocked(emitSnapKeyringEvent); - - const result = await keyringHandler.createAccount(); + it('propagates errors when account listing fails', async () => { + const { listAccountsSpy } = getAccountServiceSpies(); + listAccountsSpy.mockRejectedValue(new Error('Account listing failed')); - expect(result).toStrictEqual(toKeyringAccount(mockAccount)); - expect(emitSnapKeyringEventSpy).not.toHaveBeenCalled(); + await expect(keyringHandler.getAccounts()).rejects.toThrow( + 'Account listing failed', + ); }); + }); - it('propagates errors when account creation fails', async () => { - const { createAccountSpy } = getAccountServiceSpies(); - createAccountSpy.mockRejectedValue(new Error('Account creation failed')); + describe('listAccounts (v1)', () => { + it('returns the same accounts as getAccounts', async () => { + const expectedAccounts = generateMockStellarKeyringAccounts( + 3, + entropySourceId, + ); + const { listAccountsSpy } = getAccountServiceSpies(); + listAccountsSpy.mockResolvedValue(expectedAccounts); - await expect(keyringHandler.createAccount()).rejects.toThrow( - 'Account creation failed', + expect(await keyringHandler.listAccounts()).toStrictEqual( + expectedAccounts.map((account) => toKeyringAccount(account)), ); }); + }); - it('rolls back and throws when account-created event emission fails', async () => { - const { createAccountSpy, deleteSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue(mockCreateAccountResult(mockAccount)); - deleteSpy.mockResolvedValue(undefined); + describe('discoverAccounts (v1)', () => { + it('returns the derived account when it is activated on chain', async () => { + jest + .spyOn(AccountService.prototype, 'deriveKeyringAccount') + .mockResolvedValue(mockAccount); jest - .mocked(emitSnapKeyringEvent) - .mockRejectedValue(new Error('User rejected')); + .spyOn(OnChainAccountService.prototype, 'isAccountActivated') + .mockResolvedValue(true); - await expect(keyringHandler.createAccount()).rejects.toThrow( - KeyringEmitAccountCreatedEventException, + const result = await keyringHandler.discoverAccounts( + [KnownCaip2ChainId.Mainnet], + entropySourceId, + 0, ); - expect(deleteSpy).toHaveBeenCalledWith(mockAccount.id); + + expect(result).toStrictEqual([ + { + type: DiscoveredAccountType.Bip44, + scopes: [KnownCaip2ChainId.Mainnet], + derivationPath: mockAccount.derivationPath, + }, + ]); }); - it('throws KeyringAccountRollbackException when rollback fails', async () => { - const { createAccountSpy, deleteSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue(mockCreateAccountResult(mockAccount)); - deleteSpy.mockRejectedValue(new Error('Rollback failed')); + it('returns an empty array when the account has no on-chain activity', async () => { jest - .mocked(emitSnapKeyringEvent) - .mockRejectedValue(new Error('User rejected')); + .spyOn(AccountService.prototype, 'deriveKeyringAccount') + .mockResolvedValue(mockAccount); + jest + .spyOn(OnChainAccountService.prototype, 'isAccountActivated') + .mockResolvedValue(false); - await expect(keyringHandler.createAccount()).rejects.toThrow( - KeyringAccountRollbackException, - ); + expect( + await keyringHandler.discoverAccounts( + [KnownCaip2ChainId.Mainnet], + entropySourceId, + 0, + ), + ).toStrictEqual([]); + }); + + it('throws an error if the discovery request is invalid', async () => { + await expect( + keyringHandler.discoverAccounts([], entropySourceId, 0), + ).rejects.toThrow(InvalidParamsError); }); }); describe('createAccounts', () => { + const accountsAt = (...indexes: number[]) => + indexes.map((index) => + generateStellarKeyringAccount( + `id-${index}`, + mockAccount.address, + entropySourceId, + index, + ), + ); + it('creates one account for bip44:derive-index without emitting AccountCreated', async () => { - const { createAccountSpy } = getAccountServiceSpies(); - createAccountSpy.mockResolvedValue(mockCreateAccountResult(mockAccount)); - const emitSnapKeyringEventSpy = jest.mocked(emitSnapKeyringEvent); + const { batchCreateAccountSpy } = getAccountServiceSpies(); + batchCreateAccountSpy.mockResolvedValue([mockAccount]); const result = await keyringHandler.createAccounts({ type: AccountCreationType.Bip44DeriveIndex, @@ -344,40 +359,18 @@ describe('KeyringHandler', () => { groupIndex: 2, }); - expect(createAccountSpy).toHaveBeenCalledTimes(1); - expect(createAccountSpy).toHaveBeenCalledWith({ + expect(batchCreateAccountSpy).toHaveBeenCalledWith({ entropySource: entropySourceId, - index: 2, + fromIndex: 2, + toIndex: 2, }); expect(result).toStrictEqual([toKeyringAccount(mockAccount)]); - expect(emitSnapKeyringEventSpy).not.toHaveBeenCalled(); + expect(jest.mocked(emitSnapKeyringEvent)).not.toHaveBeenCalled(); }); it('creates accounts for each index in bip44:derive-index-range', async () => { const { batchCreateAccountSpy } = getAccountServiceSpies(); - const accountAt1 = generateStellarKeyringAccount( - 'id-1', - mockAccount.address, - entropySourceId, - 1, - ); - const accountAt2 = generateStellarKeyringAccount( - 'id-2', - mockAccount.address, - entropySourceId, - 2, - ); - const accountAt3 = generateStellarKeyringAccount( - 'id-3', - mockAccount.address, - entropySourceId, - 3, - ); - batchCreateAccountSpy.mockResolvedValue([ - accountAt1, - accountAt2, - accountAt3, - ]); + batchCreateAccountSpy.mockResolvedValue(accountsAt(1, 2, 3)); const result = await keyringHandler.createAccounts({ type: AccountCreationType.Bip44DeriveIndexRange, @@ -385,7 +378,6 @@ describe('KeyringHandler', () => { range: { from: 1, to: 3 }, }); - expect(batchCreateAccountSpy).toHaveBeenCalledTimes(1); expect(batchCreateAccountSpy).toHaveBeenCalledWith({ entropySource: entropySourceId, fromIndex: 1, @@ -395,9 +387,6 @@ describe('KeyringHandler', () => { expect(result[0]?.options).toMatchObject({ entropy: expect.objectContaining({ groupIndex: 1 }), }); - expect(result[1]?.options).toMatchObject({ - entropy: expect.objectContaining({ groupIndex: 2 }), - }); expect(result[2]?.options).toMatchObject({ entropy: expect.objectContaining({ groupIndex: 3 }), }); @@ -405,19 +394,9 @@ describe('KeyringHandler', () => { }); it('propagates errors when account creation fails', async () => { - const { createAccountSpy, batchCreateAccountSpy } = - getAccountServiceSpies(); - createAccountSpy.mockRejectedValue(new Error('Batch create failed')); + const { batchCreateAccountSpy } = getAccountServiceSpies(); batchCreateAccountSpy.mockRejectedValue(new Error('Batch create failed')); - await expect( - keyringHandler.createAccounts({ - type: AccountCreationType.Bip44DeriveIndex, - entropySource: entropySourceId, - groupIndex: 0, - }), - ).rejects.toThrow('Batch create failed'); - await expect( keyringHandler.createAccounts({ type: AccountCreationType.Bip44DeriveIndexRange, @@ -427,18 +406,61 @@ describe('KeyringHandler', () => { ).rejects.toThrow('Batch create failed'); }); + it('creates the discovered account when it is activated on chain', async () => { + const { batchCreateAccountSpy } = getAccountServiceSpies(); + jest + .spyOn(AccountService.prototype, 'deriveKeyringAccount') + .mockResolvedValue(mockAccount); + jest + .spyOn(OnChainAccountService.prototype, 'isAccountActivated') + .mockResolvedValue(true); + batchCreateAccountSpy.mockResolvedValue([mockAccount]); + + const result = await keyringHandler.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: entropySourceId, + groupIndex: 0, + }); + + expect(batchCreateAccountSpy).toHaveBeenCalledWith({ + entropySource: entropySourceId, + fromIndex: 0, + toIndex: 0, + }); + expect(result).toStrictEqual([toKeyringAccount(mockAccount)]); + }); + + it('creates no account when discovery finds no on-chain activity', async () => { + const { batchCreateAccountSpy } = getAccountServiceSpies(); + jest + .spyOn(AccountService.prototype, 'deriveKeyringAccount') + .mockResolvedValue(mockAccount); + jest + .spyOn(OnChainAccountService.prototype, 'isAccountActivated') + .mockResolvedValue(false); + + const result = await keyringHandler.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: entropySourceId, + groupIndex: 0, + }); + + expect(result).toStrictEqual([]); + expect(batchCreateAccountSpy).not.toHaveBeenCalled(); + }); + it('throws when create account option type is not supported', async () => { await expect( keyringHandler.createAccounts({ - type: AccountCreationType.Bip44Discover, + type: AccountCreationType.Bip44DerivePath, entropySource: entropySourceId, - groupIndex: 0, + derivationPath: `m/44'/148'/0'`, }), ).rejects.toThrow('Unsupported create account option type'); }); }); - describe('listAccountAssets', () => { + describe('getAccountAssets', () => { it('returns on-chain asset ids for the account', async () => { const { resolveAccountSpy } = getAccountServiceSpies(); resolveAccountSpy.mockResolvedValue({ account: mockAccount }); @@ -450,7 +472,7 @@ describe('KeyringHandler', () => { ) .mockResolvedValue(onChainAccount); - const result = await keyringHandler.listAccountAssets(mockAccountId); + const result = await keyringHandler.getAccountAssets(mockAccountId); expect(result).toStrictEqual(onChainAccount.assetIds); }); @@ -466,7 +488,7 @@ describe('KeyringHandler', () => { ) .mockResolvedValue(null); - const result = await keyringHandler.listAccountAssets(mockAccountId); + const result = await keyringHandler.getAccountAssets(mockAccountId); expect(result).toStrictEqual([slipId]); }); @@ -482,18 +504,18 @@ describe('KeyringHandler', () => { .mockRejectedValue(new Error('Horizon unavailable')); await expect( - keyringHandler.listAccountAssets(mockAccountId), + keyringHandler.getAccountAssets(mockAccountId), ).rejects.toThrow('Horizon unavailable'); }); it('rejects invalid account id', async () => { - await expect( - keyringHandler.listAccountAssets('not-uuid'), - ).rejects.toThrow(InvalidParamsError); + await expect(keyringHandler.getAccountAssets('not-uuid')).rejects.toThrow( + InvalidParamsError, + ); }); }); - describe('listAccountTransactions', () => { + describe('getAccountTransactions', () => { it('lists the account transactions', async () => { const { resolveAccountSpy } = getAccountServiceSpies(); resolveAccountSpy.mockResolvedValue({ @@ -508,7 +530,7 @@ describe('KeyringHandler', () => { }); transactionServiceFindByAccountIdSpy.mockResolvedValue(mockTransactions); - const result = await keyringHandler.listAccountTransactions( + const result = await keyringHandler.getAccountTransactions( mockAccountId, { limit: 10, @@ -535,7 +557,7 @@ describe('KeyringHandler', () => { }); transactionServiceFindByAccountIdSpy.mockResolvedValue(mockTransactions); - const result = await keyringHandler.listAccountTransactions( + const result = await keyringHandler.getAccountTransactions( mockAccountId, { limit: 5, @@ -564,7 +586,7 @@ describe('KeyringHandler', () => { transactionServiceFindByAccountIdSpy.mockResolvedValue(mockTransactions); await expect( - keyringHandler.listAccountTransactions(mockAccountId, { + keyringHandler.getAccountTransactions(mockAccountId, { limit: 2, next: '00000000-0000-4000-8000-000000000000', }), @@ -572,132 +594,6 @@ describe('KeyringHandler', () => { }); }); - describe('discoverAccounts', () => { - it('discovers an account', async () => { - const deriveKeyringAccountSpy = jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - const isAccountActivatedSpy = jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValue(true); - - const result = await keyringHandler.discoverAccounts( - [KnownCaip2ChainId.Mainnet], - 'entropy-source-1', - 0, - ); - - expect(deriveKeyringAccountSpy).toHaveBeenCalledWith({ - entropySource: 'entropy-source-1', - index: 0, - }); - expect(isAccountActivatedSpy).toHaveBeenCalledWith({ - accountAddress: mockAccount.address, - scope: KnownCaip2ChainId.Mainnet, - }); - expect(result).toStrictEqual([ - { - type: DiscoveredAccountType.Bip44, - scopes: [KnownCaip2ChainId.Mainnet], - derivationPath: mockAccount.derivationPath, - }, - ]); - }); - - it('returns empty array if the account is not activated on any requested scope', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValue(false); - - const result = await keyringHandler.discoverAccounts( - [KnownCaip2ChainId.Mainnet], - 'entropy-source-1', - 0, - ); - - expect(result).toStrictEqual([]); - }); - - it('returns empty array when the account is not activated on any of multiple scopes', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - const isAccountActivatedSpy = jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValue(false); - - const scopes = [KnownCaip2ChainId.Mainnet, KnownCaip2ChainId.Testnet]; - const result = await keyringHandler.discoverAccounts( - scopes, - 'entropy-source-1', - 0, - ); - - expect(isAccountActivatedSpy).toHaveBeenCalledTimes(2); - expect(isAccountActivatedSpy).toHaveBeenCalledWith({ - accountAddress: mockAccount.address, - scope: KnownCaip2ChainId.Mainnet, - }); - expect(isAccountActivatedSpy).toHaveBeenCalledWith({ - accountAddress: mockAccount.address, - scope: KnownCaip2ChainId.Testnet, - }); - expect(result).toStrictEqual([]); - }); - - it('discovers an account when activated on any requested scope', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(true); - - const scopes = [KnownCaip2ChainId.Mainnet, KnownCaip2ChainId.Testnet]; - const result = await keyringHandler.discoverAccounts( - scopes, - 'entropy-source-1', - 0, - ); - - expect(result).toStrictEqual([ - { - type: DiscoveredAccountType.Bip44, - scopes, - derivationPath: mockAccount.derivationPath, - }, - ]); - }); - - it('propagates errors when account discovery fails', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockRejectedValue(new Error('Account discovery failed')); - - await expect( - keyringHandler.discoverAccounts( - [KnownCaip2ChainId.Mainnet], - 'entropy-source-1', - 0, - ), - ).rejects.toThrow('Account discovery failed'); - }); - - it('throws an error if the account discovery request is invalid', async () => { - await expect( - keyringHandler.discoverAccounts( - ['invalid:chain' as KnownCaip2ChainId], - 'entropy-source-1', - 0, - ), - ).rejects.toThrow(InvalidParamsError); - }); - }); - describe('getAccountBalances', () => { it('returns balances for assets with positive balance and metadata', async () => { const slipId = getSlip44AssetId(KnownCaip2ChainId.Mainnet); @@ -853,90 +749,33 @@ describe('KeyringHandler', () => { }); }); - describe('filterAccountChains', () => { - it('throws MethodNotSupportedError', async () => { - await expect( - keyringHandler.filterAccountChains('1', [KnownCaip2ChainId.Mainnet]), - ).rejects.toThrow(MethodNotSupportedError); - }); - }); - - describe('updateAccount', () => { - it('throws MethodNotSupportedError', async () => { - await expect( - keyringHandler.updateAccount({ - type: KEYRING_ACCOUNT_TYPE, - id: '1', - address: '1', - scopes: [KnownCaip2ChainId.Mainnet], - options: {}, - methods: [], - }), - ).rejects.toThrow(MethodNotSupportedError); - }); - }); - describe('deleteAccount', () => { it('deletes an account', async () => { - const { deleteSpy, resolveAccountSpy } = getAccountServiceSpies(); - resolveAccountSpy.mockResolvedValue({ account: mockAccount }); - const emitSnapKeyringEventSpy = jest.mocked(emitSnapKeyringEvent); - emitSnapKeyringEventSpy.mockResolvedValue(); + const { deleteSpy } = getAccountServiceSpies(); await keyringHandler.deleteAccount(mockAccountId); expect(deleteSpy).toHaveBeenCalledWith(mockAccountId); - expect(resolveAccountSpy).toHaveBeenCalledWith({ - accountId: mockAccountId, - }); - expect(deleteSpy.mock.invocationCallOrder).toHaveLength(1); - expect(emitSnapKeyringEventSpy.mock.invocationCallOrder).toHaveLength(1); - expect( - Number(emitSnapKeyringEventSpy.mock.invocationCallOrder[0]), - ).toBeLessThan(Number(deleteSpy.mock.invocationCallOrder[0])); - expect(emitSnapKeyringEventSpy).toHaveBeenCalledWith( - getSnapProvider(), - KeyringEvent.AccountDeleted, - { - id: mockAccountId, - }, - ); + expect(jest.mocked(emitSnapKeyringEvent)).not.toHaveBeenCalled(); }); it('propagates errors when account deletion fails', async () => { - const { deleteSpy, resolveAccountSpy } = getAccountServiceSpies(); - resolveAccountSpy.mockResolvedValue({ account: mockAccount }); + const { deleteSpy } = getAccountServiceSpies(); deleteSpy.mockRejectedValue(new Error('Account deletion failed')); - const emitSnapKeyringEventSpy = jest.mocked(emitSnapKeyringEvent); - emitSnapKeyringEventSpy.mockResolvedValue(); await expect(keyringHandler.deleteAccount(mockAccountId)).rejects.toThrow( 'Account deletion failed', ); }); - it('throws KeyringEmitAccountDeletedEventException when delete event emission fails', async () => { - const { deleteSpy, resolveAccountSpy } = getAccountServiceSpies(); - resolveAccountSpy.mockResolvedValue({ account: mockAccount }); - jest - .mocked(emitSnapKeyringEvent) - .mockRejectedValue(new Error('Event emission failed')); + it('deletes idempotently when the account does not exist', async () => { + const { deleteSpy } = getAccountServiceSpies(); - await expect(keyringHandler.deleteAccount(mockAccountId)).rejects.toThrow( - KeyringEmitAccountDeletedEventException, - ); - expect(deleteSpy).not.toHaveBeenCalled(); - }); - - it('propagates AccountNotFoundException when the account to delete is not found', async () => { - const { resolveAccountSpy } = getAccountServiceSpies(); - resolveAccountSpy.mockRejectedValue( - new AccountNotFoundException(mockAccountId), - ); + expect( + await keyringHandler.deleteAccount(NON_EXISTENT_ID), + ).toBeUndefined(); - await expect(keyringHandler.deleteAccount(mockAccountId)).rejects.toThrow( - AccountNotFoundException, - ); + expect(deleteSpy).toHaveBeenCalledWith(NON_EXISTENT_ID); }); it('throws an error if the account deletion request is invalid', async () => { @@ -982,10 +821,7 @@ describe('KeyringHandler', () => { signMessagePayload, ); expect(mockSignTransactionHandler.handle).not.toHaveBeenCalled(); - expect(result).toStrictEqual({ - pending: false, - result: expectedResult, - }); + expect(result).toStrictEqual(expectedResult); }); it('submits a sign transaction request', async () => { @@ -1019,10 +855,7 @@ describe('KeyringHandler', () => { ); expect(mockSignMessageHandler.handle).not.toHaveBeenCalled(); expect(mockSignAuthEntryHandler.handle).not.toHaveBeenCalled(); - expect(result).toStrictEqual({ - pending: false, - result: expectedResult, - }); + expect(result).toStrictEqual(expectedResult); }); it('submits a sign auth entry request', async () => { @@ -1058,10 +891,7 @@ describe('KeyringHandler', () => { ); expect(mockSignMessageHandler.handle).not.toHaveBeenCalled(); expect(mockSignTransactionHandler.handle).not.toHaveBeenCalled(); - expect(result).toStrictEqual({ - pending: false, - result: expectedResult, - }); + expect(result).toStrictEqual(expectedResult); }); it('throws an error if the request is invalid', async () => { @@ -1108,13 +938,7 @@ describe('KeyringHandler', () => { }; const response = await keyringHandler.submitRequest(signMessagePayload); - expect(response).toMatchObject({ pending: false }); - expect(() => - create( - (response as { pending: false; result: Json }).result, - SignMessageResponseStruct, - ), - ).not.toThrow(); + expect(() => create(response, SignMessageResponseStruct)).not.toThrow(); }); it('exposes a sign-tx submitRequest result that satisfies the SEP-43 response struct', async () => { @@ -1142,12 +966,8 @@ describe('KeyringHandler', () => { const response = await keyringHandler.submitRequest( signTransactionPayload, ); - expect(response).toMatchObject({ pending: false }); expect(() => - create( - (response as { pending: false; result: Json }).result, - SignTransactionResponseStruct, - ), + create(response, SignTransactionResponseStruct), ).not.toThrow(); }); @@ -1174,13 +994,7 @@ describe('KeyringHandler', () => { }; const response = await keyringHandler.submitRequest(signAuthEntryPayload); - expect(response).toMatchObject({ pending: false }); - expect(() => - create( - (response as { pending: false; result: Json }).result, - SignAuthEntryResponseStruct, - ), - ).not.toThrow(); + expect(() => create(response, SignAuthEntryResponseStruct)).not.toThrow(); }); }); diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index cedc45e5..e294643e 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -1,39 +1,35 @@ -import { - AccountCreationType, - assertCreateAccountOptionIsSupported, - DiscoveredAccountType, - KeyringEvent, -} from '@metamask/keyring-api'; import type { CreateAccountOptions as KeyringApiCreateAccountOptions, DiscoveredAccount, EntropySourceId, - Keyring, KeyringAccount, KeyringRequest, - KeyringResponse, Pagination, ResolvedAccountAddress, Transaction, Balance, } from '@metamask/keyring-api'; +import type { Keyring } from '@metamask/keyring-api'; import { - emitSnapKeyringEvent, - handleKeyringRequest, - MethodNotSupportedError, -} from '@metamask/keyring-snap-sdk'; + AccountCreationType, + assertCreateAccountOptionIsSupported, + DiscoveredAccountType, +} from '@metamask/keyring-api'; +import { KeyringRpcMethod } from '@metamask/keyring-api'; +import type { KeyringSnapRpc } from '@metamask/keyring-api/v2'; +import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; +import { handleKeyringRequest as handleKeyringRequestV1 } from '@metamask/keyring-snap-sdk'; +import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError } from '@metamask/snaps-sdk'; import type { Json, JsonRpcRequest } from '@metamask/snaps-sdk'; import type { CaipAssetTypeOrId } from '@metamask/utils'; import type { - CreateAccountOptions, GetAccountRequest, - ResolveAccountAddressJsonRpcRequest, MultichainMethod, + ResolveAccountAddressJsonRpcRequest, } from './api'; import { - CreateAccountOptionsStruct, DeleteAccountRequestStruct, DiscoverAccountsStruct, GetAccountRequestStruct, @@ -45,11 +41,6 @@ import { GetAccountBalancesRequestStruct, } from './api'; import type { IKeyringRequestHandler } from './base'; -import { - KeyringAccountRollbackException, - KeyringEmitAccountCreatedEventException, - KeyringEmitAccountDeletedEventException, -} from './exceptions'; import type { KnownCaip19AssetIdOrSlip44Id, KnownCaip2ChainId, @@ -76,7 +67,6 @@ import { createPrefixedLogger, Duration, getSlip44AssetId, - getSnapProvider, isClassicAssetId, isSlip44Id, validateOrigin, @@ -85,7 +75,29 @@ import { } from '../../utils'; import { SyncAccountsHandler } from '../cronjob/syncAccounts'; -export class KeyringHandler implements Keyring { +/** + * Methods routed to the Keyring API v2 dispatcher. Everything else falls back + * to the v1 dispatcher, which MetaMask still uses for `keyring_listAccounts` + * (account resync) and `keyring_discoverAccounts` (discovery). + */ +const V2_METHODS = new Set([ + ...Object.values(KeyringSnapRpcMethod), + // The v2 dispatcher also accepts these deprecated v1 method names. + KeyringRpcMethod.ListAccountAssets, + KeyringRpcMethod.ListAccountTransactions, +]); + +/** + * `getAccountBalances` and `resolveAccountAddress` are omitted from the + * implemented type because they intentionally narrow their parameters to the + * Stellar scopes and asset IDs. The Keyring API declares them as function-typed + * properties, so their parameters are checked contravariantly and a narrower + * type is rejected. Both are validated at runtime by `validateRequest`. + */ +export class KeyringHandler implements Omit< + KeyringSnapRpc, + 'getAccountBalances' | 'resolveAccountAddress' +> { readonly #logger: ILogger; readonly #accountService: AccountService; @@ -124,7 +136,12 @@ export class KeyringHandler implements Keyring { method: request.method, }); validateOrigin(origin, request.method); - const keyringRequestResult = await handleKeyringRequest(this, request); + const keyringRequestResult = V2_METHODS.has(request.method) + ? await handleKeyringRequest( + this as unknown as KeyringSnapRpc, + request, + ) + : await handleKeyringRequestV1(this as unknown as Keyring, request); this.#logger.debug('Keyring request handled', { origin, method: request.method, @@ -136,46 +153,28 @@ export class KeyringHandler implements Keyring { return result; } - async listAccounts(): Promise { - const accounts = await this.#accountService.listAccounts(); - return accounts.map((account) => this.#toKeyringAccount(account)); - } - - async getAccount( - accountId: GetAccountRequest, - ): Promise { + async getAccount(accountId: GetAccountRequest): Promise { validateRequest(accountId, GetAccountRequestStruct); const account = await this.#accountService.findById(accountId); - return account ? this.#toKeyringAccount(account) : undefined; + if (!account) { + throw new AccountNotFoundException(accountId); + } + return this.#toKeyringAccount(account); } - async createAccount(options?: CreateAccountOptions): Promise { - validateRequest(options, CreateAccountOptionsStruct); - - const { account, isNewAccount } = - await this.#accountService.create(options); - - if (isNewAccount) { - try { - await this.#emitCreatedAccountEvent(account, options); - } catch (error: unknown) { - // Rollback if the event emission fails, e.g user rejected the account creation - try { - await this.#accountService.delete(account.id); - } catch (deleteError: unknown) { - // A more specific exception for the delete operation - throw new KeyringAccountRollbackException(account.id, { - cause: deleteError, - }); - } - - throw new KeyringEmitAccountCreatedEventException({ - cause: error, - }); - } - } + async getAccounts(): Promise { + const accounts = await this.#accountService.listAccounts(); + return accounts.map((account) => this.#toKeyringAccount(account)); + } - return this.#toKeyringAccount(account); + /** + * Keyring API v1 alias of {@link getAccounts}. MetaMask still calls + * `keyring_listAccounts` when resyncing Snap accounts. + * + * @returns The keyring accounts. + */ + async listAccounts(): Promise { + return this.getAccounts(); } /** @@ -190,66 +189,41 @@ export class KeyringHandler implements Keyring { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, + `${AccountCreationType.Bip44Discover}`, ] as const); - let accounts: KeyringAccount[] = []; - - if (options.type === AccountCreationType.Bip44DeriveIndex) { - const { account } = await this.#accountService.create({ + // For discovery, only create the account if it has on-chain activity. No + // activity means we've reached the end of the discoverable accounts, so we + // return nothing and the client stops discovering. + if (options.type === AccountCreationType.Bip44Discover) { + const account = await this.#accountService.deriveKeyringAccount({ entropySource: options.entropySource, index: options.groupIndex, }); - accounts.push(this.#toKeyringAccount(account)); - } else { - const createdAccounts = await this.#accountService.batchCreate({ - entropySource: options.entropySource, - fromIndex: options.range.from, - toIndex: options.range.to, - }); - accounts = createdAccounts.map((account) => - this.#toKeyringAccount(account), - ); + + if ( + !(await this.#hasOnChainActivity(account, [AppConfig.selectedNetwork])) + ) { + return []; + } } - return accounts; - } + let range; + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + range = options.range; + } else { + // Bip44DeriveIndex | Bip44Discover — a single group index. Ranges are + // inclusive, so `from` and `to` are the same. + range = { from: options.groupIndex, to: options.groupIndex }; + } - /** - * Emits the account-created event to the wallet. - * This triggers the wallet to prompt the user to add the account. - * If the user accepts, the account is added; if the user rejects, an error is thrown. - * - * @param account - The account to emit the event for. - * @param options - The options for the account creation. - * @returns A Promise that resolves when the event is emitted. - */ - async #emitCreatedAccountEvent( - account: StellarKeyringAccount, - options?: CreateAccountOptions, - ): Promise { - const keyringAccount = this.#toKeyringAccount(account); - await emitSnapKeyringEvent(getSnapProvider(), KeyringEvent.AccountCreated, { - /** - * We can't pass the `keyringAccount` object because it contains the index - * and the Snaps SDK does not allow extra properties. - */ - account: keyringAccount, - /** - * Skip account creation confirmation dialogs to make it look like a native - * account creation flow. - */ - displayConfirmation: false, - /** - * Internal options to MetaMask that include a correlation ID. We need - * to also emit this ID to the Snap keyring. - * Must be nested under `metamask` (keyring API). Do not spread - * `options.metamask` onto params or `correlationId` ends up at - * `params.correlationId` and fails validation (`never`). - */ - ...(options?.metamask?.correlationId === undefined - ? {} - : { metamask: { correlationId: options.metamask.correlationId } }), + const createdAccounts = await this.#accountService.batchCreate({ + entropySource: options.entropySource, + fromIndex: range.from, + toIndex: range.to, }); + + return createdAccounts.map((account) => this.#toKeyringAccount(account)); } #toKeyringAccount(account: StellarKeyringAccount): KeyringAccount { @@ -264,7 +238,7 @@ export class KeyringHandler implements Keyring { }; } - async listAccountAssets(accountId: string): Promise { + async getAccountAssets(accountId: string): Promise { validateRequest(accountId, ListAccountAssetsRequestStruct); const scope = AppConfig.selectedNetwork; @@ -283,7 +257,7 @@ export class KeyringHandler implements Keyring { return onChainAccount.assetIds; } - async listAccountTransactions( + async getAccountTransactions( accountId: string, pagination: Pagination, ): Promise<{ @@ -355,18 +329,7 @@ export class KeyringHandler implements Keyring { index: groupIndex, }); - const activityOnScopes = await Promise.all( - scopes.map(async (scope) => - this.#onChainAccountService.isAccountActivated({ - accountAddress: account.address, - scope, - }), - ), - ); - - const isActivated = activityOnScopes.some((active) => active); - - if (!isActivated) { + if (!(await this.#hasOnChainActivity(account, scopes))) { return []; } @@ -379,6 +342,29 @@ export class KeyringHandler implements Keyring { ]; } + /** + * Checks whether the given account is activated on any of the given scopes. + * + * @param account - The derived account to check. + * @param scopes - The scopes to check for on-chain activity. + * @returns Whether the account is activated on at least one scope. + */ + async #hasOnChainActivity( + account: StellarKeyringAccount, + scopes: KnownCaip2ChainId[], + ): Promise { + const activityOnScopes = await Promise.all( + scopes.map(async (scope) => + this.#onChainAccountService.isAccountActivated({ + accountAddress: account.address, + scope, + }), + ), + ); + + return activityOnScopes.some((active) => active); + } + async getAccountBalances( accountId: string, assets: KnownCaip19AssetIdOrSlip44Id[], @@ -454,40 +440,9 @@ export class KeyringHandler implements Keyring { } } - async filterAccountChains(_id: string, _chains: string[]): Promise { - // eslint-disable-next-line @typescript-eslint/only-throw-error -- MethodNotSupportedError is the keyring snap error surface - throw new MethodNotSupportedError('filterAccountChains'); - } - - async updateAccount(_account: KeyringAccount): Promise { - // eslint-disable-next-line @typescript-eslint/only-throw-error -- MethodNotSupportedError is the keyring snap error surface - throw new MethodNotSupportedError('updateAccount'); - } - async deleteAccount(accountId: string): Promise { validateRequest(accountId, DeleteAccountRequestStruct); - const { account } = await this.#accountService.resolveAccount({ - accountId, - }); - - try { - // The delete event is idempotent, so it is safe to emit it even if the - // account does not exist. - // @see https://github.com/MetaMask/accounts/blob/main/packages/keyring-api/README.md?plain=1#L162 - await emitSnapKeyringEvent( - getSnapProvider(), - KeyringEvent.AccountDeleted, - { - id: account.id, - }, - ); - } catch (error: unknown) { - throw new KeyringEmitAccountDeletedEventException({ - cause: error, - }); - } - await this.#accountService.delete(accountId); } @@ -518,11 +473,7 @@ export class KeyringHandler implements Keyring { } } - async submitRequest(request: KeyringRequest): Promise { - return { pending: false, result: await this.#handleSubmitRequest(request) }; - } - - async #handleSubmitRequest(request: KeyringRequest): Promise { + async submitRequest(request: KeyringRequest): Promise { const { method } = request.request; this.#assertMethodIsValid(method); diff --git a/packages/snap/src/permissions.ts b/packages/snap/src/permissions.ts index 658c3989..233efe0d 100644 --- a/packages/snap/src/permissions.ts +++ b/packages/snap/src/permissions.ts @@ -1,4 +1,5 @@ import { KeyringRpcMethod } from '@metamask/keyring-api'; +import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { Environment } from './api'; import { AppConfig } from './config'; @@ -11,34 +12,45 @@ const allowedOrigins = isDev ? ['http://localhost:3000'] : prodOrigins; const dappPermissions = isDev ? new Set([ - // Keyring methods + // Keyring v2 methods + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.CreateAccounts, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, + // Keyring v1 methods kept for backwards compatibility — callers using + // old method names are still accepted by the permission layer. KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.CreateAccounts, - KeyringRpcMethod.DeleteAccount, + KeyringRpcMethod.FilterAccountChains, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, ]) : new Set([]); const metamaskPermissions = new Set([ - // Keyring methods + // Keyring v2 methods + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.CreateAccounts, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, + KeyringSnapRpcMethod.ResolveAccountAddress, + KeyringSnapRpcMethod.SetSelectedAccounts, + // Keyring v1 methods kept for backwards compatibility — callers using + // old method names are still accepted by the permission layer. KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.CreateAccounts, - KeyringRpcMethod.DeleteAccount, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, - KeyringRpcMethod.ResolveAccountAddress, - KeyringRpcMethod.SetSelectedAccounts, ]); const metamask = METAMASK_ORIGIN; diff --git a/packages/snap/src/services/account/AccountService.test.ts b/packages/snap/src/services/account/AccountService.test.ts index 45da0fd6..2697d384 100644 --- a/packages/snap/src/services/account/AccountService.test.ts +++ b/packages/snap/src/services/account/AccountService.test.ts @@ -6,7 +6,6 @@ import { DerivedAccountAddressMismatchException, } from './exceptions'; import { KnownCaip2ChainId } from '../../api'; -import { KEYRING_ACCOUNT_TYPE } from '../../constants'; import { generateMockStellarKeyringAccounts, mockAccountService, @@ -14,7 +13,7 @@ import { import { MultichainMethod } from '../../handlers/keyring/api'; import { mockBip32Node } from '../../utils/__mocks__/fixtures'; import { getBip32Entropy, getDefaultEntropySource } from '../../utils/snap'; -import { WalletService, getDerivationPath } from '../wallet'; +import { WalletService } from '../wallet'; import type { Wallet } from '../wallet/Wallet'; jest.mock('../../utils/logger'); @@ -51,149 +50,6 @@ describe('AccountService', () => { ), }); - describe('create', () => { - it('creates an account with default options', async () => { - const entropySource = 'entropy-source-default'; - const expectedIndex = 0; - const expectedDerivationPath = getDerivationPath(expectedIndex); - const { deriveAddressSpy } = getWalletServiceSpies(); - const { saveSpy, getAllSpy } = getAccountsRepositorySpies(); - getAllSpy.mockResolvedValue([]); - jest.mocked(getDefaultEntropySource).mockResolvedValue(entropySource); - - const result = await accountService.create(); - - expect(saveSpy).toHaveBeenCalledWith(result.account); - expect(deriveAddressSpy).toHaveBeenCalledWith({ - entropySource, - index: expectedIndex, - }); - expect(result).toStrictEqual({ - account: { - id: expect.any(String), - entropySource, - derivationPath: expectedDerivationPath, - index: expectedIndex, - type: KEYRING_ACCOUNT_TYPE, - address: expect.any(String), - scopes: [KnownCaip2ChainId.Mainnet], - methods: ['signMessage', 'signTransaction', 'signAuthEntry'], - options: { - entropy: { - type: 'mnemonic', - id: entropySource, - derivationPath: expectedDerivationPath, - groupIndex: expectedIndex, - }, - exportable: true, - }, - }, - isNewAccount: true, - }); - }); - - it('creates an account with options', async () => { - const { saveSpy, getAllSpy } = getAccountsRepositorySpies(); - getAllSpy.mockResolvedValue([]); - - const result = await accountService.create({ - entropySource: 'entropy-source-2', - index: 1, - }); - - expect(saveSpy).toHaveBeenCalledWith(result.account); - expect(result).toStrictEqual({ - account: { - id: expect.any(String), - entropySource: 'entropy-source-2', - derivationPath: "m/44'/148'/1'", - index: 1, - type: KEYRING_ACCOUNT_TYPE, - address: expect.any(String), - scopes: [KnownCaip2ChainId.Mainnet], - methods: [ - MultichainMethod.SignMessage, - MultichainMethod.SignTransaction, - MultichainMethod.SignAuthEntry, - ], - options: { - entropy: { - type: 'mnemonic', - id: 'entropy-source-2', - derivationPath: "m/44'/148'/1'", - groupIndex: 1, - }, - exportable: true, - }, - }, - isNewAccount: true, - }); - }); - - it('creates an account with lowest unused index', async () => { - const { saveSpy, getAllSpy } = getAccountsRepositorySpies(); - const entropySource = 'entropy-source-2'; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [_, ...restAccounts] = generateMockStellarKeyringAccounts( - 3, - entropySource, - ); - const expectedIndex = 0; - const expectedDerivationPath = getDerivationPath(expectedIndex); - getAllSpy.mockResolvedValue(restAccounts); - - const result = await accountService.create({ - entropySource, - }); - - expect(saveSpy).toHaveBeenCalledWith(result.account); - expect(result).toStrictEqual({ - account: { - id: expect.any(String), - entropySource, - derivationPath: expectedDerivationPath, - index: expectedIndex, - type: KEYRING_ACCOUNT_TYPE, - address: expect.any(String), - scopes: [KnownCaip2ChainId.Mainnet], - methods: [ - MultichainMethod.SignMessage, - MultichainMethod.SignTransaction, - MultichainMethod.SignAuthEntry, - ], - options: { - entropy: { - type: 'mnemonic', - id: entropySource, - derivationPath: expectedDerivationPath, - groupIndex: expectedIndex, - }, - exportable: true, - }, - }, - isNewAccount: true, - }); - }); - - it('returns an existing account if it already exists', async () => { - const { getAllSpy, saveSpy } = getAccountsRepositorySpies(); - const entropySource = 'entropy-source-1'; - const mockAccounts = generateMockStellarKeyringAccounts(5, entropySource); - getAllSpy.mockResolvedValue(mockAccounts); - - const result = await accountService.create({ - entropySource, - index: 0, - }); - - expect(saveSpy).not.toHaveBeenCalled(); - expect(result).toStrictEqual({ - account: mockAccounts[0], - isNewAccount: false, - }); - }); - }); - describe('batchCreate', () => { it('derives, persists in range order, and calls save once', async () => { const entropySource = 'entropy-source-default'; @@ -302,7 +158,10 @@ describe('AccountService', () => { describe('delete', () => { it('deletes an account', async () => { const { deleteSpy } = getAccountsRepositorySpies(); - const { account } = await accountService.create(); + const [account] = generateMockStellarKeyringAccounts( + 1, + 'entropy-source-1', + ) as [StellarKeyringAccount]; await accountService.delete(account.id); diff --git a/packages/snap/src/services/account/AccountService.ts b/packages/snap/src/services/account/AccountService.ts index b3049968..edfc9b68 100644 --- a/packages/snap/src/services/account/AccountService.ts +++ b/packages/snap/src/services/account/AccountService.ts @@ -15,7 +15,6 @@ import { batchesAll, createPrefixedLogger, getDefaultEntropySource, - getLowestIndex, getSnapProvider, } from '../../utils'; import { getDerivationPath } from '../wallet'; @@ -118,68 +117,6 @@ export class AccountService { return { account }; } - /** - * Creates a Stellar account with the given options. - * - * @param options - The options for account creation. - * @param options.entropySource - The entropy source to use for derivation. - * @param options.index - The derivation index to use (defaults to the lowest unused index). - * @returns A Promise that resolves to the account and whether it was newly created. - * When an account with the same derivation path and entropy source already exists, - * that account is returned with `isNewAccount: false` and nothing is persisted. - */ - async create(options?: { - entropySource?: EntropySourceId; - index?: number; - }): Promise<{ - account: StellarKeyringAccount; - isNewAccount: boolean; - }> { - const accounts = await this.#accountsRepository.getAll(); - - const entropySource = - options?.entropySource ?? (await getDefaultEntropySource()); - - const derivationIndex = - options?.index ?? - this.#getLowestUnusedIndex({ - entropySource, - accounts, - }); - - /** - * Now that we have the `entropySource` and `index` ready, - * we need to make sure that they do not correspond to an existing account already. - */ - const sameAccount = accounts.find( - (account) => - account.index === derivationIndex && - account.entropySource === entropySource, - ); - - if (sameAccount) { - this.#logger.warn( - 'An account already exists with the same derivation path and entropy source. Skipping account creation.', - ); - return { - account: sameAccount, - isNewAccount: false, - }; - } - - const account = await this.#deriveAccount({ - entropySource, - index: derivationIndex, - }); - - await this.#accountsRepository.save(account); - - return { - account, - isNewAccount: true, - }; - } - /** * Batch creates Stellar accounts with the given options. * @@ -392,36 +329,6 @@ export class AccountService { }); } - /** - * Finds the lowest unused derivation index for the given entropy source. - * Indexes are not guaranteed to be contiguous (accounts can be deleted), so this scans - * existing accounts and returns the smallest index not yet in use. - * - * @param options - The parameters for the index search. - * @param options.entropySource - The entropy source to scope the search. - * @param options.accounts - The existing accounts to check against. - * @returns The lowest unused index for that entropy source. - */ - #getLowestUnusedIndex({ - entropySource, - accounts, - }: { - entropySource: EntropySourceId; - accounts: StellarKeyringAccount[]; - }): number { - const sortedIndices: number[] = []; - if (accounts.length > 0) { - for (const account of accounts) { - if (account.entropySource === entropySource) { - sortedIndices.push(account.index); - } - } - sortedIndices.sort((first, second) => first - second); - } - - return getLowestIndex(sortedIndices); - } - #toStellarKeyringAccount({ entropySource, derivationPath, diff --git a/packages/snap/src/utils/requestResponse.test.ts b/packages/snap/src/utils/requestResponse.test.ts index 4ba0a3fe..75004403 100644 --- a/packages/snap/src/utils/requestResponse.test.ts +++ b/packages/snap/src/utils/requestResponse.test.ts @@ -1,4 +1,5 @@ import { KeyringRpcMethod } from '@metamask/keyring-api'; +import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { InvalidParamsError, SnapError, @@ -49,13 +50,18 @@ describe('validateResponse', () => { describe('validateOrigin', () => { it.each([ + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.CreateAccounts, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.DeleteAccount, + KeyringRpcMethod.FilterAccountChains, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, ])('allows method %s for allowed dapps', (method) => { @@ -65,17 +71,21 @@ describe('validateOrigin', () => { }); it.each([ + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.CreateAccounts, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, + KeyringSnapRpcMethod.ResolveAccountAddress, + KeyringSnapRpcMethod.SetSelectedAccounts, KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.DeleteAccount, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, - KeyringRpcMethod.ResolveAccountAddress, - KeyringRpcMethod.SetSelectedAccounts, ])('allows method %s for metamask', (method) => { const origin = METAMASK_ORIGIN; diff --git a/packages/snap/tsconfig.json b/packages/snap/tsconfig.json index cfb11651..fc211687 100644 --- a/packages/snap/tsconfig.json +++ b/packages/snap/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { - "module": "CommonJS", - "moduleResolution": "node", + "module": "preserve", + "moduleResolution": "bundler", "target": "es2023", "lib": ["ES2023", "DOM"], "skipLibCheck": true, diff --git a/yarn.lock b/yarn.lock index 812a9c77..d557cdc3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1625,6 +1625,19 @@ __metadata: languageName: node linkType: hard +"@metamask/approval-controller@npm:^9.0.1": + version: 9.0.2 + resolution: "@metamask/approval-controller@npm:9.0.2" + dependencies: + "@metamask/base-controller": "npm:^9.1.0" + "@metamask/messenger": "npm:^1.2.0" + "@metamask/rpc-errors": "npm:^7.0.2" + "@metamask/utils": "npm:^11.9.0" + nanoid: "npm:^3.3.8" + checksum: 10/f27d75901a1a8367a8972b30ebb097a063282c8eb40f3188a6255ed1e043313085de6d5feb9300660366d4d93c822ebe6b37219978806db0492c4fed761d3c6a + languageName: node + linkType: hard + "@metamask/auto-changelog@npm:^6.1.0": version: 6.1.1 resolution: "@metamask/auto-changelog@npm:6.1.1" @@ -1648,14 +1661,14 @@ __metadata: languageName: node linkType: hard -"@metamask/base-controller@npm:^9.0.0": - version: 9.0.0 - resolution: "@metamask/base-controller@npm:9.0.0" +"@metamask/base-controller@npm:^9.0.0, @metamask/base-controller@npm:^9.0.1, @metamask/base-controller@npm:^9.1.0": + version: 9.1.0 + resolution: "@metamask/base-controller@npm:9.1.0" dependencies: - "@metamask/messenger": "npm:^0.3.0" - "@metamask/utils": "npm:^11.8.1" + "@metamask/messenger": "npm:^1.1.1" + "@metamask/utils": "npm:^11.9.0" immer: "npm:^9.0.6" - checksum: 10/27554d34ec85c4b585b87850c90dfeaaf9c7e6430f2ab2fa80a1ec06ccc17641e118afab7ad765a0b7255ffef37bc9f6ca5065d459228a2dc660bc463293310d + checksum: 10/752b70b35026fdf31ea8feef638f06286dbbde8d17e1ba804085fdbedbb076d900d2d7b6db3d2af284b3aa8fa84c95a0ceea6a3a46ee7b7e29433f037e9afc69 languageName: node linkType: hard @@ -1673,9 +1686,9 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@npm:^11.16.0, @metamask/controller-utils@npm:^11.17.0": - version: 11.18.0 - resolution: "@metamask/controller-utils@npm:11.18.0" +"@metamask/controller-utils@npm:^11.16.0, @metamask/controller-utils@npm:^11.17.0, @metamask/controller-utils@npm:^11.19.0": + version: 11.20.0 + resolution: "@metamask/controller-utils@npm:11.20.0" dependencies: "@metamask/eth-query": "npm:^4.0.0" "@metamask/ethjs-unit": "npm:^0.3.0" @@ -1690,7 +1703,7 @@ __metadata: lodash: "npm:^4.17.21" peerDependencies: "@babel/runtime": ^7.0.0 - checksum: 10/0c96701e3639b887c51c02416158bfeef42bf80701a4ca0d1ad2c888a370ccd9a6f2cb4677ef075a17a341c0dc9135e91c4f3079a7d498d9368d2f5e6fdf5d1b + checksum: 10/a2ba778d00508a606ef4657cd6591a89f8f54136b3dd41f4f96f8effa2e9ad20de347e3c554f643cb248a002a17b478f95eafd4e3b4c3eca4bb40ae2e6bf7fdc languageName: node linkType: hard @@ -1779,17 +1792,18 @@ __metadata: languageName: node linkType: hard -"@metamask/json-rpc-engine@npm:^10.0.2, @metamask/json-rpc-engine@npm:^10.1.1, @metamask/json-rpc-engine@npm:^10.2.0, @metamask/json-rpc-engine@npm:^10.2.1, @metamask/json-rpc-engine@npm:^10.2.2": - version: 10.2.2 - resolution: "@metamask/json-rpc-engine@npm:10.2.2" +"@metamask/json-rpc-engine@npm:^10.0.2, @metamask/json-rpc-engine@npm:^10.1.1, @metamask/json-rpc-engine@npm:^10.2.1, @metamask/json-rpc-engine@npm:^10.2.2, @metamask/json-rpc-engine@npm:^10.2.4": + version: 10.5.0 + resolution: "@metamask/json-rpc-engine@npm:10.5.0" dependencies: + "@metamask/messenger": "npm:^1.2.0" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/safe-event-emitter": "npm:^3.0.0" "@metamask/utils": "npm:^11.9.0" "@types/deep-freeze-strict": "npm:^1.1.0" deep-freeze-strict: "npm:^1.1.1" klona: "npm:^2.0.6" - checksum: 10/e2449e80f8ca3aed58d0778c220eba6c98e0848359da2703bcb68879c1b315774a1a8a90b2a7cd8d3eb3e0f022f9d0e30503e75a2645ec32cbfc5ba2e537f807 + checksum: 10/2fa42bc57e6e268f0dfcdfe22d15f3c1c593acb5c96df1469403c389fc2a4e4ccb7c75efc00a02d987834d7f0dfb52f31328929ebb37d73b6719aa2e53a75466 languageName: node linkType: hard @@ -1865,6 +1879,20 @@ __metadata: languageName: node linkType: hard +"@metamask/messenger@npm:^1.0.0, @metamask/messenger@npm:^1.1.1, @metamask/messenger@npm:^1.2.0": + version: 1.2.0 + resolution: "@metamask/messenger@npm:1.2.0" + dependencies: + "@metamask/utils": "npm:^11.9.0" + yargs: "npm:^17.7.2" + peerDependencies: + typescript: ">=5.0.0" + bin: + messenger-generate-action-types: ./dist/generate-action-types/cli.mjs + checksum: 10/6818e4609d6162a436cc07955905f9e57ff6dbef841e9066a5fb9cc0538e981526fbcb5eef1fa1968d79212d57ddda2fce4dda5f87eb64d8d98f7db1216a6a98 + languageName: node + linkType: hard + "@metamask/number-to-bn@npm:^1.7.1": version: 1.7.1 resolution: "@metamask/number-to-bn@npm:1.7.1" @@ -1885,22 +1913,22 @@ __metadata: languageName: node linkType: hard -"@metamask/permission-controller@npm:^12.2.0": - version: 12.2.0 - resolution: "@metamask/permission-controller@npm:12.2.0" +"@metamask/permission-controller@npm:^12.2.0, @metamask/permission-controller@npm:^12.3.0": + version: 12.3.0 + resolution: "@metamask/permission-controller@npm:12.3.0" dependencies: - "@metamask/approval-controller": "npm:^8.0.0" - "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.17.0" - "@metamask/json-rpc-engine": "npm:^10.2.0" - "@metamask/messenger": "npm:^0.3.0" + "@metamask/approval-controller": "npm:^9.0.1" + "@metamask/base-controller": "npm:^9.0.1" + "@metamask/controller-utils": "npm:^11.19.0" + "@metamask/json-rpc-engine": "npm:^10.2.4" + "@metamask/messenger": "npm:^1.0.0" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/utils": "npm:^11.9.0" "@types/deep-freeze-strict": "npm:^1.1.0" deep-freeze-strict: "npm:^1.1.1" immer: "npm:^9.0.6" nanoid: "npm:^3.3.8" - checksum: 10/d15ce9b69b3f8dbed2409d2e789e800d06798e42e55ac05095f19db0feda7492a64e221865ec6ee3d41b6c809ba5467f2bdab443c2d99133df88ac624ae264b8 + checksum: 10/a5fe9f2bab8c2d41cd829cd6c1af970e71da97eac42de17071c10f90d975e9135a4e6987ed6b2f3ea2209b1c6c51b822508f800225fda2207cdc598c16ea77dd languageName: node linkType: hard @@ -1981,9 +2009,9 @@ __metadata: linkType: hard "@metamask/slip44@npm:^4.4.0": - version: 4.4.0 - resolution: "@metamask/slip44@npm:4.4.0" - checksum: 10/296ac7c578bd35792c7e3942a9a0b7d9d7af76cf98358b97403c1ed483faa3c2fe6c71b1c3f8c7719fbfcf9fc73e5fa8707c89ac277ee9ce6c8bc4c694b2059d + version: 4.5.0 + resolution: "@metamask/slip44@npm:4.5.0" + checksum: 10/8450d4f5afa98e8f57c67a39f12aa413888c423505f22c05a3dc181be223f7742db017f54a99852acc3826abe74b22bf7247bccd78b810e509321c075e67516f languageName: node linkType: hard @@ -2201,34 +2229,34 @@ __metadata: languageName: node linkType: hard -"@metamask/snaps-utils@npm:^12.0.0, @metamask/snaps-utils@npm:^12.1.0": - version: 12.1.0 - resolution: "@metamask/snaps-utils@npm:12.1.0" +"@metamask/snaps-utils@npm:12.2.0": + version: 12.2.0 + resolution: "@metamask/snaps-utils@npm:12.2.0" dependencies: "@babel/core": "npm:^7.23.2" "@babel/types": "npm:^7.23.0" "@metamask/key-tree": "npm:^10.1.1" - "@metamask/messenger": "npm:^0.3.0" - "@metamask/permission-controller": "npm:^12.2.0" + "@metamask/messenger": "npm:^1.1.1" + "@metamask/permission-controller": "npm:^12.3.0" "@metamask/rpc-errors": "npm:^7.0.3" "@metamask/slip44": "npm:^4.4.0" "@metamask/snaps-registry": "npm:^4.0.0" - "@metamask/snaps-sdk": "npm:^10.4.0" + "@metamask/snaps-sdk": "npm:^11.1.0" "@metamask/superstruct": "npm:^3.2.1" - "@metamask/utils": "npm:^11.9.0" + "@metamask/utils": "npm:^11.11.0" "@scure/base": "npm:^1.1.1" chalk: "npm:^4.1.2" cron-parser: "npm:^4.5.0" fast-deep-equal: "npm:^3.1.3" fast-json-stable-stringify: "npm:^2.1.0" - fast-xml-parser: "npm:^5.3.4" + fast-xml-parser: "npm:^5.5.6" luxon: "npm:^3.5.0" marked: "npm:^12.0.1" rfdc: "npm:^1.3.0" semver: "npm:^7.5.4" - ses: "npm:^1.14.0" + ses: "npm:^1.15.0" validate-npm-package-name: "npm:^5.0.0" - checksum: 10/f81a66a0f0d7710f18cfcebceacf2ec7ca9c468a4839b8cc15652cc046a15455c55a3fff846061ff7d1ecd4368c182566db4f73b3c8b4ed4b2a09adc57e2e711 + checksum: 10/0e7cb5a4deebad3dc98404486b7767be049e9f86173195e9b1ae577f197e67cdd392f1e05ebb146a199ffc3cd52013636637a4d1ac7f9fec164f3189be97df55 languageName: node linkType: hard @@ -6568,7 +6596,7 @@ __metadata: languageName: node linkType: hard -"fast-xml-parser@npm:^5.7.0": +"fast-xml-parser@npm:^5.5.6": version: 5.10.1 resolution: "fast-xml-parser@npm:5.10.1" dependencies: @@ -10495,14 +10523,14 @@ __metadata: languageName: node linkType: hard -"ses@npm:^1.14.0": - version: 1.14.0 - resolution: "ses@npm:1.14.0" +"ses@npm:^1.15.0": + version: 1.15.0 + resolution: "ses@npm:1.15.0" dependencies: "@endo/cache-map": "npm:^1.1.0" "@endo/env-options": "npm:^1.1.11" "@endo/immutable-arraybuffer": "npm:^1.1.2" - checksum: 10/bee10b958938fb3d153ea8f1b4514f8ddb390dc7533fe9cfc382dcc046bebd5fca02d80836bfb8f98e94609ffbe3580a0bb65428eb7e39d523315eacdc052300 + checksum: 10/494894d35de847103ca0e4357632d80436c0e3dbc0e0bf98448a962d2257cb9045db9c6e7737b4003846a690318fcf5c06ef5a9a51c8b47cd4ffeaf2fa330155 languageName: node linkType: hard From dd543147809533282cea99339e5a6a257d2a2fce Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Fri, 7 Aug 2026 00:43:54 +0700 Subject: [PATCH 2/5] fix: address Keyring API v2 review comments - Remove all Keyring API v1 methods, dual dispatch, and permissions - Implement the full KeyringSnapRpc interface without Omit - Use the snap manifest scopes as the source of truth for account scopes - Restrict createAccounts to the MetaMask origin - Remove the unneeded @metamask/snaps-utils resolution --- package.json | 1 - packages/snap/snap.manifest.json | 2 +- packages/snap/src/constants.ts | 11 + .../snap/src/handlers/keyring/api.test.ts | 51 --- packages/snap/src/handlers/keyring/api.ts | 9 - .../snap/src/handlers/keyring/keyring.test.ts | 133 +------- packages/snap/src/handlers/keyring/keyring.ts | 119 ++----- packages/snap/src/permissions.ts | 19 -- .../src/services/account/AccountService.ts | 7 +- .../snap/src/utils/requestResponse.test.ts | 24 +- yarn.lock | 312 ++++++++++++++++-- 11 files changed, 347 insertions(+), 341 deletions(-) diff --git a/package.json b/package.json index a527e74c..6e3a7416 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,6 @@ }, "resolutions": { "@metamask/snaps-sdk": "11.1.1", - "@metamask/snaps-utils": "12.2.0", "@stellar/stellar-sdk/axios@npm:1.14.0": "1.18.1", "axios/form-data@npm:^4.0.5": "^4.0.6", "@metamask/snaps-utils/fast-xml-parser@npm:^5.3.4": "^5.7.0", diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index f8c93cec..ffebb08f 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "R5GFkwtIWH9mNc989eLsAxLU+12Ffd2JR/N1Sqatirw=", + "shasum": "G1F+8GcOTJrPQwRXwwjvo/3yN2g0h49P9OnTvyxJm/M=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/constants.ts b/packages/snap/src/constants.ts index b5aabe20..0379c5ea 100644 --- a/packages/snap/src/constants.ts +++ b/packages/snap/src/constants.ts @@ -1,5 +1,16 @@ import { XlmAccountType } from '@metamask/keyring-api'; +import type { KnownCaip2ChainId } from './api'; +import snapManifest from '../snap.manifest.json'; + +/** + * Scopes supported by this snap for account creation and discovery. The snap + * manifest's `endowment:keyring` capabilities are the source of truth. + */ +export const SUPPORTED_SCOPES = snapManifest.initialPermissions[ + 'endowment:keyring' +].capabilities.scopes as KnownCaip2ChainId[]; + /** * The base reserve for the Stellar network. * diff --git a/packages/snap/src/handlers/keyring/api.test.ts b/packages/snap/src/handlers/keyring/api.test.ts index 613e49c0..0a07cc0c 100644 --- a/packages/snap/src/handlers/keyring/api.test.ts +++ b/packages/snap/src/handlers/keyring/api.test.ts @@ -3,7 +3,6 @@ import { assert, StructError } from '@metamask/superstruct'; import { CreateAccountOptionsStruct, ResolveAccountAddressRequestStruct, - DiscoverAccountsStruct, ListAccountTransactionsRequestStruct, MultichainMethod, MultichainMethodStruct, @@ -178,56 +177,6 @@ describe('ResolveAccountAddressRequestStruct', () => { }); }); -describe('DiscoverAccountsStruct', () => { - it('accepts a valid discoverAccounts request', () => { - const request = { - scopes: [KnownCaip2ChainId.Mainnet], - entropySource: 'entropy-source-1', - groupIndex: 0, - }; - expect(() => assert(request, DiscoverAccountsStruct)).not.toThrow(); - }); - - it('accepts multiple scopes', () => { - const request = { - scopes: [KnownCaip2ChainId.Mainnet, KnownCaip2ChainId.Testnet], - entropySource: 'entropy-source-1', - groupIndex: 0, - }; - expect(() => assert(request, DiscoverAccountsStruct)).not.toThrow(); - }); - - it.each([ - { - scopes: [], - entropySource: 'entropy-source-1', - groupIndex: 1, - }, - { - scopes: [KnownCaip2ChainId.Mainnet], - entropySource: 'entropy-source-1', - groupIndex: 1.5, - }, - { - scopes: [KnownCaip2ChainId.Mainnet], - entropySource: 'entropy-source-1', - groupIndex: -1, - }, - { - scopes: [KnownCaip2ChainId.Mainnet], - entropySource: 'entropy-source-1', - groupIndex: '0', - }, - { - scopes: 'invalid-chain-id' as KnownCaip2ChainId, - entropySource: 'entropy-source-1', - groupIndex: 0, - }, - ])('rejects an invalid discoverAccounts request', (request) => { - expect(() => assert(request, DiscoverAccountsStruct)).toThrow(StructError); - }); -}); - describe('SignMessageRequestStruct', () => { const validSignMessageRequest = { id: keyringRequestId, diff --git a/packages/snap/src/handlers/keyring/api.ts b/packages/snap/src/handlers/keyring/api.ts index 2e4d7b13..e0bd7818 100644 --- a/packages/snap/src/handlers/keyring/api.ts +++ b/packages/snap/src/handlers/keyring/api.ts @@ -91,15 +91,6 @@ export const ResolveAccountAddressRequestStruct = object({ scope: KnownCaip2ChainIdStruct, }); -/** - * Validation struct for the discoverAccounts request. - */ -export const DiscoverAccountsStruct = object({ - scopes: nonempty(array(KnownCaip2ChainIdStruct)), - entropySource: nonempty(string()), - groupIndex: min(integer(), 0), -}); - /** * Optional bag accepted by both SEP-43 sign methods. * diff --git a/packages/snap/src/handlers/keyring/keyring.test.ts b/packages/snap/src/handlers/keyring/keyring.test.ts index d7f8b0ea..8415d9f3 100644 --- a/packages/snap/src/handlers/keyring/keyring.test.ts +++ b/packages/snap/src/handlers/keyring/keyring.test.ts @@ -1,13 +1,5 @@ import type { KeyringAccount } from '@metamask/keyring-api'; -import { - AccountCreationType, - DiscoveredAccountType, - KeyringRpcMethod, -} from '@metamask/keyring-api'; -import { - emitSnapKeyringEvent, - handleKeyringRequest as handleKeyringRequestV1, -} from '@metamask/keyring-snap-sdk'; +import { AccountCreationType } from '@metamask/keyring-api'; import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError } from '@metamask/snaps-sdk'; import type { JsonRpcRequest } from '@metamask/snaps-sdk'; @@ -43,6 +35,7 @@ import { createMockTransactionService, generateMockTransactions, } from '../../services/transaction/__mocks__/transaction.fixtures'; +import { getDerivationPath } from '../../services/wallet'; import { getSlip44AssetId, getDefaultEntropySource, @@ -58,12 +51,6 @@ jest.mock('../../utils/requestResponse', () => ({ ...jest.requireActual('../../utils/requestResponse'), validateOrigin: jest.fn(), })); -jest.mock('@metamask/keyring-snap-sdk', () => ({ - emitSnapKeyringEvent: jest.fn(), - handleKeyringRequest: jest.fn(), - MethodNotSupportedError: jest.requireActual('@metamask/keyring-snap-sdk') - .MethodNotSupportedError, -})); jest.mock('@metamask/keyring-snap-sdk/v2', () => ({ handleKeyringRequest: jest.fn(), })); @@ -147,13 +134,8 @@ describe('KeyringHandler', () => { id: '1', jsonrpc: '2.0', } as JsonRpcRequest; - const v1Request = { - method: KeyringRpcMethod.ListAccounts, - id: '1', - jsonrpc: '2.0', - } as JsonRpcRequest; - it('routes Keyring API v2 methods to the v2 dispatcher', async () => { + it('routes keyring methods to the v2 dispatcher', async () => { jest.mocked(handleKeyringRequest).mockResolvedValue([]); const result = await keyringHandler.handle(METAMASK_ORIGIN, v2Request); @@ -162,46 +144,9 @@ describe('KeyringHandler', () => { keyringHandler, v2Request, ); - expect(handleKeyringRequestV1).not.toHaveBeenCalled(); expect(result).toStrictEqual([]); }); - it.each([KeyringRpcMethod.ListAccounts, KeyringRpcMethod.DiscoverAccounts])( - 'routes the v1-only method %s to the v1 dispatcher', - async (method) => { - jest.mocked(handleKeyringRequestV1).mockResolvedValue([]); - - const request = { ...v1Request, method } as JsonRpcRequest; - const result = await keyringHandler.handle(METAMASK_ORIGIN, request); - - expect(handleKeyringRequestV1).toHaveBeenCalledWith( - keyringHandler, - request, - ); - expect(handleKeyringRequest).not.toHaveBeenCalled(); - expect(result).toStrictEqual([]); - }, - ); - - it.each([ - KeyringRpcMethod.ListAccountAssets, - KeyringRpcMethod.ListAccountTransactions, - ])( - 'routes the deprecated v1 alias %s to the v2 dispatcher', - async (method) => { - jest.mocked(handleKeyringRequest).mockResolvedValue([]); - - const request = { ...v1Request, method } as JsonRpcRequest; - await keyringHandler.handle(METAMASK_ORIGIN, request); - - expect(handleKeyringRequest).toHaveBeenCalledWith( - keyringHandler, - request, - ); - expect(handleKeyringRequestV1).not.toHaveBeenCalled(); - }, - ); - it('returns null if the dispatcher returns null', async () => { jest.mocked(handleKeyringRequest).mockResolvedValue(null); @@ -275,69 +220,6 @@ describe('KeyringHandler', () => { }); }); - describe('listAccounts (v1)', () => { - it('returns the same accounts as getAccounts', async () => { - const expectedAccounts = generateMockStellarKeyringAccounts( - 3, - entropySourceId, - ); - const { listAccountsSpy } = getAccountServiceSpies(); - listAccountsSpy.mockResolvedValue(expectedAccounts); - - expect(await keyringHandler.listAccounts()).toStrictEqual( - expectedAccounts.map((account) => toKeyringAccount(account)), - ); - }); - }); - - describe('discoverAccounts (v1)', () => { - it('returns the derived account when it is activated on chain', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValue(true); - - const result = await keyringHandler.discoverAccounts( - [KnownCaip2ChainId.Mainnet], - entropySourceId, - 0, - ); - - expect(result).toStrictEqual([ - { - type: DiscoveredAccountType.Bip44, - scopes: [KnownCaip2ChainId.Mainnet], - derivationPath: mockAccount.derivationPath, - }, - ]); - }); - - it('returns an empty array when the account has no on-chain activity', async () => { - jest - .spyOn(AccountService.prototype, 'deriveKeyringAccount') - .mockResolvedValue(mockAccount); - jest - .spyOn(OnChainAccountService.prototype, 'isAccountActivated') - .mockResolvedValue(false); - - expect( - await keyringHandler.discoverAccounts( - [KnownCaip2ChainId.Mainnet], - entropySourceId, - 0, - ), - ).toStrictEqual([]); - }); - - it('throws an error if the discovery request is invalid', async () => { - await expect( - keyringHandler.discoverAccounts([], entropySourceId, 0), - ).rejects.toThrow(InvalidParamsError); - }); - }); - describe('createAccounts', () => { const accountsAt = (...indexes: number[]) => indexes.map((index) => @@ -349,7 +231,7 @@ describe('KeyringHandler', () => { ), ); - it('creates one account for bip44:derive-index without emitting AccountCreated', async () => { + it('creates one account for bip44:derive-index', async () => { const { batchCreateAccountSpy } = getAccountServiceSpies(); batchCreateAccountSpy.mockResolvedValue([mockAccount]); @@ -365,7 +247,6 @@ describe('KeyringHandler', () => { toIndex: 2, }); expect(result).toStrictEqual([toKeyringAccount(mockAccount)]); - expect(jest.mocked(emitSnapKeyringEvent)).not.toHaveBeenCalled(); }); it('creates accounts for each index in bip44:derive-index-range', async () => { @@ -390,7 +271,6 @@ describe('KeyringHandler', () => { expect(result[2]?.options).toMatchObject({ entropy: expect.objectContaining({ groupIndex: 3 }), }); - expect(jest.mocked(emitSnapKeyringEvent)).not.toHaveBeenCalled(); }); it('propagates errors when account creation fails', async () => { @@ -454,7 +334,7 @@ describe('KeyringHandler', () => { keyringHandler.createAccounts({ type: AccountCreationType.Bip44DerivePath, entropySource: entropySourceId, - derivationPath: `m/44'/148'/0'`, + derivationPath: getDerivationPath(0), }), ).rejects.toThrow('Unsupported create account option type'); }); @@ -738,7 +618,7 @@ describe('KeyringHandler', () => { it('throws an error if the account address resolution request is invalid', async () => { await expect( keyringHandler.resolveAccountAddress(KnownCaip2ChainId.Mainnet, { - method: 'invalid:method' as MultichainMethod, + method: 'invalid:method', id: '1', jsonrpc: '2.0', params: { @@ -756,7 +636,6 @@ describe('KeyringHandler', () => { await keyringHandler.deleteAccount(mockAccountId); expect(deleteSpy).toHaveBeenCalledWith(mockAccountId); - expect(jest.mocked(emitSnapKeyringEvent)).not.toHaveBeenCalled(); }); it('propagates errors when account deletion fails', async () => { diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index e294643e..5ce75c7f 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -1,7 +1,5 @@ import type { CreateAccountOptions as KeyringApiCreateAccountOptions, - DiscoveredAccount, - EntropySourceId, KeyringAccount, KeyringRequest, Pagination, @@ -9,29 +7,23 @@ import type { Transaction, Balance, } from '@metamask/keyring-api'; -import type { Keyring } from '@metamask/keyring-api'; import { AccountCreationType, assertCreateAccountOptionIsSupported, - DiscoveredAccountType, } from '@metamask/keyring-api'; -import { KeyringRpcMethod } from '@metamask/keyring-api'; import type { KeyringSnapRpc } from '@metamask/keyring-api/v2'; -import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; -import { handleKeyringRequest as handleKeyringRequestV1 } from '@metamask/keyring-snap-sdk'; import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError } from '@metamask/snaps-sdk'; import type { Json, JsonRpcRequest } from '@metamask/snaps-sdk'; -import type { CaipAssetTypeOrId } from '@metamask/utils'; - import type { - GetAccountRequest, - MultichainMethod, - ResolveAccountAddressJsonRpcRequest, -} from './api'; + CaipAssetType, + CaipAssetTypeOrId, + CaipChainId, +} from '@metamask/utils'; + +import type { GetAccountRequest, MultichainMethod } from './api'; import { DeleteAccountRequestStruct, - DiscoverAccountsStruct, GetAccountRequestStruct, ListAccountTransactionsRequestStruct, MultichainMethodStruct, @@ -46,6 +38,7 @@ import type { KnownCaip2ChainId, } from '../../api'; import { AppConfig } from '../../config'; +import { SUPPORTED_SCOPES } from '../../constants'; import type { AccountService, StellarKeyringAccount, @@ -75,29 +68,7 @@ import { } from '../../utils'; import { SyncAccountsHandler } from '../cronjob/syncAccounts'; -/** - * Methods routed to the Keyring API v2 dispatcher. Everything else falls back - * to the v1 dispatcher, which MetaMask still uses for `keyring_listAccounts` - * (account resync) and `keyring_discoverAccounts` (discovery). - */ -const V2_METHODS = new Set([ - ...Object.values(KeyringSnapRpcMethod), - // The v2 dispatcher also accepts these deprecated v1 method names. - KeyringRpcMethod.ListAccountAssets, - KeyringRpcMethod.ListAccountTransactions, -]); - -/** - * `getAccountBalances` and `resolveAccountAddress` are omitted from the - * implemented type because they intentionally narrow their parameters to the - * Stellar scopes and asset IDs. The Keyring API declares them as function-typed - * properties, so their parameters are checked contravariantly and a narrower - * type is rejected. Both are validated at runtime by `validateRequest`. - */ -export class KeyringHandler implements Omit< - KeyringSnapRpc, - 'getAccountBalances' | 'resolveAccountAddress' -> { +export class KeyringHandler implements KeyringSnapRpc { readonly #logger: ILogger; readonly #accountService: AccountService; @@ -136,12 +107,7 @@ export class KeyringHandler implements Omit< method: request.method, }); validateOrigin(origin, request.method); - const keyringRequestResult = V2_METHODS.has(request.method) - ? await handleKeyringRequest( - this as unknown as KeyringSnapRpc, - request, - ) - : await handleKeyringRequestV1(this as unknown as Keyring, request); + const keyringRequestResult = await handleKeyringRequest(this, request); this.#logger.debug('Keyring request handled', { origin, method: request.method, @@ -167,16 +133,6 @@ export class KeyringHandler implements Omit< return accounts.map((account) => this.#toKeyringAccount(account)); } - /** - * Keyring API v1 alias of {@link getAccounts}. MetaMask still calls - * `keyring_listAccounts` when resyncing Snap accounts. - * - * @returns The keyring accounts. - */ - async listAccounts(): Promise { - return this.getAccounts(); - } - /** * Batch account creation for the Snap keyring v2 path (no `AccountCreated` events). * @@ -201,9 +157,7 @@ export class KeyringHandler implements Omit< index: options.groupIndex, }); - if ( - !(await this.#hasOnChainActivity(account, [AppConfig.selectedNetwork])) - ) { + if (!(await this.#hasOnChainActivity(account, SUPPORTED_SCOPES))) { return []; } } @@ -314,34 +268,6 @@ export class KeyringHandler implements Omit< }; } - async discoverAccounts( - scopes: KnownCaip2ChainId[], - entropySource: EntropySourceId, - groupIndex: number, - ): Promise { - validateRequest( - { scopes, entropySource, groupIndex }, - DiscoverAccountsStruct, - ); - - const account = await this.#accountService.deriveKeyringAccount({ - entropySource, - index: groupIndex, - }); - - if (!(await this.#hasOnChainActivity(account, scopes))) { - return []; - } - - return [ - { - type: DiscoveredAccountType.Bip44, - scopes, - derivationPath: account.derivationPath, - }, - ]; - } - /** * Checks whether the given account is activated on any of the given scopes. * @@ -367,9 +293,12 @@ export class KeyringHandler implements Omit< async getAccountBalances( accountId: string, - assets: KnownCaip19AssetIdOrSlip44Id[], - ): Promise> { - validateRequest({ accountId, assets }, GetAccountBalancesRequestStruct); + assets: CaipAssetType[], + ): Promise> { + const { assets: knownAssets } = validateRequest( + { accountId, assets }, + GetAccountBalancesRequestStruct, + ); const scope = AppConfig.selectedNetwork; const assetBalances = {} as Record; @@ -381,14 +310,14 @@ export class KeyringHandler implements Omit< // If the account is not activated or not yet synced, return the native asset with zero balance if (onChainAccount === null) { - const nativeAssetId = assets.find(isSlip44Id); + const nativeAssetId = knownAssets.find(isSlip44Id); if (nativeAssetId !== undefined) { assetBalances[nativeAssetId] = getDefaultBalanceEntry(); } return assetBalances; } - for (const assetId of assets) { + for (const assetId of knownAssets) { const asset = onChainAccount.getAsset(assetId); // Skip when the asset is not visible (tombstone, zero SEP-41, or missing entry). if (asset === undefined) { @@ -411,10 +340,10 @@ export class KeyringHandler implements Omit< } async resolveAccountAddress( - scope: KnownCaip2ChainId, - request: ResolveAccountAddressJsonRpcRequest, + scope: CaipChainId, + request: JsonRpcRequest, ): Promise { - validateRequest( + const { scope: knownScope, request: sep43Request } = validateRequest( { request, scope, @@ -424,10 +353,10 @@ export class KeyringHandler implements Omit< try { const { account } = await this.#accountService.resolveAccount({ - scope, - accountAddress: request.params.opts.address, + scope: knownScope, + accountAddress: sep43Request.params.opts.address, }); - return { address: `${scope}:${account.address}` }; + return { address: `${knownScope}:${account.address}` }; } catch (error: unknown) { // Return `null` signals "this snap does not // own the requested address" so MetaMask's routing layer will fallback to diff --git a/packages/snap/src/permissions.ts b/packages/snap/src/permissions.ts index 233efe0d..354c9b19 100644 --- a/packages/snap/src/permissions.ts +++ b/packages/snap/src/permissions.ts @@ -1,4 +1,3 @@ -import { KeyringRpcMethod } from '@metamask/keyring-api'; import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { Environment } from './api'; @@ -12,28 +11,17 @@ const allowedOrigins = isDev ? ['http://localhost:3000'] : prodOrigins; const dappPermissions = isDev ? new Set([ - // Keyring v2 methods KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, - KeyringSnapRpcMethod.CreateAccounts, KeyringSnapRpcMethod.DeleteAccount, KeyringSnapRpcMethod.GetAccountBalances, KeyringSnapRpcMethod.SubmitRequest, KeyringSnapRpcMethod.GetAccountTransactions, KeyringSnapRpcMethod.GetAccountAssets, - // Keyring v1 methods kept for backwards compatibility — callers using - // old method names are still accepted by the permission layer. - KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.FilterAccountChains, - KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.ListAccountTransactions, - KeyringRpcMethod.ListAccountAssets, ]) : new Set([]); const metamaskPermissions = new Set([ - // Keyring v2 methods KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, KeyringSnapRpcMethod.CreateAccounts, @@ -44,13 +32,6 @@ const metamaskPermissions = new Set([ KeyringSnapRpcMethod.GetAccountAssets, KeyringSnapRpcMethod.ResolveAccountAddress, KeyringSnapRpcMethod.SetSelectedAccounts, - // Keyring v1 methods kept for backwards compatibility — callers using - // old method names are still accepted by the permission layer. - KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.ListAccountTransactions, - KeyringRpcMethod.ListAccountAssets, ]); const metamask = METAMASK_ORIGIN; diff --git a/packages/snap/src/services/account/AccountService.ts b/packages/snap/src/services/account/AccountService.ts index edfc9b68..1dada346 100644 --- a/packages/snap/src/services/account/AccountService.ts +++ b/packages/snap/src/services/account/AccountService.ts @@ -7,8 +7,7 @@ import type { StellarKeyringAccount, StellarDerivationPath } from './api'; import { AccountNotFoundException } from './exceptions'; import { assertSameAddress } from './utils'; import type { StellarAddress, KnownCaip2ChainId } from '../../api'; -import { AppConfig } from '../../config'; -import { KEYRING_ACCOUNT_TYPE } from '../../constants'; +import { KEYRING_ACCOUNT_TYPE, SUPPORTED_SCOPES } from '../../constants'; import { MultichainMethod } from '../../handlers/keyring/api'; import type { ILogger } from '../../utils'; import { @@ -349,8 +348,8 @@ export class AccountService { index, type: KEYRING_ACCOUNT_TYPE, address, - // Only selected network is supported for now - scopes: [AppConfig.selectedNetwork], + // The snap manifest's keyring capabilities are the source of truth. + scopes: SUPPORTED_SCOPES, options: { entropy: { type: 'mnemonic', diff --git a/packages/snap/src/utils/requestResponse.test.ts b/packages/snap/src/utils/requestResponse.test.ts index 75004403..ec0d0c60 100644 --- a/packages/snap/src/utils/requestResponse.test.ts +++ b/packages/snap/src/utils/requestResponse.test.ts @@ -1,4 +1,3 @@ -import { KeyringRpcMethod } from '@metamask/keyring-api'; import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { InvalidParamsError, @@ -52,24 +51,26 @@ describe('validateOrigin', () => { it.each([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, - KeyringSnapRpcMethod.CreateAccounts, KeyringSnapRpcMethod.DeleteAccount, KeyringSnapRpcMethod.GetAccountBalances, KeyringSnapRpcMethod.SubmitRequest, KeyringSnapRpcMethod.GetAccountTransactions, KeyringSnapRpcMethod.GetAccountAssets, - KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.FilterAccountChains, - KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.ListAccountTransactions, - KeyringRpcMethod.ListAccountAssets, ])('allows method %s for allowed dapps', (method) => { const origin = 'http://localhost:3000'; expect(() => validateOrigin(origin, method)).not.toThrow(); }); + it('rejects createAccounts for dapps', () => { + expect(() => + validateOrigin( + 'http://localhost:3000', + KeyringSnapRpcMethod.CreateAccounts, + ), + ).toThrow(UnauthorizedError); + }); + it.each([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, @@ -81,11 +82,6 @@ describe('validateOrigin', () => { KeyringSnapRpcMethod.GetAccountAssets, KeyringSnapRpcMethod.ResolveAccountAddress, KeyringSnapRpcMethod.SetSelectedAccounts, - KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.ListAccountTransactions, - KeyringRpcMethod.ListAccountAssets, ])('allows method %s for metamask', (method) => { const origin = METAMASK_ORIGIN; @@ -96,7 +92,7 @@ describe('validateOrigin', () => { 'rejects unauthorized origin %s', (origin) => { expect(() => - validateOrigin(origin as string, KeyringRpcMethod.ListAccounts), + validateOrigin(origin as string, KeyringSnapRpcMethod.GetAccounts), ).toThrow(UnauthorizedError); }, ); diff --git a/yarn.lock b/yarn.lock index d557cdc3..c8c5d322 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1013,6 +1013,219 @@ __metadata: languageName: node linkType: hard +"@ethersproject/abi@npm:^5.7.0": + version: 5.8.0 + resolution: "@ethersproject/abi@npm:5.8.0" + dependencies: + "@ethersproject/address": "npm:^5.8.0" + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/constants": "npm:^5.8.0" + "@ethersproject/hash": "npm:^5.8.0" + "@ethersproject/keccak256": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + "@ethersproject/strings": "npm:^5.8.0" + checksum: 10/a63ebc2c8ea795ceca5289abaf817bb402c83c330cffd0ae2d355be70c54050a21ddd408abd4fd0dce4c3fd5c5f091707be2095011c233022a52f2110e7012d6 + languageName: node + linkType: hard + +"@ethersproject/abstract-provider@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/abstract-provider@npm:5.8.0" + dependencies: + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/networks": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + "@ethersproject/transactions": "npm:^5.8.0" + "@ethersproject/web": "npm:^5.8.0" + checksum: 10/2066aa717c7ecf0b6defe47f4f0af21943ee76e47f6fdc461d89b15d8af76c37d25355b4f5d635ed30e7378eafb0599b283df8ef9133cef389d938946874200d + languageName: node + linkType: hard + +"@ethersproject/abstract-signer@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/abstract-signer@npm:5.8.0" + dependencies: + "@ethersproject/abstract-provider": "npm:^5.8.0" + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + checksum: 10/10986eb1520dd94efb34bc19de4f53a49bea023493a0df686711872eb2cb446f3cca3c98c1ecec7831497004822e16ead756d6c7d6977971eaa780f4d41db327 + languageName: node + linkType: hard + +"@ethersproject/address@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/address@npm:5.8.0" + dependencies: + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/keccak256": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/rlp": "npm:^5.8.0" + checksum: 10/4b8ef5b3001f065fae571d86f113395d0dd081a2f411c99e354da912d4138e14a1fbe206265725daeb55c4e735ddb761891b58779208c5e2acec03f3219ce6ef + languageName: node + linkType: hard + +"@ethersproject/base64@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/base64@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + checksum: 10/c83e4ee01a1e69d874277d05c0e3fbc2afcdb9c80507be6963d31c77e505e355191cbba2d8fecf1c922b68c1ff072ede7914981fd965f1d8771c5b0706beb911 + languageName: node + linkType: hard + +"@ethersproject/bignumber@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/bignumber@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + bn.js: "npm:^5.2.1" + checksum: 10/15538ba9eef8475bc14a2a2bb5f0d7ae8775cf690283cb4c7edc836761a4310f83d67afe33f6d0b8befd896b10f878d8ca79b89de6e6ebd41a9e68375ec77123 + languageName: node + linkType: hard + +"@ethersproject/bytes@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/bytes@npm:5.8.0" + dependencies: + "@ethersproject/logger": "npm:^5.8.0" + checksum: 10/b8956aa4f607d326107cec522a881effed62585d5b5c5ad66ada4f7f83b42fd6c6acb76f355ec7a57e4cadea62a0194e923f4b5142d50129fe03d2fe7fc664f8 + languageName: node + linkType: hard + +"@ethersproject/constants@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/constants@npm:5.8.0" + dependencies: + "@ethersproject/bignumber": "npm:^5.8.0" + checksum: 10/74830c44f4315a1058b905c73be7a9bb92850e45213cb28a957447b8a100f22a514f4500b0ea5ac7a995427cecef9918af39ae4e0e0ecf77aa4835b1ea5c3432 + languageName: node + linkType: hard + +"@ethersproject/hash@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/hash@npm:5.8.0" + dependencies: + "@ethersproject/abstract-signer": "npm:^5.8.0" + "@ethersproject/address": "npm:^5.8.0" + "@ethersproject/base64": "npm:^5.8.0" + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/keccak256": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + "@ethersproject/strings": "npm:^5.8.0" + checksum: 10/a355cc1120b51c5912d960c66e2d1e2fb9cceca7d02e48c3812abd32ac2480035d8345885f129d2ed1cde9fb044adad1f98e4ea39652fa96c5de9c2720e83d28 + languageName: node + linkType: hard + +"@ethersproject/keccak256@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/keccak256@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + js-sha3: "npm:0.8.0" + checksum: 10/af3621d2b18af6c8f5181dacad91e1f6da4e8a6065668b20e4c24684bdb130b31e45e0d4dbaed86d4f1314d01358aa119f05be541b696e455424c47849d81913 + languageName: node + linkType: hard + +"@ethersproject/logger@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/logger@npm:5.8.0" + checksum: 10/dab862d6cc3a4312f4c49d62b4a603f4b60707da8b8ff0fee6bdfee3cbed48b34ec8f23fedfef04dd3d24f2fa2d7ad2be753c775aa00fe24dcd400631d65004a + languageName: node + linkType: hard + +"@ethersproject/networks@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/networks@npm:5.8.0" + dependencies: + "@ethersproject/logger": "npm:^5.8.0" + checksum: 10/8e2f4c3fd3a701ebd3d767a5f3217f8ced45a9f8ebf830c73b2dd87107dd50777f4869c3c9cc946698e2c597d3fe53eadeec55d19af7769c7d6bdb4a1493fb6f + languageName: node + linkType: hard + +"@ethersproject/properties@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/properties@npm:5.8.0" + dependencies: + "@ethersproject/logger": "npm:^5.8.0" + checksum: 10/3bc1af678c1cf7c87f39aec24b1d86cfaa5da1f9f54e426558701fff1c088c1dcc9ec3e1f395e138bdfcda94a0161e7192f0596e11c8ff25d31735e6b33edc59 + languageName: node + linkType: hard + +"@ethersproject/rlp@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/rlp@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + checksum: 10/353f04618f44c822d20da607b055286b3374fc6ab9fc50b416140f21e410f6d6e89ff9d951bef667b8baf1314e2d5f0b47c5615c3f994a2c8b2d6c01c6329bb4 + languageName: node + linkType: hard + +"@ethersproject/signing-key@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/signing-key@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + bn.js: "npm:^5.2.1" + elliptic: "npm:6.6.1" + hash.js: "npm:1.1.7" + checksum: 10/07e5893bf9841e1d608c52b58aa240ed10c7aa01613ff45b15c312c1403887baa8ed543871721052d7b7dd75d80b1fa90945377b231d18ccb6986c6677c8315d + languageName: node + linkType: hard + +"@ethersproject/strings@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/strings@npm:5.8.0" + dependencies: + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/constants": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + checksum: 10/536264dad4b9ad42d8287be7b7a9f3e243d0172fafa459e22af2d416eb6fe6a46ff623ca5456457f841dec4b080939da03ed02ab9774dcd1f2391df9ef5a96bb + languageName: node + linkType: hard + +"@ethersproject/transactions@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/transactions@npm:5.8.0" + dependencies: + "@ethersproject/address": "npm:^5.8.0" + "@ethersproject/bignumber": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/constants": "npm:^5.8.0" + "@ethersproject/keccak256": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + "@ethersproject/rlp": "npm:^5.8.0" + "@ethersproject/signing-key": "npm:^5.8.0" + checksum: 10/b43fd97ee359154c9162037c7aedc23abafae3cedf78d8fd2e641e820a0443120d22c473ec9bb79e8301f179f61a6120d61b0b757560e3aad8ae2110127018ba + languageName: node + linkType: hard + +"@ethersproject/web@npm:^5.8.0": + version: 5.8.0 + resolution: "@ethersproject/web@npm:5.8.0" + dependencies: + "@ethersproject/base64": "npm:^5.8.0" + "@ethersproject/bytes": "npm:^5.8.0" + "@ethersproject/logger": "npm:^5.8.0" + "@ethersproject/properties": "npm:^5.8.0" + "@ethersproject/strings": "npm:^5.8.0" + checksum: 10/93aad7041ffae7a4f881cc8df3356a297d736b50e6e48952b3b76e547b83e4d9189bbf2f417543031e91e74568c54395d1bb43c3252c3adf4f7e1c0187012912 + languageName: node + linkType: hard + "@gar/promise-retry@npm:^1.0.0": version: 1.0.2 resolution: "@gar/promise-retry@npm:1.0.2" @@ -1707,6 +1920,28 @@ __metadata: languageName: node linkType: hard +"@metamask/controller-utils@npm:^12.0.0": + version: 12.3.0 + resolution: "@metamask/controller-utils@npm:12.3.0" + dependencies: + "@ethersproject/abi": "npm:^5.7.0" + "@metamask/eth-query": "npm:^4.0.0" + "@metamask/ethjs-unit": "npm:^0.3.0" + "@metamask/utils": "npm:^11.11.0" + "@spruceid/siwe-parser": "npm:2.1.0" + "@types/bn.js": "npm:^5.1.5" + bignumber.js: "npm:^9.1.2" + bn.js: "npm:^5.2.1" + cockatiel: "npm:^3.1.2" + eth-ens-namehash: "npm:^2.0.8" + fast-deep-equal: "npm:^3.1.3" + lodash: "npm:^4.17.21" + peerDependencies: + "@babel/runtime": ^7.0.0 + checksum: 10/4f67b480a8522f3f89e1988631f57cfde5a7fca6c6df68f4f88e1cdcb54efccb4239befd3a8b3b7b2735479a47733b99eb059d8a0c7ddf28bdc85be4b8c04afb + languageName: node + linkType: hard + "@metamask/eslint-config-jest@npm:^15.0.0": version: 15.0.0 resolution: "@metamask/eslint-config-jest@npm:15.0.0" @@ -1792,7 +2027,7 @@ __metadata: languageName: node linkType: hard -"@metamask/json-rpc-engine@npm:^10.0.2, @metamask/json-rpc-engine@npm:^10.1.1, @metamask/json-rpc-engine@npm:^10.2.1, @metamask/json-rpc-engine@npm:^10.2.2, @metamask/json-rpc-engine@npm:^10.2.4": +"@metamask/json-rpc-engine@npm:^10.0.2, @metamask/json-rpc-engine@npm:^10.1.1, @metamask/json-rpc-engine@npm:^10.2.1, @metamask/json-rpc-engine@npm:^10.2.2, @metamask/json-rpc-engine@npm:^10.2.4, @metamask/json-rpc-engine@npm:^10.5.0": version: 10.5.0 resolution: "@metamask/json-rpc-engine@npm:10.5.0" dependencies: @@ -1893,6 +2128,17 @@ __metadata: languageName: node linkType: hard +"@metamask/messenger@npm:^2.0.0": + version: 2.0.0 + resolution: "@metamask/messenger@npm:2.0.0" + dependencies: + "@metamask/utils": "npm:^11.11.0" + peerDependencies: + typescript: ">=5.0.0" + checksum: 10/c387511edd89db73b774f631fb4bb382c9254af2dd7f80f9bd45f776e618cf90f653cc0e9744b1bf491f71f99c8318b16e8230a847f619a4fca655ea3f6a08e1 + languageName: node + linkType: hard + "@metamask/number-to-bn@npm:^1.7.1": version: 1.7.1 resolution: "@metamask/number-to-bn@npm:1.7.1" @@ -1913,7 +2159,7 @@ __metadata: languageName: node linkType: hard -"@metamask/permission-controller@npm:^12.2.0, @metamask/permission-controller@npm:^12.3.0": +"@metamask/permission-controller@npm:^12.2.0": version: 12.3.0 resolution: "@metamask/permission-controller@npm:12.3.0" dependencies: @@ -1932,6 +2178,25 @@ __metadata: languageName: node linkType: hard +"@metamask/permission-controller@npm:^13.1.1": + version: 13.1.1 + resolution: "@metamask/permission-controller@npm:13.1.1" + dependencies: + "@metamask/approval-controller": "npm:^9.0.1" + "@metamask/base-controller": "npm:^9.1.0" + "@metamask/controller-utils": "npm:^12.0.0" + "@metamask/json-rpc-engine": "npm:^10.5.0" + "@metamask/messenger": "npm:^1.2.0" + "@metamask/rpc-errors": "npm:^7.0.2" + "@metamask/utils": "npm:^11.9.0" + "@types/deep-freeze-strict": "npm:^1.1.0" + deep-freeze-strict: "npm:^1.1.1" + immer: "npm:^9.0.6" + nanoid: "npm:^3.3.8" + checksum: 10/12c6a675217ff510b837543c6e71ea851a3a7bd4592195f9579e76e2779177d2d4f03fcdd4d5c12585598df73250f32d0d49cb114274fc8187cbc7081bbc913b + languageName: node + linkType: hard + "@metamask/phishing-controller@npm:^16.1.0": version: 16.1.0 resolution: "@metamask/phishing-controller@npm:16.1.0" @@ -2008,7 +2273,7 @@ __metadata: languageName: node linkType: hard -"@metamask/slip44@npm:^4.4.0": +"@metamask/slip44@npm:^4.5.0": version: 4.5.0 resolution: "@metamask/slip44@npm:4.5.0" checksum: 10/8450d4f5afa98e8f57c67a39f12aa413888c423505f22c05a3dc181be223f7742db017f54a99852acc3826abe74b22bf7247bccd78b810e509321c075e67516f @@ -2229,20 +2494,20 @@ __metadata: languageName: node linkType: hard -"@metamask/snaps-utils@npm:12.2.0": - version: 12.2.0 - resolution: "@metamask/snaps-utils@npm:12.2.0" +"@metamask/snaps-utils@npm:^12.0.0, @metamask/snaps-utils@npm:^12.1.0": + version: 12.4.0 + resolution: "@metamask/snaps-utils@npm:12.4.0" dependencies: "@babel/core": "npm:^7.23.2" "@babel/types": "npm:^7.23.0" "@metamask/key-tree": "npm:^10.1.1" - "@metamask/messenger": "npm:^1.1.1" - "@metamask/permission-controller": "npm:^12.3.0" + "@metamask/messenger": "npm:^2.0.0" + "@metamask/permission-controller": "npm:^13.1.1" "@metamask/rpc-errors": "npm:^7.0.3" - "@metamask/slip44": "npm:^4.4.0" + "@metamask/slip44": "npm:^4.5.0" "@metamask/snaps-registry": "npm:^4.0.0" - "@metamask/snaps-sdk": "npm:^11.1.0" - "@metamask/superstruct": "npm:^3.2.1" + "@metamask/snaps-sdk": "npm:^11.2.0" + "@metamask/superstruct": "npm:^3.3.0" "@metamask/utils": "npm:^11.11.0" "@scure/base": "npm:^1.1.1" chalk: "npm:^4.1.2" @@ -2254,9 +2519,9 @@ __metadata: marked: "npm:^12.0.1" rfdc: "npm:^1.3.0" semver: "npm:^7.5.4" - ses: "npm:^1.15.0" + ses: "npm:^2.1.0" validate-npm-package-name: "npm:^5.0.0" - checksum: 10/0e7cb5a4deebad3dc98404486b7767be049e9f86173195e9b1ae577f197e67cdd392f1e05ebb146a199ffc3cd52013636637a4d1ac7f9fec164f3189be97df55 + checksum: 10/8b430dabbcd6aa033ced988d91f6ad91c906015c5d43ec66adb805e057bed607e8d1fefce010b767372f933089fcd048922d58c70f19b77987c26f85ffd89d93 languageName: node linkType: hard @@ -2314,7 +2579,7 @@ __metadata: languageName: node linkType: hard -"@metamask/superstruct@npm:^3.1.0, @metamask/superstruct@npm:^3.2.1, @metamask/superstruct@npm:^3.4.1": +"@metamask/superstruct@npm:^3.1.0, @metamask/superstruct@npm:^3.2.1, @metamask/superstruct@npm:^3.3.0, @metamask/superstruct@npm:^3.4.1": version: 3.4.1 resolution: "@metamask/superstruct@npm:3.4.1" checksum: 10/d37b5662dc9bbe0d99e06eb951167fa745829ae3abfa103423e9d8c05e8712f1451376f8479d484722e1e1c1d881732da1efddccaca8868b530a786264b903b5 @@ -5729,7 +5994,7 @@ __metadata: languageName: node linkType: hard -"elliptic@npm:^6.5.3, elliptic@npm:^6.5.4": +"elliptic@npm:6.6.1, elliptic@npm:^6.5.3, elliptic@npm:^6.5.4": version: 6.6.1 resolution: "elliptic@npm:6.6.1" dependencies: @@ -7217,7 +7482,7 @@ __metadata: languageName: node linkType: hard -"hash.js@npm:^1.0.0, hash.js@npm:^1.0.3": +"hash.js@npm:1.1.7, hash.js@npm:^1.0.0, hash.js@npm:^1.0.3": version: 1.1.7 resolution: "hash.js@npm:1.1.7" dependencies: @@ -8471,6 +8736,13 @@ __metadata: languageName: node linkType: hard +"js-sha3@npm:0.8.0": + version: 0.8.0 + resolution: "js-sha3@npm:0.8.0" + checksum: 10/a49ac6d3a6bfd7091472a28ab82a94c7fb8544cc584ee1906486536ba1cb4073a166f8c7bb2b0565eade23c5b3a7b8f7816231e0309ab5c549b737632377a20c + languageName: node + linkType: hard + "js-sha3@npm:^0.5.7": version: 0.5.7 resolution: "js-sha3@npm:0.5.7" @@ -10523,14 +10795,14 @@ __metadata: languageName: node linkType: hard -"ses@npm:^1.15.0": - version: 1.15.0 - resolution: "ses@npm:1.15.0" +"ses@npm:^2.1.0": + version: 2.2.0 + resolution: "ses@npm:2.2.0" dependencies: "@endo/cache-map": "npm:^1.1.0" "@endo/env-options": "npm:^1.1.11" "@endo/immutable-arraybuffer": "npm:^1.1.2" - checksum: 10/494894d35de847103ca0e4357632d80436c0e3dbc0e0bf98448a962d2257cb9045db9c6e7737b4003846a690318fcf5c06ef5a9a51c8b47cd4ffeaf2fa330155 + checksum: 10/c74889dded06a760f164aac9d69cf02b019256167fa0efd3d9c2ee1083fe1d230e41ab6aaf32266fcda9e3b8903a8ff0313cae1a63b095808fe9c15c7b33a8b9 languageName: node linkType: hard From 21f20d090a60ed2958c35058be604e068a918a45 Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Mon, 10 Aug 2026 15:20:25 +0700 Subject: [PATCH 3/5] fix: address permissions and scopes review comments Remove the dapp permission set: the snap does not support dapp origins. Keep the Keyring API v1 asset and transaction method names for MetaMask, since consumers still call them and dropping them fails the client with an access restriction error on load. Move the scope type assertion out of the constant into a getSupportScopes util, and use it for the selected network struct so both stay aligned. --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/config.ts | 3 +- packages/snap/src/constants.ts | 6 +-- packages/snap/src/handlers/keyring/keyring.ts | 4 +- packages/snap/src/permissions.ts | 39 ++++++------------- .../src/services/account/AccountService.ts | 5 ++- .../snap/src/utils/requestResponse.test.ts | 22 +++-------- packages/snap/src/utils/scopes.ts | 18 +++++++++ 8 files changed, 45 insertions(+), 54 deletions(-) create mode 100644 packages/snap/src/utils/scopes.ts diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index ffebb08f..5f120ac6 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "G1F+8GcOTJrPQwRXwwjvo/3yN2g0h49P9OnTvyxJm/M=", + "shasum": "M0Q7GRawo81/jn38QVK631MifKbjQJLaa/jKw5RSci4=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/config.ts b/packages/snap/src/config.ts index 434c3db2..cb7ca0ec 100644 --- a/packages/snap/src/config.ts +++ b/packages/snap/src/config.ts @@ -19,6 +19,7 @@ import { UrlStruct, KnownCaip2ChainId, } from './api'; +import { getSupportScopes } from './utils/scopes'; const DEFAULT_TOKEN_API_BASE_URL = 'https://tokens.api.cx.metamask.io'; @@ -77,7 +78,7 @@ const networkConfigStruct = object({ * If the selected network is empty, it returns the default selected network. */ const selectedNetworkStruct = coerce( - defaulted(KnownCaip2ChainIdStruct, KnownCaip2ChainId.Mainnet), + defaulted(enums(getSupportScopes()), KnownCaip2ChainId.Mainnet), string(), (value: string) => (value === '' ? undefined : value.toLowerCase()), ); diff --git a/packages/snap/src/constants.ts b/packages/snap/src/constants.ts index 0379c5ea..588dafa3 100644 --- a/packages/snap/src/constants.ts +++ b/packages/snap/src/constants.ts @@ -1,15 +1,13 @@ import { XlmAccountType } from '@metamask/keyring-api'; -import type { KnownCaip2ChainId } from './api'; import snapManifest from '../snap.manifest.json'; /** * Scopes supported by this snap for account creation and discovery. The snap * manifest's `endowment:keyring` capabilities are the source of truth. */ -export const SUPPORTED_SCOPES = snapManifest.initialPermissions[ - 'endowment:keyring' -].capabilities.scopes as KnownCaip2ChainId[]; +export const SUPPORTED_SCOPES = + snapManifest.initialPermissions['endowment:keyring'].capabilities.scopes; /** * The base reserve for the Stellar network. diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index 5ce75c7f..f0c61f6b 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -38,7 +38,6 @@ import type { KnownCaip2ChainId, } from '../../api'; import { AppConfig } from '../../config'; -import { SUPPORTED_SCOPES } from '../../constants'; import type { AccountService, StellarKeyringAccount, @@ -66,6 +65,7 @@ import { validateRequest, withCatchAndThrowSnapError, } from '../../utils'; +import { getSupportScopes } from '../../utils/scopes'; import { SyncAccountsHandler } from '../cronjob/syncAccounts'; export class KeyringHandler implements KeyringSnapRpc { @@ -157,7 +157,7 @@ export class KeyringHandler implements KeyringSnapRpc { index: options.groupIndex, }); - if (!(await this.#hasOnChainActivity(account, SUPPORTED_SCOPES))) { + if (!(await this.#hasOnChainActivity(account, getSupportScopes()))) { return []; } } diff --git a/packages/snap/src/permissions.ts b/packages/snap/src/permissions.ts index 354c9b19..e9691cd9 100644 --- a/packages/snap/src/permissions.ts +++ b/packages/snap/src/permissions.ts @@ -1,26 +1,8 @@ +import { KeyringRpcMethod } from '@metamask/keyring-api'; import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; -import { Environment } from './api'; -import { AppConfig } from './config'; import { METAMASK_ORIGIN } from './constants'; -const isDev = AppConfig.environment !== Environment.Production; - -const prodOrigins = ['https://portfolio.metamask.io']; -const allowedOrigins = isDev ? ['http://localhost:3000'] : prodOrigins; - -const dappPermissions = isDev - ? new Set([ - KeyringSnapRpcMethod.GetAccounts, - KeyringSnapRpcMethod.GetAccount, - KeyringSnapRpcMethod.DeleteAccount, - KeyringSnapRpcMethod.GetAccountBalances, - KeyringSnapRpcMethod.SubmitRequest, - KeyringSnapRpcMethod.GetAccountTransactions, - KeyringSnapRpcMethod.GetAccountAssets, - ]) - : new Set([]); - const metamaskPermissions = new Set([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, @@ -32,13 +14,16 @@ const metamaskPermissions = new Set([ KeyringSnapRpcMethod.GetAccountAssets, KeyringSnapRpcMethod.ResolveAccountAddress, KeyringSnapRpcMethod.SetSelectedAccounts, + /** + * Keyring API v1 method names, kept because consumers still call them. + * Dropping them makes the client fail with an access restriction error on + * initial load. The v2 dispatcher maps them onto `getAccountAssets` and + * `getAccountTransactions`. + */ + KeyringRpcMethod.ListAccountAssets, + KeyringRpcMethod.ListAccountTransactions, ]); -const metamask = METAMASK_ORIGIN; - -export const originPermissions = new Map>([]); - -for (const origin of allowedOrigins) { - originPermissions.set(origin, dappPermissions); -} -originPermissions.set(metamask, metamaskPermissions); +export const originPermissions = new Map>([ + [METAMASK_ORIGIN, metamaskPermissions], +]); diff --git a/packages/snap/src/services/account/AccountService.ts b/packages/snap/src/services/account/AccountService.ts index 1dada346..729c923a 100644 --- a/packages/snap/src/services/account/AccountService.ts +++ b/packages/snap/src/services/account/AccountService.ts @@ -7,7 +7,7 @@ import type { StellarKeyringAccount, StellarDerivationPath } from './api'; import { AccountNotFoundException } from './exceptions'; import { assertSameAddress } from './utils'; import type { StellarAddress, KnownCaip2ChainId } from '../../api'; -import { KEYRING_ACCOUNT_TYPE, SUPPORTED_SCOPES } from '../../constants'; +import { KEYRING_ACCOUNT_TYPE } from '../../constants'; import { MultichainMethod } from '../../handlers/keyring/api'; import type { ILogger } from '../../utils'; import { @@ -16,6 +16,7 @@ import { getDefaultEntropySource, getSnapProvider, } from '../../utils'; +import { getSupportScopes } from '../../utils/scopes'; import { getDerivationPath } from '../wallet'; import type { WalletService } from '../wallet'; @@ -349,7 +350,7 @@ export class AccountService { type: KEYRING_ACCOUNT_TYPE, address, // The snap manifest's keyring capabilities are the source of truth. - scopes: SUPPORTED_SCOPES, + scopes: getSupportScopes(), options: { entropy: { type: 'mnemonic', diff --git a/packages/snap/src/utils/requestResponse.test.ts b/packages/snap/src/utils/requestResponse.test.ts index ec0d0c60..e322046b 100644 --- a/packages/snap/src/utils/requestResponse.test.ts +++ b/packages/snap/src/utils/requestResponse.test.ts @@ -51,24 +51,12 @@ describe('validateOrigin', () => { it.each([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, - KeyringSnapRpcMethod.DeleteAccount, - KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.CreateAccounts, KeyringSnapRpcMethod.SubmitRequest, - KeyringSnapRpcMethod.GetAccountTransactions, - KeyringSnapRpcMethod.GetAccountAssets, - ])('allows method %s for allowed dapps', (method) => { - const origin = 'http://localhost:3000'; - - expect(() => validateOrigin(origin, method)).not.toThrow(); - }); - - it('rejects createAccounts for dapps', () => { - expect(() => - validateOrigin( - 'http://localhost:3000', - KeyringSnapRpcMethod.CreateAccounts, - ), - ).toThrow(UnauthorizedError); + ])('rejects method %s for dapps', (method) => { + expect(() => validateOrigin('http://localhost:3000', method)).toThrow( + UnauthorizedError, + ); }); it.each([ diff --git a/packages/snap/src/utils/scopes.ts b/packages/snap/src/utils/scopes.ts new file mode 100644 index 00000000..a33e69d3 --- /dev/null +++ b/packages/snap/src/utils/scopes.ts @@ -0,0 +1,18 @@ +import { array, assert } from '@metamask/superstruct'; + +import type { KnownCaip2ChainId } from '../api'; +import { KnownCaip2ChainIdStruct } from '../api'; +import { SUPPORTED_SCOPES } from '../constants'; + +/** + * Scopes this snap supports, read from the `endowment:keyring` capabilities in + * the snap manifest. + * + * @returns The supported scopes. + * @throws When the manifest declares a scope that is not a known chain id. + */ +export function getSupportScopes(): KnownCaip2ChainId[] { + // Make sure the manifest only declares scopes this snap knows about. + assert(SUPPORTED_SCOPES, array(KnownCaip2ChainIdStruct)); + return SUPPORTED_SCOPES; +} From 0fcd967a06cc94f4fd080df2e76bd3a3836a32b1 Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Mon, 10 Aug 2026 16:29:01 +0700 Subject: [PATCH 4/5] fix: keep the connected dapp origin with an empty permission set The snap exposes no keyring method to dapps, so the origin maps to an empty set and every dapp call is rejected. --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/permissions.ts | 18 +++++++++++++++--- .../snap/src/utils/requestResponse.test.ts | 12 ++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 5f120ac6..dd124f8c 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "M0Q7GRawo81/jn38QVK631MifKbjQJLaa/jKw5RSci4=", + "shasum": "rXpQ2FCk6rexkojr8GZC4LrOn8SJyF/ucYVF35uYdRU=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/permissions.ts b/packages/snap/src/permissions.ts index e9691cd9..7106f893 100644 --- a/packages/snap/src/permissions.ts +++ b/packages/snap/src/permissions.ts @@ -3,6 +3,15 @@ import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { METAMASK_ORIGIN } from './constants'; +const prodOrigins = ['https://portfolio.metamask.io']; +const allowedOrigins = prodOrigins; + +/** + * Dapp origins are connected to the snap, but the snap does not expose any + * keyring method to them. The set is empty so every dapp call is rejected. + */ +const dappPermissions = new Set([]); + const metamaskPermissions = new Set([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, @@ -24,6 +33,9 @@ const metamaskPermissions = new Set([ KeyringRpcMethod.ListAccountTransactions, ]); -export const originPermissions = new Map>([ - [METAMASK_ORIGIN, metamaskPermissions], -]); +export const originPermissions = new Map>([]); + +for (const origin of allowedOrigins) { + originPermissions.set(origin, dappPermissions); +} +originPermissions.set(METAMASK_ORIGIN, metamaskPermissions); diff --git a/packages/snap/src/utils/requestResponse.test.ts b/packages/snap/src/utils/requestResponse.test.ts index e322046b..35bc5cab 100644 --- a/packages/snap/src/utils/requestResponse.test.ts +++ b/packages/snap/src/utils/requestResponse.test.ts @@ -1,3 +1,4 @@ +import { KeyringRpcMethod } from '@metamask/keyring-api'; import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { InvalidParamsError, @@ -59,6 +60,17 @@ describe('validateOrigin', () => { ); }); + it.each([ + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.SubmitRequest, + KeyringRpcMethod.ListAccountAssets, + ])('rejects method %s for the connected dapp origin', (method) => { + expect(() => + validateOrigin('https://portfolio.metamask.io', method), + ).toThrow(UnauthorizedError); + }); + it.each([ KeyringSnapRpcMethod.GetAccounts, KeyringSnapRpcMethod.GetAccount, From d6fe87e11ebdfb2b608c7e099fba4481e76e2f00 Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Tue, 11 Aug 2026 16:24:48 +0700 Subject: [PATCH 5/5] fix: address review nits Rename getSupportScopes to getSupportedScopes, use a plain request name in the handle tests, and fold the unknown-account test into the getAccount block. --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/config.ts | 4 ++-- packages/snap/src/handlers/keyring/keyring.test.ts | 12 +++++------- packages/snap/src/handlers/keyring/keyring.ts | 4 ++-- packages/snap/src/services/account/AccountService.ts | 4 ++-- packages/snap/src/utils/scopes.ts | 2 +- 6 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index dd124f8c..e5cf4d00 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "rXpQ2FCk6rexkojr8GZC4LrOn8SJyF/ucYVF35uYdRU=", + "shasum": "KKCzkWtnRdUUT9Y0h8yBcyM0ZVt9ZQHAjRBpSr921h8=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/config.ts b/packages/snap/src/config.ts index cb7ca0ec..90395aef 100644 --- a/packages/snap/src/config.ts +++ b/packages/snap/src/config.ts @@ -19,7 +19,7 @@ import { UrlStruct, KnownCaip2ChainId, } from './api'; -import { getSupportScopes } from './utils/scopes'; +import { getSupportedScopes } from './utils/scopes'; const DEFAULT_TOKEN_API_BASE_URL = 'https://tokens.api.cx.metamask.io'; @@ -78,7 +78,7 @@ const networkConfigStruct = object({ * If the selected network is empty, it returns the default selected network. */ const selectedNetworkStruct = coerce( - defaulted(enums(getSupportScopes()), KnownCaip2ChainId.Mainnet), + defaulted(enums(getSupportedScopes()), KnownCaip2ChainId.Mainnet), string(), (value: string) => (value === '' ? undefined : value.toLowerCase()), ); diff --git a/packages/snap/src/handlers/keyring/keyring.test.ts b/packages/snap/src/handlers/keyring/keyring.test.ts index 8415d9f3..6fce0ea8 100644 --- a/packages/snap/src/handlers/keyring/keyring.test.ts +++ b/packages/snap/src/handlers/keyring/keyring.test.ts @@ -129,7 +129,7 @@ describe('KeyringHandler', () => { }); describe('handle', () => { - const v2Request = { + const request = { method: 'keyring_getAccounts', id: '1', jsonrpc: '2.0', @@ -138,11 +138,11 @@ describe('KeyringHandler', () => { it('routes keyring methods to the v2 dispatcher', async () => { jest.mocked(handleKeyringRequest).mockResolvedValue([]); - const result = await keyringHandler.handle(METAMASK_ORIGIN, v2Request); + const result = await keyringHandler.handle(METAMASK_ORIGIN, request); expect(handleKeyringRequest).toHaveBeenCalledWith( keyringHandler, - v2Request, + request, ); expect(result).toStrictEqual([]); }); @@ -150,7 +150,7 @@ describe('KeyringHandler', () => { it('returns null if the dispatcher returns null', async () => { jest.mocked(handleKeyringRequest).mockResolvedValue(null); - const result = await keyringHandler.handle(METAMASK_ORIGIN, v2Request); + const result = await keyringHandler.handle(METAMASK_ORIGIN, request); expect(result).toBeNull(); }); @@ -181,10 +181,8 @@ describe('KeyringHandler', () => { InvalidParamsError, ); }); - }); - describe('getAccount (v2 semantics)', () => { - it('throws for an unknown account id instead of returning undefined', async () => { + it('throws for an unknown account id', async () => { const { findByIdSpy } = getAccountServiceSpies(); findByIdSpy.mockResolvedValue(undefined); diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index f0c61f6b..c01ddde4 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -65,7 +65,7 @@ import { validateRequest, withCatchAndThrowSnapError, } from '../../utils'; -import { getSupportScopes } from '../../utils/scopes'; +import { getSupportedScopes } from '../../utils/scopes'; import { SyncAccountsHandler } from '../cronjob/syncAccounts'; export class KeyringHandler implements KeyringSnapRpc { @@ -157,7 +157,7 @@ export class KeyringHandler implements KeyringSnapRpc { index: options.groupIndex, }); - if (!(await this.#hasOnChainActivity(account, getSupportScopes()))) { + if (!(await this.#hasOnChainActivity(account, getSupportedScopes()))) { return []; } } diff --git a/packages/snap/src/services/account/AccountService.ts b/packages/snap/src/services/account/AccountService.ts index 729c923a..376312cb 100644 --- a/packages/snap/src/services/account/AccountService.ts +++ b/packages/snap/src/services/account/AccountService.ts @@ -16,7 +16,7 @@ import { getDefaultEntropySource, getSnapProvider, } from '../../utils'; -import { getSupportScopes } from '../../utils/scopes'; +import { getSupportedScopes } from '../../utils/scopes'; import { getDerivationPath } from '../wallet'; import type { WalletService } from '../wallet'; @@ -350,7 +350,7 @@ export class AccountService { type: KEYRING_ACCOUNT_TYPE, address, // The snap manifest's keyring capabilities are the source of truth. - scopes: getSupportScopes(), + scopes: getSupportedScopes(), options: { entropy: { type: 'mnemonic', diff --git a/packages/snap/src/utils/scopes.ts b/packages/snap/src/utils/scopes.ts index a33e69d3..36937663 100644 --- a/packages/snap/src/utils/scopes.ts +++ b/packages/snap/src/utils/scopes.ts @@ -11,7 +11,7 @@ import { SUPPORTED_SCOPES } from '../constants'; * @returns The supported scopes. * @throws When the manifest declares a scope that is not a known chain id. */ -export function getSupportScopes(): KnownCaip2ChainId[] { +export function getSupportedScopes(): KnownCaip2ChainId[] { // Make sure the manifest only declares scopes this snap knows about. assert(SUPPORTED_SCOPES, array(KnownCaip2ChainIdStruct)); return SUPPORTED_SCOPES;