From 219982ea4736da9e1325ea6e684f2494b017d203 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 10 Sep 2024 12:44:32 -0400 Subject: [PATCH] wip: edit persisted.md --- docs/adr/0002_persisted.md | 82 +++++++++++++++----------------------- 1 file changed, 33 insertions(+), 49 deletions(-) diff --git a/docs/adr/0002_persisted.md b/docs/adr/0002_persisted.md index 27589a644..1b6f7287f 100644 --- a/docs/adr/0002_persisted.md +++ b/docs/adr/0002_persisted.md @@ -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` and `PersistAsyncWith`. 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, …] -* … - -## Considered Options - -* [option 1] -* [option 2] -* [option 3] -* … - -## 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 - -* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …] -* … - -### Negative Consequences - -* [e.g., compromising quality attribute, follow-up decisions required, …] -* … - -## Pros and Cons of the Options - -### [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 | …] +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] -* … +`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 | …] + 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] -* … +## Pros and Cons -## Links +TODO -* [Link type] [Link to ADR] -* … + +[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)