From 8c1e9e536ad855be17135715bc6e55d3a854bc35 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 15 Apr 2026 19:04:29 -0700 Subject: [PATCH 01/18] feat(wallet): Add split persistence backed by better-sqlite3 Persist controller state to SQLite using a per-property key-value scheme (one row per ControllerName.propertyName). Writes are synchronous within the same call stack as controller state updates, using stateChanged events and Immer patches to write only changed, persist-flagged properties. Defaults to ':memory:' when no database path is provided. Co-Authored-By: Claude Opus 4.6 --- packages/wallet/package.json | 4 +- packages/wallet/src/Wallet.ts | 32 +- packages/wallet/src/index.ts | 1 + .../src/persistence/KeyValueStore.test.ts | 80 ++++ .../wallet/src/persistence/KeyValueStore.ts | 62 +++ packages/wallet/src/persistence/index.ts | 2 + .../src/persistence/persistence.test.ts | 352 ++++++++++++++++++ .../wallet/src/persistence/persistence.ts | 155 ++++++++ yarn.lock | 264 ++++++++++++- 9 files changed, 940 insertions(+), 12 deletions(-) create mode 100644 packages/wallet/src/persistence/KeyValueStore.test.ts create mode 100644 packages/wallet/src/persistence/KeyValueStore.ts create mode 100644 packages/wallet/src/persistence/index.ts create mode 100644 packages/wallet/src/persistence/persistence.test.ts create mode 100644 packages/wallet/src/persistence/persistence.ts diff --git a/packages/wallet/package.json b/packages/wallet/package.json index 0ad8e733fae..d3ca0fb194f 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -65,12 +65,14 @@ "@metamask/remote-feature-flag-controller": "^4.2.0", "@metamask/scure-bip39": "^2.1.1", "@metamask/transaction-controller": "^64.3.0", - "@metamask/utils": "^11.9.0" + "@metamask/utils": "^11.9.0", + "better-sqlite3": "^11.9.1" }, "devDependencies": { "@metamask/auto-changelog": "^6.1.0", "@metamask/foundryup": "^1.0.1", "@ts-bridge/cli": "^0.6.4", + "@types/better-sqlite3": "^7.6.13", "@types/jest": "^29.5.14", "deepmerge": "^4.2.2", "jest": "^29.7.0", diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index 3f60491b496..663ee5b9c0f 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -1,5 +1,4 @@ import { Messenger } from '@metamask/messenger'; -import type { Json } from '@metamask/utils'; import type { DefaultActions, @@ -9,11 +8,12 @@ import type { RootMessenger, } from './initialization'; import { initialize } from './initialization'; +import { KeyValueStore, loadState, subscribeToChanges } from './persistence'; import type { WalletOptions } from './types'; export type WalletConstructorArgs = { - state?: Record; options: WalletOptions; + databasePath?: string; }; export class Wallet { @@ -22,12 +22,30 @@ export class Wallet { readonly #instances: DefaultInstances; - constructor({ state = {}, options }: WalletConstructorArgs) { + readonly #store: KeyValueStore; + + readonly #unsubscribePersistence: () => void; + + constructor({ options, databasePath = ':memory:' }: WalletConstructorArgs) { this.messenger = new Messenger({ namespace: 'Root', }); - this.#instances = initialize({ state, messenger: this.messenger, options }); + this.#store = new KeyValueStore(databasePath); + + const state = loadState(this.#store); + + this.#instances = initialize({ + state, + messenger: this.messenger, + options, + }); + + this.#unsubscribePersistence = subscribeToChanges( + this.messenger, + this.#instances, + this.#store, + ); } get state(): DefaultState { @@ -41,16 +59,18 @@ export class Wallet { } async destroy(): Promise { + this.#unsubscribePersistence(); + await Promise.all( Object.values(this.#instances).map((instance) => { - // @ts-expect-error Accessing protected property. if (typeof instance.destroy === 'function') { - // @ts-expect-error Accessing protected property. return instance.destroy(); } /* istanbul ignore next */ return undefined; }), ); + + this.#store.close(); } } diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index a3db3b1b449..d64b0e79fc8 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1 +1,2 @@ export { Wallet } from './Wallet'; +export { KeyValueStore } from './persistence'; diff --git a/packages/wallet/src/persistence/KeyValueStore.test.ts b/packages/wallet/src/persistence/KeyValueStore.test.ts new file mode 100644 index 00000000000..de3143593ec --- /dev/null +++ b/packages/wallet/src/persistence/KeyValueStore.test.ts @@ -0,0 +1,80 @@ +import type { Json } from '@metamask/utils'; + +import { KeyValueStore } from './KeyValueStore'; + +describe('KeyValueStore', () => { + let store: KeyValueStore; + + beforeEach(() => { + store = new KeyValueStore(':memory:'); + }); + + afterEach(() => { + store.close(); + }); + + describe('set and get', () => { + it('stores and retrieves a string value', () => { + store.set('key1', 'hello'); + expect(store.get('key1')).toBe('hello'); + }); + + it('stores and retrieves a number value', () => { + store.set('key1', 42); + expect(store.get('key1')).toBe(42); + }); + + it('stores and retrieves a boolean value', () => { + store.set('key1', true); + expect(store.get('key1')).toBe(true); + }); + + it('stores and retrieves null', () => { + store.set('key1', null); + expect(store.get('key1')).toBeNull(); + }); + + it('stores and retrieves a complex object', () => { + const makeValue = (): Json => ({ + nested: { array: [1, 'two', null, { deep: true }] }, + }); + store.set('key1', makeValue()); + expect(store.get('key1')).toStrictEqual(makeValue()); + }); + + it('returns undefined for a nonexistent key', () => { + expect(store.get('missing')).toBeUndefined(); + }); + + it('overwrites an existing key', () => { + store.set('key1', 'first'); + store.set('key1', 'second'); + expect(store.get('key1')).toBe('second'); + }); + }); + + describe('getAll', () => { + it('returns an empty object when the store is empty', () => { + expect(store.getAll()).toStrictEqual({}); + }); + + it('returns all stored key-value pairs', () => { + store.set('a', 1); + store.set('b', 'two'); + store.set('c', [3]); + expect(store.getAll()).toStrictEqual({ a: 1, b: 'two', c: [3] }); + }); + }); + + describe('delete', () => { + it('removes an existing key', () => { + store.set('key1', 'value'); + store.delete('key1'); + expect(store.get('key1')).toBeUndefined(); + }); + + it('does nothing when deleting a nonexistent key', () => { + expect(() => store.delete('missing')).not.toThrow(); + }); + }); +}); diff --git a/packages/wallet/src/persistence/KeyValueStore.ts b/packages/wallet/src/persistence/KeyValueStore.ts new file mode 100644 index 00000000000..4bc6aebf01c --- /dev/null +++ b/packages/wallet/src/persistence/KeyValueStore.ts @@ -0,0 +1,62 @@ +import Database from 'better-sqlite3'; +import type { Json } from '@metamask/utils'; + +/** + * A synchronous key-value store backed by better-sqlite3. + * + * Uses a single `kv` table with TEXT key (primary key) and TEXT value + * (JSON-serialized). Intended as the persistence backend for wallet + * controller state. + */ +export class KeyValueStore { + readonly #db: Database.Database; + + readonly #getStmt: Database.Statement; + + readonly #setStmt: Database.Statement; + + readonly #deleteStmt: Database.Statement; + + readonly #getAllStmt: Database.Statement; + + constructor(databasePath: string) { + this.#db = new Database(databasePath); + this.#db.pragma('journal_mode = WAL'); + this.#db.exec( + 'CREATE TABLE IF NOT EXISTS kv (key TEXT PRIMARY KEY, value TEXT NOT NULL)', + ); + + this.#getStmt = this.#db.prepare('SELECT value FROM kv WHERE key = ?'); + this.#setStmt = this.#db.prepare( + 'INSERT OR REPLACE INTO kv (key, value) VALUES (?, ?)', + ); + this.#deleteStmt = this.#db.prepare('DELETE FROM kv WHERE key = ?'); + this.#getAllStmt = this.#db.prepare('SELECT key, value FROM kv'); + } + + get(key: string): Json | undefined { + const row = this.#getStmt.get(key) as { value: string } | undefined; + return row ? (JSON.parse(row.value) as Json) : undefined; + } + + set(key: string, value: Json): void { + this.#setStmt.run(key, JSON.stringify(value)); + } + + getAll(): Record { + const rows = this.#getAllStmt.all() as { key: string; value: string }[]; + const result: Record = {}; + for (const row of rows) { + result[row.key] = JSON.parse(row.value) as Json; + } + return result; + } + + delete(key: string): void { + this.#deleteStmt.run(key); + } + + close(): void { + this.#db.close(); + } +} diff --git a/packages/wallet/src/persistence/index.ts b/packages/wallet/src/persistence/index.ts new file mode 100644 index 00000000000..08081afc4b3 --- /dev/null +++ b/packages/wallet/src/persistence/index.ts @@ -0,0 +1,2 @@ +export { KeyValueStore } from './KeyValueStore'; +export { loadState, subscribeToChanges } from './persistence'; diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts new file mode 100644 index 00000000000..0cc84efced3 --- /dev/null +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -0,0 +1,352 @@ +import type { Json } from '@metamask/utils'; + +import { KeyValueStore } from './KeyValueStore'; +import { loadState, subscribeToChanges } from './persistence'; +import type { RootMessenger, DefaultInstances } from '../initialization'; + +describe('loadState', () => { + let store: KeyValueStore; + + beforeEach(() => { + store = new KeyValueStore(':memory:'); + }); + + afterEach(() => { + store.close(); + }); + + it('returns an empty object when the store is empty', () => { + expect(loadState(store)).toStrictEqual({}); + }); + + it('groups keys by controller name', () => { + store.set('ControllerA.prop1', 'value1'); + store.set('ControllerA.prop2', 42); + store.set('ControllerB.prop1', [1, 2, 3]); + + expect(loadState(store)).toStrictEqual({ + ControllerA: { prop1: 'value1', prop2: 42 }, + ControllerB: { prop1: [1, 2, 3] }, + }); + }); + + it('splits on the first dot only', () => { + store.set('Controller.prop.with.dots', 'value'); + + expect(loadState(store)).toStrictEqual({ + Controller: { 'prop.with.dots': 'value' }, + }); + }); + + it('throws on a key without a dot separator', () => { + store.set('noDot', 'value'); + + expect(() => loadState(store)).toThrow( + "Invalid key in store: 'noDot'. Expected format 'ControllerName.propertyName'.", + ); + }); +}); + +describe('subscribeToChanges', () => { + let store: KeyValueStore; + + beforeEach(() => { + store = new KeyValueStore(':memory:'); + }); + + afterEach(() => { + store.close(); + }); + + it('writes persist-flagged properties on state change', () => { + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + persisted: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + transient: { + persist: false, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { persisted: 'savedValue', transient: 'notSaved' }, + patches: [ + { op: 'replace', path: ['persisted'], value: 'savedValue' }, + { op: 'replace', path: ['transient'], value: 'notSaved' }, + ], + }); + + expect(store.get('TestController.persisted')).toBe('savedValue'); + expect(store.get('TestController.transient')).toBeUndefined(); + }); + + it('only writes properties that are in the patches', () => { + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + propA: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + propB: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { propA: 'changedA', propB: 'unchangedB' }, + patches: [{ op: 'replace', path: ['propA'], value: 'changedA' }], + }); + + expect(store.get('TestController.propA')).toBe('changedA'); + expect(store.get('TestController.propB')).toBeUndefined(); + }); + + it('applies StateDeriver functions before writing', () => { + const deriver = (value: never): Json => + (value as unknown as string).toUpperCase(); + + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + derived: { + persist: deriver, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { derived: 'hello' }, + patches: [{ op: 'replace', path: ['derived'], value: 'hello' }], + }); + + expect(store.get('TestController.derived')).toBe('HELLO'); + }); + + it('handles nested property changes by extracting the top-level key', () => { + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + nested: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { nested: { inner: { deep: 'value' } } }, + patches: [ + { op: 'replace', path: ['nested', 'inner', 'deep'], value: 'value' }, + ], + }); + + expect(store.get('TestController.nested')).toStrictEqual({ + inner: { deep: 'value' }, + }); + }); + + it('skips controllers with no persisted properties', () => { + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + transientOnly: { + persist: false, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + const unsubscribe = subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { transientOnly: 'value' }, + patches: [ + { op: 'replace', path: ['transientOnly'], value: 'value' }, + ], + }); + + expect(store.getAll()).toStrictEqual({}); + unsubscribe(); + }); + + it('returns an unsubscribe function that stops persistence', () => { + const { messenger, instances } = createMockSetup({ + TestController: { + metadata: { + prop: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + const unsubscribe = subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { prop: 'first' }, + patches: [{ op: 'replace', path: ['prop'], value: 'first' }], + }); + + expect(store.get('TestController.prop')).toBe('first'); + + unsubscribe(); + + publishStateChanged(messenger, 'TestController', { + state: { prop: 'second' }, + patches: [{ op: 'replace', path: ['prop'], value: 'second' }], + }); + + expect(store.get('TestController.prop')).toBe('first'); + }); + + it('handles multiple controllers independently', () => { + const { messenger, instances } = createMockSetup({ + ControllerA: { + metadata: { + data: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + ControllerB: { + metadata: { + data: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + }, + }, + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'ControllerA', { + state: { data: 'fromA' }, + patches: [{ op: 'replace', path: ['data'], value: 'fromA' }], + }); + + publishStateChanged(messenger, 'ControllerB', { + state: { data: 'fromB' }, + patches: [{ op: 'replace', path: ['data'], value: 'fromB' }], + }); + + expect(store.get('ControllerA.data')).toBe('fromA'); + expect(store.get('ControllerB.data')).toBe('fromB'); + }); +}); + +// --- Test helpers --- + +type MockControllerConfig = { + metadata: Record< + string, + { + persist: boolean | ((value: never) => Json); + includeInDebugSnapshot: boolean; + includeInStateLogs: boolean; + usedInUi: boolean; + } + >; +}; + +type MockSetup = { + messenger: RootMessenger; + instances: DefaultInstances; +}; + +/** + * Creates a mock messenger and instances map for testing persistence wiring. + * The messenger supports subscribe/unsubscribe/publish for stateChanged events. + */ +function createMockSetup( + controllers: Record, +): MockSetup { + const handlers = new Map void>>(); + + const messenger = { + subscribe: (eventType: string, handler: (...args: unknown[]) => void) => { + if (!handlers.has(eventType)) { + handlers.set(eventType, new Set()); + } + handlers.get(eventType)?.add(handler); + }, + unsubscribe: (eventType: string, handler: (...args: unknown[]) => void) => { + handlers.get(eventType)?.delete(handler); + }, + publish: (eventType: string, ...payload: unknown[]) => { + const subs = handlers.get(eventType); + if (subs) { + for (const handler of subs) { + handler(...payload); + } + } + }, + } as unknown as RootMessenger; + + const instances: Record = {}; + for (const [name, config] of Object.entries(controllers)) { + instances[name] = { metadata: config.metadata }; + } + + return { messenger, instances: instances as unknown as DefaultInstances }; +} + +/** + * Publishes a stateChanged event on the mock messenger. + */ +function publishStateChanged( + messenger: RootMessenger, + controllerName: string, + { state, patches }: { state: Record; patches: unknown[] }, +): void { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (messenger as any).publish( + `${controllerName}:stateChanged`, + state, + patches, + ); +} diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts new file mode 100644 index 00000000000..48b4cd03b3c --- /dev/null +++ b/packages/wallet/src/persistence/persistence.ts @@ -0,0 +1,155 @@ +import type { + BaseControllerInstance, + StateMetadataConstraint, + StatePropertyMetadataConstraint, +} from '@metamask/base-controller'; +import type { Json } from '@metamask/utils'; +import type { Patch } from 'immer'; + +import type { KeyValueStore } from './KeyValueStore'; +import type { DefaultInstances, RootMessenger } from '../initialization'; + +/** + * Load persisted state from the key-value store and reconstruct it as + * a record keyed by controller name. + * + * Keys in the store follow the format `ControllerName.propertyName`. + * This function groups them into `{ [controllerName]: { [propertyName]: value } }`. + * + * @param store - The key-value store to read from. + * @returns A record of controller states, suitable for passing to `initialize()`. + */ +export function loadState(store: KeyValueStore): Record { + const allPairs = store.getAll(); + const state: Record> = {}; + + for (const [key, value] of Object.entries(allPairs)) { + const dotIndex = key.indexOf('.'); + if (dotIndex === -1) { + throw new Error( + `Invalid key in store: '${key}'. Expected format 'ControllerName.propertyName'.`, + ); + } + const controllerName = key.slice(0, dotIndex); + const propertyName = key.slice(dotIndex + 1); + + if (!state[controllerName]) { + state[controllerName] = {}; + } + state[controllerName][propertyName] = value; + } + + return state as Record; +} + +/** + * Extracts the set of top-level property names that changed from an + * array of Immer patches. + * + * @param patches - Immer patches from a state update. + * @returns A set of top-level property names that were modified. + */ +function getChangedProperties(patches: Patch[]): Set { + const changed = new Set(); + for (const patch of patches) { + if (patch.path.length > 0) { + changed.add(String(patch.path[0])); + } + } + return changed; +} + +/** + * Subscribe to all controller `stateChanged` events and persist changes + * to the key-value store. + * + * For each controller instance, this function reads the controller's metadata + * to determine which state properties are persist-flagged. When a `stateChanged` + * event fires, it uses the Immer patches to identify which top-level properties + * changed, filters to only persist-flagged properties, and writes them to the + * store. + * + * @param messenger - The root messenger to subscribe on. + * @param instances - The controller instances returned by `initialize()`. + * @param store - The key-value store to write to. + * @returns A function that unsubscribes all persistence handlers. + */ +export function subscribeToChanges( + messenger: RootMessenger, + instances: DefaultInstances, + store: KeyValueStore, +): () => void { + const unsubscribers: (() => void)[] = []; + + for (const [controllerName, instance] of Object.entries(instances)) { + const controller = instance as unknown as BaseControllerInstance; + const { metadata } = controller; + + const persistedProperties = getPersistPropertyNames(metadata); + if (persistedProperties.size === 0) { + continue; + } + + const eventType = `${controllerName}:stateChanged`; + + const handler = ( + state: Record, + patches: Patch[], + ): void => { + const changed = getChangedProperties(patches); + + for (const prop of changed) { + if (!persistedProperties.has(prop)) { + continue; + } + + const key = `${controllerName}.${prop}`; + const persistFlag = metadata[prop]?.persist; + + if (typeof persistFlag === 'function') { + store.set(key, persistFlag(state[prop] as never)); + } else { + store.set(key, state[prop] as Json); + } + } + }; + + // Type assertion needed: the event type string is dynamically constructed, + // and the messenger's type system expects a literal from DefaultEvents. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + messenger.subscribe(eventType as any, handler as any); + + unsubscribers.push(() => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + messenger.unsubscribe(eventType as any, handler as any); + }); + } + + return () => { + for (const unsubscribe of unsubscribers) { + unsubscribe(); + } + }; +} + +/** + * Get the set of property names that have `persist: true` (or a persist deriver) + * in the given state metadata. + * + * @param metadata - The controller's state metadata. + * @returns A set of property names that should be persisted. + */ +function getPersistPropertyNames( + metadata: StateMetadataConstraint, +): Set { + const names = new Set(); + for (const [key, propertyMetadata] of Object.entries(metadata) as [ + string, + StatePropertyMetadataConstraint, + ][]) { + if (propertyMetadata.persist) { + names.add(key); + } + } + return names; +} diff --git a/yarn.lock b/yarn.lock index b28ccc07915..8a5f4386ced 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5715,7 +5715,9 @@ __metadata: "@metamask/transaction-controller": "npm:^64.3.0" "@metamask/utils": "npm:^11.9.0" "@ts-bridge/cli": "npm:^0.6.4" + "@types/better-sqlite3": "npm:^7.6.13" "@types/jest": "npm:^29.5.14" + better-sqlite3: "npm:^11.9.1" deepmerge: "npm:^4.2.2" jest: "npm:^29.7.0" nock: "npm:^13.3.1" @@ -6753,6 +6755,15 @@ __metadata: languageName: node linkType: hard +"@types/better-sqlite3@npm:^7.6.13": + version: 7.6.13 + resolution: "@types/better-sqlite3@npm:7.6.13" + dependencies: + "@types/node": "npm:*" + checksum: 10/c74dafa3c550ac866737870016d7b1a735c7d450c16d40962eeb54510fa150e91752bfdf678f55e91894d8853771b95f909b0062122116cddac4d80491b74411 + languageName: node + linkType: hard + "@types/bn.js@npm:*, @types/bn.js@npm:^5.1.0, @types/bn.js@npm:^5.1.5": version: 5.1.6 resolution: "@types/bn.js@npm:5.1.6" @@ -7787,6 +7798,17 @@ __metadata: languageName: node linkType: hard +"better-sqlite3@npm:^11.9.1": + version: 11.10.0 + resolution: "better-sqlite3@npm:11.10.0" + dependencies: + bindings: "npm:^1.5.0" + node-gyp: "npm:latest" + prebuild-install: "npm:^7.1.1" + checksum: 10/5e4c7437c4fe6033335a79c82974d7ab29f33c51c36f48b73e87e087d21578468575de1c56a7badd4f76f17255e25abefddaeacf018e5eeb9e0cb8d6e3e4a5e1 + languageName: node + linkType: hard + "bignumber.js@npm:^9.1.2": version: 9.1.2 resolution: "bignumber.js@npm:9.1.2" @@ -7806,6 +7828,15 @@ __metadata: languageName: node linkType: hard +"bindings@npm:^1.5.0": + version: 1.5.0 + resolution: "bindings@npm:1.5.0" + dependencies: + file-uri-to-path: "npm:1.0.0" + checksum: 10/593d5ae975ffba15fbbb4788fe5abd1e125afbab849ab967ab43691d27d6483751805d98cb92f7ac24a2439a8a8678cd0131c535d5d63de84e383b0ce2786133 + languageName: node + linkType: hard + "bitcoin-address-validation@npm:^2.2.3": version: 2.2.3 resolution: "bitcoin-address-validation@npm:2.2.3" @@ -7817,6 +7848,17 @@ __metadata: languageName: node linkType: hard +"bl@npm:^4.0.3": + version: 4.1.0 + resolution: "bl@npm:4.1.0" + dependencies: + buffer: "npm:^5.5.0" + inherits: "npm:^2.0.4" + readable-stream: "npm:^3.4.0" + checksum: 10/b7904e66ed0bdfc813c06ea6c3e35eafecb104369dbf5356d0f416af90c1546de3b74e5b63506f0629acf5e16a6f87c3798f16233dcff086e9129383aa02ab55 + languageName: node + linkType: hard + "blakejs@npm:^1.1.0": version: 1.2.1 resolution: "blakejs@npm:1.2.1" @@ -7994,6 +8036,16 @@ __metadata: languageName: node linkType: hard +"buffer@npm:^5.5.0": + version: 5.7.1 + resolution: "buffer@npm:5.7.1" + dependencies: + base64-js: "npm:^1.3.1" + ieee754: "npm:^1.1.13" + checksum: 10/997434d3c6e3b39e0be479a80288875f71cd1c07d75a3855e6f08ef848a3c966023f79534e22e415ff3a5112708ce06127277ab20e527146d55c84566405c7c6 + languageName: node + linkType: hard + "buffer@npm:^6.0.3": version: 6.0.3 resolution: "buffer@npm:6.0.3" @@ -8139,6 +8191,13 @@ __metadata: languageName: node linkType: hard +"chownr@npm:^1.1.1": + version: 1.1.4 + resolution: "chownr@npm:1.1.4" + checksum: 10/115648f8eb38bac5e41c3857f3e663f9c39ed6480d1349977c4d96c95a47266fcacc5a5aabf3cb6c481e22d72f41992827db47301851766c4fd77ac21a4f081d + languageName: node + linkType: hard + "chownr@npm:^2.0.0": version: 2.0.0 resolution: "chownr@npm:2.0.0" @@ -8614,6 +8673,15 @@ __metadata: languageName: node linkType: hard +"decompress-response@npm:^6.0.0": + version: 6.0.0 + resolution: "decompress-response@npm:6.0.0" + dependencies: + mimic-response: "npm:^3.1.0" + checksum: 10/d377cf47e02d805e283866c3f50d3d21578b779731e8c5072d6ce8c13cc31493db1c2f6784da9d1d5250822120cefa44f1deab112d5981015f2e17444b763812 + languageName: node + linkType: hard + "dedent@npm:^1.0.0": version: 1.7.1 resolution: "dedent@npm:1.7.1" @@ -8626,6 +8694,13 @@ __metadata: languageName: node linkType: hard +"deep-extend@npm:^0.6.0": + version: 0.6.0 + resolution: "deep-extend@npm:0.6.0" + checksum: 10/7be7e5a8d468d6b10e6a67c3de828f55001b6eb515d014f7aeb9066ce36bd5717161eb47d6a0f7bed8a9083935b465bc163ee2581c8b128d29bf61092fdf57a7 + languageName: node + linkType: hard + "deep-freeze-strict@npm:^1.1.1": version: 1.1.1 resolution: "deep-freeze-strict@npm:1.1.1" @@ -8764,6 +8839,13 @@ __metadata: languageName: node linkType: hard +"detect-libc@npm:^2.0.0": + version: 2.1.2 + resolution: "detect-libc@npm:2.1.2" + checksum: 10/b736c8d97d5d46164c0d1bed53eb4e6a3b1d8530d460211e2d52f1c552875e706c58a5376854e4e54f8b828c9cada58c855288c968522eb93ac7696d65970766 + languageName: node + linkType: hard + "detect-newline@npm:^3.0.0": version: 3.1.0 resolution: "detect-newline@npm:3.1.0" @@ -8919,6 +9001,15 @@ __metadata: languageName: node linkType: hard +"end-of-stream@npm:^1.1.0, end-of-stream@npm:^1.4.1": + version: 1.4.5 + resolution: "end-of-stream@npm:1.4.5" + dependencies: + once: "npm:^1.4.0" + checksum: 10/1e0cfa6e7f49887544e03314f9dfc56a8cb6dde910cbb445983ecc2ff426fc05946df9d75d8a21a3a64f2cecfe1bf88f773952029f46756b2ed64a24e95b1fb8 + languageName: node + linkType: hard + "enhanced-resolve@npm:^5.15.0, enhanced-resolve@npm:^5.17.1": version: 5.18.0 resolution: "enhanced-resolve@npm:5.18.0" @@ -9677,6 +9768,13 @@ __metadata: languageName: node linkType: hard +"expand-template@npm:^2.0.3": + version: 2.0.3 + resolution: "expand-template@npm:2.0.3" + checksum: 10/588c19847216421ed92befb521767b7018dc88f88b0576df98cb242f20961425e96a92cbece525ef28cc5becceae5d544ae0f5b9b5e2aa05acb13716ca5b3099 + languageName: node + linkType: hard + "expand-tilde@npm:^2.0.0, expand-tilde@npm:^2.0.2": version: 2.0.2 resolution: "expand-tilde@npm:2.0.2" @@ -9937,6 +10035,13 @@ __metadata: languageName: node linkType: hard +"file-uri-to-path@npm:1.0.0": + version: 1.0.0 + resolution: "file-uri-to-path@npm:1.0.0" + checksum: 10/b648580bdd893a008c92c7ecc96c3ee57a5e7b6c4c18a9a09b44fb5d36d79146f8e442578bc0e173dc027adf3987e254ba1dfd6e3ec998b7c282873010502144 + languageName: node + linkType: hard + "fill-range@npm:^7.1.1": version: 7.1.1 resolution: "fill-range@npm:7.1.1" @@ -10098,6 +10203,13 @@ __metadata: languageName: node linkType: hard +"fs-constants@npm:^1.0.0": + version: 1.0.0 + resolution: "fs-constants@npm:1.0.0" + checksum: 10/18f5b718371816155849475ac36c7d0b24d39a11d91348cfcb308b4494824413e03572c403c86d3a260e049465518c4f0d5bd00f0371cdfcad6d4f30a85b350d + languageName: node + linkType: hard + "fs-extra@npm:^10.1.0": version: 10.1.0 resolution: "fs-extra@npm:10.1.0" @@ -10250,6 +10362,13 @@ __metadata: languageName: node linkType: hard +"github-from-package@npm:0.0.0": + version: 0.0.0 + resolution: "github-from-package@npm:0.0.0" + checksum: 10/2a091ba07fbce22205642543b4ea8aaf068397e1433c00ae0f9de36a3607baf5bcc14da97fbb798cfca6393b3c402031fca06d8b491a44206d6efef391c58537 + languageName: node + linkType: hard + "glob-parent@npm:^5.1.2": version: 5.1.2 resolution: "glob-parent@npm:5.1.2" @@ -10628,7 +10747,7 @@ __metadata: languageName: node linkType: hard -"ieee754@npm:^1.2.1": +"ieee754@npm:^1.1.13, ieee754@npm:^1.2.1": version: 1.2.1 resolution: "ieee754@npm:1.2.1" checksum: 10/d9f2557a59036f16c282aaeb107832dc957a93d73397d89bbad4eb1130560560eb695060145e8e6b3b498b15ab95510226649a0b8f52ae06583575419fe10fc4 @@ -10709,7 +10828,7 @@ __metadata: languageName: node linkType: hard -"ini@npm:^1.3.4": +"ini@npm:^1.3.4, ini@npm:~1.3.0": version: 1.3.8 resolution: "ini@npm:1.3.8" checksum: 10/314ae176e8d4deb3def56106da8002b462221c174ddb7ce0c49ee72c8cd1f9044f7b10cc555a7d8850982c3b9ca96fc212122749f5234bc2b6fb05fb942ed566 @@ -12208,6 +12327,13 @@ __metadata: languageName: node linkType: hard +"mimic-response@npm:^3.1.0": + version: 3.1.0 + resolution: "mimic-response@npm:3.1.0" + checksum: 10/7e719047612411fe071332a7498cf0448bbe43c485c0d780046c76633a771b223ff49bd00267be122cedebb897037fdb527df72335d0d0f74724604ca70b37ad + languageName: node + linkType: hard + "min-indent@npm:^1.0.0": version: 1.0.1 resolution: "min-indent@npm:1.0.1" @@ -12267,7 +12393,7 @@ __metadata: languageName: node linkType: hard -"minimist@npm:^1.2.5": +"minimist@npm:^1.2.0, minimist@npm:^1.2.3, minimist@npm:^1.2.5": version: 1.2.8 resolution: "minimist@npm:1.2.8" checksum: 10/908491b6cc15a6c440ba5b22780a0ba89b9810e1aea684e253e43c4e3b8d56ec1dcdd7ea96dde119c29df59c936cde16062159eae4225c691e19c70b432b6e6f @@ -12374,6 +12500,13 @@ __metadata: languageName: node linkType: hard +"mkdirp-classic@npm:^0.5.2, mkdirp-classic@npm:^0.5.3": + version: 0.5.3 + resolution: "mkdirp-classic@npm:0.5.3" + checksum: 10/3f4e088208270bbcc148d53b73e9a5bd9eef05ad2cbf3b3d0ff8795278d50dd1d11a8ef1875ff5aea3fa888931f95bfcb2ad5b7c1061cfefd6284d199e6776ac + languageName: node + linkType: hard + "mkdirp@npm:^1.0.3": version: 1.0.4 resolution: "mkdirp@npm:1.0.4" @@ -12449,6 +12582,13 @@ __metadata: languageName: node linkType: hard +"napi-build-utils@npm:^2.0.0": + version: 2.0.0 + resolution: "napi-build-utils@npm:2.0.0" + checksum: 10/69adcdb828481737f1ec64440286013f6479d5b264e24d5439ba795f65293d0bb6d962035de07c65fae525ed7d2fcd0baab6891d8e3734ea792fec43918acf83 + languageName: node + linkType: hard + "natural-compare@npm:^1.4.0": version: 1.4.0 resolution: "natural-compare@npm:1.4.0" @@ -12481,6 +12621,15 @@ __metadata: languageName: node linkType: hard +"node-abi@npm:^3.3.0": + version: 3.89.0 + resolution: "node-abi@npm:3.89.0" + dependencies: + semver: "npm:^7.3.5" + checksum: 10/8fc84f775475b81256cf208c0ed79fe53eca6e5f5e1fe8c263f63f0250b9454b9f3a144f389bf4dbda258f5f916ee0bbc5bb0ebf0f13de78fd80390dac0227b7 + languageName: node + linkType: hard + "node-addon-api@npm:^2.0.0": version: 2.0.2 resolution: "node-addon-api@npm:2.0.2" @@ -12702,7 +12851,7 @@ __metadata: languageName: node linkType: hard -"once@npm:^1.3.0, once@npm:^1.4.0": +"once@npm:^1.3.0, once@npm:^1.3.1, once@npm:^1.4.0": version: 1.4.0 resolution: "once@npm:1.4.0" dependencies: @@ -13124,6 +13273,28 @@ __metadata: languageName: node linkType: hard +"prebuild-install@npm:^7.1.1": + version: 7.1.3 + resolution: "prebuild-install@npm:7.1.3" + dependencies: + detect-libc: "npm:^2.0.0" + expand-template: "npm:^2.0.3" + github-from-package: "npm:0.0.0" + minimist: "npm:^1.2.3" + mkdirp-classic: "npm:^0.5.3" + napi-build-utils: "npm:^2.0.0" + node-abi: "npm:^3.3.0" + pump: "npm:^3.0.0" + rc: "npm:^1.2.7" + simple-get: "npm:^4.0.0" + tar-fs: "npm:^2.0.0" + tunnel-agent: "npm:^0.6.0" + bin: + prebuild-install: bin.js + checksum: 10/1b7e4c00d2750b532a4fc2a83ffb0c5fefa1b6f2ad071896ead15eeadc3255f5babd816949991af083cf7429e375ae8c7d1c51f73658559da36f948a020a3a11 + languageName: node + linkType: hard + "prelude-ls@npm:^1.2.1": version: 1.2.1 resolution: "prelude-ls@npm:1.2.1" @@ -13278,6 +13449,16 @@ __metadata: languageName: node linkType: hard +"pump@npm:^3.0.0": + version: 3.0.4 + resolution: "pump@npm:3.0.4" + dependencies: + end-of-stream: "npm:^1.1.0" + once: "npm:^1.3.1" + checksum: 10/d043c3e710c56ffd280711e98a94e863ab334f79ea43cee0fb70e1349b2355ffd2ff287c7522e4c960a247699d5b7825f00fa090b85d6179c973be13f78a6c49 + languageName: node + linkType: hard + "punycode@npm:2.1.0": version: 2.1.0 resolution: "punycode@npm:2.1.0" @@ -13364,6 +13545,20 @@ __metadata: languageName: node linkType: hard +"rc@npm:^1.2.7": + version: 1.2.8 + resolution: "rc@npm:1.2.8" + dependencies: + deep-extend: "npm:^0.6.0" + ini: "npm:~1.3.0" + minimist: "npm:^1.2.0" + strip-json-comments: "npm:~2.0.1" + bin: + rc: ./cli.js + checksum: 10/5c4d72ae7eec44357171585938c85ce066da8ca79146b5635baf3d55d74584c92575fa4e2c9eac03efbed3b46a0b2e7c30634c012b4b4fa40d654353d3c163eb + languageName: node + linkType: hard + "react-is@npm:^18.0.0": version: 18.3.1 resolution: "react-is@npm:18.3.1" @@ -13401,7 +13596,7 @@ __metadata: languageName: node linkType: hard -"readable-stream@npm:3.6.2, readable-stream@npm:^3.0.2, readable-stream@npm:^3.6.0, readable-stream@npm:^3.6.2": +"readable-stream@npm:3.6.2, readable-stream@npm:^3.0.2, readable-stream@npm:^3.1.1, readable-stream@npm:^3.4.0, readable-stream@npm:^3.6.0, readable-stream@npm:^3.6.2": version: 3.6.2 resolution: "readable-stream@npm:3.6.2" dependencies: @@ -13950,6 +14145,24 @@ __metadata: languageName: node linkType: hard +"simple-concat@npm:^1.0.0": + version: 1.0.1 + resolution: "simple-concat@npm:1.0.1" + checksum: 10/4d211042cc3d73a718c21ac6c4e7d7a0363e184be6a5ad25c8a1502e49df6d0a0253979e3d50dbdd3f60ef6c6c58d756b5d66ac1e05cda9cacd2e9fc59e3876a + languageName: node + linkType: hard + +"simple-get@npm:^4.0.0": + version: 4.0.1 + resolution: "simple-get@npm:4.0.1" + dependencies: + decompress-response: "npm:^6.0.0" + once: "npm:^1.3.1" + simple-concat: "npm:^1.0.0" + checksum: 10/93f1b32319782f78f2f2234e9ce34891b7ab6b990d19d8afefaa44423f5235ce2676aae42d6743fecac6c8dfff4b808d4c24fe5265be813d04769917a9a44f36 + languageName: node + linkType: hard + "simple-git-hooks@npm:^2.8.0": version: 2.11.1 resolution: "simple-git-hooks@npm:2.11.1" @@ -14272,6 +14485,13 @@ __metadata: languageName: node linkType: hard +"strip-json-comments@npm:~2.0.1": + version: 2.0.1 + resolution: "strip-json-comments@npm:2.0.1" + checksum: 10/1074ccb63270d32ca28edfb0a281c96b94dc679077828135141f27d52a5a398ef5e78bcf22809d23cadc2b81dfbe345eb5fd8699b385c8b1128907dec4a7d1e1 + languageName: node + linkType: hard + "strnum@npm:^2.2.2": version: 2.2.2 resolution: "strnum@npm:2.2.2" @@ -14338,6 +14558,31 @@ __metadata: languageName: node linkType: hard +"tar-fs@npm:^2.0.0": + version: 2.1.4 + resolution: "tar-fs@npm:2.1.4" + dependencies: + chownr: "npm:^1.1.1" + mkdirp-classic: "npm:^0.5.2" + pump: "npm:^3.0.0" + tar-stream: "npm:^2.1.4" + checksum: 10/bdf7e3cb039522e39c6dae3084b1bca8d7bcc1de1906eae4a1caea6a2250d22d26dcc234118bf879b345d91ebf250a744b196e379334a4abcbb109a78db7d3be + languageName: node + linkType: hard + +"tar-stream@npm:^2.1.4": + version: 2.2.0 + resolution: "tar-stream@npm:2.2.0" + dependencies: + bl: "npm:^4.0.3" + end-of-stream: "npm:^1.4.1" + fs-constants: "npm:^1.0.0" + inherits: "npm:^2.0.3" + readable-stream: "npm:^3.1.1" + checksum: 10/1a52a51d240c118cbcd30f7368ea5e5baef1eac3e6b793fb1a41e6cd7319296c79c0264ccc5859f5294aa80f8f00b9239d519e627b9aade80038de6f966fec6a + languageName: node + linkType: hard + "tar-stream@npm:^3.1.7": version: 3.1.7 resolution: "tar-stream@npm:3.1.7" @@ -14568,6 +14813,15 @@ __metadata: languageName: node linkType: hard +"tunnel-agent@npm:^0.6.0": + version: 0.6.0 + resolution: "tunnel-agent@npm:0.6.0" + dependencies: + safe-buffer: "npm:^5.0.1" + checksum: 10/7f0d9ed5c22404072b2ae8edc45c071772affd2ed14a74f03b4e71b4dd1a14c3714d85aed64abcaaee5fec2efc79002ba81155c708f4df65821b444abb0cfade + languageName: node + linkType: hard + "tweetnacl@npm:^1.0.3": version: 1.0.3 resolution: "tweetnacl@npm:1.0.3" From 0b122f7b29216595381ecbd7cebba878f29ce2f8 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 15 Apr 2026 19:45:34 -0700 Subject: [PATCH 02/18] fix(wallet): Restore @ts-expect-error for protected destroy access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build tsconfig is stricter than the dev tsconfig — the union type of controller instances does not expose the protected `destroy` method. Co-Authored-By: Claude Opus 4.6 --- packages/wallet/src/Wallet.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index 663ee5b9c0f..d22d7842048 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -63,7 +63,9 @@ export class Wallet { await Promise.all( Object.values(this.#instances).map((instance) => { + // @ts-expect-error Accessing protected property. if (typeof instance.destroy === 'function') { + // @ts-expect-error Accessing protected property. return instance.destroy(); } /* istanbul ignore next */ From 331ef42e666026e72019f204bf01a52f32a42830 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 15 Apr 2026 19:45:58 -0700 Subject: [PATCH 03/18] chore: Bump better-sqlite3 --- package.json | 3 ++- packages/wallet/README.md | 10 ++++++++++ packages/wallet/package.json | 2 +- yarn.lock | 10 +++++----- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 630047a945f..91f75aa2c98 100644 --- a/package.json +++ b/package.json @@ -113,7 +113,8 @@ "babel-runtime>core-js": false, "simple-git-hooks": false, "tsx>esbuild": false, - "eslint-plugin-import-x>unrs-resolver": false + "eslint-plugin-import-x>unrs-resolver": false, + "better-sqlite3": true } } } diff --git a/packages/wallet/README.md b/packages/wallet/README.md index da275a947df..f8e976b6042 100644 --- a/packages/wallet/README.md +++ b/packages/wallet/README.md @@ -10,6 +10,16 @@ or `npm install @metamask/wallet` +## Troubleshooting + +### Rebuilding `better-sqlite3` + +This package depends on `better-sqlite3`, which includes a native C addon. The prebuilt binary is downloaded automatically during `yarn install`. If you switch Node versions without reinstalling, rebuild it with: + +```sh +yarn rebuild better-sqlite3 +``` + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/wallet/package.json b/packages/wallet/package.json index d3ca0fb194f..dda917a0261 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -66,7 +66,7 @@ "@metamask/scure-bip39": "^2.1.1", "@metamask/transaction-controller": "^64.3.0", "@metamask/utils": "^11.9.0", - "better-sqlite3": "^11.9.1" + "better-sqlite3": "^12.9.0" }, "devDependencies": { "@metamask/auto-changelog": "^6.1.0", diff --git a/yarn.lock b/yarn.lock index 8a5f4386ced..f38b0532be9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5717,7 +5717,7 @@ __metadata: "@ts-bridge/cli": "npm:^0.6.4" "@types/better-sqlite3": "npm:^7.6.13" "@types/jest": "npm:^29.5.14" - better-sqlite3: "npm:^11.9.1" + better-sqlite3: "npm:^12.9.0" deepmerge: "npm:^4.2.2" jest: "npm:^29.7.0" nock: "npm:^13.3.1" @@ -7798,14 +7798,14 @@ __metadata: languageName: node linkType: hard -"better-sqlite3@npm:^11.9.1": - version: 11.10.0 - resolution: "better-sqlite3@npm:11.10.0" +"better-sqlite3@npm:^12.9.0": + version: 12.9.0 + resolution: "better-sqlite3@npm:12.9.0" dependencies: bindings: "npm:^1.5.0" node-gyp: "npm:latest" prebuild-install: "npm:^7.1.1" - checksum: 10/5e4c7437c4fe6033335a79c82974d7ab29f33c51c36f48b73e87e087d21578468575de1c56a7badd4f76f17255e25abefddaeacf018e5eeb9e0cb8d6e3e4a5e1 + checksum: 10/0b32b06140f2a98ce7fbcf7d30b56b46e16d0b6fbfb63157a7bb61dea7265e176ab362d439785e852716a03a130db5f0ab356b6a68cbcb23d59a362c1a8b01d4 languageName: node linkType: hard From 283accaf002271c4215b9bc0476ef906a3efb574 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 10:45:26 -0700 Subject: [PATCH 04/18] chore: lint --- .../wallet/src/persistence/KeyValueStore.ts | 14 +++++++------- .../src/persistence/persistence.test.ts | 19 +++++++++++-------- .../wallet/src/persistence/persistence.ts | 13 +++---------- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/packages/wallet/src/persistence/KeyValueStore.ts b/packages/wallet/src/persistence/KeyValueStore.ts index 4bc6aebf01c..35a683624df 100644 --- a/packages/wallet/src/persistence/KeyValueStore.ts +++ b/packages/wallet/src/persistence/KeyValueStore.ts @@ -1,5 +1,5 @@ -import Database from 'better-sqlite3'; import type { Json } from '@metamask/utils'; +import BetterSqlite3 from 'better-sqlite3'; /** * A synchronous key-value store backed by better-sqlite3. @@ -9,18 +9,18 @@ import type { Json } from '@metamask/utils'; * controller state. */ export class KeyValueStore { - readonly #db: Database.Database; + readonly #db: BetterSqlite3.Database; - readonly #getStmt: Database.Statement; + readonly #getStmt: BetterSqlite3.Statement; - readonly #setStmt: Database.Statement; + readonly #setStmt: BetterSqlite3.Statement; - readonly #deleteStmt: Database.Statement; + readonly #deleteStmt: BetterSqlite3.Statement; - readonly #getAllStmt: Database.Statement; + readonly #getAllStmt: BetterSqlite3.Statement; constructor(databasePath: string) { - this.#db = new Database(databasePath); + this.#db = new BetterSqlite3(databasePath); this.#db.pragma('journal_mode = WAL'); this.#db.exec( 'CREATE TABLE IF NOT EXISTS kv (key TEXT PRIMARY KEY, value TEXT NOT NULL)', diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts index 0cc84efced3..9c08851d635 100644 --- a/packages/wallet/src/persistence/persistence.test.ts +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -196,9 +196,7 @@ describe('subscribeToChanges', () => { publishStateChanged(messenger, 'TestController', { state: { transientOnly: 'value' }, - patches: [ - { op: 'replace', path: ['transientOnly'], value: 'value' }, - ], + patches: [{ op: 'replace', path: ['transientOnly'], value: 'value' }], }); expect(store.getAll()).toStrictEqual({}); @@ -301,6 +299,9 @@ type MockSetup = { /** * Creates a mock messenger and instances map for testing persistence wiring. * The messenger supports subscribe/unsubscribe/publish for stateChanged events. + * + * @param controllers - Map of controller names to their mock configurations. + * @returns A mock messenger and instances map. */ function createMockSetup( controllers: Record, @@ -337,6 +338,12 @@ function createMockSetup( /** * Publishes a stateChanged event on the mock messenger. + * + * @param messenger - The mock messenger to publish on. + * @param controllerName - The name of the controller whose state changed. + * @param options0 - The state and patches to publish. + * @param options0.state - The new controller state. + * @param options0.patches - The Immer patches describing the state change. */ function publishStateChanged( messenger: RootMessenger, @@ -344,9 +351,5 @@ function publishStateChanged( { state, patches }: { state: Record; patches: unknown[] }, ): void { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (messenger as any).publish( - `${controllerName}:stateChanged`, - state, - patches, - ); + (messenger as any).publish(`${controllerName}:stateChanged`, state, patches); } diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index 48b4cd03b3c..58952648c80 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -1,7 +1,6 @@ import type { BaseControllerInstance, StateMetadataConstraint, - StatePropertyMetadataConstraint, } from '@metamask/base-controller'; import type { Json } from '@metamask/utils'; import type { Patch } from 'immer'; @@ -92,10 +91,7 @@ export function subscribeToChanges( const eventType = `${controllerName}:stateChanged`; - const handler = ( - state: Record, - patches: Patch[], - ): void => { + const handler = (state: Record, patches: Patch[]): void => { const changed = getChangedProperties(patches); for (const prop of changed) { @@ -109,7 +105,7 @@ export function subscribeToChanges( if (typeof persistFlag === 'function') { store.set(key, persistFlag(state[prop] as never)); } else { - store.set(key, state[prop] as Json); + store.set(key, state[prop]); } } }; @@ -143,10 +139,7 @@ function getPersistPropertyNames( metadata: StateMetadataConstraint, ): Set { const names = new Set(); - for (const [key, propertyMetadata] of Object.entries(metadata) as [ - string, - StatePropertyMetadataConstraint, - ][]) { + for (const [key, propertyMetadata] of Object.entries(metadata)) { if (propertyMetadata.persist) { names.add(key); } From 18b57d1dbcdec57e151d64c2c3fd463b9f3173e9 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 11:37:38 -0700 Subject: [PATCH 05/18] docs: Fix better-sqlite3 rebuild instructions --- packages/wallet/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/wallet/README.md b/packages/wallet/README.md index f8e976b6042..bc3cf7e5c1b 100644 --- a/packages/wallet/README.md +++ b/packages/wallet/README.md @@ -14,10 +14,10 @@ or ### Rebuilding `better-sqlite3` -This package depends on `better-sqlite3`, which includes a native C addon. The prebuilt binary is downloaded automatically during `yarn install`. If you switch Node versions without reinstalling, rebuild it with: +This package depends on `better-sqlite3`, which includes a native C addon. The prebuilt binary is downloaded automatically during `yarn install`. If you switch Node versions or branches and the binding is missing, rebuild it with: ```sh -yarn rebuild better-sqlite3 +cd node_modules/better-sqlite3 && npx prebuild-install ``` ## Contributing From b6b059667496107c687ddc635f45a592798f2d52 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 11:44:59 -0700 Subject: [PATCH 06/18] refactor: Tweak sqlite imports --- packages/wallet/src/persistence/KeyValueStore.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/wallet/src/persistence/KeyValueStore.ts b/packages/wallet/src/persistence/KeyValueStore.ts index 35a683624df..516e1989bfb 100644 --- a/packages/wallet/src/persistence/KeyValueStore.ts +++ b/packages/wallet/src/persistence/KeyValueStore.ts @@ -1,5 +1,5 @@ import type { Json } from '@metamask/utils'; -import BetterSqlite3 from 'better-sqlite3'; +import Sqlite from 'better-sqlite3'; /** * A synchronous key-value store backed by better-sqlite3. @@ -9,18 +9,18 @@ import BetterSqlite3 from 'better-sqlite3'; * controller state. */ export class KeyValueStore { - readonly #db: BetterSqlite3.Database; + readonly #db: Sqlite.Database; - readonly #getStmt: BetterSqlite3.Statement; + readonly #getStmt: Sqlite.Statement; - readonly #setStmt: BetterSqlite3.Statement; + readonly #setStmt: Sqlite.Statement; - readonly #deleteStmt: BetterSqlite3.Statement; + readonly #deleteStmt: Sqlite.Statement; - readonly #getAllStmt: BetterSqlite3.Statement; + readonly #getAllStmt: Sqlite.Statement; constructor(databasePath: string) { - this.#db = new BetterSqlite3(databasePath); + this.#db = new Sqlite(databasePath); this.#db.pragma('journal_mode = WAL'); this.#db.exec( 'CREATE TABLE IF NOT EXISTS kv (key TEXT PRIMARY KEY, value TEXT NOT NULL)', From 428d427739dade9e182c8f73a9f0904dbfdf9073 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 12:27:36 -0700 Subject: [PATCH 07/18] refactor: Tweak types, simplify tests --- .../wallet/src/persistence/KeyValueStore.ts | 16 +- .../src/persistence/persistence.test.ts | 162 ++++++------------ .../wallet/src/persistence/persistence.ts | 46 +++-- 3 files changed, 83 insertions(+), 141 deletions(-) diff --git a/packages/wallet/src/persistence/KeyValueStore.ts b/packages/wallet/src/persistence/KeyValueStore.ts index 516e1989bfb..2da7f38ab66 100644 --- a/packages/wallet/src/persistence/KeyValueStore.ts +++ b/packages/wallet/src/persistence/KeyValueStore.ts @@ -11,13 +11,13 @@ import Sqlite from 'better-sqlite3'; export class KeyValueStore { readonly #db: Sqlite.Database; - readonly #getStmt: Sqlite.Statement; + readonly #getStmt: Sqlite.Statement<[string], { value: string } | undefined>; - readonly #setStmt: Sqlite.Statement; + readonly #setStmt: Sqlite.Statement<[string, string], void>; - readonly #deleteStmt: Sqlite.Statement; + readonly #deleteStmt: Sqlite.Statement<[string], void>; - readonly #getAllStmt: Sqlite.Statement; + readonly #getAllStmt: Sqlite.Statement<[], { key: string; value: string }>; constructor(databasePath: string) { this.#db = new Sqlite(databasePath); @@ -35,8 +35,8 @@ export class KeyValueStore { } get(key: string): Json | undefined { - const row = this.#getStmt.get(key) as { value: string } | undefined; - return row ? (JSON.parse(row.value) as Json) : undefined; + const row = this.#getStmt.get(key); + return row ? JSON.parse(row.value) : undefined; } set(key: string, value: Json): void { @@ -44,10 +44,10 @@ export class KeyValueStore { } getAll(): Record { - const rows = this.#getAllStmt.all() as { key: string; value: string }[]; + const rows = this.#getAllStmt.all(); const result: Record = {}; for (const row of rows) { - result[row.key] = JSON.parse(row.value) as Json; + result[row.key] = JSON.parse(row.value); } return result; } diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts index 9c08851d635..c8c7001ca6b 100644 --- a/packages/wallet/src/persistence/persistence.test.ts +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -59,23 +59,11 @@ describe('subscribeToChanges', () => { }); it('writes persist-flagged properties on state change', () => { - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - persisted: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - transient: { - persist: false, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([ + ['persisted', true], + ['transient', false], + ]), }); subscribeToChanges(messenger, instances, store); @@ -93,23 +81,11 @@ describe('subscribeToChanges', () => { }); it('only writes properties that are in the patches', () => { - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - propA: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - propB: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([ + ['propA', true], + ['propB', true], + ]), }); subscribeToChanges(messenger, instances, store); @@ -127,17 +103,8 @@ describe('subscribeToChanges', () => { const deriver = (value: never): Json => (value as unknown as string).toUpperCase(); - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - derived: { - persist: deriver, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['derived', deriver]]), }); subscribeToChanges(messenger, instances, store); @@ -151,17 +118,8 @@ describe('subscribeToChanges', () => { }); it('handles nested property changes by extracting the top-level key', () => { - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - nested: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['nested', true]]), }); subscribeToChanges(messenger, instances, store); @@ -179,17 +137,8 @@ describe('subscribeToChanges', () => { }); it('skips controllers with no persisted properties', () => { - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - transientOnly: { - persist: false, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['transientOnly', false]]), }); const unsubscribe = subscribeToChanges(messenger, instances, store); @@ -204,17 +153,8 @@ describe('subscribeToChanges', () => { }); it('returns an unsubscribe function that stops persistence', () => { - const { messenger, instances } = createMockSetup({ - TestController: { - metadata: { - prop: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['prop', true]]), }); const unsubscribe = subscribeToChanges(messenger, instances, store); @@ -237,27 +177,9 @@ describe('subscribeToChanges', () => { }); it('handles multiple controllers independently', () => { - const { messenger, instances } = createMockSetup({ - ControllerA: { - metadata: { - data: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, - ControllerB: { - metadata: { - data: { - persist: true, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - }, - }, + const { messenger, instances } = createMockControllers({ + ControllerA: createStateMetadata([['data', true]]), + ControllerB: createStateMetadata([['data', true]]), }); subscribeToChanges(messenger, instances, store); @@ -277,8 +199,6 @@ describe('subscribeToChanges', () => { }); }); -// --- Test helpers --- - type MockControllerConfig = { metadata: Record< string, @@ -291,11 +211,35 @@ type MockControllerConfig = { >; }; -type MockSetup = { +type MockControllers = { messenger: RootMessenger; instances: DefaultInstances; }; +/** + * Creates a state metadata object for a mock controller. + * + * @param properties - An array of [property name, persist value] pairs. + * @returns A state metadata object. + */ +function createStateMetadata( + properties: [string, boolean | ((value: never) => Json)][], +): MockControllerConfig { + return { + metadata: Object.fromEntries( + properties.map(([name, persist]) => [ + name, + { + persist, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + ]), + ), + }; +} + /** * Creates a mock messenger and instances map for testing persistence wiring. * The messenger supports subscribe/unsubscribe/publish for stateChanged events. @@ -303,9 +247,9 @@ type MockSetup = { * @param controllers - Map of controller names to their mock configurations. * @returns A mock messenger and instances map. */ -function createMockSetup( +function createMockControllers( controllers: Record, -): MockSetup { +): MockControllers { const handlers = new Map void>>(); const messenger = { @@ -341,15 +285,15 @@ function createMockSetup( * * @param messenger - The mock messenger to publish on. * @param controllerName - The name of the controller whose state changed. - * @param options0 - The state and patches to publish. - * @param options0.state - The new controller state. - * @param options0.patches - The Immer patches describing the state change. + * @param options - The state and patches to publish. + * @param options.state - The new controller state. + * @param options.patches - The Immer patches describing the state change. */ function publishStateChanged( messenger: RootMessenger, controllerName: string, { state, patches }: { state: Record; patches: unknown[] }, ): void { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (messenger as any).publish(`${controllerName}:stateChanged`, state, patches); + // @ts-expect-error Event type is dynamically constructed, but we know it's valid. + messenger.publish(`${controllerName}:stateChanged`, state, patches); } diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index 58952648c80..344e9e5abd7 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -38,24 +38,7 @@ export function loadState(store: KeyValueStore): Record { state[controllerName][propertyName] = value; } - return state as Record; -} - -/** - * Extracts the set of top-level property names that changed from an - * array of Immer patches. - * - * @param patches - Immer patches from a state update. - * @returns A set of top-level property names that were modified. - */ -function getChangedProperties(patches: Patch[]): Set { - const changed = new Set(); - for (const patch of patches) { - if (patch.path.length > 0) { - changed.add(String(patch.path[0])); - } - } - return changed; + return state; } /** @@ -110,14 +93,12 @@ export function subscribeToChanges( } }; - // Type assertion needed: the event type string is dynamically constructed, - // and the messenger's type system expects a literal from DefaultEvents. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messenger.subscribe(eventType as any, handler as any); + // @ts-expect-error Event type is dynamically constructed, but we know it's valid. + messenger.subscribe(eventType, handler); unsubscribers.push(() => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messenger.unsubscribe(eventType as any, handler as any); + // @ts-expect-error Event type is dynamically constructed, but we know it's valid. + messenger.unsubscribe(eventType, handler); }); } @@ -146,3 +127,20 @@ function getPersistPropertyNames( } return names; } + +/** + * Extracts the set of top-level property names that changed from an + * array of Immer patches. + * + * @param patches - Immer patches from a state update. + * @returns A set of top-level property names that were modified. + */ +function getChangedProperties(patches: Patch[]): Set { + const changed = new Set(); + for (const patch of patches) { + if (patch.path.length > 0) { + changed.add(String(patch.path[0])); + } + } + return changed; +} From c7ca3561d8c23924a296cbab14077122f8025abe Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:07:35 -0700 Subject: [PATCH 08/18] fix: Harden persistence layer error handling and lifecycle management Address review findings: add constructor cleanup on failure, make destroy() idempotent with Promise.allSettled and finally, handle remove patches via store.delete(), catch and log handler write failures, validate degenerate store keys, handle root state replacement patches, add contextful JSON.parse errors, tighten loadState return type, extract PersistableController type and storeKey utility, remove premature KeyValueStore public export, and add tests for new behaviors. Co-Authored-By: Claude Opus 4.6 --- packages/wallet/src/Wallet.ts | 60 +++++--- packages/wallet/src/index.ts | 1 - .../wallet/src/persistence/KeyValueStore.ts | 15 +- .../src/persistence/persistence.test.ts | 143 +++++++++++++++++- .../wallet/src/persistence/persistence.ts | 99 ++++++++---- 5 files changed, 263 insertions(+), 55 deletions(-) diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index d22d7842048..a6bc5c38c0f 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -26,6 +26,8 @@ export class Wallet { readonly #unsubscribePersistence: () => void; + #destroyed = false; + constructor({ options, databasePath = ':memory:' }: WalletConstructorArgs) { this.messenger = new Messenger({ namespace: 'Root', @@ -33,19 +35,24 @@ export class Wallet { this.#store = new KeyValueStore(databasePath); - const state = loadState(this.#store); + try { + const state = loadState(this.#store); - this.#instances = initialize({ - state, - messenger: this.messenger, - options, - }); + this.#instances = initialize({ + state, + messenger: this.messenger, + options, + }); - this.#unsubscribePersistence = subscribeToChanges( - this.messenger, - this.#instances, - this.#store, - ); + this.#unsubscribePersistence = subscribeToChanges( + this.messenger, + this.#instances, + this.#store, + ); + } catch (error) { + this.#store.close(); + throw error; + } } get state(): DefaultState { @@ -59,20 +66,27 @@ export class Wallet { } async destroy(): Promise { + if (this.#destroyed) { + return; + } + this.#destroyed = true; + this.#unsubscribePersistence(); - await Promise.all( - Object.values(this.#instances).map((instance) => { - // @ts-expect-error Accessing protected property. - if (typeof instance.destroy === 'function') { + try { + await Promise.allSettled( + Object.values(this.#instances).map((instance) => { // @ts-expect-error Accessing protected property. - return instance.destroy(); - } - /* istanbul ignore next */ - return undefined; - }), - ); - - this.#store.close(); + if (typeof instance.destroy === 'function') { + // @ts-expect-error Accessing protected property. + return instance.destroy(); + } + /* istanbul ignore next */ + return undefined; + }), + ); + } finally { + this.#store.close(); + } } } diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index d64b0e79fc8..a3db3b1b449 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1,2 +1 @@ export { Wallet } from './Wallet'; -export { KeyValueStore } from './persistence'; diff --git a/packages/wallet/src/persistence/KeyValueStore.ts b/packages/wallet/src/persistence/KeyValueStore.ts index 2da7f38ab66..b7b0d654596 100644 --- a/packages/wallet/src/persistence/KeyValueStore.ts +++ b/packages/wallet/src/persistence/KeyValueStore.ts @@ -36,7 +36,14 @@ export class KeyValueStore { get(key: string): Json | undefined { const row = this.#getStmt.get(key); - return row ? JSON.parse(row.value) : undefined; + if (!row) { + return undefined; + } + try { + return JSON.parse(row.value); + } catch { + throw new Error(`Failed to parse stored value for key '${key}'`); + } } set(key: string, value: Json): void { @@ -47,7 +54,11 @@ export class KeyValueStore { const rows = this.#getAllStmt.all(); const result: Record = {}; for (const row of rows) { - result[row.key] = JSON.parse(row.value); + try { + result[row.key] = JSON.parse(row.value); + } catch { + throw new Error(`Failed to parse stored value for key '${row.key}'`); + } } return result; } diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts index c8c7001ca6b..71c01aa2295 100644 --- a/packages/wallet/src/persistence/persistence.test.ts +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -45,6 +45,22 @@ describe('loadState', () => { "Invalid key in store: 'noDot'. Expected format 'ControllerName.propertyName'.", ); }); + + it('throws on a key with an empty controller name', () => { + store.set('.propName', 'value'); + + expect(() => loadState(store)).toThrow( + "Invalid key in store: '.propName'. Both controller name and property name must be non-empty.", + ); + }); + + it('throws on a key with an empty property name', () => { + store.set('ControllerName.', 'value'); + + expect(() => loadState(store)).toThrow( + "Invalid key in store: 'ControllerName.'. Both controller name and property name must be non-empty.", + ); + }); }); describe('subscribeToChanges', () => { @@ -176,6 +192,100 @@ describe('subscribeToChanges', () => { expect(store.get('TestController.prop')).toBe('first'); }); + it('deletes persisted property when it is removed from state', () => { + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['removable', true]]), + }); + + subscribeToChanges(messenger, instances, store); + + // First, persist a value + publishStateChanged(messenger, 'TestController', { + state: { removable: 'exists' }, + patches: [{ op: 'replace', path: ['removable'], value: 'exists' }], + }); + + expect(store.get('TestController.removable')).toBe('exists'); + + // Now remove it — state no longer contains the property + publishStateChanged(messenger, 'TestController', { + state: {}, + patches: [{ op: 'remove', path: ['removable'] }], + }); + + expect(store.get('TestController.removable')).toBeUndefined(); + }); + + it('persists all flagged properties on root state replacement', () => { + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([ + ['propA', true], + ['propB', true], + ['transient', false], + ]), + }); + + subscribeToChanges(messenger, instances, store); + + publishStateChanged(messenger, 'TestController', { + state: { propA: 'newA', propB: 'newB', transient: 'skip' }, + patches: [ + { + op: 'replace', + path: [], + value: { propA: 'newA', propB: 'newB', transient: 'skip' }, + }, + ], + }); + + expect(store.get('TestController.propA')).toBe('newA'); + expect(store.get('TestController.propB')).toBe('newB'); + expect(store.get('TestController.transient')).toBeUndefined(); + }); + + it('logs and continues when store.set throws', () => { + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([ + ['propA', true], + ['propB', true], + ]), + }); + + subscribeToChanges(messenger, instances, store); + + const error = new Error('disk full'); + const originalSet = store.set.bind(store); + let callCount = 0; + jest.spyOn(store, 'set').mockImplementation((key, value) => { + callCount += 1; + if (callCount === 1) { + throw error; + } + originalSet(key, value); + }); + + const consoleSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + publishStateChanged(messenger, 'TestController', { + state: { propA: 'a', propB: 'b' }, + patches: [ + { op: 'replace', path: ['propA'], value: 'a' }, + { op: 'replace', path: ['propB'], value: 'b' }, + ], + }); + + expect(consoleSpy).toHaveBeenCalledWith( + 'Failed to persist state for TestController.propA', + error, + ); + // propB should still be persisted despite propA failing + expect(store.get('TestController.propB')).toBe('b'); + + consoleSpy.mockRestore(); + }); + it('handles multiple controllers independently', () => { const { messenger, instances } = createMockControllers({ ControllerA: createStateMetadata([['data', true]]), @@ -199,6 +309,37 @@ describe('subscribeToChanges', () => { }); }); +describe('subscribeToChanges unsubscribe', () => { + let store: KeyValueStore; + + beforeEach(() => { + store = new KeyValueStore(':memory:'); + }); + + afterEach(() => { + store.close(); + }); + + it('stops persistence so writes to a subsequently closed store do not throw', () => { + const { messenger, instances } = createMockControllers({ + TestController: createStateMetadata([['prop', true]]), + }); + + const unsubscribe = subscribeToChanges(messenger, instances, store); + + unsubscribe(); + store.close(); + + // This should not throw — the handler was unsubscribed before close. + expect(() => + publishStateChanged(messenger, 'TestController', { + state: { prop: 'after-close' }, + patches: [{ op: 'replace', path: ['prop'], value: 'after-close' }], + }), + ).not.toThrow(); + }); +}); + type MockControllerConfig = { metadata: Record< string, @@ -220,7 +361,7 @@ type MockControllers = { * Creates a state metadata object for a mock controller. * * @param properties - An array of [property name, persist value] pairs. - * @returns A state metadata object. + * @returns A mock controller configuration containing the state metadata. */ function createStateMetadata( properties: [string, boolean | ((value: never) => Json)][], diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index 344e9e5abd7..ad3cd9153a0 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -1,13 +1,30 @@ -import type { - BaseControllerInstance, - StateMetadataConstraint, -} from '@metamask/base-controller'; +import type { StateMetadataConstraint } from '@metamask/base-controller'; +import { hasProperty } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import type { Patch } from 'immer'; import type { KeyValueStore } from './KeyValueStore'; import type { DefaultInstances, RootMessenger } from '../initialization'; +/** + * A controller instance that has a `metadata` property describing which + * state properties should be persisted. + */ +type PersistableController = { + metadata: StateMetadataConstraint; +}; + +/** + * Construct a store key from a controller name and property name. + * + * @param controllerName - The controller name. + * @param propertyName - The property name. + * @returns The store key in the format `ControllerName.propertyName`. + */ +function storeKey(controllerName: string, propertyName: string): string { + return `${controllerName}.${propertyName}`; +} + /** * Load persisted state from the key-value store and reconstruct it as * a record keyed by controller name. @@ -18,7 +35,9 @@ import type { DefaultInstances, RootMessenger } from '../initialization'; * @param store - The key-value store to read from. * @returns A record of controller states, suitable for passing to `initialize()`. */ -export function loadState(store: KeyValueStore): Record { +export function loadState( + store: KeyValueStore, +): Record> { const allPairs = store.getAll(); const state: Record> = {}; @@ -32,12 +51,17 @@ export function loadState(store: KeyValueStore): Record { const controllerName = key.slice(0, dotIndex); const propertyName = key.slice(dotIndex + 1); + if (!controllerName || !propertyName) { + throw new Error( + `Invalid key in store: '${key}'. Both controller name and property name must be non-empty.`, + ); + } + if (!state[controllerName]) { state[controllerName] = {}; } state[controllerName][propertyName] = value; } - return state; } @@ -64,7 +88,7 @@ export function subscribeToChanges( const unsubscribers: (() => void)[] = []; for (const [controllerName, instance] of Object.entries(instances)) { - const controller = instance as unknown as BaseControllerInstance; + const controller = instance as unknown as PersistableController; const { metadata } = controller; const persistedProperties = getPersistPropertyNames(metadata); @@ -75,20 +99,27 @@ export function subscribeToChanges( const eventType = `${controllerName}:stateChanged`; const handler = (state: Record, patches: Patch[]): void => { - const changed = getChangedProperties(patches); + const changed = getChangedProperties(patches, persistedProperties); for (const prop of changed) { - if (!persistedProperties.has(prop)) { - continue; - } - - const key = `${controllerName}.${prop}`; - const persistFlag = metadata[prop]?.persist; - - if (typeof persistFlag === 'function') { - store.set(key, persistFlag(state[prop] as never)); - } else { - store.set(key, state[prop]); + const key = storeKey(controllerName, prop); + + try { + if (!hasProperty(state, prop)) { + store.delete(key); + continue; + } + + const persistFlag = metadata[prop]?.persist; + + if (typeof persistFlag === 'function') { + store.set(key, persistFlag(state[prop] as never)); + } else { + store.set(key, state[prop]); + } + } catch (error) { + // TODO: Handle persistence failure to protect the user from data loss. + console.error(`Failed to persist state for ${key}`, error); } } }; @@ -110,8 +141,8 @@ export function subscribeToChanges( } /** - * Get the set of property names that have `persist: true` (or a persist deriver) - * in the given state metadata. + * Get the set of property names whose `persist` metadata is truthy + * (either `true` or a `StateDeriver` function). * * @param metadata - The controller's state metadata. * @returns A set of property names that should be persisted. @@ -120,8 +151,8 @@ function getPersistPropertyNames( metadata: StateMetadataConstraint, ): Set { const names = new Set(); - for (const [key, propertyMetadata] of Object.entries(metadata)) { - if (propertyMetadata.persist) { + for (const key of Object.keys(metadata)) { + if (metadata[key].persist) { names.add(key); } } @@ -129,17 +160,29 @@ function getPersistPropertyNames( } /** - * Extracts the set of top-level property names that changed from an - * array of Immer patches. + * Extracts the set of persist-flagged top-level property names that changed + * from an array of Immer patches. + * + * If any patch has an empty path (indicating a root state replacement), + * all persist-flagged properties are returned. * * @param patches - Immer patches from a state update. + * @param persistedProperties - The set of persist-flagged property names. * @returns A set of top-level property names that were modified. */ -function getChangedProperties(patches: Patch[]): Set { +function getChangedProperties( + patches: Patch[], + persistedProperties: Set, +): Set { const changed = new Set(); for (const patch of patches) { - if (patch.path.length > 0) { - changed.add(String(patch.path[0])); + if (patch.path.length === 0) { + return persistedProperties; + } + + const prop = String(patch.path[0]); + if (persistedProperties.has(prop)) { + changed.add(prop); } } return changed; From c6a88f5172b39b3a12b061433c257ae3ecce1407 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 14:07:32 -0700 Subject: [PATCH 09/18] tests: Cover persistence error paths and lifecycle edge cases Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet/src/Wallet.test.ts | 43 +++++++++++++++++++ .../src/persistence/KeyValueStore.test.ts | 37 ++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 13cc7a899d0..6b799960502 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -9,6 +9,8 @@ import { enableNetConnect } from 'nock'; import { startAnvil } from '../test/anvil'; import type { AnvilInstance } from '../test/anvil'; +import * as persistenceModule from './persistence'; +import { KeyValueStore } from './persistence'; import { createSecretRecoveryPhrase, importSecretRecoveryPhrase, @@ -169,4 +171,45 @@ describe('Wallet', () => { vault: expect.any(String), }); }); + + describe('lifecycle', () => { + const options = { + infuraProjectId: 'fake-infura-project-id', + clientVersion: '1.0.0', + showApprovalRequest: (): undefined => undefined, + clientConfigApiService: new ClientConfigApiService({ + fetch: globalThis.fetch, + config: { + client: ClientType.Extension, + distribution: DistributionType.Main, + environment: EnvironmentType.Production, + }, + }), + getMetaMetricsId: (): string => 'fake-metrics-id', + }; + + it('closes the store if construction fails', () => { + const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); + jest.spyOn(persistenceModule, 'loadState').mockImplementationOnce(() => { + throw new Error('load failed'); + }); + + expect(() => new Wallet({ options })).toThrow('load failed'); + expect(closeSpy).toHaveBeenCalledTimes(1); + + closeSpy.mockRestore(); + }); + + it('handles multiple destroy calls gracefully', async () => { + wallet = new Wallet({ options }); + const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); + + await wallet.destroy(); + await wallet.destroy(); + + expect(closeSpy).toHaveBeenCalledTimes(1); + + closeSpy.mockRestore(); + }); + }); }); diff --git a/packages/wallet/src/persistence/KeyValueStore.test.ts b/packages/wallet/src/persistence/KeyValueStore.test.ts index de3143593ec..ffed4322a60 100644 --- a/packages/wallet/src/persistence/KeyValueStore.test.ts +++ b/packages/wallet/src/persistence/KeyValueStore.test.ts @@ -1,4 +1,8 @@ import type { Json } from '@metamask/utils'; +import Sqlite from 'better-sqlite3'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import { KeyValueStore } from './KeyValueStore'; @@ -77,4 +81,37 @@ describe('KeyValueStore', () => { expect(() => store.delete('missing')).not.toThrow(); }); }); + + describe('corrupt data', () => { + let tempPath: string; + let corruptStore: KeyValueStore; + + beforeEach(() => { + tempPath = path.join(os.tmpdir(), `kv-test-${Date.now()}.db`); + corruptStore = new KeyValueStore(tempPath); + + const rawDb = new Sqlite(tempPath); + rawDb + .prepare('INSERT INTO kv (key, value) VALUES (?, ?)') + .run('bad', 'not json'); + rawDb.close(); + }); + + afterEach(() => { + corruptStore.close(); + fs.unlinkSync(tempPath); + }); + + it('throws when get() encounters a non-JSON value', () => { + expect(() => corruptStore.get('bad')).toThrow( + "Failed to parse stored value for key 'bad'", + ); + }); + + it('throws when getAll() encounters a non-JSON value', () => { + expect(() => corruptStore.getAll()).toThrow( + "Failed to parse stored value for key 'bad'", + ); + }); + }); }); From 62ff2e3ec7d6bcbf2e4cdf17618125f1709bca05 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 14:31:47 -0700 Subject: [PATCH 10/18] chore: oxfmt & eslint --- packages/wallet/src/persistence/KeyValueStore.test.ts | 6 +++--- packages/wallet/src/persistence/persistence.test.ts | 2 +- packages/wallet/src/persistence/persistence.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/wallet/src/persistence/KeyValueStore.test.ts b/packages/wallet/src/persistence/KeyValueStore.test.ts index ffed4322a60..f7c3ad6445d 100644 --- a/packages/wallet/src/persistence/KeyValueStore.test.ts +++ b/packages/wallet/src/persistence/KeyValueStore.test.ts @@ -1,6 +1,6 @@ import type { Json } from '@metamask/utils'; import Sqlite from 'better-sqlite3'; -import fs from 'fs'; +import { unlink } from 'fs/promises'; import os from 'os'; import path from 'path'; @@ -97,9 +97,9 @@ describe('KeyValueStore', () => { rawDb.close(); }); - afterEach(() => { + afterEach(async () => { corruptStore.close(); - fs.unlinkSync(tempPath); + await unlink(tempPath); }); it('throws when get() encounters a non-JSON value', () => { diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts index 71c01aa2295..ae2e78c5ca8 100644 --- a/packages/wallet/src/persistence/persistence.test.ts +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -1,8 +1,8 @@ import type { Json } from '@metamask/utils'; +import type { RootMessenger, DefaultInstances } from '../initialization'; import { KeyValueStore } from './KeyValueStore'; import { loadState, subscribeToChanges } from './persistence'; -import type { RootMessenger, DefaultInstances } from '../initialization'; describe('loadState', () => { let store: KeyValueStore; diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index ad3cd9153a0..5a0ea946d59 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -3,8 +3,8 @@ import { hasProperty } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import type { Patch } from 'immer'; -import type { KeyValueStore } from './KeyValueStore'; import type { DefaultInstances, RootMessenger } from '../initialization'; +import type { KeyValueStore } from './KeyValueStore'; /** * A controller instance that has a `metadata` property describing which From 6df4a996ecc81d78ea77a5e504ab7190c823f3e8 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:11:41 -0700 Subject: [PATCH 11/18] refactor: Make change and persist sets readonly --- packages/wallet/src/persistence/persistence.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index 5a0ea946d59..16f515c31a5 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -149,7 +149,7 @@ export function subscribeToChanges( */ function getPersistPropertyNames( metadata: StateMetadataConstraint, -): Set { +): ReadonlySet { const names = new Set(); for (const key of Object.keys(metadata)) { if (metadata[key].persist) { @@ -172,8 +172,8 @@ function getPersistPropertyNames( */ function getChangedProperties( patches: Patch[], - persistedProperties: Set, -): Set { + persistedProperties: ReadonlySet, +): ReadonlySet { const changed = new Set(); for (const patch of patches) { if (patch.path.length === 0) { From cfad38769fb71229ed8a9566e0433c0c4e2a93d4 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:23:54 -0700 Subject: [PATCH 12/18] refactor: Unsubscribe in try block --- packages/wallet/src/Wallet.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index a6bc5c38c0f..d9e2af6a1c8 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -71,10 +71,8 @@ export class Wallet { } this.#destroyed = true; - this.#unsubscribePersistence(); - try { - await Promise.allSettled( + const destroyed = Promise.allSettled( Object.values(this.#instances).map((instance) => { // @ts-expect-error Accessing protected property. if (typeof instance.destroy === 'function') { @@ -85,6 +83,9 @@ export class Wallet { return undefined; }), ); + + this.#unsubscribePersistence(); + await destroyed; } finally { this.#store.close(); } From b36d336517246c65e4217e2f6cc4b49b8037e7af Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 14:49:28 -0700 Subject: [PATCH 13/18] refactor: Decouple persistence from Wallet constructor Make `@metamask/wallet` platform-agnostic by removing the SQLite-backed `KeyValueStore` from the `Wallet` constructor. `Wallet` now accepts a flat `WalletOptions` (including an optional `state` payload) and owns no persistence state. Consumers arrange persistence themselves. - `Wallet` exposes a `controllerMetadata` getter so persistence can operate on metadata alone, without reaching into controller instances. - A new `Root:walletDestroyed` event is published after controllers tear down during `destroy()`. `subscribeToChanges` listens for it and self-unsubscribes, eliminating the need for callers to hold an unsubscribe handle. - `subscribeToChanges` now takes a `Record` instead of the controller-instance map, and specializes its messenger type to `RootMessenger` so subscribing to `Root:walletDestroyed` type-checks without suppression. - `KeyValueStore`, `loadState`, and `subscribeToChanges` are exposed via a new `@metamask/wallet/persistence` subpath export; they'll move to their own package later. --- packages/wallet/package.json | 10 ++ packages/wallet/src/Wallet.test.ts | 82 ++++----- packages/wallet/src/Wallet.ts | 83 +++++---- packages/wallet/src/index.ts | 1 + .../wallet/src/initialization/defaults.ts | 9 +- packages/wallet/src/initialization/index.ts | 1 + .../src/persistence/persistence.test.ts | 161 +++++++++++------- .../wallet/src/persistence/persistence.ts | 51 +++--- packages/wallet/src/types.ts | 2 + 9 files changed, 226 insertions(+), 174 deletions(-) diff --git a/packages/wallet/package.json b/packages/wallet/package.json index dda917a0261..80fbbdb6cf5 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -32,6 +32,16 @@ "default": "./dist/index.cjs" } }, + "./persistence": { + "import": { + "types": "./dist/persistence/index.d.mts", + "default": "./dist/persistence/index.mjs" + }, + "require": { + "types": "./dist/persistence/index.d.cts", + "default": "./dist/persistence/index.cjs" + } + }, "./package.json": "./package.json" }, "publishConfig": { diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 6b799960502..99a38cacc59 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -9,8 +9,6 @@ import { enableNetConnect } from 'nock'; import { startAnvil } from '../test/anvil'; import type { AnvilInstance } from '../test/anvil'; -import * as persistenceModule from './persistence'; -import { KeyValueStore } from './persistence'; import { createSecretRecoveryPhrase, importSecretRecoveryPhrase, @@ -24,20 +22,18 @@ const TEST_PASSWORD = 'testpass'; async function setupWallet(): Promise { const wallet = new Wallet({ - options: { - infuraProjectId: 'fake-infura-project-id', - clientVersion: '1.0.0', - showApprovalRequest: (): undefined => undefined, - clientConfigApiService: new ClientConfigApiService({ - fetch: globalThis.fetch, - config: { - client: ClientType.Extension, - distribution: DistributionType.Main, - environment: EnvironmentType.Production, - }, - }), - getMetaMetricsId: (): string => 'fake-metrics-id', - }, + infuraProjectId: 'fake-infura-project-id', + clientVersion: '1.0.0', + showApprovalRequest: (): undefined => undefined, + clientConfigApiService: new ClientConfigApiService({ + fetch: globalThis.fetch, + config: { + client: ClientType.Extension, + distribution: DistributionType.Main, + environment: EnvironmentType.Production, + }, + }), + getMetaMetricsId: (): string => 'fake-metrics-id', }); await importSecretRecoveryPhrase(wallet, TEST_PASSWORD, TEST_PHRASE); @@ -136,20 +132,18 @@ describe('Wallet', () => { it('can create secret recovery phrase', async () => { wallet = new Wallet({ - options: { - infuraProjectId: 'fake-infura-project-id', - clientVersion: '1.0.0', - showApprovalRequest: (): undefined => undefined, - clientConfigApiService: new ClientConfigApiService({ - fetch: globalThis.fetch, - config: { - client: ClientType.Extension, - distribution: DistributionType.Main, - environment: EnvironmentType.Production, - }, - }), - getMetaMetricsId: (): string => 'fake-metrics-id', - }, + infuraProjectId: 'fake-infura-project-id', + clientVersion: '1.0.0', + showApprovalRequest: (): undefined => undefined, + clientConfigApiService: new ClientConfigApiService({ + fetch: globalThis.fetch, + config: { + client: ClientType.Extension, + distribution: DistributionType.Main, + environment: EnvironmentType.Production, + }, + }), + getMetaMetricsId: (): string => 'fake-metrics-id', }); await createSecretRecoveryPhrase(wallet, TEST_PASSWORD); @@ -188,28 +182,26 @@ describe('Wallet', () => { getMetaMetricsId: (): string => 'fake-metrics-id', }; - it('closes the store if construction fails', () => { - const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); - jest.spyOn(persistenceModule, 'loadState').mockImplementationOnce(() => { - throw new Error('load failed'); - }); + it('exposes controllerMetadata for each initialized controller', () => { + wallet = new Wallet(options); - expect(() => new Wallet({ options })).toThrow('load failed'); - expect(closeSpy).toHaveBeenCalledTimes(1); - - closeSpy.mockRestore(); + const names = Object.keys(wallet.controllerMetadata); + expect(names).toStrictEqual(Object.keys(wallet.state)); + for (const name of names) { + expect(wallet.controllerMetadata[name]).toBeDefined(); + } }); - it('handles multiple destroy calls gracefully', async () => { - wallet = new Wallet({ options }); - const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); + it('publishes Root:walletDestroyed exactly once on destroy', async () => { + wallet = new Wallet(options); + + const listener = jest.fn(); + wallet.messenger.subscribe('Root:walletDestroyed', listener); await wallet.destroy(); await wallet.destroy(); - expect(closeSpy).toHaveBeenCalledTimes(1); - - closeSpy.mockRestore(); + expect(listener).toHaveBeenCalledTimes(1); }); }); }); diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index d9e2af6a1c8..a4eca4fe414 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -1,3 +1,4 @@ +import type { StateMetadataConstraint } from '@metamask/base-controller'; import { Messenger } from '@metamask/messenger'; import type { @@ -8,12 +9,10 @@ import type { RootMessenger, } from './initialization'; import { initialize } from './initialization'; -import { KeyValueStore, loadState, subscribeToChanges } from './persistence'; import type { WalletOptions } from './types'; -export type WalletConstructorArgs = { - options: WalletOptions; - databasePath?: string; +type PersistableController = { + metadata: StateMetadataConstraint; }; export class Wallet { @@ -22,37 +21,34 @@ export class Wallet { readonly #instances: DefaultInstances; - readonly #store: KeyValueStore; - - readonly #unsubscribePersistence: () => void; + readonly #controllerMetadata: Readonly< + Record> + >; #destroyed = false; - constructor({ options, databasePath = ':memory:' }: WalletConstructorArgs) { + constructor({ state, ...options }: WalletOptions) { this.messenger = new Messenger({ namespace: 'Root', }); - this.#store = new KeyValueStore(databasePath); - - try { - const state = loadState(this.#store); + this.messenger.registerInitialEventPayload({ + eventType: 'Root:walletDestroyed', + getPayload: () => [], + }); - this.#instances = initialize({ - state, - messenger: this.messenger, - options, - }); + this.#instances = initialize({ + state: state ?? {}, + messenger: this.messenger, + options, + }); - this.#unsubscribePersistence = subscribeToChanges( - this.messenger, - this.#instances, - this.#store, - ); - } catch (error) { - this.#store.close(); - throw error; - } + this.#controllerMetadata = Object.fromEntries( + Object.entries(this.#instances).map(([name, instance]) => [ + name, + (instance as unknown as PersistableController).metadata, + ]), + ); } get state(): DefaultState { @@ -65,29 +61,30 @@ export class Wallet { ) as DefaultState; } + get controllerMetadata(): Readonly< + Record> + > { + return this.#controllerMetadata; + } + async destroy(): Promise { if (this.#destroyed) { return; } this.#destroyed = true; - try { - const destroyed = Promise.allSettled( - Object.values(this.#instances).map((instance) => { + await Promise.allSettled( + Object.values(this.#instances).map((instance) => { + // @ts-expect-error Accessing protected property. + if (typeof instance.destroy === 'function') { // @ts-expect-error Accessing protected property. - if (typeof instance.destroy === 'function') { - // @ts-expect-error Accessing protected property. - return instance.destroy(); - } - /* istanbul ignore next */ - return undefined; - }), - ); - - this.#unsubscribePersistence(); - await destroyed; - } finally { - this.#store.close(); - } + return instance.destroy(); + } + /* istanbul ignore next */ + return undefined; + }), + ); + + this.messenger.publish('Root:walletDestroyed'); } } diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index a3db3b1b449..2d62e4172a1 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1 +1,2 @@ export { Wallet } from './Wallet'; +export type { WalletOptions } from './types'; diff --git a/packages/wallet/src/initialization/defaults.ts b/packages/wallet/src/initialization/defaults.ts index 85b02c841f4..ce0c51dabfc 100644 --- a/packages/wallet/src/initialization/defaults.ts +++ b/packages/wallet/src/initialization/defaults.ts @@ -38,7 +38,14 @@ export type DefaultInstances = { export type DefaultActions = MessengerActions; -export type DefaultEvents = MessengerEvents; +export type WalletDestroyedEvent = { + type: 'Root:walletDestroyed'; + payload: []; +}; + +export type DefaultEvents = + | MessengerEvents + | WalletDestroyedEvent; export type RootMessenger< AllowedActions extends ActionConstraint = ActionConstraint, diff --git a/packages/wallet/src/initialization/index.ts b/packages/wallet/src/initialization/index.ts index 5d17e1a18fa..757f51fc01d 100644 --- a/packages/wallet/src/initialization/index.ts +++ b/packages/wallet/src/initialization/index.ts @@ -4,5 +4,6 @@ export type { DefaultInstances, DefaultState, RootMessenger, + WalletDestroyedEvent, } from './defaults'; export { initialize } from './initialization'; diff --git a/packages/wallet/src/persistence/persistence.test.ts b/packages/wallet/src/persistence/persistence.test.ts index ae2e78c5ca8..e02f58bc019 100644 --- a/packages/wallet/src/persistence/persistence.test.ts +++ b/packages/wallet/src/persistence/persistence.test.ts @@ -1,9 +1,16 @@ +import type { StateMetadataConstraint } from '@metamask/base-controller'; import type { Json } from '@metamask/utils'; -import type { RootMessenger, DefaultInstances } from '../initialization'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../initialization'; import { KeyValueStore } from './KeyValueStore'; import { loadState, subscribeToChanges } from './persistence'; +type TestMessenger = RootMessenger; + describe('loadState', () => { let store: KeyValueStore; @@ -75,14 +82,14 @@ describe('subscribeToChanges', () => { }); it('writes persist-flagged properties on state change', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([ ['persisted', true], ['transient', false], ]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'TestController', { state: { persisted: 'savedValue', transient: 'notSaved' }, @@ -97,14 +104,14 @@ describe('subscribeToChanges', () => { }); it('only writes properties that are in the patches', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([ ['propA', true], ['propB', true], ]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'TestController', { state: { propA: 'changedA', propB: 'unchangedB' }, @@ -119,11 +126,11 @@ describe('subscribeToChanges', () => { const deriver = (value: never): Json => (value as unknown as string).toUpperCase(); - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['derived', deriver]]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'TestController', { state: { derived: 'hello' }, @@ -134,11 +141,11 @@ describe('subscribeToChanges', () => { }); it('handles nested property changes by extracting the top-level key', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['nested', true]]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'TestController', { state: { nested: { inner: { deep: 'value' } } }, @@ -153,11 +160,15 @@ describe('subscribeToChanges', () => { }); it('skips controllers with no persisted properties', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['transientOnly', false]]), }); - const unsubscribe = subscribeToChanges(messenger, instances, store); + const unsubscribe = subscribeToChanges( + messenger, + controllerMetadata, + store, + ); publishStateChanged(messenger, 'TestController', { state: { transientOnly: 'value' }, @@ -169,11 +180,15 @@ describe('subscribeToChanges', () => { }); it('returns an unsubscribe function that stops persistence', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['prop', true]]), }); - const unsubscribe = subscribeToChanges(messenger, instances, store); + const unsubscribe = subscribeToChanges( + messenger, + controllerMetadata, + store, + ); publishStateChanged(messenger, 'TestController', { state: { prop: 'first' }, @@ -193,11 +208,11 @@ describe('subscribeToChanges', () => { }); it('deletes persisted property when it is removed from state', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['removable', true]]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); // First, persist a value publishStateChanged(messenger, 'TestController', { @@ -217,7 +232,7 @@ describe('subscribeToChanges', () => { }); it('persists all flagged properties on root state replacement', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([ ['propA', true], ['propB', true], @@ -225,7 +240,7 @@ describe('subscribeToChanges', () => { ]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'TestController', { state: { propA: 'newA', propB: 'newB', transient: 'skip' }, @@ -244,14 +259,14 @@ describe('subscribeToChanges', () => { }); it('logs and continues when store.set throws', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([ ['propA', true], ['propB', true], ]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); const error = new Error('disk full'); const originalSet = store.set.bind(store); @@ -287,12 +302,12 @@ describe('subscribeToChanges', () => { }); it('handles multiple controllers independently', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ ControllerA: createStateMetadata([['data', true]]), ControllerB: createStateMetadata([['data', true]]), }); - subscribeToChanges(messenger, instances, store); + subscribeToChanges(messenger, controllerMetadata, store); publishStateChanged(messenger, 'ControllerA', { state: { data: 'fromA' }, @@ -320,12 +335,40 @@ describe('subscribeToChanges unsubscribe', () => { store.close(); }); + it('self-unsubscribes when Root:walletDestroyed is published', () => { + const { messenger, controllerMetadata } = createMockControllers({ + TestController: createStateMetadata([['prop', true]]), + }); + + subscribeToChanges(messenger, controllerMetadata, store); + + publishStateChanged(messenger, 'TestController', { + state: { prop: 'before' }, + patches: [{ op: 'replace', path: ['prop'], value: 'before' }], + }); + + expect(store.get('TestController.prop')).toBe('before'); + + messenger.publish('Root:walletDestroyed'); + + publishStateChanged(messenger, 'TestController', { + state: { prop: 'after' }, + patches: [{ op: 'replace', path: ['prop'], value: 'after' }], + }); + + expect(store.get('TestController.prop')).toBe('before'); + }); + it('stops persistence so writes to a subsequently closed store do not throw', () => { - const { messenger, instances } = createMockControllers({ + const { messenger, controllerMetadata } = createMockControllers({ TestController: createStateMetadata([['prop', true]]), }); - const unsubscribe = subscribeToChanges(messenger, instances, store); + const unsubscribe = subscribeToChanges( + messenger, + controllerMetadata, + store, + ); unsubscribe(); store.close(); @@ -340,56 +383,52 @@ describe('subscribeToChanges unsubscribe', () => { }); }); -type MockControllerConfig = { - metadata: Record< - string, - { - persist: boolean | ((value: never) => Json); - includeInDebugSnapshot: boolean; - includeInStateLogs: boolean; - usedInUi: boolean; - } - >; -}; +type MockMetadata = Record< + string, + { + persist: boolean | ((value: never) => Json); + includeInDebugSnapshot: boolean; + includeInStateLogs: boolean; + usedInUi: boolean; + } +>; type MockControllers = { - messenger: RootMessenger; - instances: DefaultInstances; + messenger: TestMessenger; + controllerMetadata: Record; }; /** * Creates a state metadata object for a mock controller. * * @param properties - An array of [property name, persist value] pairs. - * @returns A mock controller configuration containing the state metadata. + * @returns A mock metadata object. */ function createStateMetadata( properties: [string, boolean | ((value: never) => Json)][], -): MockControllerConfig { - return { - metadata: Object.fromEntries( - properties.map(([name, persist]) => [ - name, - { - persist, - includeInDebugSnapshot: false, - includeInStateLogs: false, - usedInUi: false, - }, - ]), - ), - }; +): MockMetadata { + return Object.fromEntries( + properties.map(([name, persist]) => [ + name, + { + persist, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: false, + }, + ]), + ); } /** - * Creates a mock messenger and instances map for testing persistence wiring. - * The messenger supports subscribe/unsubscribe/publish for stateChanged events. + * Creates a mock messenger and controllerMetadata map for testing persistence + * wiring. The messenger supports subscribe/unsubscribe/publish. * - * @param controllers - Map of controller names to their mock configurations. - * @returns A mock messenger and instances map. + * @param controllers - Map of controller names to their metadata. + * @returns A mock messenger and a controllerMetadata map. */ function createMockControllers( - controllers: Record, + controllers: Record, ): MockControllers { const handlers = new Map void>>(); @@ -411,14 +450,14 @@ function createMockControllers( } } }, - } as unknown as RootMessenger; + } as unknown as TestMessenger; - const instances: Record = {}; - for (const [name, config] of Object.entries(controllers)) { - instances[name] = { metadata: config.metadata }; + const controllerMetadata: Record = {}; + for (const [name, metadata] of Object.entries(controllers)) { + controllerMetadata[name] = metadata; } - return { messenger, instances: instances as unknown as DefaultInstances }; + return { messenger, controllerMetadata }; } /** diff --git a/packages/wallet/src/persistence/persistence.ts b/packages/wallet/src/persistence/persistence.ts index 16f515c31a5..7d6c7002b65 100644 --- a/packages/wallet/src/persistence/persistence.ts +++ b/packages/wallet/src/persistence/persistence.ts @@ -3,17 +3,13 @@ import { hasProperty } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import type { Patch } from 'immer'; -import type { DefaultInstances, RootMessenger } from '../initialization'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../initialization'; import type { KeyValueStore } from './KeyValueStore'; -/** - * A controller instance that has a `metadata` property describing which - * state properties should be persisted. - */ -type PersistableController = { - metadata: StateMetadataConstraint; -}; - /** * Construct a store key from a controller name and property name. * @@ -69,28 +65,27 @@ export function loadState( * Subscribe to all controller `stateChanged` events and persist changes * to the key-value store. * - * For each controller instance, this function reads the controller's metadata - * to determine which state properties are persist-flagged. When a `stateChanged` - * event fires, it uses the Immer patches to identify which top-level properties - * changed, filters to only persist-flagged properties, and writes them to the - * store. + * For each controller's metadata, this function determines which state + * properties are persist-flagged. When a `stateChanged` event fires, it uses + * the Immer patches to identify which top-level properties changed, filters + * to only persist-flagged properties, and writes them to the store. + * + * Also subscribes to `Root:walletDestroyed` — when the Wallet publishes that + * event, persistence tears itself down. * * @param messenger - The root messenger to subscribe on. - * @param instances - The controller instances returned by `initialize()`. + * @param controllerMetadata - A map from controller name to its state metadata. * @param store - The key-value store to write to. * @returns A function that unsubscribes all persistence handlers. */ export function subscribeToChanges( - messenger: RootMessenger, - instances: DefaultInstances, + messenger: RootMessenger, + controllerMetadata: Record, store: KeyValueStore, ): () => void { const unsubscribers: (() => void)[] = []; - for (const [controllerName, instance] of Object.entries(instances)) { - const controller = instance as unknown as PersistableController; - const { metadata } = controller; - + for (const [controllerName, metadata] of Object.entries(controllerMetadata)) { const persistedProperties = getPersistPropertyNames(metadata); if (persistedProperties.size === 0) { continue; @@ -133,11 +128,19 @@ export function subscribeToChanges( }); } - return () => { - for (const unsubscribe of unsubscribers) { - unsubscribe(); + const unsubscribeAll = (): void => { + while (unsubscribers.length > 0) { + unsubscribers.pop()?.(); } }; + + const destroyedHandler = (): void => unsubscribeAll(); + messenger.subscribe('Root:walletDestroyed', destroyedHandler); + unsubscribers.push(() => { + messenger.unsubscribe('Root:walletDestroyed', destroyedHandler); + }); + + return unsubscribeAll; } /** diff --git a/packages/wallet/src/types.ts b/packages/wallet/src/types.ts index e808637e474..865d6241e47 100644 --- a/packages/wallet/src/types.ts +++ b/packages/wallet/src/types.ts @@ -1,6 +1,8 @@ import type { ClientConfigApiService } from '@metamask/remote-feature-flag-controller'; +import type { Json } from '@metamask/utils'; export type WalletOptions = { + state?: Record>; infuraProjectId: string; clientVersion: string; showApprovalRequest: () => void; From 366b265f11fdcfa55bf1a3565a5a03ed3198e9ed Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 14:52:40 -0700 Subject: [PATCH 14/18] chore: Re-export persistence-facing types from wallet entry So that @metamask/wallet/persistence can be moved to its own package without needing to deep-import from the wallet package. --- packages/wallet/src/index.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index 2d62e4172a1..5fa0502ec2b 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1,2 +1,8 @@ export { Wallet } from './Wallet'; export type { WalletOptions } from './types'; +export type { + DefaultActions, + DefaultEvents, + RootMessenger, + WalletDestroyedEvent, +} from './initialization'; From f4e8dfa4c2553000a44fb462f10b96f48059d666 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 14:58:54 -0700 Subject: [PATCH 15/18] fix: Prevent synchronous errors from interrupting destroy procedure --- packages/wallet/src/Wallet.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/wallet/src/Wallet.ts b/packages/wallet/src/Wallet.ts index a4eca4fe414..c50f7096d48 100644 --- a/packages/wallet/src/Wallet.ts +++ b/packages/wallet/src/Wallet.ts @@ -78,10 +78,10 @@ export class Wallet { // @ts-expect-error Accessing protected property. if (typeof instance.destroy === 'function') { // @ts-expect-error Accessing protected property. - return instance.destroy(); + return (async (): Promise => await instance.destroy())(); } /* istanbul ignore next */ - return undefined; + return Promise.resolve(); }), ); From 86a69b0b78c7aa657c39010c0b0ed3ea6e3a0180 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 15:07:50 -0700 Subject: [PATCH 16/18] test: Add tests for destruction robustuness --- packages/wallet/src/Wallet.test.ts | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 99a38cacc59..dc3a67ea23c 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -5,6 +5,7 @@ import { DistributionType, EnvironmentType, } from '@metamask/remote-feature-flag-controller'; +import { TransactionController } from '@metamask/transaction-controller'; import { enableNetConnect } from 'nock'; import { startAnvil } from '../test/anvil'; @@ -203,5 +204,37 @@ describe('Wallet', () => { expect(listener).toHaveBeenCalledTimes(1); }); + + it('publishes Root:walletDestroyed even if a controller destroy throws synchronously', async () => { + wallet = new Wallet(options); + + jest + .spyOn(TransactionController.prototype, 'destroy') + .mockImplementation(() => { + throw new Error('sync destroy error'); + }); + + const listener = jest.fn(); + wallet.messenger.subscribe('Root:walletDestroyed', listener); + + await wallet.destroy(); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('publishes Root:walletDestroyed even if a controller destroy rejects', async () => { + wallet = new Wallet(options); + + jest + .spyOn(TransactionController.prototype, 'destroy') + .mockRejectedValue(new Error('async destroy error')); + + const listener = jest.fn(); + wallet.messenger.subscribe('Root:walletDestroyed', listener); + + await wallet.destroy(); + + expect(listener).toHaveBeenCalledTimes(1); + }); }); }); From ce80922b32cb799c9f2229d8add7ded70b22b865 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 15:11:57 -0700 Subject: [PATCH 17/18] fix: Rebuild better-sqlite3 if bindings are missing at test time Renames install-anvil.sh to install-binaries.sh and adds a better-sqlite3 native addon rebuild step for CI environments where the prebuilt binding is absent or incompatible with the Node version. Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet/package.json | 2 +- packages/wallet/scripts/install-anvil.sh | 17 -------------- packages/wallet/scripts/install-binaries.sh | 26 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 18 deletions(-) delete mode 100755 packages/wallet/scripts/install-anvil.sh create mode 100755 packages/wallet/scripts/install-binaries.sh diff --git a/packages/wallet/package.json b/packages/wallet/package.json index 80fbbdb6cf5..d2668b01442 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -57,7 +57,7 @@ "messenger-action-types:check": "tsx ../../packages/messenger-cli/src/cli.ts --check", "messenger-action-types:generate": "tsx ../../packages/messenger-cli/src/cli.ts --generate", "since-latest-release": "../../scripts/since-latest-release.sh", - "test:prepare": "./scripts/install-anvil.sh", + "test:prepare": "./scripts/install-binaries.sh", "test": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", "test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose", diff --git a/packages/wallet/scripts/install-anvil.sh b/packages/wallet/scripts/install-anvil.sh deleted file mode 100755 index 80b2a68ed35..00000000000 --- a/packages/wallet/scripts/install-anvil.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/usr/bin/env bash - -set -e -set -o pipefail - -# mm-foundryup installs anvil to `/node_modules/.bin/anvil`. Pin cwd to the -# package root so the install location is predictable regardless of how this -# script is invoked. -cd "$(cd "$(dirname "$0")/.." && pwd)" - -# Run foundryup's TypeScript entry point directly via tsx. This avoids having -# to build @metamask/foundryup first, which matters in CI where workspace deps -# aren't built before tests run. -if ! output=$(yarn tsx ../foundryup/src/cli.ts --binaries anvil 2>&1); then - echo "$output" >&2 - exit 1 -fi diff --git a/packages/wallet/scripts/install-binaries.sh b/packages/wallet/scripts/install-binaries.sh new file mode 100755 index 00000000000..fb96bb9a460 --- /dev/null +++ b/packages/wallet/scripts/install-binaries.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +set -e +set -o pipefail + +# Pin cwd to the package root so all paths are predictable regardless of how +# this script is invoked. Also derive the monorepo root (two levels up). +PACKAGE_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +MONOREPO_ROOT="$(cd "${PACKAGE_ROOT}/../.." && pwd)" +cd "${PACKAGE_ROOT}" + +# Run foundryup's TypeScript entry point directly via tsx. This avoids having +# to build @metamask/foundryup first, which matters in CI where workspace deps +# aren't built before tests run. +if ! output=$(yarn tsx ../foundryup/src/cli.ts --binaries anvil 2>&1); then + echo "$output" >&2 + exit 1 +fi + +# Rebuild better-sqlite3's native addon if it can't be loaded. This is +# necessary when switching Node versions or branches where the prebuilt binary +# is missing or incompatible. +if ! node -e "require('better-sqlite3')" 2>/dev/null; then + echo "Rebuilding better-sqlite3 native addon..." + (cd "${MONOREPO_ROOT}/node_modules/better-sqlite3" && "${MONOREPO_ROOT}/node_modules/.bin/prebuild-install") +fi From bd9c50431ca22d5855000882f93374ea2aa415dc Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Apr 2026 15:38:31 -0700 Subject: [PATCH 18/18] fix: Only rebuild better-sqlite3 addon when missing Check for the compiled addon at `node_modules/better-sqlite3/build/Release/better_sqlite3.node` and skip `prebuild-install` when it's already present. Avoids unnecessary network calls on every `test:prepare` run. Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet/scripts/install-binaries.sh | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/wallet/scripts/install-binaries.sh b/packages/wallet/scripts/install-binaries.sh index fb96bb9a460..cf087190a59 100755 --- a/packages/wallet/scripts/install-binaries.sh +++ b/packages/wallet/scripts/install-binaries.sh @@ -17,10 +17,15 @@ if ! output=$(yarn tsx ../foundryup/src/cli.ts --binaries anvil 2>&1); then exit 1 fi -# Rebuild better-sqlite3's native addon if it can't be loaded. This is -# necessary when switching Node versions or branches where the prebuilt binary -# is missing or incompatible. -if ! node -e "require('better-sqlite3')" 2>/dev/null; then - echo "Rebuilding better-sqlite3 native addon..." - (cd "${MONOREPO_ROOT}/node_modules/better-sqlite3" && "${MONOREPO_ROOT}/node_modules/.bin/prebuild-install") +# Install the better-sqlite3 native addon if missing. Yarn has +# `enableScripts: false` globally, so install scripts never run during +# `yarn install` and the addon may be absent from the filesystem. Invoke the +# prebuild-install binary directly to fetch a matching prebuild for the active +# Node version and platform. +BETTER_SQLITE3_DIR="${MONOREPO_ROOT}/node_modules/better-sqlite3" +if [ ! -f "${BETTER_SQLITE3_DIR}/build/Release/better_sqlite3.node" ]; then + ( + cd "${BETTER_SQLITE3_DIR}" + "${MONOREPO_ROOT}/node_modules/.bin/prebuild-install" + ) fi