From 405937aa62b8115fe51585105647e6ebfd730360 Mon Sep 17 00:00:00 2001 From: coreyphillips Date: Wed, 26 Aug 2026 14:27:03 -0400 Subject: [PATCH] fix: drop legacy unique indexes on activity id Databases created before activity data became wallet-scoped can still carry idx_onchain_id and idx_lightning_id, which make an activity id globally unique and so contradict the PRIMARY KEY (wallet_id, id) both tables now use. A transaction visible to two wallet scopes (paying your own hardware wallet) then fails to insert, and since the watcher writes its snapshot in one transaction, the whole snapshot is rejected and the hardware wallet shows an empty activity list on every poll. Drop both indexes on every init: the statements that created them are gone from this crate, but CREATE ... IF NOT EXISTS never undid them, and running an older build against a migrated database can recreate them. --- src/modules/activity/implementation.rs | 23 +++ src/modules/activity/tests.rs | 187 +++++++++++++++++++++++++ 2 files changed, 210 insertions(+) diff --git a/src/modules/activity/implementation.rs b/src/modules/activity/implementation.rs index f85cdb62..e1352a71 100644 --- a/src/modules/activity/implementation.rs +++ b/src/modules/activity/implementation.rs @@ -176,6 +176,20 @@ const INDEX_STATEMENTS: &[&str] = &[ "CREATE INDEX IF NOT EXISTS idx_transaction_details_txid ON transaction_details(tx_id)" ]; +/// Unique indexes created by pre-wallet-scoped releases. They make an activity +/// id globally unique, which contradicts the `PRIMARY KEY (wallet_id, id)` the +/// activity tables now use: the same on-chain id (the txid) has to be storable +/// once per wallet scope, e.g. when a transaction is visible to both the Bitkit +/// wallet and a watched hardware wallet. +/// +/// The statements that created them are long gone from this crate, but +/// `CREATE ... IF NOT EXISTS` never removed them from databases that already +/// had them, so they are dropped unconditionally on every init. +const LEGACY_INDEX_DROP_STATEMENTS: &[&str] = &[ + "DROP INDEX IF EXISTS idx_onchain_id", + "DROP INDEX IF EXISTS idx_lightning_id", +]; + const TRIGGER_STATEMENTS: &[&str] = &[ // Update timestamp trigger "CREATE TRIGGER IF NOT EXISTS activities_update_trigger @@ -324,6 +338,15 @@ impl ActivityDB { self.migrate_transaction_details_table()?; self.migrate_pre_activity_metadata_table()?; + // Drop indexes older releases created that the current schema forbids + for statement in LEGACY_INDEX_DROP_STATEMENTS { + if let Err(e) = self.conn.execute(statement, []) { + return Err(ActivityError::InitializationError { + error_details: format!("Error dropping legacy index: {}", e), + }); + } + } + // Create indexes for statement in INDEX_STATEMENTS { if let Err(e) = self.conn.execute(statement, []) { diff --git a/src/modules/activity/tests.rs b/src/modules/activity/tests.rs index f9a1ff5f..071b8da1 100644 --- a/src/modules/activity/tests.rs +++ b/src/modules/activity/tests.rs @@ -36,6 +36,43 @@ mod tests { columns.into_iter().map(|(_, column)| column).collect() } + /// Names of the unique indexes on `table` that are not the implicit + /// primary-key one, i.e. every unique constraint the schema does not declare. + fn extra_unique_index_names(db: &ActivityDB, table: &str) -> Vec { + let mut stmt = db + .conn + .prepare(&format!("PRAGMA index_list({})", table)) + .unwrap(); + let indexes = stmt + .query_map([], |row| { + Ok(( + row.get::<_, String>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, String>(3)?, + )) + }) + .unwrap() + .collect::, _>>() + .unwrap(); + + indexes + .into_iter() + .filter(|(_, unique, origin)| *unique == 1 && origin != "pk") + .map(|(name, _, _)| name) + .collect() + } + + fn create_legacy_activity_id_indexes(db_path: &str) { + let conn = rusqlite::Connection::open(db_path).unwrap(); + conn.execute_batch( + " + 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); + ", + ) + .unwrap(); + } + fn create_test_onchain_activity() -> OnchainActivity { OnchainActivity { wallet_id: DEFAULT_WALLET_ID.to_string(), @@ -284,6 +321,156 @@ mod tests { cleanup(&db_path); } + #[test] + fn test_init_drops_legacy_activity_id_unique_indexes() { + let (db, db_path) = setup(); + drop(db); + create_legacy_activity_id_indexes(&db_path); + + let mut db = ActivityDB::new(&db_path).unwrap(); + + assert!(extra_unique_index_names(&db, "onchain_activity").is_empty()); + assert!(extra_unique_index_names(&db, "lightning_activity").is_empty()); + + // The hardware wallet snapshot shape: one transaction seen by two + // wallet scopes, written in a single transaction. + let shared_txid = "84e3cf34"; + let mut main = create_test_onchain_activity(); + main.id = shared_txid.to_string(); + main.tx_id = shared_txid.to_string(); + main.value = 10_000; + + let mut hardware = create_test_onchain_activity(); + hardware.wallet_id = "trezor:84e3cf34".to_string(); + hardware.id = shared_txid.to_string(); + hardware.tx_id = shared_txid.to_string(); + hardware.value = 20_000; + + db.upsert_onchain_activities(&[main.clone(), hardware.clone()]) + .unwrap(); + + for expected in [&main, &hardware] { + let activity = db + .get_activity_by_id(&expected.wallet_id, shared_txid) + .unwrap() + .unwrap(); + match activity { + Activity::Onchain(activity) => { + assert_eq!(activity.wallet_id, expected.wallet_id); + assert_eq!(activity.value, expected.value); + } + Activity::Lightning(_) => panic!("Expected onchain activity"), + } + } + + let mut main_lightning = create_test_lightning_activity(); + main_lightning.id = "shared_payment_hash".to_string(); + let mut scoped_lightning = main_lightning.clone(); + scoped_lightning.wallet_id = "trezor:84e3cf34".to_string(); + + db.insert_lightning_activity(&main_lightning).unwrap(); + db.insert_lightning_activity(&scoped_lightning).unwrap(); + + cleanup(&db_path); + } + + #[test] + fn test_legacy_schema_migration_drops_activity_id_unique_indexes() { + let db_path = format!("test_db_{}.sqlite", random::()); + { + let conn = rusqlite::Connection::open(&db_path).unwrap(); + conn.execute_batch( + " + CREATE TABLE activities ( + id TEXT PRIMARY KEY, + activity_type TEXT NOT NULL CHECK (activity_type IN ('onchain', 'lightning')), + tx_type TEXT NOT NULL CHECK (tx_type IN ('sent', 'received')), + timestamp INTEGER NOT NULL CHECK (timestamp > 0), + created_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now')), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now')) + ); + CREATE TABLE onchain_activity ( + id TEXT PRIMARY KEY, + tx_id TEXT NOT NULL, + address TEXT NOT NULL CHECK (length(address) > 0), + confirmed BOOLEAN NOT NULL, + value INTEGER NOT NULL CHECK (value >= 0), + fee INTEGER NOT NULL CHECK (fee >= 0), + fee_rate INTEGER NOT NULL CHECK (fee_rate >= 0), + is_boosted BOOLEAN NOT NULL, + boost_tx_ids TEXT NOT NULL, + is_transfer BOOLEAN NOT NULL, + does_exist BOOLEAN NOT NULL, + confirm_timestamp INTEGER, + channel_id TEXT, + transfer_tx_id TEXT + ); + CREATE TABLE lightning_activity ( + id TEXT PRIMARY KEY, + invoice TEXT NOT NULL CHECK (length(invoice) > 0), + value INTEGER NOT NULL CHECK (value >= 0), + status TEXT NOT NULL CHECK (status IN ('pending', 'succeeded', 'failed')), + fee INTEGER, + message TEXT NOT NULL, + preimage TEXT + ); + CREATE TABLE activity_tags ( + activity_id TEXT NOT NULL, + tag TEXT NOT NULL, + PRIMARY KEY (activity_id, tag) + ); + 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); + ", + ) + .unwrap(); + } + + let mut db = ActivityDB::new(&db_path).unwrap(); + + assert!(extra_unique_index_names(&db, "onchain_activity").is_empty()); + assert!(extra_unique_index_names(&db, "lightning_activity").is_empty()); + + let mut main = create_test_onchain_activity(); + main.id = "shared_txid".to_string(); + main.tx_id = "shared_txid".to_string(); + + let mut hardware = main.clone(); + hardware.wallet_id = "trezor:84e3cf34".to_string(); + hardware.value = 20_000; + + db.upsert_onchain_activities(&[main.clone(), hardware.clone()]) + .unwrap(); + + for expected in [&main, &hardware] { + let activity = db + .get_activity_by_id(&expected.wallet_id, "shared_txid") + .unwrap() + .unwrap(); + assert_eq!(activity.get_wallet_id(), expected.wallet_id); + } + + cleanup(&db_path); + } + + #[test] + fn test_activity_tables_have_no_extra_unique_indexes() { + let (db, db_path) = setup(); + + // Only the composite primary keys may constrain uniqueness; anything + // else would scope an activity id more narrowly than (wallet_id, id). + for table in ["activities", "onchain_activity", "lightning_activity"] { + assert_eq!( + extra_unique_index_names(&db, table), + Vec::::new(), + "unexpected unique index on {}", + table + ); + } + + cleanup(&db_path); + } + #[test] fn test_activity_migration_defaults_pre_wallet_rows_to_bitkit() { let db_path = format!("test_db_{}.sqlite", random::());