Merge bitcoindevkit#1514: refactor(wallet)!: rework persistence, chan…

…geset, and construction

64eb576 chore(wallet): Fix ChangeSet::merge (LLFourn)
8875c92 chore(wallet): Fix descriptor mismatch error keychain (LLFourn)
2cf07d6 refactor(chain,wallet)!: move rusqlite things into it's own file (志宇)
93f9b83 chore(chain): rm unused `sqlite` types (志宇)
892b97d feat(chain,wallet)!: Change persist-traits to be "safer" (志宇)
3aed4cf test(wallet): ensure checks work when loading wallet (志宇)
af4ee0f refactor(wallet)!: Make `bdk_wallet::ChangeSet` non-exhaustive (志宇)
22d02ed feat!: improve wallet building methods (志宇)
eb73f06 refactor!: move `WalletChangeSet` to `bdk_wallet` and fix import paths (志宇)
6b43001 feat!: Rework sqlite, changesets, persistence and wallet-construction (志宇)

Pull request description:

  Closes bitcoindevkit#1496
  Closes bitcoindevkit#1498
  Closes bitcoindevkit#1500

  ### Description

  Rework sqlite: Instead of only supported one schema (defined in `bdk_sqlite`), we have a schema per changeset type for more flexiblity.

  * rm `bdk_sqlite` crate (as we don't need `bdk_sqlite::Store` anymore).
  * add `sqlite` feature on `bdk_chain` which adds methods on each changeset type for initializing tables, loading the changeset and writing.

  Rework changesets: Some callers may want to use `KeychainTxOutIndex` where `K` may change per descriptor on every run. So we only want to persist the last revealed indices by `DescriptorId` (which uniquely-ish identifies the descriptor).

  * rm `keychain_added` field from `keychain_txout`'s changeset.
  * Add `keychain_added` to `CombinedChangeSet` (which is renamed to `WalletChangeSet`).

  Rework persistence: add back some safety and convenience when persisting our types. Working with changeset directly (as we were doing before) can be cumbersome.

  * Intoduce `struct Persisted<T>` which wraps a type `T` which stores staged changes to it. This adds safety when creating and or loading `T` from db.
  * `struct Persisted<T>` methods, `create`, `load` and `persist`, are available if `trait PersistWith<Db>` is implemented for `T`. `Db` represents the database connection and `PersistWith` should be implemented per database-type.
  * For async, we have `trait PersistedAsyncWith<Db>`.
  * `Wallet` has impls of `PersistedWith<rusqlite::Connection>`, `PersistedWith<rusqlite::Transaction>` and `PersistedWith<bdk_file_store::Store>` by default.

  Rework wallet-construction: Before, we had multiple methods for loading and creating with different input-counts so it would be unwieldly to add more parameters in the future. This also makes it difficult to impl `PersistWith` (which has a single method for `load` that takes in `PersistWith::LoadParams` and a single method for `create` that takes in `PersistWith::CreateParams`).

  * Introduce a builder pattern when constructing a `Wallet`. For loading from persistence or `ChangeSet`, we have `LoadParams`. For creating a new wallet, we have `CreateParams`.

  ### Notes to the reviewers

  TODO

  ### Changelog notice

  ```
  ### Added

  - Add `sqlite` feature to `bdk_chain` which introduces methods on changeset types that encode/decode changesets to SQLite database.
  * Introduce `PersistWith<Db>` and `PersistAsyncWith<Db>` traits and a `Persisted` wrapper. This ergonomically makes sure user inits the db before reading/writing to it.

  ### Changed

  - Moved `bdk_chain::CombinedChangeSet` to `bdk_wallet::ChangeSet` and added `keychain_added` field.
  - `bdk_wallet::Wallet` construction now uses a builder API using the newly introduced `CreateParams` and `LoadParams`.

  ### Removed

  - Remove `keychains_added` field from `bdk_chain::keychain_txout::ChangeSet`.

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

  #### New Features:

  * [ ] I've added tests for the new feature
  * [x] I've added docs for the new feature

ACKs for top commit:
  LLFourn:
    ACK: 64eb576
  notmandatory:
    Re ACK 64eb576

Tree-SHA512: b8a1d48aea26d9fa293a8387a3533cd16c8ddae890f94d61fb91efa492fb05ac5e0a66200d64d7c857774368d5f0f8991a98684307029c25f50a1d8fceee8e67

Cargo.toml

@@ -4,7 +4,6 @@ members = [
     "crates/wallet",
     "crates/chain",
     "crates/file_store",
-    "crates/sqlite",
     "crates/electrum",
     "crates/esplora",
     "crates/bitcoind_rpc",

crates/bitcoind_rpc/tests/test_emitter.rs

@@ -4,7 +4,8 @@ use bdk_bitcoind_rpc::Emitter;
 use bdk_chain::{
     bitcoin::{Address, Amount, Txid},
     local_chain::{CheckPoint, LocalChain},
-    Balance, BlockId, IndexedTxGraph, Merge, SpkTxOutIndex,
+    spk_txout::SpkTxOutIndex,
+    Balance, BlockId, IndexedTxGraph, Merge,
 };
 use bdk_testenv::{anyhow, TestEnv};
 use bitcoin::{hashes::Hash, Block, OutPoint, ScriptBuf, WScriptHash};
@@ -47,7 +48,7 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
 
         assert_eq!(
             local_chain.apply_update(emission.checkpoint,)?,
-            BTreeMap::from([(height, Some(hash))]),
+            [(height, Some(hash))].into(),
             "chain update changeset is unexpected",
         );
     }
@@ -93,11 +94,13 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
         assert_eq!(
             local_chain.apply_update(emission.checkpoint,)?,
             if exp_height == exp_hashes.len() - reorged_blocks.len() {
-                core::iter::once((height, Some(hash)))
-                    .chain((height + 1..exp_hashes.len() as u32).map(|h| (h, None)))
-                    .collect::<bdk_chain::local_chain::ChangeSet>()
+                bdk_chain::local_chain::ChangeSet {
+                    blocks: core::iter::once((height, Some(hash)))
+                        .chain((height + 1..exp_hashes.len() as u32).map(|h| (h, None)))
+                        .collect(),
+                }
             } else {
-                BTreeMap::from([(height, Some(hash))])
+                [(height, Some(hash))].into()
             },
             "chain update changeset is unexpected",
         );
@@ -193,15 +196,15 @@ fn test_into_tx_graph() -> anyhow::Result<()> {
         let indexed_additions = indexed_tx_graph.batch_insert_unconfirmed(mempool_txs);
         assert_eq!(
             indexed_additions
-                .graph
+                .tx_graph
                 .txs
                 .iter()
                 .map(|tx| tx.compute_txid())
                 .collect::<BTreeSet<Txid>>(),
             exp_txids,
             "changeset should have the 3 mempool transactions",
         );
-        assert!(indexed_additions.graph.anchors.is_empty());
+        assert!(indexed_additions.tx_graph.anchors.is_empty());
     }
 
     // mine a block that confirms the 3 txs
@@ -224,9 +227,9 @@ fn test_into_tx_graph() -> anyhow::Result<()> {
         let height = emission.block_height();
         let _ = chain.apply_update(emission.checkpoint)?;
         let indexed_additions = indexed_tx_graph.apply_block_relevant(&emission.block, height);
-        assert!(indexed_additions.graph.txs.is_empty());
-        assert!(indexed_additions.graph.txouts.is_empty());
-        assert_eq!(indexed_additions.graph.anchors, exp_anchors);
+        assert!(indexed_additions.tx_graph.txs.is_empty());
+        assert!(indexed_additions.tx_graph.txouts.is_empty());
+        assert_eq!(indexed_additions.tx_graph.anchors, exp_anchors);
     }
 
     Ok(())

crates/chain/Cargo.toml

@@ -20,6 +20,10 @@ serde_crate = { package = "serde", version = "1", optional = true, features = ["
 hashbrown = { version = "0.9.1", optional = true, features = ["serde"] }
 miniscript = { version = "12.0.0", optional = true, default-features = false }
 
+# Feature dependencies
+rusqlite_crate = { package = "rusqlite", version = "0.31.0", features = ["bundled"], optional = true }
+serde_json = {version = "1", optional = true }
+
 [dev-dependencies]
 rand = "0.8"
 proptest = "1.2.0"
@@ -28,3 +32,4 @@ proptest = "1.2.0"
 default = ["std", "miniscript"]
 std = ["bitcoin/std", "miniscript?/std"]
 serde = ["serde_crate", "bitcoin/serde", "miniscript?/serde"]
+rusqlite = ["std", "rusqlite_crate", "serde", "serde_json"]

crates/chain/src/indexed_tx_graph.rs

@@ -1,5 +1,7 @@
 //! Contains the [`IndexedTxGraph`] and associated types. Refer to the
 //! [`IndexedTxGraph`] documentation for more.
+use core::fmt::Debug;
+
 use alloc::vec::Vec;
 use bitcoin::{Block, OutPoint, Transaction, TxOut, Txid};
 
@@ -47,21 +49,24 @@ impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I> {
     pub fn apply_changeset(&mut self, changeset: ChangeSet<A, I::ChangeSet>) {
         self.index.apply_changeset(changeset.indexer);
 
-        for tx in &changeset.graph.txs {
+        for tx in &changeset.tx_graph.txs {
             self.index.index_tx(tx);
         }
-        for (&outpoint, txout) in &changeset.graph.txouts {
+        for (&outpoint, txout) in &changeset.tx_graph.txouts {
             self.index.index_txout(outpoint, txout);
         }
 
-        self.graph.apply_changeset(changeset.graph);
+        self.graph.apply_changeset(changeset.tx_graph);
     }
 
     /// Determines the [`ChangeSet`] between `self` and an empty [`IndexedTxGraph`].
     pub fn initial_changeset(&self) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.initial_changeset();
         let indexer = self.index.initial_changeset();
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 }
 
@@ -89,21 +94,30 @@ where
     pub fn apply_update(&mut self, update: TxGraph<A>) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.apply_update(update);
         let indexer = self.index_tx_graph_changeset(&graph);
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 
     /// Insert a floating `txout` of given `outpoint`.
     pub fn insert_txout(&mut self, outpoint: OutPoint, txout: TxOut) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.insert_txout(outpoint, txout);
         let indexer = self.index_tx_graph_changeset(&graph);
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 
     /// Insert and index a transaction into the graph.
     pub fn insert_tx(&mut self, tx: Transaction) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.insert_tx(tx);
         let indexer = self.index_tx_graph_changeset(&graph);
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 
     /// Insert an `anchor` for a given transaction.
@@ -151,7 +165,10 @@ where
             }
         }
 
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 
     /// Batch insert unconfirmed transactions, filtering out those that are irrelevant.
@@ -185,7 +202,10 @@ where
                 .map(|(tx, seen_at)| (tx.clone(), seen_at)),
         );
 
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 
     /// Batch insert unconfirmed transactions.
@@ -203,7 +223,10 @@ where
     ) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.batch_insert_unconfirmed(txs);
         let indexer = self.index_tx_graph_changeset(&graph);
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
     }
 }
 
@@ -236,9 +259,9 @@ where
             if self.index.is_tx_relevant(tx) {
                 let txid = tx.compute_txid();
                 let anchor = A::from_block_position(block, block_id, tx_pos);
-                changeset.graph.merge(self.graph.insert_tx(tx.clone()));
+                changeset.tx_graph.merge(self.graph.insert_tx(tx.clone()));
                 changeset
-                    .graph
+                    .tx_graph
                     .merge(self.graph.insert_anchor(txid, anchor));
             }
         }
@@ -265,7 +288,16 @@ where
             graph.merge(self.graph.insert_tx(tx.clone()));
         }
         let indexer = self.index_tx_graph_changeset(&graph);
-        ChangeSet { graph, indexer }
+        ChangeSet {
+            tx_graph: graph,
+            indexer,
+        }
+    }
+}
+
+impl<A, I> AsRef<TxGraph<A>> for IndexedTxGraph<A, I> {
+    fn as_ref(&self) -> &TxGraph<A> {
+        &self.graph
     }
 }
 
@@ -285,54 +317,46 @@ where
 #[must_use]
 pub struct ChangeSet<A, IA> {
     /// [`TxGraph`] changeset.
-    pub graph: tx_graph::ChangeSet<A>,
+    pub tx_graph: tx_graph::ChangeSet<A>,
     /// [`Indexer`] changeset.
     pub indexer: IA,
 }
 
 impl<A, IA: Default> Default for ChangeSet<A, IA> {
     fn default() -> Self {
         Self {
-            graph: Default::default(),
+            tx_graph: Default::default(),
             indexer: Default::default(),
         }
     }
 }
 
 impl<A: Anchor, IA: Merge> Merge for ChangeSet<A, IA> {
     fn merge(&mut self, other: Self) {
-        self.graph.merge(other.graph);
+        self.tx_graph.merge(other.tx_graph);
         self.indexer.merge(other.indexer);
     }
 
     fn is_empty(&self) -> bool {
-        self.graph.is_empty() && self.indexer.is_empty()
+        self.tx_graph.is_empty() && self.indexer.is_empty()
     }
 }
 
 impl<A, IA: Default> From<tx_graph::ChangeSet<A>> for ChangeSet<A, IA> {
     fn from(graph: tx_graph::ChangeSet<A>) -> Self {
         Self {
-            graph,
+            tx_graph: graph,
             ..Default::default()
         }
     }
 }
 
 #[cfg(feature = "miniscript")]
-impl<A, K> From<crate::indexer::keychain_txout::ChangeSet<K>>
-    for ChangeSet<A, crate::indexer::keychain_txout::ChangeSet<K>>
-{
-    fn from(indexer: crate::indexer::keychain_txout::ChangeSet<K>) -> Self {
+impl<A> From<crate::keychain_txout::ChangeSet> for ChangeSet<A, crate::keychain_txout::ChangeSet> {
+    fn from(indexer: crate::keychain_txout::ChangeSet) -> Self {
         Self {
-            graph: Default::default(),
+            tx_graph: Default::default(),
             indexer,
         }
     }
 }
-
-impl<A, I> AsRef<TxGraph<A>> for IndexedTxGraph<A, I> {
-    fn as_ref(&self) -> &TxGraph<A> {
-        &self.graph
-    }
-}