From 3b99fcf40a43574ec8835556b8c8cbc914cb7e0f Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 27 May 2026 22:01:30 +0200 Subject: [PATCH] feat(snap-account-service): always forward non-keyring events in :handleKeyringSnapMessage --- packages/accounts-controller/CHANGELOG.md | 8 ++ .../src/AccountsController.test.ts | 91 ++++++++++++++ .../src/AccountsController.ts | 45 +++++++ packages/accounts-controller/src/index.ts | 3 + packages/snap-account-service/CHANGELOG.md | 5 + .../src/SnapAccountService.test.ts | 111 ++++++++++++++++++ .../src/SnapAccountService.ts | 103 +++++++++++++++- packages/snap-account-service/src/index.ts | 3 + 8 files changed, 367 insertions(+), 2 deletions(-) diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index eb53fa85f3..40589f96db 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Now requires `SnapAccountService:account{AssetList,Balances,Transactions}Updated` events to be registered on the messenger. + +### Fixed + +- Re-publish `SnapAccountService:account{AssetList,Balances,Transactions}Updated` events as `AccountsController:account{AssetList,Balances,Transactions}Updated` events. + ## [38.1.1] ### Changed diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 939c27df91..d115dc812a 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -331,6 +331,9 @@ function buildAccountsControllerMessenger( 'SnapKeyring:accountAssetListUpdated', 'SnapKeyring:accountBalancesUpdated', 'SnapKeyring:accountTransactionsUpdated', + 'SnapAccountService:accountAssetListUpdated', + 'SnapAccountService:accountBalancesUpdated', + 'SnapAccountService:accountTransactionsUpdated', 'MultichainNetworkController:networkDidChange', ], }); @@ -2167,6 +2170,29 @@ describe('AccountsController', () => { expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); }); + it('re-publishes keyring events: SnapAccountService:accountBalancesUpdated', () => { + const { account, messenger } = setupTest(); + + const payload: AccountBalancesUpdatedEventPayload = { + balances: { + [account.id]: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + amount: '0.1', + unit: 'BTC', + }, + }, + }, + }; + + const mockRePublishedCallback = jest.fn(); + messenger.subscribe( + 'AccountsController:accountBalancesUpdated', + mockRePublishedCallback, + ); + messenger.publish('SnapAccountService:accountBalancesUpdated', payload); + expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); + }); + it('re-publishes keyring events: SnapKeyring:accountAssetListUpdated', () => { const { account, messenger } = setupTest(); @@ -2188,6 +2214,27 @@ describe('AccountsController', () => { expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); }); + it('re-publishes keyring events: SnapAccountService:accountAssetListUpdated', () => { + const { account, messenger } = setupTest(); + + const payload: AccountAssetListUpdatedEventPayload = { + assets: { + [account.id]: { + added: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + removed: ['bip122:000000000933ea01ad0ee984209779ba/slip44:0'], + }, + }, + }; + + const mockRePublishedCallback = jest.fn(); + messenger.subscribe( + 'AccountsController:accountAssetListUpdated', + mockRePublishedCallback, + ); + messenger.publish('SnapAccountService:accountAssetListUpdated', payload); + expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); + }); + it('re-publishes keyring events: SnapKeyring:accountTransactionsUpdated', () => { const { account, messenger } = setupTest(); @@ -2228,6 +2275,50 @@ describe('AccountsController', () => { messenger.publish('SnapKeyring:accountTransactionsUpdated', payload); expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); }); + + it('re-publishes keyring events: SnapAccountService:accountTransactionsUpdated', () => { + const { account, messenger } = setupTest(); + + const payload: AccountTransactionsUpdatedEventPayload = { + transactions: { + [account.id]: [ + { + id: 'f5d8ee39a430901c91a5917b9f2dc19d6d1a0e9cea205b009ca73dd04470b9a6', + timestamp: null, + chain: 'bip122:000000000019d6689c085ae165831e93', + status: 'submitted', + type: 'receive', + account: account.id, + from: [], + to: [], + fees: [ + { + type: 'base', + asset: { + fungible: true, + type: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + unit: 'BTC', + amount: '0.0001', + }, + }, + ], + events: [], + }, + ], + }, + }; + + const mockRePublishedCallback = jest.fn(); + messenger.subscribe( + 'AccountsController:accountTransactionsUpdated', + mockRePublishedCallback, + ); + messenger.publish( + 'SnapAccountService:accountTransactionsUpdated', + payload, + ); + expect(mockRePublishedCallback).toHaveBeenCalledWith(payload); + }); }); describe('handle MultichainNetworkController:networkDidChange event', () => { diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index e5216b49b1..55b6122932 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -206,6 +206,21 @@ export type AccountsControllerAccountAssetListUpdatedEvent = { payload: SnapKeyringAccountAssetListUpdatedEvent['payload']; }; +export type SnapAccountServiceAccountBalancesUpdatedEvent = { + type: `SnapAccountService:accountBalancesUpdated`; + payload: SnapKeyringAccountBalancesUpdatedEvent['payload']; +}; + +export type SnapAccountServiceAccountTransactionsUpdatedEvent = { + type: `SnapAccountService:accountTransactionsUpdated`; + payload: SnapKeyringAccountTransactionsUpdatedEvent['payload']; +}; + +export type SnapAccountServiceAccountAssetListUpdatedEvent = { + type: `SnapAccountService:accountAssetListUpdated`; + payload: SnapKeyringAccountAssetListUpdatedEvent['payload']; +}; + /** * @deprecated This type is deprecated and will be removed in a future version. * Use `AccountTreeController`, `MultichainAccountService`, or the Keyring API v2 instead. @@ -215,6 +230,9 @@ export type AllowedEvents = | SnapKeyringAccountAssetListUpdatedEvent | SnapKeyringAccountBalancesUpdatedEvent | SnapKeyringAccountTransactionsUpdatedEvent + | SnapAccountServiceAccountAssetListUpdatedEvent + | SnapAccountServiceAccountBalancesUpdatedEvent + | SnapAccountServiceAccountTransactionsUpdatedEvent | MultichainNetworkControllerNetworkDidChangeEvent; /** @@ -1287,6 +1305,15 @@ export class AccountsController extends BaseController< ), ); + this.messenger.subscribe( + 'SnapAccountService:accountAssetListUpdated', + (snapAccountEvent) => + this.#handleOnSnapKeyringAccountEvent( + 'AccountsController:accountAssetListUpdated', + snapAccountEvent, + ), + ); + this.messenger.subscribe( 'SnapKeyring:accountBalancesUpdated', (snapAccountEvent) => @@ -1296,6 +1323,15 @@ export class AccountsController extends BaseController< ), ); + this.messenger.subscribe( + 'SnapAccountService:accountBalancesUpdated', + (snapAccountEvent) => + this.#handleOnSnapKeyringAccountEvent( + 'AccountsController:accountBalancesUpdated', + snapAccountEvent, + ), + ); + this.messenger.subscribe( 'SnapKeyring:accountTransactionsUpdated', (snapAccountEvent) => @@ -1305,6 +1341,15 @@ export class AccountsController extends BaseController< ), ); + this.messenger.subscribe( + 'SnapAccountService:accountTransactionsUpdated', + (snapAccountEvent) => + this.#handleOnSnapKeyringAccountEvent( + 'AccountsController:accountTransactionsUpdated', + snapAccountEvent, + ), + ); + // Handle account change when multichain network is changed this.messenger.subscribe( 'MultichainNetworkController:networkDidChange', diff --git a/packages/accounts-controller/src/index.ts b/packages/accounts-controller/src/index.ts index bfaf0be8e5..8700a73f61 100644 --- a/packages/accounts-controller/src/index.ts +++ b/packages/accounts-controller/src/index.ts @@ -15,6 +15,9 @@ export type { AccountsControllerAccountBalancesUpdatesEvent, AccountsControllerAccountTransactionsUpdatedEvent, AccountsControllerAccountAssetListUpdatedEvent, + SnapAccountServiceAccountBalancesUpdatedEvent, + SnapAccountServiceAccountTransactionsUpdatedEvent, + SnapAccountServiceAccountAssetListUpdatedEvent, AllowedEvents, AccountsControllerEvents, AccountsControllerMessenger, diff --git a/packages/snap-account-service/CHANGELOG.md b/packages/snap-account-service/CHANGELOG.md index fb4a3878ae..ac626bb3d4 100644 --- a/packages/snap-account-service/CHANGELOG.md +++ b/packages/snap-account-service/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `SnapAccountService:account{AssetList,Balances,Transactions}Updated` events. + ### Changed - Faster `:getLegacySnapKeyring` ([#8865](https://github.com/MetaMask/core/pull/8865)) @@ -15,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Re-publish account-data update events from `:handleKeyringSnapMessage` without requiring the legacy Snap keyring. - Prevent double-lock in `:handleKeyringSnapMessage` for some events/methods ([#8860](https://github.com/MetaMask/core/pull/8860)) - The service messenger now requires the `KeyringController:withKeyringUnsafe` action. - We now check if the keyring is available before delegating those messages. diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index 6f0475964d..a0cccc11fb 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,5 +1,10 @@ import type { AccountGroupId } from '@metamask/account-api'; import type { SnapKeyring, SnapMessage } from '@metamask/eth-snap-keyring'; +import type { + AccountAssetListUpdatedEventPayload, + AccountBalancesUpdatedEventPayload, + AccountTransactionsUpdatedEventPayload, +} from '@metamask/keyring-api'; import { KeyringEvent } from '@metamask/keyring-api'; import { KeyringControllerError, @@ -663,6 +668,7 @@ describe('SnapAccountService', () => { method: KeyringEvent.AccountUpdated, params: {}, } as unknown as SnapMessage; + const MOCK_ACCOUNT_ID = '00000000-0000-4000-8000-000000000001'; it('forwards the call to the legacy Snap keyring and returns its result', async () => { const { service, mocks } = setup(); @@ -728,6 +734,111 @@ describe('SnapAccountService', () => { expect(mocks.KeyringController.withController).not.toHaveBeenCalled(); }); + describe('when the message is an account data update event', () => { + const accountBalancesUpdatedPayload: AccountBalancesUpdatedEventPayload = + { + balances: { + [MOCK_ACCOUNT_ID]: { + 'eip155:1/slip44:60': { + amount: '1', + unit: 'ETH', + }, + }, + }, + }; + const accountAssetListUpdatedPayload: AccountAssetListUpdatedEventPayload = + { + assets: { + [MOCK_ACCOUNT_ID]: { + added: ['eip155:1/slip44:60'], + removed: [], + }, + }, + }; + const accountTransactionsUpdatedPayload: AccountTransactionsUpdatedEventPayload = + { + transactions: { + [MOCK_ACCOUNT_ID]: [], + }, + }; + + it.each([ + [ + KeyringEvent.AccountBalancesUpdated, + 'SnapAccountService:accountBalancesUpdated', + accountBalancesUpdatedPayload, + ], + [ + KeyringEvent.AccountAssetListUpdated, + 'SnapAccountService:accountAssetListUpdated', + accountAssetListUpdatedPayload, + ], + [ + KeyringEvent.AccountTransactionsUpdated, + 'SnapAccountService:accountTransactionsUpdated', + accountTransactionsUpdatedPayload, + ], + ] as const)( + 'publishes %s without requiring the legacy Snap keyring', + async (method, event, payload) => { + const { service, rootMessenger, mocks } = setup(); + const handleKeyringSnapMessage = jest + .fn() + .mockResolvedValue({ ok: true }); + mockLegacySnapKeyring(mocks, { handleKeyringSnapMessage }); + const listener = jest.fn(); + rootMessenger.subscribe(event, listener); + + const result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, { + method, + params: payload, + }); + + expect(result).toBeNull(); + expect(listener).toHaveBeenCalledWith(payload); + expect(handleKeyringSnapMessage).not.toHaveBeenCalled(); + expect( + mocks.KeyringController.withKeyringUnsafe, + ).not.toHaveBeenCalled(); + expect(mocks.KeyringController.withController).not.toHaveBeenCalled(); + }, + ); + }); + + describe('when the message is a request resolution event', () => { + it.each([ + [ + KeyringEvent.RequestApproved, + { id: '00000000-0000-0000-0000-000000000002', result: true }, + ], + [ + KeyringEvent.RequestRejected, + { id: '00000000-0000-0000-0000-000000000002' }, + ], + ] as const)( + 'delegates %s to the legacy Snap keyring', + async (method, params) => { + const { service, mocks } = setup(); + const message = { method, params }; + const handleKeyringSnapMessage = jest + .fn() + .mockResolvedValue({ ok: true }); + mockLegacySnapKeyring(mocks, { handleKeyringSnapMessage }); + + const result = await service.handleKeyringSnapMessage( + MOCK_SNAP_ID, + message, + ); + + expect(handleKeyringSnapMessage).toHaveBeenCalledWith( + MOCK_SNAP_ID, + message, + ); + expect(result).toStrictEqual({ ok: true }); + }, + ); + }); + it('propagates non-KeyringNotFound errors from withKeyringUnsafe', async () => { const { service, mocks } = setup(); mocks.KeyringController.withKeyringUnsafe.mockImplementation(async () => { diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 4a3bf8eaa2..d6ef5fa48e 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -3,7 +3,17 @@ import { SnapKeyring as LegacySnapKeyring, SnapMessage, } from '@metamask/eth-snap-keyring'; -import { KeyringEvent } from '@metamask/keyring-api'; +import type { + AccountAssetListUpdatedEventPayload, + AccountBalancesUpdatedEventPayload, + AccountTransactionsUpdatedEventPayload, +} from '@metamask/keyring-api'; +import { + AccountAssetListUpdatedEventStruct, + AccountBalancesUpdatedEventStruct, + AccountTransactionsUpdatedEventStruct, + KeyringEvent, +} from '@metamask/keyring-api'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -33,6 +43,7 @@ import type { } from '@metamask/snaps-controllers'; import { SnapId } from '@metamask/snaps-sdk'; import type { Json } from '@metamask/utils'; +import { assertStruct } from '@metamask/utils'; import { projectLogger as log } from './logger'; import type { @@ -96,7 +107,25 @@ type AllowedActions = /** * Events that {@link SnapAccountService} exposes to other consumers. */ -export type SnapAccountServiceEvents = never; +export type SnapAccountServiceAccountBalancesUpdatedEvent = { + type: `${typeof serviceName}:accountBalancesUpdated`; + payload: [AccountBalancesUpdatedEventPayload]; +}; + +export type SnapAccountServiceAccountAssetListUpdatedEvent = { + type: `${typeof serviceName}:accountAssetListUpdated`; + payload: [AccountAssetListUpdatedEventPayload]; +}; + +export type SnapAccountServiceAccountTransactionsUpdatedEvent = { + type: `${typeof serviceName}:accountTransactionsUpdated`; + payload: [AccountTransactionsUpdatedEventPayload]; +}; + +export type SnapAccountServiceEvents = + | SnapAccountServiceAccountAssetListUpdatedEvent + | SnapAccountServiceAccountBalancesUpdatedEvent + | SnapAccountServiceAccountTransactionsUpdatedEvent; /** * Events from other messengers that {@link SnapAccountService} subscribes to. @@ -154,6 +183,27 @@ function isLegacySnapKeyring(keyring: { return keyring.type === KeyringTypes.snap; } +type AccountDataUpdatedKeyringEvent = + | KeyringEvent.AccountAssetListUpdated + | KeyringEvent.AccountBalancesUpdated + | KeyringEvent.AccountTransactionsUpdated; + +/** + * Checks if a Snap message method is an account data update event. + * + * @param method - The Snap message method. + * @returns `true` if the method can be forwarded without the legacy Snap keyring. + */ +function isAccountDataUpdatedKeyringEvent( + method: string, +): method is AccountDataUpdatedKeyringEvent { + return ( + method === KeyringEvent.AccountAssetListUpdated || + method === KeyringEvent.AccountBalancesUpdated || + method === KeyringEvent.AccountTransactionsUpdated + ); +} + /** * Service responsible for managing account management snaps. */ @@ -397,6 +447,12 @@ export class SnapAccountService { snapId: SnapId, message: SnapMessage, ): Promise { + const { method } = message; + + if (isAccountDataUpdatedKeyringEvent(method)) { + return this.#publishAccountDataUpdatedEvent(snapId, method, message); + } + let snapKeyring: LegacySnapKeyring | undefined = await this.#getLegacySnapKeyringIfAvailable(); @@ -437,6 +493,49 @@ export class SnapAccountService { return snapKeyring.handleKeyringSnapMessage(snapId, message); } + /** + * Publishes an account data update event from a Snap. + * + * @param snapId - ID of the Snap. + * @param method - Account data update event method. + * @param message - Message sent by the Snap. + * @returns `null`. + */ + #publishAccountDataUpdatedEvent( + snapId: SnapId, + method: AccountDataUpdatedKeyringEvent, + message: SnapMessage, + ): null { + log( + `Forwarding message "${method}" from Snap "${snapId}" as a SnapAccountService event...`, + ); + + if (method === KeyringEvent.AccountAssetListUpdated) { + assertStruct(message, AccountAssetListUpdatedEventStruct); + this.#messenger.publish( + 'SnapAccountService:accountAssetListUpdated', + message.params, + ); + return null; + } + + if (method === KeyringEvent.AccountBalancesUpdated) { + assertStruct(message, AccountBalancesUpdatedEventStruct); + this.#messenger.publish( + 'SnapAccountService:accountBalancesUpdated', + message.params, + ); + return null; + } + + assertStruct(message, AccountTransactionsUpdatedEventStruct); + this.#messenger.publish( + 'SnapAccountService:accountTransactionsUpdated', + message.params, + ); + return null; + } + /** * Forwards the accounts of the given account group to the Snap keyring. * diff --git a/packages/snap-account-service/src/index.ts b/packages/snap-account-service/src/index.ts index 1713e9e6ce..be70e8ca14 100644 --- a/packages/snap-account-service/src/index.ts +++ b/packages/snap-account-service/src/index.ts @@ -3,6 +3,9 @@ export type { SnapAccountServiceActions, SnapAccountServiceConfig, SnapAccountServiceEvents, + SnapAccountServiceAccountBalancesUpdatedEvent, + SnapAccountServiceAccountTransactionsUpdatedEvent, + SnapAccountServiceAccountAssetListUpdatedEvent, SnapAccountServiceMessenger, SnapAccountServiceOptions, } from './SnapAccountService';