feat(keyring-controller): add keyring.fingerprint support

packages/keyring-controller/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add optional `getFingerprint` to `KeyringBuilder` to associate a deterministic fingerprint with a keyring instance ([#8341](https://github.com/MetaMask/core/pull/8341))
+  - Add `KeyringMetadata.fingerprint` field, populated when the builder provides `getFingerprint`.
+  - Add `{ fingerprint: string }` as a new `KeyringSelector` variant for use with `withKeyring`.
+
 ## [25.1.1]
 
 ### Changed

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

@@ -561,6 +561,69 @@ describe('KeyringController', () => {
       });
     });
 
+    const mockFingerprint = 'mock:fp';
+
+    describe('when the builder provides `getFingerprint`', () => {
+      it('stores the fingerprint in the keyring metadata', async () => {
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = (): string => mockFingerprint;
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            const metadata = await controller.addNewKeyring(MockKeyring.type);
+            expect(metadata.fingerprint).toBe(mockFingerprint);
+          },
+        );
+      });
+
+      it('passes the keyring instance to `getFingerprint`', async () => {
+        const getFingerprintSpy = jest.fn().mockReturnValue(mockFingerprint);
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = getFingerprintSpy;
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            await controller.addNewKeyring(MockKeyring.type);
+            expect(getFingerprintSpy).toHaveBeenCalledWith(
+              expect.any(MockKeyring),
+            );
+          },
+        );
+      });
+
+      it('leaves `fingerprint` undefined and logs the error if `getFingerprint` throws', async () => {
+        const fingerprintError = new Error('getFingerprint failed');
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = (): string => {
+          throw fingerprintError;
+        };
+        const consoleErrorSpy = jest.spyOn(console, 'error');
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            const metadata = await controller.addNewKeyring(MockKeyring.type);
+            expect(metadata.fingerprint).toBeUndefined();
+            expect(consoleErrorSpy).toHaveBeenCalledWith(
+              'Unable to compute fingerprint (skipping):',
+              fingerprintError,
+            );
+          },
+        );
+      });
+    });
+
+    describe('when the builder does not provide `getFingerprint`', () => {
+      it('leaves `fingerprint` undefined in the keyring metadata', async () => {
+        await withController(async ({ controller }) => {
+          const metadata = await controller.addNewKeyring(KeyringTypes.simple);
+          expect(metadata.fingerprint).toBeUndefined();
+        });
+      });
+    });
+
     describe('when there is no builder for the given type', () => {
       it('should throw error', async () => {
         await withController(async ({ controller }) => {
@@ -3018,6 +3081,78 @@ describe('KeyringController', () => {
       );
     });
 
+    it('keeps the keyring and leaves `fingerprint` undefined if `getFingerprint` throws during restore', async () => {
+      const fingerprintError = new Error('getFingerprint failed');
+      const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+      fingerprintBuilder.getFingerprint = (): string => {
+        throw fingerprintError;
+      };
+      const consoleErrorSpy = jest.spyOn(console, 'error');
+
+      await withController(
+        {
+          keyringBuilders: [fingerprintBuilder],
+          skipVaultCreation: true,
+          state: {
+            vault: createVault([
+              {
+                type: MockKeyring.type,
+                data: {},
+                metadata: { id: 'existing-id', name: 'existing-name' },
+              },
+            ]),
+          },
+        },
+        async ({ controller }) => {
+          await controller.submitPassword(password);
+
+          const keyringObject = controller.state.keyrings.find(
+            (k) => k.type === MockKeyring.type,
+          );
+          expect(keyringObject).toBeDefined();
+          expect(keyringObject?.metadata.fingerprint).toBeUndefined();
+          expect(consoleErrorSpy).toHaveBeenCalledWith(
+            'Unable to compute fingerprint (skipping):',
+            fingerprintError,
+          );
+        },
+      );
+    });
+
+    it('recomputes fingerprint from builder when restoring keyrings from vault', async () => {
+      const mockFingerprint = 'mock:fp';
+      const getFingerprintSpy = jest.fn().mockReturnValue(mockFingerprint);
+      const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+      fingerprintBuilder.getFingerprint = getFingerprintSpy;
+
+      await withController(
+        {
+          keyringBuilders: [fingerprintBuilder],
+          skipVaultCreation: true,
+          state: {
+            vault: createVault([
+              {
+                type: MockKeyring.type,
+                data: {},
+                metadata: { id: 'existing-id', name: 'existing-name' },
+              },
+            ]),
+          },
+        },
+        async ({ controller }) => {
+          await controller.submitPassword(password);
+
+          const keyringObject = controller.state.keyrings.find(
+            (k) => k.type === MockKeyring.type,
+          );
+          expect(keyringObject?.metadata.fingerprint).toBe(mockFingerprint);
+          expect(getFingerprintSpy).toHaveBeenCalledWith(
+            expect.any(MockKeyring),
+          );
+        },
+      );
+    });
+
     it('should unlock the wallet discarding existing duplicate accounts', async () => {
       stubKeyringClassWithAccount(MockKeyring, '0x123');
       stubKeyringClassWithAccount(HdKeyring, '0x123');
@@ -3854,6 +3989,84 @@ describe('KeyringController', () => {
       });
     });
 
+    describe('when the keyring is selected by fingerprint', () => {
+      const mockFingerprint = 'mock:fp';
+
+      it('calls the given function with the matching keyring', async () => {
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = (): string => mockFingerprint;
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            await controller.addNewKeyring(MockKeyring.type);
+            const fn = jest.fn();
+
+            await controller.withKeyring({ fingerprint: mockFingerprint }, fn);
+
+            expect(fn).toHaveBeenCalledWith({
+              keyring: expect.any(MockKeyring),
+              metadata: expect.objectContaining({
+                fingerprint: mockFingerprint,
+              }),
+            });
+          },
+        );
+      });
+
+      it('returns the result of the function', async () => {
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = (): string => mockFingerprint;
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            await controller.addNewKeyring(MockKeyring.type);
+
+            const result = await controller.withKeyring(
+              { fingerprint: mockFingerprint },
+              async (): Promise<string> => Promise.resolve('hello'),
+            );
+            expect(result).toBe('hello');
+          },
+        );
+      });
+
+      it('throws an error if the callback returns the selected keyring', async () => {
+        const fingerprintBuilder = keyringBuilderFactory(MockKeyring);
+        fingerprintBuilder.getFingerprint = (): string => mockFingerprint;
+
+        await withController(
+          { keyringBuilders: [fingerprintBuilder] },
+          async ({ controller }) => {
+            await controller.addNewKeyring(MockKeyring.type);
+
+            await expect(
+              controller.withKeyring(
+                { fingerprint: mockFingerprint },
+                async ({ keyring }) => keyring,
+              ),
+            ).rejects.toThrow(
+              KeyringControllerErrorMessage.UnsafeDirectKeyringAccess,
+            );
+          },
+        );
+      });
+
+      describe('when the keyring is not found', () => {
+        it('throws an error when no keyring matches the fingerprint', async () => {
+          await withController(async ({ controller }) => {
+            await expect(
+              controller.withKeyring(
+                { fingerprint: 'non-existent-fingerprint' },
+                jest.fn(),
+              ),
+            ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound);
+          });
+        });
+      });
+    });
+
     it('should throw KeyringNotFound if keyring metadata is not found (internal consistency check)', async () => {
       // This test verifies the defensive #getKeyringMetadata guard that ensures
       // internal state consistency. In normal operation, this should never occur,

packages/keyring-controller/src/KeyringController.ts

@@ -309,6 +309,11 @@ export type KeyringMetadata = {
    * Keyring name
    */
   name: string;
+  /**
+   * Keyring fingerprint, computed by the builder's `getFingerprint` function.
+   * Absent if the builder does not provide `getFingerprint`.
+   */
+  fingerprint?: string;
 };
 
 /**
@@ -513,6 +518,9 @@ export type KeyringSelector =
     }
   | {
       id: string;
+    }
+  | {
+      fingerprint: string;
     };
 
 /**
@@ -521,6 +529,14 @@ export type KeyringSelector =
 export type KeyringBuilder = {
   (): Keyring;
   type: string;
+  /**
+   * Compute a deterministic fingerprint for a keyring instance.
+   * Called after the keyring is fully initialized (post `deserialize` and `init`).
+   *
+   * @param keyring - The initialized keyring instance.
+   * @returns An opaque string that uniquely identifies this keyring.
+   */
+  getFingerprint?: (keyring: Keyring) => string;
 };
 
 /**
@@ -1778,6 +1794,10 @@ export class KeyringController<
         }
       } else if ('id' in selector) {
         keyring = this.#getKeyringById(selector.id) as SelectedKeyring;
+      } else if ('fingerprint' in selector) {
+        keyring = this.#getKeyringByFingerprint(
+          selector.fingerprint,
+        ) as SelectedKeyring;
       }
 
       if (!keyring) {
@@ -1924,6 +1944,36 @@ export class KeyringController<
       ?.keyring;
   }
 
+  #getKeyringByFingerprint(fingerprint: string): EthKeyring | undefined {
+    return this.#keyrings.find(
+      ({ metadata }) => metadata.fingerprint === fingerprint,
+    )?.keyring;
+  }
+
+  /**
+   * Safely compute the fingerprint for a keyring using the given builder.
+   * Returns `undefined` if the builder has no `getFingerprint`, or if
+   * `getFingerprint` throws — in which case the error is logged.
+   *
+   * @param builder - The keyring builder, or undefined if none is registered.
+   * @param keyring - The initialized keyring instance.
+   * @returns The fingerprint string, or `undefined`.
+   */
+  #getSafeFingerprint(
+    builder: KeyringBuilder | undefined,
+    keyring: EthKeyring,
+  ): string | undefined {
+    if (!builder?.getFingerprint) {
+      return undefined;
+    }
+    try {
+      return builder.getFingerprint(keyring);
+    } catch (error) {
+      console.error('Unable to compute fingerprint (skipping):', error);
+      return undefined;
+    }
+  }
+
   /**
    * Get the keyring by id or return the first keyring if the id is not found.
    *
@@ -1962,9 +2012,7 @@ export class KeyringController<
    * @param type - The type of keyring to get the builder for.
    * @returns The keyring builder, or undefined if none exists.
    */
-  #getKeyringBuilderForType(
-    type: string,
-  ): { (): EthKeyring; type: string } | undefined {
+  #getKeyringBuilderForType(type: string): KeyringBuilder | undefined {
     return this.#keyringBuilders.find(
       (keyringBuilder) => keyringBuilder.type === type,
     );
@@ -2451,8 +2499,13 @@ export class KeyringController<
    */
   async #newKeyring(type: string, data?: unknown): Promise<EthKeyring> {
     const keyring = await this.#createKeyring(type, data);
-
-    this.#keyrings.push({ keyring, metadata: getDefaultKeyringMetadata() });
+    const metadata = getDefaultKeyringMetadata();
+    const builder = this.#getKeyringBuilderForType(type);
+    const fingerprint = this.#getSafeFingerprint(builder, keyring);
+    if (fingerprint !== undefined) {
+      metadata.fingerprint = fingerprint;
+    }
+    this.#keyrings.push({ keyring, metadata });
 
     return keyring;
   }
@@ -2555,6 +2608,14 @@ export class KeyringController<
         newMetadata = true;
         metadata = getDefaultKeyringMetadata();
       }
+      // We recompute the fingerprint for the keyring. This way, if an existing
+      // keyring now supports fingerprinting, we can still get the fingerprint
+      // for keyrings that were created before fingerprinting was supported.
+      const builder = this.#getKeyringBuilderForType(type);
+      const fingerprint = this.#getSafeFingerprint(builder, keyring);
+      if (fingerprint !== undefined) {
+        metadata = { ...metadata, fingerprint };
+      }
       // The keyring is added to the keyrings array only if it's successfully restored
       // and the metadata is successfully added to the controller
       this.#keyrings.push({