Skip to content

Commit

Permalink
wip: edit persisted.md
Browse files Browse the repository at this point in the history
  • Loading branch information
ValuedMammal committed Sep 10, 2024
1 parent 96997d2 commit 219982e
Showing 1 changed file with 33 additions and 49 deletions.
82 changes: 33 additions & 49 deletions docs/adr/0002_persisted.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,66 +4,50 @@
* Deciders: [list everyone involved in the decision]
* Date: 2024-08-19

Technical Story: PR [#1547](https://github.com/bitcoindevkit/bdk/pull/1547), [ADR-0001](./0001_persist.md)
Technical Story:

* [ADR-0001](./0001_persist.md)
* [#1514 - refactor(wallet)!: rework persistence, changeset, and construction](https://github.com/bitcoindevkit/bdk/pull/1514)
* [#1547 - Simplify wallet persistence](https://github.com/bitcoindevkit/bdk/pull/1547)

## Context and Problem

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` that the user will call and everything will "just work."
BDK v1.0.0-beta.1 introduced new persistence traits `PersistWith<Db>` and `PersistAsyncWith<Db>`. The design was [slightly overwrought][0], the latter proving difficult to implement by wallet users.

## Decision Drivers

* [driver 1, e.g., a force, facing concern, …]
* [driver 2, e.g., a force, facing concern, …]
*<!-- numbers of drivers can vary -->

## Considered Options

* [option 1]
* [option 2]
* [option 3]
*<!-- numbers of options can vary -->

## Decision Outcome

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`.

### 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, …]
*

## Pros and Cons of the Options <!-- optional -->

### [option 1]

Open questions

* How does a user know when and how often to call `persist`? (Short answer: whenever we mutate the wallet)
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."

### [option 2]
## Chosen option

[example | description | pointer to more information | …] <!-- optional -->
Introduce a new type `PersistedWallet` that wraps a BDK `Wallet` and provides a convenient interface for executing persistence operations aided by a `WalletPersister`.

* Good, because [argument a]
* Good, because [argument b]
* Bad, because [argument c]
*<!-- numbers of pros and cons can vary -->
`PersistedWallet` internally calls the 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.

### [option 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`][2] 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 {
...

[example | description | pointer to more information | …] <!-- optional -->
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)
}
}
```

* Good, because [argument a]
* Good, because [argument b]
* Bad, because [argument c]
*<!-- numbers of pros and cons can vary -->
## Pros and Cons

## Links <!-- optional -->
TODO

* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
*<!-- numbers of links can vary -->
<!-- ## Links -->
[0]: (https://github.com/bitcoindevkit/bdk/pull/1514#issuecomment-2241715165)
[1]: (https://github.com/bitcoindevkit/bdk/blob/8760653339d3a4c66dfa9a54a7b9d943a065f924/crates/wallet/src/wallet/persisted.rs#L52)
[2]: (https://github.com/bitcoindevkit/bdk/blob/8760653339d3a4c66dfa9a54a7b9d943a065f924/crates/wallet/src/wallet/persisted.rs#L55)

0 comments on commit 219982e

Please sign in to comment.