Merge bitcoindevkit#1454: Refactor wallet and persist mod, remove bdk…

…_persist crate

ec36c7e feat(example): use changeset staging with rpc polling example (志宇)
19328d4 feat(wallet)!: change persist API to use `StageExt` and `StageExtAsync` (志宇)
2e40b01 feat(chain): reintroduce a way to stage changesets before persisting (志宇)
36e82ec chore(chain): relax `miniscript` feature flag scope (志宇)
9e97ac0 refactor(persist): update file_store, sqlite, wallet to use bdk_chain::persist (Steve Myers)
54b0c11 feat(persist): add PersistAsync trait and StagedPersistAsync struct (Steve Myers)
aa640ab refactor(persist): rename PersistBackend to Persist, move to chain crate (Steve Myers)

Pull request description:

  ### Description

  Sorry to submit another refactor PR for the persist related stuff, but I think it's worth revisiting. My primary motivations are:

  1. remove `db` from `Wallet` so users have the ability to use `async` storage crates, for example using `sqlx`. I updated docs and examples to let users know they are responsible for persisting changes.
  2. remove the `anyhow` dependency everywhere (except as a dev test dependency). It really doesn't belong in a lib and by removing persistence from `Wallet` it isn't needed.
  3. remove the `bdk_persist` crate and revert back to the original design with generic error types. I kept the `Debug` and `Display` constrains on persist errors so they could still be used with the `anyhow!` macro.

  ### Notes to the reviewers

  I also replaced/renamed old `Persist` with `StagedPersist` struct inspired by bitcoindevkit#1453, it is only used in examples. The `Wallet` handles it's own staging.

  ### Changelog notice

  Changed

  - Removed `db` from `Wallet`, users are now responsible for persisting changes, see docs and examples.
  - Removed the `bdk_persist` crate and moved logic back to `bdk_chain::persist` module.
  - Renamed `PersistBackend` trait to `Persist`
  - Replaced `Persist` struct with `StagedPersist`

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  ValuedMammal:
    ACK ec36c7e

Tree-SHA512: 380b94ae42411ea174720b7c185493c640425f551742f25576a6259a51c1037b441a238c6043f4fdfbf1490aa15f948a34139f1850d0c18d285110f6a9f36018

Cargo.toml

@@ -9,7 +9,6 @@ members = [
     "crates/esplora",
     "crates/bitcoind_rpc",
     "crates/hwi",
-    "crates/persist",
     "crates/testenv",
     "example-crates/example_cli",
     "example-crates/example_electrum",

crates/chain/Cargo.toml

@@ -19,6 +19,7 @@ serde_crate = { package = "serde", version = "1", optional = true, features = ["
 # Use hashbrown as a feature flag to have HashSet and HashMap from it.
 hashbrown = { version = "0.9.1", optional = true, features = ["serde"] }
 miniscript = { version = "12.0.0", optional = true, default-features = false }
+async-trait = {  version = "0.1.80", optional = true }
 
 [dev-dependencies]
 rand = "0.8"
@@ -28,3 +29,4 @@ proptest = "1.2.0"
 default = ["std", "miniscript"]
 std = ["bitcoin/std", "miniscript?/std"]
 serde = ["serde_crate", "bitcoin/serde", "miniscript?/serde"]
+async = ["async-trait"]

crates/chain/src/keychain/txout_index.rs

@@ -207,7 +207,7 @@ impl<K> KeychainTxOutIndex<K> {
 impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     /// Return a reference to the internal [`SpkTxOutIndex`].
     ///
-    /// **WARNING:** The internal index will contain lookahead spks. Refer to
+    /// **WARNING**: The internal index will contain lookahead spks. Refer to
     /// [struct-level docs](KeychainTxOutIndex) for more about `lookahead`.
     pub fn inner(&self) -> &SpkTxOutIndex<(K, u32)> {
         &self.inner

crates/chain/src/lib.rs

@@ -50,6 +50,7 @@ pub use descriptor_ext::{DescriptorExt, DescriptorId};
 mod spk_iter;
 #[cfg(feature = "miniscript")]
 pub use spk_iter::*;
+pub mod persist;
 pub mod spk_client;
 
 #[allow(unused_imports)]

crates/chain/src/persist.rs

@@ -0,0 +1,279 @@
+//! This module is home to the [`PersistBackend`] trait which defines the behavior of a data store
+//! required to persist changes made to BDK data structures.
+//!
+//! The [`CombinedChangeSet`] type encapsulates a combination of [`crate`] structures that are
+//! typically persisted together.
+
+#[cfg(feature = "async")]
+use alloc::boxed::Box;
+#[cfg(feature = "async")]
+use async_trait::async_trait;
+use core::convert::Infallible;
+use core::fmt::{Debug, Display};
+
+use crate::Append;
+
+/// A changeset containing [`crate`] structures typically persisted together.
+#[derive(Debug, Clone, PartialEq)]
+#[cfg(feature = "miniscript")]
+#[cfg_attr(
+    feature = "serde",
+    derive(crate::serde::Deserialize, crate::serde::Serialize),
+    serde(
+        crate = "crate::serde",
+        bound(
+            deserialize = "A: Ord + crate::serde::Deserialize<'de>, K: Ord + crate::serde::Deserialize<'de>",
+            serialize = "A: Ord + crate::serde::Serialize, K: Ord + crate::serde::Serialize",
+        ),
+    )
+)]
+pub struct CombinedChangeSet<K, A> {
+    /// Changes to the [`LocalChain`](crate::local_chain::LocalChain).
+    pub chain: crate::local_chain::ChangeSet,
+    /// Changes to [`IndexedTxGraph`](crate::indexed_tx_graph::IndexedTxGraph).
+    pub indexed_tx_graph: crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>,
+    /// Stores the network type of the transaction data.
+    pub network: Option<bitcoin::Network>,
+}
+
+#[cfg(feature = "miniscript")]
+impl<K, A> core::default::Default for CombinedChangeSet<K, A> {
+    fn default() -> Self {
+        Self {
+            chain: core::default::Default::default(),
+            indexed_tx_graph: core::default::Default::default(),
+            network: None,
+        }
+    }
+}
+
+#[cfg(feature = "miniscript")]
+impl<K: Ord, A: crate::Anchor> crate::Append for CombinedChangeSet<K, A> {
+    fn append(&mut self, other: Self) {
+        crate::Append::append(&mut self.chain, other.chain);
+        crate::Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph);
+        if other.network.is_some() {
+            debug_assert!(
+                self.network.is_none() || self.network == other.network,
+                "network type must either be just introduced or remain the same"
+            );
+            self.network = other.network;
+        }
+    }
+
+    fn is_empty(&self) -> bool {
+        self.chain.is_empty() && self.indexed_tx_graph.is_empty() && self.network.is_none()
+    }
+}
+
+#[cfg(feature = "miniscript")]
+impl<K, A> From<crate::local_chain::ChangeSet> for CombinedChangeSet<K, A> {
+    fn from(chain: crate::local_chain::ChangeSet) -> Self {
+        Self {
+            chain,
+            ..Default::default()
+        }
+    }
+}
+
+#[cfg(feature = "miniscript")]
+impl<K, A> From<crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>>
+    for CombinedChangeSet<K, A>
+{
+    fn from(
+        indexed_tx_graph: crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>,
+    ) -> Self {
+        Self {
+            indexed_tx_graph,
+            ..Default::default()
+        }
+    }
+}
+
+#[cfg(feature = "miniscript")]
+impl<K, A> From<crate::keychain::ChangeSet<K>> for CombinedChangeSet<K, A> {
+    fn from(indexer: crate::keychain::ChangeSet<K>) -> Self {
+        Self {
+            indexed_tx_graph: crate::indexed_tx_graph::ChangeSet {
+                indexer,
+                ..Default::default()
+            },
+            ..Default::default()
+        }
+    }
+}
+
+/// A persistence backend for writing and loading changesets.
+///
+/// `C` represents the changeset; a datatype that records changes made to in-memory data structures
+/// that are to be persisted, or retrieved from persistence.
+pub trait PersistBackend<C> {
+    /// The error the backend returns when it fails to write.
+    type WriteError: Debug + Display;
+
+    /// The error the backend returns when it fails to load changesets `C`.
+    type LoadError: Debug + Display;
+
+    /// Writes a changeset to the persistence backend.
+    ///
+    /// It is up to the backend what it does with this. It could store every changeset in a list or
+    /// it inserts the actual changes into a more structured database. All it needs to guarantee is
+    /// that [`load_from_persistence`] restores a keychain tracker to what it should be if all
+    /// changesets had been applied sequentially.
+    ///
+    /// [`load_from_persistence`]: Self::load_changes
+    fn write_changes(&mut self, changeset: &C) -> Result<(), Self::WriteError>;
+
+    /// Return the aggregate changeset `C` from persistence.
+    fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError>;
+}
+
+impl<C> PersistBackend<C> for () {
+    type WriteError = Infallible;
+    type LoadError = Infallible;
+
+    fn write_changes(&mut self, _changeset: &C) -> Result<(), Self::WriteError> {
+        Ok(())
+    }
+
+    fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError> {
+        Ok(None)
+    }
+}
+
+#[cfg(feature = "async")]
+/// An async persistence backend for writing and loading changesets.
+///
+/// `C` represents the changeset; a datatype that records changes made to in-memory data structures
+/// that are to be persisted, or retrieved from persistence.
+#[async_trait]
+pub trait PersistBackendAsync<C> {
+    /// The error the backend returns when it fails to write.
+    type WriteError: Debug + Display;
+
+    /// The error the backend returns when it fails to load changesets `C`.
+    type LoadError: Debug + Display;
+
+    /// Writes a changeset to the persistence backend.
+    ///
+    /// It is up to the backend what it does with this. It could store every changeset in a list or
+    /// it inserts the actual changes into a more structured database. All it needs to guarantee is
+    /// that [`load_from_persistence`] restores a keychain tracker to what it should be if all
+    /// changesets had been applied sequentially.
+    ///
+    /// [`load_from_persistence`]: Self::load_changes
+    async fn write_changes(&mut self, changeset: &C) -> Result<(), Self::WriteError>;
+
+    /// Return the aggregate changeset `C` from persistence.
+    async fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError>;
+}
+
+#[cfg(feature = "async")]
+#[async_trait]
+impl<C> PersistBackendAsync<C> for () {
+    type WriteError = Infallible;
+    type LoadError = Infallible;
+
+    async fn write_changes(&mut self, _changeset: &C) -> Result<(), Self::WriteError> {
+        Ok(())
+    }
+
+    async fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError> {
+        Ok(None)
+    }
+}
+
+/// Extends a changeset so that it acts as a convenient staging area for any [`PersistBackend`].
+///
+/// Not all changes to the in-memory representation needs to be written to disk right away.
+/// [`Append::append`] can be used to *stage* changes first and then [`StageExt::commit_to`] can be
+/// used to write changes to disk.
+pub trait StageExt: Append + Default + Sized {
+    /// Commit the staged changes to the persistence `backend`.
+    ///
+    /// Changes that are committed (if any) are returned.
+    ///
+    /// # Error
+    ///
+    /// Returns a backend-defined error if this fails.
+    fn commit_to<B>(&mut self, backend: &mut B) -> Result<Option<Self>, B::WriteError>
+    where
+        B: PersistBackend<Self>,
+    {
+        // do not do anything if changeset is empty
+        if self.is_empty() {
+            return Ok(None);
+        }
+        backend.write_changes(&*self)?;
+        // only clear if changes are written successfully to backend
+        Ok(Some(core::mem::take(self)))
+    }
+
+    /// Stages a new `changeset` and commits it (alongside any other previously staged changes) to
+    /// the persistence `backend`.
+    ///
+    /// Convenience method for calling [`Append::append`] and then [`StageExt::commit_to`].
+    fn append_and_commit_to<B>(
+        &mut self,
+        changeset: Self,
+        backend: &mut B,
+    ) -> Result<Option<Self>, B::WriteError>
+    where
+        B: PersistBackend<Self>,
+    {
+        Append::append(self, changeset);
+        self.commit_to(backend)
+    }
+}
+
+impl<C: Append + Default> StageExt for C {}
+
+/// Extends a changeset so that it acts as a convenient staging area for any
+/// [`PersistBackendAsync`].
+///
+/// Not all changes to the in-memory representation needs to be written to disk right away.
+/// [`Append::append`] can be used to *stage* changes first and then [`StageExtAsync::commit_to`]
+/// can be used to write changes to disk.
+#[cfg(feature = "async")]
+#[async_trait]
+pub trait StageExtAsync: Append + Default + Sized + Send + Sync {
+    /// Commit the staged changes to the persistence `backend`.
+    ///
+    /// Changes that are committed (if any) are returned.
+    ///
+    /// # Error
+    ///
+    /// Returns a backend-defined error if this fails.
+    async fn commit_to<B>(&mut self, backend: &mut B) -> Result<Option<Self>, B::WriteError>
+    where
+        B: PersistBackendAsync<Self> + Send + Sync,
+    {
+        // do not do anything if changeset is empty
+        if self.is_empty() {
+            return Ok(None);
+        }
+        backend.write_changes(&*self).await?;
+        // only clear if changes are written successfully to backend
+        Ok(Some(core::mem::take(self)))
+    }
+
+    /// Stages a new `changeset` and commits it (alongside any other previously staged changes) to
+    /// the persistence `backend`.
+    ///
+    /// Convenience method for calling [`Append::append`] and then [`StageExtAsync::commit_to`].
+    async fn append_and_commit_to<B>(
+        &mut self,
+        changeset: Self,
+        backend: &mut B,
+    ) -> Result<Option<Self>, B::WriteError>
+    where
+        B: PersistBackendAsync<Self> + Send + Sync,
+    {
+        Append::append(self, changeset);
+        self.commit_to(backend).await
+    }
+}
+
+#[cfg(feature = "async")]
+#[async_trait]
+impl<C: Append + Default + Send + Sync> StageExtAsync for C {}

crates/chain/src/spk_client.rs

@@ -4,9 +4,9 @@ use crate::{
     collections::BTreeMap, keychain::Indexed, local_chain::CheckPoint,
     ConfirmationTimeHeightAnchor, TxGraph,
 };
-use alloc::{boxed::Box, vec::Vec};
+use alloc::boxed::Box;
 use bitcoin::{OutPoint, Script, ScriptBuf, Txid};
-use core::{fmt::Debug, marker::PhantomData, ops::RangeBounds};
+use core::marker::PhantomData;
 
 /// Data required to perform a spk-based blockchain client sync.
 ///
@@ -158,12 +158,13 @@ impl SyncRequest {
     /// This consumes the [`SyncRequest`] and returns the updated one.
     #[cfg(feature = "miniscript")]
     #[must_use]
-    pub fn populate_with_revealed_spks<K: Clone + Ord + Debug + Send + Sync>(
+    pub fn populate_with_revealed_spks<K: Clone + Ord + core::fmt::Debug + Send + Sync>(
         self,
         index: &crate::keychain::KeychainTxOutIndex<K>,
-        spk_range: impl RangeBounds<K>,
+        spk_range: impl core::ops::RangeBounds<K>,
     ) -> Self {
         use alloc::borrow::ToOwned;
+        use alloc::vec::Vec;
         self.chain_spks(
             index
                 .revealed_spks(spk_range)
@@ -223,7 +224,7 @@ impl<K: Ord + Clone> FullScanRequest<K> {
         index: &crate::keychain::KeychainTxOutIndex<K>,
     ) -> Self
     where
-        K: Debug,
+        K: core::fmt::Debug,
     {
         let mut req = Self::from_chain_tip(chain_tip);
         for (keychain, spks) in index.all_unbounded_spk_iters() {

crates/file_store/Cargo.toml

@@ -5,15 +5,13 @@ edition = "2021"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/bitcoindevkit/bdk"
 documentation = "https://docs.rs/bdk_file_store"
-description = "A simple append-only flat file implementation of Persist for Bitcoin Dev Kit."
+description = "A simple append-only flat file implementation of PersistBackend for Bitcoin Dev Kit."
 keywords = ["bitcoin", "persist", "persistence", "bdk", "file"]
 authors = ["Bitcoin Dev Kit Developers"]
 readme = "README.md"
 
 [dependencies]
-anyhow = { version = "1", default-features = false }
 bdk_chain = { path = "../chain", version = "0.15.0", features = [ "serde", "miniscript" ] }
-bdk_persist = { path = "../persist", version = "0.3.0"}
 bincode = { version = "1" }
 serde = { version = "1", features = ["derive"] }
 

crates/file_store/README.md

@@ -1,6 +1,6 @@
 # BDK File Store
 
-This is a simple append-only flat file implementation of [`PersistBackend`](bdk_persist::PersistBackend).
+This is a simple append-only flat file implementation of [`PersistBackend`](bdk_chain::persist::PersistBackend).
 
 The main structure is [`Store`] which works with any [`bdk_chain`] based changesets to persist data into a flat file.