Skip to content

Commit 408e040

Browse files
sirtimidclaude
andauthored
feat(wallet-cli): add SQLite-backed controller-state persistence (MetaMask#9067)
## Explanation `@metamask/wallet-cli` runs a background daemon that hosts a `@metamask/wallet` instance. For controller state to survive daemon restarts, it needs durable local storage. This PR adds that persistence layer. It is purely additive and not yet wired into a daemon process — the daemon entry point that consumes it lands in a follow-up. - **`KeyValueStore`** — a synchronous `better-sqlite3`-backed key-value store (a single `kv` table with a TEXT key and a JSON-serialized TEXT value). Corrupt rows fail loud (a descriptive, key-scoped error) rather than returning garbage. - **`loadState(store)`** — rehydrates persisted state into the `{ [controllerName]: { [property]: value } }` shape accepted by the `Wallet` constructor's `state` option. - **`subscribeToChanges(messenger, controllerMetadata, store, log?)`** — subscribes to every `<Controller>:stateChanged` event and writes persist-flagged top-level properties through to the store (applying `StateDeriver` persist functions, and deleting a key when its property is removed from state). It returns an unsubscribe handle; teardown is owned by the caller (a `dispose()` handle in a later PR), so there is no package-level teardown event. ### `better-sqlite3` native addon `better-sqlite3` ships a native C addon. The monorepo runs Yarn with `enableScripts: false`, so the addon is not built during `yarn install`. To handle this: - a `test:prepare` step (`scripts/install-binaries.sh`) fetches a matching prebuild via `prebuild-install` before tests run; - `engines.node` is set to `>=20` (better-sqlite3 v12 only ships prebuilt binaries for Node 20+); - `@metamask/wallet-cli` is excluded from the Node 18.x CI test matrix; - `yarn.config.cjs` exempts the package from the standard `test`-script and `engines.node` constraints accordingly. ## References - Closes MetaMask#8682 - Builds on MetaMask#9065 (the `@metamask/wallet-cli` package scaffold). This PR targets that branch and should merge after it. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > New durable state path can diverge from in-memory state on disk write failures (logged only today); native addon and Node 20+ requirement affect install and CI. > > **Overview** > Adds **SQLite-backed persistence** in `@metamask/wallet-cli` so wallet controller state can survive restarts (not yet wired into the daemon). > > A synchronous **`KeyValueStore`** (`better-sqlite3`, JSON values in a `kv` table) is the storage backend. **`loadState`** rebuilds `{ controllerName: { property: value } }` for `Wallet` construction, honoring only properties still marked `persist` in controller metadata so stale rows are not resurrected. **`subscribeToChanges`** listens on `<Controller>:stateChanged`, writes persist-flagged top-level fields from Immer patches (including `StateDeriver` transforms and deletes when properties are removed), logs write failures without stopping the process, and returns an unsubscribe handle. > > **`better-sqlite3`** is added with **`engines.node` `>=20`**, a **`test:prepare`** / **`install-binaries.sh`** prebuild step (monorepo `enableScripts: false`), CI matrix exclusion for Node 18 on this package, and **`yarn.config.cjs`** exceptions for the custom test script and engines field. Dependencies on `@metamask/wallet`, `@metamask/base-controller`, and `immer` are added with matching tsconfig project references. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b9854de. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8b8969c commit 408e040

13 files changed

Lines changed: 1372 additions & 17 deletions

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,8 @@ linkStyle default opacity:0.5
608608
wallet --> messenger;
609609
wallet --> remote_feature_flag_controller;
610610
wallet --> storage_service;
611+
wallet_cli --> base_controller;
612+
wallet_cli --> wallet;
611613
```
612614

613615
<!-- end dependency graph -->

packages/wallet-cli/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- Add SQLite-backed persistence for wallet controller state ([#9067](https://github.com/MetaMask/core/pull/9067))
13+
- A `KeyValueStore` backed by `better-sqlite3` for synchronous reads and writes.
14+
- `loadState` to rehydrate persist-flagged controller state from the store and `subscribeToChanges` to write persist-flagged controller state through to disk on every `stateChanged` event.
1215
- Initial package scaffold for `@metamask/wallet-cli`, an [oclif](https://oclif.io)-based `mm` CLI for `@metamask/wallet` ([#9065](https://github.com/MetaMask/core/pull/9065)).
1316

1417
[Unreleased]: https://github.com/MetaMask/core/

packages/wallet-cli/README.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,24 @@ or
1010

1111
`npm install @metamask/wallet-cli`
1212

13+
## Troubleshooting
14+
15+
### Rebuilding `better-sqlite3`
16+
17+
This package depends on `better-sqlite3`, which ships a native C addon. The monorepo runs Yarn with `enableScripts: false`, so the addon is **not** fetched automatically during `yarn install`. Instead, the package's `test:prepare` script (`scripts/install-binaries.sh`) downloads the matching prebuild on demand the first time you run tests, falling back to compiling the addon from source (via `node-gyp`) when no prebuild is published for your Node ABI/platform.
18+
19+
If you switch Node versions or branches and the binding is missing, re-run:
20+
21+
```sh
22+
yarn workspace @metamask/wallet-cli run test:prepare
23+
```
24+
25+
Or invoke `prebuild-install` directly from the monorepo root (where `better-sqlite3` is hoisted):
26+
27+
```sh
28+
cd node_modules/better-sqlite3 && node ../.bin/prebuild-install
29+
```
30+
1331
## Contributing
1432

1533
This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme).

packages/wallet-cli/package.json

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,24 @@
3636
"changelog:update": "../../scripts/update-changelog.sh @metamask/wallet-cli",
3737
"changelog:validate": "../../scripts/validate-changelog.sh @metamask/wallet-cli",
3838
"since-latest-release": "../../scripts/since-latest-release.sh",
39-
"test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter",
39+
"test:prepare": "./scripts/install-binaries.sh",
40+
"test": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter",
4041
"test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache",
4142
"test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose",
4243
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch"
4344
},
4445
"dependencies": {
45-
"@oclif/core": "^4.10.5"
46+
"@metamask/base-controller": "^9.1.0",
47+
"@metamask/utils": "^11.11.0",
48+
"@metamask/wallet": "^3.0.0",
49+
"@oclif/core": "^4.10.5",
50+
"better-sqlite3": "^12.9.0",
51+
"immer": "^9.0.6"
4652
},
4753
"devDependencies": {
4854
"@metamask/auto-changelog": "^6.1.0",
4955
"@ts-bridge/cli": "^0.6.4",
56+
"@types/better-sqlite3": "^7.6.13",
5057
"@types/jest": "^29.5.14",
5158
"deepmerge": "^4.2.2",
5259
"jest": "^29.7.0",
@@ -60,6 +67,6 @@
6067
"topicSeparator": " "
6168
},
6269
"engines": {
63-
"node": "^18.18 || >=20"
70+
"node": ">=20"
6471
}
6572
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#!/usr/bin/env bash
2+
3+
set -e
4+
set -o pipefail
5+
6+
# Pin cwd to the package root so all paths are predictable regardless of how
7+
# this script is invoked. Also derive the monorepo root (two levels up).
8+
PACKAGE_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
9+
MONOREPO_ROOT="$(cd "${PACKAGE_ROOT}/../.." && pwd)"
10+
cd "${PACKAGE_ROOT}"
11+
12+
# Install the better-sqlite3 native addon if missing. Yarn has
13+
# `enableScripts: false` globally, so install scripts never run during
14+
# `yarn install` and the addon may be absent from the filesystem. Reproduce
15+
# better-sqlite3's own install step (`prebuild-install || node-gyp rebuild
16+
# --release`): fetch a matching prebuild for the active Node version and
17+
# platform, and fall back to compiling from source when no prebuild is
18+
# published for that ABI/libc combination (e.g. some Linux CI runners).
19+
BETTER_SQLITE3_DIR="${MONOREPO_ROOT}/node_modules/better-sqlite3"
20+
if [ ! -f "${BETTER_SQLITE3_DIR}/build/Release/better_sqlite3.node" ]; then
21+
(
22+
cd "${BETTER_SQLITE3_DIR}"
23+
if ! "${MONOREPO_ROOT}/node_modules/.bin/prebuild-install"; then
24+
echo "wallet-cli: prebuild-install failed (see its output above); compiling better-sqlite3 from source. This needs a C/C++ toolchain and Python." >&2
25+
"${MONOREPO_ROOT}/node_modules/.bin/node-gyp" rebuild --release
26+
fi
27+
)
28+
fi
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import type { Json } from '@metamask/utils';
2+
import Sqlite from 'better-sqlite3';
3+
import { unlink } from 'fs/promises';
4+
import os from 'os';
5+
import path from 'path';
6+
7+
import { KeyValueStore } from './KeyValueStore';
8+
9+
describe('KeyValueStore', () => {
10+
let store: KeyValueStore;
11+
12+
beforeEach(() => {
13+
store = new KeyValueStore(':memory:');
14+
});
15+
16+
afterEach(() => {
17+
store.close();
18+
});
19+
20+
describe('set and get', () => {
21+
it('stores and retrieves a string value', () => {
22+
store.set('key1', 'hello');
23+
expect(store.get('key1')).toBe('hello');
24+
});
25+
26+
it('stores and retrieves a number value', () => {
27+
store.set('key1', 42);
28+
expect(store.get('key1')).toBe(42);
29+
});
30+
31+
it('stores and retrieves a boolean value', () => {
32+
store.set('key1', true);
33+
expect(store.get('key1')).toBe(true);
34+
});
35+
36+
it('stores and retrieves null', () => {
37+
store.set('key1', null);
38+
expect(store.get('key1')).toBeNull();
39+
});
40+
41+
it('stores and retrieves a complex object', () => {
42+
const makeValue = (): Json => ({
43+
nested: { array: [1, 'two', null, { deep: true }] },
44+
});
45+
store.set('key1', makeValue());
46+
expect(store.get('key1')).toStrictEqual(makeValue());
47+
});
48+
49+
it('returns undefined for a nonexistent key', () => {
50+
expect(store.get('missing')).toBeUndefined();
51+
});
52+
53+
it('overwrites an existing key', () => {
54+
store.set('key1', 'first');
55+
store.set('key1', 'second');
56+
expect(store.get('key1')).toBe('second');
57+
});
58+
});
59+
60+
describe('getAll', () => {
61+
it('returns an empty object when the store is empty', () => {
62+
expect(store.getAll()).toStrictEqual({});
63+
});
64+
65+
it('returns all stored key-value pairs', () => {
66+
store.set('a', 1);
67+
store.set('b', 'two');
68+
store.set('c', [3]);
69+
expect(store.getAll()).toStrictEqual({ a: 1, b: 'two', c: [3] });
70+
});
71+
});
72+
73+
describe('delete', () => {
74+
it('removes an existing key', () => {
75+
store.set('key1', 'value');
76+
store.delete('key1');
77+
expect(store.get('key1')).toBeUndefined();
78+
});
79+
80+
it('does nothing when deleting a nonexistent key', () => {
81+
expect(() => store.delete('missing')).not.toThrow();
82+
});
83+
});
84+
85+
describe('corrupt data', () => {
86+
let tempPath: string;
87+
let corruptStore: KeyValueStore;
88+
89+
beforeEach(() => {
90+
tempPath = path.join(os.tmpdir(), `kv-test-${Date.now()}.db`);
91+
corruptStore = new KeyValueStore(tempPath);
92+
93+
const rawDb = new Sqlite(tempPath);
94+
rawDb
95+
.prepare('INSERT INTO kv (key, value) VALUES (?, ?)')
96+
.run('bad', 'not json');
97+
rawDb.close();
98+
});
99+
100+
afterEach(async () => {
101+
corruptStore.close();
102+
await unlink(tempPath);
103+
});
104+
105+
it('throws when get() encounters a non-JSON value', () => {
106+
expect(() => corruptStore.get('bad')).toThrow(
107+
"Failed to parse stored value for key 'bad'",
108+
);
109+
});
110+
111+
it('throws when getAll() encounters a non-JSON value', () => {
112+
expect(() => corruptStore.getAll()).toThrow(
113+
"Failed to parse stored value for key 'bad'",
114+
);
115+
});
116+
});
117+
});
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import type { Json } from '@metamask/utils';
2+
import Sqlite from 'better-sqlite3';
3+
4+
/**
5+
* A synchronous key-value store backed by better-sqlite3.
6+
*
7+
* Uses a single `kv` table with TEXT key (primary key) and TEXT value
8+
* (JSON-serialized). Intended as the persistence backend for wallet
9+
* controller state.
10+
*/
11+
export class KeyValueStore {
12+
readonly #db: Sqlite.Database;
13+
14+
readonly #getStmt: Sqlite.Statement<[string], { value: string } | undefined>;
15+
16+
readonly #setStmt: Sqlite.Statement<[string, string], void>;
17+
18+
readonly #deleteStmt: Sqlite.Statement<[string], void>;
19+
20+
readonly #getAllStmt: Sqlite.Statement<[], { key: string; value: string }>;
21+
22+
constructor(databasePath: string) {
23+
this.#db = new Sqlite(databasePath);
24+
this.#db.pragma('journal_mode = WAL');
25+
this.#db.exec(
26+
'CREATE TABLE IF NOT EXISTS kv (key TEXT PRIMARY KEY, value TEXT NOT NULL)',
27+
);
28+
29+
this.#getStmt = this.#db.prepare('SELECT value FROM kv WHERE key = ?');
30+
this.#setStmt = this.#db.prepare(
31+
'INSERT OR REPLACE INTO kv (key, value) VALUES (?, ?)',
32+
);
33+
this.#deleteStmt = this.#db.prepare('DELETE FROM kv WHERE key = ?');
34+
this.#getAllStmt = this.#db.prepare('SELECT key, value FROM kv');
35+
}
36+
37+
get(key: string): Json | undefined {
38+
const row = this.#getStmt.get(key);
39+
if (!row) {
40+
return undefined;
41+
}
42+
try {
43+
return JSON.parse(row.value);
44+
} catch {
45+
throw new Error(`Failed to parse stored value for key '${key}'`);
46+
}
47+
}
48+
49+
set(key: string, value: Json): void {
50+
this.#setStmt.run(key, JSON.stringify(value));
51+
}
52+
53+
getAll(): Record<string, Json> {
54+
const rows = this.#getAllStmt.all();
55+
const result: Record<string, Json> = {};
56+
for (const row of rows) {
57+
try {
58+
result[row.key] = JSON.parse(row.value);
59+
} catch {
60+
throw new Error(`Failed to parse stored value for key '${row.key}'`);
61+
}
62+
}
63+
return result;
64+
}
65+
66+
delete(key: string): void {
67+
this.#deleteStmt.run(key);
68+
}
69+
70+
close(): void {
71+
this.#db.close();
72+
}
73+
}

0 commit comments

Comments
 (0)