Various improvements not included in #1203 (new)

crates/chain/src/spk_client.rs

@@ -195,11 +195,10 @@ impl SyncRequest {
         index: &crate::keychain::KeychainTxOutIndex<K>,
         spk_range: impl RangeBounds<K>,
     ) -> Self {
-        use alloc::borrow::ToOwned;
         self.chain_spks(
             index
                 .revealed_spks(spk_range)
-                .map(|(_, _, spk)| spk.to_owned())
+                .map(|(_, _, spk)| spk)
                 .collect::<Vec<_>>(),
         )
     }

crates/chain/src/spk_txout_index.rs

@@ -174,8 +174,8 @@ impl<I: Clone + Ord> SpkTxOutIndex<I> {
     /// Returns the script that has been inserted at the `index`.
     ///
     /// If that index hasn't been inserted yet, it will return `None`.
-    pub fn spk_at_index(&self, index: &I) -> Option<&Script> {
-        self.spks.get(index).map(|s| s.as_script())
+    pub fn spk_at_index(&self, index: &I) -> Option<ScriptBuf> {
+        self.spks.get(index).cloned()
     }
 
     /// The script pubkeys that are being tracked by the index.
@@ -215,7 +215,10 @@ impl<I: Clone + Ord> SpkTxOutIndex<I> {
     /// let unused_change_spks =
     ///     txout_index.unused_spks((change_index, u32::MIN)..(change_index, u32::MAX));
     /// ```
-    pub fn unused_spks<R>(&self, range: R) -> impl DoubleEndedIterator<Item = (&I, &Script)> + Clone
+    pub fn unused_spks<R>(
+        &self,
+        range: R,
+    ) -> impl DoubleEndedIterator<Item = (&I, ScriptBuf)> + Clone
     where
         R: RangeBounds<I>,
     {

crates/chain/tests/test_indexed_tx_graph.rs

@@ -150,22 +150,19 @@ fn test_list_owned_txouts() {
 
     {
         // we need to scope here to take immutanble reference of the graph
-        for _ in 0..10 {
-            let ((_, script), _) = graph
-                .index
-                .reveal_next_spk(&"keychain_1".to_string())
-                .unwrap();
-            // TODO Assert indexes
-            trusted_spks.push(script.to_owned());
+        for exp_i in 0_u32..10 {
+            let (next_spk, _) = graph.index.reveal_next_spk(&"keychain_1".to_string());
+            let (spk_i, spk) = next_spk.expect("must exist");
+            assert_eq!(spk_i, exp_i);
+            trusted_spks.push(spk);
         }
     }
     {
-        for _ in 0..10 {
-            let ((_, script), _) = graph
-                .index
-                .reveal_next_spk(&"keychain_2".to_string())
-                .unwrap();
-            untrusted_spks.push(script.to_owned());
+        for exp_i in 0_u32..10 {
+            let (next_spk, _) = graph.index.reveal_next_spk(&"keychain_2".to_string());
+            let (spk_i, spk) = next_spk.expect("must exist");
+            assert_eq!(spk_i, exp_i);
+            untrusted_spks.push(spk);
         }
     }
 

crates/chain/tests/test_keychain_txout_index.rs

@@ -106,8 +106,8 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
     assert_eq!(
         txout_index.keychains().collect::<Vec<_>>(),
         vec![
-            (&TestKeychain::External, &external_descriptor),
-            (&TestKeychain::Internal, &internal_descriptor)
+            (TestKeychain::External, &external_descriptor),
+            (TestKeychain::Internal, &internal_descriptor)
         ]
     );
 
@@ -120,8 +120,8 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
     assert_eq!(
         txout_index.keychains().collect::<Vec<_>>(),
         vec![
-            (&TestKeychain::External, &internal_descriptor),
-            (&TestKeychain::Internal, &internal_descriptor)
+            (TestKeychain::External, &internal_descriptor),
+            (TestKeychain::Internal, &internal_descriptor)
         ]
     );
 
@@ -134,8 +134,8 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
     assert_eq!(
         txout_index.keychains().collect::<Vec<_>>(),
         vec![
-            (&TestKeychain::External, &internal_descriptor),
-            (&TestKeychain::Internal, &external_descriptor)
+            (TestKeychain::External, &internal_descriptor),
+            (TestKeychain::Internal, &external_descriptor)
         ]
     );
 }
@@ -186,9 +186,9 @@ fn test_lookahead() {
     // - scripts cached in spk_txout_index should increase correctly
     // - stored scripts of external keychain should be of expected counts
     for index in (0..20).skip_while(|i| i % 2 == 1) {
-        let (revealed_spks, revealed_changeset) = txout_index
-            .reveal_to_target(&TestKeychain::External, index)
-            .unwrap();
+        let (revealed_spks, revealed_changeset) =
+            txout_index.reveal_to_target(&TestKeychain::External, index);
+        let revealed_spks = revealed_spks.expect("must exist");
         assert_eq!(
             revealed_spks.collect::<Vec<_>>(),
             vec![(index, spk_at_index(&external_descriptor, index))],
@@ -237,9 +237,9 @@ fn test_lookahead() {
     // - derivation index is set ahead of current derivation index + lookahead
     // expect:
     // - scripts cached in spk_txout_index should increase correctly, a.k.a. no scripts are skipped
-    let (revealed_spks, revealed_changeset) = txout_index
-        .reveal_to_target(&TestKeychain::Internal, 24)
-        .unwrap();
+    let (revealed_spks, revealed_changeset) =
+        txout_index.reveal_to_target(&TestKeychain::Internal, 24);
+    let revealed_spks = revealed_spks.expect("must exist");
     assert_eq!(
         revealed_spks.collect::<Vec<_>>(),
         (0..=24)
@@ -333,8 +333,7 @@ fn test_lookahead() {
 fn test_scan_with_lookahead() {
     let external_descriptor = parse_descriptor(DESCRIPTORS[0]);
     let internal_descriptor = parse_descriptor(DESCRIPTORS[1]);
-    let mut txout_index =
-        init_txout_index(external_descriptor.clone(), internal_descriptor.clone(), 10);
+    let mut txout_index = init_txout_index(external_descriptor.clone(), internal_descriptor, 10);
 
     let spks: BTreeMap<u32, ScriptBuf> = [0, 10, 20, 30]
         .into_iter()
@@ -390,7 +389,7 @@ fn test_scan_with_lookahead() {
 fn test_wildcard_derivations() {
     let external_descriptor = parse_descriptor(DESCRIPTORS[0]);
     let internal_descriptor = parse_descriptor(DESCRIPTORS[1]);
-    let mut txout_index = init_txout_index(external_descriptor.clone(), internal_descriptor.clone(), 0);
+    let mut txout_index = init_txout_index(external_descriptor.clone(), internal_descriptor, 0);
     let external_spk_0 = external_descriptor.at_derivation_index(0).unwrap().script_pubkey();
     let external_spk_16 = external_descriptor.at_derivation_index(16).unwrap().script_pubkey();
     let external_spk_26 = external_descriptor.at_derivation_index(26).unwrap().script_pubkey();
@@ -403,11 +402,11 @@ fn test_wildcard_derivations() {
     // - derive_new() == ((0, <spk>), keychain::ChangeSet)
     // - next_unused() == ((0, <spk>), keychain::ChangeSet:is_empty())
     assert_eq!(txout_index.next_index(&TestKeychain::External).unwrap(), (0, true));
-    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External).unwrap();
-    assert_eq!(spk, (0_u32, external_spk_0.as_script()));
+    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0_u32, external_spk_0.clone())));
     assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 0)].into());
-    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap();
-    assert_eq!(spk, (0_u32, external_spk_0.as_script()));
+    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0_u32, external_spk_0)));
     assert_eq!(&changeset.last_revealed, &[].into());
 
     // - derived till 25
@@ -426,13 +425,13 @@ fn test_wildcard_derivations() {
 
     assert_eq!(txout_index.next_index(&TestKeychain::External).unwrap(), (26, true));
 
-    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External).unwrap();
-    assert_eq!(spk, (26, external_spk_26.as_script()));
+    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((26, external_spk_26)));
 
     assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 26)].into());
 
-    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap();
-    assert_eq!(spk, (16, external_spk_16.as_script()));
+    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((16, external_spk_16)));
     assert_eq!(&changeset.last_revealed, &[].into());
 
     // - Use all the derived till 26.
@@ -441,8 +440,8 @@ fn test_wildcard_derivations() {
         txout_index.mark_used(TestKeychain::External, index);
     });
 
-    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap();
-    assert_eq!(spk, (27, external_spk_27.as_script()));
+    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((27, external_spk_27)));
     assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 27)].into());
 }
 
@@ -470,19 +469,15 @@ fn test_non_wildcard_derivations() {
         txout_index.next_index(&TestKeychain::External).unwrap(),
         (0, true)
     );
-    let (spk, changeset) = txout_index
-        .reveal_next_spk(&TestKeychain::External)
-        .unwrap();
-    assert_eq!(spk, (0, external_spk.as_script()));
+    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0, external_spk.clone())));
     assert_eq!(
         &changeset.last_revealed,
         &[(no_wildcard_descriptor.descriptor_id(), 0)].into()
     );
 
-    let (spk, changeset) = txout_index
-        .next_unused_spk(&TestKeychain::External)
-        .unwrap();
-    assert_eq!(spk, (0, external_spk.as_script()));
+    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0, external_spk.clone())));
     assert_eq!(&changeset.last_revealed, &[].into());
 
     // given:
@@ -497,21 +492,16 @@ fn test_non_wildcard_derivations() {
     );
     txout_index.mark_used(TestKeychain::External, 0);
 
-    let (spk, changeset) = txout_index
-        .reveal_next_spk(&TestKeychain::External)
-        .unwrap();
-    assert_eq!(spk, (0, external_spk.as_script()));
+    let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0, external_spk.clone())));
     assert_eq!(&changeset.last_revealed, &[].into());
 
-    let (spk, changeset) = txout_index
-        .next_unused_spk(&TestKeychain::External)
-        .unwrap();
-    assert_eq!(spk, (0, external_spk.as_script()));
+    let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External);
+    assert_eq!(spk, Some((0, external_spk)));
     assert_eq!(&changeset.last_revealed, &[].into());
-    let (revealed_spks, revealed_changeset) = txout_index
-        .reveal_to_target(&TestKeychain::External, 200)
-        .unwrap();
-    assert_eq!(revealed_spks.count(), 0);
+    let (revealed_spks, revealed_changeset) =
+        txout_index.reveal_to_target(&TestKeychain::External, 200);
+    assert_eq!(revealed_spks.map(Iterator::count), Some(0));
     assert!(revealed_changeset.is_empty());
 
     // we check that spks_of_keychain returns a SpkIterator with just one element
@@ -646,7 +636,7 @@ fn index_txout_after_changing_descriptor_under_keychain() {
 
     // Introduce `desc_a` under keychain `()` and replace the descriptor.
     let _ = txout_index.insert_descriptor((), desc_a.clone());
-    let _ = txout_index.insert_descriptor((), desc_b.clone());
+    let _ = txout_index.insert_descriptor((), desc_b);
 
     // Loop through spks in intervals of `lookahead` to create outputs with. We should always be
     // able to index these outputs if `lookahead` is respected.
@@ -689,7 +679,7 @@ fn insert_descriptor_no_change() {
         },
     );
     assert_eq!(
-        txout_index.insert_descriptor((), desc.clone()),
+        txout_index.insert_descriptor((), desc),
         keychain::ChangeSet::default(),
         "inserting the same descriptor for keychain should return an empty changeset",
     );
@@ -754,18 +744,16 @@ fn test_only_highest_ord_keychain_is_returned() {
     let _ = indexer.insert_descriptor(TestKeychain::External, desc);
 
     // reveal_next_spk will work with either keychain
-    let spk0: ScriptBuf = indexer
-        .reveal_next_spk(&TestKeychain::External)
-        .unwrap()
-        .0
-         .1
-        .into();
-    let spk1: ScriptBuf = indexer
-        .reveal_next_spk(&TestKeychain::Internal)
-        .unwrap()
-        .0
-         .1
-        .into();
+    let spk0 = {
+        let (spk, _) = indexer.reveal_next_spk(&TestKeychain::External);
+        let (_, spk) = spk.expect("must exist");
+        spk
+    };
+    let spk1 = {
+        let (spk, _) = indexer.reveal_next_spk(&TestKeychain::Internal);
+        let (_, spk) = spk.expect("must exist");
+        spk
+    };
 
     // index_of_spk will always return External
     assert_eq!(

crates/persist/src/changeset.rs

@@ -52,6 +52,15 @@ impl<K: Ord, A: Anchor> Append for CombinedChangeSet<K, A> {
     }
 }
 
+impl<K, A> From<keychain::ChangeSet<K>> for CombinedChangeSet<K, A> {
+    fn from(keychain_changeset: keychain::ChangeSet<K>) -> Self {
+        Self {
+            indexed_tx_graph: keychain_changeset.into(),
+            ..Default::default()
+        }
+    }
+}
+
 impl<K, A> From<local_chain::ChangeSet> for CombinedChangeSet<K, A> {
     fn from(chain: local_chain::ChangeSet) -> Self {
         Self {