refactor: improve database wipe methods based on PR feedback

- Add get_activity_db() helper to reduce code duplication
- Ensure wipe_all_databases() requires initialized DBs
- Remove unused methods: remove_all_tags(), remove_all_info()
- Consolidate remove_all() into wipe_all() to remove the wrapper
- Document that enum state tables persist across wipes

Author: coreyphillips

src/lib.rs

@@ -45,6 +45,14 @@ fn ensure_runtime() -> &'static Runtime {
     })
 }
 
+/// Helper function to get a reference to the activity database connections
+fn get_activity_db() -> Result<std::sync::MutexGuard<'static, DatabaseConnections>, ActivityError> {
+    let cell = DB.get().ok_or(ActivityError::ConnectionError {
+        error_details: "Database not initialized. Call init_db first.".to_string()
+    })?;
+    Ok(cell.lock().unwrap())
+}
+
 #[uniffi::export]
 pub async fn decode(invoice: String) -> Result<Scanner, DecodingError> {
     let rt = ensure_runtime();
@@ -290,10 +298,7 @@ pub fn get_activities(
     limit: Option<u32>,
     sort_direction: Option<SortDirection>
 ) -> Result<Vec<Activity>, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let guard = cell.lock().unwrap();
+    let guard = get_activity_db()?;
     let db = guard.activity_db.as_ref().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -302,10 +307,7 @@ pub fn get_activities(
 
 #[uniffi::export]
 pub fn upsert_activity(activity: Activity) -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -314,10 +316,7 @@ pub fn upsert_activity(activity: Activity) -> Result<(), ActivityError> {
 
 #[uniffi::export]
 pub fn insert_activity(activity: Activity) -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -329,10 +328,7 @@ pub fn insert_activity(activity: Activity) -> Result<(), ActivityError> {
 
 #[uniffi::export]
 pub fn update_activity(activity_id: String, activity: Activity) -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -344,10 +340,7 @@ pub fn update_activity(activity_id: String, activity: Activity) -> Result<(), Ac
 
 #[uniffi::export]
 pub fn get_activity_by_id(activity_id: String) -> Result<Option<Activity>, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let guard = cell.lock().unwrap();
+    let guard = get_activity_db()?;
     let db = guard.activity_db.as_ref().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -356,10 +349,7 @@ pub fn get_activity_by_id(activity_id: String) -> Result<Option<Activity>, Activ
 
 #[uniffi::export]
 pub fn delete_activity_by_id(activity_id: String) -> Result<bool, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -368,10 +358,7 @@ pub fn delete_activity_by_id(activity_id: String) -> Result<bool, ActivityError>
 
 #[uniffi::export]
 pub fn add_tags(activity_id: String, tags: Vec<String>) -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -380,10 +367,7 @@ pub fn add_tags(activity_id: String, tags: Vec<String>) -> Result<(), ActivityEr
 
 #[uniffi::export]
 pub fn remove_tags(activity_id: String, tags: Vec<String>) -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -392,10 +376,7 @@ pub fn remove_tags(activity_id: String, tags: Vec<String>) -> Result<(), Activit
 
 #[uniffi::export]
 pub fn get_tags(activity_id: String) -> Result<Vec<String>, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let guard = cell.lock().unwrap();
+    let guard = get_activity_db()?;
     let db = guard.activity_db.as_ref().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -404,10 +385,7 @@ pub fn get_tags(activity_id: String) -> Result<Vec<String>, ActivityError> {
 
 #[uniffi::export]
 pub fn get_activities_by_tag(tag: String, limit: Option<u32>, sort_direction: Option<SortDirection>) -> Result<Vec<Activity>, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let guard = cell.lock().unwrap();
+    let guard = get_activity_db()?;
     let db = guard.activity_db.as_ref().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -416,10 +394,7 @@ pub fn get_activities_by_tag(tag: String, limit: Option<u32>, sort_direction: Op
 
 #[uniffi::export]
 pub fn get_all_unique_tags() -> Result<Vec<String>, ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let guard = cell.lock().unwrap();
+    let guard = get_activity_db()?;
     let db = guard.activity_db.as_ref().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -1248,10 +1223,7 @@ pub fn trezor_compose_transaction(
 
 #[uniffi::export]
 pub fn activity_wipe_all() -> Result<(), ActivityError> {
-    let cell = DB.get().ok_or(ActivityError::ConnectionError {
-        error_details: "Database not initialized. Call init_db first.".to_string()
-    })?;
-    let mut guard = cell.lock().unwrap();
+    let mut guard = get_activity_db()?;
     let db = guard.activity_db.as_mut().ok_or(ActivityError::ConnectionError {
         error_details: "Database not initialized. Call init_db first.".to_string()
     })?;
@@ -1307,30 +1279,32 @@ pub async fn blocktank_wipe_all() -> Result<(), BlocktankError> {
 pub async fn wipe_all_databases() -> Result<String, DbError> {
     let rt = ensure_runtime();
 
-    // Wipe activity database
+    // Wipe activity database - require it to be initialized
     {
         let cell = DB.get().ok_or(DbError::InitializationError {
             error_details: "Database not initialized. Call init_db first.".to_string()
         })?;
         let mut guard = cell.lock().unwrap();
-        if let Some(db) = guard.activity_db.as_mut() {
-            db.wipe_all().map_err(|e| DbError::InitializationError {
-                error_details: format!("Failed to wipe activity database: {}", e)
-            })?;
-        }
+        let db = guard.activity_db.as_mut().ok_or(DbError::InitializationError {
+            error_details: "Activity database not initialized. Call init_db first.".to_string()
+        })?;
+        db.wipe_all().map_err(|e| DbError::InitializationError {
+            error_details: format!("Failed to wipe activity database: {}", e)
+        })?;
     }
 
-    // Wipe blocktank database
+    // Wipe blocktank database - require it to be initialized
     rt.spawn(async move {
         let cell = ASYNC_DB.get().ok_or(DbError::InitializationError {
             error_details: "Database not initialized. Call init_db first.".to_string()
         })?;
         let guard = cell.lock().await;
-        if let Some(db) = guard.blocktank_db.as_ref() {
-            db.wipe_all().await.map_err(|e| DbError::InitializationError {
-                error_details: format!("Failed to wipe blocktank database: {}", e)
-            })?;
-        }
+        let db = guard.blocktank_db.as_ref().ok_or(DbError::InitializationError {
+            error_details: "Blocktank database not initialized. Call init_db first.".to_string()
+        })?;
+        db.wipe_all().await.map_err(|e| DbError::InitializationError {
+            error_details: format!("Failed to wipe blocktank database: {}", e)
+        })?;
         Ok::<(), DbError>(())
     }).await.unwrap()?;
 

src/modules/activity/implementation.rs

@@ -1020,8 +1020,9 @@ impl ActivityDB {
         }
     }
 
-    /// Removes all activities from the database
-    pub fn remove_all(&mut self) -> Result<(), ActivityError> {
+    /// Wipes all activity data from the database
+    /// This deletes all activities, which cascades to delete all tags due to foreign key constraints
+    pub fn wipe_all(&mut self) -> Result<(), ActivityError> {
         let tx = self.conn.transaction().map_err(|e| ActivityError::DataError {
             error_details: format!("Failed to start transaction: {}", e),
         })?;
@@ -1038,19 +1039,4 @@ impl ActivityDB {
 
         Ok(())
     }
-
-    /// Removes all tags from the database
-    pub fn remove_all_tags(&mut self) -> Result<(), ActivityError> {
-        self.conn.execute("DELETE FROM activity_tags", [])
-            .map_err(|e| ActivityError::DataError {
-                error_details: format!("Failed to delete all tags: {}", e),
-            })?;
-
-        Ok(())
-    }
-
-    /// Wipes all activity data from the database
-    pub fn wipe_all(&mut self) -> Result<(), ActivityError> {
-        self.remove_all()
-    }
 }

src/modules/activity/tests.rs

@@ -1281,80 +1281,6 @@ mod tests {
         cleanup(&db_path);
     }
 
-    #[test]
-    fn test_remove_all() {
-        let (mut db, db_path) = setup();
-
-        // Insert multiple activities
-        let activity1 = create_test_onchain_activity();
-        let mut activity2 = create_test_lightning_activity();
-        activity2.id = "test_lightning_2".to_string();
-        let mut activity3 = create_test_onchain_activity();
-        activity3.id = "test_onchain_3".to_string();
-
-        db.insert_onchain_activity(&activity1).unwrap();
-        db.insert_lightning_activity(&activity2).unwrap();
-        db.insert_onchain_activity(&activity3).unwrap();
-
-        // Add tags to verify cascade deletion
-        db.add_tags(&activity1.id, &["tag1".to_string(), "tag2".to_string()]).unwrap();
-        db.add_tags(&activity2.id, &["tag3".to_string()]).unwrap();
-
-        // Verify activities exist
-        let activities = db.get_activities(None, None, None, None, None, None, None, None).unwrap();
-        assert_eq!(activities.len(), 3);
-
-        // Verify tags exist
-        let tags = db.get_all_unique_tags().unwrap();
-        assert_eq!(tags.len(), 3);
-
-        // Remove all activities
-        db.remove_all().unwrap();
-
-        // Verify all activities are deleted
-        let activities_after = db.get_activities(None, None, None, None, None, None, None, None).unwrap();
-        assert_eq!(activities_after.len(), 0);
-
-        // Verify tags are also deleted (cascade)
-        let tags_after = db.get_all_unique_tags().unwrap();
-        assert_eq!(tags_after.len(), 0);
-
-        cleanup(&db_path);
-    }
-
-    #[test]
-    fn test_remove_all_tags() {
-        let (mut db, db_path) = setup();
-
-        // Insert activities
-        let activity1 = create_test_onchain_activity();
-        let mut activity2 = create_test_lightning_activity();
-        activity2.id = "test_lightning_2".to_string();
-
-        db.insert_onchain_activity(&activity1).unwrap();
-        db.insert_lightning_activity(&activity2).unwrap();
-
-        // Add multiple tags
-        db.add_tags(&activity1.id, &["tag1".to_string(), "tag2".to_string(), "tag3".to_string()]).unwrap();
-        db.add_tags(&activity2.id, &["tag4".to_string(), "tag5".to_string()]).unwrap();
-
-        // Verify tags exist
-        let tags = db.get_all_unique_tags().unwrap();
-        assert_eq!(tags.len(), 5);
-
-        // Remove all tags
-        db.remove_all_tags().unwrap();
-
-        // Verify all tags are deleted
-        let tags_after = db.get_all_unique_tags().unwrap();
-        assert_eq!(tags_after.len(), 0);
-
-        // Verify activities still exist
-        let activities = db.get_activities(None, None, None, None, None, None, None, None).unwrap();
-        assert_eq!(activities.len(), 2);
-
-        cleanup(&db_path);
-    }
 
     #[test]
     fn test_wipe_all() {

src/modules/blocktank/db.rs

@@ -846,19 +846,15 @@ impl BlocktankDB {
         Ok(())
     }
 
-    /// Removes all info entries from the database
-    pub async fn remove_all_info(&self) -> Result<(), BlocktankError> {
-        let conn = self.conn.lock().await;
-
-        conn.execute("DELETE FROM info", [])
-            .map_err(|e| BlocktankError::DatabaseError {
-                error_details: format!("Failed to delete all info entries: {}", e),
-            })?;
-
-        Ok(())
-    }
-
     /// Removes all data from all Blocktank tables
+    ///
+    /// This wipes:
+    /// - All orders
+    /// - All CJIT entries
+    /// - All info entries
+    ///
+    /// Note: This does NOT delete the enum state tables (order_states, payment_states, cjit_states)
+    /// as these contain static reference data that should persist across wipes.
     pub async fn wipe_all(&self) -> Result<(), BlocktankError> {
         let mut conn = self.conn.lock().await;
 

src/modules/blocktank/tests.rs

@@ -1533,36 +1533,6 @@ mod tests {
         assert_eq!(entries_new.len(), 1);
     }
 
-    #[tokio::test]
-    async fn test_remove_all_info() {
-        let temp_dir = tempfile::tempdir().expect("Failed to create temp directory");
-        let db_path = format!("{}/test_blocktank.db", temp_dir.path().display());
-
-        let db = BlocktankDB::new(&db_path, None).await
-            .expect("Failed to create BlocktankDB");
-
-        // Create and insert test info
-        let info = create_test_info();
-        db.upsert_info(&info).await.expect("Failed to insert info");
-
-        // Verify info exists
-        let retrieved_info = db.get_info().await.expect("Failed to get info");
-        assert!(retrieved_info.is_some());
-
-        // Remove all info
-        db.remove_all_info().await.expect("Failed to remove all info");
-
-        // Verify info is deleted
-        let info_after = db.get_info().await.expect("Failed to get info");
-        assert!(info_after.is_none());
-
-        // Verify we can still insert new info after wipe
-        let new_info = create_test_info();
-        db.upsert_info(&new_info).await.expect("Failed to insert new info");
-        let info_new = db.get_info().await.expect("Failed to get info");
-        assert!(info_new.is_some());
-    }
-
     #[tokio::test]
     async fn test_wipe_all() {
         let temp_dir = tempfile::tempdir().expect("Failed to create temp directory");