refactor(wallet)!: Replace new_or_load() with load_with_descriptors()

Updated load_with_descriptors() requires a ChangeSet and descriptors, validates they match. If the given descriptors have private keys signers are added. Also rename load_from_changeset() to load() and remove no longer needed NewOrLoadError.
notmandatory · Jul 12, 2024 · fa2f2ab · fa2f2ab
1 parent d99b3ef
commit fa2f2ab
Show file tree

Hide file tree

Showing 7 changed files with 159 additions and 296 deletions.
diff --git a/crates/wallet/README.md b/crates/wallet/README.md
@@ -79,9 +79,11 @@ let mut db =
 let descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/0/*)";
 let change_descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/1/*)";
 let changeset = db.aggregate_changesets().expect("changeset loaded");
-let mut wallet =
-    Wallet::new_or_load(descriptor, change_descriptor, changeset, Network::Testnet)
-        .expect("create or load wallet");
+let mut wallet = if let Some(changeset) = changeset {
+    Wallet::load(changeset).expect("loaded wallet")
+} else {
+    Wallet::new(descriptor, change_descriptor, Network::Testnet).expect("created new wallet")
+};
 
 // Get a new address to receive bitcoin.
 let receive_address = wallet.reveal_next_address(KeychainKind::External);

diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs
diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs
@@ -141,7 +141,7 @@ fn load_recovers_wallet() -> anyhow::Result<()> {
             let db = &mut recover(&file_path).expect("must recover db");
             let changeset = read(db).expect("must recover wallet").expect("changeset");
 
-            let wallet = Wallet::load_from_changeset(changeset).expect("must recover wallet");
+            let wallet = Wallet::load(changeset).expect("must recover wallet");
             assert_eq!(wallet.network(), Network::Testnet);
             assert_eq!(
                 wallet.spk_index().keychains().collect::<Vec<_>>(),
@@ -182,78 +182,32 @@ fn load_recovers_wallet() -> anyhow::Result<()> {
 }
 
 #[test]
-fn new_or_load() -> anyhow::Result<()> {
-    fn run<Db, NewOrRecover, Read, Write>(
+fn new_and_load() -> anyhow::Result<()> {
+    fn run<Db, Recover, Read, Write>(
         filename: &str,
-        new_or_load: NewOrRecover,
+        load: Recover,
         read: Read,
         write: Write,
     ) -> anyhow::Result<()>
     where
-        NewOrRecover: Fn(&Path) -> anyhow::Result<Db>,
+        Recover: Fn(&Path) -> anyhow::Result<Db>,
         Read: Fn(&mut Db) -> anyhow::Result<Option<ChangeSet>>,
         Write: Fn(&mut Db, &ChangeSet) -> anyhow::Result<()>,
     {
         let temp_dir = tempfile::tempdir().expect("must create tempdir");
         let file_path = temp_dir.path().join(filename);
         let (desc, change_desc) = get_test_wpkh_with_change_desc();
 
-        // init wallet when non-existent
+        // init wallet
         let wallet_keychains: BTreeMap<_, _> = {
-            let wallet = &mut Wallet::new_or_load(desc, change_desc, None, Network::Testnet)
-                .expect("must init wallet");
-            let mut db = new_or_load(&file_path).expect("must create db");
-            if let Some(changeset) = wallet.take_staged() {
-                write(&mut db, &changeset)?;
-            }
+            let wallet =
+                &mut Wallet::new(desc, change_desc, Network::Testnet).expect("must init wallet");
+            let changeset = wallet.take_staged().expect("must have changeset");
+            let db = &mut load(&file_path).expect("must open db");
+            write(db, &changeset)?;
             wallet.keychains().map(|(k, v)| (*k, v.clone())).collect()
         };
 
-        // wrong network
-        {
-            let mut db = new_or_load(&file_path).expect("must create db");
-            let changeset = read(&mut db)?;
-            let err = Wallet::new_or_load(desc, change_desc, changeset, Network::Bitcoin)
-                .expect_err("wrong network");
-            assert!(
-                matches!(
-                    err,
-                    bdk_wallet::wallet::NewOrLoadError::LoadedNetworkDoesNotMatch {
-                        got: Some(Network::Testnet),
-                        expected: Network::Bitcoin
-                    }
-                ),
-                "err: {}",
-                err,
-            );
-        }
-
-        // wrong genesis hash
-        {
-            let exp_blockhash = BlockHash::all_zeros();
-            let got_blockhash = bitcoin::constants::genesis_block(Network::Testnet).block_hash();
-
-            let db = &mut new_or_load(&file_path).expect("must open db");
-            let changeset = read(db)?;
-            let err = Wallet::new_or_load_with_genesis_hash(
-                desc,
-                change_desc,
-                changeset,
-                Network::Testnet,
-                exp_blockhash,
-            )
-            .expect_err("wrong genesis hash");
-            assert!(
-                matches!(
-                    err,
-                    bdk_wallet::wallet::NewOrLoadError::LoadedGenesisDoesNotMatch { got, expected }
-                    if got == Some(got_blockhash) && expected == exp_blockhash
-                ),
-                "err: {}",
-                err,
-            );
-        }
-
         // wrong external descriptor
         {
             let (exp_descriptor, exp_change_desc) = get_test_tr_single_sig_xprv_with_change_desc();
@@ -262,15 +216,14 @@ fn new_or_load() -> anyhow::Result<()> {
                 .unwrap()
                 .0;
 
-            let db = &mut new_or_load(&file_path).expect("must open db");
-            let changeset = read(db)?;
-            let err =
-                Wallet::new_or_load(exp_descriptor, exp_change_desc, changeset, Network::Testnet)
-                    .expect_err("wrong external descriptor");
+            let db = &mut load(&file_path).expect("must open db");
+            let changeset = read(db)?.expect("changeset must exist");
+            let err = Wallet::load_with_descriptors(exp_descriptor, exp_change_desc, changeset)
+                .expect_err("wrong external descriptor");
             assert!(
                 matches!(
                     err,
-                    bdk_wallet::wallet::NewOrLoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
+                    bdk_wallet::wallet::LoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
                     if got == &Some(got_descriptor) && keychain == KeychainKind::External
                 ),
                 "err: {}",
@@ -286,14 +239,14 @@ fn new_or_load() -> anyhow::Result<()> {
                 .unwrap()
                 .0;
 
-            let db = &mut new_or_load(&file_path).expect("must open db");
-            let changeset = read(db)?;
-            let err = Wallet::new_or_load(desc, exp_descriptor, changeset, Network::Testnet)
+            let db = &mut load(&file_path).expect("must open db");
+            let changeset = read(db)?.expect("changeset must exist");
+            let err = Wallet::load_with_descriptors(desc, exp_descriptor, changeset)
                 .expect_err("wrong internal descriptor");
             assert!(
                 matches!(
                     err,
-                    bdk_wallet::wallet::NewOrLoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
+                    bdk_wallet::wallet::LoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
                     if got == &Some(got_descriptor) && keychain == KeychainKind::Internal
                 ),
                 "err: {}",
@@ -303,16 +256,33 @@ fn new_or_load() -> anyhow::Result<()> {
 
         // all parameters match
         {
-            let db = &mut new_or_load(&file_path).expect("must open db");
-            let changeset = read(db)?;
-            let wallet = Wallet::new_or_load(desc, change_desc, changeset, Network::Testnet)
+            let db = &mut load(&file_path).expect("must open db");
+            let changeset = read(db)?.expect("changeset must exist");
+            let wallet = Wallet::load_with_descriptors(desc, change_desc, changeset)
                 .expect("must recover wallet");
             assert_eq!(wallet.network(), Network::Testnet);
             assert!(wallet
                 .keychains()
                 .map(|(k, v)| (*k, v.clone()))
                 .eq(wallet_keychains));
         }
+
+        // signers added
+        {
+            let db = &mut load(&file_path).expect("must open db");
+            let changeset = read(db)?.expect("changeset must exist");
+            let wallet = Wallet::load_with_descriptors(desc, change_desc, changeset)
+                .expect("must recover wallet");
+            assert_eq!(
+                wallet.get_signers(KeychainKind::External).signers().len(),
+                1
+            );
+            assert_eq!(
+                wallet.get_signers(KeychainKind::Internal).signers().len(),
+                1
+            );
+        }
+
         Ok(())
     }
 

diff --git a/example-crates/wallet_electrum/src/main.rs b/example-crates/wallet_electrum/src/main.rs
@@ -24,12 +24,11 @@ fn main() -> Result<(), anyhow::Error> {
     let changeset = db
         .aggregate_changesets()
         .map_err(|e| anyhow!("load changes error: {}", e))?;
-    let mut wallet = Wallet::new_or_load(
-        external_descriptor,
-        internal_descriptor,
-        changeset,
-        Network::Testnet,
-    )?;
+    let mut wallet = if let Some(changeset) = changeset {
+        Wallet::load_with_descriptors(external_descriptor, internal_descriptor, changeset)?
+    } else {
+        Wallet::new(external_descriptor, internal_descriptor, Network::Testnet)?
+    };
 
     let address = wallet.next_unused_address(KeychainKind::External);
     if let Some(changeset) = wallet.take_staged() {

diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs
@@ -21,12 +21,11 @@ async fn main() -> Result<(), anyhow::Error> {
     let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)";
     let changeset = db.read()?;
 
-    let mut wallet = Wallet::new_or_load(
-        external_descriptor,
-        internal_descriptor,
-        changeset,
-        Network::Signet,
-    )?;
+    let mut wallet = if let Some(changeset) = changeset {
+        Wallet::load_with_descriptors(external_descriptor, internal_descriptor, changeset)?
+    } else {
+        Wallet::new(external_descriptor, internal_descriptor, Network::Testnet)?
+    };
 
     let address = wallet.next_unused_address(KeychainKind::External);
     if let Some(changeset) = wallet.take_staged() {

diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs
@@ -20,12 +20,11 @@ fn main() -> Result<(), anyhow::Error> {
     let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)";
     let changeset = db.aggregate_changesets()?;
 
-    let mut wallet = Wallet::new_or_load(
-        external_descriptor,
-        internal_descriptor,
-        changeset,
-        Network::Testnet,
-    )?;
+    let mut wallet = if let Some(changeset) = changeset {
+        Wallet::load_with_descriptors(external_descriptor, internal_descriptor, changeset)?
+    } else {
+        Wallet::new(external_descriptor, internal_descriptor, Network::Testnet)?
+    };
 
     let address = wallet.next_unused_address(KeychainKind::External);
     if let Some(changeset) = wallet.take_staged() {

diff --git a/example-crates/wallet_rpc/src/main.rs b/example-crates/wallet_rpc/src/main.rs
@@ -92,12 +92,12 @@ fn main() -> anyhow::Result<()> {
     )?;
     let changeset = db.aggregate_changesets()?;
 
-    let mut wallet = Wallet::new_or_load(
-        &args.descriptor,
-        &args.change_descriptor,
-        changeset,
-        args.network,
-    )?;
+    let mut wallet = if let Some(changeset) = changeset {
+        Wallet::load_with_descriptors(&args.descriptor, &args.change_descriptor, changeset)?
+    } else {
+        Wallet::new(&args.descriptor, &args.change_descriptor, Network::Testnet)?
+    };
+
     println!(
         "Loaded wallet in {}s",
         start_load_wallet.elapsed().as_secs_f32()