Skip to content

Commit 9b1aac1

Browse files
mathieuartujuanmigdr
authored andcommitted
feat(multichain-account-service): add per-provider throttling for non-EVM account creation (#7000)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References Related to [MUL-1222](https://consensyssoftware.atlassian.net/browse/MUL-1222), [MUL-1221](https://consensyssoftware.atlassian.net/browse/MUL-1221) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes [MUL-1222]: https://consensyssoftware.atlassian.net/browse/MUL-1222?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds per-provider concurrency limits for non-EVM account creation (Solana defaults to 3), refactors creation flow to optionally await/aggregate errors, and updates providers/tests accordingly. > > - **Core** > - Introduce `withMaxConcurrency` in `SnapAccountProvider` using a semaphore; add optional `maxConcurrency` to `SnapAccountProviderConfig`. > - Add `toRejectedErrorMessage` util and factor non‑EVM creation into `MultichainAccountWallet.#createNonEvmAccounts`, supporting await-all with aggregated errors and background mode logging. > - Forward optional `providerConfigs` safely (`?.`) and allow Solana-specific config (including `maxConcurrency`). > - **Providers** > - Update `SolAccountProvider`, `BtcAccountProvider`, and `TrxAccountProvider` to use `SnapAccountProviderConfig` and throttle `createAccounts` via `withMaxConcurrency`. > - Default Solana `maxConcurrency: 3`; others unthrottled by default. > - **Tests** > - Add concurrency tests for `SnapAccountProvider`. > - Update wallet tests to verify aggregated non‑EVM failures when awaiting all and provider config forwarding. > - **Docs** > - Update `CHANGELOG.md` with new throttling behavior and defaults. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6e21486. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [MUL-1221]: https://consensyssoftware.atlassian.net/browse/MUL-1221?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent 7887a12 commit 9b1aac1

12 files changed

+440
-162
lines changed

packages/multichain-account-service/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add per-provider throttling for non-EVM account creation to improve performance on low-end devices ([#7000](https://github.com/MetaMask/core/pull/7000))
13+
- Solana provider is now limited to 3 concurrent account creations by default when creating multichain account groups.
14+
- Other providers remain unthrottled by default.
15+
1016
## [2.0.1]
1117

1218
### Fixed

packages/multichain-account-service/src/MultichainAccountService.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ describe('MultichainAccountService', () => {
200200
},
201201
},
202202
[SOL_ACCOUNT_PROVIDER_NAME]: {
203+
maxConcurrency: 3,
203204
discovery: {
204205
timeoutMs: 5000,
205206
maxAttempts: 4,
@@ -218,11 +219,11 @@ describe('MultichainAccountService', () => {
218219

219220
expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith(
220221
messenger,
221-
providerConfigs[EvmAccountProvider.NAME],
222+
providerConfigs?.[EvmAccountProvider.NAME],
222223
);
223224
expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith(
224225
messenger,
225-
providerConfigs[SolAccountProvider.NAME],
226+
providerConfigs?.[SolAccountProvider.NAME],
226227
);
227228
});
228229

@@ -232,6 +233,7 @@ describe('MultichainAccountService', () => {
232233
// NOTE: We use constants here, since `*AccountProvider` are mocked, thus, their `.NAME` will
233234
// be `undefined`.
234235
[SOL_ACCOUNT_PROVIDER_NAME]: {
236+
maxConcurrency: 3,
235237
discovery: {
236238
timeoutMs: 5000,
237239
maxAttempts: 4,
@@ -255,7 +257,7 @@ describe('MultichainAccountService', () => {
255257
);
256258
expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith(
257259
messenger,
258-
providerConfigs[SolAccountProvider.NAME],
260+
providerConfigs?.[SolAccountProvider.NAME],
259261
);
260262
});
261263
});

packages/multichain-account-service/src/MultichainAccountService.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,16 @@ import { MultichainAccountWallet } from './MultichainAccountWallet';
1818
import type {
1919
EvmAccountProviderConfig,
2020
NamedAccountProvider,
21-
SolAccountProviderConfig,
2221
} from './providers';
2322
import {
2423
AccountProviderWrapper,
2524
isAccountProviderWrapper,
2625
} from './providers/AccountProviderWrapper';
2726
import { EvmAccountProvider } from './providers/EvmAccountProvider';
28-
import { SolAccountProvider } from './providers/SolAccountProvider';
27+
import {
28+
SolAccountProvider,
29+
type SolAccountProviderConfig,
30+
} from './providers/SolAccountProvider';
2931
import type { MultichainAccountServiceMessenger } from './types';
3032

3133
export const serviceName = 'MultichainAccountService';

packages/multichain-account-service/src/MultichainAccountWallet.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,53 @@ describe('MultichainAccountWallet', () => {
364364
'Unable to create multichain account group for index: 1',
365365
);
366366
});
367+
368+
it('aggregates non-EVM failures when waiting for all providers', async () => {
369+
const startingIndex = 0;
370+
371+
const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1)
372+
.withEntropySource(MOCK_HD_KEYRING_1.metadata.id)
373+
.withGroupIndex(startingIndex)
374+
.get();
375+
376+
const { wallet, providers } = setup({
377+
providers: [
378+
setupNamedAccountProvider({ accounts: [mockEvmAccount], index: 0 }),
379+
setupNamedAccountProvider({
380+
name: 'Non-EVM Provider',
381+
accounts: [],
382+
index: 1,
383+
}),
384+
],
385+
});
386+
387+
const nextIndex = 1;
388+
const nextEvmAccount = MockAccountBuilder.from(mockEvmAccount)
389+
.withGroupIndex(nextIndex)
390+
.get();
391+
392+
const [evmProvider, solProvider] = providers;
393+
evmProvider.createAccounts.mockResolvedValueOnce([nextEvmAccount]);
394+
evmProvider.getAccounts.mockReturnValueOnce([nextEvmAccount]);
395+
evmProvider.getAccount.mockReturnValueOnce(nextEvmAccount);
396+
397+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
398+
399+
const SOL_PROVIDER_ERROR = 'SOL create failed';
400+
solProvider.createAccounts.mockRejectedValueOnce(
401+
new Error(SOL_PROVIDER_ERROR),
402+
);
403+
404+
await expect(
405+
wallet.createMultichainAccountGroup(nextIndex, {
406+
waitForAllProvidersToFinishCreatingAccounts: true,
407+
}),
408+
).rejects.toThrow(
409+
`Unable to create multichain account group for index: ${nextIndex}:\n- Error: ${SOL_PROVIDER_ERROR}`,
410+
);
411+
412+
expect(warnSpy).toHaveBeenCalled();
413+
});
367414
});
368415

369416
describe('createNextMultichainAccountGroup', () => {

packages/multichain-account-service/src/MultichainAccountWallet.ts

Lines changed: 90 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import { MultichainAccountGroup } from './MultichainAccountGroup';
2727
import { EvmAccountProvider, type NamedAccountProvider } from './providers';
2828
import type { MultichainAccountServiceMessenger } from './types';
29+
import { toRejectedErrorMessage } from './utils';
2930

3031
/**
3132
* The context for a provider discovery.
@@ -220,6 +221,65 @@ export class MultichainAccountWallet<
220221
}
221222
}
222223

224+
/**
225+
* Create accounts with non‑EVM providers. Optional throttling is managed by each provider internally.
226+
* When awaitAll is true, waits for all providers and throws if any failed.
227+
* When false, starts work in background and logs errors without throwing.
228+
*
229+
* @param options - Method options.
230+
* @param options.groupIndex - The group index to create accounts for.
231+
* @param options.providers - The non‑EVM account providers.
232+
* @param options.awaitAll - Whether to wait for all providers to finish.
233+
* @throws If awaitAll is true and any provider fails to create accounts.
234+
* @returns A promise that resolves when done (if awaitAll is true) or immediately (if false).
235+
*/
236+
async #createNonEvmAccounts({
237+
groupIndex,
238+
providers,
239+
awaitAll,
240+
}: {
241+
groupIndex: number;
242+
providers: NamedAccountProvider<Account>[];
243+
awaitAll: boolean;
244+
}): Promise<void> {
245+
if (awaitAll) {
246+
const tasks = providers.map((provider) =>
247+
provider.createAccounts({
248+
entropySource: this.#entropySource,
249+
groupIndex,
250+
}),
251+
);
252+
253+
const results = await Promise.allSettled(tasks);
254+
if (results.some((r) => r.status === 'rejected')) {
255+
const errorMessage = toRejectedErrorMessage(
256+
`Unable to create multichain account group for index: ${groupIndex}`,
257+
results,
258+
);
259+
260+
this.#log(`${WARNING_PREFIX} ${errorMessage}`);
261+
console.warn(errorMessage);
262+
throw new Error(errorMessage);
263+
}
264+
return;
265+
}
266+
267+
// Background mode: start tasks and log errors.
268+
// Optional throttling is handled internally by each provider based on its config.
269+
providers.forEach((provider) => {
270+
// eslint-disable-next-line no-void
271+
void provider
272+
.createAccounts({
273+
entropySource: this.#entropySource,
274+
groupIndex,
275+
})
276+
.catch((error) => {
277+
const errorMessage = `Unable to create multichain account group for index: ${groupIndex} (background mode with provider "${provider.getName()}")`;
278+
this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error);
279+
});
280+
});
281+
}
282+
223283
/**
224284
* Gets multichain account for a given ID.
225285
* The default group ID will default to the multichain account with index 0.
@@ -335,70 +395,39 @@ export class MultichainAccountWallet<
335395

336396
this.#log(`Creating new group for index ${groupIndex}...`);
337397

338-
if (options?.waitForAllProvidersToFinishCreatingAccounts) {
339-
// Create account with all providers and await them.
340-
const results = await Promise.allSettled(
341-
this.#providers.map((provider) =>
342-
provider.createAccounts({
343-
entropySource: this.#entropySource,
344-
groupIndex,
345-
}),
346-
),
347-
);
398+
// Extract the EVM provider from the list of providers.
399+
// We always await EVM account creation first.
400+
const [evmProvider, ...otherProviders] = this.#providers;
401+
assert(
402+
evmProvider instanceof EvmAccountProvider,
403+
'EVM account provider must be first',
404+
);
348405

349-
// If any of the provider failed to create their accounts, then we consider the
350-
// multichain account group to have failed too.
351-
if (results.some((result) => result.status === 'rejected')) {
352-
// NOTE: Some accounts might still have been created on other account providers. We
353-
// don't rollback them.
354-
const error = `Unable to create multichain account group for index: ${groupIndex}`;
355-
356-
let message = `${error}:`;
357-
for (const result of results) {
358-
if (result.status === 'rejected') {
359-
message += `\n- ${result.reason}`;
360-
}
361-
}
362-
this.#log(`${WARNING_PREFIX} ${message}`);
363-
console.warn(message);
406+
try {
407+
await evmProvider.createAccounts({
408+
entropySource: this.#entropySource,
409+
groupIndex,
410+
});
411+
} catch (error) {
412+
const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`;
413+
this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error);
414+
throw new Error(errorMessage);
415+
}
364416

365-
throw new Error(error);
366-
}
417+
// We then create accounts with other providers (some being throttled if configured).
418+
// Depending on the options, we either await all providers or run them in background.
419+
if (options?.waitForAllProvidersToFinishCreatingAccounts) {
420+
await this.#createNonEvmAccounts({
421+
groupIndex,
422+
providers: otherProviders,
423+
awaitAll: true,
424+
});
367425
} else {
368-
// Extract the EVM provider from the list of providers.
369-
// We will only await the EVM provider to create its accounts, while
370-
// all other providers will be started in the background.
371-
const [evmProvider, ...otherProviders] = this.#providers;
372-
assert(
373-
evmProvider instanceof EvmAccountProvider,
374-
'EVM account provider must be first',
375-
);
376-
377-
// Create account with the EVM provider first and await it.
378-
// If it fails, we don't start creating accounts with other providers.
379-
try {
380-
await evmProvider.createAccounts({
381-
entropySource: this.#entropySource,
382-
groupIndex,
383-
});
384-
} catch (error) {
385-
const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`;
386-
this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error);
387-
throw new Error(errorMessage);
388-
}
389-
390-
// Create account with other providers in the background
391-
otherProviders.forEach((provider) => {
392-
provider
393-
.createAccounts({
394-
entropySource: this.#entropySource,
395-
groupIndex,
396-
})
397-
.catch((error) => {
398-
// Log errors from background providers but don't fail the operation
399-
const errorMessage = `Could not to create account with provider "${provider.getName()}" for multichain account group index: ${groupIndex}`;
400-
this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error);
401-
});
426+
// eslint-disable-next-line no-void
427+
void this.#createNonEvmAccounts({
428+
groupIndex,
429+
providers: otherProviders,
430+
awaitAll: false,
402431
});
403432
}
404433

packages/multichain-account-service/src/providers/BtcAccountProvider.ts

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,14 @@ import type { SnapId } from '@metamask/snaps-sdk';
77
import { HandlerType } from '@metamask/snaps-utils';
88
import type { Json, JsonRpcRequest } from '@metamask/utils';
99

10-
import { SnapAccountProvider } from './SnapAccountProvider';
10+
import {
11+
SnapAccountProvider,
12+
type SnapAccountProviderConfig,
13+
} from './SnapAccountProvider';
1114
import { withRetry, withTimeout } from './utils';
1215
import type { MultichainAccountServiceMessenger } from '../types';
1316

14-
export type BtcAccountProviderConfig = {
15-
discovery: {
16-
maxAttempts: number;
17-
timeoutMs: number;
18-
backOffMs: number;
19-
};
20-
createAccounts: {
21-
timeoutMs: number;
22-
};
23-
};
17+
export type BtcAccountProviderConfig = SnapAccountProviderConfig;
2418

2519
export const BTC_ACCOUNT_PROVIDER_NAME = 'Bitcoin' as const;
2620

@@ -31,8 +25,6 @@ export class BtcAccountProvider extends SnapAccountProvider {
3125

3226
readonly #client: KeyringClient;
3327

34-
readonly #config: BtcAccountProviderConfig;
35-
3628
constructor(
3729
messenger: MultichainAccountServiceMessenger,
3830
config: BtcAccountProviderConfig = {
@@ -46,11 +38,10 @@ export class BtcAccountProvider extends SnapAccountProvider {
4638
},
4739
},
4840
) {
49-
super(BtcAccountProvider.BTC_SNAP_ID, messenger);
41+
super(BtcAccountProvider.BTC_SNAP_ID, messenger, config);
5042
this.#client = this.#getKeyringClientFromSnapId(
5143
BtcAccountProvider.BTC_SNAP_ID,
5244
);
53-
this.#config = config;
5445
}
5546

5647
getName(): string {
@@ -88,20 +79,22 @@ export class BtcAccountProvider extends SnapAccountProvider {
8879
entropySource: EntropySourceId;
8980
groupIndex: number;
9081
}): Promise<Bip44Account<KeyringAccount>[]> {
91-
const createAccount = await this.getRestrictedSnapAccountCreator();
92-
93-
const account = await withTimeout(
94-
createAccount({
95-
entropySource,
96-
index,
97-
addressType: BtcAccountType.P2wpkh,
98-
scope: BtcScope.Mainnet,
99-
}),
100-
this.#config.createAccounts.timeoutMs,
101-
);
102-
103-
assertIsBip44Account(account);
104-
return [account];
82+
return this.withMaxConcurrency(async () => {
83+
const createAccount = await this.getRestrictedSnapAccountCreator();
84+
85+
const account = await withTimeout(
86+
createAccount({
87+
entropySource,
88+
index,
89+
addressType: BtcAccountType.P2wpkh,
90+
scope: BtcScope.Mainnet,
91+
}),
92+
this.config.createAccounts.timeoutMs,
93+
);
94+
95+
assertIsBip44Account(account);
96+
return [account];
97+
});
10598
}
10699

107100
async discoverAccounts({
@@ -119,11 +112,11 @@ export class BtcAccountProvider extends SnapAccountProvider {
119112
entropySource,
120113
groupIndex,
121114
),
122-
this.#config.discovery.timeoutMs,
115+
this.config.discovery.timeoutMs,
123116
),
124117
{
125-
maxAttempts: this.#config.discovery.maxAttempts,
126-
backOffMs: this.#config.discovery.backOffMs,
118+
maxAttempts: this.config.discovery.maxAttempts,
119+
backOffMs: this.config.discovery.backOffMs,
127120
},
128121
);
129122

0 commit comments

Comments
 (0)