feat!: remove KeyringVersion + simplify KeyringRequestWithoutOrigin logic

packages/keyring-internal-api/CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **BREAKING:** Rename `KeyringRequestV1` type to `KeyringRequestWithoutOrigin` ([#423](https://github.com/MetaMask/accounts/pull/423))
+- **BREAKING:** Rename `toKeyringRequestV1` method to `toKeyringRequestWithoutOrigin` ([#423](https://github.com/MetaMask/accounts/pull/423))
+
+### Removed
+
+- **BREAKING:** Remove `KeyringVersion` support ([#423](https://github.com/MetaMask/accounts/pull/423))
+
 ## [9.1.1]
 
 ### Changed

packages/keyring-internal-api/src/compatibility/index.ts

@@ -1 +1 @@
-export * from './v1';
+export * from './keyring-request';

packages/keyring-internal-api/src/compatibility/v1.test.ts → packages/keyring-internal-api/src/compatibility/keyring-request.test.ts

@@ -1,9 +1,9 @@
 import { EthMethod, EthScope } from '@metamask/keyring-api';
 
-import { toKeyringRequestV1 } from './v1';
+import { toKeyringRequestWithoutOrigin } from './keyring-request';
 
 describe('v1', () => {
-  describe('toKeyringRequestV1', () => {
+  describe('toKeyringRequestWithoutOrigin', () => {
     const request = {
       id: 'mock-request-id',
       scope: EthScope.Mainnet,
@@ -17,7 +17,7 @@ describe('v1', () => {
     const { origin, ...requestV1 } = request;
 
     it('converts a keyring request to a keyring request v1', () => {
-      expect(toKeyringRequestV1(request)).toStrictEqual(requestV1);
+      expect(toKeyringRequestWithoutOrigin(request)).toStrictEqual(requestV1);
     });
   });
 });

packages/keyring-internal-api/src/compatibility/keyring-request.ts

@@ -0,0 +1,42 @@
+import type { KeyringRequest } from '@metamask/keyring-api';
+import {
+  KeyringRequestStruct,
+  KeyringResponseStruct,
+} from '@metamask/keyring-api';
+import { omit, type Infer } from '@metamask/superstruct';
+
+/**
+ * Keyring request without `origin` support.
+ */
+export const KeyringRequestWithoutOriginStruct = omit(KeyringRequestStruct, [
+  'origin',
+]);
+
+export type KeyringRequestWithoutOrigin = Infer<
+  typeof KeyringRequestWithoutOriginStruct
+>;
+
+/**
+ * Response to a call to `submitRequest` (v1).
+ */
+export const KeyringResponseWithoutOriginStruct = KeyringResponseStruct;
+
+export type KeyringResponseWithoutOrigin = Infer<
+  typeof KeyringResponseWithoutOriginStruct
+>;
+
+export const SubmitRequestResponseV1Struct = KeyringResponseWithoutOriginStruct;
+
+/**
+ * Converts a keyring request to a keyring request v1.
+ *
+ * @param request - A keyring request.
+ * @returns A keyring request v1.
+ */
+export function toKeyringRequestWithoutOrigin(
+  request: KeyringRequest,
+): KeyringRequestWithoutOrigin {
+  const { origin, ...requestV1 } = request;
+
+  return requestV1;
+}

packages/keyring-internal-api/src/index.ts

@@ -1,4 +1,3 @@
 export * from './compatibility';
 export type * from './eth';
 export * from './types';
-export * from './versions';

packages/keyring-internal-snap-client/CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bump `@metamask/snaps-sdk` from `^9.0.0` to `^10.3.0` ([#422](https://github.com/MetaMask/accounts/pull/422))
 - Bump `@metamask/snaps-utils` from `^11.0.0` to `^11.7.0` ([#422](https://github.com/MetaMask/accounts/pull/422))
 
+### Removed
+
+- **BREAKING:** Rename `submitRequestV1` method to `submitRequestWithoutOrigin` ([#423](https://github.com/MetaMask/accounts/pull/423))
+
 ## [8.0.1]
 
 ### Changed

packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.test.ts

@@ -3,7 +3,7 @@ import {
   EthScope,
   type KeyringAccount,
 } from '@metamask/keyring-api';
-import type { KeyringRequestV1 } from '@metamask/keyring-internal-api';
+import type { KeyringRequestWithoutOrigin } from '@metamask/keyring-internal-api';
 import type { SnapId } from '@metamask/snaps-sdk';
 
 import {
@@ -87,8 +87,8 @@ describe('KeyringInternalSnapClient', () => {
     });
   });
 
-  describe('submitRequestV1', () => {
-    const keyringRequest: KeyringRequestV1 = {
+  describe('submitRequestWithoutOrigin', () => {
+    const keyringRequest: KeyringRequestWithoutOrigin = {
       id: 'mock-request-id',
       scope: EthScope.Mainnet,
       account: MOCK_ACCOUNT.id,
@@ -109,7 +109,7 @@ describe('KeyringInternalSnapClient', () => {
       },
     };
 
-    it('calls the submitRequestV1 method', async () => {
+    it('calls the submitRequestWithoutOrigin method', async () => {
       const client = new KeyringInternalSnapClient({
         messenger: messenger as unknown as KeyringInternalSnapClientMessenger,
         snapId,
@@ -119,7 +119,7 @@ describe('KeyringInternalSnapClient', () => {
         pending: false,
         result: null,
       });
-      await client.submitRequestV1(keyringRequest);
+      await client.submitRequestWithoutOrigin(keyringRequest);
       expect(messenger.call).toHaveBeenCalledWith(
         'SnapController:handleRequest',
         request,

packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.ts

@@ -1,7 +1,7 @@
 import { KeyringRpcMethod } from '@metamask/keyring-api';
 import type {
-  KeyringRequestV1,
-  KeyringResponseV1,
+  KeyringRequestWithoutOrigin,
+  KeyringResponseWithoutOrigin,
 } from '@metamask/keyring-internal-api';
 import { SubmitRequestResponseV1Struct } from '@metamask/keyring-internal-api';
 import { KeyringClient, type Sender } from '@metamask/keyring-snap-client';
@@ -131,7 +131,9 @@ export class KeyringInternalSnapClient extends KeyringClient {
    * @param request - Keyring request.
    * @returns Keyring request's response.
    */
-  async submitRequestV1(request: KeyringRequestV1): Promise<KeyringResponseV1> {
+  async submitRequestWithoutOrigin(
+    request: KeyringRequestWithoutOrigin,
+  ): Promise<KeyringResponseWithoutOrigin> {
     return strictMask(
       await this.send({
         method: KeyringRpcMethod.SubmitRequest,

packages/keyring-snap-bridge/CHANGELOG.md

@@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bump `@metamask/snaps-sdk` from `^9.0.0` to `^10.3.0` ([#422](https://github.com/MetaMask/accounts/pull/422))
 - Bump `@metamask/snaps-utils` from `^11.0.0` to `^11.7.0` ([#422](https://github.com/MetaMask/accounts/pull/422))
 
+### Removed
+
+- **BREAKING:** Remove `KeyringVersion` support ([#423](https://github.com/MetaMask/accounts/pull/423))
+  - Both `getKeyringVersionFromPlatform`, `PLATFORM_VERSION_TO_KEYRING_VERSION` got removed too.
+- **BREAKING:** Rename `submitRequestV1` method to `submitRequestWithoutOrigin` ([#423](https://github.com/MetaMask/accounts/pull/423))
+
 ## [18.0.2]
 
 ### Fixed

packages/keyring-snap-bridge/src/SnapKeyring.ts

@@ -13,7 +13,6 @@ import type {
   ResolvedAccountAddress,
   CaipChainId,
   MetaMaskOptions,
-  KeyringRequest,
   KeyringResponse,
 } from '@metamask/keyring-api';
 import {
@@ -28,11 +27,8 @@ import {
   AccountTransactionsUpdatedEventStruct,
   AnyAccountType,
 } from '@metamask/keyring-api';
-import {
-  KeyringVersion,
-  toKeyringRequestV1,
-  type InternalAccount,
-} from '@metamask/keyring-internal-api';
+import type { InternalAccount } from '@metamask/keyring-internal-api';
+import { toKeyringRequestWithoutOrigin } from '@metamask/keyring-internal-api';
 import { KeyringInternalSnapClient } from '@metamask/keyring-internal-snap-client';
 import {
   type GetSelectedAccountsResponse,
@@ -69,6 +65,7 @@ import {
   getInternalOptionsOf,
   type SnapKeyringInternalOptions,
 } from './options';
+import { PLATFORM_VERSION_FOR_KEYRING_REQUEST_WITH_ORIGIN } from './platform-versions';
 import { SnapIdMap } from './SnapIdMap';
 import type {
   SnapKeyringEvents,
@@ -84,7 +81,6 @@ import {
   toJson,
   unique,
 } from './util';
-import { getKeyringVersionFromPlatform } from './versions';
 
 export const SNAP_KEYRING_TYPE = 'Snap Keyring';
 
@@ -248,19 +244,21 @@ export class SnapKeyring {
   }
 
   /**
-   * Gets keyring's version for a given Snap.
+   * Checks whether a Snap meets a minimum platform version.
    *
    * @param snapId - The Snap ID.
-   * @returns The Snap's keyring version.
+   * @param platformVersion - Platform version to check.
+   * @returns True if the Snap meets the minimum version, false otherwise.
    */
-  #getKeyringVersion(snapId: SnapId): KeyringVersion {
-    return getKeyringVersionFromPlatform((version: SemVerVersion) => {
-      return this.#messenger.call(
-        'SnapController:isMinimumPlatformVersion',
-        snapId,
-        version,
-      );
-    });
+  #isMinimumPlatformVersion(
+    snapId: SnapId,
+    platformVersion: SemVerVersion,
+  ): boolean {
+    return this.#messenger.call(
+      'SnapController:isMinimumPlatformVersion',
+      snapId,
+      platformVersion,
+    );
   }
 
   /**
@@ -1012,34 +1010,6 @@ export class SnapKeyring {
     });
   }
 
-  /**
-   * Submits a request to a Snap and fix-up the payload depending on the version currently supported.
-   *
-   * @param options - The options for the Snap request.
-   * @param options.version - The supported keyring version for the Snap.
-   * @param options.snapId - The Snap ID to submit the request to.
-   * @param options.request - The Snap request.
-   * @returns A promise that resolves to the keyring response from the Snap.
-   */
-  async #submitSnapRequestForVersion({
-    snapId,
-    version,
-    request,
-  }: {
-    snapId: SnapId;
-    version: KeyringVersion;
-    request: KeyringRequest;
-  }): Promise<KeyringResponse> {
-    // Get specific client for that Snap.
-    const client = this.#snapClient.withSnapId(snapId);
-
-    if (version === KeyringVersion.V1) {
-      return await client.submitRequestV1(toKeyringRequestV1(request));
-    }
-
-    return await client.submitRequest(request);
-  }
-
   /**
    * Submits a request to a Snap.
    *
@@ -1093,7 +1063,15 @@ export class SnapKeyring {
     );
 
     try {
-      const version = this.#getKeyringVersion(snapId);
+      // Snaps are expecting to receive the `origin` field after a specific
+      // platform version.
+      //
+      // We need to check the Snap platform version to know whether we can
+      // include it or not.
+      const useOrigin = this.#isMinimumPlatformVersion(
+        snapId,
+        PLATFORM_VERSION_FOR_KEYRING_REQUEST_WITH_ORIGIN,
+      );
 
       const request = {
         id: requestId,
@@ -1108,11 +1086,18 @@ export class SnapKeyring {
 
       log('Submit Snap request: ', request);
 
-      const response = await this.#submitSnapRequestForVersion({
-        version,
-        snapId,
-        request,
-      });
+      // Get specific client for that Snap.
+      const client = this.#snapClient.withSnapId(snapId);
+
+      let response: KeyringResponse;
+      if (useOrigin) {
+        response = await client.submitRequest(request);
+      } else {
+        // Legacy keyring request did not support the `origin` field.
+        response = await client.submitRequestWithoutOrigin(
+          toKeyringRequestWithoutOrigin(request),
+        );
+      }
 
       // Some methods, like the ones used to prepare and patch user operations,
       // require the Snap to answer synchronously in order to work with the

packages/keyring-snap-bridge/src/platform-versions.ts

@@ -0,0 +1,5 @@
+import type { SemVerVersion } from '@metamask/utils';
+
+/** Introduction of `KeyringRequest.origin`. */
+export const PLATFORM_VERSION_FOR_KEYRING_REQUEST_WITH_ORIGIN: SemVerVersion =
+  '7.0.0' as SemVerVersion;