diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index e7ed4a1094c..856b36af70e 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `isEnabled` callback to `SnapAccountProviderConfig` ([#8287](https://github.com/MetaMask/core/pull/8287)) + - Snap-based providers now accept an optional `isEnabled?: () => boolean` in their config. + - When provided, all provider operations (`getAccounts`, `getAccount`, `createAccounts`, `discoverAccounts`, `resyncAccounts`) are gated on this callback. + - Defaults to always enabled when not provided. - Use `{Btc,Sol}AccountProvider` as default providers ([#8262](https://github.com/MetaMask/core/pull/8262)) - Those providers were initially provided by the clients. - Add new `createMultichainAccountGroups` support to create multiple groups in batch ([#7801](https://github.com/MetaMask/core/pull/7801)), ([#8190](https://github.com/MetaMask/core/pull/8190)) @@ -39,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +- **BREAKING:** Remove `AccountProviderWrapper` class ([#8287](https://github.com/MetaMask/core/pull/8287)) + - This class is no longer exported. Use the `isEnabled` callback in `SnapAccountProviderConfig` instead to control provider availability. - **BREAKING:** Remove `MultichainAccountGroup.alignAccounts` method ([#7801](https://github.com/MetaMask/core/pull/7801)) - Use `MultichainAccountWallet.alignAccountsOf` instead, since this method properly lock the wallet (parent of this group) state. diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index d8b3456d113..d6c2fb7063c 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -1,9 +1,6 @@ import { isBip44Account } from '@metamask/account-api'; import { mnemonicPhraseToBytes } from '@metamask/key-tree'; -import type { - CreateAccountOptions, - KeyringAccount, -} from '@metamask/keyring-api'; +import type { KeyringAccount } from '@metamask/keyring-api'; import { AccountCreationType, BtcAccountType, @@ -20,20 +17,23 @@ import type { MultichainAccountServiceOptions } from './MultichainAccountService import { MultichainAccountService } from './MultichainAccountService'; import type { Bip44AccountProvider } from './providers'; import { TimeoutError } from './providers'; -import { AccountProviderWrapper } from './providers/AccountProviderWrapper'; import { + BTC_ACCOUNT_PROVIDER_DEFAULT_CONFIG, BTC_ACCOUNT_PROVIDER_NAME, BtcAccountProvider, } from './providers/BtcAccountProvider'; import { + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, EVM_ACCOUNT_PROVIDER_NAME, EvmAccountProvider, } from './providers/EvmAccountProvider'; import { + SOL_ACCOUNT_PROVIDER_DEFAULT_CONFIG, SOL_ACCOUNT_PROVIDER_NAME, SolAccountProvider, } from './providers/SolAccountProvider'; import { + TRX_ACCOUNT_PROVIDER_DEFAULT_CONFIG, TRX_ACCOUNT_PROVIDER_NAME, TrxAccountProvider, } from './providers/TrxAccountProvider'; @@ -381,7 +381,10 @@ describe('MultichainAccountService', () => { ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME], + expect.objectContaining({ + ...providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME], + isEnabled: expect.any(Function), + }), expect.any(Function), // TraceCallback ); }); @@ -396,12 +399,12 @@ describe('MultichainAccountService', () => { expect(withLocalPerfTrace).not.toHaveBeenCalled(); expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, traceFallback, ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + expect.objectContaining({ isEnabled: expect.any(Function) }), traceFallback, ); }); @@ -418,12 +421,12 @@ describe('MultichainAccountService', () => { expect(withLocalPerfTrace).not.toHaveBeenCalled(); expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, customTrace, ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + expect.objectContaining({ isEnabled: expect.any(Function) }), customTrace, ); }); @@ -441,12 +444,12 @@ describe('MultichainAccountService', () => { expect(withLocalPerfTrace).toHaveBeenCalledWith(traceFallback); expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, wrappedTrace, ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + expect.objectContaining({ isEnabled: expect.any(Function) }), wrappedTrace, ); }); @@ -465,7 +468,7 @@ describe('MultichainAccountService', () => { expect(withLocalPerfTrace).toHaveBeenCalledWith(customTrace); expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( expect.anything(), - undefined, + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, wrappedTrace, ); }); @@ -496,12 +499,15 @@ describe('MultichainAccountService', () => { expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, - undefined, + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, expect.any(Function), // TraceCallback ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME], + expect.objectContaining({ + ...providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME], + isEnabled: expect.any(Function), + }), expect.any(Function), // TraceCallback ); }); @@ -1335,132 +1341,70 @@ describe('MultichainAccountService', () => { // This tests the simplified parameter signature expect(await service.setBasicFunctionality(false)).toBeUndefined(); }); - }); - - describe('AccountProviderWrapper', () => { - let wrapper: AccountProviderWrapper; - let solProvider: SolAccountProvider; - - beforeEach(async () => { - const { rootMessenger } = await setup({ - accounts: [MOCK_HD_ACCOUNT_1], - }); - - // Create actual SolAccountProvider instance for wrapping - solProvider = new SolAccountProvider( - getMultichainAccountServiceMessenger(rootMessenger), - ); - - // Spy on the provider methods - jest.spyOn(solProvider, 'resyncAccounts'); - jest.spyOn(solProvider, 'getAccounts'); - jest.spyOn(solProvider, 'getAccount'); - jest.spyOn(solProvider, 'createAccounts'); - jest.spyOn(solProvider, 'discoverAccounts'); - jest.spyOn(solProvider, 'isAccountCompatible'); - - wrapper = new AccountProviderWrapper( - getMultichainAccountServiceMessenger(rootMessenger), - solProvider, - ); - }); - - it('forwards capabilities from adapted provider', async () => { - expect(wrapper.capabilities).toStrictEqual(solProvider.capabilities); - }); - it('forwards resyncAccounts() if provider is enabled', async () => { - const spy = jest.spyOn(solProvider, 'resyncAccounts'); + it('snap provider isEnabled callbacks return true by default', async () => { + const { mocks } = await setup({ accounts: [MOCK_HD_ACCOUNT_1] }); - // Enable first - should work normally - spy.mockResolvedValue(undefined); - await wrapper.resyncAccounts([]); - expect(spy).toHaveBeenCalledTimes(1); + const solIsEnabled = + mocks.SolAccountProvider.constructor.mock.calls[0][1].isEnabled; + const btcIsEnabled = + mocks.BtcAccountProvider.constructor.mock.calls[0][1].isEnabled; + const trxIsEnabled = + mocks.TrxAccountProvider.constructor.mock.calls[0][1].isEnabled; - // Disable - should return empty array - wrapper.setEnabled(false); - await wrapper.resyncAccounts([]); - expect(spy).toHaveBeenCalledTimes(1); // No new call, still 1 call + expect(solIsEnabled()).toBe(true); + expect(btcIsEnabled()).toBe(true); + expect(trxIsEnabled()).toBe(true); }); - it('returns empty array when getAccounts() is disabled', () => { - // Enable first - should work normally - (solProvider.getAccounts as jest.Mock).mockReturnValue([ - MOCK_HD_ACCOUNT_1, - ]); - expect(wrapper.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); + it('snap provider isEnabled callbacks return false after setBasicFunctionality(false)', async () => { + const { mocks, service } = await setup({ accounts: [MOCK_HD_ACCOUNT_1] }); - // Disable - should return empty array - wrapper.setEnabled(false); - expect(wrapper.getAccounts()).toStrictEqual([]); - }); + await service.setBasicFunctionality(false); - it('throws error when getAccount() is disabled', () => { - // Enable first - should work normally - (solProvider.getAccount as jest.Mock).mockReturnValue(MOCK_HD_ACCOUNT_1); - expect(wrapper.getAccount('test-id')).toStrictEqual(MOCK_HD_ACCOUNT_1); + const solIsEnabled = + mocks.SolAccountProvider.constructor.mock.calls[0][1].isEnabled; + const btcIsEnabled = + mocks.BtcAccountProvider.constructor.mock.calls[0][1].isEnabled; + const trxIsEnabled = + mocks.TrxAccountProvider.constructor.mock.calls[0][1].isEnabled; - // Disable - should throw error - wrapper.setEnabled(false); - expect(() => wrapper.getAccount('test-id')).toThrow( - 'Provider is disabled', - ); + expect(solIsEnabled()).toBe(false); + expect(btcIsEnabled()).toBe(false); + expect(trxIsEnabled()).toBe(false); }); - it('returns empty array when createAccounts() is disabled', async () => { - const options: CreateAccountOptions = { - type: AccountCreationType.Bip44DeriveIndex, - entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id, - groupIndex: 0, - }; + it('snap provider isEnabled callbacks compose with consumer-provided isEnabled', async () => { + const consumerIsEnabled = jest.fn().mockReturnValue(false); - // Enable first - should work normally - (solProvider.createAccounts as jest.Mock).mockResolvedValue([ - MOCK_HD_ACCOUNT_1, - ]); - expect(await wrapper.createAccounts(options)).toStrictEqual([ - MOCK_HD_ACCOUNT_1, - ]); - - // Disable - should return empty array and not call underlying provider - wrapper.setEnabled(false); - - const result = await wrapper.createAccounts(options); - expect(result).toStrictEqual([]); - }); - - it('returns empty array when discoverAccounts() is disabled', async () => { - const options = { - entropySource: MOCK_HD_ACCOUNT_1.options.entropy.id, - groupIndex: 0, - }; - - // Enable first - should work normally - (solProvider.discoverAccounts as jest.Mock).mockResolvedValue([ - MOCK_HD_ACCOUNT_1, - ]); - expect(await wrapper.discoverAccounts(options)).toStrictEqual([ - MOCK_HD_ACCOUNT_1, - ]); - - // Disable - should return empty array - wrapper.setEnabled(false); - - const result = await wrapper.discoverAccounts(options); - expect(result).toStrictEqual([]); - }); + const { mocks } = await setup({ + accounts: [MOCK_HD_ACCOUNT_1], + providerConfigs: { + [SOL_ACCOUNT_PROVIDER_NAME]: { + ...SOL_ACCOUNT_PROVIDER_DEFAULT_CONFIG, + isEnabled: consumerIsEnabled, + }, + [BTC_ACCOUNT_PROVIDER_NAME]: { + ...BTC_ACCOUNT_PROVIDER_DEFAULT_CONFIG, + isEnabled: consumerIsEnabled, + }, + [TRX_ACCOUNT_PROVIDER_NAME]: { + ...TRX_ACCOUNT_PROVIDER_DEFAULT_CONFIG, + isEnabled: consumerIsEnabled, + }, + }, + }); - it('delegates isAccountCompatible() to wrapped provider', () => { - // Mock the provider's compatibility check - (solProvider.isAccountCompatible as jest.Mock).mockReturnValue(true); - expect(wrapper.isAccountCompatible(MOCK_HD_ACCOUNT_1)).toBe(true); - expect(solProvider.isAccountCompatible).toHaveBeenCalledWith( - MOCK_HD_ACCOUNT_1, - ); + const solIsEnabled = + mocks.SolAccountProvider.constructor.mock.calls[0][1].isEnabled; + const btcIsEnabled = + mocks.BtcAccountProvider.constructor.mock.calls[0][1].isEnabled; + const trxIsEnabled = + mocks.TrxAccountProvider.constructor.mock.calls[0][1].isEnabled; - // Test with false return - (solProvider.isAccountCompatible as jest.Mock).mockReturnValue(false); - expect(wrapper.isAccountCompatible(MOCK_HD_ACCOUNT_1)).toBe(false); + expect(solIsEnabled()).toBe(false); + expect(btcIsEnabled()).toBe(false); + expect(trxIsEnabled()).toBe(false); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index b6ab4e4e357..9d626778198 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -29,15 +29,17 @@ import { TRX_ACCOUNT_PROVIDER_NAME, BtcAccountProvider, TrxAccountProvider, + BTC_ACCOUNT_PROVIDER_DEFAULT_CONFIG, + TRX_ACCOUNT_PROVIDER_DEFAULT_CONFIG, } from './providers'; import { - AccountProviderWrapper, - isAccountProviderWrapper, -} from './providers/AccountProviderWrapper'; -import { EvmAccountProvider } from './providers/EvmAccountProvider'; + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG, + EvmAccountProvider, +} from './providers/EvmAccountProvider'; import { SolAccountProvider } from './providers/SolAccountProvider'; import { SOL_ACCOUNT_PROVIDER_NAME, + SOL_ACCOUNT_PROVIDER_DEFAULT_CONFIG, SolAccountProviderConfig, } from './providers/SolAccountProvider'; import { SnapPlatformWatcher } from './snaps/SnapPlatformWatcher'; @@ -133,6 +135,8 @@ export class MultichainAccountService { readonly #trace: TraceCallback; + #basicFunctionalityEnabled: boolean = true; + readonly #wallets: Map< MultichainAccountWalletId, MultichainAccountWallet> @@ -177,36 +181,52 @@ export class MultichainAccountService { // This trace is passed down to wallets and providers to be used for tracing operations within them. this.#trace = trace; + // Provider configs: + const evmProviderConfig = + providerConfigs?.[EVM_ACCOUNT_PROVIDER_NAME] ?? + EVM_ACCOUNT_PROVIDER_DEFAULT_CONFIG; + const solProviderConfig = + providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME] ?? + SOL_ACCOUNT_PROVIDER_DEFAULT_CONFIG; + const btcProviderConfig = + providerConfigs?.[BTC_ACCOUNT_PROVIDER_NAME] ?? + BTC_ACCOUNT_PROVIDER_DEFAULT_CONFIG; + const trxProviderConfig = + providerConfigs?.[TRX_ACCOUNT_PROVIDER_NAME] ?? + TRX_ACCOUNT_PROVIDER_DEFAULT_CONFIG; + // TODO: Rely on keyring capabilities once the keyring API is used by all keyrings. this.#providers = [ - new EvmAccountProvider( + new EvmAccountProvider(this.#messenger, evmProviderConfig, trace), + new SolAccountProvider( this.#messenger, - providerConfigs?.[EVM_ACCOUNT_PROVIDER_NAME], + { + ...solProviderConfig, + isEnabled: (): boolean => + this.#basicFunctionalityEnabled && + (solProviderConfig.isEnabled?.() ?? true), + }, trace, ), - new AccountProviderWrapper( + new BtcAccountProvider( this.#messenger, - new SolAccountProvider( - this.#messenger, - providerConfigs?.[SOL_ACCOUNT_PROVIDER_NAME], - trace, - ), - ), - new AccountProviderWrapper( - this.#messenger, - new BtcAccountProvider( - this.#messenger, - providerConfigs?.[BTC_ACCOUNT_PROVIDER_NAME], - trace, - ), + { + ...btcProviderConfig, + isEnabled: (): boolean => + this.#basicFunctionalityEnabled && + (btcProviderConfig.isEnabled?.() ?? true), + }, + trace, ), - new AccountProviderWrapper( + new TrxAccountProvider( this.#messenger, - new TrxAccountProvider( - this.#messenger, - providerConfigs?.[TRX_ACCOUNT_PROVIDER_NAME], - trace, - ), + { + ...trxProviderConfig, + isEnabled: (): boolean => + this.#basicFunctionalityEnabled && + (trxProviderConfig.isEnabled?.() ?? true), + }, + trace, ), // Custom account providers that can be provided by the MetaMask client. ...providers, @@ -694,16 +714,7 @@ export class MultichainAccountService { async setBasicFunctionality(enabled: boolean): Promise { log(`Turning basic functionality: ${enabled ? 'ON' : 'OFF'}`); - // Loop through providers and enable/disable only wrapped ones when basic functionality changes - for (const provider of this.#providers) { - if (isAccountProviderWrapper(provider)) { - log( - `${enabled ? 'Enabling' : 'Disabling'} account provider: "${provider.getName()}"`, - ); - provider.setEnabled(enabled); - } - // Regular providers (like EVM) are never disabled for basic functionality - } + this.#basicFunctionalityEnabled = enabled; // Trigger alignment only when basic functionality is enabled if (enabled) { diff --git a/packages/multichain-account-service/src/index.ts b/packages/multichain-account-service/src/index.ts index 802cbfbc978..81c065ff83e 100644 --- a/packages/multichain-account-service/src/index.ts +++ b/packages/multichain-account-service/src/index.ts @@ -23,7 +23,6 @@ export type { MultichainAccountServiceAlignWalletAction, } from './MultichainAccountService-method-action-types'; export { - AccountProviderWrapper, BaseBip44AccountProvider, SnapAccountProvider, TimeoutError, diff --git a/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts b/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts deleted file mode 100644 index ed4c40c8259..00000000000 --- a/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts +++ /dev/null @@ -1,163 +0,0 @@ -import type { Bip44Account } from '@metamask/account-api'; -import type { - CreateAccountOptions, - EntropySourceId, - KeyringAccount, - KeyringCapabilities, -} from '@metamask/keyring-api'; -import type { InternalAccount } from '@metamask/keyring-internal-api'; - -import { BaseBip44AccountProvider } from './BaseBip44AccountProvider'; -import type { MultichainAccountServiceMessenger } from '../types'; - -/** - * A simple wrapper that adds disable functionality to any BaseBip44AccountProvider. - * When disabled, the provider will not create new accounts and return empty results. - */ -export class AccountProviderWrapper extends BaseBip44AccountProvider { - private isEnabled: boolean = true; - - private readonly provider: BaseBip44AccountProvider; - - constructor( - messenger: MultichainAccountServiceMessenger, - provider: BaseBip44AccountProvider, - ) { - super(messenger); - this.provider = provider; - } - - override getName(): string { - return this.provider.getName(); - } - - get capabilities(): KeyringCapabilities { - return this.provider.capabilities; - } - - /** - * Forward initialization to the wrapped provider to ensure both - * instances share the same visible account IDs. - * - * @param accounts - Account IDs to initialize with. - */ - override init(accounts: Bip44Account['id'][]): void { - this.provider.init(accounts); - } - - /** - * Set the enabled state for this provider. - * - * @param enabled - Whether the provider should be enabled. - */ - setEnabled(enabled: boolean): void { - this.isEnabled = enabled; - } - - /** - * Check if the provider is disabled. - * - * @returns True if the provider is disabled, false otherwise. - */ - isDisabled(): boolean { - return !this.isEnabled; - } - - /** - * Override resyncAccounts to not execute it when disabled. - * - * @param accounts - List of local accounts. - */ - override async resyncAccounts( - accounts: Bip44Account[], - ): Promise { - if (!this.isEnabled) { - return; - } - await this.provider.resyncAccounts(accounts); - } - - /** - * Override getAccounts to return empty array when disabled. - * - * @returns Array of accounts, or empty array if disabled. - */ - override getAccounts(): Bip44Account[] { - if (!this.isEnabled) { - return []; - } - return this.provider.getAccounts(); - } - - /** - * Override getAccount to throw when disabled. - * - * @param id - The account ID to retrieve. - * @returns The account with the specified ID. - * @throws When disabled or account not found. - */ - override getAccount( - id: Bip44Account['id'], - ): Bip44Account { - if (!this.isEnabled) { - throw new Error('Provider is disabled'); - } - return this.provider.getAccount(id); - } - - /** - * Implement abstract method: Check if account is compatible. - * Delegates directly to wrapped provider - no runtime checks needed! - * - * @param account - The account to check. - * @returns True if the account is compatible. - */ - isAccountCompatible(account: Bip44Account): boolean { - return this.provider.isAccountCompatible(account); - } - - /** - * Implement abstract method: Create accounts, returns empty array when disabled. - * - * @param options - Account creation options. - * @returns Promise resolving to created accounts, or empty array if disabled. - */ - async createAccounts( - options: CreateAccountOptions, - ): Promise[]> { - if (!this.isEnabled) { - return []; - } - return this.provider.createAccounts(options); - } - - /** - * Implement abstract method: Discover and create accounts, returns empty array when disabled. - * - * @param options - Account discovery options. - * @param options.entropySource - The entropy source to use. - * @param options.groupIndex - The group index to use. - * @returns Promise resolving to discovered accounts, or empty array if disabled. - */ - async discoverAccounts(options: { - entropySource: EntropySourceId; - groupIndex: number; - }): Promise[]> { - if (!this.isEnabled) { - return []; - } - return this.provider.discoverAccounts(options); - } -} - -/** - * Simple type guard to check if a provider is wrapped. - * - * @param provider - The provider to check. - * @returns True if the provider is an AccountProviderWrapper. - */ -export function isAccountProviderWrapper( - provider: unknown, -): provider is AccountProviderWrapper { - return provider instanceof AccountProviderWrapper; -} diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts index 8edb7197846..881bfc343de 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts @@ -9,7 +9,6 @@ import type { import { SnapControllerState } from '@metamask/snaps-controllers'; import deepmerge from 'deepmerge'; -import { AccountProviderWrapper } from './AccountProviderWrapper'; import { BTC_ACCOUNT_PROVIDER_DEFAULT_CONFIG, BTC_ACCOUNT_PROVIDER_NAME, @@ -164,7 +163,7 @@ function setup({ accounts?: InternalAccount[]; config?: SnapAccountProviderConfig; } = {}): { - provider: AccountProviderWrapper; + provider: MockBtcAccountProvider; messenger: RootMessenger; keyring: MockBtcKeyring; mocks: { @@ -227,14 +226,13 @@ function setup({ }); const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const btcProvider = new MockBtcAccountProvider( + const provider = new MockBtcAccountProvider( multichainMessenger, config, mockTrace, ); const accountIds = accounts.map((account) => account.id); - btcProvider.init(accountIds); - const provider = new AccountProviderWrapper(multichainMessenger, btcProvider); + provider.init(accountIds); return { provider, @@ -693,11 +691,7 @@ describe('BtcAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); // No trace callback (defaults to `traceFallback`). - const btcProvider = new MockBtcAccountProvider(multichainMessenger); - const provider = new AccountProviderWrapper( - multichainMessenger, - btcProvider, - ); + const provider = new MockBtcAccountProvider(multichainMessenger); const discovered = await provider.discoverAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -742,23 +736,68 @@ describe('BtcAccountProvider', () => { }); }); - describe('isDisabled', () => { - it('returns false when the provider is enabled (default)', () => { - const { provider } = setup(); - expect(provider.isDisabled()).toBe(false); + describe('isEnabled config callback', () => { + it('provider is enabled by default when no isEnabled callback is provided', () => { + const accounts = [MOCK_BTC_P2WPKH_ACCOUNT_1]; + const { provider } = setup({ accounts }); + expect(provider.getAccounts()).toStrictEqual(accounts); + }); + + it('blocks getAccounts when isEnabled returns false', () => { + const accounts = [MOCK_BTC_P2WPKH_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(provider.getAccounts()).toStrictEqual([]); }); - it('returns true after setEnabled(false)', () => { - const { provider } = setup(); - provider.setEnabled(false); - expect(provider.isDisabled()).toBe(true); + it('throws on getAccount when isEnabled returns false', () => { + const accounts = [MOCK_BTC_P2WPKH_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(() => provider.getAccount(MOCK_BTC_P2WPKH_ACCOUNT_1.id)).toThrow( + 'Provider is disabled', + ); }); - it('returns false after re-enabling', () => { - const { provider } = setup(); - provider.setEnabled(false); - provider.setEnabled(true); - expect(provider.isDisabled()).toBe(false); + it('blocks createAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); + }); + + it('blocks discoverAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); + }); + + it('re-enables operations when isEnabled returns true again', () => { + const accounts = [MOCK_BTC_P2WPKH_ACCOUNT_1]; + let enabled = false; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => enabled }), + }); + + expect(provider.getAccounts()).toStrictEqual([]); + + enabled = true; + expect(provider.getAccounts()).toStrictEqual(accounts); }); }); }); diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts index b8d1b95506e..6089e3b8098 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts @@ -91,7 +91,7 @@ export class BtcAccountProvider extends SnapAccountProvider { }); } - async discoverAccounts({ + protected override async discoverBip44Accounts({ entropySource, groupIndex, }: { diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts index 05721d34d57..f4b036c5d97 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts @@ -952,6 +952,19 @@ describe('SnapAccountProvider', () => { consoleWarnSpy.mockRestore(); consoleErrorSpy.mockRestore(); }); + + it('returns early without contacting snap when isEnabled returns false', async () => { + const { provider, mocks } = setup({ + accounts: mockAccounts, + config: { isEnabled: () => false }, + }); + + await provider.resyncAccounts(mockAccounts); + + expect( + mocks.SnapController.handleKeyringRequest.listAccounts, + ).not.toHaveBeenCalled(); + }); }); describe('ensureCanUseSnapPlatform', () => { diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 55dfeecff10..1d01e3d8c04 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -39,6 +39,13 @@ export type RestrictedSnapKeyring = { }; export type SnapAccountProviderConfig = { + /** + * Optional callback to determine if the provider is enabled. When provided, + * all provider operations check this callback before proceeding. If the callback + * returns `false`, operations return empty results or throw as appropriate. + * Defaults to always enabled when not provided. + */ + isEnabled?: () => boolean; maxConcurrency?: number; discovery: { enabled?: boolean; @@ -113,6 +120,31 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { this.#trace = trace; } + /** + * Returns whether the provider is currently enabled. + * + * @returns `true` if enabled (default when no callback is provided), `false` otherwise. + */ + #isEnabled(): boolean { + return this.config.isEnabled?.() ?? true; + } + + override getAccounts(): Bip44Account[] { + if (!this.#isEnabled()) { + return []; + } + return super.getAccounts(); + } + + override getAccount( + id: Bip44Account['id'], + ): Bip44Account { + if (!this.#isEnabled()) { + throw new Error('Provider is disabled'); + } + return super.getAccount(id); + } + /** * Ensures that the Snap platform is ready to be used. * @@ -202,6 +234,10 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { async resyncAccounts( accounts: Bip44Account[], ): Promise { + if (!this.#isEnabled()) { + return; + } + await this.withSnap(async ({ keyring }) => { const localSnapAccounts = accounts.filter( (account) => account.metadata.snap?.id === this.snapId, @@ -439,6 +475,10 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { async createAccounts( options: CreateAccountOptions, ): Promise[]> { + if (!this.#isEnabled()) { + return []; + } + assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -449,7 +489,18 @@ export abstract class SnapAccountProvider extends BaseBip44AccountProvider { ); } - abstract discoverAccounts(options: { + async discoverAccounts(options: { + entropySource: EntropySourceId; + groupIndex: number; + }): Promise[]> { + if (!this.#isEnabled()) { + return []; + } + + return this.discoverBip44Accounts(options); + } + + protected abstract discoverBip44Accounts(options: { entropySource: EntropySourceId; groupIndex: number; }): Promise[]>; diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index 7445ccda062..bf172baae88 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -9,7 +9,6 @@ import type { import { SnapControllerState } from '@metamask/snaps-controllers'; import deepmerge from 'deepmerge'; -import { AccountProviderWrapper } from './AccountProviderWrapper'; import type { SnapAccountProviderConfig } from './SnapAccountProvider'; import { SOL_ACCOUNT_PROVIDER_DEFAULT_CONFIG, @@ -148,7 +147,7 @@ function setup({ accounts?: InternalAccount[]; config?: SnapAccountProviderConfig; } = {}): { - provider: AccountProviderWrapper; + provider: MockSolAccountProvider; messenger: RootMessenger; keyring: MockSolanaKeyring; mocks: { @@ -212,14 +211,13 @@ function setup({ ); const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const solProvider = new MockSolAccountProvider( + const provider = new MockSolAccountProvider( multichainMessenger, config, mockTrace, ); const accountIds = accounts.map((account) => account.id); - solProvider.init(accountIds); - const provider = new AccountProviderWrapper(multichainMessenger, solProvider); + provider.init(accountIds); return { provider, @@ -653,15 +651,11 @@ describe('SolAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const solProvider = new MockSolAccountProvider( + const provider = new MockSolAccountProvider( multichainMessenger, undefined, mocks.trace, ); - const provider = new AccountProviderWrapper( - multichainMessenger, - solProvider, - ); const discovered = await provider.discoverAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -686,11 +680,7 @@ describe('SolAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); // No trace callback (defaults to `traceFallback`). - const solProvider = new MockSolAccountProvider(multichainMessenger); - const provider = new AccountProviderWrapper( - multichainMessenger, - solProvider, - ); + const provider = new MockSolAccountProvider(multichainMessenger); const discovered = await provider.discoverAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -709,15 +699,11 @@ describe('SolAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const solProvider = new MockSolAccountProvider( + const provider = new MockSolAccountProvider( multichainMessenger, undefined, mocks.trace, ); - const provider = new AccountProviderWrapper( - multichainMessenger, - solProvider, - ); const discovered = await provider.discoverAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -738,15 +724,11 @@ describe('SolAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const solProvider = new MockSolAccountProvider( + const provider = new MockSolAccountProvider( multichainMessenger, undefined, mocks.trace, ); - const provider = new AccountProviderWrapper( - multichainMessenger, - solProvider, - ); await expect( provider.discoverAccounts({ @@ -759,23 +741,68 @@ describe('SolAccountProvider', () => { }); }); - describe('isDisabled', () => { - it('returns false when the provider is enabled (default)', () => { - const { provider } = setup(); - expect(provider.isDisabled()).toBe(false); + describe('isEnabled config callback', () => { + it('provider is enabled by default when no isEnabled callback is provided', () => { + const accounts = [MOCK_SOL_ACCOUNT_1]; + const { provider } = setup({ accounts }); + expect(provider.getAccounts()).toStrictEqual(accounts); + }); + + it('blocks getAccounts when isEnabled returns false', () => { + const accounts = [MOCK_SOL_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(provider.getAccounts()).toStrictEqual([]); + }); + + it('throws on getAccount when isEnabled returns false', () => { + const accounts = [MOCK_SOL_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(() => provider.getAccount(MOCK_SOL_ACCOUNT_1.id)).toThrow( + 'Provider is disabled', + ); + }); + + it('blocks createAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); }); - it('returns true after setEnabled(false)', () => { - const { provider } = setup(); - provider.setEnabled(false); - expect(provider.isDisabled()).toBe(true); + it('blocks discoverAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); }); - it('returns false after re-enabling', () => { - const { provider } = setup(); - provider.setEnabled(false); - provider.setEnabled(true); - expect(provider.isDisabled()).toBe(false); + it('re-enables operations when isEnabled returns true again', () => { + const accounts = [MOCK_SOL_ACCOUNT_1]; + let enabled = false; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => enabled }), + }); + + expect(provider.getAccounts()).toStrictEqual([]); + + enabled = true; + expect(provider.getAccounts()).toStrictEqual(accounts); }); }); }); diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index b9bad3cc5cc..d02c8c8edd4 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -116,7 +116,7 @@ export class SolAccountProvider extends SnapAccountProvider { return account; } - async discoverAccounts({ + protected override async discoverBip44Accounts({ entropySource, groupIndex, }: { diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts index 832b843ceb0..053168c1b44 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts @@ -9,7 +9,6 @@ import type { import { SnapControllerState } from '@metamask/snaps-controllers'; import deepmerge from 'deepmerge'; -import { AccountProviderWrapper } from './AccountProviderWrapper'; import type { SnapAccountProviderConfig } from './SnapAccountProvider'; import { TRX_ACCOUNT_PROVIDER_DEFAULT_CONFIG, @@ -137,7 +136,7 @@ function setup({ accounts?: InternalAccount[]; config?: SnapAccountProviderConfig; } = {}): { - provider: AccountProviderWrapper; + provider: MockTrxAccountProvider; messenger: RootMessenger; keyring: MockTronKeyring; mocks: { @@ -207,14 +206,13 @@ function setup({ }); const multichainMessenger = getMultichainAccountServiceMessenger(messenger); - const trxProvider = new MockTrxAccountProvider( + const provider = new MockTrxAccountProvider( multichainMessenger, config, mockTrace, ); const accountIds = accounts.map((account) => account.id); - trxProvider.init(accountIds); - const provider = new AccountProviderWrapper(multichainMessenger, trxProvider); + provider.init(accountIds); return { provider, @@ -682,11 +680,7 @@ describe('TrxAccountProvider', () => { const multichainMessenger = getMultichainAccountServiceMessenger(messenger); // No trace callback (defaults to `traceFallback`). - const trxProvider = new MockTrxAccountProvider(multichainMessenger); - const provider = new AccountProviderWrapper( - multichainMessenger, - trxProvider, - ); + const provider = new MockTrxAccountProvider(multichainMessenger); const discovered = await provider.discoverAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -731,23 +725,68 @@ describe('TrxAccountProvider', () => { }); }); - describe('isDisabled', () => { - it('returns false when the provider is enabled (default)', () => { - const { provider } = setup(); - expect(provider.isDisabled()).toBe(false); + describe('isEnabled config callback', () => { + it('provider is enabled by default when no isEnabled callback is provided', () => { + const accounts = [MOCK_TRX_ACCOUNT_1]; + const { provider } = setup({ accounts }); + expect(provider.getAccounts()).toStrictEqual(accounts); + }); + + it('blocks getAccounts when isEnabled returns false', () => { + const accounts = [MOCK_TRX_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(provider.getAccounts()).toStrictEqual([]); }); - it('returns true after setEnabled(false)', () => { - const { provider } = setup(); - provider.setEnabled(false); - expect(provider.isDisabled()).toBe(true); + it('throws on getAccount when isEnabled returns false', () => { + const accounts = [MOCK_TRX_ACCOUNT_1]; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => false }), + }); + expect(() => provider.getAccount(MOCK_TRX_ACCOUNT_1.id)).toThrow( + 'Provider is disabled', + ); }); - it('returns false after re-enabling', () => { - const { provider } = setup(); - provider.setEnabled(false); - provider.setEnabled(true); - expect(provider.isDisabled()).toBe(false); + it('blocks createAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); + }); + + it('blocks discoverAccounts when isEnabled returns false', async () => { + const { provider } = setup({ + config: asConfig({ isEnabled: () => false }), + }); + const result = await provider.discoverAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(result).toStrictEqual([]); + }); + + it('re-enables operations when isEnabled returns true again', () => { + const accounts = [MOCK_TRX_ACCOUNT_1]; + let enabled = false; + const { provider } = setup({ + accounts, + config: asConfig({ isEnabled: () => enabled }), + }); + + expect(provider.getAccounts()).toStrictEqual([]); + + enabled = true; + expect(provider.getAccounts()).toStrictEqual(accounts); }); }); }); diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts index d95776266a0..7ab91c7a2d9 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts @@ -92,7 +92,7 @@ export class TrxAccountProvider extends SnapAccountProvider { }); } - async discoverAccounts({ + protected override async discoverBip44Accounts({ entropySource, groupIndex, }: { diff --git a/packages/multichain-account-service/src/providers/index.ts b/packages/multichain-account-service/src/providers/index.ts index 0504f99fea6..6041aab4e19 100644 --- a/packages/multichain-account-service/src/providers/index.ts +++ b/packages/multichain-account-service/src/providers/index.ts @@ -1,6 +1,5 @@ export * from './BaseBip44AccountProvider'; export * from './SnapAccountProvider'; -export * from './AccountProviderWrapper'; // Errors that can bubble up outside of provider calls. export { TimeoutError, isTimeoutError } from './utils'; diff --git a/packages/multichain-account-service/src/tests/providers.ts b/packages/multichain-account-service/src/tests/providers.ts index 9544c9f9780..78c5d87f2ca 100644 --- a/packages/multichain-account-service/src/tests/providers.ts +++ b/packages/multichain-account-service/src/tests/providers.ts @@ -5,7 +5,7 @@ import type { KeyringCapabilities, } from '@metamask/keyring-api'; -import { AccountProviderWrapper, EvmAccountProvider } from '../providers'; +import { EvmAccountProvider, SnapAccountProvider } from '../providers'; import { GroupIndexRange } from '../utils'; export type MockAccountProvider = { @@ -22,9 +22,6 @@ export type MockAccountProvider = { discoverAccounts: jest.Mock; isAccountCompatible: jest.Mock; getName: jest.Mock; - isEnabled: boolean; - isDisabled: jest.Mock; - setEnabled: jest.Mock; }; export function makeMockAccountProvider( @@ -53,9 +50,6 @@ export function makeMockAccountProvider( discoverAccounts: jest.fn(), isAccountCompatible: jest.fn(), getName: jest.fn(), - isDisabled: jest.fn(), - setEnabled: jest.fn(), - isEnabled: true, }; } @@ -75,11 +69,6 @@ export function setupBip44AccountProvider({ // of accounts. mocks.mockAccounts = accounts; mocks.accounts = new Set(accounts.map((account) => account.id)); - // Toggle enabled state only - mocks.setEnabled.mockImplementation((enabled: boolean) => { - mocks.isEnabled = enabled; - }); - mocks.isDisabled.mockImplementation(() => !mocks.isEnabled); const getAccounts = (): KeyringAccount[] => mocks.mockAccounts.filter((account) => @@ -108,7 +97,7 @@ export function setupBip44AccountProvider({ } if (index !== 0) { - Object.setPrototypeOf(mocks, AccountProviderWrapper.prototype); + Object.setPrototypeOf(mocks, SnapAccountProvider.prototype); } return mocks;