Skip to content

Commit 782eb56

Browse files
committed
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
2 parents 1c593a3 + ec36c7e commit 782eb56

File tree

40 files changed

+852
-795
lines changed

40 files changed

+852
-795
lines changed

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ members = [
99
"crates/esplora",
1010
"crates/bitcoind_rpc",
1111
"crates/hwi",
12-
"crates/persist",
1312
"crates/testenv",
1413
"example-crates/example_cli",
1514
"example-crates/example_electrum",

crates/chain/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ serde_crate = { package = "serde", version = "1", optional = true, features = ["
1919
# Use hashbrown as a feature flag to have HashSet and HashMap from it.
2020
hashbrown = { version = "0.9.1", optional = true, features = ["serde"] }
2121
miniscript = { version = "12.0.0", optional = true, default-features = false }
22+
async-trait = { version = "0.1.80", optional = true }
2223

2324
[dev-dependencies]
2425
rand = "0.8"
@@ -28,3 +29,4 @@ proptest = "1.2.0"
2829
default = ["std", "miniscript"]
2930
std = ["bitcoin/std", "miniscript?/std"]
3031
serde = ["serde_crate", "bitcoin/serde", "miniscript?/serde"]
32+
async = ["async-trait"]

crates/chain/src/keychain/txout_index.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ impl<K> KeychainTxOutIndex<K> {
207207
impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
208208
/// Return a reference to the internal [`SpkTxOutIndex`].
209209
///
210-
/// **WARNING:** The internal index will contain lookahead spks. Refer to
210+
/// **WARNING**: The internal index will contain lookahead spks. Refer to
211211
/// [struct-level docs](KeychainTxOutIndex) for more about `lookahead`.
212212
pub fn inner(&self) -> &SpkTxOutIndex<(K, u32)> {
213213
&self.inner

crates/chain/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub use descriptor_ext::{DescriptorExt, DescriptorId};
5050
mod spk_iter;
5151
#[cfg(feature = "miniscript")]
5252
pub use spk_iter::*;
53+
pub mod persist;
5354
pub mod spk_client;
5455

5556
#[allow(unused_imports)]

crates/chain/src/persist.rs

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,279 @@
1+
//! This module is home to the [`PersistBackend`] trait which defines the behavior of a data store
2+
//! required to persist changes made to BDK data structures.
3+
//!
4+
//! The [`CombinedChangeSet`] type encapsulates a combination of [`crate`] structures that are
5+
//! typically persisted together.
6+
7+
#[cfg(feature = "async")]
8+
use alloc::boxed::Box;
9+
#[cfg(feature = "async")]
10+
use async_trait::async_trait;
11+
use core::convert::Infallible;
12+
use core::fmt::{Debug, Display};
13+
14+
use crate::Append;
15+
16+
/// A changeset containing [`crate`] structures typically persisted together.
17+
#[derive(Debug, Clone, PartialEq)]
18+
#[cfg(feature = "miniscript")]
19+
#[cfg_attr(
20+
feature = "serde",
21+
derive(crate::serde::Deserialize, crate::serde::Serialize),
22+
serde(
23+
crate = "crate::serde",
24+
bound(
25+
deserialize = "A: Ord + crate::serde::Deserialize<'de>, K: Ord + crate::serde::Deserialize<'de>",
26+
serialize = "A: Ord + crate::serde::Serialize, K: Ord + crate::serde::Serialize",
27+
),
28+
)
29+
)]
30+
pub struct CombinedChangeSet<K, A> {
31+
/// Changes to the [`LocalChain`](crate::local_chain::LocalChain).
32+
pub chain: crate::local_chain::ChangeSet,
33+
/// Changes to [`IndexedTxGraph`](crate::indexed_tx_graph::IndexedTxGraph).
34+
pub indexed_tx_graph: crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>,
35+
/// Stores the network type of the transaction data.
36+
pub network: Option<bitcoin::Network>,
37+
}
38+
39+
#[cfg(feature = "miniscript")]
40+
impl<K, A> core::default::Default for CombinedChangeSet<K, A> {
41+
fn default() -> Self {
42+
Self {
43+
chain: core::default::Default::default(),
44+
indexed_tx_graph: core::default::Default::default(),
45+
network: None,
46+
}
47+
}
48+
}
49+
50+
#[cfg(feature = "miniscript")]
51+
impl<K: Ord, A: crate::Anchor> crate::Append for CombinedChangeSet<K, A> {
52+
fn append(&mut self, other: Self) {
53+
crate::Append::append(&mut self.chain, other.chain);
54+
crate::Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph);
55+
if other.network.is_some() {
56+
debug_assert!(
57+
self.network.is_none() || self.network == other.network,
58+
"network type must either be just introduced or remain the same"
59+
);
60+
self.network = other.network;
61+
}
62+
}
63+
64+
fn is_empty(&self) -> bool {
65+
self.chain.is_empty() && self.indexed_tx_graph.is_empty() && self.network.is_none()
66+
}
67+
}
68+
69+
#[cfg(feature = "miniscript")]
70+
impl<K, A> From<crate::local_chain::ChangeSet> for CombinedChangeSet<K, A> {
71+
fn from(chain: crate::local_chain::ChangeSet) -> Self {
72+
Self {
73+
chain,
74+
..Default::default()
75+
}
76+
}
77+
}
78+
79+
#[cfg(feature = "miniscript")]
80+
impl<K, A> From<crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>>
81+
for CombinedChangeSet<K, A>
82+
{
83+
fn from(
84+
indexed_tx_graph: crate::indexed_tx_graph::ChangeSet<A, crate::keychain::ChangeSet<K>>,
85+
) -> Self {
86+
Self {
87+
indexed_tx_graph,
88+
..Default::default()
89+
}
90+
}
91+
}
92+
93+
#[cfg(feature = "miniscript")]
94+
impl<K, A> From<crate::keychain::ChangeSet<K>> for CombinedChangeSet<K, A> {
95+
fn from(indexer: crate::keychain::ChangeSet<K>) -> Self {
96+
Self {
97+
indexed_tx_graph: crate::indexed_tx_graph::ChangeSet {
98+
indexer,
99+
..Default::default()
100+
},
101+
..Default::default()
102+
}
103+
}
104+
}
105+
106+
/// A persistence backend for writing and loading changesets.
107+
///
108+
/// `C` represents the changeset; a datatype that records changes made to in-memory data structures
109+
/// that are to be persisted, or retrieved from persistence.
110+
pub trait PersistBackend<C> {
111+
/// The error the backend returns when it fails to write.
112+
type WriteError: Debug + Display;
113+
114+
/// The error the backend returns when it fails to load changesets `C`.
115+
type LoadError: Debug + Display;
116+
117+
/// Writes a changeset to the persistence backend.
118+
///
119+
/// It is up to the backend what it does with this. It could store every changeset in a list or
120+
/// it inserts the actual changes into a more structured database. All it needs to guarantee is
121+
/// that [`load_from_persistence`] restores a keychain tracker to what it should be if all
122+
/// changesets had been applied sequentially.
123+
///
124+
/// [`load_from_persistence`]: Self::load_changes
125+
fn write_changes(&mut self, changeset: &C) -> Result<(), Self::WriteError>;
126+
127+
/// Return the aggregate changeset `C` from persistence.
128+
fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError>;
129+
}
130+
131+
impl<C> PersistBackend<C> for () {
132+
type WriteError = Infallible;
133+
type LoadError = Infallible;
134+
135+
fn write_changes(&mut self, _changeset: &C) -> Result<(), Self::WriteError> {
136+
Ok(())
137+
}
138+
139+
fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError> {
140+
Ok(None)
141+
}
142+
}
143+
144+
#[cfg(feature = "async")]
145+
/// An async persistence backend for writing and loading changesets.
146+
///
147+
/// `C` represents the changeset; a datatype that records changes made to in-memory data structures
148+
/// that are to be persisted, or retrieved from persistence.
149+
#[async_trait]
150+
pub trait PersistBackendAsync<C> {
151+
/// The error the backend returns when it fails to write.
152+
type WriteError: Debug + Display;
153+
154+
/// The error the backend returns when it fails to load changesets `C`.
155+
type LoadError: Debug + Display;
156+
157+
/// Writes a changeset to the persistence backend.
158+
///
159+
/// It is up to the backend what it does with this. It could store every changeset in a list or
160+
/// it inserts the actual changes into a more structured database. All it needs to guarantee is
161+
/// that [`load_from_persistence`] restores a keychain tracker to what it should be if all
162+
/// changesets had been applied sequentially.
163+
///
164+
/// [`load_from_persistence`]: Self::load_changes
165+
async fn write_changes(&mut self, changeset: &C) -> Result<(), Self::WriteError>;
166+
167+
/// Return the aggregate changeset `C` from persistence.
168+
async fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError>;
169+
}
170+
171+
#[cfg(feature = "async")]
172+
#[async_trait]
173+
impl<C> PersistBackendAsync<C> for () {
174+
type WriteError = Infallible;
175+
type LoadError = Infallible;
176+
177+
async fn write_changes(&mut self, _changeset: &C) -> Result<(), Self::WriteError> {
178+
Ok(())
179+
}
180+
181+
async fn load_changes(&mut self) -> Result<Option<C>, Self::LoadError> {
182+
Ok(None)
183+
}
184+
}
185+
186+
/// Extends a changeset so that it acts as a convenient staging area for any [`PersistBackend`].
187+
///
188+
/// Not all changes to the in-memory representation needs to be written to disk right away.
189+
/// [`Append::append`] can be used to *stage* changes first and then [`StageExt::commit_to`] can be
190+
/// used to write changes to disk.
191+
pub trait StageExt: Append + Default + Sized {
192+
/// Commit the staged changes to the persistence `backend`.
193+
///
194+
/// Changes that are committed (if any) are returned.
195+
///
196+
/// # Error
197+
///
198+
/// Returns a backend-defined error if this fails.
199+
fn commit_to<B>(&mut self, backend: &mut B) -> Result<Option<Self>, B::WriteError>
200+
where
201+
B: PersistBackend<Self>,
202+
{
203+
// do not do anything if changeset is empty
204+
if self.is_empty() {
205+
return Ok(None);
206+
}
207+
backend.write_changes(&*self)?;
208+
// only clear if changes are written successfully to backend
209+
Ok(Some(core::mem::take(self)))
210+
}
211+
212+
/// Stages a new `changeset` and commits it (alongside any other previously staged changes) to
213+
/// the persistence `backend`.
214+
///
215+
/// Convenience method for calling [`Append::append`] and then [`StageExt::commit_to`].
216+
fn append_and_commit_to<B>(
217+
&mut self,
218+
changeset: Self,
219+
backend: &mut B,
220+
) -> Result<Option<Self>, B::WriteError>
221+
where
222+
B: PersistBackend<Self>,
223+
{
224+
Append::append(self, changeset);
225+
self.commit_to(backend)
226+
}
227+
}
228+
229+
impl<C: Append + Default> StageExt for C {}
230+
231+
/// Extends a changeset so that it acts as a convenient staging area for any
232+
/// [`PersistBackendAsync`].
233+
///
234+
/// Not all changes to the in-memory representation needs to be written to disk right away.
235+
/// [`Append::append`] can be used to *stage* changes first and then [`StageExtAsync::commit_to`]
236+
/// can be used to write changes to disk.
237+
#[cfg(feature = "async")]
238+
#[async_trait]
239+
pub trait StageExtAsync: Append + Default + Sized + Send + Sync {
240+
/// Commit the staged changes to the persistence `backend`.
241+
///
242+
/// Changes that are committed (if any) are returned.
243+
///
244+
/// # Error
245+
///
246+
/// Returns a backend-defined error if this fails.
247+
async fn commit_to<B>(&mut self, backend: &mut B) -> Result<Option<Self>, B::WriteError>
248+
where
249+
B: PersistBackendAsync<Self> + Send + Sync,
250+
{
251+
// do not do anything if changeset is empty
252+
if self.is_empty() {
253+
return Ok(None);
254+
}
255+
backend.write_changes(&*self).await?;
256+
// only clear if changes are written successfully to backend
257+
Ok(Some(core::mem::take(self)))
258+
}
259+
260+
/// Stages a new `changeset` and commits it (alongside any other previously staged changes) to
261+
/// the persistence `backend`.
262+
///
263+
/// Convenience method for calling [`Append::append`] and then [`StageExtAsync::commit_to`].
264+
async fn append_and_commit_to<B>(
265+
&mut self,
266+
changeset: Self,
267+
backend: &mut B,
268+
) -> Result<Option<Self>, B::WriteError>
269+
where
270+
B: PersistBackendAsync<Self> + Send + Sync,
271+
{
272+
Append::append(self, changeset);
273+
self.commit_to(backend).await
274+
}
275+
}
276+
277+
#[cfg(feature = "async")]
278+
#[async_trait]
279+
impl<C: Append + Default + Send + Sync> StageExtAsync for C {}

crates/chain/src/spk_client.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ use crate::{
44
collections::BTreeMap, keychain::Indexed, local_chain::CheckPoint,
55
ConfirmationTimeHeightAnchor, TxGraph,
66
};
7-
use alloc::{boxed::Box, vec::Vec};
7+
use alloc::boxed::Box;
88
use bitcoin::{OutPoint, Script, ScriptBuf, Txid};
9-
use core::{fmt::Debug, marker::PhantomData, ops::RangeBounds};
9+
use core::marker::PhantomData;
1010

1111
/// Data required to perform a spk-based blockchain client sync.
1212
///
@@ -158,12 +158,13 @@ impl SyncRequest {
158158
/// This consumes the [`SyncRequest`] and returns the updated one.
159159
#[cfg(feature = "miniscript")]
160160
#[must_use]
161-
pub fn populate_with_revealed_spks<K: Clone + Ord + Debug + Send + Sync>(
161+
pub fn populate_with_revealed_spks<K: Clone + Ord + core::fmt::Debug + Send + Sync>(
162162
self,
163163
index: &crate::keychain::KeychainTxOutIndex<K>,
164-
spk_range: impl RangeBounds<K>,
164+
spk_range: impl core::ops::RangeBounds<K>,
165165
) -> Self {
166166
use alloc::borrow::ToOwned;
167+
use alloc::vec::Vec;
167168
self.chain_spks(
168169
index
169170
.revealed_spks(spk_range)
@@ -223,7 +224,7 @@ impl<K: Ord + Clone> FullScanRequest<K> {
223224
index: &crate::keychain::KeychainTxOutIndex<K>,
224225
) -> Self
225226
where
226-
K: Debug,
227+
K: core::fmt::Debug,
227228
{
228229
let mut req = Self::from_chain_tip(chain_tip);
229230
for (keychain, spks) in index.all_unbounded_spk_iters() {

crates/file_store/Cargo.toml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@ edition = "2021"
55
license = "MIT OR Apache-2.0"
66
repository = "https://github.com/bitcoindevkit/bdk"
77
documentation = "https://docs.rs/bdk_file_store"
8-
description = "A simple append-only flat file implementation of Persist for Bitcoin Dev Kit."
8+
description = "A simple append-only flat file implementation of PersistBackend for Bitcoin Dev Kit."
99
keywords = ["bitcoin", "persist", "persistence", "bdk", "file"]
1010
authors = ["Bitcoin Dev Kit Developers"]
1111
readme = "README.md"
1212

1313
[dependencies]
14-
anyhow = { version = "1", default-features = false }
1514
bdk_chain = { path = "../chain", version = "0.15.0", features = [ "serde", "miniscript" ] }
16-
bdk_persist = { path = "../persist", version = "0.3.0"}
1715
bincode = { version = "1" }
1816
serde = { version = "1", features = ["derive"] }
1917

crates/file_store/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# BDK File Store
22

3-
This is a simple append-only flat file implementation of [`PersistBackend`](bdk_persist::PersistBackend).
3+
This is a simple append-only flat file implementation of [`PersistBackend`](bdk_chain::persist::PersistBackend).
44

55
The main structure is [`Store`] which works with any [`bdk_chain`] based changesets to persist data into a flat file.
66

0 commit comments

Comments
 (0)