Skip to content

Commit 086b7f0

Browse files
ccharlymathieuartu
andauthored
feat(keyring-controller): add withKeyringV2 support (#8390)
## Explanation Another approach for `withKeyringV2` where we actually extend the existing list of keyring instances to have an optional `keyringV2` field. This allow to keep them in memory and have the same lifecycle than v1 keyring instance (and since all v2 implementations are wrappers for now, they are "bound" to v1 instances anyway). Original PR: - #8372 ## References N/A ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Introduces new V2 keyring selection/execution paths and caches V2 adapters alongside legacy keyrings, which could affect keyring lifecycle, rollback/persistence behavior, and error handling. Dependency bumps to keyring packages may also subtly change signing/account behavior. > > **Overview** > Adds first-class `KeyringV2` support by storing an optional in-memory V2 adapter next to each legacy keyring entry and creating/destroying these adapters during keyring create/restore/clear flows. > > Introduces `withKeyringV2` (locked, persisted/rollback) and `withKeyringV2Unsafe` (lock-free, no rollback) APIs plus messenger action types, including a new `KeyringSelectorV2` (`type`/`address`/`id`/`filter`) and a `KeyringV2NotSupported` error when no V2 builder exists for a selected keyring. > > Updates tests to cover selector variants, lock/rollback semantics, and messenger callability; bumps `@metamask/eth-hd-keyring`/`@metamask/eth-simple-keyring` versions and adjusts Jest coverage thresholds accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d8bc565. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Mathieu Artu <mathieu.artu@consensys.net>
1 parent 96955b7 commit 086b7f0

10 files changed

Lines changed: 947 additions & 93 deletions

File tree

packages/keyring-controller/CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Expose `KeyringController:signTransaction` method through `KeyringController` messenger ([#8408](https://github.com/MetaMask/core/pull/8408))
1313
- Persist vault when keyring state changes during unlock ([#8415](https://github.com/MetaMask/core/pull/8415))
1414
- If a keyring's serialized state differs after deserialization (e.g. a migration ran, or metadata was missing), the vault is now re-persisted so the change is not lost on the next unlock.
15+
- Added `KeyringV2` support ([#8390](https://github.com/MetaMask/core/pull/8390))
16+
- The controller now maintains a list of `KeyringV2` instance in memory alongside previous `Keyring` instance.
17+
- This new keyring interface is more generic and will become the new standard to interact with keyring (creating accounts, executing logic that involves accounts like signing, etc...).
18+
- For now, most `KeyringV2` are wrappers (read adapters) around existing `Keyring` instance.
19+
- Added `withKeyringV2Unsafe` method and `KeyringController:withKeyringV2Unsafe` messenger action for lock-free read-only access to `KeyringV2` adapters ([#8390](https://github.com/MetaMask/core/pull/8390))
20+
- Mirrors `withKeyringUnsafe` semantics: no mutex acquired, no persistence or rollback.
21+
- Caller is responsible for ensuring the operation is read-only and accesses only immutable keyring data.
22+
- Added `withKeyringV2` method and `KeyringController:withKeyringV2` messenger action for atomic operations using the `KeyringV2` API ([#8390](https://github.com/MetaMask/core/pull/8390))
23+
- Accepts a `KeyringSelectorV2` to select keyrings by `type`, `address`, `id`, or `filter`.
24+
- Ships with default V2 builders for HD (`HdKeyringV2`) and Simple (`SimpleKeyringV2`) keyrings; additional builders can be registered via the `keyringV2Builders` constructor option.
1525

1626
### Changed
1727

packages/keyring-controller/jest.config.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
1717
// An object that configures minimum threshold enforcement for coverage results
1818
coverageThreshold: {
1919
global: {
20-
branches: 95.09,
20+
branches: 94.91,
2121
functions: 100,
22-
lines: 99,
23-
statements: 99,
22+
lines: 99.08,
23+
statements: 99.08,
2424
},
2525
},
2626

packages/keyring-controller/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,9 @@
5252
"@ethereumjs/util": "^9.1.0",
5353
"@metamask/base-controller": "^9.0.1",
5454
"@metamask/browser-passworder": "^6.0.0",
55-
"@metamask/eth-hd-keyring": "^13.0.0",
55+
"@metamask/eth-hd-keyring": "^13.1.1",
5656
"@metamask/eth-sig-util": "^8.2.0",
57-
"@metamask/eth-simple-keyring": "^11.0.0",
57+
"@metamask/eth-simple-keyring": "^11.1.2",
5858
"@metamask/keyring-api": "^21.6.0",
5959
"@metamask/keyring-internal-api": "^10.0.0",
6060
"@metamask/messenger": "^1.1.1",

packages/keyring-controller/src/KeyringController-method-action-types.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,71 @@ export type KeyringControllerWithKeyringUnsafeAction = {
310310
handler: KeyringController['withKeyringUnsafe'];
311311
};
312312

313+
/**
314+
* Select a keyring using its `KeyringV2` adapter, and execute
315+
* the given operation with the wrapped keyring as a mutually
316+
* exclusive atomic operation.
317+
*
318+
* The cached `KeyringV2` adapter is retrieved from the keyring
319+
* entry.
320+
*
321+
* A `KeyringV2Builder` for the selected keyring's type must exist
322+
* (either as a default or registered via the `keyringV2Builders`
323+
* constructor option); otherwise an error is thrown.
324+
*
325+
* The method automatically persists changes at the end of the
326+
* function execution, or rolls back the changes if an error
327+
* is thrown.
328+
*
329+
* @param selector - Keyring selector object.
330+
* @param operation - Function to execute with the wrapped V2 keyring.
331+
* @returns Promise resolving to the result of the function execution.
332+
* @template CallbackResult - The type of the value resolved by the callback function.
333+
*/
334+
export type KeyringControllerWithKeyringV2Action = {
335+
type: `KeyringController:withKeyringV2`;
336+
handler: KeyringController['withKeyringV2'];
337+
};
338+
339+
/**
340+
* Select a keyring, wrap it in a `KeyringV2` adapter, and execute
341+
* the given read-only operation **without** acquiring the controller's
342+
* mutual exclusion lock.
343+
*
344+
* ## When to use this method
345+
*
346+
* This method is an escape hatch for read-only access to keyring data that
347+
* is immutable once the keyring is initialized. A typical safe use case is
348+
* reading immutable fields from a `KeyringV2` adapter: data that is set
349+
* during initialization and never mutated afterwards.
350+
*
351+
* ## Why it is "unsafe"
352+
*
353+
* The "unsafe" designation mirrors the semantics of `unsafe { }` blocks in
354+
* Rust: the method itself does not enforce thread-safety guarantees. By
355+
* calling this method the **caller** explicitly takes responsibility for
356+
* ensuring that:
357+
*
358+
* - The operation is **read-only** — no state is mutated.
359+
* - The data being read is **immutable** after the keyring is initialized,
360+
* so concurrent locked operations cannot alter it while this callback
361+
* runs.
362+
*
363+
* Do **not** use this method to:
364+
* - Mutate keyring state (add accounts, sign, etc.) — use `withKeyringV2`.
365+
* - Read mutable fields that could change during concurrent operations.
366+
*
367+
* @param selector - Keyring selector object.
368+
* @param operation - Read-only function to execute with the wrapped V2 keyring.
369+
* @returns Promise resolving to the result of the function execution.
370+
* @template SelectedKeyring - The type of the selected V2 keyring.
371+
* @template CallbackResult - The type of the value resolved by the callback function.
372+
*/
373+
export type KeyringControllerWithKeyringV2UnsafeAction = {
374+
type: `KeyringController:withKeyringV2Unsafe`;
375+
handler: KeyringController['withKeyringV2Unsafe'];
376+
};
377+
313378
/**
314379
* Union of all KeyringController action types.
315380
*/
@@ -334,4 +399,6 @@ export type KeyringControllerMethodActions =
334399
| KeyringControllerPatchUserOperationAction
335400
| KeyringControllerSignUserOperationAction
336401
| KeyringControllerWithKeyringAction
337-
| KeyringControllerWithKeyringUnsafeAction;
402+
| KeyringControllerWithKeyringUnsafeAction
403+
| KeyringControllerWithKeyringV2Action
404+
| KeyringControllerWithKeyringV2UnsafeAction;

0 commit comments

Comments
 (0)