Skip to content

fix: drop legacy unique indexes on activity id that block wallet-scoped rows #137

Description

@jvsena42

Databases created before activity data became wallet-scoped still carry two legacy unique indexes that the wallet-scoping change never dropped:

CREATE UNIQUE INDEX idx_onchain_id   ON onchain_activity(id)
CREATE UNIQUE INDEX idx_lightning_id ON lightning_activity(id)

Both tables are now keyed by PRIMARY KEY (wallet_id, id), so these indexes contradict the schema: they make an activity id globally unique across wallet scopes, when the whole point of the change was that the same id can exist once per wallet.

Impact

On-chain activity ids are the txid, so a single transaction that is visible to two wallet scopes cannot be stored. The realistic case is a user paying their own hardware wallet: the transaction lands under bitkit first, and the watcher's upsert for trezor:{hash} is then rejected.

Because the snapshot is written in one transaction, the collision fails the entire write, not just the colliding row — so the hardware wallet persists zero activities and shows an empty list forever, retrying and failing on every watcher poll:

Failed to persist HW snapshot for 'trezor:84e3cf34…':
BitkitCore.ActivityError.InsertError(errorDetails: "Failed to upsert onchain_activity:
UNIQUE constraint failed: onchain_activity.id")

Only databases that predate wallet scoping are affected, so this is invisible in clean-slate testing and reproduces only on upgraded installs. Both iOS and Android are affected, since the schema is core's.

idx_lightning_id has not been observed failing in practice — watch-only hardware wallets have no Lightning — but it is the same defect and should go with it.

Root cause

Both indexes were created by older core versions, e.g. 0.1.40 at src/modules/activity/implementation.rs:126 and :129:

"CREATE UNIQUE INDEX IF NOT EXISTS idx_onchain_id ON onchain_activity(id)",
"CREATE UNIQUE INDEX IF NOT EXISTS idx_lightning_id ON lightning_activity(id)",

By 0.5.3 they are gone from the source — that version creates no unique indexes at all — but nothing removes them from existing databases. There is no DROP INDEX statement anywhere in the crate, and CREATE ... IF NOT EXISTS obviously cannot undo an index that already exists. The exact release that removed the statements was not pinned down; the boundary is somewhere between 0.1.40 and 0.5.3.

Reproduction

Real-world, on a wallet whose database predates wallet scoping:

  1. Pair a hardware wallet and let the watcher sync — its activities appear.
  2. Send any on-chain amount from the Bitkit wallet to an address of that hardware wallet.
  3. Wait for the watcher to report. The hardware wallet's activity list is now empty, and the log shows UNIQUE constraint failed: onchain_activity.id on every poll.

Minimal, against any current database:

-- reproduce the legacy state
CREATE UNIQUE INDEX IF NOT EXISTS idx_onchain_id ON onchain_activity(id);

then upsert the same activity id under two different wallet_id values. The second fails, though the composite primary key permits it.

Confirmed on a live regtest database: 57 activities under the standard-wallet scope, 0 under the hidden-wallet scope that hit the collision, with both stale indexes present in sqlite_master.

Suggested fix

Drop both indexes in the migration that introduces the composite primary key, and for databases already past that point, unconditionally on init:

DROP INDEX IF EXISTS idx_onchain_id;
DROP INDEX IF EXISTS idx_lightning_id;

It may be worth auditing for other indexes removed from source without a corresponding drop, since the same oversight pattern would apply.

Suggested regression tests

  1. Behavioural, the one that matters. Upsert the same activity id under two different wallet ids and assert both rows persist and are readable by scope. This fails today on a legacy database and passes on a fresh one, so it guards the invariant regardless of how the schema got there.
  2. Migration. Open a database, create idx_onchain_id / idx_lightning_id by hand to simulate the legacy state, run init/migration, then assert sqlite_master contains neither — and that a two-scope upsert then succeeds.
  3. Schema drift guard. Assert that after init, onchain_activity and lightning_activity carry no unique index other than the implicit primary-key one. That catches any future reintroduction, and would have caught this.

References

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