fix(sqlite)!: some anchor/keychain types paniced on decode

This was caused by the use of `json_extract()` SQL function. SQLite had
funky behavior where not all types returned is encoded in JSON. This
made `serde_json` fail. We fixed this by using `json()` instead.

Tests: updated `bdk_wallet` tests to test with both `bdk_file_store` and
`bdk_sqlite_store`.

Additionally, change constructor of `bdk_sqlite_store::Store` to take in
a `rusqlite::Connection` directly.

Author: evanlinjin

crates/sqlite_store/README.md

@@ -2,7 +2,7 @@
 
 This is a simple [SQLite] relational database schema backed implementation of [`PersistBackend`](bdk_persist::PersistBackend).
 
-The main structure is [`Store`](store::Store) which works with any [`bdk_chain`] based changesets to persist data into a SQLite database file. 
+The main structure is `Store` which works with any [`bdk_chain`] based changesets to persist data into a SQLite database file. 
 
 To use `Store` with [`Wallet`](bdk_wallet::wallet::Wallet) enable the `wallet` feature.
 

crates/sqlite_store/src/lib.rs

@@ -4,7 +4,7 @@
 
 pub mod persist;
 mod schema;
-pub mod store;
+mod store;
 #[cfg(feature = "wallet")]
 #[cfg_attr(docsrs, doc(cfg(feature = "wallet")))]
 pub mod wallet;
@@ -13,6 +13,7 @@ use bdk_chain::bitcoin::Network;
 use bdk_chain::{indexed_tx_graph, keychain, local_chain, Anchor, Append};
 pub use rusqlite;
 use serde::{Deserialize, Serialize};
+pub use store::Store;
 
 /// Change set representing changes to [`local_chain::ChangeSet`] and [`indexed_tx_graph::ChangeSet`].
 ///

crates/sqlite_store/src/store.rs

@@ -7,7 +7,6 @@ use rusqlite::{named_params, Connection};
 use serde::{Deserialize, Serialize};
 use std::collections::{BTreeMap, BTreeSet};
 use std::marker::PhantomData;
-use std::path::Path;
 use std::str::FromStr;
 use std::sync::{Arc, Mutex};
 
@@ -19,9 +18,7 @@ use bdk_chain::{
 
 /// Persists [`ChangeSet`] data in to a relational schema based SQLite database file.
 ///
-/// The changesets loaded or stored represent changes to keychain and blockchain data. If the
-/// keychain (K) is a simple enum without variant fields you must enable the
-/// [serde internal tag](https://serde.rs/enum-representations.html#internally-tagged) feature.
+/// The changesets loaded or stored represent changes to keychain and blockchain data.
 #[derive(Debug)]
 pub struct Store<K, A> {
     // A rusqlite connection to the SQLite database. Uses a Mutex for thread safety.
@@ -35,27 +32,9 @@ where
     K: Ord + for<'de> Deserialize<'de> + Serialize + Send,
     A: Anchor + for<'de> Deserialize<'de> + Serialize + Send,
 {
-    /// Creates a new store from a [`Path`].
-    ///
-    /// The file must be able to be opened with read and write permissions.
-    pub fn new<P: AsRef<Path>>(path: P) -> Result<Self, rusqlite::Error> {
-        let mut conn = Connection::open(path)?;
+    /// Creates a new store from a [`Connection`].
+    pub fn new(mut conn: Connection) -> Result<Self, rusqlite::Error> {
         Self::migrate(&mut conn)?;
-
-        Ok(Self {
-            conn: Mutex::new(conn),
-            keychain_marker: Default::default(),
-            anchor_marker: Default::default(),
-        })
-    }
-
-    /// Creates a new in-memory, not persisted database store.
-    ///
-    /// This is primarily used for testing.
-    pub fn new_memory() -> Result<Self, rusqlite::Error> {
-        let mut conn = Connection::open_in_memory()?;
-        Self::migrate(&mut conn)?;
-
         Ok(Self {
             conn: Mutex::new(conn),
             keychain_marker: Default::default(),
@@ -261,13 +240,13 @@ where
         db_transaction: &rusqlite::Transaction,
     ) -> Result<BTreeMap<K, Descriptor<DescriptorPublicKey>>, Error> {
         let mut select_keychains_added_stmt = db_transaction
-            .prepare_cached("SELECT json_extract(keychain, '$'), descriptor FROM keychain")
+            .prepare_cached("SELECT json(keychain), descriptor FROM keychain")
             .expect("select keychains statement");
 
         let keychains = select_keychains_added_stmt
             .query_map([], |row| {
                 let keychain = row.get_unwrap::<usize, String>(0);
-                let keychain: K = serde_json::from_str(keychain.as_str()).expect("keychain");
+                let keychain = serde_json::from_str::<K>(keychain.as_str()).expect("keychain");
                 let descriptor = row.get_unwrap::<usize, String>(1);
                 let descriptor = Descriptor::from_str(descriptor.as_str()).expect("descriptor");
                 Ok((keychain, descriptor))
@@ -487,7 +466,7 @@ where
     ) -> Result<BTreeSet<(A, Txid)>, Error> {
         // serde_json::from_str
         let mut select_anchor_stmt = db_transaction
-            .prepare_cached("SELECT block_hash, json_extract(anchor, '$'), txid FROM anchor_tx")
+            .prepare_cached("SELECT block_hash, json(anchor), txid FROM anchor_tx")
             .expect("select anchor statement");
         let anchors = select_anchor_stmt
             .query_map([], |row| {
@@ -626,19 +605,18 @@ mod test {
     }
 
     #[test]
-    fn insert_and_load_aggregate_changesets_with_confirmation_time_height_anchor() {
+    fn insert_and_load_aggregate_changesets_with_confirmation_time_height_anchor(
+    ) -> anyhow::Result<()> {
         let (test_changesets, agg_test_changesets) =
             create_test_changesets(&|height, time, hash| ConfirmationTimeHeightAnchor {
                 confirmation_height: height,
                 confirmation_time: time,
                 anchor_block: (height, hash).into(),
             });
 
-        let mut store = Store::<Keychain, ConfirmationTimeHeightAnchor>::new_memory()
+        let conn = Connection::open_in_memory().expect("in memory connection");
+        let mut store = Store::<Keychain, ConfirmationTimeHeightAnchor>::new(conn)
             .expect("create new memory db store");
-        // let mut store =
-        //     Store::<Keychain, ConfirmationTimeHeightAnchor>::new(Path::new("test_agg.sqlite"))
-        //         .expect("create new file db store");
 
         test_changesets.iter().for_each(|changeset| {
             store.write_changes(changeset).expect("write changeset");
@@ -647,21 +625,21 @@ mod test {
         let agg_changeset = store.load_from_persistence().expect("aggregated changeset");
 
         assert_eq!(agg_changeset, Some(agg_test_changesets));
+        Ok(())
     }
 
     #[test]
-    fn insert_and_load_aggregate_changesets_with_confirmation_height_anchor() {
+    fn insert_and_load_aggregate_changesets_with_confirmation_height_anchor() -> anyhow::Result<()>
+    {
         let (test_changesets, agg_test_changesets) =
             create_test_changesets(&|height, _time, hash| ConfirmationHeightAnchor {
                 confirmation_height: height,
                 anchor_block: (height, hash).into(),
             });
 
-        let mut store = Store::<Keychain, ConfirmationHeightAnchor>::new_memory()
+        let conn = Connection::open_in_memory().expect("in memory connection");
+        let mut store = Store::<Keychain, ConfirmationHeightAnchor>::new(conn)
             .expect("create new memory db store");
-        // let mut store =
-        //     Store::<Keychain, ConfirmationHeightAnchor>::new(Path::new("test_agg.sqlite"))
-        //         .expect("create new file db store");
 
         test_changesets.iter().for_each(|changeset| {
             store.write_changes(changeset).expect("write changeset");
@@ -670,17 +648,16 @@ mod test {
         let agg_changeset = store.load_from_persistence().expect("aggregated changeset");
 
         assert_eq!(agg_changeset, Some(agg_test_changesets));
+        Ok(())
     }
 
     #[test]
-    fn insert_and_load_aggregate_changesets_with_blockid_anchor() {
+    fn insert_and_load_aggregate_changesets_with_blockid_anchor() -> anyhow::Result<()> {
         let (test_changesets, agg_test_changesets) =
             create_test_changesets(&|height, _time, hash| BlockId { height, hash });
 
-        let mut store =
-            Store::<Keychain, BlockId>::new_memory().expect("create new memory db store");
-        // let mut store = Store::<Keychain, BlockId>::new(Path::new("test_agg.sqlite"))
-        //     .expect("create new file db store");
+        let conn = Connection::open_in_memory().expect("in memory connection");
+        let mut store = Store::<Keychain, BlockId>::new(conn).expect("create new memory db store");
 
         test_changesets.iter().for_each(|changeset| {
             store.write_changes(changeset).expect("write changeset");
@@ -689,6 +666,7 @@ mod test {
         let agg_changeset = store.load_from_persistence().expect("aggregated changeset");
 
         assert_eq!(agg_changeset, Some(agg_test_changesets));
+        Ok(())
     }
 
     fn create_test_changesets<A: Anchor + Copy>(

crates/wallet/Cargo.toml

@@ -46,6 +46,7 @@ lazy_static = "1.4"
 assert_matches = "1.5.0"
 tempfile = "3"
 bdk_sqlite_store = { path = "../sqlite_store", features = ["wallet"] }
+bdk_file_store = { path = "../file_store" }
 anyhow = "1"
 
 [package.metadata.docs.rs]

crates/wallet/src/types.rs

@@ -20,7 +20,6 @@ use serde::{Deserialize, Serialize};
 
 /// Types of keychains
 #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)]
-#[serde(tag = "type")]
 pub enum KeychainKind {
     /// External keychain, used for deriving recipient addresses.
     External = 0,

crates/wallet/src/wallet/mod.rs

@@ -531,12 +531,13 @@ impl Wallet {
     /// # use bdk_wallet::descriptor::Descriptor;
     /// # use bitcoin::key::Secp256k1;
     /// # use bdk_wallet::KeychainKind;
-    /// # use bdk_sqlite_store::store::Store;
+    /// # use bdk_sqlite_store::{Store, rusqlite::Connection};
     /// #
     /// # fn main() -> Result<(), anyhow::Error> {
     /// # let temp_dir = tempfile::tempdir().expect("must create tempdir");
     /// # let file_path = temp_dir.path().join("store.db");
-    /// # let db = Store::new(&file_path).expect("must create db");
+    /// # let conn = Connection::open(file_path).expect("must open connection");
+    /// # let db = Store::new(conn).expect("must create db");
     /// let secp = Secp256k1::new();
     ///
     /// let (external_descriptor, external_keymap) = Descriptor::parse_descriptor(&secp, "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)").unwrap();