feat: make the KeyringAccount type "abstract" (#311)

* feat: make `KeyringAccount` generic again * fix: fix circular dependency * fix: reinstate ERC-4337 methods to the `EthMethod` enum * feat: add type tests to ETH- and BTC-specific account types * chore: revert change to `jest.config.js` * chore: add missing empty line at the end of the file * chore: apply linter * chore: move fix `EthErc4337Method` -> `EthMethod` to a different PR * test: put `api.test.ts` file back * chore: update comments * chore: destructure schema instead of using `assign` * chore: update jsdoc * chore: fix typo Co-authored-by: Charly Chevalier <[email protected]> * chore: revert `enum` to `literal` --------- Co-authored-by: Charly Chevalier <[email protected]>
MetaMask · May 27, 2024 · 4dc235a · 4dc235a
1 parent 426e83a
commit 4dc235a
Show file tree

Hide file tree

Showing 15 changed files with 151 additions and 174 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -0,0 +1,3 @@
+{
+  "cSpell.words": ["bech32", "p2wpkh", "sendmany"]
+}
diff --git a/src/api.test.ts b/src/api.test.ts
@@ -1,8 +1,10 @@
 import { assert } from 'superstruct';
 
-import { KeyringAccountStruct, KeyringAccountStructs } from './api'; // Import from `index.ts` to test the public API
+import { KeyringAccountStruct } from './api';
 
-const supportedKeyringAccountTypes = Object.keys(KeyringAccountStructs)
+const supportedKeyringAccountTypes = Object.keys(
+  KeyringAccountStruct.schema.type.schema,
+)
   .map((type: string) => `"${type}"`)
   .join(',');
 

diff --git a/src/api.ts b/src/api.ts
@@ -1,53 +1,42 @@
 import type { Json } from '@metamask/utils';
 import { JsonStruct } from '@metamask/utils';
-import type { Infer, Struct } from 'superstruct';
-import {
-  enums,
-  array,
-  define,
-  validate,
-  literal,
-  record,
-  string,
-  union,
-  mask,
-} from 'superstruct';
-
-import type { StaticAssertAbstractAccount } from './base-types';
-import type { BtcP2wpkhAccount } from './btc';
-import { BtcP2wpkhAccountStruct, BtcAccountType } from './btc';
-import type { EthEoaAccount, EthErc4337Account } from './eth';
-import {
-  EthEoaAccountStruct,
-  EthErc4337AccountStruct,
-  EthAccountType,
-} from './eth';
+import type { Infer } from 'superstruct';
+import { enums, array, literal, record, string, union } from 'superstruct';
+
 import { exactOptional, object } from './superstruct';
 import { UuidStruct } from './utils';
 
+// ! The `*AccountType` enums defined below should be kept in this file to
+// ! avoid circular dependencies when the API is used by other files.
+
 /**
- * Type of supported accounts.
+ * Supported Ethereum account types.
  */
-export type KeyringAccounts = StaticAssertAbstractAccount<
-  EthEoaAccount | EthErc4337Account | BtcP2wpkhAccount
->;
+export enum EthAccountType {
+  Eoa = 'eip155:eoa',
+  Erc4337 = 'eip155:erc4337',
+}
 
 /**
- * Mapping between account types and their matching `superstruct` schema.
+ * Supported Bitcoin account types.
  */
-export const KeyringAccountStructs: Record<
-  string,
-  Struct<EthEoaAccount> | Struct<EthErc4337Account> | Struct<BtcP2wpkhAccount>
-> = {
-  [`${EthAccountType.Eoa}`]: EthEoaAccountStruct,
-  [`${EthAccountType.Erc4337}`]: EthErc4337AccountStruct,
-  [`${BtcAccountType.P2wpkh}`]: BtcP2wpkhAccountStruct,
-};
+export enum BtcAccountType {
+  P2wpkh = 'bip122:p2wpkh',
+}
 
 /**
- * Base type for `KeyringAccount` as a `superstruct.object`.
+ * A struct which represents a Keyring account object. It is abstract enough to
+ * be used with any blockchain. Specific blockchain account types should extend
+ * this struct.
+ *
+ * See {@link KeyringAccount}.
  */
-export const BaseKeyringAccountStruct = object({
+export const KeyringAccountStruct = object({
+  /**
+   * Account ID (UUIDv4).
+   */
+  id: UuidStruct,
+
   /**
    * Account type.
    */
@@ -56,38 +45,26 @@ export const BaseKeyringAccountStruct = object({
     `${EthAccountType.Erc4337}`,
     `${BtcAccountType.P2wpkh}`,
   ]),
-});
 
-/**
- * Account as a `superstruct.object`.
- *
- * See {@link KeyringAccount}.
- */
-export const KeyringAccountStruct = define<KeyringAccounts>(
-  // We do use a custom `define` for this type to avoid having to use a `union` since error
-  // messages are a bit confusing.
-  //
-  // Doing manual validation allows us to use the "concrete" type of each supported acounts giving
-  // use a much nicer message from `superstruct`.
-  'KeyringAccount',
-  (value: unknown) => {
-    // This will also raise if `value` does not match any of the supported account types!
-    const account = mask(value, BaseKeyringAccountStruct);
-
-    // At this point, we know that `value.type` can be used as an index for `KeyringAccountStructs`
-    const [error] = validate(
-      value,
-      KeyringAccountStructs[account.type] as Struct,
-    );
-
-    return error ?? true;
-  },
-);
+  /**
+   * Account main address.
+   */
+  address: string(),
+
+  /**
+   * Account options.
+   */
+  options: record(string(), JsonStruct),
+
+  /**
+   * Account supported methods.
+   */
+  methods: array(string()),
+});
 
 /**
- * Account object.
- *
- * Represents an account with its properties and capabilities.
+ * Keyring Account type represents an account and its properties from the
+ * point of view of the keyring.
  */
 export type KeyringAccount = Infer<typeof KeyringAccountStruct>;
 

diff --git a/src/base-types.ts b/src/base-types.ts
diff --git a/src/btc/types.test-d.ts b/src/btc/types.test-d.ts
@@ -0,0 +1,7 @@
+import type { KeyringAccount } from '../api';
+import type { Extends } from '../utils';
+import { expectTrue } from '../utils';
+import type { BtcP2wpkhAccount } from './types';
+
+// `BtcP2wpkhAccount` extends `KeyringAccount`
+expectTrue<Extends<BtcP2wpkhAccount, KeyringAccount>>();
diff --git a/src/btc/types.ts b/src/btc/types.ts
@@ -1,8 +1,9 @@
 import { bech32 } from 'bech32';
 import type { Infer } from 'superstruct';
-import { object, string, array, enums, literal, refine } from 'superstruct';
+import { string, array, enums, refine, literal } from 'superstruct';
 
-import { BaseAccount } from '../base-types';
+import { KeyringAccountStruct, BtcAccountType } from '../api';
+import { object } from '../superstruct';
 
 export const BtcP2wpkhAddressStruct = refine(
   string(),
@@ -27,15 +28,13 @@ export enum BtcMethod {
   SendMany = 'btc_sendmany',
 }
 
-/**
- * Supported Bitcoin account types.
- */
-export enum BtcAccountType {
-  P2wpkh = 'bip122:p2wpkh',
-}
-
 export const BtcP2wpkhAccountStruct = object({
-  ...BaseAccount,
+  ...KeyringAccountStruct.schema,
+
+  /**
+   * Account address.
+   */
+  address: BtcP2wpkhAddressStruct,
 
   /**
    * Account type.

diff --git a/src/eth/types.test-d.ts b/src/eth/types.test-d.ts
@@ -1,7 +1,11 @@
 import { expectAssignable, expectNotAssignable } from 'tsd';
 
+import type { KeyringAccount } from '../api';
+import { EthAccountType } from '../api';
+import type { Extends } from '../utils';
+import { expectTrue } from '../utils';
 import type { EthEoaAccount, EthErc4337Account } from './types';
-import { EthAccountType, EthErc4337Method, EthMethod } from './types';
+import { EthMethod, EthErc4337Method } from './types';
 
 const id = '606a7759-b0fb-48e4-9874-bab62ff8e7eb';
 const address = '0x000';
@@ -109,3 +113,9 @@ expectNotAssignable<EthErc4337Account>({
     `${EthErc4337Method.SignUserOperation}`,
   ],
 });
+
+// `EthEoaAccount` extends `KeyringAccount`
+expectTrue<Extends<EthEoaAccount, KeyringAccount>>();
+
+// `EthErc4337Account` extends `KeyringAccount`
+expectTrue<Extends<EthErc4337Account, KeyringAccount>>();
diff --git a/src/eth/types.ts b/src/eth/types.ts
@@ -1,8 +1,8 @@
 import type { Infer } from 'superstruct';
-import { object, array, enums, literal } from 'superstruct';
+import { array, enums, literal } from 'superstruct';
 
-import { BaseAccount } from '../base-types';
-import { definePattern } from '../superstruct';
+import { EthAccountType, KeyringAccountStruct } from '../api';
+import { object, definePattern } from '../superstruct';
 
 export const EthBytesStruct = definePattern('EthBytes', /^0x[0-9a-f]*$/iu);
 
@@ -39,16 +39,13 @@ export enum EthErc4337Method {
   SignUserOperation = 'eth_signUserOperation',
 }
 
-/**
- * Supported Ethereum account types.
- */
-export enum EthAccountType {
-  Eoa = 'eip155:eoa',
-  Erc4337 = 'eip155:erc4337',
-}
-
 export const EthEoaAccountStruct = object({
-  ...BaseAccount,
+  ...KeyringAccountStruct.schema,
+
+  /**
+   * Account address.
+   */
+  address: EthAddressStruct,
 
   /**
    * Account type.
@@ -73,7 +70,12 @@ export const EthEoaAccountStruct = object({
 export type EthEoaAccount = Infer<typeof EthEoaAccountStruct>;
 
 export const EthErc4337AccountStruct = object({
-  ...BaseAccount,
+  ...KeyringAccountStruct.schema,
+
+  /**
+   * Account address.
+   */
+  address: EthAddressStruct,
 
   /**
    * Account type.

diff --git a/src/eth/utils.test.ts b/src/eth/utils.test.ts
@@ -1,5 +1,4 @@
-import { EthAccountType } from '.';
-import { BtcAccountType } from '../btc';
+import { BtcAccountType, EthAccountType } from '../api';
 import { isEvmAccountType } from './utils';
 
 describe('isEvmAccountType', () => {

diff --git a/src/eth/utils.ts b/src/eth/utils.ts
@@ -1,5 +1,5 @@
+import { EthAccountType } from '../api';
 import type { InternalAccountType } from '../internal';
-import { EthAccountType } from './types';
 
 /**
  * Checks if the given type is an EVM account type.

diff --git a/src/internal/events.test.ts b/src/internal/events.test.ts
@@ -1,6 +1,6 @@
 import { is } from 'superstruct';
 
-import { EthAccountType } from '../eth/types';
+import { EthAccountType } from '../api';
 import { KeyringEvent } from '../events';
 import {
   AccountCreatedEventStruct,

diff --git a/src/internal/types.ts b/src/internal/types.ts
@@ -1,13 +1,9 @@
 import type { Infer, Struct } from 'superstruct';
-import { boolean, string, number, define, mask, validate } from 'superstruct';
+import { boolean, string, number } from 'superstruct';
 
-import { BaseKeyringAccountStruct } from '../api';
-import { BtcP2wpkhAccountStruct, BtcAccountType } from '../btc/types';
-import {
-  EthEoaAccountStruct,
-  EthErc4337AccountStruct,
-  EthAccountType,
-} from '../eth/types';
+import { BtcAccountType, EthAccountType, KeyringAccountStruct } from '../api';
+import { BtcP2wpkhAccountStruct } from '../btc/types';
+import { EthEoaAccountStruct, EthErc4337AccountStruct } from '../eth/types';
 import { exactOptional, object } from '../superstruct';
 
 export type InternalAccountType = EthAccountType | BtcAccountType;
@@ -34,7 +30,7 @@ export const InternalAccountMetadataStruct = object({
  * Creates an `InternalAccount` from an existing account `superstruct` object.
  *
  * @param accountStruct - An account `superstruct` object.
- * @returns The `InternalAccount` assocaited to `accountStruct`.
+ * @returns The `InternalAccount` associated to `accountStruct`.
  */
 function asInternalAccountStruct<Account, AccountSchema>(
   accountStruct: Struct<Account, AccountSchema>,
@@ -82,20 +78,10 @@ export type InternalAccountTypes =
   | InternalEthErc4337Account
   | InternalBtcP2wpkhAccount;
 
-export const InternalAccountStruct = define<InternalAccountTypes>(
-  'InternalAccount',
-  (value: unknown) => {
-    const account = mask(value, BaseKeyringAccountStruct);
-
-    // At this point, we know that `value.type` can be used as an index for `KeyringAccountStructs`
-    const [error] = validate(
-      value,
-      InternalAccountStructs[account.type] as Struct,
-    );
-
-    return error ?? true;
-  },
-);
+export const InternalAccountStruct = object({
+  ...KeyringAccountStruct.schema,
+  ...InternalAccountMetadataStruct.schema,
+});
 
 /**
  * Internal account representation.