From 8081592b66606a8ec383c4cb9a69230b5d79d257 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 13 Aug 2026 11:12:06 +0200 Subject: [PATCH 1/9] perf(tron-wallet-snap): reduce createAccounts extension RPC round trips --- packages/tron-wallet-snap/CHANGELOG.md | 3 + packages/tron-wallet-snap/snap.manifest.json | 2 +- .../accounts/AccountsRepository.test.ts | 28 +++++++++ .../services/accounts/AccountsRepository.ts | 25 +++++++- .../services/accounts/AccountsService.test.ts | 57 +++++++++++++++-- .../src/services/accounts/AccountsService.ts | 62 ++++++++++++++----- 6 files changed, 151 insertions(+), 26 deletions(-) diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index a091cac27..f78aba543 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Reduce extension RPC round trips in `keyring_createAccounts` from 5 to at most 4 ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) + - `mergeKeyringAccounts` now returns the merge result instead of requiring a post-merge state re-read, and the existing-accounts read runs in parallel with the BIP-32 entropy fetch. + - `snap_getBip32Entropy` is now called even when all requested indices already exist (this path only occurs on idempotent retries); no new permissions are required. - Extract shared asset util functions and inject `SnapAssetsAdapter` from `context` into `AssetsService` ([#143](https://github.com/MetaMask/internal-snaps/pull/143)) - Rename `getByKeyringAccountId` to `getAccountAssets` (with essential-asset synthesis) and update keyring callers ([#143](https://github.com/MetaMask/internal-snaps/pull/143)) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 0f47ebb3f..b4f24d6cd 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "NpxOo6DkB0sBh8xpisBu3o+7G6MJriDNsADcHVb/Qp8=", + "shasum": "b0zTF4I78txypck9p+FhyttFm/3cDVM0jqOpFQl/u9E=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.test.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.test.ts index a972dda70..b53cc0b0e 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.test.ts @@ -144,6 +144,34 @@ describe('AccountsRepository', () => { ]); }); + it('returns the merged state and added accounts from mergeKeyringAccounts', async () => { + const existing = createTestAccount({ id: 'existing-0' }); + const repository = new AccountsRepository( + createEmptyState({ [existing.id]: existing }), + ); + const newIndexAccount = createTestAccount({ + id: 'new-index', + index: 1, + derivationPath: "m/44'/195'/0'/0/1", + address: 'TAddress1', + }); + + const result = await repository.mergeKeyringAccounts({ + 'duplicate-index': { + ...existing, + id: 'duplicate-index', + }, + [newIndexAccount.id]: newIndexAccount, + }); + + // The conflict loser is omitted from `added`; the winner is in `merged`. + expect(Object.keys(result.added)).toStrictEqual(['new-index']); + expect(result.merged).toStrictEqual({ + 'existing-0': existing, + 'new-index': newIndexAccount, + }); + }); + it('skips duplicate indices within the same merge batch', async () => { const base = createTestAccount({ id: 'first' }); const repository = new AccountsRepository(createEmptyState()); diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.ts index 0436768d5..c50e7b86e 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsRepository.ts @@ -17,6 +17,18 @@ type AccountCreationRange = { type KeyringAccountsState = Record; +/** + * Result of merging accounts into `keyringAccounts`. + * + * @param merged - The full post-merge keyring accounts state. + * @param added - The subset of incoming accounts that was actually persisted; + * conflict losers are omitted (their winners are present in `merged`). + */ +export type KeyringAccountsMergeResult = { + merged: Record; + added: Record; +}; + /** * Tron accounts use a fixed BIP-44 path template; uniqueness is entropy + index. * @@ -165,17 +177,24 @@ export class AccountsRepository { * Merges multiple keyring accounts into `keyringAccounts` in a single atomic state update. * * @param newAccounts - The new accounts to merge. + * @returns The post-merge state and the subset of accounts actually added, + * so callers can resolve persisted accounts (including conflict winners) + * without re-reading state. */ async mergeKeyringAccounts( newAccounts: Record, - ): Promise { + ): Promise { + let result: KeyringAccountsMergeResult = { merged: {}, added: {} }; + await this.#state.setKeyWith( this.#storageKey, (current) => { - const existing = current ?? {}; - return mergeAccountsWithoutIndexConflicts(existing, newAccounts).merged; + result = mergeAccountsWithoutIndexConflicts(current ?? {}, newAccounts); + return result.merged; }, ); + + return result; } async delete(id: string): Promise { diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts index b75f006e0..822835744 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts @@ -213,15 +213,24 @@ async function withAccountsService( .mockImplementation( async (newAccounts: Record) => { const occupied = new Set(keyringAccounts.map(getAccountIndexKey)); + const added: Record = {}; - for (const account of Object.values(newAccounts)) { + for (const [id, account] of Object.entries(newAccounts)) { const indexKey = getAccountIndexKey(account); if (!occupied.has(indexKey)) { keyringAccounts.push(account); occupied.add(indexKey); + added[id] = account; } } + + return { + merged: Object.fromEntries( + keyringAccounts.map((account) => [account.id, account]), + ), + added, + }; }, ), delete: jest.fn().mockImplementation(async (id: string) => { @@ -419,9 +428,10 @@ describe('AccountsService', () => { expect( mockAccountsRepository.findByEntropySourceAndRange, ).toHaveBeenCalledWith('test-entropy', { from: 0, to: 1 }); + // No post-merge re-read: the merge result is used instead. expect( mockAccountsRepository.findByEntropySourceAndRange, - ).toHaveBeenCalledTimes(2); + ).toHaveBeenCalledTimes(1); expect(mockAccountsRepository.getAll).not.toHaveBeenCalled(); expect( @@ -489,9 +499,15 @@ describe('AccountsService', () => { await withAccountsService( async ({ accountsService, mockAccountsRepository }) => { - mockAccountsRepository.findByEntropySourceAndRange - .mockResolvedValueOnce([]) - .mockResolvedValueOnce([concurrentAccount]); + // The first read sees nothing; a concurrent writer wins the merge, + // so the winner only appears in the merge result. + mockAccountsRepository.findByEntropySourceAndRange.mockResolvedValue( + [], + ); + mockAccountsRepository.mergeKeyringAccounts.mockResolvedValue({ + merged: { [concurrentAccount.id]: concurrentAccount }, + added: {}, + }); const result = await accountsService.createAccounts({ type: AccountCreationType.Bip44DeriveIndex, @@ -501,6 +517,9 @@ describe('AccountsService', () => { expect(result).toHaveLength(1); expect(result[0]?.id).toBe('concurrent-0'); + expect( + mockAccountsRepository.findByEntropySourceAndRange, + ).toHaveBeenCalledTimes(1); }, coinJson, ); @@ -550,12 +569,38 @@ describe('AccountsService', () => { expect( mockAccountsRepository.mergeKeyringAccounts, ).not.toHaveBeenCalled(); - expect(mockSnapClient.getBip32Entropy).not.toHaveBeenCalled(); + // The coin-type entropy fetch runs in parallel with the state read, + // so it happens (speculatively) even when the range already exists. + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledTimes(1); + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledWith({ + entropySource: 'test-entropy', + path: ['m', "44'", "195'"], + curve: 'secp256k1', + }); }, coinJson, ); }); + it('logs phase timings for a batch creation', async () => { + const coinJson = await getTronTestCoinTypeJson(); + + await withAccountsService(async ({ accountsService }) => { + await accountsService.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: 'test-entropy', + range: { from: 0, to: 1 }, + }); + + expect(mockLogger.log).toHaveBeenCalledWith( + '[🔑 AccountsService]', + expect.stringMatching( + /^\[createAccounts\] Phase timings \{.*"created":2.*"readAndEntropyMs":\d+.*"deriveMs":\d+.*"mergeMs":\d+.*"totalMs":\d+.*\}$/u, + ), + ); + }, coinJson); + }); + it('throws before storage or entropy access when the range is invalid', async () => { await withAccountsService( async ({ accountsService, mockAccountsRepository, mockSnapClient }) => { diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts index 3f2674fe8..266c6b5cd 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts @@ -384,12 +384,20 @@ export class AccountsService { } validateAccountCreationRange(range); - // Get existing accounts for the same entropy source/range to avoid duplicate state writes. - const existingAccounts = - await this.#accountsRepository.findByEntropySourceAndRange( + const startMs = Date.now(); + + // The existing-accounts read and the coin-type entropy fetch are + // independent RPCs, so overlap them. This makes the entropy fetch + // speculative when every requested index already exists, but that only + // happens on idempotent retries. + const [existingAccounts, tronAddressDeriver] = await Promise.all([ + this.#accountsRepository.findByEntropySourceAndRange( entropySource, range, - ); + ), + this.#createTronAddressDeriver(entropySource), + ]); + const readAndEntropyMs = Date.now() - startMs; const allAccounts = new Map(); for (const account of existingAccounts) { @@ -404,10 +412,11 @@ export class AccountsService { } const newAccounts: Record = {}; + let deriveMs = 0; + let mergeMs = 0; if (missingIndices.length > 0) { - const tronAddressDeriver = - await this.#createTronAddressDeriver(entropySource); + const deriveStartMs = Date.now(); for (const groupIndex of missingIndices) { const id = globalThis.crypto.randomUUID(); @@ -439,19 +448,40 @@ export class AccountsService { newAccounts[id] = tronKeyringAccount; } - await this.#accountsRepository.mergeKeyringAccounts(newAccounts); - - const persistedAccounts = - await this.#accountsRepository.findByEntropySourceAndRange( - entropySource, - range, - ); - - for (const account of persistedAccounts) { - allAccounts.set(account.index, account); + deriveMs = Date.now() - deriveStartMs; + + const mergeStartMs = Date.now(); + const { merged } = + await this.#accountsRepository.mergeKeyringAccounts(newAccounts); + mergeMs = Date.now() - mergeStartMs; + + // Resolve the persisted account for each requested index from the merge + // result: for indices lost to a concurrent writer, `merged` holds the + // winner's account rather than the one derived above. + for (const account of Object.values(merged)) { + if ( + account.entropySource === entropySource && + account.index >= range.from && + account.index <= range.to + ) { + allAccounts.set(account.index, account); + } } } + // Stringified so the values survive in the console after the snap's + // execution environment is torn down (live objects become unexpandable). + this.#logger.log( + `[createAccounts] Phase timings ${JSON.stringify({ + range, + created: missingIndices.length, + readAndEntropyMs, + deriveMs, + mergeMs, + totalMs: Date.now() - startMs, + })}`, + ); + const result: KeyringAccount[] = []; for (let groupIndex = range.from; groupIndex <= range.to; groupIndex += 1) { const account = allAccounts.get(groupIndex); From 8730c7071d77f8a975c3fb0909a9e8bbe225a20d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 13 Aug 2026 11:17:47 +0200 Subject: [PATCH 2/9] perf(tron-wallet-snap): fetch entropy once during BIP-44 account discovery --- packages/tron-wallet-snap/CHANGELOG.md | 1 + packages/tron-wallet-snap/snap.manifest.json | 2 +- .../services/accounts/AccountsService.test.ts | 64 +++++++++++++++++++ .../src/services/accounts/AccountsService.ts | 16 ++--- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index f78aba543..dea1b3e25 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Reduce BIP-44 account discovery to a single entropy fetch by reusing the coin-type deriver for the on-chain activity check ([#150](https://github.com/MetaMask/internal-snaps/pull/150)) - Reduce extension RPC round trips in `keyring_createAccounts` from 5 to at most 4 ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) - `mergeKeyringAccounts` now returns the merge result instead of requiring a post-merge state re-read, and the existing-accounts read runs in parallel with the BIP-32 entropy fetch. - `snap_getBip32Entropy` is now called even when all requested indices already exist (this path only occurs on idempotent retries); no new permissions are required. diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index b4f24d6cd..afc1b2982 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "b0zTF4I78txypck9p+FhyttFm/3cDVM0jqOpFQl/u9E=", + "shasum": "+gro5SDzUm1zy/pTkNUPXyBQeFVQTv4jJiiXs9quzrg=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts index 822835744..91078f0d8 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts @@ -702,6 +702,70 @@ describe('AccountsService', () => { coinJson, ); }); + + it('fetches entropy once for bip44:discover, reusing the coin-type deriver for the activity check', async () => { + const coinJson = await getTronTestCoinTypeJson(); + + await withAccountsService( + async ({ + accountsService, + mockSnapClient, + mockTransactionsService, + }) => { + mockTransactionsService.checkAddressActivity.mockResolvedValueOnce( + true, + ); + + const result = await accountsService.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: 'test-entropy', + groupIndex: 2, + }); + + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledTimes(1); + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledWith({ + entropySource: 'test-entropy', + path: ['m', "44'", "195'"], + curve: 'secp256k1', + }); + + // The address probed for activity is the one persisted. + const checkedAddress = + mockTransactionsService.checkAddressActivity.mock.calls[0]?.[1]; + expect(result[0]?.address).toBe(checkedAddress); + }, + coinJson, + ); + }); + + it('fetches entropy once for bip44:discover even when no activity is found', async () => { + const coinJson = await getTronTestCoinTypeJson(); + + await withAccountsService( + async ({ + accountsService, + mockSnapClient, + mockTransactionsService, + }) => { + mockTransactionsService.checkAddressActivity.mockResolvedValue(false); + + const result = await accountsService.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: 'test-entropy', + groupIndex: 0, + }); + + expect(result).toStrictEqual([]); + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledTimes(1); + expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledWith({ + entropySource: 'test-entropy', + path: ['m', "44'", "195'"], + curve: 'secp256k1', + }); + }, + coinJson, + ); + }); }); describe('create', () => { diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts index 266c6b5cd..a192a2306 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts @@ -354,18 +354,16 @@ export class AccountsService { // For discovery, only proceed if the account at groupIndex has on-chain // activity. No activity signals end-of-discovery; return [] to the client. + // The deriver created here doubles as the entropy fetch for the derivation + // below, so discovery costs a single `snap_getBip32Entropy` call. + let discoverDeriver: TronAddressDeriver | undefined; if (options.type === AccountCreationType.Bip44Discover) { const { groupIndex } = options; - const derivedAccount = await this.deriveAccount({ - entropySource, - index: groupIndex, - }); + discoverDeriver = await this.#createTronAddressDeriver(entropySource); + const { address } = await discoverDeriver(groupIndex); const activityChecks = await Promise.all( SUPPORTED_SCOPES.map((scope) => - this.#transactionsService.checkAddressActivity( - scope, - derivedAccount.address, - ), + this.#transactionsService.checkAddressActivity(scope, address), ), ); if (!activityChecks.some(Boolean)) { @@ -395,7 +393,7 @@ export class AccountsService { entropySource, range, ), - this.#createTronAddressDeriver(entropySource), + discoverDeriver ?? this.#createTronAddressDeriver(entropySource), ]); const readAndEntropyMs = Date.now() - startMs; From f729c3800ed88ee8436b520eb5afc824c2fa62ac Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 13 Aug 2026 13:14:25 +0200 Subject: [PATCH 3/9] fix(tron-wallet-snap): coalesce concurrent account synchronization runs --- packages/snap-networks-utils/CHANGELOG.md | 4 + packages/snap-networks-utils/package.json | 10 ++ .../src/dedupe/InFlightCoalescer.test.ts | 77 +++++++++++++++ .../src/dedupe/InFlightCoalescer.ts | 24 +++++ .../snap-networks-utils/src/dedupe/index.ts | 1 + packages/tron-wallet-snap/CHANGELOG.md | 6 +- packages/tron-wallet-snap/snap.manifest.json | 2 +- .../services/accounts/AccountsService.test.ts | 97 +++++++++++++++++++ .../src/services/accounts/AccountsService.ts | 22 ++++- 9 files changed, 237 insertions(+), 6 deletions(-) create mode 100644 packages/snap-networks-utils/src/dedupe/InFlightCoalescer.test.ts create mode 100644 packages/snap-networks-utils/src/dedupe/InFlightCoalescer.ts create mode 100644 packages/snap-networks-utils/src/dedupe/index.ts diff --git a/packages/snap-networks-utils/CHANGELOG.md b/packages/snap-networks-utils/CHANGELOG.md index 8c3f233d3..feb84d2a5 100644 --- a/packages/snap-networks-utils/CHANGELOG.md +++ b/packages/snap-networks-utils/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `InFlightCoalescer`, exported from a new `./dedupe` entry point, which coalesces concurrent async operations by key so callers share one in-flight run ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) + ### Changed - **BREAKING** Replace the logger utilities with a configurable `Logger` class that requires a log level and supports level filtering, per-instance prefixes, and method decorators. diff --git a/packages/snap-networks-utils/package.json b/packages/snap-networks-utils/package.json index d713604dd..edbebc452 100644 --- a/packages/snap-networks-utils/package.json +++ b/packages/snap-networks-utils/package.json @@ -32,6 +32,16 @@ "default": "./dist/index.cjs" } }, + "./dedupe": { + "import": { + "types": "./dist/dedupe/index.d.mts", + "default": "./dist/dedupe/index.mjs" + }, + "require": { + "types": "./dist/dedupe/index.d.cts", + "default": "./dist/dedupe/index.cjs" + } + }, "./logger": { "import": { "types": "./dist/logger/index.d.mts", diff --git a/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.test.ts b/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.test.ts new file mode 100644 index 000000000..722fb1acb --- /dev/null +++ b/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.test.ts @@ -0,0 +1,77 @@ +import { InFlightCoalescer } from './InFlightCoalescer'; + +describe('InFlightCoalescer', () => { + it('returns the result of the wrapped function', async () => { + const coalescer = new InFlightCoalescer(); + + const result = await coalescer.run('key', async () => 'value'); + + expect(result).toBe('value'); + }); + + it('shares one in-flight run between concurrent callers with the same key', async () => { + const coalescer = new InFlightCoalescer(); + let resolveRun: (value: string) => void = () => undefined; + const fn = jest.fn( + async () => + new Promise((resolve) => { + resolveRun = resolve; + }), + ); + + const first = coalescer.run('key', fn); + const second = coalescer.run('key', fn); + resolveRun('shared'); + + expect(await first).toBe('shared'); + expect(await second).toBe('shared'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('runs again once the previous run for the key has settled', async () => { + const coalescer = new InFlightCoalescer(); + const fn = jest.fn(async () => 'value'); + + await coalescer.run('key', fn); + await coalescer.run('key', fn); + + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('runs concurrent callers with different keys independently', async () => { + const coalescer = new InFlightCoalescer(); + const fnA = jest.fn(async () => 'a'); + const fnB = jest.fn(async () => 'b'); + + const [resultA, resultB] = await Promise.all([ + coalescer.run('a', fnA), + coalescer.run('b', fnB), + ]); + + expect(resultA).toBe('a'); + expect(resultB).toBe('b'); + expect(fnA).toHaveBeenCalledTimes(1); + expect(fnB).toHaveBeenCalledTimes(1); + }); + + it('propagates rejections to coalesced callers and clears the entry', async () => { + const coalescer = new InFlightCoalescer(); + let rejectRun: (error: Error) => void = () => undefined; + const failing = jest.fn( + async () => + new Promise((_resolve, reject) => { + rejectRun = reject; + }), + ); + + const first = coalescer.run('key', failing); + const second = coalescer.run('key', failing); + rejectRun(new Error('boom')); + + await expect(first).rejects.toThrow('boom'); + await expect(second).rejects.toThrow('boom'); + expect(failing).toHaveBeenCalledTimes(1); + + expect(await coalescer.run('key', async () => 'ok')).toBe('ok'); + }); +}); diff --git a/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.ts b/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.ts new file mode 100644 index 000000000..231dcbbab --- /dev/null +++ b/packages/snap-networks-utils/src/dedupe/InFlightCoalescer.ts @@ -0,0 +1,24 @@ +/** + * Coalesces concurrent async operations by key: while a call for a key is in + * flight, subsequent calls with the same key await the same promise instead of + * starting duplicate work. Once a run settles, the next call starts a fresh one. + * + * Note that coalesced callers share the run's outcome, including rejections. + */ +export class InFlightCoalescer { + readonly #inFlight = new Map>(); + + async run(key: string, fn: () => Promise): Promise { + const pending = this.#inFlight.get(key); + if (pending) { + return pending as Promise; + } + + const task = fn().finally(() => { + this.#inFlight.delete(key); + }); + this.#inFlight.set(key, task); + + return task; + } +} diff --git a/packages/snap-networks-utils/src/dedupe/index.ts b/packages/snap-networks-utils/src/dedupe/index.ts new file mode 100644 index 000000000..600b16218 --- /dev/null +++ b/packages/snap-networks-utils/src/dedupe/index.ts @@ -0,0 +1 @@ +export { InFlightCoalescer } from './InFlightCoalescer'; diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index dea1b3e25..f9e9e6056 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -9,13 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Reduce BIP-44 account discovery to a single entropy fetch by reusing the coin-type deriver for the on-chain activity check ([#150](https://github.com/MetaMask/internal-snaps/pull/150)) +- Reduce BIP-44 account discovery to a single entropy fetch by reusing the coin-type deriver for the on-chain activity check ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) - Reduce extension RPC round trips in `keyring_createAccounts` from 5 to at most 4 ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) - `mergeKeyringAccounts` now returns the merge result instead of requiring a post-merge state re-read, and the existing-accounts read runs in parallel with the BIP-32 entropy fetch. - `snap_getBip32Entropy` is now called even when all requested indices already exist (this path only occurs on idempotent retries); no new permissions are required. - Extract shared asset util functions and inject `SnapAssetsAdapter` from `context` into `AssetsService` ([#143](https://github.com/MetaMask/internal-snaps/pull/143)) - Rename `getByKeyringAccountId` to `getAccountAssets` (with essential-asset synthesis) and update keyring callers ([#143](https://github.com/MetaMask/internal-snaps/pull/143)) +### Fixed + +- Coalesce concurrent account synchronization runs for the same accounts so stacked triggers (cronjob and background events) share one run instead of duplicating network fetches, state writes, and keyring events ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) + ## [3.1.0] ### Added diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index afc1b2982..1c1d450ea 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "+gro5SDzUm1zy/pTkNUPXyBQeFVQTv4jJiiXs9quzrg=", + "shasum": "EAiF9pFDki9e+I9CynRfMC3MM1Q55KARtLzMy7QhpHE=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts index 91078f0d8..efda8d848 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts @@ -1480,5 +1480,102 @@ describe('AccountsService', () => { }, ); }); + + const makeSyncAccount = ( + id: string, + index: number, + ): TronKeyringAccount => ({ + id, + address: `TCoalesce${index}2345678901234567890`, + type: TrxAccountType.Eoa, + options: {}, + methods: [], + scopes: [], + entropySource: 'e1', + derivationPath: `m/44'/195'/0'/0/${index}`, + index, + }); + + it('coalesces concurrent synchronize calls for the same accounts into one run', async () => { + const account = makeSyncAccount('coalesce-id', 0); + + await withAccountsService( + async ({ + accountsService, + mockConfigProvider, + mockAssetsService, + mockTransactionsService, + }) => { + mockConfigProvider.get.mockReturnValue({ + ...MOCK_CONFIG, + activeNetworks: [Network.Mainnet], + }); + + await Promise.all([ + accountsService.synchronize([account]), + accountsService.synchronize([account]), + accountsService.synchronize([account]), + ]); + + expect( + mockAssetsService.fetchAssetsAndBalancesForAccount, + ).toHaveBeenCalledTimes(1); + expect( + mockTransactionsService.fetchNewTransactionsForAccount, + ).toHaveBeenCalledTimes(1); + expect(mockAssetsService.saveMany).toHaveBeenCalledTimes(1); + expect(mockTransactionsService.saveMany).toHaveBeenCalledTimes(1); + }, + ); + }); + + it('runs synchronize again once the previous run has finished', async () => { + const account = makeSyncAccount('sequential-id', 0); + + await withAccountsService( + async ({ accountsService, mockConfigProvider, mockAssetsService }) => { + mockConfigProvider.get.mockReturnValue({ + ...MOCK_CONFIG, + activeNetworks: [Network.Mainnet], + }); + + await accountsService.synchronize([account]); + await accountsService.synchronize([account]); + + expect( + mockAssetsService.fetchAssetsAndBalancesForAccount, + ).toHaveBeenCalledTimes(2); + }, + ); + }); + + it('does not coalesce concurrent synchronize calls for different accounts', async () => { + const accountA = makeSyncAccount('different-a', 0); + const accountB = makeSyncAccount('different-b', 1); + + await withAccountsService( + async ({ accountsService, mockConfigProvider, mockAssetsService }) => { + mockConfigProvider.get.mockReturnValue({ + ...MOCK_CONFIG, + activeNetworks: [Network.Mainnet], + }); + + await Promise.all([ + accountsService.synchronize([accountA]), + accountsService.synchronize([accountB]), + ]); + + expect( + mockAssetsService.fetchAssetsAndBalancesForAccount, + ).toHaveBeenCalledTimes(2); + expect( + mockAssetsService.fetchAssetsAndBalancesForAccount, + ).toHaveBeenCalledWith(Network.Mainnet, accountA); + expect( + mockAssetsService.fetchAssetsAndBalancesForAccount, + ).toHaveBeenCalledWith(Network.Mainnet, accountB); + }, + ); + }); }); }); diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts index a192a2306..2e0fd2ae2 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts @@ -14,6 +14,7 @@ import { emitSnapKeyringEvent, getSelectedAccounts, } from '@metamask/keyring-snap-sdk'; +import { InFlightCoalescer } from '@metamask/snap-networks-utils/dedupe'; import type { Json } from '@metamask/snaps-sdk'; import { assert } from '@metamask/superstruct'; import { hexToBytes } from '@metamask/utils'; @@ -114,6 +115,8 @@ export class AccountsService { readonly #snapClient: SnapClient; + readonly #syncCoalescer = new InFlightCoalescer(); + constructor({ accountsRepository, configProvider, @@ -597,10 +600,21 @@ export class AccountsService { } async synchronize(accounts: TronKeyringAccount[]): Promise { - await Promise.allSettled([ - this.synchronizeAssets(accounts), - this.synchronizeTransactions(accounts), - ]); + // Sync triggers stack up (60s cronjob, a background event scheduled by + // every `setSelectedAccounts` call, post-transaction refreshes), so + // concurrent invocations for the same accounts share one run instead of + // duplicating network fetches, state writes, and keyring events. + const key = accounts + .map(({ id }) => id) + .sort() + .join(','); + + await this.#syncCoalescer.run(key, async () => { + await Promise.allSettled([ + this.synchronizeAssets(accounts), + this.synchronizeTransactions(accounts), + ]); + }); } async #createTronAddressDeriver( From 345193fe7d5aef27c05adcc781d2f818f3bfa696 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 13 Aug 2026 13:54:26 +0200 Subject: [PATCH 4/9] chore(tron-wallet-snap): update shasum --- packages/tron-wallet-snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 1c1d450ea..43d94e6e8 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "EAiF9pFDki9e+I9CynRfMC3MM1Q55KARtLzMy7QhpHE=", + "shasum": "UqNjKwhzbfBmIKjP1Hlx/jxUnEJz61HxvTP0FJBThQ4=", "location": { "npm": { "filePath": "dist/bundle.js", From 4f00b99767c8fdb4ec2c398502805c5a54970bb1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 14 Aug 2026 12:54:12 +0200 Subject: [PATCH 5/9] refactor(tron-wallet-snap): remove unreachable v1 account-creation path --- packages/tron-wallet-snap/snap.manifest.json | 2 +- .../src/handlers/keyring/keyring.test.ts | 2 - .../services/accounts/AccountsService.test.ts | 366 +----------------- .../src/services/accounts/AccountsService.ts | 163 +------- .../src/services/accounts/types.ts | 8 - .../src/utils/getLowestUnusedIndex.ts | 44 --- 6 files changed, 4 insertions(+), 581 deletions(-) delete mode 100644 packages/tron-wallet-snap/src/services/accounts/types.ts delete mode 100644 packages/tron-wallet-snap/src/utils/getLowestUnusedIndex.ts diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 43d94e6e8..127de4db9 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "UqNjKwhzbfBmIKjP1Hlx/jxUnEJz61HxvTP0FJBThQ4=", + "shasum": "CkbJj+1wNlwEe63xVizdwEQ2FAEwaho+WoX/gsmP9V0=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts b/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts index de310fec6..ac90d8db0 100644 --- a/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts +++ b/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts @@ -73,8 +73,6 @@ describe('KeyringHandler', () => { mockAccountsService = { findById: jest.fn().mockResolvedValue(mockAccount), findByIdOrThrow: jest.fn().mockResolvedValue(mockAccount), - deriveAccount: jest.fn(), - create: jest.fn(), createAccounts: jest.fn(), getAll: jest.fn().mockResolvedValue([mockAccount]), deriveTronKeypair: jest.fn().mockResolvedValue({ diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts index efda8d848..8282f6c29 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts @@ -8,15 +8,8 @@ import type { CreateAccountOptions as KeyringBatchCreateAccountOptions, Transaction, } from '@metamask/keyring-api'; -import { - AccountCreationType, - KeyringEvent, - TrxAccountType, -} from '@metamask/keyring-api'; -import { - emitSnapKeyringEvent, - getSelectedAccounts, -} from '@metamask/keyring-snap-sdk'; +import { AccountCreationType, TrxAccountType } from '@metamask/keyring-api'; +import { getSelectedAccounts } from '@metamask/keyring-snap-sdk'; import type { SnapClient } from '../../clients/snap/SnapClient'; import { Network } from '../../constants'; @@ -32,13 +25,9 @@ import type { AccountsRepository } from './AccountsRepository'; import { AccountsService, SUPPORTED_SCOPES } from './AccountsService'; jest.mock('@metamask/keyring-snap-sdk', () => ({ - emitSnapKeyringEvent: jest.fn(), getSelectedAccounts: jest.fn().mockResolvedValue([]), })); -const mockedEmitSnapKeyringEvent = emitSnapKeyringEvent as jest.MockedFunction< - typeof emitSnapKeyringEvent ->; const mockedGetSelectedAccounts = getSelectedAccounts as jest.MockedFunction< typeof getSelectedAccounts >; @@ -328,58 +317,6 @@ describe('AccountsService', () => { }); }); - describe('deriveAccount', () => { - it('returns TronKeyringAccount with correct structure for index 0', async () => { - await withAccountsService(async ({ accountsService, mockSnapClient }) => { - const result = await accountsService.deriveAccount({ - entropySource: 'test-entropy', - index: 0, - }); - - expect(result).toMatchObject({ - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - scopes: SUPPORTED_SCOPES, - methods: ['signMessage', 'signTransaction'], - }); - expect(result.id).toBeDefined(); - expect(typeof result.id).toBe('string'); - expect(result.address).toBeDefined(); - expect(result.address.length).toBeGreaterThan(0); - expect(result.options.entropy).toMatchObject({ - type: 'mnemonic', - id: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - groupIndex: 0, - }); - - expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledWith({ - entropySource: 'test-entropy', - path: ['m', "44'", "195'", "0'", '0', '0'], - curve: 'secp256k1', - }); - }); - }); - - it('returns correct derivation path for index 5', async () => { - await withAccountsService(async ({ accountsService, mockSnapClient }) => { - const result = await accountsService.deriveAccount({ - entropySource: 'test-entropy', - index: 5, - }); - - expect(result.derivationPath).toBe("m/44'/195'/0'/0/5"); - expect(mockSnapClient.getBip32Entropy).toHaveBeenCalledWith( - expect.objectContaining({ - path: ['m', "44'", "195'", "0'", '0', '5'], - }), - ); - }); - }); - }); - describe('deriveTronKeypair', () => { it('throws when getBip32Entropy returns missing key material', async () => { await withAccountsService(async ({ accountsService, mockSnapClient }) => { @@ -768,305 +705,6 @@ describe('AccountsService', () => { }); }); - describe('create', () => { - it('creates and persists a new account', async () => { - mockedEmitSnapKeyringEvent.mockResolvedValue(); - - await withAccountsService( - async ({ accountsService, mockAccountsRepository }) => { - jest.spyOn(accountsService, 'deriveAccount').mockResolvedValue({ - id: 'test-uuid-123', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TTestAddress1234567890123456789', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: { - entropy: { - type: 'mnemonic', - id: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - groupIndex: 0, - }, - exportable: true, - }, - methods: ['signMessage', 'signTransaction'], - }); - - const result = await accountsService.create({ - entropySource: 'test-entropy', - index: 0, - }); - - expect(result.id).toBe('test-uuid-123'); - expect(mockAccountsRepository.create).toHaveBeenCalledWith( - expect.objectContaining({ id: 'test-uuid-123' }), - ); - expect(mockedEmitSnapKeyringEvent).toHaveBeenCalledWith( - expect.anything(), - KeyringEvent.AccountCreated, - expect.objectContaining({ - account: expect.objectContaining({ id: 'test-uuid-123' }), - }), - ); - }, - ); - }); - - it('uses default entropy source and lowest unused index when options are omitted', async () => { - mockedEmitSnapKeyringEvent.mockResolvedValue(); - - const existingAccount: TronKeyringAccount = { - id: 'existing-default-0', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TExistingDefault0', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: {}, - methods: ['signMessage', 'signTransaction'], - }; - - await withAccountsService( - async ({ accountsService, mockAccountsRepository, mockSnapClient }) => { - mockAccountsRepository.getAll.mockResolvedValue([existingAccount]); - const deriveAccount = jest - .spyOn(accountsService, 'deriveAccount') - .mockResolvedValue({ - id: 'default-create-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/1", - index: 1, - type: TrxAccountType.Eoa, - address: 'TDefaultCreate1', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: { - entropy: { - type: 'mnemonic', - id: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/1", - groupIndex: 1, - }, - exportable: true, - }, - methods: ['signMessage', 'signTransaction'], - }); - - const result = await accountsService.create(); - - expect(result.id).toBe('default-create-id'); - expect(mockSnapClient.listEntropySources).toHaveBeenCalledTimes(1); - expect(deriveAccount).toHaveBeenCalledWith({ - entropySource: 'test-entropy', - index: 1, - }); - expect(mockAccountsRepository.create).toHaveBeenCalledWith( - expect.objectContaining({ id: 'default-create-id', index: 1 }), - ); - }, - ); - }); - - it('returns existing account when same derivation path exists', async () => { - const existingAccount: TronKeyringAccount = { - id: 'existing-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TExisting123456789012345678901', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: {}, - methods: ['signMessage', 'signTransaction'], - }; - - await withAccountsService( - async ({ accountsService, mockAccountsRepository }) => { - mockAccountsRepository.getAll.mockResolvedValue([existingAccount]); - - const result = await accountsService.create({ - entropySource: 'test-entropy', - index: 0, - }); - - expect(result.id).toBe('existing-id'); - expect(mockAccountsRepository.create).not.toHaveBeenCalled(); - expect(mockLogger.warn).toHaveBeenCalled(); - }, - ); - }); - - it('rolls back persisted account when event emission fails', async () => { - mockedEmitSnapKeyringEvent.mockRejectedValue( - new Error('Event emission failed'), - ); - - await withAccountsService( - async ({ accountsService, mockAccountsRepository }) => { - jest.spyOn(accountsService, 'deriveAccount').mockResolvedValue({ - id: 'rollback-test-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TRollback12345678901234567890', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: { - entropy: { - type: 'mnemonic', - id: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - groupIndex: 0, - }, - exportable: true, - }, - methods: ['signMessage', 'signTransaction'], - }); - - await expect( - accountsService.create({ - entropySource: 'test-entropy', - index: 0, - }), - ).rejects.toThrow('Event emission failed'); - - expect(mockAccountsRepository.create).toHaveBeenCalled(); - expect(mockAccountsRepository.delete).toHaveBeenCalledWith( - 'rollback-test-id', - ); - }, - ); - }); - - it('preserves the original error when rollback delete also fails', async () => { - mockedEmitSnapKeyringEvent.mockRejectedValue( - new Error('Event emission failed'), - ); - - await withAccountsService( - async ({ accountsService, mockAccountsRepository }) => { - mockAccountsRepository.delete.mockRejectedValue( - new Error('Delete failed'), - ); - jest.spyOn(accountsService, 'deriveAccount').mockResolvedValue({ - id: 'rollback-fail-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TRollback12345678901234567890', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: { - entropy: { - type: 'mnemonic', - id: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - groupIndex: 0, - }, - exportable: true, - }, - methods: ['signMessage', 'signTransaction'], - }); - - await expect( - accountsService.create({ - entropySource: 'test-entropy', - index: 0, - }), - ).rejects.toThrow('Event emission failed'); - - expect(mockAccountsRepository.delete).toHaveBeenCalledWith( - 'rollback-fail-id', - ); - expect(mockLogger.error).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ accountId: 'rollback-fail-id' }), - 'Failed to rollback account creation', - ); - }, - ); - }); - - it('passes metamask options through to emit', async () => { - mockedEmitSnapKeyringEvent.mockResolvedValue(); - - await withAccountsService(async ({ accountsService }) => { - jest.spyOn(accountsService, 'deriveAccount').mockResolvedValue({ - id: 'meta-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TMeta1234567890123456789012', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: {}, - methods: ['signMessage', 'signTransaction'], - }); - - await accountsService.create({ - entropySource: 'test-entropy', - index: 0, - metamask: { correlationId: 'corr-123' }, - }); - - expect(mockedEmitSnapKeyringEvent).toHaveBeenCalledWith( - expect.anything(), - KeyringEvent.AccountCreated, - expect.objectContaining({ - metamask: { correlationId: 'corr-123' }, - }), - ); - }); - }); - - it('returns the persisted account and warns when repository create returns a conflicting account', async () => { - const conflictingAccount: TronKeyringAccount = { - id: 'pre-existing-conflict-id', - entropySource: 'test-entropy', - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - type: TrxAccountType.Eoa, - address: 'TConflict12345678901234567890', - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: {}, - methods: ['signMessage', 'signTransaction'], - }; - - await withAccountsService( - async ({ accountsService, mockAccountsRepository }) => { - mockAccountsRepository.create.mockResolvedValue(conflictingAccount); - - const result = await accountsService.create({ - entropySource: 'test-entropy', - index: 0, - }); - - expect(result.id).toBe('pre-existing-conflict-id'); - expect(mockLogger.warn).toHaveBeenCalled(); - }, - ); - }); - - it('throws when no primary entropy source is available', async () => { - await withAccountsService(async ({ accountsService, mockSnapClient }) => { - mockSnapClient.listEntropySources.mockResolvedValue([ - { - id: 'non-primary', - primary: false, - type: 'mnemonic', - name: 'Non-Primary', - }, - ]); - - await expect(accountsService.create()).rejects.toThrow( - 'No default entropy source found', - ); - }); - }); - }); - describe('getAll', () => { it('delegates to repository and returns result', async () => { const accounts: TronKeyringAccount[] = [ diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts index 2e0fd2ae2..c664b2e9b 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts @@ -7,15 +7,10 @@ import type { import { AccountCreationType, assertCreateAccountOptionIsSupported, - KeyringEvent, TrxAccountType, } from '@metamask/keyring-api'; -import { - emitSnapKeyringEvent, - getSelectedAccounts, -} from '@metamask/keyring-snap-sdk'; +import { getSelectedAccounts } from '@metamask/keyring-snap-sdk'; import { InFlightCoalescer } from '@metamask/snap-networks-utils/dedupe'; -import type { Json } from '@metamask/snaps-sdk'; import { assert } from '@metamask/superstruct'; import { hexToBytes } from '@metamask/utils'; import { computeAddress } from 'ethers'; @@ -28,7 +23,6 @@ import { asStrictKeyringAccount } from '../../entities/keyring-account'; import type { TronKeyringAccount } from '../../entities/keyring-account'; import { createTronBip44AddressDeriver } from '../../utils/deriveTronFromCoinTypeNode'; import { sanitizeSensitiveError } from '../../utils/errors'; -import { getLowestUnusedIndex } from '../../utils/getLowestUnusedIndex'; import { createPrefixedLogger } from '../../utils/logger'; import type { ILogger } from '../../utils/logger'; import { DerivationPathStruct } from '../../validation/structs'; @@ -36,7 +30,6 @@ import type { AssetsService } from '../assets/AssetsService'; import type { ConfigProvider } from '../config'; import type { TransactionsService } from '../transactions/TransactionsService'; import type { AccountsRepository } from './AccountsRepository'; -import type { CreateAccountOptions } from './types'; /** * Elliptic curve for TRON (same as Ethereum) @@ -204,136 +197,6 @@ export class AccountsService { } } - async deriveAccount({ - entropySource, - index, - }: { - entropySource: EntropySourceId; - index: number; - }): Promise { - const derivationPath = AccountsService.getDefaultDerivationPath(index); - const { address } = await this.deriveTronKeypair({ - entropySource, - derivationPath, - }); - - return { - id: globalThis.crypto.randomUUID(), - entropySource, - derivationPath, - index, - type: TrxAccountType.Eoa, - address, - scopes: SUPPORTED_SCOPES as unknown as Network[], - options: { - entropy: { - type: 'mnemonic', - id: entropySource, - derivationPath, - groupIndex: index, - }, - exportable: true, - }, - methods: ['signMessage', 'signTransaction'], - }; - } - - async create(options?: CreateAccountOptions): Promise { - const accounts = await this.#accountsRepository.getAll(); - - const entropySource = - options?.entropySource ?? (await this.#getDefaultEntropySource()); - const index = - options?.index ?? - this.#getLowestUnusedKeyringAccountIndex(accounts, entropySource); - - /** - * Now that we have the `entropySource` and `index` ready, - * we need to make sure that they do not correspond to an existing account already. - */ - const sameAccount = accounts.find( - (account) => - account.index === index && account.entropySource === entropySource, - ); - - if (sameAccount) { - this.#logger.warn( - '[🔑 Keyring] An account already exists with the same derivation path and entropy source. Skipping account creation.', - ); - return asStrictKeyringAccount(sameAccount); - } - - const derivedAccount = await this.deriveAccount({ - entropySource, - index, - }); - - const { metamask: metamaskOptions, ...remainingOptions } = options ?? {}; - - const tronKeyringAccount: TronKeyringAccount = { - ...derivedAccount, - options: { - ...derivedAccount.options, - ...(Object.fromEntries( - Object.entries(remainingOptions).filter( - ([, value]) => value !== undefined, - ), - ) as Record), - groupIndex: index, - }, - }; - - const persistedAccount = - await this.#accountsRepository.create(tronKeyringAccount); - - if (persistedAccount.id !== tronKeyringAccount.id) { - this.#logger.warn( - '[🔑 Keyring] An account already exists with the same derivation path and entropy source. Skipping account creation.', - ); - return asStrictKeyringAccount(persistedAccount); - } - - try { - const keyringAccount = asStrictKeyringAccount(tronKeyringAccount); - - await emitSnapKeyringEvent(snap, KeyringEvent.AccountCreated, { - /** - * We can't pass the `keyringAccount` object because it contains the index - * and the snaps sdk does not allow extra properties. - */ - account: keyringAccount, - /** - * Skip account creation confirmation dialogs to make it look like a native - * account creation flow. - */ - displayConfirmation: false, - /** - * Internal options to MetaMask that includes a correlation ID. We need - * to also emit this ID to the Snap keyring. - */ - ...(metamaskOptions - ? { - metamask: metamaskOptions, - } - : {}), - }); - - return keyringAccount; - } catch (error) { - // Rollback: if the event emission fails after the account was persisted, - // remove it from state so we don't end up with an orphaned record. - try { - await this.#accountsRepository.delete(tronKeyringAccount.id); - } catch (deleteError) { - this.#logger.error( - { deleteError, accountId: tronKeyringAccount.id }, - 'Failed to rollback account creation', - ); - } - throw error; - } - } - /** * Batch-creates Tron accounts for a BIP-44 index or index range. Existing accounts for the * same entropy source and index are returned without duplicate state writes. @@ -629,31 +492,7 @@ export class AccountsService { return createTronBip44AddressDeriver(bip44Node); } - #getLowestUnusedKeyringAccountIndex( - accounts: TronKeyringAccount[], - entropySource: EntropySourceId, - ): number { - const accountsFilteredByEntropySourceId = accounts.filter( - (account) => account.entropySource === entropySource, - ); - - return getLowestUnusedIndex(accountsFilteredByEntropySourceId); - } - static getDefaultDerivationPath(index: number): `m/${string}` { return `m/44'/195'/0'/0/${index}`; } - - async #getDefaultEntropySource(): Promise { - const entropySources = await this.#snapClient.listEntropySources(); - const defaultEntropySource = entropySources.find(({ primary }) => primary); - - if (!defaultEntropySource) { - throw new Error( - 'No default entropy source found - this can never happen', - ); - } - - return defaultEntropySource.id; - } } diff --git a/packages/tron-wallet-snap/src/services/accounts/types.ts b/packages/tron-wallet-snap/src/services/accounts/types.ts deleted file mode 100644 index af4ed23d2..000000000 --- a/packages/tron-wallet-snap/src/services/accounts/types.ts +++ /dev/null @@ -1,8 +0,0 @@ -import type { EntropySourceId, MetaMaskOptions } from '@metamask/keyring-api'; -import type { Json } from '@metamask/snaps-sdk'; - -export type CreateAccountOptions = { - entropySource?: EntropySourceId; - index?: number; - [key: string]: Json | undefined; -} & MetaMaskOptions; diff --git a/packages/tron-wallet-snap/src/utils/getLowestUnusedIndex.ts b/packages/tron-wallet-snap/src/utils/getLowestUnusedIndex.ts deleted file mode 100644 index fc15d695a..000000000 --- a/packages/tron-wallet-snap/src/utils/getLowestUnusedIndex.ts +++ /dev/null @@ -1,44 +0,0 @@ -export type WithIndex = { - index: number; -}; - -/** - * Generating a new index for the KeyringAccount is not as straightforward as one might think. - * We cannot assume that this number will continuosly increase because one can delete an account with - * an index in the middle of the list. The right way to do it is to loop through the keyringAccounts - * and get the lowest index that is not yet used. - * - * This function does precisely that, in a generic way, as it can work with any array of items that - * have a field `index`. - * - * Eg: - * Used Indices: [] -> Lowest is 0. - * Used Indices: [0, 1, 2, 4] -> Lowest is 3. - * - * @param items - The items to check. - * @returns The lowest unused index. - */ -export function getLowestUnusedIndex(items: WithIndex[]): number { - if (items.length === 0) { - return 0; - } - - const usedIndices = items - .map((item) => item.index) - .sort((first, second) => first - second); - - let lowestUnusedIndex = 0; - - for (const usedIndex of usedIndices) { - /** - * From lower to higher, the moment we find a gap, we can use it - */ - if (usedIndex !== lowestUnusedIndex) { - break; - } - - lowestUnusedIndex += 1; - } - - return lowestUnusedIndex; -} From 183bc251de0e67889cba8dcfb61fbe909d5d784a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 14 Aug 2026 12:54:12 +0200 Subject: [PATCH 6/9] chore(tron-wallet-snap): update shasum --- packages/tron-wallet-snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 127de4db9..1ec3d2bc2 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "CkbJj+1wNlwEe63xVizdwEQ2FAEwaho+WoX/gsmP9V0=", + "shasum": "b8h4FBAtY+QB079E+s6C+5/ZWAGA18CNAHwwvotJy3k=", "location": { "npm": { "filePath": "dist/bundle.js", From 90cc05d303084469c0295caa2d4129cb433d6ca8 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 14 Aug 2026 13:08:48 +0200 Subject: [PATCH 7/9] fix(tron-wallet-snap): remove AccountDeleted emission that broke v2 account deletion --- packages/tron-wallet-snap/CHANGELOG.md | 2 ++ packages/tron-wallet-snap/snap.manifest.json | 2 +- .../src/handlers/keyring/keyring.test.ts | 27 +++++++++++++++++++ .../src/handlers/keyring/keyring.ts | 15 ++++------- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index f9e9e6056..d13e1e0c3 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fix account deletion failing against keyring v2 clients by removing the `AccountDeleted` event emission from `keyring_deleteAccount` ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) + - v2 clients reject v1 lifecycle events, which aborted the deletion before the account was removed from state. Deletion is client-initiated in v2, so no event is needed. - Coalesce concurrent account synchronization runs for the same accounts so stacked triggers (cronjob and background events) share one run instead of duplicating network fetches, state writes, and keyring events ([#149](https://github.com/MetaMask/internal-snaps/pull/149)) ## [3.1.0] diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 1ec3d2bc2..fcea9f3e5 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "b8h4FBAtY+QB079E+s6C+5/ZWAGA18CNAHwwvotJy3k=", + "shasum": "KQqXlTpbyKV2yXqWLYKmcmFRQhM5U0us9/1QkZQJOZM=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts b/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts index ac90d8db0..7817f8e2c 100644 --- a/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts +++ b/packages/tron-wallet-snap/src/handlers/keyring/keyring.test.ts @@ -74,6 +74,7 @@ describe('KeyringHandler', () => { findById: jest.fn().mockResolvedValue(mockAccount), findByIdOrThrow: jest.fn().mockResolvedValue(mockAccount), createAccounts: jest.fn(), + delete: jest.fn().mockResolvedValue(undefined), getAll: jest.fn().mockResolvedValue([mockAccount]), deriveTronKeypair: jest.fn().mockResolvedValue({ privateKeyHex: 'a'.repeat(64), @@ -559,6 +560,32 @@ describe('KeyringHandler', () => { }); }); + describe('deleteAccount', () => { + it('deletes the account without emitting keyring events', async () => { + await keyringHandler.deleteAccount(mockAccount.id); + + expect(mockAccountsService.delete).toHaveBeenCalledWith(mockAccount.id); + }); + + it('throws for an invalid account id', async () => { + await expect(keyringHandler.deleteAccount('not-a-uuid')).rejects.toThrow( + expect.anything(), + ); + + expect(mockAccountsService.delete).not.toHaveBeenCalled(); + }); + + it('throws when the account does not exist', async () => { + mockAccountsService.findById.mockResolvedValue(null); + + await expect( + keyringHandler.deleteAccount(mockAccount.id), + ).rejects.toThrow(`Account "${mockAccount.id}" not found`); + + expect(mockAccountsService.delete).not.toHaveBeenCalled(); + }); + }); + describe('createAccounts', () => { it('delegates to accountsService.createAccounts and returns the result', async () => { const createdAccounts = [ diff --git a/packages/tron-wallet-snap/src/handlers/keyring/keyring.ts b/packages/tron-wallet-snap/src/handlers/keyring/keyring.ts index 710278d18..02c1c27eb 100644 --- a/packages/tron-wallet-snap/src/handlers/keyring/keyring.ts +++ b/packages/tron-wallet-snap/src/handlers/keyring/keyring.ts @@ -1,7 +1,4 @@ -import { - KeyringEvent, - ListAccountAssetsResponseStruct, -} from '@metamask/keyring-api'; +import { ListAccountAssetsResponseStruct } from '@metamask/keyring-api'; import type { Balance, CreateAccountOptions as KeyringBatchCreateAccountOptions, @@ -16,7 +13,6 @@ import type { ExportedAccount, KeyringSnapRpc, } from '@metamask/keyring-api/v2'; -import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError, @@ -381,12 +377,11 @@ export class KeyringHandler implements KeyringSnapRpc { try { validateRequest({ accountId }, DeleteAccountStruct); - const account = await this.#getAccountOrThrow(accountId); - - await emitSnapKeyringEvent(snap, KeyringEvent.AccountDeleted, { - id: account.id, - }); + await this.#getAccountOrThrow(accountId); + // No AccountDeleted event: deletion is client-initiated in keyring v2, + // and v2 clients reject v1 lifecycle events (which would abort the + // deletion below). await this.#accountsService.delete(accountId); } catch (error: unknown) { this.#logger.error({ error }, 'Error deleting account'); From 534a6b23608f01fb6e64426dd332ef6df9687a8a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 14 Aug 2026 13:25:19 +0200 Subject: [PATCH 8/9] chore(tron-wallet-snap): update shasum --- packages/tron-wallet-snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index fcea9f3e5..5630cb017 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "KQqXlTpbyKV2yXqWLYKmcmFRQhM5U0us9/1QkZQJOZM=", + "shasum": "FFwqtuyYlJM+7NFbnb+sYAhnjlQOQUB2Rbhfx0IZkmM=", "location": { "npm": { "filePath": "dist/bundle.js", From 280061322709cbba196f036f0e43c07b7ad2f28d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 19 Aug 2026 16:30:39 +0200 Subject: [PATCH 9/9] chore(tron-wallet-snap): update shasum --- packages/tron-wallet-snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 5630cb017..dc870e3c2 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "FFwqtuyYlJM+7NFbnb+sYAhnjlQOQUB2Rbhfx0IZkmM=", + "shasum": "BWejLCfSNzan3omP7THB+pL6KhsuVTCGaW39L3v6KTU=", "location": { "npm": { "filePath": "dist/bundle.js",