-
-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add keyring state migration framework #505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
94a742a
3361c30
091e618
c12fe95
ad0c009
3d24a0b
1f544b7
53fcb9f
bfd9445
08461ac
1ff2a37
cd11da6
6920d01
be2b5cb
16c0d01
3aade3f
8cd7239
e217cc8
872f1dd
f1f4f53
9ccbf0a
03fb855
e4eabba
a63e0a8
0739302
57e11c3
1d6d984
d04146a
2901740
2f754ad
b849a10
e84fb6f
2aa615e
776ca1f
200a55c
b23dc4e
3ab5aeb
de9280f
28a63ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| # Keyring State Migrations | ||
|
|
||
| A framework for evolving keyring serialized state across versions. Migrations run during `deserialize()` and transform old state into the current format. | ||
|
|
||
| Versioned state is stored as an envelope: | ||
|
|
||
| ```json | ||
| { | ||
| "version": 2, | ||
| "data": { "accountCount": 3, "mnemonic": [...], "hdPath": "m/44'/60'/0'/0" } | ||
| } | ||
| ``` | ||
|
|
||
| Unversioned state (vaults created before migration support was added) has no envelope. The framework treats it as version 0 and applies all migrations. | ||
|
|
||
| ## Key Concepts | ||
|
|
||
| - **`schema`**: Validates the **output** of a migration at runtime. | ||
| - **`inputSchema`**: (Optional) Validates the **input** before the `migrate` function is called. | ||
| - **`defineMigration<Output, Input>`**: Ensures the `migrate` function's return type matches the `schema` type at compile time. | ||
| - **`as const` arrays**: Use an array with `as const` to allow TypeScript to infer the final state type via `applyMigrations`. | ||
|
|
||
| ## Example | ||
|
|
||
| ### 1. Define State Schemas | ||
|
|
||
| Define a schema for each version of your state. | ||
|
|
||
| ```typescript | ||
| import { object, array, number, string } from '@metamask/superstruct'; | ||
| import type { Infer } from '@metamask/superstruct'; | ||
|
|
||
| const HdStateV0Schema = object({ | ||
| numberOfAccounts: number(), // legacy field name | ||
| mnemonic: array(number()), | ||
| hdPath: string(), | ||
| }); | ||
| type HdStateV0 = Infer<typeof HdStateV0Schema>; | ||
|
|
||
| const HdStateV1Schema = object({ | ||
| accountCount: number(), // renamed from numberOfAccounts | ||
| mnemonic: array(number()), | ||
| hdPath: string(), | ||
| }); | ||
| type HdStateV1 = Infer<typeof HdStateV1Schema>; | ||
|
|
||
| const HdStateV2Schema = object({ | ||
| accountCount: number(), | ||
| mnemonic: array(number()), | ||
| hdPath: string(), | ||
| createdAt: number(), // new field | ||
| }); | ||
| type HdStateV2 = Infer<typeof HdStateV2Schema>; | ||
|
Comment on lines
+33
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: But would be nice to enforce that all schemas are JSON-compatible too (not sure we have something ready for this). Maybe we don't need to enforce this in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's already the case (at least in compile-time), |
||
| ``` | ||
|
|
||
| ### 2. Define the Migration Chain | ||
|
|
||
| ```typescript | ||
| import { defineMigration } from '@metamask/keyring-sdk'; | ||
|
|
||
| const migrations = [ | ||
| defineMigration({ | ||
| version: 1, | ||
| inputSchema: HdStateV0Schema, | ||
| schema: HdStateV1Schema, | ||
| migrate: (data) => ({ | ||
| accountCount: data.numberOfAccounts, | ||
| mnemonic: data.mnemonic, | ||
| hdPath: data.hdPath, | ||
| }), | ||
| }), | ||
| defineMigration<HdStateV2, HdStateV1>({ | ||
| version: 2, | ||
| schema: HdStateV2Schema, | ||
| migrate: (data) => ({ ...data, createdAt: Date.now() }), | ||
| }), | ||
| ] as const; | ||
| ``` | ||
|
|
||
| ### 3. Implement in your Keyring | ||
|
|
||
| ```typescript | ||
| import { applyMigrations, getLatestVersion } from '@metamask/keyring-sdk'; | ||
| import type { VersionedState } from '@metamask/keyring-sdk'; | ||
| import type { Json } from '@metamask/utils'; | ||
|
|
||
| class MyKeyring { | ||
| async deserialize(state: Json): Promise<void> { | ||
| const { data } = await applyMigrations(state, migrations); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about renaming this just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // data is typed as HdStateV2 | ||
| this.#mnemonic = data.mnemonic; | ||
| this.#accountCount = data.accountCount; | ||
| this.#hdPath = data.hdPath; | ||
| } | ||
|
|
||
| async serialize(): Promise<VersionedState<HdStateV2>> { | ||
| return { | ||
| version: getLatestVersion(migrations), | ||
| data: { | ||
| mnemonic: this.#mnemonic, | ||
| accountCount: this.#accountCount, | ||
| hdPath: this.#hdPath, | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Best Practices | ||
|
|
||
| - **`as const` arrays**: Declare the migrations array with `as const` so TypeScript infers the final state type from `applyMigrations`. Without it, `data` falls back to `Json`. | ||
| - **Idempotent migrations**: Design migrations so re-running them on already-migrated data is harmless. | ||
| - **Immutability**: Treat the input `data` as immutable within the `migrate` function. | ||
| - **Schema coverage**: Ensure `schema` covers all fields expected in the new version to prevent runtime errors. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - **Sequential versions**: migrations must be numbered 1, 2, 3, ... with no gaps. | ||
| - **Forward-only**: there is no downgrade path; code that does not understand the versioned envelope will fail on migrated state. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| export * from './keyring-account-registry'; | ||
| export * from './migration'; | ||
| export * from './mnemonic'; | ||
| export * from './eth'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have expected to have input first, then output? But I guess it's because we want to have the type for the right version "first"?
Like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's intentional since
Outputis the primary type.Inputcan be omitted and it will default toJson.