Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
117ee20
feat: add tracing to multichain account service
montelaidev Oct 28, 2025
af3d908
chore: update changelog
montelaidev Oct 30, 2025
d5c1eb3
fix: lint
montelaidev Oct 30, 2025
3190bca
fix: add test coverage
montelaidev Oct 30, 2025
291b8c6
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Oct 31, 2025
5170994
refactor: discover to AccountProviderWrapper
montelaidev Oct 31, 2025
0f351eb
fix: lint
montelaidev Oct 31, 2025
90d5e8c
fix: check provider before determining name
montelaidev Oct 31, 2025
82bf3f7
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Oct 31, 2025
a06ed0a
chore: add comment
montelaidev Oct 31, 2025
56a9cc2
refactor: move trace to concrete provider
montelaidev Nov 3, 2025
a38300d
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Nov 3, 2025
ccec24f
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Nov 4, 2025
019f4f8
fix: trace bind
montelaidev Nov 5, 2025
7683d3b
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Nov 5, 2025
553e283
fix: use constants in test
montelaidev Nov 6, 2025
7d6ffe4
fix: create mock provider
montelaidev Nov 6, 2025
d225a86
fix: replace mock with spy
montelaidev Nov 6, 2025
bc61d3a
Merge remote-tracking branch 'origin/main' into fix/mul-1186
montelaidev Nov 7, 2025
24b68c6
fix: lint
montelaidev Nov 7, 2025
3f5ebc4
fix: lint
montelaidev Nov 7, 2025
aebc20c
fix: use constantsfor trace names and data
montelaidev Nov 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/multichain-account-service/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- Adds an optional trace argument to MultichainAccountService and MultichainAccountWallet ([#7006](https://github.com/MetaMask/core/pull/7006))

## [2.0.0]

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,30 @@ describe('MultichainAccountService', () => {
);
});

it('accepts trace callback in config', () => {
const mockTrace = jest.fn().mockImplementation((_request, fn) => fn?.());
const config = { trace: mockTrace };

// Test that the service can be constructed with a trace config
const rootMessenger = getRootMessenger();
const messenger = getMultichainAccountServiceMessenger(rootMessenger);

// Mock the required handlers for initialization
rootMessenger.registerActionHandler(
'KeyringController:getState',
jest.fn().mockReturnValue({ isUnlocked: true, keyrings: [] }),
);

const serviceWithTrace = new MultichainAccountService({
messenger,
config,
});

serviceWithTrace.init();

expect(serviceWithTrace).toBeDefined();
});

it('allows optional configs for some providers', () => {
const providerConfigs: MultichainAccountServiceOptions['providerConfigs'] =
{
Expand Down Expand Up @@ -1090,6 +1114,152 @@ describe('MultichainAccountService', () => {
(solProvider.isAccountCompatible as jest.Mock).mockReturnValue(false);
expect(wrapper.isAccountCompatible(MOCK_HD_ACCOUNT_1)).toBe(false);
});

it('calls trace callback when discoverAccounts() is enabled', async () => {
const mockTrace = jest.fn().mockImplementation(async (request, fn) => {
expect(request.name).toBe('Snap Discover Accounts');
expect(request.data).toStrictEqual({
providerName: solProvider.getName(),
});
return await fn();
});

const options = {
entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id,
groupIndex: 0,
};

const wrapperWithTrace = new AccountProviderWrapper(
getMultichainAccountServiceMessenger(
setup({ accounts: [MOCK_HD_ACCOUNT_1] }).rootMessenger,
),
solProvider,
mockTrace,
);

(solProvider.discoverAccounts as jest.Mock).mockResolvedValue([
MOCK_HD_ACCOUNT_1,
]);

const result = await wrapperWithTrace.discoverAccounts(options);

expect(result).toStrictEqual([MOCK_HD_ACCOUNT_1]);
expect(mockTrace).toHaveBeenCalledTimes(1);
expect(solProvider.discoverAccounts).toHaveBeenCalledWith(options);
});

it('uses fallback trace when no trace callback is provided', async () => {
const options = {
entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id,
groupIndex: 0,
};

(solProvider.discoverAccounts as jest.Mock).mockResolvedValue([
MOCK_HD_ACCOUNT_1,
]);

// Wrapper without trace callback should use fallback
const result = await wrapper.discoverAccounts(options);

expect(result).toStrictEqual([MOCK_HD_ACCOUNT_1]);
expect(solProvider.discoverAccounts).toHaveBeenCalledWith(options);
});

it('uses correct trace name for Snap-based providers', async () => {
const mockTrace = jest.fn().mockImplementation(async (request, fn) => {
expect(request.name).toBe('Snap Discover Accounts');
expect(request.data).toStrictEqual({
providerName: solProvider.getName(),
});
return await fn();
});

const options = {
entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id,
groupIndex: 0,
};

const wrapperWithTrace = new AccountProviderWrapper(
getMultichainAccountServiceMessenger(
setup({ accounts: [MOCK_HD_ACCOUNT_1] }).rootMessenger,
),
solProvider,
mockTrace,
);

(solProvider.discoverAccounts as jest.Mock).mockResolvedValue([
MOCK_HD_ACCOUNT_1,
]);

const result = await wrapperWithTrace.discoverAccounts(options);

expect(result).toStrictEqual([MOCK_HD_ACCOUNT_1]);
expect(mockTrace).toHaveBeenCalledTimes(1);
});

it('uses correct trace name for EVM providers', async () => {
const mockTrace = jest.fn().mockImplementation(async (request, fn) => {
expect(request.name).toBe('EVM Discover Accounts');
expect(request.data).toStrictEqual({
providerName: 'EVM',
});
return await fn();
});

const { rootMessenger } = setup({ accounts: [MOCK_HD_ACCOUNT_1] });
const evmProvider = new EvmAccountProvider(
getMultichainAccountServiceMessenger(rootMessenger),
);

jest.spyOn(evmProvider, 'discoverAccounts');
jest.spyOn(evmProvider, 'getName').mockReturnValue('EVM');

const options = {
entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id,
groupIndex: 0,
};

const evmWrapper = new AccountProviderWrapper(
getMultichainAccountServiceMessenger(rootMessenger),
evmProvider,
mockTrace,
);

(evmProvider.discoverAccounts as jest.Mock).mockResolvedValue([
MOCK_HD_ACCOUNT_1,
]);

const result = await evmWrapper.discoverAccounts(options);

expect(result).toStrictEqual([MOCK_HD_ACCOUNT_1]);
expect(mockTrace).toHaveBeenCalledTimes(1);
expect(evmProvider.discoverAccounts).toHaveBeenCalledWith(options);
});

it('returns empty array when disabled without calling trace', async () => {
const mockTrace = jest.fn();
const options = {
entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id,
groupIndex: 0,
};

const wrapperWithTrace = new AccountProviderWrapper(
getMultichainAccountServiceMessenger(
setup({ accounts: [MOCK_HD_ACCOUNT_1] }).rootMessenger,
),
solProvider,
mockTrace,
);

// Disable the wrapper
wrapperWithTrace.setEnabled(false);

const result = await wrapperWithTrace.discoverAccounts(options);

expect(result).toStrictEqual([]);
expect(mockTrace).not.toHaveBeenCalled();
expect(solProvider.discoverAccounts).not.toHaveBeenCalled();
});
});

describe('createMultichainAccountWallet', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import type {
MultichainAccountWalletId,
Bip44Account,
} from '@metamask/account-api';
import type { TraceCallback } from '@metamask/controller-utils';
import type { HdKeyring } from '@metamask/eth-hd-keyring';
import { mnemonicPhraseToBytes } from '@metamask/key-tree';
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';
Expand All @@ -26,7 +28,10 @@ import {
} from './providers/AccountProviderWrapper';
import { EvmAccountProvider } from './providers/EvmAccountProvider';
import { SolAccountProvider } from './providers/SolAccountProvider';
import type { MultichainAccountServiceMessenger } from './types';
import type {
MultichainAccountServiceConfig,
MultichainAccountServiceMessenger,
} from './types';

export const serviceName = 'MultichainAccountService';

Expand All @@ -40,6 +45,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. */
Expand All @@ -66,6 +72,8 @@ export class MultichainAccountService {
AccountContext<Bip44Account<KeyringAccount>>
>;

readonly #trace: TraceCallback;

/**
* The name of the service.
*/
Expand All @@ -79,16 +87,18 @@ 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();
this.#trace = config?.trace ?? traceFallback;

// TODO: Rely on keyring capabilities once the keyring API is used by all keyrings.
this.#providers = [
Expand All @@ -102,6 +112,7 @@ export class MultichainAccountService {
this.#messenger,
providerConfigs?.[SolAccountProvider.NAME],
),
this.#trace,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Bug

The EvmAccountProvider is instantiated without receiving the trace callback from the MultichainAccountService configuration. This causes inconsistent tracing behavior, as EvmAccountProvider will always use the default traceFallback function, while other providers (like SolAccountProvider via AccountProviderWrapper) correctly utilize the service's configured trace callback.

Fix in Cursor Fix in Web

),
// Custom account providers that can be provided by the MetaMask client.
...providers,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './traces';
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
25 changes: 25 additions & 0 deletions packages/multichain-account-service/src/analytics/traces.ts
Original file line number Diff line number Diff line change
@@ -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 <ReturnType>(
_request: TraceRequest,
fn?: (context?: TraceContext) => ReturnType,
): Promise<ReturnType> => {
if (!fn) {
return undefined as ReturnType;
}
return await Promise.resolve(fn());
};
4 changes: 4 additions & 0 deletions packages/multichain-account-service/src/constants/traces.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export enum TraceName {
'SnapDiscoverAccounts' = 'Snap Discover Accounts',
'EvmDiscoverAccounts' = 'EVM Discover Accounts',
}
Loading
Loading