refactor(storage): use associated type for BatchWriter and add SQLite CI

- Change StorageBackend::batch() to return Self::Batch associated type instead
  of Box<dyn BatchWriter> to avoid unnecessary heap allocation
- Update BatchWriter::commit() signature from Box<Self> to self
- Update all three backends (RocksDB, SQLite, IndexedDB) to use the new pattern
- Add BatchWriter import to storage_trait.rs for default implementations
- Add SQLite CI test job for ubuntu, macos, and windows
- Add make clippy-sqlite and test-sqlite targets
- Forward sqlite feature from light-client-bin to light-client-lib
- Fix TempDir lifetime issue in test helpers to keep temp directory alive

Author: quake

.github/workflows/ci.yaml

@@ -91,6 +91,22 @@ jobs:
       - name: UnitTest
         if: matrix.os == 'macos-latest'
         run: make test-portable
+  test-sqlite:
+    name: Tests / Build & Test (SQLite)
+    needs: [ rustfmt, clippy ]
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ ubuntu-latest, macos-latest, windows-2022 ]
+    steps:
+      - name: Checkout the Repository
+        uses: actions/checkout@v4
+      - name: Install cargo-nextest
+        uses: taiki-e/install-action@nextest
+      - name: Clippy (SQLite)
+        run: make clippy-sqlite
+      - name: UnitTest (SQLite)
+        run: make test-sqlite
   code_coverage:
     name: Code Coverage
     needs: [ test ]

Makefile

@@ -14,6 +14,11 @@ clippy:
 	cargo clippy --target wasm32-unknown-unknown -p light-client-wasm -p ckb-light-client-lib -p light-client-db-common -p light-client-db-worker --locked -- --deny warnings
 	# Run clippy for native targets
 	cargo clippy -p ckb-light-client --locked -- --deny warnings
+
+clippy-sqlite:
+	# Run clippy for native targets with sqlite feature
+	cargo clippy --features sqlite -p ckb-light-client-lib -p ckb-light-client --locked -- --deny warnings
+
 build:
 	cargo build
 
@@ -27,6 +32,9 @@ test:
 test-portable:
 	cargo nextest run --features portable --hide-progress-bar --success-output immediate --failure-output immediate -p ckb-light-client-lib -p ckb-light-client
 
+test-sqlite:
+	cargo nextest run --features sqlite --hide-progress-bar --success-output immediate --failure-output immediate -p ckb-light-client-lib -p ckb-light-client
+
 test-wasm:
 	wasm-pack test --node ./wasm/light-client-db-common/
 

light-client-bin/Cargo.toml

@@ -49,6 +49,7 @@ tempfile = "3.0"
 default = []
 portable = ["rocksdb/portable"]
 march-native = ["rocksdb/march-native"]
+sqlite = ["ckb-light-client-lib/sqlite"]
 
 # [profile.release]
 # overflow-checks = true

light-client-bin/src/tests/mod.rs

@@ -7,13 +7,15 @@ use ckb_light_client_lib::{
     storage::{LightClientStorage, Storage, StorageWithChainData},
 };
 use ckb_resource::Resource;
+use tempfile::TempDir;
 
 use crate::rpc::{BlockFilterRpcImpl, ChainRpcImpl, TransactionRpcImpl};
 use std::sync::Arc;
 
-pub(crate) fn new_storage(prefix: &str) -> Storage {
+pub(crate) fn new_storage(prefix: &str) -> (Storage, TempDir) {
     let tmp_dir = tempfile::Builder::new().prefix(prefix).tempdir().unwrap();
-    Storage::new(tmp_dir.path().to_str().unwrap())
+    let storage = Storage::new(tmp_dir.path().to_str().unwrap());
+    (storage, tmp_dir)
 }
 
 pub(crate) fn create_peers() -> Arc<Peers> {
@@ -32,6 +34,8 @@ pub(crate) fn create_peers() -> Arc<Peers> {
 pub(crate) struct MockChain {
     storage: Storage,
     consensus: Consensus,
+    #[allow(dead_code)]
+    tmp_dir: TempDir,
 }
 
 impl MockChain {
@@ -43,7 +47,11 @@ impl MockChain {
             .build_consensus()
             .expect("build consensus should be OK");
         storage.init_genesis_block(consensus.genesis_block().data());
-        MockChain { storage, consensus }
+        MockChain {
+            storage,
+            consensus,
+            tmp_dir,
+        }
     }
 
     pub(crate) fn new_with_default_pow(prefix: &str) -> Self {

light-client-bin/src/tests/service.rs

@@ -34,7 +34,7 @@ use crate::{
 
 #[test]
 fn rpc() {
-    let storage = new_storage("rpc");
+    let (storage, _tmp_dir) = new_storage("rpc");
     let rpc = create_block_filter_rpc(storage.clone(), create_peers());
 
     // setup test data
@@ -1062,7 +1062,7 @@ fn rpc() {
 
 #[test]
 fn get_cells_capacity_bug() {
-    let storage = new_storage("get_cells_capacity_bug");
+    let (storage, _tmp_dir) = new_storage("get_cells_capacity_bug");
     let rpc = create_block_filter_rpc(storage.clone(), create_peers());
 
     // setup test data
@@ -1186,7 +1186,7 @@ fn get_cells_capacity_bug() {
 
 #[test]
 fn get_cells_after_rollback_bug() {
-    let storage = new_storage("get_cells_after_rollback_bug");
+    let (storage, _tmp_dir) = new_storage("get_cells_after_rollback_bug");
     let rpc = create_block_filter_rpc(storage.clone(), create_peers());
 
     // setup test data
@@ -1378,7 +1378,7 @@ fn get_cells_after_rollback_bug() {
 
 #[test]
 fn test_set_scripts_clear_matched_blocks() {
-    let storage = new_storage("set-scripts-clear-matched-blocks");
+    let (storage, _tmp_dir) = new_storage("set-scripts-clear-matched-blocks");
     let peers = create_peers();
     let rpc = create_block_filter_rpc(storage.clone(), Arc::clone(&peers));
 
@@ -1427,7 +1427,7 @@ fn test_set_scripts_clear_matched_blocks() {
 
 #[test]
 fn test_set_scripts_command() {
-    let storage = new_storage("set-scripts-command");
+    let (storage, _tmp_dir) = new_storage("set-scripts-command");
     let peers = create_peers();
     let rpc = create_block_filter_rpc(storage.clone(), Arc::clone(&peers));
 
@@ -1513,7 +1513,7 @@ fn test_set_scripts_command() {
 
 #[test]
 fn test_set_scripts_partial_min_filtered_block_number_bug() {
-    let storage = new_storage("set_scripts_partial_min_filtered_block_number_bug");
+    let (storage, _tmp_dir) = new_storage("set_scripts_partial_min_filtered_block_number_bug");
     let peers = create_peers();
     let rpc = create_block_filter_rpc(storage.clone(), Arc::clone(&peers));
 
@@ -1562,7 +1562,7 @@ fn test_set_scripts_partial_min_filtered_block_number_bug() {
 
 #[test]
 fn test_set_scripts_delete_min_filtered_block_number_bug() {
-    let storage = new_storage("set_scripts_delete_min_filtered_block_number_bug");
+    let (storage, _tmp_dir) = new_storage("set_scripts_delete_min_filtered_block_number_bug");
     let peers = create_peers();
     let rpc = create_block_filter_rpc(storage.clone(), Arc::clone(&peers));
 
@@ -1608,7 +1608,7 @@ fn test_set_scripts_delete_min_filtered_block_number_bug() {
 
 #[test]
 fn test_chain_txs_in_same_block_bug() {
-    let storage = new_storage("chain_txs_in_same_block_bug");
+    let (storage, _tmp_dir) = new_storage("chain_txs_in_same_block_bug");
     let rpc = create_block_filter_rpc(storage.clone(), create_peers());
 
     // setup test data

light-client-lib/src/storage/backend.rs

@@ -26,7 +26,7 @@ pub trait BatchWriter {
     fn delete(&mut self, key: &[u8]);
 
     /// Commit all operations in the batch atomically
-    fn commit(self: Box<Self>) -> Result<()>;
+    fn commit(self) -> Result<()>;
 }
 
 /// Low-level storage backend trait
@@ -35,6 +35,9 @@ pub trait BatchWriter {
 /// (RocksDB, SQLite, IndexedDB) must implement. Business logic is implemented
 /// as default methods in `LightClientStorage` which builds on top of this trait.
 pub trait StorageBackend: Send + Sync {
+    /// The batch type for this storage backend
+    type Batch: BatchWriter;
+
     // ========== Basic KV operations ==========
 
     /// Get value by key
@@ -49,7 +52,7 @@ pub trait StorageBackend: Send + Sync {
     // ========== Batch operations ==========
 
     /// Create a new batch for atomic writes
-    fn batch(&self) -> Box<dyn BatchWriter>;
+    fn batch(&self) -> Self::Batch;
 
     // ========== Iterator operations ==========
 

light-client-lib/src/storage/db/browser.rs

@@ -299,14 +299,6 @@ impl Storage {
         Self { channel: chan }
     }
 
-    fn batch(&self) -> Batch {
-        Batch {
-            add: vec![],
-            delete: vec![],
-            comm_arrays: self.channel.clone(),
-        }
-    }
-
     pub fn get<K: AsRef<[u8]>>(&self, key: K) -> Result<Option<Vec<u8>>> {
         let values = self
             .channel
@@ -404,7 +396,19 @@ pub struct Batch {
     comm_arrays: CommunicationChannel,
 }
 
-impl Batch {
+// Implement BatchWriter trait for Batch
+impl BatchWriter for Batch {
+    fn put(&mut self, key: &[u8], value: &[u8]) {
+        self.add.push(KV {
+            key: key.to_vec(),
+            value: value.to_vec(),
+        });
+    }
+
+    fn delete(&mut self, key: &[u8]) {
+        self.delete.push(key.to_vec());
+    }
+
     fn commit(self) -> Result<()> {
         if !self.add.is_empty() {
             self.comm_arrays
@@ -428,26 +432,10 @@ impl Batch {
     }
 }
 
-// Implement BatchWriter trait for Batch
-impl BatchWriter for Batch {
-    fn put(&mut self, key: &[u8], value: &[u8]) {
-        self.add.push(KV {
-            key: key.to_vec(),
-            value: value.to_vec(),
-        });
-    }
-
-    fn delete(&mut self, key: &[u8]) {
-        self.delete.push(key.to_vec());
-    }
-
-    fn commit(self: Box<Self>) -> Result<()> {
-        Batch::commit(*self)
-    }
-}
-
 // Implementation of StorageBackend trait for IndexedDB
 impl StorageBackend for Storage {
+    type Batch = Batch;
+
     fn get(&self, key: Vec<u8>) -> Result<Option<Vec<u8>>> {
         let values = self
             .channel
@@ -479,8 +467,12 @@ impl StorageBackend for Storage {
             .map_err(|e| Error::Indexdb(format!("{:?}", e)))
     }
 
-    fn batch(&self) -> Box<dyn BatchWriter> {
-        Box::new(Storage::batch(self))
+    fn batch(&self) -> Self::Batch {
+        Batch {
+            add: vec![],
+            delete: vec![],
+            comm_arrays: self.channel.clone(),
+        }
     }
 
     fn collect_iterator(

light-client-lib/src/storage/db/native.rs

@@ -70,14 +70,16 @@ impl BatchWriter for Batch {
         self.wb.delete(key).expect("batch delete should be ok");
     }
 
-    fn commit(self: Box<Self>) -> Result<()> {
+    fn commit(self) -> Result<()> {
         self.db.write(&self.wb)?;
         Ok(())
     }
 }
 
 // Implementation of StorageBackend trait for RocksDB
 impl StorageBackend for Storage {
+    type Batch = Batch;
+
     fn get(&self, key: Vec<u8>) -> Result<Option<Vec<u8>>> {
         self.db
             .get(key.as_slice())
@@ -93,8 +95,8 @@ impl StorageBackend for Storage {
         self.db.delete(key).map_err(Into::into)
     }
 
-    fn batch(&self) -> Box<dyn BatchWriter> {
-        Box::new(Storage::batch(self))
+    fn batch(&self) -> Self::Batch {
+        Storage::batch(self)
     }
 
     fn collect_iterator(

light-client-lib/src/storage/db/sqlite.rs

@@ -18,7 +18,7 @@ use ckb_types::{
 };
 use rusqlite::{Connection, OpenFlags};
 use std::{
-    path::Path,
+    path::{Path, PathBuf},
     sync::{Arc, Mutex},
 };
 
@@ -29,13 +29,21 @@ pub struct Storage {
 
 impl Storage {
     pub fn new<P: AsRef<Path>>(path: P) -> Self {
+        // If path is a directory, append db.sqlite filename
+        // This allows consistent API with RocksDB which expects a directory
+        let db_path: PathBuf = if path.as_ref().is_dir() {
+            path.as_ref().join("db.sqlite")
+        } else {
+            path.as_ref().to_path_buf()
+        };
+
         let conn = Connection::open_with_flags(
-            path,
+            &db_path,
             OpenFlags::SQLITE_OPEN_READ_WRITE
                 | OpenFlags::SQLITE_OPEN_CREATE
                 | OpenFlags::SQLITE_OPEN_NO_MUTEX,
         )
-        .expect("Failed to open sqlite database");
+        .unwrap_or_else(|e| panic!("Failed to open sqlite database: {:?}", e));
 
         // Initialize the schema
         Self::init_schema(&conn).expect("Failed to initialize sqlite schema");
@@ -115,7 +123,7 @@ impl BatchWriter for Batch {
         self.operations.push(BatchOp::Delete(key.to_vec()));
     }
 
-    fn commit(self: Box<Self>) -> Result<()> {
+    fn commit(self) -> Result<()> {
         let conn = self.conn.lock().unwrap();
         let tx = conn.unchecked_transaction().map_err(|e| {
             crate::error::Error::runtime(format!("Failed to begin transaction: {}", e))
@@ -153,6 +161,8 @@ impl BatchWriter for Batch {
 
 // Implementation of StorageBackend trait for SQLite
 impl StorageBackend for Storage {
+    type Batch = Batch;
+
     fn get(&self, key: Vec<u8>) -> Result<Option<Vec<u8>>> {
         Storage::get(self, key)
     }
@@ -174,8 +184,8 @@ impl StorageBackend for Storage {
         Ok(())
     }
 
-    fn batch(&self) -> Box<dyn BatchWriter> {
-        Box::new(Storage::batch(self))
+    fn batch(&self) -> Self::Batch {
+        Storage::batch(self)
     }
 
     fn collect_iterator(

light-client-lib/src/storage/storage_trait.rs

@@ -6,11 +6,13 @@
 use std::collections::{HashMap, HashSet};
 
 use super::{
-    backend::StorageBackend, db::IteratorDirection, parse_matched_blocks, BlockNumber, Byte32,
-    CellIndex, CellType, CpIndex, HeaderWithExtension, Key, KeyPrefix, MatchedBlock, MatchedBlocks,
-    OutputIndex, Script, ScriptStatus, ScriptType, SetScriptsCommand, TxIndex, Value,
-    WrappedBlockView, FILTER_SCRIPTS_KEY, GENESIS_BLOCK_KEY, LAST_N_HEADERS_KEY, LAST_STATE_KEY,
-    MATCHED_FILTER_BLOCKS_KEY, MAX_CHECK_POINT_INDEX, MIN_FILTERED_BLOCK_NUMBER,
+    backend::{BatchWriter, StorageBackend},
+    db::IteratorDirection,
+    parse_matched_blocks, BlockNumber, Byte32, CellIndex, CellType, CpIndex, HeaderWithExtension,
+    Key, KeyPrefix, MatchedBlock, MatchedBlocks, OutputIndex, Script, ScriptStatus, ScriptType,
+    SetScriptsCommand, TxIndex, Value, WrappedBlockView, FILTER_SCRIPTS_KEY, GENESIS_BLOCK_KEY,
+    LAST_N_HEADERS_KEY, LAST_STATE_KEY, MATCHED_FILTER_BLOCKS_KEY, MAX_CHECK_POINT_INDEX,
+    MIN_FILTERED_BLOCK_NUMBER,
 };
 use ckb_types::{
     core::HeaderView,