diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 99c954f498f..a26ea87f020 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Add optional tracing configuration ([#7006](https://github.com/MetaMask/core/pull/7006)) + - For now, only the account discovery is being traced. - Limit Bitcoin and Tron providers to 3 concurrent account creations by default when creating multichain account groups ([#7052](https://github.com/MetaMask/core/pull/7052)) ## [2.1.0] diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 144d709033e..a62b47767d3 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -220,10 +220,12 @@ describe('MultichainAccountService', () => { expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, providerConfigs?.[EvmAccountProvider.NAME], + expect.any(Function), // TraceCallback ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, providerConfigs?.[SolAccountProvider.NAME], + expect.any(Function), // TraceCallback ); }); @@ -254,10 +256,12 @@ describe('MultichainAccountService', () => { expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, undefined, + expect.any(Function), // TraceCallback ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, providerConfigs?.[SolAccountProvider.NAME], + expect.any(Function), // TraceCallback ); }); }); @@ -563,10 +567,20 @@ describe('MultichainAccountService', () => { ); // Should emit updated event for the existing group + expect(publishSpy).toHaveBeenCalled(); expect(publishSpy).toHaveBeenCalledWith( 'MultichainAccountService:multichainAccountGroupUpdated', - expect.any(Object), + expect.objectContaining({ + groupIndex: 0, + }), ); + + const emittedGroup = publishSpy.mock.calls[0][1]; + expect(emittedGroup).toBeDefined(); + expect(emittedGroup).toHaveProperty('groupIndex', 0); + expect(emittedGroup).toHaveProperty('getAccounts'); + expect(emittedGroup).toHaveProperty('select'); + expect(emittedGroup).toHaveProperty('get'); }); it('creates new detected wallets and update reverse mapping on AccountsController:accountAdded', () => { diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index b628ab0558c..b019c2275c1 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -12,6 +12,7 @@ import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import { areUint8ArraysEqual } from '@metamask/utils'; +import { traceFallback } from './analytics'; import { projectLogger as log } from './logger'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; @@ -28,7 +29,10 @@ import { SolAccountProvider, type SolAccountProviderConfig, } from './providers/SolAccountProvider'; -import type { MultichainAccountServiceMessenger } from './types'; +import type { + MultichainAccountServiceConfig, + MultichainAccountServiceMessenger, +} from './types'; export const serviceName = 'MultichainAccountService'; @@ -42,6 +46,7 @@ export type MultichainAccountServiceOptions = { [EvmAccountProvider.NAME]?: EvmAccountProviderConfig; [SolAccountProvider.NAME]?: SolAccountProviderConfig; }; + config?: MultichainAccountServiceConfig; }; /** Reverse mapping object used to map account IDs and their wallet/multichain account. */ @@ -81,28 +86,35 @@ export class MultichainAccountService { * MultichainAccountService. * @param options.providers - Optional list of account * @param options.providerConfigs - Optional provider configs - * providers. + * @param options.config - Optional config. */ constructor({ messenger, providers = [], providerConfigs, + config, }: MultichainAccountServiceOptions) { this.#messenger = messenger; this.#wallets = new Map(); this.#accountIdToContext = new Map(); + // Pass trace callback directly to preserve original 'this' context + // This avoids binding the callback to the MultichainAccountService instance + const traceCallback = config?.trace ?? traceFallback; + // TODO: Rely on keyring capabilities once the keyring API is used by all keyrings. this.#providers = [ new EvmAccountProvider( this.#messenger, providerConfigs?.[EvmAccountProvider.NAME], + traceCallback, ), new AccountProviderWrapper( this.#messenger, new SolAccountProvider( this.#messenger, providerConfigs?.[SolAccountProvider.NAME], + traceCallback, ), ), // Custom account providers that can be provided by the MetaMask client. diff --git a/packages/multichain-account-service/src/analytics/index.ts b/packages/multichain-account-service/src/analytics/index.ts new file mode 100644 index 00000000000..bc9f922b435 --- /dev/null +++ b/packages/multichain-account-service/src/analytics/index.ts @@ -0,0 +1 @@ +export * from './traces'; diff --git a/packages/multichain-account-service/src/analytics/traces.test.ts b/packages/multichain-account-service/src/analytics/traces.test.ts new file mode 100644 index 00000000000..9f97d94bb0d --- /dev/null +++ b/packages/multichain-account-service/src/analytics/traces.test.ts @@ -0,0 +1,76 @@ +import type { TraceRequest } from '@metamask/controller-utils'; + +import { traceFallback } from './traces'; +import { TraceName } from '../constants/traces'; + +describe('MultichainAccountService - Traces', () => { + describe('TraceName', () => { + it('contains expected trace names', () => { + expect(TraceName).toStrictEqual({ + SnapDiscoverAccounts: 'Snap Discover Accounts', + EvmDiscoverAccounts: 'EVM Discover Accounts', + }); + }); + }); + + describe('traceFallback', () => { + let mockTraceRequest: TraceRequest; + + beforeEach(() => { + mockTraceRequest = { + name: TraceName.SnapDiscoverAccounts, + id: 'trace-id-123', + tags: {}, + }; + }); + + it('returns undefined when no function is provided', async () => { + const result = await traceFallback(mockTraceRequest); + + expect(result).toBeUndefined(); + }); + + it('executes the provided function and return its result', async () => { + const mockResult = 'test-result'; + const mockFn = jest.fn().mockReturnValue(mockResult); + + const result = await traceFallback(mockTraceRequest, mockFn); + + expect(mockFn).toHaveBeenCalledTimes(1); + expect(mockFn).toHaveBeenCalledWith(); + expect(result).toBe(mockResult); + }); + + it('executes async function and return its result', async () => { + const mockResult = { data: 'async-result' }; + const mockAsyncFn = jest.fn().mockResolvedValue(mockResult); + + const result = await traceFallback(mockTraceRequest, mockAsyncFn); + + expect(mockAsyncFn).toHaveBeenCalledTimes(1); + expect(result).toBe(mockResult); + }); + + it('handles function that throws an error', async () => { + const mockError = new Error('Test error'); + const mockFn = jest.fn().mockImplementation(() => { + throw mockError; + }); + + await expect(traceFallback(mockTraceRequest, mockFn)).rejects.toThrow( + mockError, + ); + expect(mockFn).toHaveBeenCalledTimes(1); + }); + + it('handles function that returns a rejected promise', async () => { + const mockError = new Error('Async error'); + const mockFn = jest.fn().mockRejectedValue(mockError); + + await expect(traceFallback(mockTraceRequest, mockFn)).rejects.toThrow( + mockError, + ); + expect(mockFn).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/multichain-account-service/src/analytics/traces.ts b/packages/multichain-account-service/src/analytics/traces.ts new file mode 100644 index 00000000000..57eee11c914 --- /dev/null +++ b/packages/multichain-account-service/src/analytics/traces.ts @@ -0,0 +1,25 @@ +import type { + TraceCallback, + TraceContext, + TraceRequest, +} from '@metamask/controller-utils'; + +/** + * Fallback function for tracing. + * This function is used when no specific trace function is provided. + * It executes the provided function in a trace context if available. + * + * @param _request - The trace request containing additional data and context. + * @param fn - The function to execute within the trace context. + * @returns A promise that resolves to the result of the executed function. + * If no function is provided, it resolves to undefined. + */ +export const traceFallback: TraceCallback = async ( + _request: TraceRequest, + fn?: (context?: TraceContext) => ReturnType, +): Promise => { + if (!fn) { + return undefined as ReturnType; + } + return await Promise.resolve(fn()); +}; diff --git a/packages/multichain-account-service/src/constants/traces.ts b/packages/multichain-account-service/src/constants/traces.ts new file mode 100644 index 00000000000..59d74c0a9da --- /dev/null +++ b/packages/multichain-account-service/src/constants/traces.ts @@ -0,0 +1,4 @@ +export enum TraceName { + 'SnapDiscoverAccounts' = 'Snap Discover Accounts', + 'EvmDiscoverAccounts' = 'EVM Discover Accounts', +} diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts index f9e0c8014ff..d61b7362146 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts @@ -8,7 +8,11 @@ import type { } from '@metamask/keyring-internal-api'; import { AccountProviderWrapper } from './AccountProviderWrapper'; -import { BtcAccountProvider } from './BtcAccountProvider'; +import { + BTC_ACCOUNT_PROVIDER_NAME, + BtcAccountProvider, +} from './BtcAccountProvider'; +import { TraceName } from '../constants/traces'; import { getMultichainAccountServiceMessenger, getRootMessenger, @@ -324,4 +328,129 @@ describe('BtcAccountProvider', () => { expect(discovered).toStrictEqual([]); }); + + describe('trace functionality', () => { + it('calls trace callback during account discovery', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.SnapDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: BTC_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + // Simulate one discovered account at the requested index. + mocks.handleRequest.mockReturnValue([MOCK_BTC_P2TR_DISCOVERED_ACCOUNT_1]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const btcProvider = new BtcAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + btcProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + + it('uses fallback trace when no trace callback is provided', async () => { + const { provider, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([MOCK_BTC_P2TR_DISCOVERED_ACCOUNT_1]); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + // No trace errors, fallback trace should be used silently + }); + + it('trace callback is called even when discovery returns empty results', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.SnapDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: BTC_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const btcProvider = new BtcAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + btcProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toStrictEqual([]); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + + it('trace callback receives error when discovery fails', async () => { + const mockError = new Error('Discovery failed'); + const mockTrace = jest.fn().mockImplementation(async (_request, fn) => { + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockRejectedValue(mockError); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const btcProvider = new BtcAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + btcProvider, + ); + + await expect( + provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }), + ).rejects.toThrow(mockError); + + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts index f6a2d96d086..e7cadc56196 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts @@ -1,4 +1,5 @@ import { assertIsBip44Account, type Bip44Account } from '@metamask/account-api'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { BtcAccountType, BtcScope } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; @@ -12,6 +13,8 @@ import { type SnapAccountProviderConfig, } from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; +import { traceFallback } from '../analytics'; +import { TraceName } from '../constants/traces'; import type { MultichainAccountServiceMessenger } from '../types'; export type BtcAccountProviderConfig = SnapAccountProviderConfig; @@ -38,8 +41,9 @@ export class BtcAccountProvider extends SnapAccountProvider { backOffMs: 1000, }, }, + trace: TraceCallback = traceFallback, ) { - super(BtcAccountProvider.BTC_SNAP_ID, messenger, config); + super(BtcAccountProvider.BTC_SNAP_ID, messenger, config, trace); this.#client = this.#getKeyringClientFromSnapId( BtcAccountProvider.BTC_SNAP_ID, ); @@ -104,32 +108,45 @@ export class BtcAccountProvider extends SnapAccountProvider { }: { entropySource: EntropySourceId; groupIndex: number; - }) { - const discoveredAccounts = await withRetry( - () => - withTimeout( - this.#client.discoverAccounts( - [BtcScope.Mainnet], - entropySource, - groupIndex, - ), - this.config.discovery.timeoutMs, - ), + }): Promise[]> { + return await super.trace( { - maxAttempts: this.config.discovery.maxAttempts, - backOffMs: this.config.discovery.backOffMs, + name: TraceName.SnapDiscoverAccounts, + data: { + provider: this.getName(), + }, }, - ); + async () => { + const discoveredAccounts = await withRetry( + () => + withTimeout( + this.#client.discoverAccounts( + [BtcScope.Mainnet], + entropySource, + groupIndex, + ), + this.config.discovery.timeoutMs, + ), + { + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, + }, + ); - if (!Array.isArray(discoveredAccounts) || discoveredAccounts.length === 0) { - return []; - } + if ( + !Array.isArray(discoveredAccounts) || + discoveredAccounts.length === 0 + ) { + return []; + } - const createdAccounts = await this.createAccounts({ - entropySource, - groupIndex, - }); + const createdAccounts = await this.createAccounts({ + entropySource, + groupIndex, + }); - return createdAccounts; + return createdAccounts; + }, + ); } } diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 251dfd7cb56..cc10197dfe4 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -12,8 +12,12 @@ import type { import type { Hex } from '@metamask/utils'; import { createBytes } from '@metamask/utils'; -import { EvmAccountProvider } from './EvmAccountProvider'; +import { + EVM_ACCOUNT_PROVIDER_NAME, + EvmAccountProvider, +} from './EvmAccountProvider'; import { TimeoutError } from './utils'; +import { TraceName } from '../constants/traces'; import { getMultichainAccountServiceMessenger, getRootMessenger, @@ -203,7 +207,7 @@ function setup({ describe('EvmAccountProvider', () => { it('getName returns EVM', () => { const { provider } = setup({ accounts: [] }); - expect(provider.getName()).toBe('EVM'); + expect(provider.getName()).toBe(EVM_ACCOUNT_PROVIDER_NAME); }); it('gets accounts', () => { @@ -408,4 +412,117 @@ describe('EvmAccountProvider', () => { }), ).toStrictEqual([MOCK_HD_ACCOUNT_1]); }); + + it('calls trace callback during account discovery', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.EvmDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: EVM_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger } = setup({ + accounts: [], + }); + + const account = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withAddressSuffix('0') + .get(); + + const expectedAccount = { + ...account, + id: expect.any(String), + }; + + mockNextDiscoveryAddressOnce(account.address); + + // Create provider with custom trace callback + const providerWithTrace = new EvmAccountProvider( + getMultichainAccountServiceMessenger(messenger), + { + discovery: { + maxAttempts: 3, + timeoutMs: 500, + backOffMs: 500, + }, + }, + mockTrace, + ); + + const result = await providerWithTrace.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(result).toStrictEqual([expectedAccount]); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + + it('uses fallback trace when no trace callback is provided', async () => { + const { provider } = setup({ + accounts: [], + }); + + const account = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withAddressSuffix('0') + .get(); + + const expectedAccount = { + ...account, + id: expect.any(String), + }; + + mockNextDiscoveryAddressOnce(account.address); + + const result = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(result).toStrictEqual([expectedAccount]); + }); + + it('trace callback is called even when discovery returns empty results', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.EvmDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: EVM_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger } = setup({ + accounts: [], + discovery: { + transactionCount: '0x0', // No transactions, should return empty + }, + }); + + const account = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withAddressSuffix('0') + .get(); + + mockNextDiscoveryAddressOnce(account.address); + + const providerWithTrace = new EvmAccountProvider( + getMultichainAccountServiceMessenger(messenger), + { + discovery: { + maxAttempts: 3, + timeoutMs: 500, + backOffMs: 500, + }, + }, + mockTrace, + ); + + const result = await providerWithTrace.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(result).toStrictEqual([]); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 530d4e59093..2e4b60a210f 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -1,5 +1,6 @@ import { publicToAddress } from '@ethereumjs/util'; import type { Bip44Account } from '@metamask/account-api'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { HdKeyring } from '@metamask/eth-hd-keyring'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType } from '@metamask/keyring-api'; @@ -17,6 +18,8 @@ import { BaseBip44AccountProvider, } from './BaseBip44AccountProvider'; import { withRetry, withTimeout } from './utils'; +import { traceFallback } from '../analytics'; +import { TraceName } from '../constants/traces'; import type { MultichainAccountServiceMessenger } from '../types'; const ETH_MAINNET_CHAIN_ID = '0x1'; @@ -50,6 +53,8 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { readonly #config: EvmAccountProviderConfig; + readonly #trace: TraceCallback; + constructor( messenger: MultichainAccountServiceMessenger, config: EvmAccountProviderConfig = { @@ -59,9 +64,11 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { backOffMs: 500, }, }, + trace?: TraceCallback, ) { super(messenger); this.#config = config; + this.#trace = trace ?? traceFallback; } isAccountCompatible(account: Bip44Account): boolean { @@ -213,38 +220,48 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const provider = this.getEvmProvider(); - const { entropySource, groupIndex } = opts; + return this.#trace( + { + name: TraceName.EvmDiscoverAccounts, + data: { + provider: this.getName(), + }, + }, + async () => { + const provider = this.getEvmProvider(); + const { entropySource, groupIndex } = opts; - const addressFromGroupIndex = await this.#getAddressFromGroupIndex({ - entropySource, - groupIndex, - }); + const addressFromGroupIndex = await this.#getAddressFromGroupIndex({ + entropySource, + groupIndex, + }); - const count = await this.#getTransactionCount( - provider, - addressFromGroupIndex, - ); - if (count === 0) { - return []; - } + const count = await this.#getTransactionCount( + provider, + addressFromGroupIndex, + ); + if (count === 0) { + return []; + } - // We have some activity on this address, we try to create the account. - const [address] = await this.#createAccount({ - entropySource, - groupIndex, - }); - assert( - addressFromGroupIndex === address, - 'Created account does not match address from group index.', - ); + // We have some activity on this address, we try to create the account. + const [address] = await this.#createAccount({ + entropySource, + groupIndex, + }); + assert( + addressFromGroupIndex === address, + 'Created account does not match address from group index.', + ); - const account = this.messenger.call( - 'AccountsController:getAccountByAddress', - address, + const account = this.messenger.call( + 'AccountsController:getAccountByAddress', + address, + ); + assertInternalAccountExists(account); + assertIsBip44Account(account); + return [account]; + }, ); - assertInternalAccountExists(account); - assertIsBip44Account(account); - return [account]; } } diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts index 8d6b1c6f83d..588ebc3e960 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts @@ -1,22 +1,68 @@ import type { Bip44Account } from '@metamask/account-api'; +import type { TraceCallback, TraceRequest } from '@metamask/controller-utils'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { SnapId } from '@metamask/snaps-sdk'; +import { BtcAccountProvider } from './BtcAccountProvider'; import { isSnapAccountProvider, SnapAccountProvider, } from './SnapAccountProvider'; import { SolAccountProvider } from './SolAccountProvider'; +import { TrxAccountProvider } from './TrxAccountProvider'; +import { traceFallback } from '../analytics'; import { getMultichainAccountServiceMessenger, getRootMessenger, } from '../tests'; import type { MultichainAccountServiceMessenger } from '../types'; +jest.mock('../analytics', () => { + const actual = jest.requireActual('../analytics'); + return { + ...actual, + traceFallback: jest.fn(), + }; +}); + const THROTTLED_OPERATION_DELAY_MS = 10; const TEST_SNAP_ID = 'npm:@metamask/test-snap' as SnapId; const TEST_ENTROPY_SOURCE = 'test-entropy-source' as EntropySourceId; +// Helper to create a test provider that exposes protected trace method +class TestSnapAccountProvider extends SnapAccountProvider { + getName(): string { + return 'Test Provider'; + } + + isAccountCompatible(_account: Bip44Account): boolean { + return true; + } + + async discoverAccounts(_options: { + entropySource: EntropySourceId; + groupIndex: number; + }): Promise[]> { + return []; + } + + async createAccounts(_options: { + entropySource: EntropySourceId; + groupIndex: number; + }): Promise[]> { + return []; + } + + // Expose protected trace method as public for testing + async trace( + request: TraceRequest, + fn: () => Promise, + ): Promise { + return super.trace(request, fn); + } +} + // Helper to create a tracked provider that monitors concurrent execution const createTrackedProvider = (maxConcurrency?: number) => { const tracker: { @@ -83,6 +129,109 @@ const createTrackedProvider = (maxConcurrency?: number) => { }; describe('SnapAccountProvider', () => { + describe('constructor default parameters', () => { + const mockMessenger = { + call: jest.fn().mockResolvedValue({}), + registerActionHandler: jest.fn(), + subscribe: jest.fn(), + registerMethodActionHandlers: jest.fn(), + unregisterActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), + } as unknown as MultichainAccountServiceMessenger; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('creates SolAccountProvider with default trace using 1 parameter', () => { + const provider = new SolAccountProvider(mockMessenger); + expect(provider).toBeDefined(); + expect(provider.snapId).toBe(SolAccountProvider.SOLANA_SNAP_ID); + }); + + it('creates SolAccountProvider with default trace using 2 parameters', () => { + const provider = new SolAccountProvider(mockMessenger, undefined); + expect(provider).toBeDefined(); + expect(provider.snapId).toBe(SolAccountProvider.SOLANA_SNAP_ID); + }); + + it('creates SolAccountProvider with custom trace using 3 parameters', () => { + const customTrace = jest.fn(); + const provider = new SolAccountProvider( + mockMessenger, + undefined, + customTrace, + ); + expect(provider).toBeDefined(); + expect(provider.snapId).toBe(SolAccountProvider.SOLANA_SNAP_ID); + }); + + it('creates SolAccountProvider with custom config and default trace', () => { + const customConfig = { + discovery: { + timeoutMs: 3000, + maxAttempts: 5, + backOffMs: 2000, + }, + createAccounts: { + timeoutMs: 5000, + }, + }; + const provider = new SolAccountProvider(mockMessenger, customConfig); + expect(provider).toBeDefined(); + expect(provider.snapId).toBe(SolAccountProvider.SOLANA_SNAP_ID); + }); + + it('creates BtcAccountProvider with default trace', () => { + // Test other subclasses to ensure branch coverage + const btcProvider = new BtcAccountProvider(mockMessenger); + + expect(btcProvider).toBeDefined(); + expect(isSnapAccountProvider(btcProvider)).toBe(true); + }); + + it('creates TrxAccountProvider with custom trace', () => { + const customTrace = jest.fn(); + + // Explicitly test with all three parameters + const trxProvider = new TrxAccountProvider( + mockMessenger, + undefined, + customTrace, + ); + + expect(trxProvider).toBeDefined(); + expect(isSnapAccountProvider(trxProvider)).toBe(true); + }); + + it('creates provider without trace parameter', () => { + // Test creating provider without passing trace parameter + const provider = new SolAccountProvider(mockMessenger, undefined); + + expect(provider).toBeDefined(); + }); + + it('tests parameter spreading to trigger branch coverage', () => { + type SolConfig = ConstructorParameters[1]; + type ProviderArgs = [ + MultichainAccountServiceMessenger, + SolConfig?, + TraceCallback?, + ]; + const args: ProviderArgs = [mockMessenger]; + const provider1 = new SolAccountProvider(...args); + + args.push(undefined); + args.push(jest.fn()); + const provider2 = new SolAccountProvider(...args); + + expect(provider1).toBeDefined(); + expect(provider2).toBeDefined(); + }); + }); + describe('isSnapAccountProvider', () => { it('returns false for plain object with snapId property', () => { const mockProvider = { snapId: 'test-snap-id' }; @@ -128,6 +277,189 @@ describe('SnapAccountProvider', () => { }); }); + describe('trace functionality', () => { + const mockMessenger = { + call: jest.fn().mockResolvedValue({}), + registerActionHandler: jest.fn(), + subscribe: jest.fn(), + registerMethodActionHandlers: jest.fn(), + unregisterActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), + } as unknown as MultichainAccountServiceMessenger; + + const traceFallbackMock = traceFallback as jest.MockedFunction< + typeof traceFallback + >; + + beforeEach(() => { + jest.clearAllMocks(); + traceFallbackMock.mockClear(); + }); + + it('uses default trace parameter when only messenger is provided', async () => { + traceFallbackMock.mockImplementation(async (_request, fn) => fn?.()); + + // Test with default config and trace + const defaultConfig = { + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }; + const testProvider = new TestSnapAccountProvider( + TEST_SNAP_ID, + mockMessenger, + defaultConfig, + ); + const request = { name: 'Test Request', data: {} }; + const fn = jest.fn().mockResolvedValue('defaultResult'); + + await testProvider.trace(request, fn); + + expect(traceFallbackMock).toHaveBeenCalledTimes(1); + expect(traceFallbackMock).toHaveBeenCalledWith( + request, + expect.any(Function), + ); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('uses custom trace when explicitly provided with all parameters', async () => { + const customTrace = jest.fn().mockImplementation(async (_request, fn) => { + return await fn(); + }); + + // Test with all parameters including custom trace + const testProvider = new TestSnapAccountProvider( + TEST_SNAP_ID, + mockMessenger, + { + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }, + customTrace, + ); + const request = { name: 'Test Request', data: {} }; + const fn = jest.fn().mockResolvedValue('customResult'); + + const result = await testProvider.trace(request, fn); + + expect(result).toBe('customResult'); + expect(customTrace).toHaveBeenCalledTimes(1); + expect(customTrace).toHaveBeenCalledWith(request, expect.any(Function)); + expect(traceFallbackMock).not.toHaveBeenCalled(); + }); + + it('calls trace callback with the correct arguments', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request).toStrictEqual({ + name: 'Test Request', + data: { test: 'data' }, + }); + return await fn(); + }); + + const defaultConfig = { + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }; + const testProvider = new TestSnapAccountProvider( + TEST_SNAP_ID, + mockMessenger, + defaultConfig, + mockTrace, + ); + const request = { name: 'Test Request', data: { test: 'data' } }; + const fn = jest.fn().mockResolvedValue('testResult'); + + const result = await testProvider.trace(request, fn); + + expect(result).toBe('testResult'); + expect(mockTrace).toHaveBeenCalledTimes(1); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('propagates errors through trace callback', async () => { + const mockError = new Error('Test error'); + const mockTrace = jest.fn().mockImplementation(async (_request, fn) => { + return await fn(); + }); + + const defaultConfig = { + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }; + const testProvider = new TestSnapAccountProvider( + TEST_SNAP_ID, + mockMessenger, + defaultConfig, + mockTrace, + ); + const request = { name: 'Test Request', data: {} }; + const fn = jest.fn().mockRejectedValue(mockError); + + await expect(testProvider.trace(request, fn)).rejects.toThrow(mockError); + + expect(mockTrace).toHaveBeenCalledTimes(1); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('handles trace callback returning undefined', async () => { + const mockTrace = jest.fn().mockImplementation(async (_request, fn) => { + return await fn(); + }); + + const defaultConfig = { + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }; + const testProvider = new TestSnapAccountProvider( + TEST_SNAP_ID, + mockMessenger, + defaultConfig, + mockTrace, + ); + const request = { name: 'Test Request', data: {} }; + const fn = jest.fn().mockResolvedValue(undefined); + + const result = await testProvider.trace(request, fn); + + expect(result).toBeUndefined(); + expect(mockTrace).toHaveBeenCalledTimes(1); + expect(fn).toHaveBeenCalledTimes(1); + }); + }); + describe('withMaxConcurrency', () => { afterEach(() => { jest.clearAllTimers(); diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index a549600ee30..4f519b9b309 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -1,4 +1,5 @@ import { type Bip44Account } from '@metamask/account-api'; +import type { TraceCallback, TraceRequest } from '@metamask/controller-utils'; import type { SnapKeyring } from '@metamask/eth-snap-keyring'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; @@ -7,6 +8,7 @@ import type { Json, SnapId } from '@metamask/snaps-sdk'; import { Semaphore } from 'async-mutex'; import { BaseBip44AccountProvider } from './BaseBip44AccountProvider'; +import { traceFallback } from '../analytics'; import type { MultichainAccountServiceMessenger } from '../types'; export type RestrictedSnapKeyringCreateAccount = ( @@ -32,10 +34,14 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { readonly #queue?: Semaphore; + readonly #trace: TraceCallback; + constructor( snapId: SnapId, messenger: MultichainAccountServiceMessenger, config: SnapAccountProviderConfig, + /* istanbul ignore next */ + trace: TraceCallback = traceFallback, ) { super(messenger); @@ -51,6 +57,8 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { if (isFinite(maxConcurrency)) { this.#queue = new Semaphore(maxConcurrency); } + + this.#trace = trace; } /** @@ -70,6 +78,13 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { return operation(); } + protected async trace( + request: TraceRequest, + fn: () => Promise, + ): Promise { + return this.#trace(request, fn); + } + protected async getRestrictedSnapAccountCreator(): Promise { // NOTE: We're not supposed to make the keyring instance escape `withKeyring` but // we have to use the `SnapKeyring` instance to be able to create Solana account diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index d808d429b4b..68645fa31d9 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -7,7 +7,11 @@ import type { } from '@metamask/keyring-internal-api'; import { AccountProviderWrapper } from './AccountProviderWrapper'; -import { SolAccountProvider } from './SolAccountProvider'; +import { + SOL_ACCOUNT_PROVIDER_NAME, + SolAccountProvider, +} from './SolAccountProvider'; +import { TraceName } from '../constants/traces'; import { getMultichainAccountServiceMessenger, getRootMessenger, @@ -99,6 +103,7 @@ function setup({ keyring: { createAccount: jest.Mock; }; + trace: jest.Mock; }; } { const keyring = new MockSolanaKeyring(accounts); @@ -113,6 +118,15 @@ function setup({ .mockImplementation((address: string) => keyring.accounts.find((account) => account.address === address), ); + + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.SnapDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: SOL_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + messenger.registerActionHandler( 'SnapController:handleRequest', mockHandleRequest, @@ -132,7 +146,7 @@ function setup({ const multichainMessenger = getMultichainAccountServiceMessenger(messenger); const provider = new AccountProviderWrapper( multichainMessenger, - new SolAccountProvider(multichainMessenger), + new SolAccountProvider(multichainMessenger, undefined, mockTrace), ); return { @@ -144,6 +158,7 @@ function setup({ keyring: { createAccount: keyring.createAccount as jest.Mock, }, + trace: mockTrace, }, }; } @@ -307,4 +322,107 @@ describe('SolAccountProvider', () => { expect(discovered).toStrictEqual([]); }); + + describe('trace functionality', () => { + it('calls trace callback during account discovery', async () => { + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([MOCK_SOL_DISCOVERED_ACCOUNT_1]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const solProvider = new SolAccountProvider( + multichainMessenger, + undefined, + mocks.trace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + solProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + expect(mocks.trace).toHaveBeenCalledTimes(1); + }); + + it('uses fallback trace when no trace callback is provided', async () => { + const { provider, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([MOCK_SOL_DISCOVERED_ACCOUNT_1]); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + }); + + it('trace callback is called even when discovery returns empty results', async () => { + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const solProvider = new SolAccountProvider( + multichainMessenger, + undefined, + mocks.trace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + solProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toStrictEqual([]); + expect(mocks.trace).toHaveBeenCalledTimes(1); + }); + + it('trace callback receives error when discovery fails', async () => { + const mockError = new Error('Discovery failed'); + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockRejectedValue(mockError); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const solProvider = new SolAccountProvider( + multichainMessenger, + undefined, + mocks.trace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + solProvider, + ); + + await expect( + provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }), + ).rejects.toThrow(mockError); + + expect(mocks.trace).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 635104f5f4a..0404aba0893 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -1,4 +1,5 @@ import { assertIsBip44Account, type Bip44Account } from '@metamask/account-api'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { SolScope } from '@metamask/keyring-api'; import { @@ -17,6 +18,8 @@ import { type SnapAccountProviderConfig, } from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; +import { traceFallback } from '../analytics'; +import { TraceName } from '../constants/traces'; import type { MultichainAccountServiceMessenger } from '../types'; export type SolAccountProviderConfig = SnapAccountProviderConfig; @@ -43,8 +46,9 @@ export class SolAccountProvider extends SnapAccountProvider { timeoutMs: 3000, }, }, + trace: TraceCallback = traceFallback, ) { - super(SolAccountProvider.SOLANA_SNAP_ID, messenger, config); + super(SolAccountProvider.SOLANA_SNAP_ID, messenger, config, trace); this.#client = this.#getKeyringClientFromSnapId( SolAccountProvider.SOLANA_SNAP_ID, ); @@ -131,36 +135,46 @@ export class SolAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const discoveredAccounts = await withRetry( - () => - withTimeout( - this.#client.discoverAccounts( - [SolScope.Mainnet], - entropySource, - groupIndex, - ), - this.config.discovery.timeoutMs, - ), + return await super.trace( { - maxAttempts: this.config.discovery.maxAttempts, - backOffMs: this.config.discovery.backOffMs, + name: TraceName.SnapDiscoverAccounts, + data: { + provider: this.getName(), + }, }, - ); + async () => { + const discoveredAccounts = await withRetry( + () => + withTimeout( + this.#client.discoverAccounts( + [SolScope.Mainnet], + entropySource, + groupIndex, + ), + this.config.discovery.timeoutMs, + ), + { + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, + }, + ); - if (!discoveredAccounts.length) { - return []; - } - - const createdAccounts = await Promise.all( - discoveredAccounts.map((d) => - this.#createAccount({ - entropySource, - groupIndex, - derivationPath: d.derivationPath, - }), - ), - ); + if (!discoveredAccounts.length) { + return []; + } + + const createdAccounts = await Promise.all( + discoveredAccounts.map((d) => + this.#createAccount({ + entropySource, + groupIndex, + derivationPath: d.derivationPath, + }), + ), + ); - return createdAccounts; + return createdAccounts; + }, + ); } } diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts index 6c95295d316..fc0ca2e8c73 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts @@ -7,7 +7,11 @@ import type { } from '@metamask/keyring-internal-api'; import { AccountProviderWrapper } from './AccountProviderWrapper'; -import { TrxAccountProvider } from './TrxAccountProvider'; +import { + TRX_ACCOUNT_PROVIDER_NAME, + TrxAccountProvider, +} from './TrxAccountProvider'; +import { TraceName } from '../constants/traces'; import { getMultichainAccountServiceMessenger, getRootMessenger, @@ -308,4 +312,133 @@ describe('TrxAccountProvider', () => { expect(discovered).toStrictEqual([]); }); + + describe('trace functionality', () => { + it('calls trace callback during account discovery', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.SnapDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: TRX_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + // Simulate one discovered account at the requested index. + mocks.keyring.discoverAccounts.mockResolvedValue([ + MOCK_TRX_DISCOVERED_ACCOUNT_1, + ]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const trxProvider = new TrxAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + trxProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + + it('uses fallback trace when no trace callback is provided', async () => { + const { provider, mocks } = setup({ + accounts: [], + }); + + mocks.keyring.discoverAccounts.mockResolvedValue([ + MOCK_TRX_DISCOVERED_ACCOUNT_1, + ]); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toHaveLength(1); + // No trace errors, fallback trace should be used silently + }); + + it('trace callback is called even when discovery returns empty results', async () => { + const mockTrace = jest.fn().mockImplementation(async (request, fn) => { + expect(request.name).toBe(TraceName.SnapDiscoverAccounts); + expect(request.data).toStrictEqual({ + provider: TRX_ACCOUNT_PROVIDER_NAME, + }); + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.keyring.discoverAccounts.mockResolvedValue([]); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const trxProvider = new TrxAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + trxProvider, + ); + + const discovered = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toStrictEqual([]); + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + + it('trace callback receives error when discovery fails', async () => { + const mockError = new Error('Discovery failed'); + const mockTrace = jest.fn().mockImplementation(async (_request, fn) => { + return await fn(); + }); + + const { messenger, mocks } = setup({ + accounts: [], + }); + + mocks.keyring.discoverAccounts.mockRejectedValue(mockError); + + const multichainMessenger = + getMultichainAccountServiceMessenger(messenger); + const trxProvider = new TrxAccountProvider( + multichainMessenger, + undefined, + mockTrace, + ); + const provider = new AccountProviderWrapper( + multichainMessenger, + trxProvider, + ); + + await expect( + provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }), + ).rejects.toThrow(mockError); + + expect(mockTrace).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts index 87bd29102e2..eaa40ccba42 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts @@ -1,4 +1,5 @@ import { assertIsBip44Account, type Bip44Account } from '@metamask/account-api'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { TrxAccountType, TrxScope } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; @@ -13,6 +14,8 @@ import { type SnapAccountProviderConfig, } from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; +import { traceFallback } from '../analytics'; +import { TraceName } from '../constants/traces'; import type { MultichainAccountServiceMessenger } from '../types'; export type TrxAccountProviderConfig = SnapAccountProviderConfig; @@ -39,8 +42,9 @@ export class TrxAccountProvider extends SnapAccountProvider { timeoutMs: 3000, }, }, + trace: TraceCallback = traceFallback, ) { - super(TrxAccountProvider.TRX_SNAP_ID, messenger, config); + super(TrxAccountProvider.TRX_SNAP_ID, messenger, config, trace); this.#client = this.#getKeyringClientFromSnapId( TrxAccountProvider.TRX_SNAP_ID, ); @@ -106,31 +110,41 @@ export class TrxAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const discoveredAccounts = await withRetry( - () => - withTimeout( - this.#client.discoverAccounts( - [TrxScope.Mainnet], - entropySource, - groupIndex, - ), - this.config.discovery.timeoutMs, - ), + return await super.trace( { - maxAttempts: this.config.discovery.maxAttempts, - backOffMs: this.config.discovery.backOffMs, + name: TraceName.SnapDiscoverAccounts, + data: { + provider: this.getName(), + }, }, - ); + async () => { + const discoveredAccounts = await withRetry( + () => + withTimeout( + this.#client.discoverAccounts( + [TrxScope.Mainnet], + entropySource, + groupIndex, + ), + this.config.discovery.timeoutMs, + ), + { + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, + }, + ); - if (!discoveredAccounts.length) { - return []; - } + if (!discoveredAccounts.length) { + return []; + } - const createdAccounts = await this.createAccounts({ - entropySource, - groupIndex, - }); + const createdAccounts = await this.createAccounts({ + entropySource, + groupIndex, + }); - return createdAccounts; + return createdAccounts; + }, + ); } } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 0783b539799..700bc1d4c12 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -11,6 +11,7 @@ import type { AccountsControllerGetAccountByAddressAction, AccountsControllerListMultichainAccountsAction, } from '@metamask/accounts-controller'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { ErrorReportingServiceCaptureExceptionAction } from '@metamask/error-reporting-service'; import type { KeyringAccount } from '@metamask/keyring-api'; import type { @@ -157,3 +158,7 @@ export type MultichainAccountServiceMessenger = Messenger< MultichainAccountServiceActions | AllowedActions, MultichainAccountServiceEvents | AllowedEvents >; + +export type MultichainAccountServiceConfig = { + trace?: TraceCallback; +};