Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Introduce `DatabaseFactory` trait.

## [v0.20.0] - [v0.19.0]

Expand Down
39 changes: 39 additions & 0 deletions src/database/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,3 +425,42 @@ impl ConfigurableDatabase for AnyDatabase {
impl_from!((), AnyDatabaseConfig, Memory,);
impl_from!(SledDbConfiguration, AnyDatabaseConfig, Sled, #[cfg(feature = "key-value-db")]);
impl_from!(SqliteDbConfiguration, AnyDatabaseConfig, Sqlite, #[cfg(feature = "sqlite")]);

/// Type that implements [`DatabaseFactory`] that builds [`AnyDatabase`].
pub enum AnyDatabaseFactory {
/// Memory database factory
Memory(memory::MemoryDatabaseFactory),
#[cfg(feature = "key-value-db")]
#[cfg_attr(docsrs, doc(cfg(feature = "key-value-db")))]
/// Key-value database factory
Sled(sled::Db),
#[cfg(feature = "sqlite")]
#[cfg_attr(docsrs, doc(cfg(feature = "sqlite")))]
/// Sqlite database factory
Sqlite(sqlite::SqliteDatabaseFactory<String>),
}

impl DatabaseFactory for AnyDatabaseFactory {
type Inner = AnyDatabase;

fn build(
&self,
descriptor: &ExtendedDescriptor,
network: Network,
secp: &SecpCtx,
) -> Result<Self::Inner, Error> {
match self {
AnyDatabaseFactory::Memory(f) => {
f.build(descriptor, network, secp).map(Self::Inner::Memory)
}
#[cfg(feature = "key-value-db")]
AnyDatabaseFactory::Sled(f) => {
f.build(descriptor, network, secp).map(Self::Inner::Sled)
}
#[cfg(feature = "sqlite")]
AnyDatabaseFactory::Sqlite(f) => {
f.build(descriptor, network, secp).map(Self::Inner::Sqlite)
}
}
}
}
30 changes: 30 additions & 0 deletions src/database/keyvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ use crate::database::memory::MapKey;
use crate::database::{BatchDatabase, BatchOperations, Database, SyncTime};
use crate::error::Error;
use crate::types::*;
use crate::wallet::wallet_name_from_descriptor;

use super::DatabaseFactory;

macro_rules! impl_batch_operations {
( { $($after_insert:tt)* }, $process_delete:ident ) => {
Expand Down Expand Up @@ -402,6 +405,21 @@ impl BatchDatabase for Tree {
}
}

/// A [`DatabaseFactory`] implementation that builds [`Tree`]
impl DatabaseFactory for sled::Db {
type Inner = sled::Tree;

fn build(
&self,
descriptor: &crate::descriptor::ExtendedDescriptor,
network: bitcoin::Network,
secp: &crate::wallet::utils::SecpCtx,
) -> Result<Self::Inner, Error> {
let name = wallet_name_from_descriptor(descriptor.clone(), None, network, secp)?;
self.open_tree(&name).map_err(Error::Sled)
}
}

#[cfg(test)]
mod test {
use lazy_static::lazy_static;
Expand Down Expand Up @@ -492,4 +510,16 @@ mod test {
fn test_sync_time() {
crate::database::test::test_sync_time(get_tree());
}

#[test]
fn test_factory() {
let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let mut dir = std::env::temp_dir();
dir.push(format!("bdk_{}", time.as_nanos()));

let fac = sled::open(&dir).unwrap();
crate::database::test::test_factory(&fac);

std::fs::remove_dir_all(&dir).unwrap();
}
}
27 changes: 26 additions & 1 deletion src/database/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ use crate::database::{BatchDatabase, BatchOperations, ConfigurableDatabase, Data
use crate::error::Error;
use crate::types::*;

use super::DatabaseFactory;

// path -> script p{i,e}<path> -> script
// script -> path s<script> -> {i,e}<path>
// outpoint u<outpoint> -> txout
Expand Down Expand Up @@ -475,6 +477,23 @@ impl ConfigurableDatabase for MemoryDatabase {
}
}

/// A [`DatabaseFactory`] implementation that builds [`MemoryDatabase`].
#[derive(Debug, Default)]
pub struct MemoryDatabaseFactory;

impl DatabaseFactory for MemoryDatabaseFactory {
type Inner = MemoryDatabase;

fn build(
&self,
_descriptor: &crate::descriptor::ExtendedDescriptor,
_network: bitcoin::Network,
_secp: &crate::wallet::utils::SecpCtx,
) -> Result<Self::Inner, Error> {
Ok(MemoryDatabase::default())
}
}

#[macro_export]
#[doc(hidden)]
/// Artificially insert a tx in the database, as if we had found it with a `sync`. This is a hidden
Expand Down Expand Up @@ -579,7 +598,7 @@ macro_rules! doctest_wallet {

#[cfg(test)]
mod test {
use super::MemoryDatabase;
use super::{MemoryDatabase, MemoryDatabaseFactory};

fn get_tree() -> MemoryDatabase {
MemoryDatabase::new()
Expand Down Expand Up @@ -629,4 +648,10 @@ mod test {
fn test_sync_time() {
crate::database::test::test_sync_time(get_tree());
}

#[test]
fn test_factory() {
let fac = MemoryDatabaseFactory;
crate::database::test::test_factory(&fac);
}
}
57 changes: 56 additions & 1 deletion src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
use serde::{Deserialize, Serialize};

use bitcoin::hash_types::Txid;
use bitcoin::{OutPoint, Script, Transaction, TxOut};
use bitcoin::{Network, OutPoint, Script, Transaction, TxOut};

use crate::descriptor::ExtendedDescriptor;
use crate::error::Error;
use crate::types::*;
use crate::wallet::utils::SecpCtx;

pub mod any;
pub use any::{AnyDatabase, AnyDatabaseConfig};
Expand Down Expand Up @@ -212,12 +214,28 @@ pub(crate) trait DatabaseUtils: Database {

impl<T: Database> DatabaseUtils for T {}

/// A factory trait that builds databases which share underlying configurations and/or storage
/// paths.
pub trait DatabaseFactory: Sized {
/// Inner type to build
type Inner: BatchDatabase;

/// Builds the defined [`DatabaseFactory::Inner`] type.
fn build(
&self,
descriptor: &ExtendedDescriptor,
network: Network,
secp: &SecpCtx,
) -> Result<Self::Inner, Error>;
}

#[cfg(test)]
pub mod test {
use std::str::FromStr;

use bitcoin::consensus::encode::deserialize;
use bitcoin::hashes::hex::*;
use bitcoin::util::bip32::{self, DerivationPath, ExtendedPubKey};
use bitcoin::*;

use super::*;
Expand Down Expand Up @@ -441,5 +459,42 @@ pub mod test {
assert!(tree.get_sync_time().unwrap().is_none());
}

pub fn test_factory<F: DatabaseFactory>(fac: &F) {
let secp = SecpCtx::new();
let network = Network::Regtest;
let master_privkey = bip32::ExtendedPrivKey::from_str("tprv8ZgxMBicQKsPdowxEXJxXqPYd7i7WN3jG8NTVsq9MYVaR7qnLgi5xo1KZq4z1T89GfGs7BwQTVrtVKWozxwuQLgFNcd3snADMeivux1Y5u5").unwrap();

let descriptor = |acc: usize| -> ExtendedDescriptor {
let path = DerivationPath::from_str(&format!("m/84h/1h/{}h", acc)).unwrap();
let sk = master_privkey.derive_priv(&secp, &path).unwrap();
let pk = ExtendedPubKey::from_priv(&secp, &sk);
ExtendedDescriptor::from_str(&format!(
"wpkh([{}/84h/1h/{}h]{}/0/*)",
master_privkey.fingerprint(&secp),
acc,
pk
))
.unwrap()
};

let mut acc_index = 0_usize;
let mut database = || {
let db = fac.build(&descriptor(acc_index), network, &secp).unwrap();
acc_index += 1;
db
};

test_script_pubkey(database());
test_batch_script_pubkey(database());
test_iter_script_pubkey(database());
test_del_script_pubkey(database());
test_utxo(database());
test_raw_tx(database());
test_tx(database());
test_list_transaction(database());
test_last_index(database());
test_sync_time(database());
}

// TODO: more tests...
}
58 changes: 58 additions & 0 deletions src/database/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@
// You may not use this file except in accordance with one or both of these
// licenses.

use std::path::Path;

use bitcoin::consensus::encode::{deserialize, serialize};
use bitcoin::hash_types::Txid;
use bitcoin::{OutPoint, Script, Transaction, TxOut};

use crate::database::{BatchDatabase, BatchOperations, Database, SyncTime};
use crate::error::Error;
use crate::types::*;
use crate::wallet::wallet_name_from_descriptor;

use rusqlite::{named_params, Connection};

use super::DatabaseFactory;

static MIGRATIONS: &[&str] = &[
"CREATE TABLE version (version INTEGER)",
"INSERT INTO version VALUES (1)",
Expand Down Expand Up @@ -970,11 +975,48 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
Ok(())
}

/// A [`DatabaseFactory`] implementation that builds [`SqliteDatabase`].
///
/// Each built database is stored in path of format: `<path_root>_<hash>.<path_ext>`
/// Where `hash` contains identifying data derived with inputs provided by
/// [`DatabaseFactory::build`] or [`DatabaseFactory::build_with_change`] calls.
pub struct SqliteDatabaseFactory<P> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had an idea for this, let me know what you think: in theory it should be possible to just have a single database by adding an extra column to all the relevant tables that identifies to which descriptor the rows belong.

So my idea for the factory would be something that wraps the db (probably in an Arc) and all the actual SqliteDatabase struct would have a member inside to keep track of what id it belongs to. If you create a SqliteDatabase directly this could either be None or just an empty string.

Then we just have to add an additional WHERE clause to the queries and everything should work magically.

Why we may want to do this? Mainly because I guess having a single file makes it easier to work with, backup, etc. And also this opens the possibility to add more methods in the future that iterate over the whole db, which would probably bring a performance benefit to MultiTracker.

Disadvantages? I guess the extra WHERE clause might cause a small performance hit? But we can also add an index on that column if we feel like it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea! I would assume that performance wouldn't be too much of an issue since we are already using indexes for almost all tables/queries.

However, I feel like this should be in it's separate issue/PR since we need to change the "nature" of SqliteDatabase? What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @afilini's suggestion can be done in this PR if you want to give it a shot. Once that's in this should be ready for general review right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notmandatory Thank you for including this PR in the last call (which I failed to attend 😢 ). I have not touched this PR since the last review because I am skeptical on whether this is actually needed.

My current thinking is that Database itself is NOT an ideal model for implementing multi-descriptor, and the correct approach seems to be that of which bdk_core is doing. I think more discussion is needed before I would be willing to continue/close this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, I think it's fine to leave this one on hold and see how a better approach shapes up with bdk_core.

pub dir: P,
pub ext: String,
}

impl<P: AsRef<Path>> DatabaseFactory for SqliteDatabaseFactory<P> {
type Inner = SqliteDatabase;

fn build(
&self,
descriptor: &crate::descriptor::ExtendedDescriptor,
network: bitcoin::Network,
secp: &crate::wallet::utils::SecpCtx,
) -> Result<Self::Inner, Error> {
// ensure dir exists
std::fs::create_dir_all(&self.dir).map_err(|e| Error::Generic(e.to_string()))?;

let name = wallet_name_from_descriptor(descriptor.clone(), None, network, secp)?;
let ext = self.ext.trim_start_matches('.');

let mut path = std::path::PathBuf::new();
path.push(&self.dir);
path.push(name);
path.set_extension(ext);

// TODO: This is stupid, fix this
Ok(Self::Inner::new(path.to_str().unwrap().to_string()))
}
}

#[cfg(test)]
pub mod test {
use crate::database::SqliteDatabase;
use std::time::{SystemTime, UNIX_EPOCH};

use super::SqliteDatabaseFactory;

fn get_database() -> SqliteDatabase {
let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let mut dir = std::env::temp_dir();
Expand Down Expand Up @@ -1031,4 +1073,20 @@ pub mod test {
fn test_txs() {
crate::database::test::test_list_transaction(get_database());
}

#[test]
fn test_factory() {
let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let mut dir = std::env::temp_dir();
dir.push(format!("bdk_{}", time.as_nanos()));

let fac = SqliteDatabaseFactory {
dir: dir.clone(),
ext: "db".to_string(),
};

crate::database::test::test_factory(&fac);

std::fs::remove_dir_all(&dir).unwrap();
}
}