diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 3e958920..04b91d56 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -265,14 +265,6 @@ "count": 4 } }, - "packages/tron-wallet-snap/src/handlers/keyring.test.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 10 - }, - "import-x/no-extraneous-dependencies": { - "count": 1 - } - }, "packages/tron-wallet-snap/src/handlers/keyring.ts": { "import-x/no-extraneous-dependencies": { "count": 1 diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index a21d70a7..47345baf 100644 --- a/packages/bitcoin-wallet-snap/CHANGELOG.md +++ b/packages/bitcoin-wallet-snap/CHANGELOG.md @@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Migrate `KeyringHandler` to implement `KeyringSnapRpc` from `@metamask/keyring-api/v2` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) - Update `snap.manifest.json` to declare `derivePath` capability in the `bip44` keyring block ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) - Mark accounts as exportable in the keyring account mapping ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Bump `@metamask/snaps-cli` from `^8.3.0` to `^8.4.1` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Bump `@metamask/keyring-api` from `^23.2.0` to `^23.7.0` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Bump `@metamask/keyring-snap-sdk` from `^8.0.0` to `^9.2.1` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Bump `@metamask/snaps-sdk` from `11.1.1` to `11.2.0` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) ## [1.15.2] diff --git a/packages/sample-snap/CHANGELOG.md b/packages/sample-snap/CHANGELOG.md index cbeb3390..a88d9d4b 100644 --- a/packages/sample-snap/CHANGELOG.md +++ b/packages/sample-snap/CHANGELOG.md @@ -11,4 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add package to `internal-snaps` monorepo ([#8](https://github.com/MetaMask/internal-snaps/pull/8)) +### Changed + +- Bump `@metamask/snaps-cli` from `^8.3.0` to `^8.4.1` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Bump `@metamask/snaps-sdk` from `11.1.1` to `11.2.0` ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) + [Unreleased]: https://github.com/MetaMask/internal-snaps/ diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index 5c738618..cb7ff0c6 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING** Implement Keyring API v2 (`KeyringSnapRpc` interface): rename `listAccounts` → `getAccounts`, `listAccountAssets` → `getAccountAssets`, `listAccountTransactions` → `getAccountTransactions`; `getAccount` now throws instead of returning `undefined`; add `exportAccount` with hexadecimal private key export using `sensitive()` for redaction; remove v1-only methods `createAccount`, `discoverAccounts`, `filterAccountChains`, and `updateAccount`. ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Add `bip44:discover` support to `createAccounts`: checks on-chain activity across all Tron networks before persisting; returns `[]` if no activity to signal end-of-discovery to the client. ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) +- Add `endowment:keyring` capabilities to manifest declaring the `tron:728126428` scope, hexadecimal private key export, and BIP-44 derivation strategies. ([#56](https://github.com/MetaMask/internal-snaps/pull/56)) + ### Changed - Bump `@metamask/keyring-api` from `^23.2.0` to `^23.7.0` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 066007d9..7f6e425c 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": "83y66B4a9awX0MoxVkfv7HV+Jt5S04octhhc5dUELM4=", + "shasum": "dp+m4eGZFWjHhiJ3lVXiQ2lyprExWYyvCTQkx/myroU=", "location": { "npm": { "filePath": "dist/bundle.js", @@ -23,7 +23,22 @@ }, "initialPermissions": { "endowment:keyring": { - "allowedOrigins": ["https://portfolio.metamask.io"] + "allowedOrigins": ["https://portfolio.metamask.io"], + "capabilities": { + "scopes": ["tron:728126428"], + "privateKey": { + "exportFormats": [ + { + "encoding": "hexadecimal" + } + ] + }, + "bip44": { + "deriveIndex": true, + "deriveIndexRange": true, + "discover": true + } + } }, "snap_getBip32Entropy": [ { diff --git a/packages/tron-wallet-snap/src/handlers/keyring.test.ts b/packages/tron-wallet-snap/src/handlers/keyring.test.ts index b3c68105..b158892f 100644 --- a/packages/tron-wallet-snap/src/handlers/keyring.test.ts +++ b/packages/tron-wallet-snap/src/handlers/keyring.test.ts @@ -7,7 +7,6 @@ import { InvalidParamsError, UserRejectedRequestError, } from '@metamask/snaps-sdk'; -import { bytesToBase64, bytesToHex, stringToBytes } from '@metamask/utils'; import type { SnapClient } from '../clients/snap/SnapClient'; import { Network } from '../constants'; @@ -28,7 +27,7 @@ import { TronMultichainMethod } from './keyring-types'; * @returns Base64 encoded string. */ function toBase64(str: string): string { - return bytesToBase64(stringToBytes(str)); + return Buffer.from(str).toString('base64'); } /** @@ -38,7 +37,7 @@ function toBase64(str: string): string { * @returns Hex encoded string (without 0x prefix). */ function toHex(str: string): string { - return bytesToHex(stringToBytes(str)).slice(2); + return Buffer.from(str).toString('hex'); } describe('KeyringHandler', () => { @@ -52,7 +51,7 @@ describe('KeyringHandler', () => { ], type: 'tron:eoa', scopes: [Network.Mainnet, Network.Shasta], - entropySource: 'entropy-source-1' as any, + entropySource: 'entropy-source-1', derivationPath: "m/44'/195'/0'/0/0", index: 0, }; @@ -69,7 +68,7 @@ describe('KeyringHandler', () => { mockSnapClient = { scheduleBackgroundEvent: jest.fn().mockResolvedValue(undefined), trackError: jest.fn(), - } as any; + } as unknown as jest.Mocked; mockAccountsService = { findById: jest.fn().mockResolvedValue(mockAccount), findByIdOrThrow: jest.fn().mockResolvedValue(mockAccount), @@ -77,19 +76,22 @@ describe('KeyringHandler', () => { create: jest.fn(), createAccounts: jest.fn(), getAll: jest.fn().mockResolvedValue([mockAccount]), - } as any; - mockAssetsService = {} as any; + } as unknown as jest.Mocked; + mockAssetsService = { + getByKeyringAccountId: jest.fn().mockResolvedValue([]), + } as unknown as jest.Mocked; mockTransactionsService = { checkAddressActivity: jest.fn(), - } as any; + findByAccounts: jest.fn().mockResolvedValue([]), + } as unknown as jest.Mocked; mockWalletService = { handleKeyringRequest: jest .fn() .mockResolvedValue({ signature: '0xsignature123' }), - } as any; + } as unknown as jest.Mocked; mockConfirmationHandler = { handleKeyringRequest: jest.fn().mockResolvedValue(true), - } as any; + } as unknown as jest.Mocked; keyringHandler = new KeyringHandler({ logger: mockLogger, @@ -338,7 +340,7 @@ describe('KeyringHandler', () => { method: TronMultichainMethod.SignMessage, params: {}, }, - } as any; + } as unknown as KeyringRequest; await expect( keyringHandler.submitRequest(invalidRequest), @@ -355,7 +357,7 @@ describe('KeyringHandler', () => { method: TronMultichainMethod.SignMessage, // Missing params }, - } as any; + } as unknown as KeyringRequest; await expect(keyringHandler.submitRequest(request)).rejects.toThrow( 'satisfy a union', @@ -513,114 +515,6 @@ describe('KeyringHandler', () => { }); }); - describe('discoverAccounts', () => { - const mockDerivedAccount: TronKeyringAccount = { - id: '123e4567-e89b-42d3-a456-426614174001', - address: 'TDerivedAddress12345678901234567', - options: {}, - methods: [ - TronMultichainMethod.SignMessage, - TronMultichainMethod.SignTransaction, - ], - type: 'tron:eoa', - scopes: [Network.Mainnet, Network.Shasta], - entropySource: 'test-entropy-source' as any, - derivationPath: "m/44'/195'/0'/0/0", - index: 0, - }; - - beforeEach(() => { - jest - .spyOn(mockAccountsService, 'deriveAccount') - .mockImplementation() - .mockResolvedValue(mockDerivedAccount); - jest - .spyOn(mockTransactionsService, 'checkAddressActivity') - .mockImplementation(); - }); - - it('returns empty array if there is no activity on any of the scopes', async () => { - mockTransactionsService.checkAddressActivity - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(false); - - const result = await keyringHandler.discoverAccounts?.( - [Network.Mainnet, Network.Shasta], - 'test-entropy-source', - 0, - ); - - expect(result).toStrictEqual([]); - expect(mockAccountsService.deriveAccount).toHaveBeenCalledWith({ - entropySource: 'test-entropy-source', - index: 0, - }); - }); - - it('returns discovered accounts when there is activity on any scope', async () => { - mockTransactionsService.checkAddressActivity - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(true); - - const result = await keyringHandler.discoverAccounts?.( - [Network.Mainnet, Network.Shasta], - 'test-entropy-source', - 3, - ); - - expect(result).toStrictEqual([ - { - type: 'bip44', - scopes: [Network.Mainnet, Network.Shasta], - derivationPath: mockDerivedAccount.derivationPath, - }, - ]); - }); - - it('throws error if there is an error fetching transactions', async () => { - mockTransactionsService.checkAddressActivity.mockRejectedValue( - new Error('Network error'), - ); - - await expect( - keyringHandler.discoverAccounts?.([Network.Mainnet], 'test', 0), - ).rejects.toThrow('Network error'); - }); - - it('throws error if no scopes are provided', async () => { - await expect( - keyringHandler.discoverAccounts?.([], 'test', 0), - ).rejects.toThrow('Expected a nonempty array but received an empty one'); - expect( - mockTransactionsService.checkAddressActivity, - ).not.toHaveBeenCalled(); - }); - - it('throws error if scope is not a valid Tron network', async () => { - await expect( - keyringHandler.discoverAccounts?.( - ['invalid:network' as Network], - 'test', - 0, - ), - ).rejects.toThrow(/Expected one of/u); - expect( - mockTransactionsService.checkAddressActivity, - ).not.toHaveBeenCalled(); - }); - - it('throws error if groupIndex is negative', async () => { - await expect( - keyringHandler.discoverAccounts?.([Network.Mainnet], 'test', -1), - ).rejects.toThrow( - 'Expected a number greater than or equal to 0 but received `-1`', - ); - expect( - mockTransactionsService.checkAddressActivity, - ).not.toHaveBeenCalled(); - }); - }); - describe('setSelectedAccounts', () => { const NON_EXISTENT_ACCOUNT_ID = '123e4567-e89b-42d3-a456-426614174999'; @@ -654,29 +548,15 @@ describe('KeyringHandler', () => { }); }); - describe('listAccounts', () => { + describe('getAccounts', () => { it('fails with cause', async () => { const causeError = new Error('Account error'); mockAccountsService.getAll.mockRejectedValue(causeError); - await expect(keyringHandler.listAccounts()).rejects.toMatchObject({ - message: 'Error listing accounts', - cause: causeError, - }); - }); - }); - - describe('createAccount', () => { - it('fails with cause', async () => { - const causeError = new Error('Account error'); - - mockAccountsService.create.mockRejectedValue(causeError); - - await expect(keyringHandler.createAccount()).rejects.toMatchObject({ - message: `Error creating account: ${causeError.message}`, - cause: causeError, - }); + await expect(keyringHandler.getAccounts()).rejects.toThrow( + 'Error listing accounts', + ); }); }); @@ -721,5 +601,78 @@ describe('KeyringHandler', () => { 'Key derivation failed. Please check your connection and try again.', ); }); + + it('delegates bip44:discover to accountsService.createAccounts and returns the account', async () => { + const createdAccounts = [ + { + id: mockAccount.id, + address: mockAccount.address, + type: mockAccount.type, + options: mockAccount.options, + methods: mockAccount.methods, + scopes: mockAccount.scopes, + }, + ]; + const options: KeyringBatchCreateAccountOptions = { + type: AccountCreationType.Bip44Discover, + entropySource: 'entropy-source-1', + groupIndex: 0, + }; + + mockAccountsService.createAccounts.mockResolvedValue(createdAccounts); + + const result = await keyringHandler.createAccounts(options); + + expect(mockAccountsService.createAccounts).toHaveBeenCalledWith(options); + expect(result).toStrictEqual(createdAccounts); + }); + + it('delegates bip44:discover to accountsService.createAccounts and returns empty array when no activity', async () => { + const options: KeyringBatchCreateAccountOptions = { + type: AccountCreationType.Bip44Discover, + entropySource: 'entropy-source-1', + groupIndex: 5, + }; + + mockAccountsService.createAccounts.mockResolvedValue([]); + + const result = await keyringHandler.createAccounts(options); + + expect(mockAccountsService.createAccounts).toHaveBeenCalledWith(options); + expect(result).toStrictEqual([]); + }); + }); + + describe('getAccountAssets', () => { + it('returns asset types for an account', async () => { + const result = await keyringHandler.getAccountAssets(mockAccount.id); + + expect(result).toStrictEqual([]); + expect(mockAssetsService.getByKeyringAccountId).toHaveBeenCalledWith( + mockAccount.id, + ); + }); + + it('throws when the account is not found', async () => { + mockAccountsService.findById.mockResolvedValue(null); + + await expect( + keyringHandler.getAccountAssets(mockAccount.id), + ).rejects.toThrow('not found'); + }); + }); + + describe('getAccountTransactions', () => { + it('returns paginated transactions for an account', async () => { + const result = await keyringHandler.getAccountTransactions( + mockAccount.id, + { limit: 10, next: null }, + ); + + expect(result).toStrictEqual({ data: [], next: null }); + expect(mockTransactionsService.findByAccounts).toHaveBeenCalledWith([ + mockAccount, + ]); + }); }); }); diff --git a/packages/tron-wallet-snap/src/handlers/keyring.ts b/packages/tron-wallet-snap/src/handlers/keyring.ts index 7864a0c6..f6f0e5cc 100644 --- a/packages/tron-wallet-snap/src/handlers/keyring.ts +++ b/packages/tron-wallet-snap/src/handlers/keyring.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ import { KeyringEvent, ListAccountAssetsResponseStruct, @@ -6,9 +5,6 @@ import { import type { Balance, CreateAccountOptions as KeyringBatchCreateAccountOptions, - DiscoveredAccount, - EntropySourceId, - Keyring, KeyringAccount, KeyringRequest, KeyringResponse, @@ -16,17 +12,20 @@ import type { ResolvedAccountAddress, Transaction, } from '@metamask/keyring-api'; -import { - emitSnapKeyringEvent, - handleKeyringRequest, -} from '@metamask/keyring-snap-sdk'; +import type { + ExportAccountOptions, + ExportedAccount, + KeyringSnapRpc, +} from '@metamask/keyring-api/v2'; +import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; +import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import { InvalidParamsError, SnapError, UserRejectedRequestError, } from '@metamask/snaps-sdk'; import type { Json, JsonRpcRequest } from '@metamask/snaps-sdk'; -import { array, assert } from '@metamask/superstruct'; +import { array, assert, is, sensitive } from '@metamask/superstruct'; import type { CaipAssetType, CaipAssetTypeOrId, @@ -40,7 +39,6 @@ import type { Network } from '../constants'; import { asStrictKeyringAccount } from '../entities/keyring-account'; import type { TronKeyringAccount } from '../entities/keyring-account'; import type { AccountsService } from '../services/accounts/AccountsService'; -import type { CreateAccountOptions } from '../services/accounts/types'; import type { AssetsService } from '../services/assets/AssetsService'; import type { ConfirmationHandler } from '../services/confirmation/ConfirmationHandler'; import type { TransactionsService } from '../services/transactions/TransactionsService'; @@ -49,14 +47,14 @@ import { sanitizeSensitiveError } from '../utils/errors'; import { createPrefixedLogger } from '../utils/logger'; import type { ILogger } from '../utils/logger'; import { - CreateAccountOptionsStruct, DeleteAccountStruct, - DiscoverAccountsStruct, + ExportAccountRequestStruct, GetAccounBalancesResponseStruct, GetAccountBalancesStruct, GetAccountStruct, ListAccountAssetsStruct, ListAccountTransactionsStruct, + PrivateKeyHexStruct, SignTransactionRequestStruct, TronKeyringRequestStruct, UuidStruct, @@ -70,7 +68,7 @@ import { import { BackgroundEventMethod } from './cronjob'; import { TronMultichainMethod } from './keyring-types'; -export class KeyringHandler implements Keyring { +export class KeyringHandler implements KeyringSnapRpc { readonly #logger: ILogger; readonly #snapClient: SnapClient; @@ -128,8 +126,13 @@ export class KeyringHandler implements Keyring { } } - async listAccounts(): Promise { - return (await this.#listAccounts()).map(asStrictKeyringAccount); + async getAccounts(): Promise { + try { + return (await this.#listAccounts()).map(asStrictKeyringAccount); + } catch (error: unknown) { + this.#logger.error({ error }, 'Error listing accounts'); + throw new SnapError(error as Error); + } } async #getAccount( @@ -140,20 +143,23 @@ export class KeyringHandler implements Keyring { (await this.#accountsService.findById(accountId)) ?? undefined; return account; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + } catch (error: unknown) { this.#logger.error({ error }, 'Error getting account'); - throw new SnapError(error); + throw new SnapError(error as Error); } } - async getAccount(accountId: string): Promise { - validateRequest({ accountId }, GetAccountStruct); + async getAccount(accountId: string): Promise { + try { + validateRequest({ accountId }, GetAccountStruct); - const account = await this.#getAccount(accountId); + const account = await this.#getAccountOrThrow(accountId); - return account ? asStrictKeyringAccount(account) : undefined; + return asStrictKeyringAccount(account); + } catch (error: unknown) { + this.#logger.error({ error }, 'Error getting account'); + throw new SnapError(error as Error); + } } async #getAccountOrThrow(accountId: string): Promise { @@ -166,21 +172,6 @@ export class KeyringHandler implements Keyring { return account; } - async createAccount(options?: CreateAccountOptions): Promise { - validateRequest(options, CreateAccountOptionsStruct); - - try { - return await this.#accountsService.create(options); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - this.#logger.error({ error }, 'Error creating account'); - throw new Error(`Error creating account: ${error.message}`, { - cause: error, - }); - } - } - async createAccounts( options: KeyringBatchCreateAccountOptions, ): Promise { @@ -192,7 +183,7 @@ export class KeyringHandler implements Keyring { } } - async listAccountAssets(accountId: string): Promise { + async getAccountAssets(accountId: string): Promise { try { validateRequest({ accountId }, ListAccountAssetsStruct); @@ -214,9 +205,7 @@ export class KeyringHandler implements Keyring { validateResponse(result, ListAccountAssetsResponseStruct); return result; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + } catch (error: unknown) { this.#logger.error({ error }, 'Error listing account assets'); throw error; } @@ -231,7 +220,7 @@ export class KeyringHandler implements Keyring { * @param pagination.next - The next signature to fetch from. * @returns The transactions for the given account. */ - async listAccountTransactions( + async getAccountTransactions( accountId: string, pagination: Pagination, ): Promise<{ @@ -275,64 +264,12 @@ export class KeyringHandler implements Keyring { data: accountTransactions, next: nextSignature, }; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + } catch (error: unknown) { this.#logger.error({ error }, 'Error listing account transactions'); throw error; } } - async discoverAccounts?( - scopes: CaipChainId[], - entropySource: EntropySourceId, - groupIndex: number, - ): Promise { - try { - validateRequest( - { scopes, entropySource, groupIndex }, - DiscoverAccountsStruct, - ); - - const derivedAccount = await this.#accountsService.deriveAccount({ - entropySource, - index: groupIndex, - }); - - const activityChecksPromises = []; - - for (const scope of scopes) { - activityChecksPromises.push( - this.#transactionsService.checkAddressActivity( - scope as Network, - derivedAccount.address, - ), - ); - } - - const activityOnScopes = await Promise.all(activityChecksPromises); - - const hasActivity = activityOnScopes.some((active) => active); - - if (!hasActivity) { - return []; - } - - return [ - { - type: 'bip44', - scopes, - derivationPath: derivedAccount.derivationPath, - }, - ]; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - this.#logger.error({ error }, 'Error discovering accounts'); - throw error; - } - } - async getAccountBalances( accountId: string, assets: CaipAssetType[], @@ -371,9 +308,7 @@ export class KeyringHandler implements Keyring { validateResponse(result, GetAccounBalancesResponseStruct); return result; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + } catch (error: unknown) { this.#logger.error({ error }, 'Error getting account balances'); throw error; } @@ -406,12 +341,42 @@ export class KeyringHandler implements Keyring { return caip10Address; } - async filterAccountChains(id: string, chains: string[]): Promise { - throw new Error('Method not implemented.'); - } + async exportAccount( + accountId: string, + options?: ExportAccountOptions, + ): Promise { + validateRequest({ accountId, options }, ExportAccountRequestStruct); - async updateAccount(account: KeyringAccount): Promise { - throw new Error('Method not implemented.'); + const account = await this.#getAccountOrThrow(accountId); + + const encoding = options?.encoding ?? 'hexadecimal'; + if (encoding !== 'hexadecimal') { + throw new Error('Only hexadecimal private key export is supported'); + } + + try { + const { privateKeyHex } = await this.#accountsService.deriveTronKeypair({ + entropySource: account.entropySource, + derivationPath: account.derivationPath, + }); + + // SECURITY: Wrap the struct with sensitive() so that any assertion + // failure redacts the actual value from the error message and + // StructError.value, preventing the private key from leaking into logs.A + if (!is(privateKeyHex, sensitive(PrivateKeyHexStruct))) { + throw new Error('Derived private key failed encoding validation'); + } + + return { + type: 'private-key', + encoding, + privateKey: privateKeyHex, + }; + } catch { + const errorMsg = 'Error exporting account'; + this.#logger.error(errorMsg); + throw new SnapError(errorMsg); + } } async deleteAccount(accountId: string): Promise { @@ -425,9 +390,7 @@ export class KeyringHandler implements Keyring { }); await this.#accountsService.delete(accountId); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + } catch (error: unknown) { this.#logger.error({ error }, 'Error deleting account'); throw error; } diff --git a/packages/tron-wallet-snap/src/permissions.ts b/packages/tron-wallet-snap/src/permissions.ts index 3234cca3..5c0e0ab1 100644 --- a/packages/tron-wallet-snap/src/permissions.ts +++ b/packages/tron-wallet-snap/src/permissions.ts @@ -1,4 +1,5 @@ import { KeyringRpcMethod } from '@metamask/keyring-api'; +import { KeyringSnapRpcMethod } from '@metamask/keyring-api/v2'; import { TestDappRpcRequestMethod } from './handlers/rpc/types'; @@ -10,15 +11,20 @@ const allowedOrigins = isDev ? ['http://localhost:3000'] : prodOrigins; const dappPermissions = isDev ? new Set([ - // Keyring methods + // Keyring v2 methods + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, + KeyringSnapRpcMethod.SetSelectedAccounts, + // Keyring v1 methods kept for backwards compatibility — callers using + // old method names are still accepted by the permission layer. KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.CreateAccounts, - KeyringRpcMethod.DeleteAccount, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, // Test dapp specific methods @@ -27,19 +33,25 @@ const dappPermissions = isDev : new Set([]); const metamaskPermissions = new Set([ - // Keyring methods + // Keyring v2 methods + KeyringSnapRpcMethod.GetAccounts, + KeyringSnapRpcMethod.GetAccount, + KeyringSnapRpcMethod.CreateAccounts, + KeyringSnapRpcMethod.DeleteAccount, + KeyringSnapRpcMethod.GetAccountBalances, + KeyringSnapRpcMethod.SubmitRequest, + KeyringSnapRpcMethod.GetAccountTransactions, + KeyringSnapRpcMethod.GetAccountAssets, + KeyringSnapRpcMethod.ResolveAccountAddress, + KeyringSnapRpcMethod.SetSelectedAccounts, + KeyringSnapRpcMethod.ExportAccount, + // Keyring v1 methods kept for backwards compatibility — callers using + // old method names are still accepted by the permission layer. KeyringRpcMethod.ListAccounts, - KeyringRpcMethod.GetAccount, KeyringRpcMethod.CreateAccount, - KeyringRpcMethod.CreateAccounts, - KeyringRpcMethod.DeleteAccount, KeyringRpcMethod.DiscoverAccounts, - KeyringRpcMethod.GetAccountBalances, - KeyringRpcMethod.SubmitRequest, KeyringRpcMethod.ListAccountTransactions, KeyringRpcMethod.ListAccountAssets, - KeyringRpcMethod.ResolveAccountAddress, - KeyringRpcMethod.SetSelectedAccounts, ]); const metamask = 'metamask'; 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 72fff9db..e8bdd0a7 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.test.ts @@ -126,7 +126,10 @@ type WithAccountsServiceCallback = (payload: { Pick >; mockTransactionsService: jest.Mocked< - Pick + Pick< + TransactionsService, + 'fetchNewTransactionsForAccount' | 'saveMany' | 'checkAddressActivity' + > >; }) => void | Promise; @@ -272,10 +275,14 @@ async function withAccountsService( }; const mockTransactionsService: jest.Mocked< - Pick + Pick< + TransactionsService, + 'fetchNewTransactionsForAccount' | 'saveMany' | 'checkAddressActivity' + > > = { fetchNewTransactionsForAccount: jest.fn().mockResolvedValue([]), saveMany: jest.fn().mockResolvedValue(undefined), + checkAddressActivity: jest.fn().mockResolvedValue(false), }; const accountsService = new AccountsService({ @@ -365,6 +372,24 @@ describe('AccountsService', () => { }); }); + describe('deriveTronKeypair', () => { + it('throws when getBip32Entropy returns missing key material', async () => { + await withAccountsService(async ({ accountsService, mockSnapClient }) => { + mockSnapClient.getBip32Entropy.mockResolvedValue({ + privateKey: undefined, + publicKey: undefined, + } as unknown as JsonBIP44Node); + + await expect( + accountsService.deriveTronKeypair({ + entropySource: 'test-entropy', + derivationPath: "m/44'/195'/0'/0/0", + }), + ).rejects.toThrow('Key derivation failed'); + }); + }); + }); + describe('createAccounts', () => { it('persists new accounts with a single merge and one coin-type entropy call', async () => { const coinJson = await getTronTestCoinTypeJson(); @@ -572,6 +597,67 @@ describe('AccountsService', () => { }, ); }); + + it('returns empty array for bip44:discover when no on-chain activity', async () => { + const coinJson = await getTronTestCoinTypeJson(); + + await withAccountsService( + async ({ + accountsService, + mockAccountsRepository, + mockTransactionsService, + }) => { + mockTransactionsService.checkAddressActivity.mockResolvedValue(false); + + const result = await accountsService.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: 'test-entropy', + groupIndex: 0, + }); + + expect(result).toStrictEqual([]); + expect( + mockTransactionsService.checkAddressActivity, + ).toHaveBeenCalled(); + expect( + mockAccountsRepository.mergeKeyringAccounts, + ).not.toHaveBeenCalled(); + }, + coinJson, + ); + }); + + it('creates and returns an account for bip44:discover when on-chain activity is found', async () => { + const coinJson = await getTronTestCoinTypeJson(); + + await withAccountsService( + async ({ + accountsService, + mockAccountsRepository, + mockTransactionsService, + }) => { + mockTransactionsService.checkAddressActivity + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + + const result = await accountsService.createAccounts({ + type: AccountCreationType.Bip44Discover, + entropySource: 'test-entropy', + groupIndex: 2, + }); + + expect(result).toHaveLength(1); + expect(result[0]?.options).toMatchObject({ + exportable: true, + entropy: expect.objectContaining({ groupIndex: 2 }), + }); + expect( + mockAccountsRepository.mergeKeyringAccounts, + ).toHaveBeenCalledTimes(1); + }, + coinJson, + ); + }); }); describe('create', () => { @@ -826,6 +912,46 @@ describe('AccountsService', () => { ); }); }); + + 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: [TrxScope.Mainnet, TrxScope.Nile, TrxScope.Shasta], + 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 }, + ]); + + await expect(accountsService.create()).rejects.toThrow( + 'No default entropy source found', + ); + }); + }); }); describe('getAll', () => { diff --git a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts index 25006633..3d9bd1b7 100644 --- a/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts +++ b/packages/tron-wallet-snap/src/services/accounts/AccountsService.ts @@ -22,6 +22,7 @@ import { computeAddress } from 'ethers'; import { TronWeb } from 'tronweb'; import type { SnapClient } from '../../clients/snap/SnapClient'; +import { Network } from '../../constants'; import { asStrictKeyringAccount } from '../../entities/keyring-account'; import type { TronKeyringAccount } from '../../entities/keyring-account'; import { createTronBip44AddressDeriver } from '../../utils/deriveTronFromCoinTypeNode'; @@ -342,15 +343,41 @@ export class AccountsService { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, + `${AccountCreationType.Bip44Discover}`, ]); const { entropySource } = options; + // For discovery, only proceed if the account at groupIndex has on-chain + // activity. No activity signals end-of-discovery; return [] to the client. + if (options.type === AccountCreationType.Bip44Discover) { + const { groupIndex } = options; + const derivedAccount = await this.deriveAccount({ + entropySource, + index: groupIndex, + }); + const activityChecks = await Promise.all( + Object.values(Network).map((scope) => + this.#transactionsService.checkAddressActivity( + scope, + derivedAccount.address, + ), + ), + ); + if (!activityChecks.some(Boolean)) { + return []; + } + } + // Get the range of accounts to create - const range = - options.type === AccountCreationType.Bip44DeriveIndex - ? { from: options.groupIndex, to: options.groupIndex } - : options.range; + let range: AccountCreationRange; + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + range = options.range; + } else { + // Bip44DeriveIndex | Bip44Discover — a single group index. Ranges are + // inclusive, so `from` and `to` are the same. + range = { from: options.groupIndex, to: options.groupIndex }; + } validateAccountCreationRange(range); // Get existing accounts for the same entropy source/range to avoid duplicate state writes. diff --git a/packages/tron-wallet-snap/src/validation/structs.ts b/packages/tron-wallet-snap/src/validation/structs.ts index 093d05b3..7211f510 100644 --- a/packages/tron-wallet-snap/src/validation/structs.ts +++ b/packages/tron-wallet-snap/src/validation/structs.ts @@ -4,6 +4,7 @@ import { SolMethod, TrxAccountType, } from '@metamask/keyring-api'; +import { ExportAccountOptionsStruct } from '@metamask/keyring-api/v2'; import type { Infer, Struct } from '@metamask/superstruct'; import { array, @@ -11,10 +12,7 @@ import { enums, integer, literal, - min, - nonempty, nullable, - number, object, optional, pattern, @@ -196,6 +194,11 @@ export const ListAccountTransactionsStruct = object({ }), }); +export const ExportAccountRequestStruct = object({ + accountId: UuidStruct, + options: optional(ExportAccountOptionsStruct), +}); + export const NetworkStruct = enums(Object.values(Network)); /** @@ -217,18 +220,6 @@ export const CreateAccountOptionsStruct = optional( }), ); -/** - * Validates discoverAccounts parameters. - * - scopes: Non-empty array of valid Tron network scopes (e.g., 'tron:728126428') - * - entropySource: String for the entropy source (UUID or ULID format) - * - groupIndex: Non-negative integer for the group index - */ -export const DiscoverAccountsStruct = object({ - scopes: nonempty(array(NetworkStruct)), - entropySource: string(), - groupIndex: min(number(), 0), -}); - export const GetAccounBalancesResponseStruct = record( CaipAssetTypeStruct, object({ @@ -315,6 +306,23 @@ export const GetFeeForTransactionResponseStruct = object({ value: nullable(PositiveNumberStringStruct), }); +/** + * Validates a Tron private key: exactly 64 lowercase hexadecimal characters + * (32 bytes without the 0x prefix). + */ +export const PrivateKeyHexStruct: Struct = define( + 'PrivateKeyHex', + (value) => { + if (typeof value !== 'string') { + return `Expected a string, but received: ${typeof value}`; + } + if (!/^[0-9a-f]{64}$/u.test(value)) { + return 'Expected a 64-character lowercase hexadecimal private key'; + } + return true; + }, +); + /** * Validates if a string is Base58 encoded. * Base58 uses alphanumeric characters excluding 0, O, I, and l. diff --git a/packages/tron-wallet-snap/tsconfig.json b/packages/tron-wallet-snap/tsconfig.json index 943ed741..df5a8f6b 100644 --- a/packages/tron-wallet-snap/tsconfig.json +++ b/packages/tron-wallet-snap/tsconfig.json @@ -11,8 +11,8 @@ "skipLibCheck": true, "lib": ["ES2023", "DOM"], "target": "es2023", - "module": "CommonJS", - "moduleResolution": "node", + "module": "preserve", + "moduleResolution": "bundler", "types": ["jest"] }, "include": ["**/*.ts", "**/*.tsx", "locales/*.json"]