fix(chain): let the indexer decide how far to rescan

crates/chain/src/indexed_tx_graph.rs

@@ -145,18 +145,16 @@ where
 
     /// Synchronizes the indexer to reflect every entry in the transaction graph.
     ///
-    /// Iterates over **all** full transactions and floating outputs in `self.graph`, passing each
-    /// into `self.index`. Any indexer-side changes produced (via `index_tx` or `index_txout`) are
-    /// merged into a fresh `ChangeSet`, which is then returned.
+    /// Hands the whole graph to [`Indexer::rescan`] and returns the indexer-side changes it
+    /// produces. How thoroughly the graph is walked is the indexer's decision: one whose set of
+    /// recognized outputs grows as it matches — `KeychainTxOutIndex` extends its lookahead past
+    /// each newly revealed index — has to look more than once to reach everything it can, and it is
+    /// the only party that knows when it is done.
     pub fn reindex(&mut self) -> ChangeSet<A, I::ChangeSet> {
-        let mut changeset = ChangeSet::<A, I::ChangeSet>::default();
-        for tx in self.graph.full_txs() {
-            changeset.indexer.merge(self.index.index_tx(&tx));
-        }
-        for (op, txout) in self.graph.floating_txouts() {
-            changeset.indexer.merge(self.index.index_txout(op, txout));
+        ChangeSet {
+            tx_graph: Default::default(),
+            indexer: self.index.rescan(&self.graph),
         }
-        changeset
     }
 
     fn index_tx_graph_changeset(

crates/chain/src/indexer.rs

@@ -2,6 +2,8 @@
 
 use bitcoin::{OutPoint, Transaction, TxOut};
 
+use crate::{tx_graph::TxGraph, Merge};
+
 #[cfg(feature = "miniscript")]
 pub mod keychain_txout;
 pub mod spk_txout;
@@ -30,4 +32,27 @@ pub trait Indexer {
 
     /// Determines whether the transaction should be included in the index.
     fn is_tx_relevant(&self, tx: &Transaction) -> bool;
+
+    /// Index everything in `graph` that this indexer has not already accounted for.
+    ///
+    /// The default implementation offers every full transaction and floating output to
+    /// [`index_tx`](Self::index_tx) and [`index_txout`](Self::index_txout) exactly once, which is
+    /// all an indexer needs when what it recognizes is fixed up front.
+    ///
+    /// Override this when a match can *widen* what the indexer recognizes, so that an output
+    /// offered earlier in the walk could match on a later look. Only the indexer knows when its
+    /// recognition set has stopped growing, so only the indexer can decide when to stop looking.
+    fn rescan<A>(&mut self, graph: &TxGraph<A>) -> Self::ChangeSet
+    where
+        Self::ChangeSet: Merge,
+    {
+        let mut changeset = Self::ChangeSet::default();
+        for tx in graph.full_txs() {
+            changeset.merge(self.index_tx(&tx));
+        }
+        for (op, txout) in graph.floating_txouts() {
+            changeset.merge(self.index_txout(op, txout));
+        }
+        changeset
+    }
 }

crates/chain/src/indexer/keychain_txout.rs

@@ -8,6 +8,7 @@ use crate::{
     spk_client::{FullScanRequestBuilder, SyncRequestBuilder},
     spk_iter::BIP32_MAX_INDEX,
     spk_txout::SpkTxOutIndex,
+    tx_graph::TxGraph,
     DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator,
 };
 use alloc::{borrow::ToOwned, vec::Vec};
@@ -198,6 +199,28 @@ impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
     fn is_tx_relevant(&self, tx: &bitcoin::Transaction) -> bool {
         self.inner.is_relevant(tx)
     }
+
+    /// Looks repeatedly, because a match here widens what the next look can find: revealing an
+    /// index replenishes the lookahead past it, bringing spks into the derived set that outputs
+    /// already walked past may pay to. Looking once would make the outcome depend on the order the
+    /// graph happens to yield its transactions.
+    fn rescan<A>(&mut self, graph: &TxGraph<A>) -> Self::ChangeSet {
+        let mut changeset = ChangeSet::default();
+        loop {
+            // The frontier, not `changeset.is_empty()`: the changeset also carries staged spk cache
+            // entries, which move without the frontier moving and would buy a pointless extra look.
+            let frontier = self.last_revealed.clone();
+            for tx in graph.full_txs() {
+                changeset.merge(self.index_tx(&tx));
+            }
+            for (op, txout) in graph.floating_txouts() {
+                changeset.merge(self.index_txout(op, txout));
+            }
+            if self.last_revealed == frontier {
+                return changeset;
+            }
+        }
+    }
 }
 
 impl<K> KeychainTxOutIndex<K> {

crates/chain/tests/test_indexed_tx_graph.rs

@@ -10,7 +10,7 @@ use bdk_chain::{
     indexer::keychain_txout::KeychainTxOutIndex,
     local_chain::LocalChain,
     spk_txout::SpkTxOutIndex,
-    tx_graph, Balance, ChainPosition, ConfirmationBlockTime, DescriptorExt, SpkIterator,
+    tx_graph, Balance, ChainPosition, ConfirmationBlockTime, DescriptorExt, Merge, SpkIterator,
 };
 use bdk_testenv::{
     anyhow::{self},
@@ -302,6 +302,69 @@ fn insert_relevant_txs() {
     assert_eq!(graph.initial_changeset(), initial_changeset);
 }
 
+/// `reindex` must climb to the highest index the lookahead can reach, not stop at whichever match
+/// it happened to see first.
+///
+/// A match widens the derived window the *remaining* outputs are judged against, so one pass can
+/// miss an output it already walked past. Two transactions would express that too, but the walk
+/// order over the graph is a `HashMap` order — the very thing that is unreliable — so such a test
+/// would pass a single-pass implementation about half the time. One transaction with both outputs
+/// pins it down: `index_tx` walks `tx.output`, a `Vec`, in vout order, so `far` at vout 0 is
+/// always judged against the initial window and always missed, and `near` at vout 1 only then
+/// lifts the frontier that brings `far` into range. Nothing here depends on a hash seed.
+#[test]
+fn reindex_reaches_fixed_point() {
+    let (descriptor, _) = Descriptor::parse_descriptor(&Secp256k1::signing_only(), DESCRIPTORS[0])
+        .expect("must be valid");
+    let did = descriptor.descriptor_id();
+
+    let lookahead = 10;
+    let near = lookahead - 1;
+    let far = lookahead + 5;
+    assert!(
+        far >= lookahead && far < 2 * lookahead,
+        "`far` must be out of the initial window but within the one `near` widens it to",
+    );
+
+    let tx = Transaction {
+        output: [far, near]
+            .iter()
+            .map(|&index| TxOut {
+                value: Amount::from_sat(10_000),
+                script_pubkey: descriptor
+                    .at_derivation_index(index)
+                    .unwrap()
+                    .script_pubkey(),
+            })
+            .collect(),
+        ..new_tx(0)
+    };
+
+    let (mut graph, changeset) =
+        IndexedTxGraph::<ConfirmationBlockTime, KeychainTxOutIndex<()>>::from_changeset(
+            indexed_tx_graph::ChangeSet {
+                tx_graph: tx_graph::ChangeSet {
+                    txs: [Arc::new(tx)].into(),
+                    ..Default::default()
+                },
+                ..Default::default()
+            },
+            |_| -> anyhow::Result<_> {
+                let mut indexer = KeychainTxOutIndex::new(lookahead, true);
+                assert!(indexer.insert_descriptor((), descriptor.clone()).unwrap());
+                Ok(indexer)
+            },
+        )
+        .expect("must construct");
+
+    assert_eq!(graph.index.last_revealed_index(()), Some(far));
+    assert_eq!(changeset.indexer.last_revealed.get(&did), Some(&far));
+    assert!(
+        graph.reindex().is_empty(),
+        "reindexing an already-reindexed graph must find nothing new",
+    );
+}
+
 /// Ensure consistency IndexedTxGraph list_* and balance methods. These methods lists
 /// relevant txouts and utxos from the information fetched from a LocalChain.
 ///