fix(keyring-api)!: use CaipAccountId for ResolveAccountAddress.address (

#186) We forgot to mention this in SIP-26, but the `address` being returned by `resolveAccountAddress` must use CAIP-10 format. This is already being checked by the consumers of this method, but it's better that we catch any errors related to this at API level.
MetaMask · Feb 4, 2025 · f378b32 · f378b32
1 parent ed24f81
commit f378b32
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 12 deletions.
diff --git a/packages/keyring-api/CHANGELOG.md b/packages/keyring-api/CHANGELOG.md
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Re-export `CaipAccountId` type and struct ([#186](https://github.com/MetaMask/accounts/pull/186))
+
+### Changed
+
+- **BREAKING:** Use `CaipAccountId` for `ResolvedAccountAddress.address` ([#186](https://github.com/MetaMask/accounts/pull/186))
+  - This was missing from SIP-26, but we expect this address to be CAIP-10 compliant.
+
 ## [16.1.0]
 
 ### Added

diff --git a/packages/keyring-api/src/api/address.test.ts b/packages/keyring-api/src/api/address.test.ts
@@ -0,0 +1,45 @@
+import { assert } from '@metamask/superstruct';
+
+import { ResolvedAccountAddressStruct } from './address';
+import type { ResolvedAccountAddress } from './address';
+import type { CaipAccountId } from './caip';
+import { BtcScope } from '../btc';
+import { EthScope } from '../eth';
+import { SolScope } from '../sol';
+
+const MOCK_ETH_ADDRESS = '0x6431726EEE67570BF6f0Cf892aE0a3988F03903F';
+const MOCK_BTC_ADDRESS = 'bc1qwl8399fz829uqvqly9tcatgrgtwp3udnhxfq4k';
+const MOCK_SOL_ADDRESS = '7EcDhSYGxXyscszYEp35KHN8vvw3svAuLKTzXwCFLtV';
+
+describe('ResolveAccountAddress', () => {
+  it.each([
+    `${EthScope.Eoa}:${MOCK_ETH_ADDRESS}`,
+    `${BtcScope.Mainnet}:${MOCK_BTC_ADDRESS}`,
+    `${SolScope.Mainnet}:${MOCK_SOL_ADDRESS}`,
+  ] as CaipAccountId[])(
+    'allows CAIP-10 account ID: %s',
+    (address: CaipAccountId) => {
+      const resolvedAddress: ResolvedAccountAddress = {
+        address,
+      };
+      expect(() =>
+        assert(resolvedAddress, ResolvedAccountAddressStruct),
+      ).not.toThrow();
+    },
+  );
+
+  it.each([MOCK_ETH_ADDRESS, MOCK_BTC_ADDRESS, MOCK_SOL_ADDRESS])(
+    'throws an error if address is not a CAIP-10 account ID: %s',
+    (address: string) => {
+      const resolvedAddress: ResolvedAccountAddress = {
+        // We type-cast to force the error.
+        address: address as CaipAccountId,
+      };
+      expect(() =>
+        assert(resolvedAddress, ResolvedAccountAddressStruct),
+      ).toThrow(
+        `At path: address -- Expected a value of type \`CaipAccountId\`, but received: \`"${address}"\``,
+      );
+    },
+  );
+});
diff --git a/packages/keyring-api/src/api/address.ts b/packages/keyring-api/src/api/address.ts
@@ -1,14 +1,15 @@
 import type { Infer } from '@metamask/superstruct';
-import { object, string } from '@metamask/superstruct';
+import { object } from '@metamask/superstruct';
+import { CaipAccountIdStruct } from '@metamask/utils';
 
 /**
  * An account's address that has been resolved from a signing request.
  */
 export const ResolvedAccountAddressStruct = object({
   /**
-   * Account main address.
+   * Account main address (CAIP-10 account ID).
    */
-  address: string(),
+  address: CaipAccountIdStruct,
 });
 
 /**

diff --git a/packages/keyring-api/src/api/caip.ts b/packages/keyring-api/src/api/caip.ts
@@ -1,22 +1,31 @@
 // istanbul ignore file
 
 import {
+  CaipAccountIdStruct,
   CaipAssetIdStruct,
   CaipAssetTypeStruct,
   CaipAssetTypeOrIdStruct,
   CaipChainIdStruct,
 } from '@metamask/utils';
 import type {
+  CaipAccountId,
   CaipAssetId,
   CaipAssetType,
   CaipAssetTypeOrId,
   CaipChainId,
 } from '@metamask/utils';
 
 export {
+  CaipAccountIdStruct,
   CaipAssetIdStruct,
   CaipAssetTypeStruct,
   CaipAssetTypeOrIdStruct,
   CaipChainIdStruct,
 };
-export type { CaipAssetId, CaipAssetType, CaipAssetTypeOrId, CaipChainId };
+export type {
+  CaipAccountId,
+  CaipAssetId,
+  CaipAssetType,
+  CaipAssetTypeOrId,
+  CaipChainId,
+};
diff --git a/packages/keyring-api/src/rpc.ts b/packages/keyring-api/src/rpc.ts
@@ -26,6 +26,7 @@ import {
   KeyringResponseStruct,
   TransactionsPageStruct,
   PaginationStruct,
+  CaipAccountIdStruct,
 } from './api';
 
 /**
@@ -201,7 +202,7 @@ export type ResolveAccountAddressRequest = Infer<
 
 export const ResolveAccountAddressResponseStruct = nullable(
   object({
-    address: string(),
+    address: CaipAccountIdStruct,
   }),
 );
 

diff --git a/packages/keyring-snap-bridge/src/SnapKeyring.test.ts b/packages/keyring-snap-bridge/src/SnapKeyring.test.ts
@@ -1761,11 +1761,11 @@ describe('SnapKeyring', () => {
   });
 
   describe('resolveAccountAddress', () => {
-    const address = '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb';
     const scope = toCaipChainId(
       KnownCaipNamespace.Eip155,
       executionContext.chainId,
     );
+    const address = '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb';
     const request: JsonRpcRequest = {
       id: '3d8a0bda-285c-4551-abe8-f52af39d3095',
       jsonrpc: '2.0',
@@ -1777,17 +1777,18 @@ describe('SnapKeyring', () => {
     };
 
     it('returns a resolved address', async () => {
-      mockMessenger.handleRequest.mockReturnValueOnce({
-        address,
-      });
+      const mockResponse = {
+        address: `${scope}:${address}`,
+      };
+      mockMessenger.handleRequest.mockReturnValueOnce(mockResponse);
 
       const resolved = await keyring.resolveAccountAddress(
         snapId,
         scope,
         request,
       );
 
-      expect(resolved).toStrictEqual({ address });
+      expect(resolved).toStrictEqual(mockResponse);
       expect(mockMessenger.handleRequest).toHaveBeenCalledWith({
         handler: 'onKeyringRequest',
         origin: 'metamask',

diff --git a/packages/keyring-snap-client/src/KeyringClient.test.ts b/packages/keyring-snap-client/src/KeyringClient.test.ts
@@ -429,7 +429,7 @@ describe('KeyringClient', () => {
 
     it('should send a request to resolve an account address from a signing request and return the response', async () => {
       const expectedResponse = {
-        address: 'tb1qspc3kwj3zfnltjpucn7ugahr8lhrgg86wzpvs3',
+        address: `${scope}:tb1qspc3kwj3zfnltjpucn7ugahr8lhrgg86wzpvs3`,
       };
 
       mockSender.send.mockResolvedValue(expectedResponse);
@@ -472,7 +472,7 @@ describe('KeyringClient', () => {
       await expect(
         keyring.resolveAccountAddress(scope, request),
       ).rejects.toThrow(
-        'At path: address -- Expected a string, but received: undefined',
+        'At path: address -- Expected a value of type `CaipAccountId`, but received: `undefined`',
       );
     });
   });