feat(btc-provider): accept P2TR accounts and add testnet4

packages/multichain-account-service/CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Accept P2TR (taproot) accounts as compatible Bitcoin accounts in `BtcAccountProvider`
+- Add testnet4 (`BtcScope.Testnet4`) to Bitcoin provider capabilities and account discovery
+
+### Fixed
+
+- Guard against accounts with empty scopes during resync to prevent undefined scope in re-creation
+- Propagate re-creation failure after keyring removal to prevent silent state inconsistency
+- Align discovery scopes with advertised capabilities (include `BtcScope.Testnet`)
+
 ### Changed
 
 - **BREAKING:** The service messenger now requires the `SnapAccountService:ensureReady` action to be declared ([#8715](https://github.com/MetaMask/core/pull/8715))

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

@@ -1,6 +1,10 @@
 import { isBip44Account } from '@metamask/account-api';
 import type { SnapKeyring } from '@metamask/eth-snap-keyring';
-import { AccountCreationType, BtcAccountType } from '@metamask/keyring-api';
+import {
+  AccountCreationType,
+  BtcAccountType,
+  BtcScope,
+} from '@metamask/keyring-api';
 import type { KeyringMetadata } from '@metamask/keyring-controller';
 import type {
   EthKeyring,
@@ -53,6 +57,8 @@ class MockBtcKeyring {
     this.accounts = accounts;
   }
 
+  removeAccount = jest.fn().mockResolvedValue(undefined);
+
   #getIndexFromDerivationPath(derivationPath: string): number {
     // eslint-disable-next-line prefer-regex-literals
     const derivationPathIndexRegex = new RegExp(
@@ -295,6 +301,14 @@ describe('BtcAccountProvider', () => {
     expect(provider.isAccountCompatible(account)).toBe(true);
   });
 
+  it('returns true if a P2TR account is compatible', () => {
+    const account = MOCK_BTC_P2TR_ACCOUNT_1;
+    const { provider } = setup({
+      accounts: [account],
+    });
+    expect(provider.isAccountCompatible(account)).toBe(true);
+  });
+
   it('returns false if an account is not compatible', () => {
     const account = MOCK_HD_ACCOUNT_1;
     const { provider } = setup({
@@ -661,6 +675,32 @@ describe('BtcAccountProvider', () => {
     ).toStrictEqual([]);
   });
 
+  it('discovers accounts across all advertised scopes', async () => {
+    const { provider, mocks } = setup({
+      accounts: [],
+    });
+
+    mocks.handleRequest.mockReturnValue([MOCK_BTC_P2TR_DISCOVERED_ACCOUNT_1]);
+
+    await provider.discoverAccounts({
+      entropySource: MOCK_HD_KEYRING_1.metadata.id,
+      groupIndex: 0,
+    });
+
+    // Verify discoverAccounts was called with all three scopes matching
+    // the provider's advertised capabilities.
+    expect(mocks.handleRequest).toHaveBeenCalledWith(
+      expect.objectContaining({
+        request: expect.objectContaining({
+          method: 'keyring_discoverAccounts',
+          params: expect.objectContaining({
+            scopes: [BtcScope.Mainnet, BtcScope.Testnet, BtcScope.Testnet4],
+          }),
+        }),
+      }),
+    );
+  });
+
   describe('trace functionality', () => {
     it('calls trace callback during account discovery', async () => {
       const { provider, mocks } = setup({
@@ -742,6 +782,155 @@ describe('BtcAccountProvider', () => {
     });
   });
 
+  describe('resyncAccounts', () => {
+    it('re-creates a P2TR account preserving the address type', async () => {
+      const account = {
+        ...MOCK_BTC_P2TR_ACCOUNT_1,
+        metadata: {
+          ...MOCK_BTC_P2TR_ACCOUNT_1.metadata,
+          snap: {
+            ...MOCK_BTC_P2TR_ACCOUNT_1.metadata.snap,
+            id: BtcAccountProvider.BTC_SNAP_ID,
+          },
+        },
+      };
+      const { provider, mocks } = setup({
+        accounts: [account],
+      });
+
+      // Snap reports no accounts — triggers the re-creation path.
+      mocks.handleRequest.mockReturnValue([]);
+
+      await provider.resyncAccounts([account]);
+
+      expect(mocks.keyring.createAccount).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          addressType: BtcAccountType.P2tr,
+          scope: BtcScope.Testnet,
+        }),
+        expect.anything(),
+      );
+    });
+
+    it('re-creates a testnet4 account preserving the scope', async () => {
+      const account = {
+        ...MOCK_BTC_P2WPKH_ACCOUNT_1,
+        scopes: [BtcScope.Testnet4],
+        metadata: {
+          ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata,
+          snap: {
+            ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata.snap,
+            id: BtcAccountProvider.BTC_SNAP_ID,
+          },
+        },
+      };
+      const { provider, mocks } = setup({
+        accounts: [account],
+      });
+
+      mocks.handleRequest.mockReturnValue([]);
+
+      await provider.resyncAccounts([account]);
+
+      expect(mocks.keyring.createAccount).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          addressType: BtcAccountType.P2wpkh,
+          scope: BtcScope.Testnet4,
+        }),
+        expect.anything(),
+      );
+    });
+
+    it('does not alter a P2WPKH mainnet account during resync', async () => {
+      const account = {
+        ...MOCK_BTC_P2WPKH_ACCOUNT_1,
+        metadata: {
+          ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata,
+          snap: {
+            ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata.snap,
+            id: BtcAccountProvider.BTC_SNAP_ID,
+          },
+        },
+      };
+      const { provider, mocks } = setup({
+        accounts: [account],
+      });
+
+      mocks.handleRequest.mockReturnValue([]);
+
+      await provider.resyncAccounts([account]);
+
+      expect(mocks.keyring.createAccount).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          addressType: BtcAccountType.P2wpkh,
+          scope: BtcScope.Mainnet,
+        }),
+        expect.anything(),
+      );
+    });
+
+    it('reports error when account has no scopes during resync', async () => {
+      const account = {
+        ...MOCK_BTC_P2WPKH_ACCOUNT_1,
+        scopes: [] as string[],
+        metadata: {
+          ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata,
+          snap: {
+            ...MOCK_BTC_P2WPKH_ACCOUNT_1.metadata.snap,
+            id: BtcAccountProvider.BTC_SNAP_ID,
+          },
+        },
+      };
+      const { provider, mocks } = setup({
+        accounts: [account],
+      });
+
+      // Snap reports no accounts — triggers re-creation path.
+      mocks.handleRequest.mockReturnValue([]);
+
+      // Should not throw — error is reported but swallowed by the outer catch.
+      await provider.resyncAccounts([account]);
+
+      // createAccount should never have been called.
+      expect(mocks.keyring.createAccount).not.toHaveBeenCalled();
+    });
+
+    it('reports error when re-creation fails after keyring removal', async () => {
+      const account = {
+        ...MOCK_BTC_P2TR_ACCOUNT_1,
+        metadata: {
+          ...MOCK_BTC_P2TR_ACCOUNT_1.metadata,
+          snap: {
+            ...MOCK_BTC_P2TR_ACCOUNT_1.metadata.snap,
+            id: BtcAccountProvider.BTC_SNAP_ID,
+          },
+        },
+      };
+      const { provider, keyring, mocks } = setup({
+        accounts: [account],
+      });
+
+      // Snap reports no accounts — triggers re-creation path.
+      mocks.handleRequest.mockReturnValue([]);
+
+      // createAccount will throw after removeAccount succeeds.
+      mocks.keyring.createAccount.mockRejectedValue(
+        new Error('Snap crashed during re-creation'),
+      );
+
+      // Should not throw — the outer catch swallows it.
+      await provider.resyncAccounts([account]);
+
+      // removeAccount was called (account was removed from keyring).
+      expect(keyring.removeAccount).toHaveBeenCalledWith(account.address);
+      // createAccount was attempted.
+      expect(mocks.keyring.createAccount).toHaveBeenCalled();
+    });
+  });
+
   describe('isDisabled', () => {
     it('returns false when the provider is enabled (default)', () => {
       const { provider } = setup();