Architectural Decision Records

docs/adr/0001_persist.md

@@ -0,0 +1,49 @@
+# Remove `persist` module from `bdk_chain`
+
+* Status: accepted
+* Authors: _
+* Date: 2024-06-14
+* Targeted modules: `bdk_chain`
+* Associated tickets: PRs #1454, #1473, refined by [ADR-0002](./0002_persisted.md)
+
+## Context and Problem
+
+How to enable the `Wallet` to work with an async persist backend?
+
+A long-standing problem of the pre-1.0 BDK was that the "wallet is not async". In particular this made the timing of database reads and writes suboptimal considering the main benefit of asynchronous programming is to do async IO. Users who were accustomed to utilizing an asynchronous framework at the persistence layer thus had a difficult time working around this limitation of the wallet.
+
+A series of persistence-related PRs took place all aimed at reducing pain points that included removing generics from the `Wallet` structure (#1387) to alleviate ffi headaches, moving the persist module to its own crate to isolate a dependency problem (#1412), taking the database out of the wallet completely using a new "take-staged" approach (#1454), and finally the decision to scrap the persist module defining the `PersistBackend` trait which previously all backends were required to implement (#1473).
+
+## Motivation
+
+Allow persistence to work with any database including the ability to utilize asynchronous IO. For example, one desired use case is to use a high-performance database such as PostgreSQL in a server environment. 
+
+## Considered options
+
+#### Option 1: Introduce a (not so) new trait `PersistBackendAsync`
+
+The first approach was to create another trait `PersistBackendAsync` that was essentially equivalent to the existing `PersistBackend`, only that its methods were declared `async fn`. To accomplish this, we added the `#[async_trait]` macro which seemed acceptable although comes with the obvious downside of duplicating code. The perceived benefit of this approach was that it offered a suitable replacement for use in an async context while providing a familiar interface.
+
+#### Option 2: Return futures from persistence backend functions
+
+Another idea that was offered was to return something implementing `Future` from methods like `commit`. The idea was that it would minimize added dependencies and increase flexiblity by allowing the caller to `await` the result. In the end it seems less of an effort was put toward executing this idea.
+
+## Decision
+
+The ultimate decision was to seek maximum flexibility by decoupling persistence from both wallet and chain, making these crates totally agnostic to the persistence layer. There is no more `PersistBackend` trait - instead the wallet and its internal structures are allowed to assume that all persisted state can be recovered intact.
+
+The critical piece that allows this to work is the `CombinedChangeSet` structure residing in `bdk_chain`. This means that anything capable of (de)serializing the combined changeset has all the information necessary to save and recover a previous state.
+
+## Outcomes
+
+#### Implications for UX
+
+The result of our decision could be more of a UX burden on users in that it requires more hands-on involvement with the `CombinedChangeSet` structure, whereas before these details were kept internal and we merely exposed an ergonomic API via `stage` and `commit`, etc.
+
+On the other hand, users no longer need to implement a persistence trait that BDK understands, and the added flexibility should enable more users to integrate BDK into their systems with fewer limitations.
+
+#### Core library development
+
+Library development is for the most part similiar to before in the sense that we stage changes as they become relevant and let the user decide when to commit them. It is believed that the increased modularity and separation of concerns should lead to a significant quality of life improvement.
+
+It remains the job of the developers to adequately document and showcase a correct use of the API. It is important that we clearly define the structure of the changeset that we now expect users to be aware of and to be proactive and transparent when communicating upgrades to the data model.

docs/adr/0002_persisted.md

@@ -0,0 +1,55 @@
+# Introduce `PersistedWallet`
+
+* Status: accepted
+* Authors: _
+* Date: 2024-08-19
+* Targeted modules: `bdk_wallet`
+* Associated tickets: #1514, #1547
+
+## Context and Problem
+
+BDK v1.0.0-beta.1 introduced new persistence traits `PersistWith<Db>` and `PersistAsyncWith<Db>`. The design was slightly cumbersome, the latter proving difficult to implement by wallet users.
+
+## Decision Drivers
+
+We would like persistence operations to be both safe and ergonomic. It would cumbersome and error prone if users were expected to ask the wallet for a changeset at the correct time and persist it on their own. Considering that the wallet no longer drives its own database, an ideal interface is to have a method on `Wallet` such as [`persist`][1] that the user will call and everything will "just work."
+
+## Chosen option
+
+#### Introduce a new type `PersistedWallet` that wraps a BDK `Wallet` and provides a convenient interface for executing persistence operations aided by a `WalletPersister`.
+
+`PersistedWallet` internally calls the methods of the `WalletPersister` trait. We do this to ensure consistency of the create and load steps particularly when it comes to handling staged changes and to reduce the surface area for footguns. Currently BDK provides implementations of `WalletPersister` for a [SQLite backend using `rusqlite`][2] as well as a [flat-file store][3].
+
+The two-trait design was kept in order to accommodate both blocking and async use cases. For `AsyncWalletPersister` the definition is modified to return a [`FutureResult`][4] which is a rust-idiomatic way of creating something that can be polled by an async runtime. For example in the case of `persist` the implementer writes an `async fn` that does the persistence operation and then calls `Box::pin` on that.
+```rust
+impl AsyncWalletPersister for MyCustomDb {
+    ...
+
+    fn persist<'a>(persister: &'a mut Self, changeset: &'a ChangeSet) -> FutureResult<'a, (), Self::Error>
+    where
+        Self: 'a,
+    {
+        let persist_fn = async move |changeset| {
+            // perform write operation...
+            Ok(())
+        };
+
+        Box::pin(persist_fn)
+    }
+}
+```
+
+**Pros:**
+
+* Relatively safe, ergonomic, and generalized to accommodate different storage implementations.
+
+**Cons:**
+
+* Requires manual intervention, i.e., how does a user know when and how often to call `persist`, whether it can be deferred until later, and what to do in case of persistence failure? As a first approximation we consider any operation that mutates the internal state of the wallet to be worthy of persisting. Others have suggested implementing more [fine-grained notifications][5] that are meant to trigger persistence.
+
+<!-- ## Links -->
+[1]: https://github.com/bitcoindevkit/bdk/blob/8760653339d3a4c66dfa9a54a7b9d943a065f924/crates/wallet/src/wallet/persisted.rs#L52
+[2]: https://github.com/bitcoindevkit/bdk/blob/88423f3a327648c6e44edd7deb15c9c92274118a/crates/wallet/src/wallet/persisted.rs#L257-L287
+[3]: https://github.com/bitcoindevkit/bdk/blob/8760653339d3a4c66dfa9a54a7b9d943a065f924/crates/wallet/src/wallet/persisted.rs#L314
+[4]: https://github.com/bitcoindevkit/bdk/blob/8760653339d3a4c66dfa9a54a7b9d943a065f924/crates/wallet/src/wallet/persisted.rs#L55
+[5]: https://github.com/bitcoindevkit/bdk/issues/1542#issuecomment-2276066581

docs/adr/template.md

@@ -0,0 +1,56 @@
+# [short title of solved problem and solution]
+
+* Status: [proposed | rejected | accepted | deprecated | … | superseded by ADR-1234]
+* Authors: [list everyone who authored the decision]
+* Date: [YYYY-MM-DD when the decision was last updated]
+* Targeted modules: [which crate or module does this change target]
+* Associated tickets/PRs: [PR/issue links]
+
+## Context and Problem Statement
+
+[Describe the context and problem statement, e.g., in free form using two to three sentences. You may want to articulate the problem in form of a question.]
+
+## Decision Drivers <!-- optional -->
+
+* [driver 1, e.g., a force, facing concern, …]
+* [driver 2, e.g., a force, facing concern, …]
+* … <!-- numbers of drivers can vary -->
+
+## Considered Options <!-- numbers of options can vary -->
+
+#### [Option 1]
+
+[example | description | pointer to more information | …]
+
+**Pros:**
+
+* Good, because [argument …]
+
+**Cons:**
+
+* Bad, because [argument …]
+
+#### [Option 2]
+...
+
+#### [Option 3]
+...
+
+## Decision Outcome
+
+Chosen option: "[option 1]", because [justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force force | … | comes out best (see below)].
+
+### Positive Consequences <!-- optional -->
+
+* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …]
+* …
+
+### Negative Consequences <!-- optional -->
+
+* [e.g., compromising quality attribute, follow-up decisions required, …]
+* …
+
+## Links <!-- optional -->
+
+* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
+* … <!-- numbers of links can vary -->