Skip to content

Commit

Permalink
fix(sqlite)!: some anchor/keychain types paniced on decode
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
evanlinjin authored and notmandatory committed May 20, 2024
1 parent 2b7e69f commit d614e9e
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 186 deletions.
2 changes: 1 addition & 1 deletion crates/sqlite_store/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 2 additions & 1 deletion crates/sqlite_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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`].
///
Expand Down
62 changes: 20 additions & 42 deletions crates/sqlite_store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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.
Expand All @@ -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(),
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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>(
Expand Down
1 change: 1 addition & 0 deletions crates/wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 0 additions & 1 deletion crates/wallet/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit d614e9e

Please sign in to comment.