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::());