Merge bitcoindevkit#1547: Simplify wallet persistence

340808e docs(wallet): fixes/improvements for `persisted` and `params` types (志宇)
9600293 feat(wallet)!: add persister (`P`) type param to `PersistedWallet<P>` (志宇)
0616057 revert(chain)!: rm `persit` module (志宇)
a9c5f76 feat(wallet)!: remove dependency on `bdk_chain::Staged` (志宇)
06a9d6c feat(chain,wallet)!: publicize `.init_sqlite_tables` changeset methods (志宇)
039622f feat(wallet)!: introduce `WalletPersister` (志宇)

Pull request description:

  ### Description

  Removed the persistence module in `bdk_chain` (which contained the `PersistWith` and `PersistAsyncWith` traits and a `Persisted<T>` wrapper). Replaced it with simplified versions that are `Wallet`-specific.

  The new traits (`WalletPersister` and `AsyncWalletPersister`) are simpler since they have less type-parameters (being wallet-specific). The old versions were more complex as they were intended to be used with any type. However, we need more time to finalize a works-for-all-bdk-types persistence API.

  Additionally, `WalletPersister` and `AsyncWalletPersister` also introduce the `initialize` method. It is handy to contain db-initialization (i.e. create tables for SQL) here. We can call `initialize` in `PersistedWallet` constructors. So the `PersistedWallet::persist` method does not need to ensure the database is initialized. To accommodate this, I made the `.init_sqlite_table` methods on changeset-types public, and removed the call of this in `from_sqlite` and `.persist_to_sqlite`. The `initialize` method now loads from the persister, so the `load` associated function is removed.

  There was a bug in the old `PersistAsyncWith` trait where the lifetime bounds were not strict enough (refer to the conversation in bitcoindevkit#1552). This is now fixed in the new `AsyncWalletPersister` trait.

  Docs for the new types are clearer (hopefully). I mentioned implementation details about the new traits and what safety checks were guaranteed in `PersistedWallet`.

  I think it makes sense just to have a wallet-specific persistence API which we will maintain alongside wallet for v1.0. This is less baggage for users and maintainers alike.

  ### Notes to the reviewers

  **How breaking are these changes?**

  Unless if you were implementing your own persistence, then not breaking at all! If you were implementing your own persistence for BDK, the breakages are minimal. The main change is introducing the `initialize` method to persistence traits (which replaces `load`).

  **Acknowledgements**

  I came up with the idea of this while responding to @tnull's suggestions on Discord. Unfortunately we cannot fulfill all of his requests. However, I do believe that this will make things easier for our users.

  ### Changelog notice

  * Removed the `persist` module in `bdk_chain`.
  * Added `WalletPersister`/`AsyncWalletPersister` traits and `PersistedWallet` struct to `bdk_wallet`. These are simplified and safer versions of old structs provided by the `persist` module in `bdk_chain`.
  * Change `.init_sqlite_tables` method to be public on changeset types. `from_sqlite` and `persist_into_sqlite` methods no longer call `.init_sqlite_tables` internally.

  ### 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

  #### What is left to do:

  - [x] Better docs.

ACKs for top commit:
  notmandatory:
    ACK 340808e

Tree-SHA512: 77643196543ad0a741f9ec51de4aad3893c6d608234ad6709c8688043bf1eef1e5e97e71681be0a6b29dec3fa255e2f0e63e08511c79ea4c0a6ef3286653d40c

crates/chain/src/lib.rs

@@ -37,8 +37,6 @@ pub use tx_data_traits::*;
 pub use tx_graph::TxGraph;
 mod chain_oracle;
 pub use chain_oracle::*;
-mod persist;
-pub use persist::*;
 
 #[doc(hidden)]
 pub mod example_utils;

crates/chain/src/rusqlite_impl.rs

@@ -225,7 +225,7 @@ where
     pub const ANCHORS_TABLE_NAME: &'static str = "bdk_anchors";
 
     /// Initialize sqlite tables.
-    fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
+    pub fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
         let schema_v0: &[&str] = &[
             // full transactions
             &format!(
@@ -264,9 +264,9 @@ where
     }
 
     /// Construct a [`TxGraph`] from an sqlite database.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn from_sqlite(db_tx: &rusqlite::Transaction) -> rusqlite::Result<Self> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut changeset = Self::default();
 
         let mut statement = db_tx.prepare(&format!(
@@ -332,9 +332,9 @@ where
     }
 
     /// Persist `changeset` to the sqlite database.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn persist_to_sqlite(&self, db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut statement = db_tx.prepare_cached(&format!(
             "INSERT INTO {}(txid, raw_tx) VALUES(:txid, :raw_tx) ON CONFLICT(txid) DO UPDATE SET raw_tx=:raw_tx",
             Self::TXS_TABLE_NAME,
@@ -396,7 +396,7 @@ impl local_chain::ChangeSet {
     pub const BLOCKS_TABLE_NAME: &'static str = "bdk_blocks";
 
     /// Initialize sqlite tables for persisting [`local_chain::LocalChain`].
-    fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
+    pub fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
         let schema_v0: &[&str] = &[
             // blocks
             &format!(
@@ -411,9 +411,9 @@ impl local_chain::ChangeSet {
     }
 
     /// Construct a [`LocalChain`](local_chain::LocalChain) from sqlite database.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn from_sqlite(db_tx: &rusqlite::Transaction) -> rusqlite::Result<Self> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut changeset = Self::default();
 
         let mut statement = db_tx.prepare(&format!(
@@ -435,9 +435,9 @@ impl local_chain::ChangeSet {
     }
 
     /// Persist `changeset` to the sqlite database.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn persist_to_sqlite(&self, db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut replace_statement = db_tx.prepare_cached(&format!(
             "REPLACE INTO {}(block_height, block_hash) VALUES(:block_height, :block_hash)",
             Self::BLOCKS_TABLE_NAME,
@@ -471,7 +471,7 @@ impl keychain_txout::ChangeSet {
 
     /// Initialize sqlite tables for persisting
     /// [`KeychainTxOutIndex`](keychain_txout::KeychainTxOutIndex).
-    fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
+    pub fn init_sqlite_tables(db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
         let schema_v0: &[&str] = &[
             // last revealed
             &format!(
@@ -487,9 +487,9 @@ impl keychain_txout::ChangeSet {
 
     /// Construct [`KeychainTxOutIndex`](keychain_txout::KeychainTxOutIndex) from sqlite database
     /// and given parameters.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn from_sqlite(db_tx: &rusqlite::Transaction) -> rusqlite::Result<Self> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut changeset = Self::default();
 
         let mut statement = db_tx.prepare(&format!(
@@ -511,9 +511,9 @@ impl keychain_txout::ChangeSet {
     }
 
     /// Persist `changeset` to the sqlite database.
+    ///
+    /// Remember to call [`Self::init_sqlite_tables`] beforehand.
     pub fn persist_to_sqlite(&self, db_tx: &rusqlite::Transaction) -> rusqlite::Result<()> {
-        Self::init_sqlite_tables(db_tx)?;
-
         let mut statement = db_tx.prepare_cached(&format!(
             "REPLACE INTO {}(descriptor_id, last_revealed) VALUES(:descriptor_id, :last_revealed)",
             Self::LAST_REVEALED_TABLE_NAME,

crates/wallet/src/wallet/changeset.rs

@@ -72,10 +72,8 @@ impl ChangeSet {
     /// Name of table to store wallet descriptors and network.
     pub const WALLET_TABLE_NAME: &'static str = "bdk_wallet";
 
-    /// Initialize sqlite tables for wallet schema & table.
-    fn init_wallet_sqlite_tables(
-        db_tx: &chain::rusqlite::Transaction,
-    ) -> chain::rusqlite::Result<()> {
+    /// Initialize sqlite tables for wallet tables.
+    pub fn init_sqlite_tables(db_tx: &chain::rusqlite::Transaction) -> chain::rusqlite::Result<()> {
         let schema_v0: &[&str] = &[&format!(
             "CREATE TABLE {} ( \
                 id INTEGER PRIMARY KEY NOT NULL CHECK (id = 0), \
@@ -85,12 +83,17 @@ impl ChangeSet {
                 ) STRICT;",
             Self::WALLET_TABLE_NAME,
         )];
-        crate::rusqlite_impl::migrate_schema(db_tx, Self::WALLET_SCHEMA_NAME, &[schema_v0])
+        crate::rusqlite_impl::migrate_schema(db_tx, Self::WALLET_SCHEMA_NAME, &[schema_v0])?;
+
+        bdk_chain::local_chain::ChangeSet::init_sqlite_tables(db_tx)?;
+        bdk_chain::tx_graph::ChangeSet::<ConfirmationBlockTime>::init_sqlite_tables(db_tx)?;
+        bdk_chain::keychain_txout::ChangeSet::init_sqlite_tables(db_tx)?;
+
+        Ok(())
     }
 
     /// Recover a [`ChangeSet`] from sqlite database.
     pub fn from_sqlite(db_tx: &chain::rusqlite::Transaction) -> chain::rusqlite::Result<Self> {
-        Self::init_wallet_sqlite_tables(db_tx)?;
         use chain::rusqlite::OptionalExtension;
         use chain::Impl;
 
@@ -129,7 +132,6 @@ impl ChangeSet {
         &self,
         db_tx: &chain::rusqlite::Transaction,
     ) -> chain::rusqlite::Result<()> {
-        Self::init_wallet_sqlite_tables(db_tx)?;
         use chain::rusqlite::named_params;
         use chain::Impl;
 

crates/wallet/src/wallet/mod.rs

@@ -45,7 +45,6 @@ use bitcoin::{
 use bitcoin::{consensus::encode::serialize, transaction, BlockHash, Psbt};
 use bitcoin::{constants::genesis_block, Amount};
 use bitcoin::{secp256k1::Secp256k1, Weight};
-use chain::Staged;
 use core::fmt;
 use core::mem;
 use core::ops::Deref;
@@ -123,14 +122,6 @@ pub struct Wallet {
     secp: SecpCtx,
 }
 
-impl Staged for Wallet {
-    type ChangeSet = ChangeSet;
-
-    fn staged(&mut self) -> &mut Self::ChangeSet {
-        &mut self.stage
-    }
-}
-
 /// An update to [`Wallet`].
 ///
 /// It updates [`KeychainTxOutIndex`], [`bdk_chain::TxGraph`] and [`local_chain::LocalChain`] atomically.
@@ -2303,7 +2294,7 @@ impl Wallet {
         Ok(())
     }
 
-    /// Get a reference of the staged [`ChangeSet`] that are yet to be committed (if any).
+    /// Get a reference of the staged [`ChangeSet`] that is yet to be committed (if any).
     pub fn staged(&self) -> Option<&ChangeSet> {
         if self.stage.is_empty() {
             None
@@ -2312,6 +2303,15 @@ impl Wallet {
         }
     }
 
+    /// Get a mutable reference of the staged [`ChangeSet`] that is yet to be commited (if any).
+    pub fn staged_mut(&mut self) -> Option<&mut ChangeSet> {
+        if self.stage.is_empty() {
+            None
+        } else {
+            Some(&mut self.stage)
+        }
+    }
+
     /// Take the staged [`ChangeSet`] to be persisted now (if any).
     pub fn take_staged(&mut self) -> Option<ChangeSet> {
         self.stage.take()