Merge pull request #890 from evanlinjin/simplify-reveal-to-target-logic

Simplify `reveal_to_target` logic

Author: evanlinjin

crates/chain/src/keychain/txout_index.rs

@@ -63,7 +63,7 @@ pub struct KeychainTxOutIndex<K> {
     inner: SpkTxOutIndex<(K, u32)>,
     // descriptors of each keychain
     keychains: BTreeMap<K, Descriptor<DescriptorPublicKey>>,
-    // last stored indexes
+    // last revealed indexes
     last_revealed: BTreeMap<K, u32>,
     // lookahead settings for each keychain
     lookahead: BTreeMap<K, u32>,
@@ -381,40 +381,39 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
         let has_wildcard = descriptor.has_wildcard();
 
         let target_index = if has_wildcard { target_index } else { 0 };
-        let next_store_index = self.next_store_index(keychain);
         let next_reveal_index = self.last_revealed.get(keychain).map_or(0, |v| *v + 1);
         let lookahead = self.lookahead.get(keychain).map_or(0, |v| *v);
 
-        // if we can reveal new indexes, the latest revealed index goes here
-        let mut revealed_index = None;
+        debug_assert_eq!(
+            next_reveal_index + lookahead,
+            self.next_store_index(keychain)
+        );
+
+        // if we need to reveal new indices, the latest revealed index goes here
+        let mut reveal_to_index = None;
 
-        // if the target is already surpassed, we have nothing to reveal
-        if next_reveal_index <= target_index
-            // if the target is already stored (due to lookahead), this can be our newly revealed index
-            && target_index < next_reveal_index + lookahead
-        {
-            revealed_index = Some(target_index);
+        // if the target is not yet revealed, but is already stored (due to lookahead), we need to
+        // set the `reveal_to_index` as target here (as the `for` loop below only updates
+        // `reveal_to_index` for indexes that are NOT stored)
+        if next_reveal_index <= target_index && target_index < next_reveal_index + lookahead {
+            reveal_to_index = Some(target_index);
         }
 
         // we range over indexes that are not stored
         let range = next_reveal_index + lookahead..=target_index + lookahead;
-
         for (new_index, new_spk) in range_descriptor_spks(Cow::Borrowed(descriptor), range) {
-            // no need to store if already stored
-            if new_index >= next_store_index {
-                let _inserted = self
-                    .inner
-                    .insert_spk((keychain.clone(), new_index), new_spk);
-                debug_assert!(_inserted, "must not have existing spk",);
-            }
+            let _inserted = self
+                .inner
+                .insert_spk((keychain.clone(), new_index), new_spk);
+            debug_assert!(_inserted, "must not have existing spk",);
 
             // everything after `target_index` is stored for lookahead only
             if new_index <= target_index {
-                revealed_index = Some(new_index);
+                reveal_to_index = Some(new_index);
             }
         }
 
-        match revealed_index {
+        match reveal_to_index {
             Some(index) => {
                 let _old_index = self.last_revealed.insert(keychain.clone(), index);
                 debug_assert!(_old_index < Some(index));

crates/chain/tests/test_keychain_txout_index.rs

@@ -7,7 +7,7 @@ use bdk_chain::{
     keychain::{DerivationAdditions, KeychainTxOutIndex},
 };
 
-use bitcoin::{secp256k1::Secp256k1, Script, Transaction, TxOut};
+use bitcoin::{secp256k1::Secp256k1, OutPoint, Script, Transaction, TxOut};
 use miniscript::{Descriptor, DescriptorPublicKey};
 
 #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd)]
@@ -210,13 +210,54 @@ fn test_lookahead() {
             last_internal_index as usize + 1,
         );
     }
+}
 
-    // when:
-    // - scanning txouts with spks above last stored index
-    // expect:
-    // - cached scripts count should increase as expected
-    // - last stored index should increase as expected
-    // TODO!
+// when:
+// - scanning txouts with spks above last stored index
+// expect:
+// - last revealed index should increase as expected
+// - last used index should change as expected
+#[test]
+fn test_scan_with_lookahead() {
+    let (mut txout_index, external_desc, _) = init_txout_index();
+    txout_index.set_lookahead_for_all(10);
+
+    let spks: BTreeMap<u32, Script> = [0, 10, 20, 30]
+        .into_iter()
+        .map(|i| (i, external_desc.at_derivation_index(i).script_pubkey()))
+        .collect();
+
+    for (&spk_i, spk) in &spks {
+        let op = OutPoint::new(h!("fake tx"), spk_i);
+        let txout = TxOut {
+            script_pubkey: spk.clone(),
+            value: 0,
+        };
+
+        let additions = txout_index.scan_txout(op, &txout);
+        assert_eq!(
+            additions.as_inner(),
+            &[(TestKeychain::External, spk_i)].into()
+        );
+        assert_eq!(
+            txout_index.last_revealed_index(&TestKeychain::External),
+            Some(spk_i)
+        );
+        assert_eq!(
+            txout_index.last_used_index(&TestKeychain::External),
+            Some(spk_i)
+        );
+    }
+
+    // now try with index 41 (lookahead surpassed), we expect that the txout to not be indexed
+    let spk_41 = external_desc.at_derivation_index(41).script_pubkey();
+    let op = OutPoint::new(h!("fake tx"), 41);
+    let txout = TxOut {
+        script_pubkey: spk_41,
+        value: 0,
+    };
+    let additions = txout_index.scan_txout(op, &txout);
+    assert!(additions.is_empty());
 }
 
 #[test]