feat(keyring-controller): add withKeyringV2 and deprecate withKeyring

packages/keyring-controller/CHANGELOG.md

@@ -7,8 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Added `withKeyringV2` method and `KeyringController:withKeyringV2` messenger action for atomic operations using the `KeyringV2` API ([#8372](https://github.com/MetaMask/core/pull/8372))
+  - Selects a V1 keyring, wraps it in an ephemeral `KeyringV2` adapter via a registered `KeyringV2Builder`, and passes it to the callback.
+  - Accepts a `KeyringSelectorV2` (alias for `KeyringSelector`) to select keyrings by `type`, `address`, `id`, or `filter`.
+  - Ships with default V2 builders for HD (`HdKeyringV2`) and Simple (`SimpleKeyringV2`) keyrings; additional builders can be registered via the `keyringV2Builders` constructor option.
+
 ### Changed
 
+- Deprecated `withKeyring` in favor of `withKeyringV2`, which supports the `KeyringV2` API.
 - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.0` ([#8364](https://github.com/MetaMask/core/pull/8364))
 
 ## [25.2.0]

packages/keyring-controller/package.json

@@ -50,9 +50,9 @@
     "@ethereumjs/util": "^9.1.0",
     "@metamask/base-controller": "^9.0.1",
     "@metamask/browser-passworder": "^6.0.0",
-    "@metamask/eth-hd-keyring": "^13.0.0",
+    "@metamask/eth-hd-keyring": "^13.1.0",
     "@metamask/eth-sig-util": "^8.2.0",
-    "@metamask/eth-simple-keyring": "^11.0.0",
+    "@metamask/eth-simple-keyring": "^11.1.1",
     "@metamask/keyring-api": "^21.6.0",
     "@metamask/keyring-internal-api": "^10.0.0",
     "@metamask/messenger": "^1.1.0",

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

@@ -4027,6 +4027,7 @@ describe('KeyringController', () => {
         await expect(
           controller.withKeyringUnsafe({ type: 'NonExistentType' }, fn),
         ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound);
+
         expect(fn).not.toHaveBeenCalled();
       });
     });
@@ -4143,6 +4144,205 @@ describe('KeyringController', () => {
     });
   });
 
+  describe('withKeyringV2', () => {
+    it('should wrap the V1 keyring using the default builder and call the operation', async () => {
+      await withController(async ({ controller }) => {
+        const fn = jest.fn();
+        await controller.withKeyringV2({ type: KeyringTypes.hd }, fn);
+
+        expect(fn).toHaveBeenCalledWith(
+          expect.objectContaining({
+            keyring: expect.any(Object),
+            metadata: expect.objectContaining({ id: expect.any(String) }),
+          }),
+        );
+      });
+    });
+
+    it('should return the result of the operation', async () => {
+      await withController(async ({ controller }) => {
+        const result = await controller.withKeyringV2(
+          { type: KeyringTypes.hd },
+          async () => 'result-value',
+        );
+
+        expect(result).toBe('result-value');
+      });
+    });
+
+    it('should throw KeyringNotFound when no keyring matches', async () => {
+      await withController(async ({ controller }) => {
+        const fn = jest.fn();
+
+        await expect(
+          controller.withKeyringV2({ type: 'non-existent' }, fn),
+        ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound);
+
+        expect(fn).not.toHaveBeenCalled();
+      });
+    });
+
+    it('should throw KeyringV2NotSupported when no builder is registered for the type', async () => {
+      await withController(
+        {
+          keyringBuilders: [keyringBuilderFactory(MockShallowKeyring)],
+        },
+        async ({ controller }) => {
+          await controller.addNewKeyring(MockShallowKeyring.type);
+
+          const fn = jest.fn();
+          await expect(
+            controller.withKeyringV2(
+              { type: MockShallowKeyring.type },
+              fn,
+            ),
+          ).rejects.toThrow(
+            KeyringControllerErrorMessage.KeyringV2NotSupported,
+          );
+
+          expect(fn).not.toHaveBeenCalled();
+        },
+      );
+    });
+
+    it('should throw an error if the callback returns the wrapped keyring', async () => {
+      await withController(async ({ controller }) => {
+        await expect(
+          controller.withKeyringV2(
+            { type: KeyringTypes.hd },
+            async ({ keyring }) => keyring,
+          ),
+        ).rejects.toThrow(
+          KeyringControllerErrorMessage.UnsafeDirectKeyringAccess,
+        );
+      });
+    });
+
+    it('should throw an error if the controller is locked', async () => {
+      await withController(async ({ controller }) => {
+        await controller.setLocked();
+
+        await expect(
+          controller.withKeyringV2(
+            { type: KeyringTypes.hd },
+            jest.fn(),
+          ),
+        ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked);
+      });
+    });
+
+    describe('when the keyring is selected by address', () => {
+      it('should wrap the V1 keyring that holds the given address', async () => {
+        await withController(async ({ controller, initialState }) => {
+          const fn = jest.fn();
+          const address = initialState.keyrings[0].accounts[0] as Hex;
+
+          await controller.withKeyringV2({ address }, fn);
+
+          expect(fn).toHaveBeenCalledWith(
+            expect.objectContaining({
+              keyring: expect.any(Object),
+              metadata: expect.objectContaining({ id: expect.any(String) }),
+            }),
+          );
+        });
+      });
+    });
+
+    describe('when the keyring is selected by id', () => {
+      it('should wrap the V1 keyring with the matching metadata id', async () => {
+        await withController(async ({ controller, initialState }) => {
+          const fn = jest.fn();
+          const keyringId = initialState.keyrings[0].metadata.id;
+
+          await controller.withKeyringV2({ id: keyringId }, fn);
+
+          expect(fn).toHaveBeenCalledWith(
+            expect.objectContaining({
+              metadata: expect.objectContaining({ id: keyringId }),
+            }),
+          );
+        });
+      });
+
+      it('should throw KeyringNotFound if no keyring has the id', async () => {
+        await withController(async ({ controller }) => {
+          await expect(
+            controller.withKeyringV2(
+              { id: 'non-existent-id' },
+              jest.fn(),
+            ),
+          ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound);
+        });
+      });
+    });
+
+    describe('when the keyring is selected by filter', () => {
+      it('should wrap the V1 keyring that matches the filter', async () => {
+        await withController(async ({ controller }) => {
+          const fn = jest.fn();
+          await controller.withKeyringV2(
+            { filter: (k): boolean => k.type === KeyringTypes.hd },
+            fn,
+          );
+
+          expect(fn).toHaveBeenCalledWith(
+            expect.objectContaining({
+              keyring: expect.any(Object),
+              metadata: expect.objectContaining({ id: expect.any(String) }),
+            }),
+          );
+        });
+      });
+
+      it('should throw KeyringNotFound if no keyring matches the filter', async () => {
+        await withController(async ({ controller }) => {
+          await expect(
+            controller.withKeyringV2(
+              { filter: (): boolean => false },
+              jest.fn(),
+            ),
+          ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound);
+        });
+      });
+    });
+
+    describe('rollback', () => {
+      it('should rollback the underlying V1 keyring if the operation throws', async () => {
+        await withController(async ({ controller, initialState }) => {
+          await expect(
+            controller.withKeyringV2(
+              { type: KeyringTypes.hd },
+              async () => {
+                throw new Error('Rollback test');
+              },
+            ),
+          ).rejects.toThrow('Rollback test');
+
+          expect(controller.state.keyrings[0].accounts).toStrictEqual(
+            initialState.keyrings[0].accounts,
+          );
+        });
+      });
+    });
+
+    describe('messenger action', () => {
+      it('should be callable through the messenger', async () => {
+        await withController(async ({ messenger }) => {
+          const fn = jest.fn();
+
+          await messenger.call(
+            'KeyringController:withKeyringV2',
+            { type: KeyringTypes.hd },
+            fn,
+          );
+
+          expect(fn).toHaveBeenCalled();
+        });
+      });
+    });
+  });
+
   describe('isCustodyKeyring', () => {
     it('should return true if keyring is custody keyring', () => {
       expect(isCustodyKeyring('Custody JSON-RPC')).toBe(true);