chore: fix lint warnings in KeyringController

packages/keyring-controller/src/KeyringController.test.ts

@@ -25,13 +25,6 @@ import {
 import * as sinon from 'sinon';
 import * as uuid from 'uuid';
 
-import MockEncryptor, {
-  MOCK_ENCRYPTION_KEY,
-} from '../tests/mocks/mockEncryptor';
-import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring';
-import { MockKeyring } from '../tests/mocks/mockKeyring';
-import MockShallowGetAccountsKeyring from '../tests/mocks/mockShallowGetAccountsKeyring';
-import { buildMockTransaction } from '../tests/mocks/mockTransaction';
 import { KeyringControllerError } from './constants';
 import type {
   KeyringControllerEvents,
@@ -47,6 +40,13 @@ import {
   isCustodyKeyring,
   keyringBuilderFactory,
 } from './KeyringController';
+import MockEncryptor, {
+  MOCK_ENCRYPTION_KEY,
+} from '../tests/mocks/mockEncryptor';
+import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring';
+import { MockKeyring } from '../tests/mocks/mockKeyring';
+import MockShallowGetAccountsKeyring from '../tests/mocks/mockShallowGetAccountsKeyring';
+import { buildMockTransaction } from '../tests/mocks/mockTransaction';
 
 jest.mock('uuid', () => {
   return {
@@ -181,12 +181,10 @@ describe('KeyringController', () => {
       it('should not add a new account if called twice with the same accountCount param', async () => {
         await withController(async ({ controller, initialState }) => {
           const accountCount = initialState.keyrings[0].accounts.length;
-          const firstAccountAdded = await controller.addNewAccount(
-            accountCount,
-          );
-          const secondAccountAdded = await controller.addNewAccount(
-            accountCount,
-          );
+          const firstAccountAdded =
+            await controller.addNewAccount(accountCount);
+          const secondAccountAdded =
+            await controller.addNewAccount(accountCount);
           expect(firstAccountAdded).toBe(secondAccountAdded);
           expect(controller.state.keyrings[0].accounts).toHaveLength(
             accountCount + 1,
@@ -220,13 +218,11 @@ describe('KeyringController', () => {
 
           const accountCount = initialState.keyrings[0].accounts.length;
           // We add a new account for "index 1" (not existing yet)
-          const firstAccountAdded = await controller.addNewAccount(
-            accountCount,
-          );
+          const firstAccountAdded =
+            await controller.addNewAccount(accountCount);
           // Adding an account for an existing index will return the existing account's address
-          const secondAccountAdded = await controller.addNewAccount(
-            accountCount,
-          );
+          const secondAccountAdded =
+            await controller.addNewAccount(accountCount);
           expect(firstAccountAdded).toBe(secondAccountAdded);
           expect(controller.state.keyrings[0].accounts).toHaveLength(
             accountCount + 1,
@@ -258,9 +254,8 @@ describe('KeyringController', () => {
           const [primaryKeyring] = controller.getKeyringsByType(
             KeyringTypes.hd,
           ) as Keyring<Json>[];
-          const addedAccountAddress = await controller.addNewAccountForKeyring(
-            primaryKeyring,
-          );
+          const addedAccountAddress =
+            await controller.addNewAccountForKeyring(primaryKeyring);
           expect(initialState.keyrings).toHaveLength(1);
           expect(initialState.keyrings[0].accounts).not.toStrictEqual(
             controller.state.keyrings[0].accounts,
@@ -306,9 +301,8 @@ describe('KeyringController', () => {
           const [primaryKeyring] = controller.getKeyringsByType(
             KeyringTypes.hd,
           ) as Keyring<Json>[];
-          const addedAccountAddress = await controller.addNewAccountForKeyring(
-            primaryKeyring,
-          );
+          const addedAccountAddress =
+            await controller.addNewAccountForKeyring(primaryKeyring);
           expect(initialState.keyrings).toHaveLength(1);
           expect(initialState.keyrings[0].accounts).not.toStrictEqual(
             controller.state.keyrings[0].accounts,
@@ -407,9 +401,8 @@ describe('KeyringController', () => {
           await withController(
             { cacheEncryptionKey },
             async ({ controller, initialState }) => {
-              const currentSeedWord = await controller.exportSeedPhrase(
-                password,
-              );
+              const currentSeedWord =
+                await controller.exportSeedPhrase(password);
 
               await controller.createNewVaultAndRestore(
                 password,
@@ -475,9 +468,8 @@ describe('KeyringController', () => {
               async ({ controller }) => {
                 await controller.createNewVaultAndKeychain(password);
 
-                const currentSeedPhrase = await controller.exportSeedPhrase(
-                  password,
-                );
+                const currentSeedPhrase =
+                  await controller.exportSeedPhrase(password);
 
                 expect(currentSeedPhrase.length).toBeGreaterThan(0);
                 expect(
@@ -565,17 +557,15 @@ describe('KeyringController', () => {
             await withController(
               { cacheEncryptionKey },
               async ({ controller, initialState }) => {
-                const initialSeedWord = await controller.exportSeedPhrase(
-                  password,
-                );
+                const initialSeedWord =
+                  await controller.exportSeedPhrase(password);
                 expect(initialSeedWord).toBeDefined();
                 const initialVault = controller.state.vault;
 
                 await controller.createNewVaultAndKeychain(password);
 
-                const currentSeedWord = await controller.exportSeedPhrase(
-                  password,
-                );
+                const currentSeedWord =
+                  await controller.exportSeedPhrase(password);
                 expect(initialState).toStrictEqual(controller.state);
                 expect(initialSeedWord).toBe(currentSeedWord);
                 expect(initialVault).toStrictEqual(controller.state.vault);
@@ -2556,21 +2546,18 @@ describe('KeyringController', () => {
           ),
         );
 
-        const firstPage = await signProcessKeyringController.connectQRHardware(
-          0,
-        );
+        const firstPage =
+          await signProcessKeyringController.connectQRHardware(0);
         expect(firstPage).toHaveLength(5);
         expect(firstPage[0].index).toBe(0);
 
-        const secondPage = await signProcessKeyringController.connectQRHardware(
-          1,
-        );
+        const secondPage =
+          await signProcessKeyringController.connectQRHardware(1);
         expect(secondPage).toHaveLength(5);
         expect(secondPage[0].index).toBe(5);
 
-        const goBackPage = await signProcessKeyringController.connectQRHardware(
-          -1,
-        );
+        const goBackPage =
+          await signProcessKeyringController.connectQRHardware(-1);
         expect(goBackPage).toStrictEqual(firstPage);
 
         await signProcessKeyringController.unlockQRHardwareWalletAccount(0);

packages/keyring-controller/src/KeyringController.ts

@@ -52,32 +52,19 @@ const name = 'KeyringController';
  * Available keyring types
  */
 export enum KeyringTypes {
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   simple = 'Simple Key Pair',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   hd = 'HD Key Tree',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   qr = 'QR Hardware Wallet Device',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   trezor = 'Trezor Hardware',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   ledger = 'Ledger Hardware',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   lattice = 'Lattice Hardware',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   snap = 'Snap Keyring',
 }
 
 /**
  * Custody keyring types are a special case, as they are not a single type
  * but they all start with the prefix "Custody".
+ *
  * @param keyringType - The type of the keyring.
  * @returns Whether the keyring type is a custody keyring.
  */
@@ -86,15 +73,9 @@ export const isCustodyKeyring = (keyringType: string): boolean => {
 };
 
 /**
- * @type KeyringControllerState
+ * KeyringControllerState
  *
- * Keyring controller state
- * @property vault - Encrypted string representing keyring data
- * @property isUnlocked - Whether vault is unlocked
- * @property keyringTypes - Account types
- * @property keyrings - Group of accounts
- * @property encryptionKey - Keyring encryption key
- * @property encryptionSalt - Keyring encryption salt
+ * The KeyringController state
  */
 export type KeyringControllerState = {
   vault?: string;
@@ -251,11 +232,9 @@ export type KeyringControllerOptions = {
 );
 
 /**
- * @type KeyringObject
+ * KeyringObject
  *
  * Keyring object to return in fullUpdate
- * @property type - Keyring type
- * @property accounts - Associated accounts
  */
 export type KeyringObject = {
   accounts: string[];
@@ -266,11 +245,7 @@ export type KeyringObject = {
  * A strategy for importing an account
  */
 export enum AccountImportStrategy {
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   privateKey = 'privateKey',
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   json = 'json',
 }
 
@@ -404,13 +379,11 @@ export type KeyringSelector =
  *
  * @param releaseLock - A function to release the lock.
  */
-// TODO: Either fix this lint violation or explain why it's necessary to ignore.
-// eslint-disable-next-line @typescript-eslint/naming-convention
-type MutuallyExclusiveCallback<T> = ({
+type MutuallyExclusiveCallback<Result> = ({
   releaseLock,
 }: {
   releaseLock: MutexInterface.Releaser;
-}) => Promise<T>;
+}) => Promise<Result>;
 
 /**
  * Get builder function for `Keyring`
@@ -586,17 +559,17 @@ export class KeyringController extends BaseController<
 
   readonly #vaultOperationMutex = new Mutex();
 
-  #keyringBuilders: { (): EthKeyring<Json>; type: string }[];
+  readonly #keyringBuilders: { (): EthKeyring<Json>; type: string }[];
 
-  #keyrings: EthKeyring<Json>[];
+  readonly #unsupportedKeyrings: SerializedKeyring[];
 
-  #unsupportedKeyrings: SerializedKeyring[];
+  readonly #encryptor: GenericEncryptor | ExportableKeyEncryptor;
 
-  #password?: string;
+  readonly #cacheEncryptionKey: boolean;
 
-  #encryptor: GenericEncryptor | ExportableKeyEncryptor;
+  #keyrings: EthKeyring<Json>[];
 
-  #cacheEncryptionKey: boolean;
+  #password?: string;
 
   #qrKeyringStateListener?: (
     state: ReturnType<IQRKeyringState['getState']>,
@@ -765,6 +738,7 @@ export class KeyringController extends BaseController<
    * If there is a pre-existing locked vault, it will be replaced.
    *
    * @param password - Password to unlock the new vault.
+   * @returns Promise resolving when the operation ends successfully.
    */
   async createNewVaultAndKeychain(password: string) {
     return this.#persistOrRollback(async () => {
@@ -989,7 +963,7 @@ export class KeyringController extends BaseController<
     return this.#persistOrRollback(async () => {
       let privateKey;
       switch (strategy) {
-        case 'privateKey':
+        case AccountImportStrategy.privateKey:
           const [importedKey] = args;
           if (!importedKey) {
             throw new Error('Cannot import an empty key.');
@@ -1013,7 +987,7 @@ export class KeyringController extends BaseController<
 
           privateKey = remove0x(prefixed);
           break;
-        case 'json':
+        case AccountImportStrategy.json:
           let wallet;
           const [input, password] = args;
           try {
@@ -1024,7 +998,7 @@ export class KeyringController extends BaseController<
           privateKey = bytesToHex(wallet.getPrivateKey());
           break;
         default:
-          throw new Error(`Unexpected import strategy: '${strategy}'`);
+          throw new Error(`Unexpected import strategy: '${String(strategy)}'`);
       }
       const newKeyring = (await this.#newKeyring(KeyringTypes.simple, [
         privateKey,
@@ -1054,7 +1028,6 @@ export class KeyringController extends BaseController<
 
       // The `removeAccount` method of snaps keyring is async. We have to update
       // the interface of the other keyrings to be async as well.
-      // eslint-disable-next-line @typescript-eslint/await-thenable
       // FIXME: We do cast to `Hex` to makes the type checker happy here, and
       // because `Keyring<State>.removeAccount` requires address to be `Hex`. Those
       // type would need to be updated for a full non-EVM support.
@@ -2203,7 +2176,6 @@ export class KeyringController extends BaseController<
       // cases and allow retro-compatibility too.
       // FIXME: For some reason, it seems that eslint is complaining about this call being non-thenable
       // even though it is... For now, we just disable it:
-      // eslint-disable-next-line @typescript-eslint/await-thenable
       await keyring.generateRandomMnemonic();
       await keyring.addAccounts(1);
     }
@@ -2353,14 +2325,14 @@ export class KeyringController extends BaseController<
    * and save the keyrings to state after it, or rollback to their
    * previous state in case of error.
    *
-   * @param fn - The function to execute.
+   * @param callback - The function to execute.
    * @returns The result of the function.
    */
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  async #persistOrRollback<T>(fn: MutuallyExclusiveCallback<T>): Promise<T> {
+  async #persistOrRollback<Result>(
+    callback: MutuallyExclusiveCallback<Result>,
+  ): Promise<Result> {
     return this.#withRollback(async ({ releaseLock }) => {
-      const callbackResult = await fn({ releaseLock });
+      const callbackResult = await callback({ releaseLock });
       // State is committed only if the operation is successful
       await this.#updateVault();
 
@@ -2372,18 +2344,18 @@ export class KeyringController extends BaseController<
    * Execute the given function after acquiring the controller lock
    * and rollback keyrings and password states in case of error.
    *
-   * @param fn - The function to execute atomically.
+   * @param callback - The function to execute atomically.
    * @returns The result of the function.
    */
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  async #withRollback<T>(fn: MutuallyExclusiveCallback<T>): Promise<T> {
+  async #withRollback<Result>(
+    callback: MutuallyExclusiveCallback<Result>,
+  ): Promise<Result> {
     return this.#withControllerLock(async ({ releaseLock }) => {
       const currentSerializedKeyrings = await this.#getSerializedKeyrings();
       const currentPassword = this.#password;
 
       try {
-        return await fn({ releaseLock });
+        return await callback({ releaseLock });
       } catch (e) {
         // Keyrings and password are restored to their previous state
         await this.#restoreSerializedKeyrings(currentSerializedKeyrings);
@@ -2414,13 +2386,13 @@ export class KeyringController extends BaseController<
    * controller and that changes its state is executed in a mutually exclusive way,
    * preventing unsafe concurrent access that could lead to unpredictable behavior.
    *
-   * @param fn - The function to execute while the controller mutex is locked.
+   * @param callback - The function to execute while the controller mutex is locked.
    * @returns The result of the function.
    */
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  async #withControllerLock<T>(fn: MutuallyExclusiveCallback<T>): Promise<T> {
-    return withLock(this.#controllerOperationMutex, fn);
+  async #withControllerLock<Result>(
+    callback: MutuallyExclusiveCallback<Result>,
+  ): Promise<Result> {
+    return withLock(this.#controllerOperationMutex, callback);
   }
 
   /**
@@ -2431,15 +2403,15 @@ export class KeyringController extends BaseController<
    * This ensures that each operation that interacts with the vault
    * is executed in a mutually exclusive way.
    *
-   * @param fn - The function to execute while the vault mutex is locked.
+   * @param callback - The function to execute while the vault mutex is locked.
    * @returns The result of the function.
    */
-  // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  async #withVaultLock<T>(fn: MutuallyExclusiveCallback<T>): Promise<T> {
+  async #withVaultLock<Result>(
+    callback: MutuallyExclusiveCallback<Result>,
+  ): Promise<Result> {
     this.#assertControllerMutexIsLocked();
 
-    return withLock(this.#vaultOperationMutex, fn);
+    return withLock(this.#vaultOperationMutex, callback);
   }
 }
 
@@ -2449,19 +2421,17 @@ export class KeyringController extends BaseController<
  * error is thrown.
  *
  * @param mutex - The mutex to lock.
- * @param fn - The function to execute while the mutex is locked.
+ * @param callback - The function to execute while the mutex is locked.
  * @returns The result of the function.
  */
-// TODO: Either fix this lint violation or explain why it's necessary to ignore.
-// eslint-disable-next-line @typescript-eslint/naming-convention
-async function withLock<T>(
+async function withLock<Result>(
   mutex: Mutex,
-  fn: MutuallyExclusiveCallback<T>,
-): Promise<T> {
+  callback: MutuallyExclusiveCallback<Result>,
+): Promise<Result> {
   const releaseLock = await mutex.acquire();
 
   try {
-    return await fn({ releaseLock });
+    return await callback({ releaseLock });
   } finally {
     releaseLock();
   }