diff --git a/packages/keyring-internal-api/CHANGELOG.md b/packages/keyring-internal-api/CHANGELOG.md index 9adb0d738..8ddda0409 100644 --- a/packages/keyring-internal-api/CHANGELOG.md +++ b/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 diff --git a/packages/keyring-internal-api/src/compatibility/index.ts b/packages/keyring-internal-api/src/compatibility/index.ts index 5b98253d9..1dbb7e2da 100644 --- a/packages/keyring-internal-api/src/compatibility/index.ts +++ b/packages/keyring-internal-api/src/compatibility/index.ts @@ -1 +1 @@ -export * from './v1'; +export * from './keyring-request'; diff --git a/packages/keyring-internal-api/src/compatibility/v1.test.ts b/packages/keyring-internal-api/src/compatibility/keyring-request.test.ts similarity index 70% rename from packages/keyring-internal-api/src/compatibility/v1.test.ts rename to packages/keyring-internal-api/src/compatibility/keyring-request.test.ts index b4d1fc4b9..a63270031 100644 --- a/packages/keyring-internal-api/src/compatibility/v1.test.ts +++ b/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); }); }); }); diff --git a/packages/keyring-internal-api/src/compatibility/keyring-request.ts b/packages/keyring-internal-api/src/compatibility/keyring-request.ts new file mode 100644 index 000000000..a1833fd94 --- /dev/null +++ b/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; +} diff --git a/packages/keyring-internal-api/src/compatibility/v1.ts b/packages/keyring-internal-api/src/compatibility/v1.ts deleted file mode 100644 index d51a09463..000000000 --- a/packages/keyring-internal-api/src/compatibility/v1.ts +++ /dev/null @@ -1,34 +0,0 @@ -import type { KeyringRequest } from '@metamask/keyring-api'; -import { - KeyringRequestStruct, - KeyringResponseStruct, -} from '@metamask/keyring-api'; -import { omit, type Infer } from '@metamask/superstruct'; - -/** - * Keyring request (v1). - */ -export const KeyringRequestV1Struct = omit(KeyringRequestStruct, ['origin']); - -export type KeyringRequestV1 = Infer; - -/** - * Response to a call to `submitRequest` (v1). - */ -export const KeyringResponseV1Struct = KeyringResponseStruct; - -export type KeyringResponseV1 = Infer; - -export const SubmitRequestResponseV1Struct = KeyringResponseV1Struct; - -/** - * Converts a keyring request to a keyring request v1. - * - * @param request - A keyring request. - * @returns A keyring request v1. - */ -export function toKeyringRequestV1(request: KeyringRequest): KeyringRequestV1 { - const { origin, ...requestV1 } = request; - - return requestV1; -} diff --git a/packages/keyring-internal-api/src/index.ts b/packages/keyring-internal-api/src/index.ts index 90f0544fa..9dc07b9a6 100644 --- a/packages/keyring-internal-api/src/index.ts +++ b/packages/keyring-internal-api/src/index.ts @@ -1,4 +1,3 @@ export * from './compatibility'; export type * from './eth'; export * from './types'; -export * from './versions'; diff --git a/packages/keyring-internal-api/src/versions.ts b/packages/keyring-internal-api/src/versions.ts deleted file mode 100644 index 4a42f19cd..000000000 --- a/packages/keyring-internal-api/src/versions.ts +++ /dev/null @@ -1,12 +0,0 @@ -export enum KeyringVersion { - /** Default. */ - V1 = 'v1', - - /** - * Introduction of `KeyringRequest.origin`. - * - * Snap will now receive the `origin` as part of a `KeyringRequest` when `submitRequest` is invoked. - * We also expect Snaps to display this `origin` in their confirmation screens (if any). - */ - V2 = 'v2', -} diff --git a/packages/keyring-internal-snap-client/CHANGELOG.md b/packages/keyring-internal-snap-client/CHANGELOG.md index 91cef694e..dbc6c6ed0 100644 --- a/packages/keyring-internal-snap-client/CHANGELOG.md +++ b/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 diff --git a/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.test.ts b/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.test.ts index 525ec0f56..f11155908 100644 --- a/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.test.ts +++ b/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, diff --git a/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.ts b/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.ts index 49b2c7917..d3f950847 100644 --- a/packages/keyring-internal-snap-client/src/KeyringInternalSnapClient.ts +++ b/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 { + async submitRequestWithoutOrigin( + request: KeyringRequestWithoutOrigin, + ): Promise { return strictMask( await this.send({ method: KeyringRpcMethod.SubmitRequest, diff --git a/packages/keyring-snap-bridge/CHANGELOG.md b/packages/keyring-snap-bridge/CHANGELOG.md index 30baa95b1..0be31ab76 100644 --- a/packages/keyring-snap-bridge/CHANGELOG.md +++ b/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 diff --git a/packages/keyring-snap-bridge/src/SnapKeyring.ts b/packages/keyring-snap-bridge/src/SnapKeyring.ts index 5e20ef214..986c84554 100644 --- a/packages/keyring-snap-bridge/src/SnapKeyring.ts +++ b/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 { - // 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 diff --git a/packages/keyring-snap-bridge/src/platform-versions.ts b/packages/keyring-snap-bridge/src/platform-versions.ts new file mode 100644 index 000000000..7fffa0665 --- /dev/null +++ b/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; diff --git a/packages/keyring-snap-bridge/src/versions.test.ts b/packages/keyring-snap-bridge/src/versions.test.ts deleted file mode 100644 index 0c39b1579..000000000 --- a/packages/keyring-snap-bridge/src/versions.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { KeyringVersion } from '@metamask/keyring-internal-api'; -import { isValidSemVerVersion, type SemVerVersion } from '@metamask/utils'; - -import { - getKeyringVersionFromPlatform, - PLATFORM_VERSION_TO_KEYRING_VERSION, -} from './versions'; - -describe('getKeyringVersionFromPlatform', () => { - it('gets the keyring version v1 as default', () => { - const isSupportedVersion = (): boolean => false; - - expect(getKeyringVersionFromPlatform(isSupportedVersion)).toBe( - KeyringVersion.V1, - ); - }); - - it('gets the keyring version v2 if the platform version is 7.0.0', () => { - const isSupportedVersion = (version: SemVerVersion): boolean => { - return version === '7.0.0'; - }; - - expect(getKeyringVersionFromPlatform(isSupportedVersion)).toBe( - KeyringVersion.V2, - ); - }); -}); - -describe('PLATFORM_VERSION_TO_KEYRING_VERSION', () => { - it.each(PLATFORM_VERSION_TO_KEYRING_VERSION)( - 'is a valid semver version: %s', - // `PLATFORM_VERSION_TO_KEYRING_VERSION` items are flattened by `it.each`, no need - // to destructure the tuple here. - (version) => { - expect(isValidSemVerVersion(version)).toBe(true); - }, - ); -}); diff --git a/packages/keyring-snap-bridge/src/versions.ts b/packages/keyring-snap-bridge/src/versions.ts deleted file mode 100644 index b09c783e6..000000000 --- a/packages/keyring-snap-bridge/src/versions.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { KeyringVersion } from '@metamask/keyring-internal-api'; -import type { SemVerVersion } from '@metamask/utils'; - -/** - * Mapping of the Snap platform version to its `KeyringVersion` equivalent. - * - * NOTE: We use an array here to preserve ordering of each versions. - */ -export const PLATFORM_VERSION_TO_KEYRING_VERSION = [ - // ! NOTE: This versions needs to be sorted in a descending order (highest platform version first). - - // Introduction of `KeyringRequest.origin`. - ['7.0.0', KeyringVersion.V2], -] as const; - -/** - * Gets keyring's version for a given Snap. - * - * @param isSupportedVersion - Predicate to check if the version is supported. - * @returns The Snap's keyring version. - */ -export function getKeyringVersionFromPlatform( - isSupportedVersion: (version: SemVerVersion) => boolean, -): KeyringVersion { - for (const [ - platformVersion, - keyringVersion, - ] of PLATFORM_VERSION_TO_KEYRING_VERSION) { - // NOTE: We are type-casting, but we have a unit tests that make sure all - // versions are following the semver spec. - if (isSupportedVersion(platformVersion as SemVerVersion)) { - return keyringVersion; - } - } - return KeyringVersion.V1; -}