Skip to content

fix(wallet-toolbox): an interrupted SQLite migration leaves a store that can never migrate again #538

Description

@E-Jacko

Area

Wallet

Affected package or service and version

@bsv/wallet-toolbox — observed on 2.10.2, code unchanged on main at 82ab4d3c (packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts:1551).

Defect

StorageKnex.migrate() passes disableTransactions: isSQLite, so on SQLite every DDL statement autocommits on its own while knex records the migration in knex_migrations only after the whole file's up() resolves.

If the process dies mid-file — or between a file's last statement and the journal insert — the store keeps objects the journal never recorded. Every later migrate.latest() re-runs that file from its first statement and fails with table ... already exists (or duplicate column name). Permanently: the store cannot migrate forward, and knex will not roll back a half-applied, unjournaled file. There is no in-package recovery path.

KnexMigrations.ts currently has 25 createTable and 45 alterTable calls (whole-file occurrence counts, up() and down() together) with 2 hasTable guards, both inside the single SYNC_TRANSFER_MIGRATION ('2026-09-09-001'), whose comment notes it "tolerates an interrupted migration". Every other migration is unguarded.

For a desktop wallet with a local SQLite store, a force-quit during first-run bring-up is enough to trigger this.

Minimal reproduction

Wrap the migration source so one file's up() executes its first createTable and then throws (simulating the kill), then migrate again:

// 1) interrupt the first migration after one statement
await storage.migrate('wallet', identityKey)   // throws

// 2) reopen and migrate again — the app's normal next launch
await storage.migrate('wallet', identityKey)
// -> create table `proven_txs` ... - table `proven_txs` already exists

A self-contained runnable script (better-sqlite3 + knex, drives the real StorageKnex.migrate()) is available; happy to attach it here if useful.

Expected behavior

An interrupted migration either does not apply at all, or converges on the next migrate.latest(). A wallet store should not become permanently unmigratable because a process was killed at the wrong moment.

Actual behavior and evidence

Measured against a built 2.10.2 artifact, better-sqlite3 + knex 3.3, driving the real StorageKnex.migrate():

step result
control (uninterrupted) 24 tables, 21 journal rows, version 2026-08-10-001
interrupt after 1 createTable proven_txs exists; knex_migrations has 0 rows
next migrate() failstable proven_txs already exists
retry fails identically — permanent

Environment

macOS, Node 22, better-sqlite3 13 / knex 3.3 (the versions @bsv/wallet-toolbox itself declares).

Notes on the fix

Two directions, both measured:

  1. Per-file transactions (disableTransactions: false). SQLite DDL is transactional, so an interrupted file rolls back whole and the journal insert stays atomic with its DDL. This closes the alterTable and index-adding migrations too, and closes the journal-write window.
  2. Idempotence guards, extending the SYNC_TRANSFER_MIGRATION pattern. Counting up() regions only: 49 DDL sites — 25 createTable, 4 column-adding alterTable, 13 index-adding alterTable, and 8 alterTable blocks containing 12 .alter() calls. knex exposes no portable hasIndex for the index group, and an .alter() rebuild is not guardable by presence at all (the column exists either way), so guards are a partial fix that also never closes the journal window.

On (1), the PRAGMA foreign_keys concern the current comment cites (knex#4155) appears to be already handled by knex 3.x: sqlite3/schema/ddl.js alter() uses enforceForeignCheck = this.client.transacting ? null : false, deliberately leaving the ambient pragma alone when already inside a transaction — and migrate() already sets PRAGMA foreign_keys = OFF outside migrate.latest(), which persists on SQLite's single-connection pool. Verified by reading PRAGMA foreign_keys from inside a migration (0 with transactions both enabled and disabled), and by running a populated-table .alter() rebuild under both settings: identical row preservation, FK-violating orphan row preserved, foreign key retained in the rebuilt schema.

Happy to open a PR for (1) if that direction is acceptable.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions